On Fri, Aug 21, 2015 at 8:08 AM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote: > Change the default MPS tuning policy to "default," which means we set every > device's MPS to match its upstream bridge. Typically this means we'll > preserve the fabric configuration done by firmware, even for hot-added > devices. > > N.B. This effectively re-enables quirk_intel_mc_errata(), which turns off > read completion coalescing on Intel 5000 and 5100 memory controllers. I think this patch might be the wrong thing for quirk_intel_mc_errata(). Currently, the default is PCIE_BUS_TUNE_OFF. In that case, the quirk does nothing, Linux doesn't touch MPS at all, and the BIOS is responsible for avoiding the erratum, either by disabling read coalescing or by avoiding MPS=256. This patch (as-is) means that even if the BIOS set MPS=128, we would disable read coalescing here, which is unnecessary. The new PCIE_BUS_DEFAULT basically means Linux will set MPS for hot-added devices the same way as for devices present at boot. That means we should be able to rely on the BIOS workaround and shouldn't have to disable read coalescing. So I think maybe we should bail out of quirk_intel_mc_errata() for both TUNE_OFF and DEFAULT, e.g., --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -2862,7 +2862,8 @@ static void quirk_intel_mc_errata(struct pci_dev *dev) int err; u16 rcc; - if (pcie_bus_config == PCIE_BUS_TUNE_OFF) + if (pcie_bus_config == PCIE_BUS_TUNE_OFF || + pcie_bus_config == PCIE_BUS_DEFAULT) return; > Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > --- > drivers/pci/pci.c | 2 +- > drivers/pci/probe.c | 3 ++- > include/linux/pci.h | 9 +++++---- > 3 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 0008c95..b96b4cc 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -81,7 +81,7 @@ unsigned long pci_cardbus_mem_size = DEFAULT_CARDBUS_MEM_SIZE; > unsigned long pci_hotplug_io_size = DEFAULT_HOTPLUG_IO_SIZE; > unsigned long pci_hotplug_mem_size = DEFAULT_HOTPLUG_MEM_SIZE; > > -enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_TUNE_OFF; > +enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_DEFAULT; > > /* > * The default CLS is used if arch didn't set CLS explicitly and not > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index f73dd7a..9da27af 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -1831,7 +1831,8 @@ static int pcie_bus_configure_set(struct pci_dev *dev, void *data) > if (!pci_is_pcie(dev)) > return 0; > > - if (pcie_bus_config == PCIE_BUS_TUNE_OFF) > + if (pcie_bus_config == PCIE_BUS_TUNE_OFF || > + pcie_bus_config == PCIE_BUS_DEFAULT) > return 0; > > mps = 128 << *(u8 *)data; > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 4d4f9d2..faf06a1 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -738,10 +738,11 @@ struct pci_driver { > void pcie_bus_configure_settings(struct pci_bus *bus); > > enum pcie_bus_config_types { > - PCIE_BUS_TUNE_OFF, > - PCIE_BUS_SAFE, > - PCIE_BUS_PERFORMANCE, > - PCIE_BUS_PEER2PEER, > + PCIE_BUS_TUNE_OFF, /* don't touch MPS at all */ > + PCIE_BUS_DEFAULT, /* MPS matches upstream bridge */ > + PCIE_BUS_SAFE, /* largest MPS supported by boot-time devices */ > + PCIE_BUS_PERFORMANCE, /* use MPS and MRRS for best performance */ > + PCIE_BUS_PEER2PEER, /* MPS = 128 for all devices */ > }; > > extern enum pcie_bus_config_types pcie_bus_config; > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html