linux-kernel - Re: [PATCH v7u1 26/31] x86: Don't enable swiotlb if there is not enough ram for it

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87k3rslg5a.fsf@xmission.com>
Date:	Fri, 04 Jan 2013 14:47:29 -0800
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Shuah Khan <shuahkhan@...il.com>
Cc:	Yinghai Lu <yinghai@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...e.hu>, "H. Peter Anvin" <hpa@...or.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Borislav Petkov <bp@...en8.de>, Jan Kiszka <jan.kiszka@....de>,
	Jason Wessel <jason.wessel@...driver.com>,
	linux-kernel@...r.kernel.org,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Joerg Roedel <joro@...tes.org>
Subject: Re: [PATCH v7u1 26/31] x86: Don't enable swiotlb if there is not enough ram for it

Shuah Khan <shuahkhan@...il.com> writes:

> On Fri, Jan 4, 2013 at 3:10 PM, Yinghai Lu <yinghai@...nel.org> wrote:
>> On Fri, Jan 4, 2013 at 1:02 PM, Shuah Khan <shuahkhan@...il.com> wrote:
>>> Pani'cing the system doesn't sound like a good option to me in this
>>> case. This change to disable swiotlb is made for kdump. However, with
>>> this change several system fail to boot, unless crashkernel_low=72M is
>>> specified.
>>
>> this patchset is new feature to put second kdump kernel above 4G.
>>
> I understand this is just one of the patches to implement the new
> kdump feature. However, I think regression on existing behavior with a
> panic is a bit of a big hammer. Thie change causes panic on systems
> even when kdump is not enabled, if I understand it correctly.
>
> Granted kdump gets enabled by several distros, but it is not a
> required feature. However, expecting system to boot with devices that
> require swiotlb fully functioning is a basic feature. So I would argue
> that not breaking the basic functionality is a higher priority over
> enabling kdump in this case.

Yinghai Lu it looks like your autodetection of the problem case in this
patch is problematic and needs a rethink.  My quick skim says you are
trying to detect failure too early in the code.  Furthermore having
kexec on panic sized magic comments without explanation is wrong.

Shuah Khan this is motivated by kdump.  However a correct implementation
should be about dealing with the case when there is simply not enough
memory available below 4G for bounce buffers.

If a device needs an iommu, and swiotlb is the only iommu option, and 
there is not enough memory below 4G panic'ing is entirely reasonable.

Do I read this discussion right that we are waisting 64M on systems
that have the swiotlb code but don't use the swiotlb? 

Eric

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/