On Fri, Aug 21, 2015 at 12:14:40PM -0700, Yinghai Lu wrote:
> On Fri, Aug 21, 2015 at 8:07 AM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote:
> > From: Keith Busch <keith.busch@xxxxxxxxx>
> >
> > Firmware typically configures the PCIe fabric with a consistent Max Payload
> > Size setting based on the devices present at boot. A hot-added device
> > typically has the power-on default MPS setting (128 bytes), which may not
> > match the fabric.
> >
> > When adding a new device via pci_device_add(), set its Max Payload Size to
> > match the upstream bridge (unless PCIe bus tuning is disabled). Note that
> > pcie_bus_configure_settings() may further change MPS based on the tuning
> > policy.
> >
> > This makes it more likely that a hot-added device will work in a system
> > with optimized MPS configuration.
> >
> > If we hot-add a device that only supports 128-byte MPS, it still likely
> > won't work because we don't reconfigure the rest of the fabric. Booting
> > with "pci=pcie_bus_peer2peer" is a workaround for this because it sets MPS
> > to 128 for everything.
> >
> > Signed-off-by: Keith Busch <keith.busch@xxxxxxxxx>
> > Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> > ---
> > drivers/pci/probe.c | 12 +++++++++++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index eb32395..f73dd7a 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1278,7 +1278,7 @@ int pci_setup_device(struct pci_dev *dev)
> > static void pci_configure_mps(struct pci_dev *dev)
> > {
> > struct pci_dev *bridge = pci_upstream_bridge(dev);
> > - int mps, p_mps;
> > + int mps, p_mps, rc;
> >
> > if (!pci_is_pcie(dev) || !bridge || !pci_is_pcie(bridge))
> > return;
> > @@ -1294,6 +1294,16 @@ static void pci_configure_mps(struct pci_dev *dev)
> > mps, pci_name(bridge), p_mps);
> > return;
> > }
> > +
>
> you may need add if the pcie_bus_config == PCIE_BUS_DEFAULT here.
>
> we should not could set mps extra for _SAFE, _PERFORMANCE, _PEER2PEER
How about this?
commit f2082d6c5e92ceedd1ed53f2d41fe50edd859557
Author: Keith Busch <keith.busch@xxxxxxxxx>
Date: Fri Aug 21 08:46:28 2015 -0500
PCI: Set MPS to match upstream bridge
Firmware typically configures the PCIe fabric with a consistent Max Payload
Size setting based on the devices present at boot. A hot-added device
typically has the power-on default MPS setting (128 bytes), which may not
match the fabric.
When adding a new device via pci_device_add(), set its Max Payload Size to
match the upstream bridge (unless PCIe bus tuning is disabled). Note that
pcie_bus_configure_settings() may further change MPS based on the tuning
policy.
This makes it more likely that a hot-added device will work in a system
with optimized MPS configuration.
If we hot-add a device that only supports 128-byte MPS, it still likely
won't work because we don't reconfigure the rest of the fabric. Booting
with "pci=pcie_bus_peer2peer" is a workaround for this because it sets MPS
to 128 for everything.
Signed-off-by: Keith Busch <keith.busch@xxxxxxxxx>
Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index eb32395..0e5d22d 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1278,7 +1278,7 @@ int pci_setup_device(struct pci_dev *dev)
static void pci_configure_mps(struct pci_dev *dev)
{
struct pci_dev *bridge = pci_upstream_bridge(dev);
- int mps, p_mps;
+ int mps, p_mps, rc;
if (!pci_is_pcie(dev) || !bridge || !pci_is_pcie(bridge))
return;
@@ -1294,6 +1294,23 @@ static void pci_configure_mps(struct pci_dev *dev)
mps, pci_name(bridge), p_mps);
return;
}
+
+ /*
+ * Fancier MPS configuration is done later by
+ * pcie_bus_configure_settings()
+ */
+ if (pcie_bus_config != PCIE_BUS_DEFAULT)
+ return;
+
+ rc = pcie_set_mps(dev, p_mps);
+ if (rc) {
+ dev_warn(&dev->dev, "can't set Max Payload Size to %d; if necessary, use \"pci=pcie_bus_safe\" and report a bug\n",
+ p_mps);
+ return;
+ }
+
+ dev_info(&dev->dev, "Max Payload Size set to %d (was %d, max %d)\n",
+ p_mps, mps, 128 << dev->pcie_mpss);
}
static struct hpp_type0 pci_default_type0 = {
--
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
