linux-kernel - Re: Workaround for Intel MPS errata

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]

Date:	Fri, 30 Sep 2011 12:38:55 -0500
From:	Jon Mason <mason@...i.com>
To:	Bjorn Helgaas <bhelgaas@...gle.com>
Cc:	Avi Kivity <avi@...hat.com>, Sven Schnelle <svens@...ckframe.org>,
	Simon Kirby <sim@...tway.ca>,
	Eric Dumazet <eric.dumazet@...il.com>,
	Niels Ole Salscheider <niels_ole@...scheider-online.de>,
	Jesse Barnes <jbarnes@...tuousgeek.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	Ben Hutchings <bhutchings@...arflare.com>
Subject: Re: Workaround for Intel MPS errata

On Fri, Sep 30, 2011 at 12:17 PM, Bjorn Helgaas <bhelgaas@...gle.com> wrote:
> On Fri, Sep 30, 2011 at 9:35 AM, Jon Mason <mason@...i.com> wrote:
>>
>> On Fri, Sep 30, 2011 at 12:01 AM, Bjorn Helgaas <bhelgaas@...gle.com> wrote:
>> > On Thu, Sep 29, 2011 at 6:16 PM, Jon Mason <mason@...i.com> wrote:
>> >> Hey Avi,
>> >> Can you try this patch?  It should resolve the issue you are seeing.
>> >>
>> >> Thanks,
>> >> Jon
>> >>
>> >>    PCI: Workaround for Intel MPS errata
>> >>
>> >>    Intel 5000 and 5100 series memory controllers have a known issue if read
>> >>    completion coalescing is enabled (the default setting) and the PCI-E
>> >>    Maximum Payload Size is set to 256B.  To work around this issue, disable
>> >>    read completion coalescing if the MPS is 256B.
>> >
>> > I'd much rather see this done as an early quirk so it doesn't clutter probe.c.
>> >
>> > I don't know how you decide whether
>> >    - no coalescing with MPS=256, or
>> >    - coalescing with MPS=128
>> > is better.  I suspect that having a quirk that doesn't change the
>> > setting, but merely limits MPS to 128 if the BIOS enabled coalescing,
>> > would be simplest and would stay in the best-tested chipset
>> > configuration.
>>
>> This is what I was debating yesterday.  Is it better to disable
>> coalescing and get better throughput (which could be a net negative if
>> the MPS isn't 256) or never allow it to be greater than 128?  There is
>> no way of knowing at quirk time if the disable is necessary or not,
>> only when setting the MPS is it known (which is why I did it this
>> way).  I could, as you suggest, simply read the bit and see if it is
>> enabled by the BIOS (which I'd bet it is every single time), and then
>> limit the MPS to 128 as a quirk.  This would be fairly simple to do.
>> However, the errata from Intel says Windows 2008 always disables the
>> coalescing and sets the MPS to 256B.  With this known, Linux's I/O
>> performance would be less than Windows on these systems. ...
>
> Presumably coalescing improves performance, too, and I don't have the
> evidence that says "no coalescing with MPS=256" performs better than
> "coalescing with MPS=128."
>
> But the fact that Windows 2008 disables coalescing is worth a lot (if
> this is in a public erratum, a URL would be good).  Given that, I'd

The URLs were in the top of the patch, but perhaps they weren't obvious.

http://www.intel.com/content/dam/doc/specification-update/5000-chipset-memory-controller-hub-specification-update.pdf
http://www.intel.com/content/dam/doc/specification-update/5100-memory-controller-hub-chipset-specification-update.pdf

Search for "MPS", as they are several pages into the errata chapter.

> probably go with "no coalescing and MPS=256" just as you did.
>
> Maybe the quirk could be moved out of the generic code by making
> pcie_set_mps() a weak function, so x86 could supply a version that
> disables coalescing if MPS=256?

Not sure what you mean here.  Are you saying to make the function
defined differently on each arch?

> No news from Avi?  Were you able to reproduce the problem and verify

None so far.  I believe he is in .il, so we might not hear back until Sunday.

> that the quirk fixes it?  I wish the kernel.org bugzilla were back.

I just found a system to try it on!  It has an Intel Corporation 5000X
Chipset Memory Controller Hub, which should have the erratum.

> Since it's not, maybe we should include the LKML URL
> (https://lkml.org/lkml/2011/9/27/274) in the changelog.

Will do.

>
> Bjorn
>
--
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/