PCI changes:
- Use match_string() instead of reimplementing it (Andy Shevchenko)
- Enable SERR# forwarding for all bridges (Bharat Kumar Gogada)
- Use Latency Tolerance Reporting if already enabled by platform (Bjorn
Helgaas)
- Save/restore LTR info for suspend/resume (Bjorn Helgaas)
- Fix DPC use of uninitialized data (Dongdong Liu)
- Probe bridge window attributes only once at enumeration-time to fix
device accesses during rescan (Bjorn Helgaas)
- Return BAR size (not "size -1 ") from pci_size() to simplify code (Du
Changbin)
- Use config header type (not class code) identify bridges more reliably
(Honghui Zhang)
- Work around Intel Denverton incorrect Trace Hub BAR size reporting
(Alexander Shishkin)
- Reorder pciehp cached state/hardware state updates to avoid missed
interrupts (Mika Westerberg)
- Turn ibmphp semaphores into completions or mutexes (Arnd Bergmann)
- Mark expected switch fall-through (Mathieu Malaterre)
- Use of_node_name_eq() for node name comparisons (Rob Herring)
- Add ACS and pciehp quirks for HXT SD4800 (Shunyong Yang)
- Consolidate Rohm Vendor ID definitions (Andy Shevchenko)
- Use u32 (not __u32) for things not exposed to userspace (Logan
Gunthorpe)
- Fix locking semantics of bus and slot reset interfaces (Alex
Williamson)
- Update PCIEPORTBUS Kconfig help text (Hou Zhiqiang)
- Allow portdrv to claim subtractive decode Ports so PCIe services will
work for them (Honghui Zhang)
- Report PCIe links that become degraded at run-time (Alexandru Gagniuc)
- Blacklist Gigabyte X299 Root Port power management to fix Thunderbolt
hotplug (Mika Westerberg)
- Revert runtime PM suspend/resume callbacks that broke PME on network
cable plug (Mika Westerberg)
- Disable Data Link State Changed interrupts to prevent wakeup
immediately after suspend (Mika Westerberg)
- Extend altera to support Stratix 10 (Ley Foon Tan)
- Allow building altera driver on ARM64 (Ley Foon Tan)
- Replace Douglas with Tom Joseph as Cadence PCI host/endpoint maintainer
(Lorenzo Pieralisi)
- Add DT support for R-Car RZ/G2E (R8A774C0) (Fabrizio Castro)
- Add dra72x/dra74x/dra76x SoC compatible strings (Kishon Vijay
Abraham I)
- Enable x2 mode support for dra72x/dra74x/dra76x SoC (Kishon Vijay
Abraham I)
- Configure dra7xx PHY to PCIe mode (Kishon Vijay Abraham I)
- Simplify dwc (remove unnecessary header includes, name variables
consistently, reduce inverted logic, etc) (Gustavo Pimentel)
- Add i.MX8MQ support (Andrey Smirnov)
- Add message to help debug dwc MSI-X mask bit errors (Gustavo Pimentel)
- Work around imx7d PCIe PLL erratum (Trent Piepho)
- Don't assert qcom reset GPIO during probe (Bjorn Andersson)
- Skip dwc MSI init if MSIs have been disabled (Lucas Stach)
- Use memcpy_fromio()/memcpy_toio() instead of plain memcpy() in PCI
endpoint framework (Wen Yang)
- Add interface to discover supported endpoint features to replace a
bitfield that wasn't flexible enough (Kishon Vijay Abraham I)
- Implement the new supported-feature interface for designware-plat,
dra7xx, rockchip, cadence (Kishon Vijay Abraham I)
- Fix issues with 64-bit BAR in endpoints (Kishon Vijay Abraham I)
- Add layerscape endpoint mode support (Xiaowei Bao)
- Remove duplicate struct hv_vp_set in favor of struct hv_vpset (Maya
Nakamura)
- Rework hv_irq_unmask() to use cpumask_to_vpset() instead of open-coded
reimplementation (Maya Nakamura)
- Align Hyper-V struct retarget_msi_interrupt arguments (Maya Nakamura)
- Fix mediatek MMIO size computation to enable full size of available
MMIO space (Honghui Zhang)
- Fix mediatek DMA window size computation to allow endpoint DMA access
to full DRAM address range (Honghui Zhang)
- Fix mvebu prefetchable BAR regression caused by common bridge emulation
that assumed all bridges had prefetchable windows (Thomas Petazzoni)
- Make advk_pci_bridge_emul_ops static (Wei Yongjun)
- Configure MPS settings for VMD root ports (Jon Derrick)
The following changes since commit bfeffd155283772bbe78c6a05dec7c0128ee500c:
Linux 5.0-rc1 (2019-01-06 17:08:20 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git tags/pci-v5.1-changes
for you to fetch changes up to dd92b6677e3d0d78e261a7f00f28e753bab41d24:
Merge branch 'remotes/lorenzo/pci/vmd' (2019-03-06 15:30:24 -0600)
----------------------------------------------------------------
pci-v5.1-changes
----------------------------------------------------------------
Alex Williamson (1):
PCI: Fix "try" semantics of bus and slot reset
Alexander Shishkin (1):
x86/PCI: Fixup RTIT_BAR of Intel Denverton Trace Hub
Alexandru Gagniuc (1):
PCI/LINK: Report degraded links via link bandwidth notification
Andrey Smirnov (11):
PCI: imx6: Introduce drvdata
PCI: imx6: Mark PHY functions as i.MX6 specific
PCI: imx6: Convert DIRECT_SPEED_CHANGE quirk code to use a flag
PCI: imx6: Add support for i.MX8MQ
dt-bindings: imx6q-pcie: Add "pcie_aux" clock for imx8mq
PCI: imx6: Add code to request/control "pcie_aux" clock for i.MX8MQ
PCI: dwc: Make use of IS_ALIGNED()
PCI: dwc: Share code for dw_pcie_rd/wr_other_conf()
PCI: dwc: Make use of BIT() in constant definitions
PCI: dwc: Make use of GENMASK/FIELD_PREP
PCI: dwc: Remove superfluous shifting in definitions
Andy Shevchenko (2):
PCI/AER: Use match_string() helper to simplify the code
PCI: Move Rohm Vendor ID to generic list
Arnd Bergmann (1):
PCI: ibmphp: Turn semaphores into completions or mutexes
Bharat Kumar Gogada (1):
PCI: Enable SERR# forwarding for all bridges
Bjorn Andersson (1):
PCI: qcom: Don't deassert reset GPIO during probe
Bjorn Helgaas (21):
PCI: Probe bridge window attributes once at enumeration-time
PCI/ASPM: Use LTR if already enabled by platform
PCI/ASPM: Save LTR Capability for suspend/resume
PCI/portdrv: Use conventional Device ID table formatting
Merge branch 'pci/aer'
Merge branch 'pci/aspm'
Merge branch 'pci/dpc'
Merge branch 'pci/enumeration'
Merge branch 'pci/hotplug'
Merge branch 'pci/misc'
Merge branch 'pci/portdrv'
Merge branch 'pci/pm'
Merge branch 'remotes/lorenzo/pci/altera'
Merge branch 'remotes/lorenzo/pci/cadence'
Merge branch 'remotes/lorenzo/pci/dt'
Merge branch 'remotes/lorenzo/pci/dwc'
Merge branch 'remotes/lorenzo/pci/endpoint'
Merge branch 'remotes/lorenzo/pci/hv'
Merge branch 'remotes/lorenzo/pci/mediatek'
Merge branch 'remotes/lorenzo/pci/misc'
Merge branch 'remotes/lorenzo/pci/vmd'
Dongdong Liu (1):
PCI/DPC: Fix print AER status in DPC event handling
Du Changbin (1):
PCI: Make pci_size() return real BAR size
Fabrizio Castro (1):
dt-bindings: PCI: rcar: Add device tree support for r8a774c0
Gustavo Pimentel (9):
PCI: dwc: Remove unnecessary header include (of_gpio.h)
PCI: dwc: Remove unnecessary header include (signal.h)
PCI: dwc: Rename variable name from data to d on dw_pci_bottom_mask/unmask()
PCI: dwc: Rename variable name from data to d on dw_pci_setup_msi_msg()
PCI: dwc: Rename variable name from data to d on dw_pci_msi_set_affinity()
PCI: dwc: Rename variable name from data to d on dw_pcie_irq_domain_free()
PCI: dwc: Improve code readability and simplify mask/unmask operations
PCI: dwc: Replace bit rotation operation (1 << bit) with BIT(bit)
PCI: dwc: Print debug error message when MSI-X entry control mask bit is set
Honghui Zhang (4):
PCI: Rely on config space header type, not class code
PCI/portdrv: Support PCIe services on subtractive decode bridges
PCI: mediatek: Fix memory mapped IO range size computation
PCI: mediatek: Enlarge PCIe2AHB window size to support 4GB DRAM
Hou Zhiqiang (1):
PCI: Update PCIEPORTBUS Kconfig help text
Jon Derrick (1):
PCI/VMD: Configure MPS settings before adding devices
Kishon Vijay Abraham I (19):
dt-bindings: PCI: dra7xx: Add SoC specific compatible strings
dt-bindings: PCI: dra7xx: Add properties to enable x2 lane in dra7
PCI: dwc: dra7xx: Enable x2 mode support for dra74x, dra76x and dra72x
PCI: dwc: dra7xx: Invoke phy_set_mode() API to set PHY mode to PHY_MODE_PCIE
PCI: endpoint: Add new pci_epc_ops to get EPC features
PCI: dwc: Add ->get_features() callback function to dw_pcie_ep_ops
PCI: designware-plat: Populate ->get_features() dw_pcie_ep_ops
PCI: pci-dra7xx: Populate ->get_features() dw_pcie_ep_ops
PCI: rockchip: Populate ->get_features() dw_pcie_ep_ops
PCI: cadence: Populate ->get_features() cdns_pcie_epc_ops
PCI: endpoint: Add helper to get first unreserved BAR
PCI: endpoint: Fix pci_epf_alloc_space() to set correct MEM TYPE flags
PCI: pci-epf-test: Remove setting epf_bar flags in function driver
PCI: pci-epf-test: Do not allocate next BARs memory if current BAR is 64Bit
PCI: pci-epf-test: Use pci_epc_get_features() to get EPC features
PCI: cadence: Remove pci_epf_linkup() from Cadence EP driver
PCI: rockchip: Remove pci_epf_linkup() from Rockchip EP driver
PCI: designware-plat: Remove setting epc->features in Designware plat EP driver
PCI: endpoint: Remove features member in struct pci_epc
Ley Foon Tan (3):
PCI: altera: Add Stratix 10 PCIe support
PCI: altera: Enable driver on ARM64
dt-bindings: PCI: altera: Add altr,pcie-root-port-2.0
Logan Gunthorpe (2):
genirq/msi: Clean up usage of __u8/__u16 types
PCI: Clean up usage of __u32 type
Lorenzo Pieralisi (1):
MAINTAINERS: Update PCI Cadence maintainer entry
Lucas Stach (1):
PCI: dwc: skip MSI init if MSIs have been explicitly disabled
Mathieu Malaterre (1):
PCI: Mark expected switch fall-through
Maya Nakamura (3):
PCI: hv: Add __aligned(8) to struct retarget_msi_interrupt
PCI: hv: Replace hv_vp_set with hv_vpset
PCI: hv: Refactor hv_irq_unmask() to use cpumask_to_vpset()
Mika Westerberg (4):
PCI: pciehp: Assign ctrl->slot_ctrl before writing it to hardware
PCI: Blacklist power management of Gigabyte X299 DESIGNARE EX PCIe ports
Revert "PCI/PME: Implement runtime PM callbacks"
PCI: pciehp: Disable Data Link Layer State Changed event on suspend
Rafael J. Wysocki (1):
PCI/PME: Fix hotplug/sysfs remove deadlock in pcie_pme_remove()
Rob Herring (1):
PCI: Use of_node_name_eq() for node name comparisons
Shunyong Yang (3):
PCI: Add HXT vendor ID
PCI: Add ACS quirk for HXT SD4800
PCI: pciehp: Add HXT quirk for Command Completed errata
Sven Van Asbroeck (1):
PCI/PME: Fix possible use-after-free on remove
Thomas Petazzoni (2):
PCI: pci-bridge-emul: Create per-bridge copy of register behavior
PCI: pci-bridge-emul: Extend pci_bridge_emul_init() with flags
Trent Piepho (3):
dt-bindings: imx6q-pcie: Add description of imx7d pcie phy
ARM: dts: imx7d: Add node for PCIe PHY
PCI: imx: Add workaround for e10728, IMX7d PCIe PLL failure
Wei Yongjun (1):
PCI: aardvark: Make symbol 'advk_pci_bridge_emul_ops' static
Wen Yang (1):
PCI: endpoint: functions: Use memcpy_fromio()/memcpy_toio()
Xiaowei Bao (4):
dt-bindings: add DT binding for the layerscape PCIe controller with EP mode
arm64: dts: Add the PCIE EP node in dts
PCI: layerscape: Add EP mode support
misc: pci_endpoint_test: Add the layerscape EP device support
.../devicetree/bindings/pci/altera-pcie.txt | 4 +-
.../devicetree/bindings/pci/fsl,imx6q-pcie.txt | 18 +-
.../devicetree/bindings/pci/layerscape-pci.txt | 3 +
Documentation/devicetree/bindings/pci/rcar-pci.txt | 4 +-
Documentation/devicetree/bindings/pci/ti-pci.txt | 11 +-
MAINTAINERS | 2 +-
arch/arm/boot/dts/imx7d.dtsi | 9 +
arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 34 ++-
arch/x86/hyperv/hv_init.c | 1 +
arch/x86/pci/fixup.c | 16 ++
drivers/dma/pch_dma.c | 1 -
drivers/gpio/gpio-ml-ioh.c | 2 -
drivers/gpio/gpio-pch.c | 1 -
drivers/i2c/busses/i2c-eg20t.c | 1 -
drivers/misc/pch_phub.c | 1 -
drivers/misc/pci_endpoint_test.c | 1 +
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 7 +-
drivers/pci/controller/Kconfig | 2 +-
drivers/pci/controller/dwc/Kconfig | 4 +-
drivers/pci/controller/dwc/Makefile | 2 +-
drivers/pci/controller/dwc/pci-dra7xx.c | 94 +++++++
drivers/pci/controller/dwc/pci-imx6.c | 224 +++++++++++++++--
drivers/pci/controller/dwc/pci-layerscape-ep.c | 156 ++++++++++++
drivers/pci/controller/dwc/pcie-designware-ep.c | 16 +-
drivers/pci/controller/dwc/pcie-designware-host.c | 115 ++++-----
drivers/pci/controller/dwc/pcie-designware-plat.c | 19 +-
drivers/pci/controller/dwc/pcie-designware.c | 6 +-
drivers/pci/controller/dwc/pcie-designware.h | 60 ++---
drivers/pci/controller/dwc/pcie-qcom.c | 2 +-
drivers/pci/controller/pci-aardvark.c | 4 +-
drivers/pci/controller/pci-hyperv.c | 61 +++--
drivers/pci/controller/pci-mvebu.c | 2 +-
drivers/pci/controller/pcie-altera.c | 270 +++++++++++++++++++--
drivers/pci/controller/pcie-cadence-ep.c | 25 +-
drivers/pci/controller/pcie-mediatek.c | 13 +-
drivers/pci/controller/pcie-rockchip-ep.c | 16 +-
drivers/pci/controller/vmd.c | 15 +-
drivers/pci/endpoint/functions/pci-epf-test.c | 97 +++++---
drivers/pci/endpoint/pci-epc-core.c | 53 ++++
drivers/pci/endpoint/pci-epf-core.c | 4 +-
drivers/pci/hotplug/ibmphp.h | 1 -
drivers/pci/hotplug/ibmphp_core.c | 2 -
drivers/pci/hotplug/ibmphp_hpc.c | 47 ++--
drivers/pci/hotplug/pciehp_hpc.c | 21 +-
drivers/pci/of.c | 2 +-
drivers/pci/pci-bridge-emul.c | 86 ++++---
drivers/pci/pci-bridge-emul.h | 13 +-
drivers/pci/pci-driver.c | 4 +-
drivers/pci/pci.c | 136 ++++++++---
drivers/pci/pcie/Kconfig | 7 +-
drivers/pci/pcie/Makefile | 1 +
drivers/pci/pcie/aer.c | 9 +-
drivers/pci/pcie/bw_notification.c | 110 +++++++++
drivers/pci/pcie/dpc.c | 27 ++-
drivers/pci/pcie/pme.c | 48 ++--
drivers/pci/pcie/portdrv.h | 6 +-
drivers/pci/pcie/portdrv_core.c | 17 +-
drivers/pci/pcie/portdrv_pci.c | 9 +-
drivers/pci/probe.c | 120 +++++++--
drivers/pci/quirks.c | 4 +-
drivers/pci/setup-bus.c | 63 +----
drivers/spi/spi-topcliff-pch.c | 1 -
drivers/tty/serial/pch_uart.c | 2 -
drivers/usb/gadget/udc/pch_udc.c | 1 -
include/linux/msi.h | 12 +-
include/linux/pci-epc.h | 31 ++-
include/linux/pci.h | 3 +
include/linux/pci_ids.h | 4 +
68 files changed, 1648 insertions(+), 515 deletions(-)
create mode 100644 drivers/pci/controller/dwc/pci-layerscape-ep.c
create mode 100644 drivers/pci/pcie/bw_notification.c
The pull request you sent on Fri, 8 Mar 2019 11:31:54 -0600:
> git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git tags/pci-v5.1-changes
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/2901752c14b8e1b7dd898d2e5245c93e531aa624
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
On Fri, Mar 8, 2019 at 9:31 AM Bjorn Helgaas <[email protected]> wrote:
>
> - Report PCIe links that become degraded at run-time (Alexandru Gagniuc)
Gaah. Only now as I'm about to do the rc1 release am I looking at new
runtime warnings, and noticing that this causes
genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 16
pcie_bw_notification: probe of 0000:00:1b.0:pcie010 failed with error -22
because you can't have a NULL handler for a level-triggered interrupt
(you need something to shut the interrupt off so that it doesn't keep
screaming!).
Happens on both my desktop and my laptop, regular intel CPU and
chipset, nothing odd in either case.
Everything else works, but the new BW notification obviously will
never work this way.
This is not holding up rc1, obviously, but I thought I'd mention it
since I noticed it as part of my "let's see that nothing regressed"
checks..
Linus
On 3/17/19 4:18 PM, Linus Torvalds wrote:
> On Fri, Mar 8, 2019 at 9:31 AM Bjorn Helgaas <[email protected]> wrote:
>>
>> - Report PCIe links that become degraded at run-time (Alexandru Gagniuc)
>
> Gaah. Only now as I'm about to do the rc1 release am I looking at new
> runtime warnings, and noticing that this causes
>
> genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 16
> pcie_bw_notification: probe of 0000:00:1b.0:pcie010 failed with error -22
>
> because you can't have a NULL handler for a level-triggered interrupt
> (you need something to shut the interrupt off so that it doesn't keep
> screaming!).
Thanks for the catch. I did not see the error on my test machines. I'll
take a look tomorrow, and update through Bjorn. Seems like an easy fix.
Alex
On Sun, Mar 17, 2019 at 07:22:17PM -0500, Alex G wrote:
> On 3/17/19 4:18 PM, Linus Torvalds wrote:
> > On Fri, Mar 8, 2019 at 9:31 AM Bjorn Helgaas <[email protected]> wrote:
> > >
> > > - Report PCIe links that become degraded at run-time (Alexandru Gagniuc)
> >
> > Gaah. Only now as I'm about to do the rc1 release am I looking at new
> > runtime warnings, and noticing that this causes
> >
> > genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 16
> > pcie_bw_notification: probe of 0000:00:1b.0:pcie010 failed with error -22
> >
> > because you can't have a NULL handler for a level-triggered interrupt
> > (you need something to shut the interrupt off so that it doesn't keep
> > screaming!).
>
> Thanks for the catch. I did not see the error on my test machines. I'll take
> a look tomorrow, and update through Bjorn. Seems like an easy fix.
My apologies for not spotting this during review.
This can't be fixed by setting the IRQF_ONESHOT flag because the IRQ is
always shared with pciehp, dpc and other port services, which do not set
the flag, so you'd get a flags mismatch error in __setup_irq().
The solution is thus to acknowledge the interrupt in
pcie_bw_notification_handler() and move the portion which may sleep
to a separate function which is used as IRQ thread. In other words,
move the down_read() / up_read() portion to a separate function and
return IRQ_WAKE_THREAD instead of IRQ_HANDLED.
The reason why it didn't show up on your test machines is likely that
they're using MSI on all PCIe ports, whereas Linus' machines appear
to possess at least one PCIe port which uses legacy INTx interrupts.
(MSI sets IRQCHIP_ONESHOT_SAFE for the irq_chip.)
Thanks,
Lukas
A threaded IRQ with a NULL handler does not work with level-triggered
interrupts. request_threaded_irq() will return an error:
genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 16
pcie_bw_notification: probe of 0000:00:1b.0:pcie010 failed with error -22
For level interrupts we need to silence the interrupt before exiting
the IRQ handler, so just clear the PCI_EXP_LNKSTA_LBMS bit there.
Reported-by: Linus Torvalds <[email protected]>
Signed-off-by: Alexandru Gagniuc <[email protected]>
---
OOPS! I'm sorry for the noise. Here's the fix.
I was able to test this on edge-triggered interrupts. None of my
machines have PCIe ports that use level-triggered interrupts. This
might not be too straightforward to test without a hardware yanker,
but if there's a way to force a specific interrupt to be level
triggered, I could do the testing on my end.
drivers/pci/pcie/bw_notification.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pcie/bw_notification.c b/drivers/pci/pcie/bw_notification.c
index d2eae3b7cc0f..001d6253ad48 100644
--- a/drivers/pci/pcie/bw_notification.c
+++ b/drivers/pci/pcie/bw_notification.c
@@ -44,11 +44,10 @@ static void pcie_disable_link_bandwidth_notification(struct pci_dev *dev)
pcie_capability_write_word(dev, PCI_EXP_LNKCTL, lnk_ctl);
}
-static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
+static irqreturn_t pcie_bw_notification_irq(int irq, void *context)
{
struct pcie_device *srv = context;
struct pci_dev *port = srv->port;
- struct pci_dev *dev;
u16 link_status, events;
int ret;
@@ -58,6 +57,17 @@ static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
if (ret != PCIBIOS_SUCCESSFUL || !events)
return IRQ_NONE;
+ pcie_capability_write_word(port, PCI_EXP_LNKSTA, events);
+ return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
+{
+ struct pcie_device *srv = context;
+ struct pci_dev *port = srv->port;
+ struct pci_dev *dev;
+ u16 link_status;
+
/*
* Print status from downstream devices, not this root port or
* downstream switch port.
@@ -67,8 +77,8 @@ static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
__pcie_print_link_status(dev, false);
up_read(&pci_bus_sem);
+ pcie_capability_read_word(port, PCI_EXP_LNKSTA, &link_status);
pcie_update_link_speed(port->subordinate, link_status);
- pcie_capability_write_word(port, PCI_EXP_LNKSTA, events);
return IRQ_HANDLED;
}
@@ -80,7 +90,8 @@ static int pcie_bandwidth_notification_probe(struct pcie_device *srv)
if (!pcie_link_bandwidth_notification_supported(srv->port))
return -ENODEV;
- ret = request_threaded_irq(srv->irq, NULL, pcie_bw_notification_handler,
+ ret = request_threaded_irq(srv->irq, pcie_bw_notification_irq,
+ pcie_bw_notification_handler,
IRQF_SHARED, "PCIe BW notif", srv);
if (ret)
return ret;
--
2.19.2
On Mon, Mar 18, 2019 at 08:12:04PM -0500, Alexandru Gagniuc wrote:
> A threaded IRQ with a NULL handler does not work with level-triggered
> interrupts. request_threaded_irq() will return an error:
>
> genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 16
> pcie_bw_notification: probe of 0000:00:1b.0:pcie010 failed with error -22
>
> For level interrupts we need to silence the interrupt before exiting
> the IRQ handler, so just clear the PCI_EXP_LNKSTA_LBMS bit there.
>
> Reported-by: Linus Torvalds <[email protected]>
> Signed-off-by: Alexandru Gagniuc <[email protected]>
I've tested this on my Ivy Bridge MacBook Pro which uses INTx on root and
downstream ports and I'm not seeing probe errors, so:
Tested-by: Lukas Wunner <[email protected]>
> @@ -67,8 +77,8 @@ static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> __pcie_print_link_status(dev, false);
> up_read(&pci_bus_sem);
>
> + pcie_capability_read_word(port, PCI_EXP_LNKSTA, &link_status);
> pcie_update_link_speed(port->subordinate, link_status);
> - pcie_capability_write_word(port, PCI_EXP_LNKSTA, events);
> return IRQ_HANDLED;
> }
I'd suggest leaving the call to pcie_update_link_speed() in
pcie_bw_notification_irq() to avoid the duplicate read of the
Link Status register.
With that addressed,
Reviewed-by: Lukas Wunner <[email protected]>
Thanks,
Lukas
On Mon, Mar 18, 2019 at 08:12:04PM -0500, Alexandru Gagniuc wrote:
> I was able to test this on edge-triggered interrupts. None of my
> machines have PCIe ports that use level-triggered interrupts. This
> might not be too straightforward to test without a hardware yanker,
> but if there's a way to force a specific interrupt to be level
> triggered, I could do the testing on my end.
Spec says INTx emulation is required, so I think it ought to be possible
to force them to level-triggered if you disable MSI. The kernel parameter
'pci=nomsi' can force everything to INTx.
Hi Alexandru,
On Mon, Mar 18, 2019 at 08:12:04PM -0500, Alexandru Gagniuc wrote:
> A threaded IRQ with a NULL handler does not work with level-triggered
> interrupts. request_threaded_irq() will return an error:
>
> genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 16
> pcie_bw_notification: probe of 0000:00:1b.0:pcie010 failed with error -22
>
> For level interrupts we need to silence the interrupt before exiting
> the IRQ handler, so just clear the PCI_EXP_LNKSTA_LBMS bit there.
>
> Reported-by: Linus Torvalds <[email protected]>
> Signed-off-by: Alexandru Gagniuc <[email protected]>
What's your thought regarding Lukas' comment? If you do repost this,
please add a Fixes: tag to help connect this with the initial commit.
If not, I can add the tag myself.
> ---
>
> OOPS! I'm sorry for the noise. Here's the fix.
>
> I was able to test this on edge-triggered interrupts. None of my
> machines have PCIe ports that use level-triggered interrupts. This
> might not be too straightforward to test without a hardware yanker,
> but if there's a way to force a specific interrupt to be level
> triggered, I could do the testing on my end.
>
> drivers/pci/pcie/bw_notification.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/pcie/bw_notification.c b/drivers/pci/pcie/bw_notification.c
> index d2eae3b7cc0f..001d6253ad48 100644
> --- a/drivers/pci/pcie/bw_notification.c
> +++ b/drivers/pci/pcie/bw_notification.c
> @@ -44,11 +44,10 @@ static void pcie_disable_link_bandwidth_notification(struct pci_dev *dev)
> pcie_capability_write_word(dev, PCI_EXP_LNKCTL, lnk_ctl);
> }
>
> -static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> +static irqreturn_t pcie_bw_notification_irq(int irq, void *context)
> {
> struct pcie_device *srv = context;
> struct pci_dev *port = srv->port;
> - struct pci_dev *dev;
> u16 link_status, events;
> int ret;
>
> @@ -58,6 +57,17 @@ static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> if (ret != PCIBIOS_SUCCESSFUL || !events)
> return IRQ_NONE;
>
> + pcie_capability_write_word(port, PCI_EXP_LNKSTA, events);
> + return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> +{
> + struct pcie_device *srv = context;
> + struct pci_dev *port = srv->port;
> + struct pci_dev *dev;
> + u16 link_status;
> +
> /*
> * Print status from downstream devices, not this root port or
> * downstream switch port.
> @@ -67,8 +77,8 @@ static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> __pcie_print_link_status(dev, false);
> up_read(&pci_bus_sem);
>
> + pcie_capability_read_word(port, PCI_EXP_LNKSTA, &link_status);
> pcie_update_link_speed(port->subordinate, link_status);
> - pcie_capability_write_word(port, PCI_EXP_LNKSTA, events);
> return IRQ_HANDLED;
> }
>
> @@ -80,7 +90,8 @@ static int pcie_bandwidth_notification_probe(struct pcie_device *srv)
> if (!pcie_link_bandwidth_notification_supported(srv->port))
> return -ENODEV;
>
> - ret = request_threaded_irq(srv->irq, NULL, pcie_bw_notification_handler,
> + ret = request_threaded_irq(srv->irq, pcie_bw_notification_irq,
> + pcie_bw_notification_handler,
> IRQF_SHARED, "PCIe BW notif", srv);
> if (ret)
> return ret;
> --
> 2.19.2
>
On 3/20/19 8:46 AM, Bjorn Helgaas wrote:
> Hi Alexandru,
>
> On Mon, Mar 18, 2019 at 08:12:04PM -0500, Alexandru Gagniuc wrote:
>> A threaded IRQ with a NULL handler does not work with level-triggered
>> interrupts. request_threaded_irq() will return an error:
>>
>> genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 16
>> pcie_bw_notification: probe of 0000:00:1b.0:pcie010 failed with error -22
>>
>> For level interrupts we need to silence the interrupt before exiting
>> the IRQ handler, so just clear the PCI_EXP_LNKSTA_LBMS bit there.
>>
>> Reported-by: Linus Torvalds <[email protected]>
>> Signed-off-by: Alexandru Gagniuc <[email protected]>
>
> What's your thought regarding Lukas' comment? If you do repost this,
> please add a Fixes: tag to help connect this with the initial commit.
I like Lukas's idea. I should have this re-posted by end of week, unless
there's an urgency to get it out earlier.
Alex
On Wed, Mar 20, 2019 at 08:48:33AM -0500, Alex G. wrote:
> On 3/20/19 8:46 AM, Bjorn Helgaas wrote:
> > On Mon, Mar 18, 2019 at 08:12:04PM -0500, Alexandru Gagniuc wrote:
> > > A threaded IRQ with a NULL handler does not work with level-triggered
> > > interrupts. request_threaded_irq() will return an error:
> > >
> > > genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 16
> > > pcie_bw_notification: probe of 0000:00:1b.0:pcie010 failed with error -22
> > >
> > > For level interrupts we need to silence the interrupt before exiting
> > > the IRQ handler, so just clear the PCI_EXP_LNKSTA_LBMS bit there.
> > >
> > > Reported-by: Linus Torvalds <[email protected]>
> > > Signed-off-by: Alexandru Gagniuc <[email protected]>
> >
> > What's your thought regarding Lukas' comment? If you do repost this,
> > please add a Fixes: tag to help connect this with the initial commit.
>
> I like Lukas's idea. I should have this re-posted by end of week, unless
> there's an urgency to get it out earlier.
It would have been ideal to get the fix in -rc2, but I guess the end
of the week is OK because it's probably already too late for me to
apply it, run it through 0-day, get it in -next, and ask Linus to pull
it for -rc2.
Bjorn
A threaded IRQ with a NULL handler does not work with level-triggered
interrupts. request_threaded_irq() will return an error:
genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 16
pcie_bw_notification: probe of 0000:00:1b.0:pcie010 failed with error -22
For level interrupts we need to silence the interrupt before exiting
the IRQ handler, so just clear the PCI_EXP_LNKSTA_LBMS bit there.
Fixes: e8303bb7a75c ("PCI/LINK: Report degraded links via link bandwidth notification")
Reported-by: Linus Torvalds <[email protected]>
Signed-off-by: Alexandru Gagniuc <[email protected]>
---
Changes since v1:
- move pcie_update_link_speed() to irq to prevent duplicate read of link_status
- Add Fixes: to commit message
drivers/pci/pcie/bw_notification.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/pcie/bw_notification.c b/drivers/pci/pcie/bw_notification.c
index d2eae3b7cc0f..c48746f1cf3c 100644
--- a/drivers/pci/pcie/bw_notification.c
+++ b/drivers/pci/pcie/bw_notification.c
@@ -44,11 +44,10 @@ static void pcie_disable_link_bandwidth_notification(struct pci_dev *dev)
pcie_capability_write_word(dev, PCI_EXP_LNKCTL, lnk_ctl);
}
-static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
+static irqreturn_t pcie_bw_notification_irq(int irq, void *context)
{
struct pcie_device *srv = context;
struct pci_dev *port = srv->port;
- struct pci_dev *dev;
u16 link_status, events;
int ret;
@@ -58,6 +57,17 @@ static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
if (ret != PCIBIOS_SUCCESSFUL || !events)
return IRQ_NONE;
+ pcie_capability_write_word(port, PCI_EXP_LNKSTA, events);
+ pcie_update_link_speed(port->subordinate, link_status);
+ return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
+{
+ struct pcie_device *srv = context;
+ struct pci_dev *port = srv->port;
+ struct pci_dev *dev;
+
/*
* Print status from downstream devices, not this root port or
* downstream switch port.
@@ -67,8 +77,6 @@ static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
__pcie_print_link_status(dev, false);
up_read(&pci_bus_sem);
- pcie_update_link_speed(port->subordinate, link_status);
- pcie_capability_write_word(port, PCI_EXP_LNKSTA, events);
return IRQ_HANDLED;
}
@@ -80,7 +88,8 @@ static int pcie_bandwidth_notification_probe(struct pcie_device *srv)
if (!pcie_link_bandwidth_notification_supported(srv->port))
return -ENODEV;
- ret = request_threaded_irq(srv->irq, NULL, pcie_bw_notification_handler,
+ ret = request_threaded_irq(srv->irq, pcie_bw_notification_irq,
+ pcie_bw_notification_handler,
IRQF_SHARED, "PCIe BW notif", srv);
if (ret)
return ret;
--
2.19.2
On Fri, Mar 22, 2019 at 07:36:51PM -0500, Alexandru Gagniuc wrote:
> A threaded IRQ with a NULL handler does not work with level-triggered
> interrupts. request_threaded_irq() will return an error:
>
> genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 16
> pcie_bw_notification: probe of 0000:00:1b.0:pcie010 failed with error -22
>
> For level interrupts we need to silence the interrupt before exiting
> the IRQ handler, so just clear the PCI_EXP_LNKSTA_LBMS bit there.
>
> Fixes: e8303bb7a75c ("PCI/LINK: Report degraded links via link bandwidth notification")
> Reported-by: Linus Torvalds <[email protected]>
> Signed-off-by: Alexandru Gagniuc <[email protected]>
Applied with the following subject line to for-linus for v5.1, thanks!
PCI/LINK: Supply IRQ handler so level-triggered IRQs are acked
> ---
> Changes since v1:
> - move pcie_update_link_speed() to irq to prevent duplicate read of link_status
> - Add Fixes: to commit message
>
> drivers/pci/pcie/bw_notification.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/pcie/bw_notification.c b/drivers/pci/pcie/bw_notification.c
> index d2eae3b7cc0f..c48746f1cf3c 100644
> --- a/drivers/pci/pcie/bw_notification.c
> +++ b/drivers/pci/pcie/bw_notification.c
> @@ -44,11 +44,10 @@ static void pcie_disable_link_bandwidth_notification(struct pci_dev *dev)
> pcie_capability_write_word(dev, PCI_EXP_LNKCTL, lnk_ctl);
> }
>
> -static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> +static irqreturn_t pcie_bw_notification_irq(int irq, void *context)
> {
> struct pcie_device *srv = context;
> struct pci_dev *port = srv->port;
> - struct pci_dev *dev;
> u16 link_status, events;
> int ret;
>
> @@ -58,6 +57,17 @@ static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> if (ret != PCIBIOS_SUCCESSFUL || !events)
> return IRQ_NONE;
>
> + pcie_capability_write_word(port, PCI_EXP_LNKSTA, events);
> + pcie_update_link_speed(port->subordinate, link_status);
> + return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> +{
> + struct pcie_device *srv = context;
> + struct pci_dev *port = srv->port;
> + struct pci_dev *dev;
> +
> /*
> * Print status from downstream devices, not this root port or
> * downstream switch port.
> @@ -67,8 +77,6 @@ static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> __pcie_print_link_status(dev, false);
> up_read(&pci_bus_sem);
>
> - pcie_update_link_speed(port->subordinate, link_status);
> - pcie_capability_write_word(port, PCI_EXP_LNKSTA, events);
> return IRQ_HANDLED;
> }
>
> @@ -80,7 +88,8 @@ static int pcie_bandwidth_notification_probe(struct pcie_device *srv)
> if (!pcie_link_bandwidth_notification_supported(srv->port))
> return -ENODEV;
>
> - ret = request_threaded_irq(srv->irq, NULL, pcie_bw_notification_handler,
> + ret = request_threaded_irq(srv->irq, pcie_bw_notification_irq,
> + pcie_bw_notification_handler,
> IRQF_SHARED, "PCIe BW notif", srv);
> if (ret)
> return ret;
> --
> 2.19.2
>
On 3/25/19 5:25 PM, Bjorn Helgaas wrote:
> On Fri, Mar 22, 2019 at 07:36:51PM -0500, Alexandru Gagniuc wrote:
>> A threaded IRQ with a NULL handler does not work with level-triggered
>> interrupts. request_threaded_irq() will return an error:
>>
>> genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 16
>> pcie_bw_notification: probe of 0000:00:1b.0:pcie010 failed with error -22
>>
>> For level interrupts we need to silence the interrupt before exiting
>> the IRQ handler, so just clear the PCI_EXP_LNKSTA_LBMS bit there.
>>
>> Fixes: e8303bb7a75c ("PCI/LINK: Report degraded links via link bandwidth notification")
>> Reported-by: Linus Torvalds <[email protected]>
>> Signed-off-by: Alexandru Gagniuc <[email protected]>
>
> Applied with the following subject line to for-linus for v5.1, thanks!
>
> PCI/LINK: Supply IRQ handler so level-triggered IRQs are acked
You're so much better at formulating commit messages. That sounds a lot
smoother. Thanks!
>> ---
>> Changes since v1:
>> - move pcie_update_link_speed() to irq to prevent duplicate read of link_status
>> - Add Fixes: to commit message
>>
>> drivers/pci/pcie/bw_notification.c | 19 ++++++++++++++-----
>> 1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/pci/pcie/bw_notification.c b/drivers/pci/pcie/bw_notification.c
>> index d2eae3b7cc0f..c48746f1cf3c 100644
>> --- a/drivers/pci/pcie/bw_notification.c
>> +++ b/drivers/pci/pcie/bw_notification.c
>> @@ -44,11 +44,10 @@ static void pcie_disable_link_bandwidth_notification(struct pci_dev *dev)
>> pcie_capability_write_word(dev, PCI_EXP_LNKCTL, lnk_ctl);
>> }
>>
>> -static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
>> +static irqreturn_t pcie_bw_notification_irq(int irq, void *context)
>> {
>> struct pcie_device *srv = context;
>> struct pci_dev *port = srv->port;
>> - struct pci_dev *dev;
>> u16 link_status, events;
>> int ret;
>>
>> @@ -58,6 +57,17 @@ static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
>> if (ret != PCIBIOS_SUCCESSFUL || !events)
>> return IRQ_NONE;
>>
>> + pcie_capability_write_word(port, PCI_EXP_LNKSTA, events);
>> + pcie_update_link_speed(port->subordinate, link_status);
>> + return IRQ_WAKE_THREAD;
>> +}
>> +
>> +static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
>> +{
>> + struct pcie_device *srv = context;
>> + struct pci_dev *port = srv->port;
>> + struct pci_dev *dev;
>> +
>> /*
>> * Print status from downstream devices, not this root port or
>> * downstream switch port.
>> @@ -67,8 +77,6 @@ static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
>> __pcie_print_link_status(dev, false);
>> up_read(&pci_bus_sem);
>>
>> - pcie_update_link_speed(port->subordinate, link_status);
>> - pcie_capability_write_word(port, PCI_EXP_LNKSTA, events);
>> return IRQ_HANDLED;
>> }
>>
>> @@ -80,7 +88,8 @@ static int pcie_bandwidth_notification_probe(struct pcie_device *srv)
>> if (!pcie_link_bandwidth_notification_supported(srv->port))
>> return -ENODEV;
>>
>> - ret = request_threaded_irq(srv->irq, NULL, pcie_bw_notification_handler,
>> + ret = request_threaded_irq(srv->irq, pcie_bw_notification_irq,
>> + pcie_bw_notification_handler,
>> IRQF_SHARED, "PCIe BW notif", srv);
>> if (ret)
>> return ret;
>> --
>> 2.19.2
>>
[+cc Borislav]
Hi Borislav, sorry; I meant to cc: you when I applied the patch below.
I did add a Reported-by for you.
On Mon, Mar 25, 2019 at 05:25:02PM -0500, Bjorn Helgaas wrote:
> On Fri, Mar 22, 2019 at 07:36:51PM -0500, Alexandru Gagniuc wrote:
> > A threaded IRQ with a NULL handler does not work with level-triggered
> > interrupts. request_threaded_irq() will return an error:
> >
> > genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 16
> > pcie_bw_notification: probe of 0000:00:1b.0:pcie010 failed with error -22
> >
> > For level interrupts we need to silence the interrupt before exiting
> > the IRQ handler, so just clear the PCI_EXP_LNKSTA_LBMS bit there.
> >
> > Fixes: e8303bb7a75c ("PCI/LINK: Report degraded links via link bandwidth notification")
> > Reported-by: Linus Torvalds <[email protected]>
> > Signed-off-by: Alexandru Gagniuc <[email protected]>
>
> Applied with the following subject line to for-linus for v5.1, thanks!
>
> PCI/LINK: Supply IRQ handler so level-triggered IRQs are acked
>
> > ---
> > Changes since v1:
> > - move pcie_update_link_speed() to irq to prevent duplicate read of link_status
> > - Add Fixes: to commit message
> >
> > drivers/pci/pcie/bw_notification.c | 19 ++++++++++++++-----
> > 1 file changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/pci/pcie/bw_notification.c b/drivers/pci/pcie/bw_notification.c
> > index d2eae3b7cc0f..c48746f1cf3c 100644
> > --- a/drivers/pci/pcie/bw_notification.c
> > +++ b/drivers/pci/pcie/bw_notification.c
> > @@ -44,11 +44,10 @@ static void pcie_disable_link_bandwidth_notification(struct pci_dev *dev)
> > pcie_capability_write_word(dev, PCI_EXP_LNKCTL, lnk_ctl);
> > }
> >
> > -static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> > +static irqreturn_t pcie_bw_notification_irq(int irq, void *context)
> > {
> > struct pcie_device *srv = context;
> > struct pci_dev *port = srv->port;
> > - struct pci_dev *dev;
> > u16 link_status, events;
> > int ret;
> >
> > @@ -58,6 +57,17 @@ static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> > if (ret != PCIBIOS_SUCCESSFUL || !events)
> > return IRQ_NONE;
> >
> > + pcie_capability_write_word(port, PCI_EXP_LNKSTA, events);
> > + pcie_update_link_speed(port->subordinate, link_status);
> > + return IRQ_WAKE_THREAD;
> > +}
> > +
> > +static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> > +{
> > + struct pcie_device *srv = context;
> > + struct pci_dev *port = srv->port;
> > + struct pci_dev *dev;
> > +
> > /*
> > * Print status from downstream devices, not this root port or
> > * downstream switch port.
> > @@ -67,8 +77,6 @@ static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> > __pcie_print_link_status(dev, false);
> > up_read(&pci_bus_sem);
> >
> > - pcie_update_link_speed(port->subordinate, link_status);
> > - pcie_capability_write_word(port, PCI_EXP_LNKSTA, events);
> > return IRQ_HANDLED;
> > }
> >
> > @@ -80,7 +88,8 @@ static int pcie_bandwidth_notification_probe(struct pcie_device *srv)
> > if (!pcie_link_bandwidth_notification_supported(srv->port))
> > return -ENODEV;
> >
> > - ret = request_threaded_irq(srv->irq, NULL, pcie_bw_notification_handler,
> > + ret = request_threaded_irq(srv->irq, pcie_bw_notification_irq,
> > + pcie_bw_notification_handler,
> > IRQF_SHARED, "PCIe BW notif", srv);
> > if (ret)
> > return ret;
> > --
> > 2.19.2
> >
On Mon, 25 Mar 2019 17:25:02 -0500
Bjorn Helgaas <[email protected]> wrote:
> On Fri, Mar 22, 2019 at 07:36:51PM -0500, Alexandru Gagniuc wrote:
> > A threaded IRQ with a NULL handler does not work with level-triggered
> > interrupts. request_threaded_irq() will return an error:
> >
> > genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 16
> > pcie_bw_notification: probe of 0000:00:1b.0:pcie010 failed with error -22
> >
> > For level interrupts we need to silence the interrupt before exiting
> > the IRQ handler, so just clear the PCI_EXP_LNKSTA_LBMS bit there.
> >
> > Fixes: e8303bb7a75c ("PCI/LINK: Report degraded links via link bandwidth notification")
> > Reported-by: Linus Torvalds <[email protected]>
> > Signed-off-by: Alexandru Gagniuc <[email protected]>
>
> Applied with the following subject line to for-linus for v5.1, thanks!
>
> PCI/LINK: Supply IRQ handler so level-triggered IRQs are acked
That made it a little tricky to track down this thread. I get a
regression bisected back to this when trying to do vfio device
assignment. I haven't dug further than the bisection, but I assume bus
resets are triggering this link bandwidth notifier code and nobody
thinks it's their interrupt:
[ 119.910738] irq 16: nobody cared (try booting with the "irqpoll" option)
[ 119.917455] CPU: 18 PID: 0 Comm: swapper/18 Not tainted 5.1.0-rc1+ #29
[ 119.923998] Hardware name: Hewlett-Packard HP Z820 Workstation/158B, BIOS J63 v03.69 03/25/2014
[ 119.932715] Call Trace:
[ 119.935169] <IRQ>
[ 119.937200] dump_stack+0x46/0x60
[ 119.940534] __report_bad_irq+0x37/0xae
[ 119.944380] note_interrupt.cold.9+0xa/0x69
[ 119.948580] handle_irq_event_percpu+0x6a/0x80
[ 119.953037] handle_irq_event+0x3d/0x5a
[ 119.956887] handle_fasteoi_irq+0x8b/0x140
[ 119.961003] handle_irq+0xbf/0x100
[ 119.964420] do_IRQ+0x49/0xd0
[ 119.967398] common_interrupt+0xf/0xf
[ 119.971074] </IRQ>
[ 119.973190] RIP: 0010:cpuidle_enter_state+0xb4/0x460
[ 119.978167] Code: 24 0f 1f 44 00 00 31 ff e8 69 bf a3 ff 80 7c 24 13 00 74 12 9c 58 f6 c4 02 0f 85 7d 03 00 00 31 ff e8 60 cf a9 ff fb 45 85 e4 <0f> 88 ae 02 00 00 49 63 cc 4c 8b 3c 24 4c 2b 7c 24 08 48 8d 04 49
[ 119.996967] RSP: 0018:ffffb6740330fe98 EFLAGS: 00000202 ORIG_RAX: ffffffffffffffda
[ 120.004549] RAX: ffff9dbfc19a1d80 RBX: ffffffff82d2c940 RCX: 000000000000001f
[ 120.011700] RDX: 0000001beb3c9b05 RSI: 00000000315975dc RDI: 0000000000000000
[ 120.018845] RBP: ffff9dbfc19acc00 R08: 0000000000000002 R09: 0000000000021640
[ 120.025990] R10: 0000027ae2689456 R11: ffff9dbfc19a0e64 R12: 0000000000000004
[ 120.033146] R13: ffffffff82d2cad8 R14: 0000000000000004 R15: 0000000000000000
[ 120.040303] ? cpuidle_enter_state+0x97/0x460
[ 120.044679] do_idle+0x1f1/0x230
[ 120.047918] cpu_startup_entry+0x19/0x20
[ 120.051856] start_secondary+0x172/0x1c0
[ 120.055796] secondary_startup_64+0xb6/0xc0
[ 120.059993] handlers:
[ 120.062283] [<0000000054c59383>] usb_hcd_irq
[ 120.066563] Disabling IRQ #16
[ 122.885627] irq 16: nobody cared (try booting with the "irqpoll" option)
[ 122.892326] CPU: 18 PID: 0 Comm: swapper/18 Not tainted 5.1.0-rc1+ #29
[ 122.898847] Hardware name: Hewlett-Packard HP Z820 Workstation/158B, BIOS J63 v03.69 03/25/2014
[ 122.907532] Call Trace:
[ 122.909985] <IRQ>
[ 122.912009] dump_stack+0x46/0x60
[ 122.915325] __report_bad_irq+0x37/0xae
[ 122.919159] note_interrupt.cold.9+0xa/0x69
[ 122.923338] handle_irq_event_percpu+0x6a/0x80
[ 122.927781] handle_irq_event+0x3d/0x5a
[ 122.931630] handle_fasteoi_irq+0x8b/0x140
[ 122.935730] handle_irq+0xbf/0x100
[ 122.939137] do_IRQ+0x49/0xd0
[ 122.942108] common_interrupt+0xf/0xf
[ 122.945772] </IRQ>
[ 122.947881] RIP: 0010:cpuidle_enter_state+0xb4/0x460
[ 122.952845] Code: 24 0f 1f 44 00 00 31 ff e8 69 bf a3 ff 80 7c 24 13 00 74 12 9c 58 f6 c4 02 0f 85 7d 03 00 00 31 ff e8 60 cf a9 ff fb 45 85 e4 <0f> 88 ae 02 00 00 49 63 cc 4c 8b 3c 24 4c 2b 7c 24 08 48 8d 04 49
[ 122.971629] RSP: 0018:ffffb6740330fe98 EFLAGS: 00000202 ORIG_RAX: ffffffffffffffda
[ 122.979212] RAX: ffff9dbfc19a1d80 RBX: ffffffff82d2c940 RCX: 000000000000001f
[ 122.986361] RDX: 0000001c9c8daa6e RSI: 00000000315975dc RDI: 0000000000000000
[ 122.993517] RBP: ffff9dbfc19acc00 R08: 0000000000000002 R09: 0000000000021640
[ 123.000655] R10: 0000027cae52b176 R11: ffff9dbfc19a0e64 R12: 0000000000000004
[ 123.007777] R13: ffffffff82d2cad8 R14: 0000000000000004 R15: 0000000000000000
[ 123.014906] ? cpuidle_enter_state+0x97/0x460
[ 123.019270] do_idle+0x1f1/0x230
[ 123.022502] cpu_startup_entry+0x19/0x20
[ 123.026426] start_secondary+0x172/0x1c0
[ 123.030352] secondary_startup_64+0xb6/0xc0
[ 123.034536] handlers:
[ 123.036821] [<0000000054c59383>] usb_hcd_irq
[ 123.041106] [<000000006da712f0>] vfio_intx_handler [vfio_pci]
[ 123.046847] [<000000006da712f0>] vfio_intx_handler [vfio_pci]
[ 123.052592] [<000000006da712f0>] vfio_intx_handler [vfio_pci]
[ 123.058336] [<000000006da712f0>] vfio_intx_handler [vfio_pci]
[ 123.064090] [<000000006da712f0>] vfio_intx_handler [vfio_pci]
[ 123.069843] Disabling IRQ #16
Thanks,
Alex
> > ---
> > Changes since v1:
> > - move pcie_update_link_speed() to irq to prevent duplicate read of link_status
> > - Add Fixes: to commit message
> >
> > drivers/pci/pcie/bw_notification.c | 19 ++++++++++++++-----
> > 1 file changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/pci/pcie/bw_notification.c b/drivers/pci/pcie/bw_notification.c
> > index d2eae3b7cc0f..c48746f1cf3c 100644
> > --- a/drivers/pci/pcie/bw_notification.c
> > +++ b/drivers/pci/pcie/bw_notification.c
> > @@ -44,11 +44,10 @@ static void pcie_disable_link_bandwidth_notification(struct pci_dev *dev)
> > pcie_capability_write_word(dev, PCI_EXP_LNKCTL, lnk_ctl);
> > }
> >
> > -static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> > +static irqreturn_t pcie_bw_notification_irq(int irq, void *context)
> > {
> > struct pcie_device *srv = context;
> > struct pci_dev *port = srv->port;
> > - struct pci_dev *dev;
> > u16 link_status, events;
> > int ret;
> >
> > @@ -58,6 +57,17 @@ static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> > if (ret != PCIBIOS_SUCCESSFUL || !events)
> > return IRQ_NONE;
> >
> > + pcie_capability_write_word(port, PCI_EXP_LNKSTA, events);
> > + pcie_update_link_speed(port->subordinate, link_status);
> > + return IRQ_WAKE_THREAD;
> > +}
> > +
> > +static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> > +{
> > + struct pcie_device *srv = context;
> > + struct pci_dev *port = srv->port;
> > + struct pci_dev *dev;
> > +
> > /*
> > * Print status from downstream devices, not this root port or
> > * downstream switch port.
> > @@ -67,8 +77,6 @@ static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> > __pcie_print_link_status(dev, false);
> > up_read(&pci_bus_sem);
> >
> > - pcie_update_link_speed(port->subordinate, link_status);
> > - pcie_capability_write_word(port, PCI_EXP_LNKSTA, events);
> > return IRQ_HANDLED;
> > }
> >
> > @@ -80,7 +88,8 @@ static int pcie_bandwidth_notification_probe(struct pcie_device *srv)
> > if (!pcie_link_bandwidth_notification_supported(srv->port))
> > return -ENODEV;
> >
> > - ret = request_threaded_irq(srv->irq, NULL, pcie_bw_notification_handler,
> > + ret = request_threaded_irq(srv->irq, pcie_bw_notification_irq,
> > + pcie_bw_notification_handler,
> > IRQF_SHARED, "PCIe BW notif", srv);
> > if (ret)
> > return ret;
> > --
> > 2.19.2
> >
On Fri, Apr 19, 2019 at 03:08:27PM -0600, Alex Williamson wrote:
> On Mon, 25 Mar 2019 17:25:02 -0500, Bjorn Helgaas <[email protected]> wrote:
> > On Fri, Mar 22, 2019 at 07:36:51PM -0500, Alexandru Gagniuc wrote:
> > > A threaded IRQ with a NULL handler does not work with level-triggered
> > > interrupts. request_threaded_irq() will return an error:
> > >
> > > genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 16
> > > pcie_bw_notification: probe of 0000:00:1b.0:pcie010 failed with error -22
> > >
> > > For level interrupts we need to silence the interrupt before exiting
> > > the IRQ handler, so just clear the PCI_EXP_LNKSTA_LBMS bit there.
> > >
> > > Fixes: e8303bb7a75c ("PCI/LINK: Report degraded links via link bandwidth notification")
> > > Reported-by: Linus Torvalds <[email protected]>
> > > Signed-off-by: Alexandru Gagniuc <[email protected]>
> >
> > Applied with the following subject line to for-linus for v5.1, thanks!
> >
> > PCI/LINK: Supply IRQ handler so level-triggered IRQs are acked
>
> That made it a little tricky to track down this thread.
Yeah, sorry about that. I've been wondering if I should add
lore.kernel.org URLs when I apply patches. Maybe this is one good
reason to do that.
Bjorn
On Fri, 19 Apr 2019 15:08:27 -0600
Alex Williamson <[email protected]> wrote:
> On Mon, 25 Mar 2019 17:25:02 -0500
> Bjorn Helgaas <[email protected]> wrote:
>
> > On Fri, Mar 22, 2019 at 07:36:51PM -0500, Alexandru Gagniuc wrote:
> > > A threaded IRQ with a NULL handler does not work with level-triggered
> > > interrupts. request_threaded_irq() will return an error:
> > >
> > > genirq: Threaded irq requested with handler=NULL and !ONESHOT for irq 16
> > > pcie_bw_notification: probe of 0000:00:1b.0:pcie010 failed with error -22
> > >
> > > For level interrupts we need to silence the interrupt before exiting
> > > the IRQ handler, so just clear the PCI_EXP_LNKSTA_LBMS bit there.
> > >
> > > Fixes: e8303bb7a75c ("PCI/LINK: Report degraded links via link bandwidth notification")
> > > Reported-by: Linus Torvalds <[email protected]>
> > > Signed-off-by: Alexandru Gagniuc <[email protected]>
> >
> > Applied with the following subject line to for-linus for v5.1, thanks!
> >
> > PCI/LINK: Supply IRQ handler so level-triggered IRQs are acked
>
> That made it a little tricky to track down this thread. I get a
> regression bisected back to this when trying to do vfio device
> assignment. I haven't dug further than the bisection, but I assume bus
> resets are triggering this link bandwidth notifier code and nobody
> thinks it's their interrupt:
I'm not sure what to do with this, I think it bisects back to commit
3e82a7f9031f simply because the interrupt was failing to register prior
to that, so the bandwidth notifier code was never activated (how was
this tested?). When I assign a GPU to a VM, the VM is manipulating the
device to change the link speed, I would have thought this would
trigger the autonomous bandwidth notification, but I can clearly see
BWMgmt+ ABWMgmt- in lspci. The root port shows:
Interrupt: pin A routed to IRQ 25
And the BW notifier interrupt is registered here:
25: 0 ... 0 IR-IO-APIC 8-fasteoi PCIe BW notif
There's no interrupt count for any CPU on this vector. For all I know,
this IRQ routing has never been exercised and could be broken in the
BIOS, resulting in the a random spurious IRQ victim. There seems to be
no good way to disable this driver other than manually unbinding root
ports via sysfs. That's not a great solution. The system is an Intel
X79 based workstation. Suggestions for further debugging? Thanks,
Alex
> [ 119.910738] irq 16: nobody cared (try booting with the "irqpoll" option)
> [ 119.917455] CPU: 18 PID: 0 Comm: swapper/18 Not tainted 5.1.0-rc1+ #29
> [ 119.923998] Hardware name: Hewlett-Packard HP Z820 Workstation/158B, BIOS J63 v03.69 03/25/2014
> [ 119.932715] Call Trace:
> [ 119.935169] <IRQ>
> [ 119.937200] dump_stack+0x46/0x60
> [ 119.940534] __report_bad_irq+0x37/0xae
> [ 119.944380] note_interrupt.cold.9+0xa/0x69
> [ 119.948580] handle_irq_event_percpu+0x6a/0x80
> [ 119.953037] handle_irq_event+0x3d/0x5a
> [ 119.956887] handle_fasteoi_irq+0x8b/0x140
> [ 119.961003] handle_irq+0xbf/0x100
> [ 119.964420] do_IRQ+0x49/0xd0
> [ 119.967398] common_interrupt+0xf/0xf
> [ 119.971074] </IRQ>
> [ 119.973190] RIP: 0010:cpuidle_enter_state+0xb4/0x460
> [ 119.978167] Code: 24 0f 1f 44 00 00 31 ff e8 69 bf a3 ff 80 7c 24 13 00 74 12 9c 58 f6 c4 02 0f 85 7d 03 00 00 31 ff e8 60 cf a9 ff fb 45 85 e4 <0f> 88 ae 02 00 00 49 63 cc 4c 8b 3c 24 4c 2b 7c 24 08 48 8d 04 49
> [ 119.996967] RSP: 0018:ffffb6740330fe98 EFLAGS: 00000202 ORIG_RAX: ffffffffffffffda
> [ 120.004549] RAX: ffff9dbfc19a1d80 RBX: ffffffff82d2c940 RCX: 000000000000001f
> [ 120.011700] RDX: 0000001beb3c9b05 RSI: 00000000315975dc RDI: 0000000000000000
> [ 120.018845] RBP: ffff9dbfc19acc00 R08: 0000000000000002 R09: 0000000000021640
> [ 120.025990] R10: 0000027ae2689456 R11: ffff9dbfc19a0e64 R12: 0000000000000004
> [ 120.033146] R13: ffffffff82d2cad8 R14: 0000000000000004 R15: 0000000000000000
> [ 120.040303] ? cpuidle_enter_state+0x97/0x460
> [ 120.044679] do_idle+0x1f1/0x230
> [ 120.047918] cpu_startup_entry+0x19/0x20
> [ 120.051856] start_secondary+0x172/0x1c0
> [ 120.055796] secondary_startup_64+0xb6/0xc0
> [ 120.059993] handlers:
> [ 120.062283] [<0000000054c59383>] usb_hcd_irq
> [ 120.066563] Disabling IRQ #16
> [ 122.885627] irq 16: nobody cared (try booting with the "irqpoll" option)
> [ 122.892326] CPU: 18 PID: 0 Comm: swapper/18 Not tainted 5.1.0-rc1+ #29
> [ 122.898847] Hardware name: Hewlett-Packard HP Z820 Workstation/158B, BIOS J63 v03.69 03/25/2014
> [ 122.907532] Call Trace:
> [ 122.909985] <IRQ>
> [ 122.912009] dump_stack+0x46/0x60
> [ 122.915325] __report_bad_irq+0x37/0xae
> [ 122.919159] note_interrupt.cold.9+0xa/0x69
> [ 122.923338] handle_irq_event_percpu+0x6a/0x80
> [ 122.927781] handle_irq_event+0x3d/0x5a
> [ 122.931630] handle_fasteoi_irq+0x8b/0x140
> [ 122.935730] handle_irq+0xbf/0x100
> [ 122.939137] do_IRQ+0x49/0xd0
> [ 122.942108] common_interrupt+0xf/0xf
> [ 122.945772] </IRQ>
> [ 122.947881] RIP: 0010:cpuidle_enter_state+0xb4/0x460
> [ 122.952845] Code: 24 0f 1f 44 00 00 31 ff e8 69 bf a3 ff 80 7c 24 13 00 74 12 9c 58 f6 c4 02 0f 85 7d 03 00 00 31 ff e8 60 cf a9 ff fb 45 85 e4 <0f> 88 ae 02 00 00 49 63 cc 4c 8b 3c 24 4c 2b 7c 24 08 48 8d 04 49
> [ 122.971629] RSP: 0018:ffffb6740330fe98 EFLAGS: 00000202 ORIG_RAX: ffffffffffffffda
> [ 122.979212] RAX: ffff9dbfc19a1d80 RBX: ffffffff82d2c940 RCX: 000000000000001f
> [ 122.986361] RDX: 0000001c9c8daa6e RSI: 00000000315975dc RDI: 0000000000000000
> [ 122.993517] RBP: ffff9dbfc19acc00 R08: 0000000000000002 R09: 0000000000021640
> [ 123.000655] R10: 0000027cae52b176 R11: ffff9dbfc19a0e64 R12: 0000000000000004
> [ 123.007777] R13: ffffffff82d2cad8 R14: 0000000000000004 R15: 0000000000000000
> [ 123.014906] ? cpuidle_enter_state+0x97/0x460
> [ 123.019270] do_idle+0x1f1/0x230
> [ 123.022502] cpu_startup_entry+0x19/0x20
> [ 123.026426] start_secondary+0x172/0x1c0
> [ 123.030352] secondary_startup_64+0xb6/0xc0
> [ 123.034536] handlers:
> [ 123.036821] [<0000000054c59383>] usb_hcd_irq
> [ 123.041106] [<000000006da712f0>] vfio_intx_handler [vfio_pci]
> [ 123.046847] [<000000006da712f0>] vfio_intx_handler [vfio_pci]
> [ 123.052592] [<000000006da712f0>] vfio_intx_handler [vfio_pci]
> [ 123.058336] [<000000006da712f0>] vfio_intx_handler [vfio_pci]
> [ 123.064090] [<000000006da712f0>] vfio_intx_handler [vfio_pci]
> [ 123.069843] Disabling IRQ #16
>
> Thanks,
> Alex
>
> > > ---
> > > Changes since v1:
> > > - move pcie_update_link_speed() to irq to prevent duplicate read of link_status
> > > - Add Fixes: to commit message
> > >
> > > drivers/pci/pcie/bw_notification.c | 19 ++++++++++++++-----
> > > 1 file changed, 14 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/pci/pcie/bw_notification.c b/drivers/pci/pcie/bw_notification.c
> > > index d2eae3b7cc0f..c48746f1cf3c 100644
> > > --- a/drivers/pci/pcie/bw_notification.c
> > > +++ b/drivers/pci/pcie/bw_notification.c
> > > @@ -44,11 +44,10 @@ static void pcie_disable_link_bandwidth_notification(struct pci_dev *dev)
> > > pcie_capability_write_word(dev, PCI_EXP_LNKCTL, lnk_ctl);
> > > }
> > >
> > > -static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> > > +static irqreturn_t pcie_bw_notification_irq(int irq, void *context)
> > > {
> > > struct pcie_device *srv = context;
> > > struct pci_dev *port = srv->port;
> > > - struct pci_dev *dev;
> > > u16 link_status, events;
> > > int ret;
> > >
> > > @@ -58,6 +57,17 @@ static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> > > if (ret != PCIBIOS_SUCCESSFUL || !events)
> > > return IRQ_NONE;
> > >
> > > + pcie_capability_write_word(port, PCI_EXP_LNKSTA, events);
> > > + pcie_update_link_speed(port->subordinate, link_status);
> > > + return IRQ_WAKE_THREAD;
> > > +}
> > > +
> > > +static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> > > +{
> > > + struct pcie_device *srv = context;
> > > + struct pci_dev *port = srv->port;
> > > + struct pci_dev *dev;
> > > +
> > > /*
> > > * Print status from downstream devices, not this root port or
> > > * downstream switch port.
> > > @@ -67,8 +77,6 @@ static irqreturn_t pcie_bw_notification_handler(int irq, void *context)
> > > __pcie_print_link_status(dev, false);
> > > up_read(&pci_bus_sem);
> > >
> > > - pcie_update_link_speed(port->subordinate, link_status);
> > > - pcie_capability_write_word(port, PCI_EXP_LNKSTA, events);
> > > return IRQ_HANDLED;
> > > }
> > >
> > > @@ -80,7 +88,8 @@ static int pcie_bandwidth_notification_probe(struct pcie_device *srv)
> > > if (!pcie_link_bandwidth_notification_supported(srv->port))
> > > return -ENODEV;
> > >
> > > - ret = request_threaded_irq(srv->irq, NULL, pcie_bw_notification_handler,
> > > + ret = request_threaded_irq(srv->irq, pcie_bw_notification_irq,
> > > + pcie_bw_notification_handler,
> > > IRQF_SHARED, "PCIe BW notif", srv);
> > > if (ret)
> > > return ret;
> > > --
> > > 2.19.2
> > >
>