From: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> This reverts commit e8303bb7a75c113388badcc49b2a84b4121c1b3e. e8303bb7a75c added logging whenever a link changed speed or width to a state that is considered degraded. Unfortunately, it cannot differentiate signal integrity-related link changes from those intentionally initiated by an endpoint driver, including drivers that may live in userspace or VMs when making use of vfio-pci. Some GPU drivers actively manage the link state to save power, which generates a stream of messages like this: vfio-pci 0000:07:00.0: 32.000 Gb/s available PCIe bandwidth, limited by 2.5 GT/s x16 link at 0000:00:02.0 (capable of 64.000 Gb/s with 5 GT/s x16 link) We really *do* want to be alerted when the link bandwidth is reduced because of hardware failures, but degradation by intentional link state management is probably far more common, so the signal-to-noise ratio is currently low. Until we figure out a way to identify the real problems or silence the intentional situations, revert the following commits, which include the initial implementation (e8303bb7a75c) and subsequent fixes: e8303bb7a75c ("PCI/LINK: Report degraded links via link bandwidth notification") 3e82a7f9031f ("PCI/LINK: Supply IRQ handler so level-triggered IRQs are acked") 55397ce8df48 ("PCI/LINK: Clear bandwidth notification interrupt before enabling it") 0fa635aec9ab ("PCI/LINK: Deduplicate bandwidth reports for multi-function devices") Link: https://lore.kernel.org/lkml/155597243666.19387.1205950870601742062.stgit@xxxxxxxxxx Link: https://lore.kernel.org/lkml/155605909349.3575.13433421148215616375.stgit@xxxxxxxxxx Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> CC: Alexandru Gagniuc <mr.nuke.me@xxxxxxxxx> CC: Lukas Wunner <lukas@xxxxxxxxx> CC: Alex Williamson <alex.williamson@xxxxxxxxxx> --- drivers/pci/pci.h | 1 - drivers/pci/pcie/Makefile | 1 - drivers/pci/pcie/bw_notification.c | 121 ----------------------------- drivers/pci/pcie/portdrv.h | 6 +- drivers/pci/pcie/portdrv_core.c | 17 ++-- drivers/pci/pcie/portdrv_pci.c | 1 - drivers/pci/probe.c | 2 +- 7 files changed, 7 insertions(+), 142 deletions(-) delete mode 100644 drivers/pci/pcie/bw_notification.c diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index d994839a3e24..224d88634115 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -273,7 +273,6 @@ enum pcie_link_width pcie_get_width_cap(struct pci_dev *dev); u32 pcie_bandwidth_capable(struct pci_dev *dev, enum pci_bus_speed *speed, enum pcie_link_width *width); void __pcie_print_link_status(struct pci_dev *dev, bool verbose); -void pcie_report_downtraining(struct pci_dev *dev); /* Single Root I/O Virtualization */ struct pci_sriov { diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile index f1d7bc1e5efa..ab514083d5d4 100644 --- a/drivers/pci/pcie/Makefile +++ b/drivers/pci/pcie/Makefile @@ -3,7 +3,6 @@ # Makefile for PCI Express features and port driver pcieportdrv-y := portdrv_core.o portdrv_pci.o err.o -pcieportdrv-y += bw_notification.o obj-$(CONFIG_PCIEPORTBUS) += pcieportdrv.o diff --git a/drivers/pci/pcie/bw_notification.c b/drivers/pci/pcie/bw_notification.c deleted file mode 100644 index 4fa9e3523ee1..000000000000 --- a/drivers/pci/pcie/bw_notification.c +++ /dev/null @@ -1,121 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ -/* - * PCI Express Link Bandwidth Notification services driver - * Author: Alexandru Gagniuc <mr.nuke.me@xxxxxxxxx> - * - * Copyright (C) 2019, Dell Inc - * - * The PCIe Link Bandwidth Notification provides a way to notify the - * operating system when the link width or data rate changes. This - * capability is required for all root ports and downstream ports - * supporting links wider than x1 and/or multiple link speeds. - * - * This service port driver hooks into the bandwidth notification interrupt - * and warns when links become degraded in operation. - */ - -#include "../pci.h" -#include "portdrv.h" - -static bool pcie_link_bandwidth_notification_supported(struct pci_dev *dev) -{ - int ret; - u32 lnk_cap; - - ret = pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnk_cap); - return (ret == PCIBIOS_SUCCESSFUL) && (lnk_cap & PCI_EXP_LNKCAP_LBNC); -} - -static void pcie_enable_link_bandwidth_notification(struct pci_dev *dev) -{ - u16 lnk_ctl; - - pcie_capability_write_word(dev, PCI_EXP_LNKSTA, PCI_EXP_LNKSTA_LBMS); - - pcie_capability_read_word(dev, PCI_EXP_LNKCTL, &lnk_ctl); - lnk_ctl |= PCI_EXP_LNKCTL_LBMIE; - pcie_capability_write_word(dev, PCI_EXP_LNKCTL, lnk_ctl); -} - -static void pcie_disable_link_bandwidth_notification(struct pci_dev *dev) -{ - u16 lnk_ctl; - - pcie_capability_read_word(dev, PCI_EXP_LNKCTL, &lnk_ctl); - lnk_ctl &= ~PCI_EXP_LNKCTL_LBMIE; - pcie_capability_write_word(dev, PCI_EXP_LNKCTL, lnk_ctl); -} - -static irqreturn_t pcie_bw_notification_irq(int irq, void *context) -{ - struct pcie_device *srv = context; - struct pci_dev *port = srv->port; - u16 link_status, events; - int ret; - - ret = pcie_capability_read_word(port, PCI_EXP_LNKSTA, &link_status); - events = link_status & PCI_EXP_LNKSTA_LBMS; - - 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. - */ - down_read(&pci_bus_sem); - list_for_each_entry(dev, &port->subordinate->devices, bus_list) - pcie_report_downtraining(dev); - up_read(&pci_bus_sem); - - return IRQ_HANDLED; -} - -static int pcie_bandwidth_notification_probe(struct pcie_device *srv) -{ - int ret; - - /* Single-width or single-speed ports do not have to support this. */ - if (!pcie_link_bandwidth_notification_supported(srv->port)) - return -ENODEV; - - ret = request_threaded_irq(srv->irq, pcie_bw_notification_irq, - pcie_bw_notification_handler, - IRQF_SHARED, "PCIe BW notif", srv); - if (ret) - return ret; - - pcie_enable_link_bandwidth_notification(srv->port); - - return 0; -} - -static void pcie_bandwidth_notification_remove(struct pcie_device *srv) -{ - pcie_disable_link_bandwidth_notification(srv->port); - free_irq(srv->irq, srv); -} - -static struct pcie_port_service_driver pcie_bandwidth_notification_driver = { - .name = "pcie_bw_notification", - .port_type = PCIE_ANY_PORT, - .service = PCIE_PORT_SERVICE_BWNOTIF, - .probe = pcie_bandwidth_notification_probe, - .remove = pcie_bandwidth_notification_remove, -}; - -int __init pcie_bandwidth_notification_init(void) -{ - return pcie_port_service_register(&pcie_bandwidth_notification_driver); -} diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h index 1d50dc58ac40..fbbf00b0992e 100644 --- a/drivers/pci/pcie/portdrv.h +++ b/drivers/pci/pcie/portdrv.h @@ -20,10 +20,8 @@ #define PCIE_PORT_SERVICE_HP (1 << PCIE_PORT_SERVICE_HP_SHIFT) #define PCIE_PORT_SERVICE_DPC_SHIFT 3 /* Downstream Port Containment */ #define PCIE_PORT_SERVICE_DPC (1 << PCIE_PORT_SERVICE_DPC_SHIFT) -#define PCIE_PORT_SERVICE_BWNOTIF_SHIFT 4 /* Bandwidth notification */ -#define PCIE_PORT_SERVICE_BWNOTIF (1 << PCIE_PORT_SERVICE_BWNOTIF_SHIFT) -#define PCIE_PORT_DEVICE_MAXSERVICES 5 +#define PCIE_PORT_DEVICE_MAXSERVICES 4 #ifdef CONFIG_PCIEAER int pcie_aer_init(void); @@ -49,8 +47,6 @@ int pcie_dpc_init(void); static inline int pcie_dpc_init(void) { return 0; } #endif -int pcie_bandwidth_notification_init(void); - /* Port Type */ #define PCIE_ANY_PORT (~0) diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 7d04f9d087a6..f458ac9cb70c 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -99,7 +99,7 @@ static int pcie_message_numbers(struct pci_dev *dev, int mask, */ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask) { - int nr_entries, nvec, pcie_irq; + int nr_entries, nvec; u32 pme = 0, aer = 0, dpc = 0; /* Allocate the maximum possible number of MSI/MSI-X vectors */ @@ -135,13 +135,10 @@ static int pcie_port_enable_irq_vec(struct pci_dev *dev, int *irqs, int mask) return nr_entries; } - /* PME, hotplug and bandwidth notification share an MSI/MSI-X vector */ - if (mask & (PCIE_PORT_SERVICE_PME | PCIE_PORT_SERVICE_HP | - PCIE_PORT_SERVICE_BWNOTIF)) { - pcie_irq = pci_irq_vector(dev, pme); - irqs[PCIE_PORT_SERVICE_PME_SHIFT] = pcie_irq; - irqs[PCIE_PORT_SERVICE_HP_SHIFT] = pcie_irq; - irqs[PCIE_PORT_SERVICE_BWNOTIF_SHIFT] = pcie_irq; + /* PME and hotplug share an MSI/MSI-X vector */ + if (mask & (PCIE_PORT_SERVICE_PME | PCIE_PORT_SERVICE_HP)) { + irqs[PCIE_PORT_SERVICE_PME_SHIFT] = pci_irq_vector(dev, pme); + irqs[PCIE_PORT_SERVICE_HP_SHIFT] = pci_irq_vector(dev, pme); } if (mask & PCIE_PORT_SERVICE_AER) @@ -253,10 +250,6 @@ static int get_port_device_capability(struct pci_dev *dev) pci_aer_available() && services & PCIE_PORT_SERVICE_AER) services |= PCIE_PORT_SERVICE_DPC; - if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM || - pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) - services |= PCIE_PORT_SERVICE_BWNOTIF; - return services; } diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c index 0a87091a0800..99d2abe88d0b 100644 --- a/drivers/pci/pcie/portdrv_pci.c +++ b/drivers/pci/pcie/portdrv_pci.c @@ -240,7 +240,6 @@ static void __init pcie_init_services(void) pcie_pme_init(); pcie_dpc_init(); pcie_hp_init(); - pcie_bandwidth_notification_init(); } static int __init pcie_portdrv_init(void) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 7e12d0163863..2ec0df04e0dc 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -2388,7 +2388,7 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn) return dev; } -void pcie_report_downtraining(struct pci_dev *dev) +static void pcie_report_downtraining(struct pci_dev *dev) { if (!pci_is_pcie(dev)) return; -- 2.21.0.593.g511ec345e18-goog