On Fri, Aug 21, 2015 at 1:05 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote:
> 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?
> @@ -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);
> }
Good to me.
Thanks
Yinghai
--
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
