All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jon Mason <mason@myri.com>
To: Jesse Brandeburg <jesse.brandeburg@gmail.com>
Cc: Avi Kivity <avi@redhat.com>, Sven Schnelle <svens@stackframe.org>,
	Simon Kirby <sim@hostway.ca>,
	Eric Dumazet <eric.dumazet@gmail.com>,
	Niels Ole Salscheider <niels_ole@salscheider-online.de>,
	Jesse Barnes <jbarnes@virtuousgeek.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	Ben Hutchings <bhutchings@solarflare.com>,
	Jesse Brandeburg <jesse.brandeburg@intel.com>
Subject: Re: Workaround for Intel MPS errata
Date: Thu, 29 Sep 2011 21:51:28 -0500	[thread overview]
Message-ID: <CAPoiz9wR9mUHdCBDOV+dOjbWiY48zQDyPUeAFv-70YneLMT1Uw@mail.gmail.com> (raw)
In-Reply-To: <CAEuXFEzESMrFLeTOq33RVQ7W3vsKuqKAuz8mx2o9K6QgBJZVnQ@mail.gmail.com>

On Thu, Sep 29, 2011 at 9:21 PM, Jesse Brandeburg
<jesse.brandeburg@gmail.com> wrote:
> On Thu, Sep 29, 2011 at 5:16 PM, Jon Mason <mason@myri.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.
>
> Hey Jon, glad I could help out by pointing this erratum out on IRC
> today.  The patch looks mostly fine, with one nit, see below.

Sorry, I thought I gave you and Ben props in the commit log.  That
will be corrected in the version I push (assuming it fixes the issue).
 You really saved my rear :)

>> +       /* Disable read completion coalescing to allow an MPS of 256 */
>> +       if (mps == 256) {
>> +               int err;
>> +               u16 rcc;
>> +
>> +               /* Intel errata specifies bits to change but does not say what
>> +                * they are.  Keeping them magical until such time as the
>> +                * registers and values can be explained.
>> +                */
>> +               err = pci_read_config_word(dev, 0x48, &rcc);
>> +               if (err) {
>> +                       dev_err(&dev->dev, "Error attempting to read the read "
>> +                               "completion coalescing register.\n");
>> +                       return;
>> +               }
>> +
>> +               rcc &= ~(1 << 10);
>> +
>> +               err = pci_write_config_word(dev, 0x48, rcc);
>> +               if (err) {
>> +                       dev_err(&dev->dev, "Error attempting to read the read "
>
> this should be "to write the read "

Good catch.

>
>> +                               "completion coalescing register.\n");
>> +                       return;
>> +               }
>
> do you have to do anything to change the bit back to 1 if something
> sets the mps back to 128?

The only time this would really be called is at boot time.  Its
possible that someone could hotplug an adapter with 256, then remove
it (and thereby reduce the performance).  The problem is that nothing
is called to reset the MPS when a device is hotplug removed, and it
seems like overkill to add a callback only for this.  However, it
should be documented in the code that the hole exists.

Thanks,
Jon

> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

  reply	other threads:[~2011-09-30  2:51 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-27 17:01 [REGRESSION] e1000e failure triggered by "PCI: Remove MRRS modification from MPS setting code" Avi Kivity
2011-09-27 17:59 ` Jon Mason
2011-09-27 18:28   ` Avi Kivity
2011-09-27 20:11     ` Jon Mason
2011-09-29  4:33       ` Benjamin Herrenschmidt
2011-09-29 13:53         ` Jon Mason
2011-09-30  0:16     ` Workaround for Intel MPS errata Jon Mason
2011-09-30  2:21       ` Jesse Brandeburg
2011-09-30  2:51         ` Jon Mason [this message]
2011-09-30  5:01       ` Bjorn Helgaas
2011-09-30 15:35         ` Jon Mason
2011-09-30 17:17           ` Bjorn Helgaas
2011-09-30 17:38             ` Jon Mason
2011-09-30 17:57               ` Bjorn Helgaas
2011-09-30  7:03       ` Rolf Eike Beer
2011-09-30 15:39         ` Jon Mason
2011-10-02  9:26       ` Avi Kivity
2011-10-03  4:58         ` Jon Mason
2011-10-03 10:11           ` Avi Kivity
2011-10-03 15:12             ` Jon Mason
2011-10-04  9:46               ` Avi Kivity
2011-10-04 13:06                 ` Avi Kivity
2011-10-04 13:11                   ` Jon Mason
2011-10-04 20:12                   ` Jon Mason
2011-10-05  3:46                   ` Jon Mason
2011-10-05 12:09                     ` Avi Kivity

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPoiz9wR9mUHdCBDOV+dOjbWiY48zQDyPUeAFv-70YneLMT1Uw@mail.gmail.com \
    --to=mason@myri.com \
    --cc=avi@redhat.com \
    --cc=bhutchings@solarflare.com \
    --cc=eric.dumazet@gmail.com \
    --cc=jbarnes@virtuousgeek.org \
    --cc=jesse.brandeburg@gmail.com \
    --cc=jesse.brandeburg@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=niels_ole@salscheider-online.de \
    --cc=sim@hostway.ca \
    --cc=svens@stackframe.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.