linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL] PCI changes for v5.1
@ 2019-03-08 17:31 Bjorn Helgaas
  2019-03-09 23:15 ` pr-tracker-bot
  2019-03-17 21:18 ` Linus Torvalds
  0 siblings, 2 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2019-03-08 17:31 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-pci, linux-kernel, Lorenzo Pieralisi

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [GIT PULL] PCI changes for v5.1
  2019-03-08 17:31 [GIT PULL] PCI changes for v5.1 Bjorn Helgaas
@ 2019-03-09 23:15 ` pr-tracker-bot
  2019-03-17 21:18 ` Linus Torvalds
  1 sibling, 0 replies; 18+ messages in thread
From: pr-tracker-bot @ 2019-03-09 23:15 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Linus Torvalds, linux-pci, linux-kernel, Lorenzo Pieralisi

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [GIT PULL] PCI changes for v5.1
  2019-03-08 17:31 [GIT PULL] PCI changes for v5.1 Bjorn Helgaas
  2019-03-09 23:15 ` pr-tracker-bot
@ 2019-03-17 21:18 ` Linus Torvalds
  2019-03-18  0:22   ` Alex G
  2019-03-19  1:12   ` [PATCH] PCI/LINK: bw_notification: Do not leave interrupt handler NULL Alexandru Gagniuc
  1 sibling, 2 replies; 18+ messages in thread
From: Linus Torvalds @ 2019-03-17 21:18 UTC (permalink / raw)
  To: Bjorn Helgaas, Alexandru Gagniuc, Lukas Wunner
  Cc: linux-pci, Linux List Kernel Mailing, Lorenzo Pieralisi

On Fri, Mar 8, 2019 at 9:31 AM Bjorn Helgaas <helgaas@kernel.org> 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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [GIT PULL] PCI changes for v5.1
  2019-03-17 21:18 ` Linus Torvalds
@ 2019-03-18  0:22   ` Alex G
  2019-03-18  4:33     ` Lukas Wunner
  2019-03-19  1:12   ` [PATCH] PCI/LINK: bw_notification: Do not leave interrupt handler NULL Alexandru Gagniuc
  1 sibling, 1 reply; 18+ messages in thread
From: Alex G @ 2019-03-18  0:22 UTC (permalink / raw)
  To: Linus Torvalds, Bjorn Helgaas, Lukas Wunner
  Cc: linux-pci, Linux List Kernel Mailing, Lorenzo Pieralisi

On 3/17/19 4:18 PM, Linus Torvalds wrote:
> On Fri, Mar 8, 2019 at 9:31 AM Bjorn Helgaas <helgaas@kernel.org> 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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [GIT PULL] PCI changes for v5.1
  2019-03-18  0:22   ` Alex G
@ 2019-03-18  4:33     ` Lukas Wunner
  0 siblings, 0 replies; 18+ messages in thread
From: Lukas Wunner @ 2019-03-18  4:33 UTC (permalink / raw)
  To: Alex G
  Cc: Linus Torvalds, Bjorn Helgaas, linux-pci,
	Linux List Kernel Mailing, Lorenzo Pieralisi

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 <helgaas@kernel.org> 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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] PCI/LINK: bw_notification: Do not leave interrupt handler NULL
  2019-03-17 21:18 ` Linus Torvalds
  2019-03-18  0:22   ` Alex G
@ 2019-03-19  1:12   ` Alexandru Gagniuc
  2019-03-19 19:25     ` Lukas Wunner
                       ` (2 more replies)
  1 sibling, 3 replies; 18+ messages in thread
From: Alexandru Gagniuc @ 2019-03-19  1:12 UTC (permalink / raw)
  To: bhelgaas
  Cc: austin_bolen, alex_gagniuc, keith.busch, Shyam_Iyer, lukas,
	okaya, torvalds, Alexandru Gagniuc, linux-pci, linux-kernel

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 <torvalds@linux-foundation.org>
Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] PCI/LINK: bw_notification: Do not leave interrupt handler NULL
  2019-03-19  1:12   ` [PATCH] PCI/LINK: bw_notification: Do not leave interrupt handler NULL Alexandru Gagniuc
@ 2019-03-19 19:25     ` Lukas Wunner
  2019-03-19 20:00     ` Keith Busch
  2019-03-20 13:46     ` Bjorn Helgaas
  2 siblings, 0 replies; 18+ messages in thread
From: Lukas Wunner @ 2019-03-19 19:25 UTC (permalink / raw)
  To: Alexandru Gagniuc
  Cc: bhelgaas, austin_bolen, alex_gagniuc, keith.busch, Shyam_Iyer,
	okaya, torvalds, linux-pci, linux-kernel

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 <torvalds@linux-foundation.org>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>

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 <lukas@wunner.de>


> @@ -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 <lukas@wunner.de>

Thanks,

Lukas

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PCI/LINK: bw_notification: Do not leave interrupt handler NULL
  2019-03-19  1:12   ` [PATCH] PCI/LINK: bw_notification: Do not leave interrupt handler NULL Alexandru Gagniuc
  2019-03-19 19:25     ` Lukas Wunner
@ 2019-03-19 20:00     ` Keith Busch
  2019-03-20 13:46     ` Bjorn Helgaas
  2 siblings, 0 replies; 18+ messages in thread
From: Keith Busch @ 2019-03-19 20:00 UTC (permalink / raw)
  To: Alexandru Gagniuc
  Cc: bhelgaas, austin_bolen, alex_gagniuc, keith.busch, Shyam_Iyer,
	lukas, okaya, torvalds, linux-pci, linux-kernel

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PCI/LINK: bw_notification: Do not leave interrupt handler NULL
  2019-03-19  1:12   ` [PATCH] PCI/LINK: bw_notification: Do not leave interrupt handler NULL Alexandru Gagniuc
  2019-03-19 19:25     ` Lukas Wunner
  2019-03-19 20:00     ` Keith Busch
@ 2019-03-20 13:46     ` Bjorn Helgaas
  2019-03-20 13:48       ` Alex G.
  2 siblings, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2019-03-20 13:46 UTC (permalink / raw)
  To: Alexandru Gagniuc
  Cc: austin_bolen, alex_gagniuc, keith.busch, Shyam_Iyer, lukas,
	okaya, torvalds, linux-pci, linux-kernel

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 <torvalds@linux-foundation.org>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>

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
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PCI/LINK: bw_notification: Do not leave interrupt handler NULL
  2019-03-20 13:46     ` Bjorn Helgaas
@ 2019-03-20 13:48       ` Alex G.
  2019-03-20 19:35         ` Bjorn Helgaas
  0 siblings, 1 reply; 18+ messages in thread
From: Alex G. @ 2019-03-20 13:48 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: austin_bolen, alex_gagniuc, keith.busch, Shyam_Iyer, lukas,
	okaya, torvalds, linux-pci, linux-kernel

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 <torvalds@linux-foundation.org>
>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> 
> 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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PCI/LINK: bw_notification: Do not leave interrupt handler NULL
  2019-03-20 13:48       ` Alex G.
@ 2019-03-20 19:35         ` Bjorn Helgaas
  2019-03-23  0:36           ` [PATCH v2] " Alexandru Gagniuc
  0 siblings, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2019-03-20 19:35 UTC (permalink / raw)
  To: Alex G.
  Cc: austin_bolen, alex_gagniuc, keith.busch, Shyam_Iyer, lukas,
	okaya, torvalds, linux-pci, linux-kernel

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 <torvalds@linux-foundation.org>
> > > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > 
> > 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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] PCI/LINK: bw_notification: Do not leave interrupt handler NULL
  2019-03-20 19:35         ` Bjorn Helgaas
@ 2019-03-23  0:36           ` Alexandru Gagniuc
  2019-03-25 22:25             ` Bjorn Helgaas
  0 siblings, 1 reply; 18+ messages in thread
From: Alexandru Gagniuc @ 2019-03-23  0:36 UTC (permalink / raw)
  To: bhelgaas
  Cc: austin_bolen, alex_gagniuc, keith.busch, Shyam_Iyer, lukas,
	okaya, torvalds, Alexandru Gagniuc, linux-pci, linux-kernel

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 <torvalds@linux-foundation.org>
Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] PCI/LINK: bw_notification: Do not leave interrupt handler NULL
  2019-03-23  0:36           ` [PATCH v2] " Alexandru Gagniuc
@ 2019-03-25 22:25             ` Bjorn Helgaas
  2019-03-25 22:26               ` Alex G.
                                 ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2019-03-25 22:25 UTC (permalink / raw)
  To: Alexandru Gagniuc
  Cc: austin_bolen, alex_gagniuc, keith.busch, Shyam_Iyer, lukas,
	okaya, torvalds, linux-pci, linux-kernel

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 <torvalds@linux-foundation.org>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>

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
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] PCI/LINK: bw_notification: Do not leave interrupt handler NULL
  2019-03-25 22:25             ` Bjorn Helgaas
@ 2019-03-25 22:26               ` Alex G.
  2019-03-25 22:59               ` Bjorn Helgaas
  2019-04-19 21:08               ` Alex Williamson
  2 siblings, 0 replies; 18+ messages in thread
From: Alex G. @ 2019-03-25 22:26 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: austin_bolen, alex_gagniuc, keith.busch, Shyam_Iyer, lukas,
	okaya, torvalds, linux-pci, linux-kernel

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 <torvalds@linux-foundation.org>
>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> 
> 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
>>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] PCI/LINK: bw_notification: Do not leave interrupt handler NULL
  2019-03-25 22:25             ` Bjorn Helgaas
  2019-03-25 22:26               ` Alex G.
@ 2019-03-25 22:59               ` Bjorn Helgaas
  2019-04-19 21:08               ` Alex Williamson
  2 siblings, 0 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2019-03-25 22:59 UTC (permalink / raw)
  To: Alexandru Gagniuc
  Cc: austin_bolen, alex_gagniuc, keith.busch, Shyam_Iyer, lukas,
	okaya, torvalds, linux-pci, linux-kernel, Borislav Petkov

[+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 <torvalds@linux-foundation.org>
> > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> 
> 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
> > 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] PCI/LINK: bw_notification: Do not leave interrupt handler NULL
  2019-03-25 22:25             ` Bjorn Helgaas
  2019-03-25 22:26               ` Alex G.
  2019-03-25 22:59               ` Bjorn Helgaas
@ 2019-04-19 21:08               ` Alex Williamson
  2019-04-19 21:25                 ` Bjorn Helgaas
  2019-04-22 21:11                 ` Alex Williamson
  2 siblings, 2 replies; 18+ messages in thread
From: Alex Williamson @ 2019-04-19 21:08 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Alexandru Gagniuc, austin_bolen, alex_gagniuc, keith.busch,
	Shyam_Iyer, lukas, okaya, torvalds, linux-pci, linux-kernel

On Mon, 25 Mar 2019 17:25:02 -0500
Bjorn Helgaas <helgaas@kernel.org> 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 <torvalds@linux-foundation.org>
> > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>  
> 
> 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
> >   


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] PCI/LINK: bw_notification: Do not leave interrupt handler NULL
  2019-04-19 21:08               ` Alex Williamson
@ 2019-04-19 21:25                 ` Bjorn Helgaas
  2019-04-22 21:11                 ` Alex Williamson
  1 sibling, 0 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2019-04-19 21:25 UTC (permalink / raw)
  To: Alex Williamson
  Cc: Alexandru Gagniuc, austin_bolen, alex_gagniuc, keith.busch,
	Shyam_Iyer, lukas, okaya, torvalds, linux-pci, linux-kernel

On Fri, Apr 19, 2019 at 03:08:27PM -0600, Alex Williamson wrote:
> On Mon, 25 Mar 2019 17:25:02 -0500, Bjorn Helgaas <helgaas@kernel.org> 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 <torvalds@linux-foundation.org>
> > > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>  
> > 
> > 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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] PCI/LINK: bw_notification: Do not leave interrupt handler NULL
  2019-04-19 21:08               ` Alex Williamson
  2019-04-19 21:25                 ` Bjorn Helgaas
@ 2019-04-22 21:11                 ` Alex Williamson
  1 sibling, 0 replies; 18+ messages in thread
From: Alex Williamson @ 2019-04-22 21:11 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Alexandru Gagniuc, austin_bolen, alex_gagniuc, keith.busch,
	Shyam_Iyer, lukas, okaya, torvalds, linux-pci, linux-kernel

On Fri, 19 Apr 2019 15:08:27 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:

> On Mon, 25 Mar 2019 17:25:02 -0500
> Bjorn Helgaas <helgaas@kernel.org> 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 <torvalds@linux-foundation.org>
> > > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>    
> > 
> > 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
> > >     
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2019-04-22 21:12 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-08 17:31 [GIT PULL] PCI changes for v5.1 Bjorn Helgaas
2019-03-09 23:15 ` pr-tracker-bot
2019-03-17 21:18 ` Linus Torvalds
2019-03-18  0:22   ` Alex G
2019-03-18  4:33     ` Lukas Wunner
2019-03-19  1:12   ` [PATCH] PCI/LINK: bw_notification: Do not leave interrupt handler NULL Alexandru Gagniuc
2019-03-19 19:25     ` Lukas Wunner
2019-03-19 20:00     ` Keith Busch
2019-03-20 13:46     ` Bjorn Helgaas
2019-03-20 13:48       ` Alex G.
2019-03-20 19:35         ` Bjorn Helgaas
2019-03-23  0:36           ` [PATCH v2] " Alexandru Gagniuc
2019-03-25 22:25             ` Bjorn Helgaas
2019-03-25 22:26               ` Alex G.
2019-03-25 22:59               ` Bjorn Helgaas
2019-04-19 21:08               ` Alex Williamson
2019-04-19 21:25                 ` Bjorn Helgaas
2019-04-22 21:11                 ` Alex Williamson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).