> -----Original Message-----
> From: Bjorn Helgaas <helgaas@xxxxxxxxxx>
> Sent: Friday, October 21, 2022 4:25 AM
> To: Tyler Hicks <code@xxxxxxxxxxx>
> Cc: bhelgaas@xxxxxxxxxx; Keith Busch <kbusch@xxxxxxxxxx>;
> linux-pci@xxxxxxxxxxxxxxx; Z.Q. Hou <zhiqiang.hou@xxxxxxx>; Lorenzo
> Pieralisi <lpieralisi@xxxxxxxxxx>
> Subject: Re: [PATCH] PCI: Align MPS to upstream bridge for SAFE and
> PERFORMANCE mode
>
> On Wed, Oct 19, 2022 at 01:25:59PM -0500, Tyler Hicks wrote:
> > On 2022-06-10 23:01:31, Zhiqiang Hou wrote:
> > > From: Hou Zhiqiang <Zhiqiang.Hou@xxxxxxx>
> > >
> > > The commit 27d868b5e6cf ("PCI: Set MPS to match upstream bridge")
> > > made the device's MPS matches upstream bridge for PCIE_BUS_DEFAULT
> > > mode, so that it's more likely that a hot-added device will work in
> > > a system with an optimized MPS configuration.
> > >
> > > Obviously, the Linux itself optimizes the MPS settings in the
> > > PCIE_BUS_SAFE and PCIE_BUS_PERFORMANCE mode, so let's do this also
> > > for these modes.
> > >
> > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@xxxxxxx>
> >
> > I wanted to give a little more information about the issue we're seeing.
> > We're having trouble retaining the optimized Max Payload Size (MPS)
> > value of a PCIe endpoint after hotplug/rescan. In this case,
> > `pcie_ports=native` and `pci=pcie_bus_safe` are set on the cmdline and
> > we expect the Linux kernel to retain the MPS value. Commit
> > 27d868b5e6cf preserved the MPS value when using the default PCIe bus
> > mode
> > (PCIE_BUS_DEFAULT) and we're hopeful that this can be extended to
> > other modes, as well.
>
> Thanks, Tyler. I need help understanding what's going on here. An example
> of the topology and what happens and what *should* happen might help.
>
> Some MPS configuration is done per-device in pci_configure_mps(), and some
> considers a hierarchy in pcie_bus_configure_settings(). In the current tree,
> in the PCIE_BUS_SAFE case:
>
> - pci_configure_mps() does nothing (except for RCiEPs).
>
> - pcie_bus_configure_settings(bus) looks at the hierarchy rooted at
> the bridge leading to "bus". If the hierarchy contains a hotplug
> Switch Downstream Port, it sets MPS and MRRS to 128 for
> everything.
>
> If it does not contain such a bridge, it finds the smallest
> MPS_Supported ("smpss") of any device in the hierarchy and sets
> MPS and MRRS to that for everything.
>
> If you boot with a hotplug Root Port leading to an empty slot, I think the RP
> MPS will end up being whatever BIOS put there.
>
> A subsequent hot-add will do nothing in pci_configure_mps(), and
> pcie_bus_configure_settings() looks like it would set the RP and EP MPS to the
> minimum of the RP and EP MPS_Supported.
>
> Is that what you see? What would you like to see instead?
>
Hi Bjorn, Thanks for your comments!
This patch is for the case that kernel boot with 'pci=pcie_buf_perf', the MPS is tuned during the enumeration, but if the EP is removed and then rescan via the sysfs, the MPS won't be tuned in the rescan process. And the MPS is also tuned in the 'pcie_bus_safe' mode (see the Documentation/admin-guide/kernel-parameters.txt, I pasted the 2 options below for your reference).
Is this expected behavior? If yes, can you help understand the reason.
pcie_bus_safe Set every device's MPS to the largest value
supported by all devices below the root complex.
pcie_bus_perf Set device MPS to the largest allowable MPS
based on its parent bus. Also set MRRS (Max
Read Request Size) to the largest supported
value (no larger than the MPS that the device
or bus can support) for best performance.
Thanks,
Zhiqiang
> Bjorn
