On Mon, Mar 03, 2025 at 10:36:30AM +0800, Feng Tang wrote: > There was problem reported by firmware developers that they received > two PCIe hotplug commands in very short intervals on an ARM server, > which doesn't comply with PCIe spec, and broke their state machine and > work flow. According to PCIe 6.1 spec, section 6.7.3.2, software needs > to wait at least 1 second for the command-complete event, before > resending the command or sending a new command. > > In the failure case, the first PCIe hotplug command firmware received > is from get_port_device_capability(), which sends command to disable > PCIe hotplug interrupts without waiting for its completion, and the > second command comes from pcie_enable_notification() of pciehp driver, > which enables hotplug interrupts again. > > One solution is to add the necessary delay after the first command [1], > while Lukas proposed an optimization that if the pciehp driver will be > loaded soon and handle the interrupts, then the hotplug and the wait > are not needed and can be saved, for every root port. > > So fix it by only disabling the hotplug interrupts when pciehp driver > is not enabled. > > [1]. https://lore.kernel.org/lkml/20250224034500.23024-1-feng.tang@xxxxxxxxxxxxxxxxx/t/#u > > Fixes: 2bd50dd800b5 ("PCI: PCIe: Disable PCIe port services during port initialization") > Suggested-by: Lukas Wunner <lukas@xxxxxxxxx> > Signed-off-by: Feng Tang <feng.tang@xxxxxxxxxxxxxxxxx> Applied to pci/hotplug for v6.15, thanks! > --- > Changelog: > > since v3: > * Separate this patch from patches dealing with irq storm in nomsi case > * Take Lukas's suggestion (Lukas Wunner) > > since v2: > * Add patch 0001, which move the waiting logic of pcie_poll_cmd from pciehp > driver to PCIe port driver for code reuse (Bjorn Helgaas) > * Separate Lucas' suggestion out as patch 0003 (Bjorn and Sathyanarayanan) > * Avoid hotplug command waiting for HW without command-complete > event support (Bjorn Helgaas) > * Fix spell issue in commit log (Bjorn and Markus) > * Add cover-letter for whole patchset (Markus Elfring) > * Handle a set-but-unused build warning (0Day lkp bot) > > since v1: > * Add the Originally-by for Liguang for patch 0002. The issue was found on > a 5.10 kernel, then 6.6. I was initially given a 5.10 kernel tar ball > without git info to debug the issue, and made the patch. Thanks to Guanghui > who recently pointed me to tree https://gitee.com/anolis/cloud-kernel which > show the wait logic in 5.10 was originally from Liguang, and never hit > mainline. > * Make the irq disabling not dependent on wthether pciehp service driver > will be loaded (Lukas Wunner) > * Use read_poll_timeout() API to simply the waiting logic (Sathyanarayanan > Kuppuswamy) > * Fix wrong email address (Markus Elfring) > * Add logic to skip irq disabling if it is already disabled. > > > drivers/pci/pcie/portdrv.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c > index 02e73099bad0..e8318fd5f6ed 100644 > --- a/drivers/pci/pcie/portdrv.c > +++ b/drivers/pci/pcie/portdrv.c > @@ -228,10 +228,12 @@ static int get_port_device_capability(struct pci_dev *dev) > > /* > * Disable hot-plug interrupts in case they have been enabled > - * by the BIOS and the hot-plug service driver is not loaded. > + * by the BIOS and the hot-plug service driver won't be loaded > + * to handle them. > */ > - pcie_capability_clear_word(dev, PCI_EXP_SLTCTL, > - PCI_EXP_SLTCTL_CCIE | PCI_EXP_SLTCTL_HPIE); > + if (!IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE)) > + pcie_capability_clear_word(dev, PCI_EXP_SLTCTL, > + PCI_EXP_SLTCTL_CCIE | PCI_EXP_SLTCTL_HPIE); > } > > #ifdef CONFIG_PCIEAER > -- > 2.43.5 >
