| Message ID | 20180211024815.73781.92853.stgit@bhelgaas-glaptop.roam.corp.google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Sat, Feb 10, 2018 at 08:48:15PM -0600, Bjorn Helgaas wrote: > 7570a333d8b0 ("PCI: Add pcie_hp=nomsi to disable MSI/MSI-X for pciehp > driver") added the "pcie_hp=nomsi" kernel parameter to work around this > error on shutdown: > > irq 16: nobody cared (try booting with the "irqpoll" option) > Pid: 1081, comm: reboot Not tainted 3.2.0 #1 > ... > Disabling IRQ #16 > > This happened on an unspecified system (possibly involving the Integrated > Device Technology, Inc. Device 807f bridge). There is no automated way to > set this parameter, so it's not very useful for distributions or end users. > > I suspect the root cause of the underlying "irq 16: nobody cared" issue was > fixed by Prarit Bhargava <prarit@redhat.com> with fda78d7a0ead ("PCI/MSI: > Stop disabling MSI/MSI-X in pci_device_shutdown()") and we probably don't > need "pcie_hp=nomsi" any more. > > Revert 7570a333d8b0 to remove the "pcie_hp=nomsi" parameter. This has been useful when bringing up broken hardware which claims to support MSI but really doesn't, such as in commit 19bf4d4f909d ("thunderbolt: Support 1st gen Light Ridge controller"). In this case I didn't know why the controller wasn't sending interrupts, on a hunch I tried "pcie_hp=nomsi" and that worked, and looking in the macOS source code I was able to verify that they disable MSI for this particular hardware as well. So please leave it in to ease developers' lives. Thanks, Lukas
On Sun, Feb 11, 2018 at 10:25:25AM +0100, Lukas Wunner wrote: > On Sat, Feb 10, 2018 at 08:48:15PM -0600, Bjorn Helgaas wrote: > > 7570a333d8b0 ("PCI: Add pcie_hp=nomsi to disable MSI/MSI-X for pciehp > > driver") added the "pcie_hp=nomsi" kernel parameter to work around this > > error on shutdown: > > > > irq 16: nobody cared (try booting with the "irqpoll" option) > > Pid: 1081, comm: reboot Not tainted 3.2.0 #1 > > ... > > Disabling IRQ #16 > > > > This happened on an unspecified system (possibly involving the Integrated > > Device Technology, Inc. Device 807f bridge). There is no automated way to > > set this parameter, so it's not very useful for distributions or end users. > > > > I suspect the root cause of the underlying "irq 16: nobody cared" issue was > > fixed by Prarit Bhargava <prarit@redhat.com> with fda78d7a0ead ("PCI/MSI: > > Stop disabling MSI/MSI-X in pci_device_shutdown()") and we probably don't > > need "pcie_hp=nomsi" any more. > > > > Revert 7570a333d8b0 to remove the "pcie_hp=nomsi" parameter. > > This has been useful when bringing up broken hardware which claims to > support MSI but really doesn't, such as in commit 19bf4d4f909d > ("thunderbolt: Support 1st gen Light Ridge controller"). In this case > I didn't know why the controller wasn't sending interrupts, on a hunch > I tried "pcie_hp=nomsi" and that worked, and looking in the macOS source > code I was able to verify that they disable MSI for this particular > hardware as well. So please leave it in to ease developers' lives. Wouldn't "pci=nomsi" be sufficient for that sort of bringup experimentation? We don't need to be super specific in that situation. The reason I want to remove it is that the port driver (drivers/pci/pcie/portdrv*) has become a rat's nest of switches and special cases, and I'm trying to simplify it. Things like "pcie_hp=nomsi" are trivial individually, but collectively it's getting unmanageable. Bjorn
On Mon, Feb 12, 2018 at 09:01:46AM -0600, Bjorn Helgaas wrote: > On Sun, Feb 11, 2018 at 10:25:25AM +0100, Lukas Wunner wrote: > > On Sat, Feb 10, 2018 at 08:48:15PM -0600, Bjorn Helgaas wrote: > > > 7570a333d8b0 ("PCI: Add pcie_hp=nomsi to disable MSI/MSI-X for pciehp > > > driver") added the "pcie_hp=nomsi" kernel parameter to work around this > > > error on shutdown: > > > > > > irq 16: nobody cared (try booting with the "irqpoll" option) > > > Pid: 1081, comm: reboot Not tainted 3.2.0 #1 > > > ... > > > Disabling IRQ #16 > > > > > > This happened on an unspecified system (possibly involving the Integrated > > > Device Technology, Inc. Device 807f bridge). There is no automated way to > > > set this parameter, so it's not very useful for distributions or end users. > > > > > > I suspect the root cause of the underlying "irq 16: nobody cared" issue was > > > fixed by Prarit Bhargava <prarit@redhat.com> with fda78d7a0ead ("PCI/MSI: > > > Stop disabling MSI/MSI-X in pci_device_shutdown()") and we probably don't > > > need "pcie_hp=nomsi" any more. > > > > > > Revert 7570a333d8b0 to remove the "pcie_hp=nomsi" parameter. > > > > This has been useful when bringing up broken hardware which claims to > > support MSI but really doesn't, such as in commit 19bf4d4f909d > > ("thunderbolt: Support 1st gen Light Ridge controller"). In this case > > I didn't know why the controller wasn't sending interrupts, on a hunch > > I tried "pcie_hp=nomsi" and that worked, and looking in the macOS source > > code I was able to verify that they disable MSI for this particular > > hardware as well. So please leave it in to ease developers' lives. > > Wouldn't "pci=nomsi" be sufficient for that sort of bringup > experimentation? We don't need to be super specific in that > situation. I'm not sure if that would work as it also disables MSI for anything not a hotplug bridge. > The reason I want to remove it is that the port driver > (drivers/pci/pcie/portdrv*) has become a rat's nest of switches and > special cases, and I'm trying to simplify it. Things like > "pcie_hp=nomsi" are trivial individually, but collectively it's > getting unmanageable. I see your point and don't want to stand in the way of progress. It just was a useful tool for me once, that's all I'm saying, I probably won't need it again, but maybe someone else will. Thanks, Lukas
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 78cdf6a637fc..ed0d26d26c26 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -3101,10 +3101,6 @@ force Enable ASPM even on devices that claim not to support it. WARNING: Forcing ASPM on may cause system lockups. - pcie_hp= [PCIE] PCI Express Hotplug driver options: - nomsi Do not use MSI for PCI Express Native Hotplug (this - makes all PCIe ports use INTx for hotplug services). - pcie_ports= [PCIE] PCIe ports handling: auto Ask the BIOS whether or not to use native PCIe services associated with PCIe ports (PME, hot-plug, AER). Use diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h index a854bc569117..b00764bc684d 100644 --- a/drivers/pci/pcie/portdrv.h +++ b/drivers/pci/pcie/portdrv.h @@ -36,18 +36,6 @@ struct pci_dev; void pcie_clear_root_pme_status(struct pci_dev *dev); -#ifdef CONFIG_HOTPLUG_PCI_PCIE -extern bool pciehp_msi_disabled; - -static inline bool pciehp_no_msi(void) -{ - return pciehp_msi_disabled; -} - -#else /* !CONFIG_HOTPLUG_PCI_PCIE */ -static inline bool pciehp_no_msi(void) { return false; } -#endif /* !CONFIG_HOTPLUG_PCI_PCIE */ - #ifdef CONFIG_PCIE_PME extern bool pcie_pme_msi_disabled; diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index ef3bad4ad010..219f0f47f6fb 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -21,17 +21,6 @@ #include "../pci.h" #include "portdrv.h" -bool pciehp_msi_disabled; - -static int __init pciehp_setup(char *str) -{ - if (!strncmp(str, "nomsi", 5)) - pciehp_msi_disabled = true; - - return 1; -} -__setup("pcie_hp=", pciehp_setup); - /** * release_pcie_device - free PCI Express port service device structure * @dev: Port service device to release @@ -169,16 +158,13 @@ static int pcie_init_service_irqs(struct pci_dev *dev, int *irqs, int mask) irqs[i] = -1; /* - * If we support PME or hotplug, but we can't use MSI/MSI-X for - * them, we have to fall back to INTx or other interrupts, e.g., a - * system shared interrupt. + * If we support PME but can't use MSI/MSI-X for it, we have to + * fall back to INTx or other interrupts, e.g., a system shared + * interrupt. */ if ((mask & PCIE_PORT_SERVICE_PME) && pcie_pme_no_msi()) goto legacy_irq; - if ((mask & PCIE_PORT_SERVICE_HP) && pciehp_no_msi()) - goto legacy_irq; - /* Try to use MSI-X or MSI if supported */ if (pcie_port_enable_irq_vec(dev, irqs, mask) == 0) return 0;