From e67ad9354a9b7621341adec4ac2c63d5269f835d Mon Sep 17 00:00:00 2001 From: Albert Zhou Date: Tue, 15 Nov 2022 22:38:56 +1100 Subject: [PATCH 1/5] PCI: pciehp: Enable by default if USB4 enabled Thunderbolt/USB4 PCIe tunneling depends on native PCIe hotplug. Enable pciehp by default if USB4 is enabled. [bhelgaas: squash, update subject, commit logs, tidy whitespace] Link: https://lore.kernel.org/r/20221115113857.35800-2-albert.zhou.50@gmail.com Link: https://lore.kernel.org/r/20221115113857.35800-3-albert.zhou.50@gmail.com Signed-off-by: Albert Zhou Signed-off-by: Bjorn Helgaas Reviewed-by: Mika Westerberg --- drivers/pci/hotplug/Kconfig | 3 +++ drivers/pci/pcie/Kconfig | 8 ++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/pci/hotplug/Kconfig b/drivers/pci/hotplug/Kconfig index 840a84bb5ee2..48113b210cf9 100644 --- a/drivers/pci/hotplug/Kconfig +++ b/drivers/pci/hotplug/Kconfig @@ -6,11 +6,14 @@ menuconfig HOTPLUG_PCI bool "Support for PCI Hotplug" depends on PCI && SYSFS + default y if USB4 help Say Y here if you have a motherboard with a PCI Hotplug controller. This allows you to add and remove PCI cards while the machine is powered up and running. + Thunderbolt/USB4 PCIe tunneling depends on native PCIe hotplug. + When in doubt, say N. if HOTPLUG_PCI diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig index 788ac8df3f9d..228652a59f27 100644 --- a/drivers/pci/pcie/Kconfig +++ b/drivers/pci/pcie/Kconfig @@ -4,6 +4,7 @@ # config PCIEPORTBUS bool "PCI Express Port Bus support" + default y if USB4 help This enables PCI Express Port Bus support. Users can then enable support for Native Hot-Plug, Advanced Error Reporting, Power @@ -15,9 +16,12 @@ config PCIEPORTBUS config HOTPLUG_PCI_PCIE bool "PCI Express Hotplug driver" depends on HOTPLUG_PCI && PCIEPORTBUS + default y if USB4 help - Say Y here if you have a motherboard that supports PCI Express Native - Hotplug + Say Y here if you have a motherboard that supports PCIe native + hotplug. + + Thunderbolt/USB4 PCIe tunneling depends on native PCIe hotplug. When in doubt, say N. From 05f5747414c6ecb8a7f9b0c1dc10bcffa6dfb5ba Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 21 Nov 2022 19:15:18 +0100 Subject: [PATCH 2/5] PCI/portdrv: Set PCIE_PORT_SERVICE_HP for Root and Downstream Ports only It is reported that on some systems pciehp binds to an Upstream Port and attempts to operate it which causes devices below the Port to disappear from the bus. This happens because acpiphp sets dev->is_hotplug_bridge for that Port (after receiving a Device Check notification on it from the platform firmware via ACPI) during the enumeration of PCI devices. get_port_device_capability() sees that dev->is_hotplug_bridge is set and adds PCIE_PORT_SERVICE_HP to Port services (which allows pciehp to bind to the Port in question) without consulting the PCIe type, which should be either Root Port or Downstream Port for the hotplug capability to be present. Per PCIe r6.0, sec 7.5.3.2, the Slot Implemented bit is only valid for Downstream Ports (including Root Ports), and PCIe hotplug depends on the Slot Capabilities / Control / Status registers. Make get_port_device_capability() more robust by adding a PCIe type check to it before adding PCIE_PORT_SERVICE_HP to Port services which helps to avoid the problem. [bhelgaas: add spec citation] Suggested-by: Lukas Wunner Link: https://lore.kernel.org/r/4786090.31r3eYUQgx@kreacher Reported-by: Rodrigo Vivi Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas Reviewed-by: Lukas Wunner --- drivers/pci/pcie/portdrv_core.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 1ac7fec47d6f..98f0126aaa90 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -209,6 +209,8 @@ static int get_port_device_capability(struct pci_dev *dev) int services = 0; if (dev->is_hotplug_bridge && + (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT || + pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM) && (pcie_ports_native || host->native_pcie_hotplug)) { services |= PCIE_PORT_SERVICE_HP; From c63a3be76df678b173c59f1d5dc19a21b2d1c753 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 21 Nov 2022 19:16:57 +0100 Subject: [PATCH 3/5] PCI: acpiphp: Avoid setting is_hotplug_bridge for PCIe Upstream Ports It is reported that on some systems pciehp binds to an Upstream Port and attempts to operate it which causes devices below the Port to disappear from the bus. This happens because acpiphp sets dev->is_hotplug_bridge for that Port (after receiving a Device Check notification on it from the platform firmware via ACPI) during the enumeration of PCI devices. get_port_device_capability() sees that dev->is_hotplug_bridge is set and adds PCIE_PORT_SERVICE_HP to Port services, which allows pciehp to bind to the Port in question. Even though this particular problem can be addressed by making the portdrv_core checks more robust, it also causes power management to work differently on the affected systems which generally is not desirable (PCIe Ports with dev->is_hotplug_bridge set have to pass additional tests to be allowed to go into the D3hot/cold power states which affects runtime PM of devices below these Ports). For this reason, amend check_hotplug_bridge() with a PCIe type check to prevent it from setting dev->is_hotplug_bridge for Upstream Ports. Suggested-by: Lukas Wunner Link: https://lore.kernel.org/r/2262230.ElGaqSPkdT@kreacher Reported-by: Rodrigo Vivi Tested-by: Rodrigo Vivi Signed-off-by: Rafael J. Wysocki Signed-off-by: Bjorn Helgaas Reviewed-by: Rodrigo Vivi Reviewed-by: Lukas Wunner --- drivers/pci/hotplug/acpiphp_glue.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c index 6efa3d8db9a5..5b1f271c6034 100644 --- a/drivers/pci/hotplug/acpiphp_glue.c +++ b/drivers/pci/hotplug/acpiphp_glue.c @@ -411,6 +411,14 @@ static void check_hotplug_bridge(struct acpiphp_slot *slot, struct pci_dev *dev) if (dev->is_hotplug_bridge) return; + /* + * In the PCIe case, only Root Ports and Downstream Ports are capable of + * accommodating hotplug devices, so avoid marking Upstream Ports as + * "hotplug bridges". + */ + if (pci_is_pcie(dev) && pci_pcie_type(dev) == PCI_EXP_TYPE_UPSTREAM) + return; + list_for_each_entry(func, &slot->funcs, sibling) { if (PCI_FUNC(dev->devfn) == func->function) { dev->is_hotplug_bridge = 1; From 9676f40618df9f8e1ab681486021d6c0df86c5fa Mon Sep 17 00:00:00 2001 From: Ian Cowan Date: Sat, 12 Nov 2022 09:28:57 -0500 Subject: [PATCH 4/5] PCI: shpchp: Remove unused get_mode1_ECC_cap callback The ->get_mode1_ECC_cap callback in the shpchp_hpc_ops struct is never called, so remove it. [bhelgaas: squash] Link: https://lore.kernel.org/r/20221112142859.319733-2-ian@linux.cowan.aero Link: https://lore.kernel.org/r/20221112142859.319733-3-ian@linux.cowan.aero Link: https://lore.kernel.org/r/20221112142859.319733-4-ian@linux.cowan.aero Signed-off-by: Ian Cowan Signed-off-by: Bjorn Helgaas --- drivers/pci/hotplug/TODO | 3 --- drivers/pci/hotplug/shpchp.h | 1 - drivers/pci/hotplug/shpchp_hpc.c | 18 ------------------ 3 files changed, 22 deletions(-) diff --git a/drivers/pci/hotplug/TODO b/drivers/pci/hotplug/TODO index 88f217c82b4f..fdb8dd6ea24d 100644 --- a/drivers/pci/hotplug/TODO +++ b/drivers/pci/hotplug/TODO @@ -58,9 +58,6 @@ shpchp: pciehp with commit 82a9e79ef132 ("PCI: pciehp: remove hpc_ops"). Clarify if there was a specific reason not to apply the same change to shpchp. -* The ->get_mode1_ECC_cap callback in shpchp_hpc_ops is never invoked. - Why was it introduced? Can it be removed? - * The hardirq handler shpc_isr() queues events on a workqueue. It can be simplified by converting it to threaded IRQ handling. Use pciehp as a template. diff --git a/drivers/pci/hotplug/shpchp.h b/drivers/pci/hotplug/shpchp.h index 6e85885b554c..3a97f455336e 100644 --- a/drivers/pci/hotplug/shpchp.h +++ b/drivers/pci/hotplug/shpchp.h @@ -311,7 +311,6 @@ struct hpc_ops { int (*get_latch_status)(struct slot *slot, u8 *status); int (*get_adapter_status)(struct slot *slot, u8 *status); int (*get_adapter_speed)(struct slot *slot, enum pci_bus_speed *speed); - int (*get_mode1_ECC_cap)(struct slot *slot, u8 *mode); int (*get_prog_int)(struct slot *slot, u8 *prog_int); int (*query_power_fault)(struct slot *slot); void (*green_led_on)(struct slot *slot); diff --git a/drivers/pci/hotplug/shpchp_hpc.c b/drivers/pci/hotplug/shpchp_hpc.c index bd7557ca4910..48e4daefc44a 100644 --- a/drivers/pci/hotplug/shpchp_hpc.c +++ b/drivers/pci/hotplug/shpchp_hpc.c @@ -489,23 +489,6 @@ static int hpc_get_adapter_speed(struct slot *slot, enum pci_bus_speed *value) return retval; } -static int hpc_get_mode1_ECC_cap(struct slot *slot, u8 *mode) -{ - int retval = 0; - struct controller *ctrl = slot->ctrl; - u16 sec_bus_status = shpc_readw(ctrl, SEC_BUS_CONFIG); - u8 pi = shpc_readb(ctrl, PROG_INTERFACE); - - if (pi == 2) { - *mode = (sec_bus_status & 0x0100) >> 8; - } else { - retval = -1; - } - - ctrl_dbg(ctrl, "Mode 1 ECC cap = %d\n", *mode); - return retval; -} - static int hpc_query_power_fault(struct slot *slot) { struct controller *ctrl = slot->ctrl; @@ -900,7 +883,6 @@ static const struct hpc_ops shpchp_hpc_ops = { .get_adapter_status = hpc_get_adapter_status, .get_adapter_speed = hpc_get_adapter_speed, - .get_mode1_ECC_cap = hpc_get_mode1_ECC_cap, .get_prog_int = hpc_get_prog_int, .query_power_fault = hpc_query_power_fault, From 6d4671b534f6c084e92ef167a52dc47e55f636c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Tue, 27 Sep 2022 16:19:17 +0200 Subject: [PATCH 5/5] PCI: pciehp: Enable Command Completed Interrupt only if supported MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The No Command Completed Support bit in the Slot Capabilities register indicates whether Command Completed Interrupt Enable is unsupported. We already check whether No Command Completed Support bit is set in pcie_wait_cmd(), and do not wait in this case. Don't enable this Command Completed Interrupt at all if NCCS is set, so that when users dump configuration space from userspace, the dump does not confuse them by saying that Command Completed Interrupt is not supported, but it is enabled. Link: https://lore.kernel.org/r/20220927141926.8895-2-kabel@kernel.org Signed-off-by: Pali Rohár Signed-off-by: Marek Behún Signed-off-by: Bjorn Helgaas Reviewed-by: Lukas Wunner --- drivers/pci/hotplug/pciehp_hpc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index 040ae076ec0e..10e9670eea0b 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -811,7 +811,9 @@ static void pcie_enable_notification(struct controller *ctrl) else cmd |= PCI_EXP_SLTCTL_PDCE; if (!pciehp_poll_mode) - cmd |= PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_CCIE; + cmd |= PCI_EXP_SLTCTL_HPIE; + if (!pciehp_poll_mode && !NO_CMD_CMPL(ctrl)) + cmd |= PCI_EXP_SLTCTL_CCIE; mask = (PCI_EXP_SLTCTL_PDCE | PCI_EXP_SLTCTL_ABPE | PCI_EXP_SLTCTL_PFDE |