[PATCH v9 0/5] PCI: Add support for resetting the Root Ports in a platform specific way

[PATCH v9 0/5] PCI: Add support for resetting the Root Ports in a platform specific way

[Manivannan Sadhasivam via B4 Relay]

Hi,

Currently, in the event of AER/DPC, PCI core will try to reset the slot (Root
Port) and its subordinate devices by invoking bridge control reset and FLR. But
in some cases like AER Fatal error, it might be necessary to reset the Root
Ports using the PCI host bridge drivers in a platform specific way (as indicated
by the TODO in the pcie_do_recovery() function in drivers/pci/pcie/err.c).
Otherwise, the PCI link won't be recovered successfully.

So this series adds a new callback 'pci_host_bridge::reset_root_port' for the
host bridge drivers to reset the Root Port when a fatal error happens.

Also, this series allows the host bridge drivers to handle PCI link down event
by resetting the Root Ports and recovering the bus. This is accomplished by the
help of the new 'pci_host_handle_link_down()' API. Host bridge drivers are
expected to call this API (preferrably from a threaded IRQ handler) with
relevant Root Port 'pci_dev' when a link down event is detected for the port.
The API will reuse the pcie_do_recovery() function to recover the link if AER
support is enabled, otherwise it will directly call the reset_root_port()
callback of the host bridge driver (if exists).

For reference, I've modified the pcie-qcom driver to call
pci_host_handle_link_down() API with Root Port 'pci_dev' after receiving the
LDn global_irq event and populated 'pci_host_bridge::reset_root_port()'
callback to reset the Root Ports.
 
Testing
-------

Tested on Qcom Lemans AU Ride platform with Host and EP SoCs connected over PCIe
link. Simulated the LDn by disabling LTSSM_EN on the EP and I could verify that
the link was getting recovered successfully.

Changes in v9:
- Rebased on top of v7.2-rc1
- Added Fixes tag to Patch 1
- Amended the commit message of Patch 3 to clarify that err_handlers are not
  triggered for RPs
- Fixed some misc issues in Patch 4 pointed by Sashiko

Changes in v8:
- Removed pci_save_state() for the Root Port during recovery as the PCI core 
- Link to v7: https://lore.kernel.org/r/20260310-pci-port-reset-v7-0-9dd00ccc25ab@oss.qualcomm.com

Changes in v7:
- Dropped Rockchip Root port reset patch due to reported issues. But the series
  works on other platforms as tested by others.
- Added pci_{lock/unlock}_rescan_remove() to guard pci_bus_error_reset() as the
  device could be removed in-between due to Native hotplug interrupt.
- Rebased on top of v7.0-rc1
- Link to v6: https://lore.kernel.org/r/20250715-pci-port-reset-v6-0-6f9cce94e7bb@oss.qualcomm.com

Changes in v6:
- Incorporated the patch: https://lore.kernel.org/all/20250524185304.26698-2-manivannan.sadhasivam@linaro.org/
- Link to v5: https://lore.kernel.org/r/20250715-pci-port-reset-v5-0-26a5d278db40@oss.qualcomm.com

Changes in v5:
* Reworked the pci_host_handle_link_down() to accept Root Port instead of
  resetting all Root Ports in the event of link down.
* Renamed 'reset_slot' to 'reset_root_port' to avoid confusion as both terms
  were used interchangibly and the series is intended to reset Root Port only.
* Added the Rockchip driver change to this series.
* Dropped the applied patches and review/tested tags due to rework.
* Rebased on top of v6.16-rc1.

Changes in v4:
- Handled link down first in the irq handler
- Updated ICC & OPP bandwidth after link up in reset_slot() callback
- Link to v3: https://lore.kernel.org/r/20250417-pcie-reset-slot-v3-0-59a10811c962@linaro.org

Changes in v3:
- Made the pci-host-common driver as a common library for host controller
  drivers
- Moved the reset slot code to pci-host-common library
- Link to v2: https://lore.kernel.org/r/20250416-pcie-reset-slot-v2-0-efe76b278c10@linaro.org

Changes in v2:
- Moved calling reset_slot() callback from pcie_do_recovery() to pcibios_reset_secondary_bus()
- Link to v1: https://lore.kernel.org/r/20250404-pcie-reset-slot-v1-0-98952918bf90@linaro.org

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
Manivannan Sadhasivam (5):
      PCI: dwc: ep: Clear MSI iATU mapping in dw_pcie_ep_cleanup()
      PCI/ERR: Add support for resetting the Root Ports in a platform specific way
      PCI: host-common: Add link down handling for Root Ports
      PCI: qcom: Add support for resetting the Root Port due to link down event
      misc: pci_endpoint_test: Add AER error handlers

 drivers/misc/pci_endpoint_test.c                |  23 ++++
 drivers/pci/controller/dwc/pcie-designware-ep.c |   5 +
 drivers/pci/controller/dwc/pcie-qcom.c          | 140 +++++++++++++++++++++++-
 drivers/pci/controller/pci-host-common.c        |  35 ++++++
 drivers/pci/controller/pci-host-common.h        |   1 +
 drivers/pci/pci.c                               |  14 +++
 drivers/pci/pcie/err.c                          |   6 +-
 include/linux/pci.h                             |   1 +
 8 files changed, 219 insertions(+), 6 deletions(-)
---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20250715-pci-port-reset-4d9519570123

Best regards,
--  
Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

[PATCH v9 1/5] PCI: dwc: ep: Clear MSI iATU mapping in dw_pcie_ep_cleanup()

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

The MSI iATU mapping is currently only cleared when the endpoint is
stopped via configfs or when the host updates the MSI address/size.
This avoids redundant iATU reconfiguration every time the endpoint
raises an MSI interrupt.

However, a fundamental reset triggered by PERST# assert/deassert
resets all iATU inbound/outbound registers without going through the
configfs stop path. If the host also retains the same MSI address/size
after PERST# deassert, the driver never clears the stale MSI iATU
mapping. It then continues using this stale mapping to raise the MSI
interrupts, which can cause IOMMU faults and MSI failures on the host.

Fix this by clearing the MSI iATU mapping inside dw_pcie_ep_cleanup(),
which is already called as part of the PERST# assert/deassert sequence.
This unmaps the MSI iATU region and sets the msi_iatu_mapped flag to
false, ensuring that dw_pcie_ep_raise_msi_irq() performs a fresh iATU
mapping on its next invocation, regardless of whether the host changed
the MSI address/size.

Fixes: 8719c64e76bf ("PCI: dwc: ep: Cache MSI outbound iATU mapping")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-designware-ep.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 7d2794945704..31402ae218c7 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -1153,6 +1153,11 @@ void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
 
+	if (ep->msi_iatu_mapped) {
+		dw_pcie_ep_unmap_addr(ep->epc, 0, 0, ep->msi_mem_phys);
+		ep->msi_iatu_mapped = false;
+	}
+
 	dwc_pcie_debugfs_deinit(pci);
 	dw_pcie_edma_remove(pci);
 }

-- 
2.51.0

Re: [PATCH v9 1/5] PCI: dwc: ep: Clear MSI iATU mapping in dw_pcie_ep_cleanup()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [High] dw_pcie_ep_cleanup() modifies ep->msi_iatu_mapped and unmaps iATU without holding epc->lock, introducing a race condition.

Pre-existing issues:
- [Critical] Repeated invocations of dwc_pcie_debugfs_deinit() lead to a double-free of pci->ptm_debugfs.
- [High] Persistent MSI iATU mapping conflicts with dynamic MSI-X iATU mapping, causing misdirected unmaps and permanent iATU window leaks.
- [High] Multi-Function (MF) MSI Requester ID is corrupted because the iATU mapping cache does not track func_no.
- [Medium] Debugfs entries are permanently lost after the first link reset.
--

--- Patch [1]: [PATCH v9 1/5] PCI: dwc: ep: Clear MSI iATU mapping in dw_pcie_ep_cleanup() ---
commit c81bd7f3f798231612f6baba5357a5d4dcc7b736
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

PCI: dwc: ep: Clear MSI iATU mapping in dw_pcie_ep_cleanup()

This commit unmaps the MSI iATU region and clears the msi_iatu_mapped
flag in dw_pcie_ep_cleanup() during a PERST# reset sequence. This ensures
that dw_pcie_ep_raise_msi_irq() performs a fresh iATU mapping on its next
invocation.

> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 7d2794945704e..31402ae218c79 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -1153,6 +1153,11 @@ void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  
> +	if (ep->msi_iatu_mapped) {
> +		dw_pcie_ep_unmap_addr(ep->epc, 0, 0, ep->msi_mem_phys);
> +		ep->msi_iatu_mapped = false;
> +	}

[Severity: High]
Does this introduce a regression due to a race condition with epc->lock?

If a user-space ioctl or workqueue in an EPF driver calls pci_epc_raise_irq(),
it takes epc->lock and modifies ep->msi_iatu_mapped and iATU registers.
Concurrently, if a host-initiated PERST# reset triggers dw_pcie_ep_cleanup()
in the controller driver's IRQ handler, it modifies the exact same state here
without taking epc->lock.

Could this lead to data corruption of iATU registers or concurrent unaligned
accesses to the outbound window bitmap?

[Severity: High]
This is a pre-existing issue, but does the persistent MSI iATU mapping
conflict with dynamic MSI-X iATU mapping?

Both MSI and MSI-X use the same physical address (ep->msi_mem_phys). If
dw_pcie_ep_raise_msi_irq() caches the ATU mapping, and then
dw_pcie_ep_raise_msix_irq() is called, it maps the same address to a new
window:

drivers/pci/controller/dwc/pcie-designware-ep.c:dw_pcie_ep_raise_msix_irq() {
    ...
    ret = dw_pcie_ep_map_addr(epc, func_no, 0, ep->msi_mem_phys, msg_addr,
                              map_size);
    ...
}

When dw_pcie_ep_raise_msix_irq() later calls dw_pcie_ep_unmap_addr(), the
lookup function returns the first matching address:

drivers/pci/controller/dwc/pcie-designware-ep.c:dw_pcie_find_index() {
    ...
    if (ep->outbound_addr[index] != addr)
        continue;
    *atu_index = index;
    return 0;
}

Could this unmap the cached MSI window instead of the newly allocated MSI-X
window, permanently leaking the MSI-X window?

[Severity: High]
This is a pre-existing issue, but is the Multi-Function (MF) MSI Requester ID
corrupted because the iATU mapping cache does not track func_no?

When multiple endpoint functions are assigned the same MSI address by the host,
dw_pcie_ep_raise_msi_irq() for function 1 hits the cache because msg_addr and
map_size match the values cached by function 0:

drivers/pci/controller/dwc/pcie-designware-ep.c:dw_pcie_ep_raise_msi_irq() {
    ...
    if (ep->msi_iatu_mapped && (ep->msi_msg_addr != msg_addr ||
                                ep->msi_map_size != map_size)) {
    ...
}

Will it reuse the iATU programmed with PCIE_ATU_FUNC_NUM(0), sending function
1's MSI with the Requester ID of function 0, causing host IOMMU faults?

> +
>  	dwc_pcie_debugfs_deinit(pci);

[Severity: Critical]
This is a pre-existing issue, but do repeated invocations of
dwc_pcie_debugfs_deinit() lead to a double-free of pci->ptm_debugfs?

dw_pcie_ep_cleanup() is called by host drivers during a PERST# reset sequence.
It invokes dwc_pcie_debugfs_deinit(), which calls pcie_ptm_destroy_debugfs()
and frees the structure:

drivers/pci/pcie/ptm.c:pcie_ptm_destroy_debugfs() {
    ...
    kfree(ptm_debugfs);
}

However, pci->ptm_debugfs is never set to NULL. If the host resets the link a
second time, will the same pointer be passed to kfree() resulting in a
double-free?

[Severity: Medium]
This is a pre-existing issue, but are debugfs entries permanently lost after
the first link reset?

dw_pcie_ep_cleanup() unconditionally removes the debugfs directory during a
PERST# reset. However, dwc_pcie_debugfs_init() is only called during the
initial driver probe.

Does this mean the debugfs structure is destroyed and never recreated on
subsequent link-ups?

>  	dw_pcie_edma_remove(pci);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-pci-port-reset-v9-0-53570b92064d@oss.qualcomm.com?part=1

[PATCH v9 2/5] PCI/ERR: Add support for resetting the Root Ports in a platform specific way

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <mani@kernel.org>

Some host bridge devices require resetting the Root Ports in a platform
specific way to recover them from error conditions such as Fatal AER
errors, Link Down etc... So introduce pci_host_bridge::reset_root_port()
callback and call it from pcibios_reset_secondary_bus() if available. Also,
save the Root Port config space before reset and restore it afterwards.

The 'reset_root_port' callback is responsible for resetting the given Root
Port referenced by the 'pci_dev' pointer in a platform specific way and
bring it back to the working state if possible. If any error occurs during
the reset operation, relevant errno should be returned.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Tested-by: Brian Norris <briannorris@chromium.org>
Tested-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Tested-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/pci.c      | 13 +++++++++++++
 drivers/pci/pcie/err.c |  5 -----
 include/linux/pci.h    |  1 +
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 77b17b13ee61..12e099489690 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4841,6 +4841,19 @@ void pci_reset_secondary_bus(struct pci_dev *dev)
 
 void __weak pcibios_reset_secondary_bus(struct pci_dev *dev)
 {
+	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
+	int ret;
+
+	if (pci_is_root_bus(dev->bus) && host->reset_root_port) {
+		ret = host->reset_root_port(host, dev);
+		if (ret)
+			pci_err(dev, "Failed to reset Root Port: %d\n", ret);
+		else
+			pci_restore_state(dev);
+
+		return;
+	}
+
 	pci_reset_secondary_bus(dev);
 }
 
diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index bebe4bc111d7..13b9d9eb714f 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -256,11 +256,6 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
 	}
 
 	if (status == PCI_ERS_RESULT_NEED_RESET) {
-		/*
-		 * TODO: Should call platform-specific
-		 * functions to reset slot before calling
-		 * drivers' slot_reset callbacks?
-		 */
 		status = PCI_ERS_RESULT_RECOVERED;
 		pci_dbg(bridge, "broadcast slot_reset message\n");
 		pci_walk_bridge(bridge, report_slot_reset, &status);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index ebb5b9d76360..be82cc0a3371 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -646,6 +646,7 @@ struct pci_host_bridge {
 	void (*release_fn)(struct pci_host_bridge *);
 	int (*enable_device)(struct pci_host_bridge *bridge, struct pci_dev *dev);
 	void (*disable_device)(struct pci_host_bridge *bridge, struct pci_dev *dev);
+	int (*reset_root_port)(struct pci_host_bridge *bridge, struct pci_dev *dev);
 	void		*release_data;
 	unsigned int	ignore_reset_delay:1;	/* For entire hierarchy */
 	unsigned int	no_ext_tags:1;		/* No Extended Tags */

-- 
2.51.0

Re: [PATCH v9 2/5] PCI/ERR: Add support for resetting the Root Ports in a platform specific way

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The Root Port configuration space is restored without being saved before the platform-specific reset, leading to permanent loss of runtime configuration changes.
--

--- Patch [2]: [PATCH v9 2/5] PCI/ERR: Add support for resetting the Root Ports in a platform specific way ---
commit 2b1b6bf7dae47cb5b5ce9a84251ceb7b5b08de2c
Author: Manivannan Sadhasivam <mani@kernel.org>
PCI/ERR: Add support for resetting the Root Ports in a platform specific way

This commit introduces a reset_root_port callback in struct pci_host_bridge
to allow platform-specific root port resets during error recovery. It
modifies pcibios_reset_secondary_bus() to invoke this callback if available.
The commit message states that it will save the Root Port config space before
the reset and restore it afterwards.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 77b17b13ee615..12e099489690b 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4841,6 +4841,19 @@ void pci_reset_secondary_bus(struct pci_dev *dev)
>  
>  void __weak pcibios_reset_secondary_bus(struct pci_dev *dev)
>  {
> +	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
> +	int ret;
> +
> +	if (pci_is_root_bus(dev->bus) && host->reset_root_port) {
> +		ret = host->reset_root_port(host, dev);
> +		if (ret)
> +			pci_err(dev, "Failed to reset Root Port: %d\n", ret);
> +		else
> +			pci_restore_state(dev);

[Severity: High]
The commit message states "save the Root Port config space before reset and
restore it afterwards". However, is a call to pci_save_state(dev) missing
before the call to host->reset_root_port()?

Calling pci_restore_state(dev) here without a preceding save operation could
restore a stale configuration saved at boot or during the last suspend. Can
this lead to the permanent loss of runtime configuration changes on the Root
Port, such as ASPM link states, dynamic AER masks, or Max Payload Size
adjustments?

> +
> +		return;
> +	}
> +
>  	pci_reset_secondary_bus(dev);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-pci-port-reset-v9-0-53570b92064d@oss.qualcomm.com?part=2

[PATCH v9 3/5] PCI: host-common: Add link down handling for Root Ports

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <mani@kernel.org>

The PCI link, when down, needs to be recovered to bring it back. But on
some platforms, that cannot be done in a generic way as link recovery
procedure is platform specific. So add a new API
pci_host_handle_link_down() that could be called by the host bridge drivers
for a specific Root Port when the link goes down.

The API accepts the 'pci_dev' corresponding to the Root Port which observed
the link down event. If CONFIG_PCIEAER is enabled, the API calls
pcie_do_recovery() function with 'pci_channel_io_frozen' as the state. This
will result in the execution of the AER Fatal error handling code. Since
the link down recovery is pretty much the same as AER Fatal error handling,
pcie_do_recovery() helper is reused here.

AER error_detected() callback will be triggered for all of the downstream
devices, but not for Root Port itself as there is nothing to do for the
Root Ports in the callbacks. Finally, pci_host_reset_root_port() will be
called for the Root Port, which will reset the Root Port using
'reset_root_port' callback to recover the link. Once that's done, resume
message will be broadcasted to the bridge and the downstream devices,
indicating successful link recovery.

But if CONFIG_PCIEAER is not enabled in the kernel, only
pci_host_reset_root_port() API will be called, which will in turn call
pci_bus_error_reset() to just reset the Root Port as there is no way we
could inform the drivers about link recovery.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Tested-by: Brian Norris <briannorris@chromium.org>
Tested-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Tested-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/controller/pci-host-common.c | 35 ++++++++++++++++++++++++++++++++
 drivers/pci/controller/pci-host-common.h |  1 +
 drivers/pci/pci.c                        |  1 +
 drivers/pci/pcie/err.c                   |  1 +
 4 files changed, 38 insertions(+)

diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
index 2ce6f4b66133..363d3f970b10 100644
--- a/drivers/pci/controller/pci-host-common.c
+++ b/drivers/pci/controller/pci-host-common.c
@@ -13,9 +13,11 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
+#include <linux/pci.h>
 #include <linux/pci-ecam.h>
 #include <linux/platform_device.h>
 
+#include "../pci.h"
 #include "pci-host-common.h"
 
 /**
@@ -342,5 +344,38 @@ bool pci_host_common_d3cold_possible(struct pci_host_bridge *bridge,
 }
 EXPORT_SYMBOL_GPL(pci_host_common_d3cold_possible);
 
+static pci_ers_result_t pci_host_reset_root_port(struct pci_dev *dev)
+{
+	int ret;
+
+	pci_lock_rescan_remove();
+	ret = pci_bus_error_reset(dev);
+	pci_unlock_rescan_remove();
+	if (ret) {
+		pci_err(dev, "Failed to reset Root Port: %d\n", ret);
+		return PCI_ERS_RESULT_DISCONNECT;
+	}
+
+	pci_info(dev, "Root Port has been reset\n");
+
+	return PCI_ERS_RESULT_RECOVERED;
+}
+
+static void pci_host_recover_root_port(struct pci_dev *port)
+{
+#if IS_ENABLED(CONFIG_PCIEAER)
+	pcie_do_recovery(port, pci_channel_io_frozen, pci_host_reset_root_port);
+#else
+	pci_host_reset_root_port(port);
+#endif
+}
+
+void pci_host_handle_link_down(struct pci_dev *port)
+{
+	pci_info(port, "Recovering Root Port due to Link Down\n");
+	pci_host_recover_root_port(port);
+}
+EXPORT_SYMBOL_GPL(pci_host_handle_link_down);
+
 MODULE_DESCRIPTION("Common library for PCI host controller drivers");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/pci/controller/pci-host-common.h b/drivers/pci/controller/pci-host-common.h
index 9f0f36a32221..51afd65d3c4a 100644
--- a/drivers/pci/controller/pci-host-common.h
+++ b/drivers/pci/controller/pci-host-common.h
@@ -48,6 +48,7 @@ int pci_host_common_init(struct platform_device *pdev,
 			 struct pci_host_bridge *bridge,
 			 const struct pci_ecam_ops *ops);
 void pci_host_common_remove(struct platform_device *pdev);
+void pci_host_handle_link_down(struct pci_dev *port);
 
 struct pci_config_window *pci_host_common_ecam_create(struct device *dev,
 	struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 12e099489690..ff6d5d059b21 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5700,6 +5700,7 @@ int pci_bus_error_reset(struct pci_dev *bridge)
 {
 	return pci_reset_bridge(bridge, PCI_RESET_NO_RESTORE);
 }
+EXPORT_SYMBOL_GPL(pci_bus_error_reset);
 
 int pci_try_reset_bridge(struct pci_dev *bridge)
 {
diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index 13b9d9eb714f..d77403d8855b 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -292,3 +292,4 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
 
 	return status;
 }
+EXPORT_SYMBOL_GPL(pcie_do_recovery);

-- 
2.51.0

Re: [PATCH v9 3/5] PCI: host-common: Add link down handling for Root Ports

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Silent device state corruption on link recovery when AER is disabled.
--

--- Patch [3]: [PATCH v9 3/5] PCI: host-common: Add link down handling for Root Ports ---
commit f0b63bf7d7d526d3c654256d686360e1e2906b25
Author: Manivannan Sadhasivam <mani@kernel.org>

PCI: host-common: Add link down handling for Root Ports

This commit introduces a new API pci_host_handle_link_down() to recover PCI
links that go down. It uses pcie_do_recovery() for AER-enabled kernels to
handle fatal errors and coordinate downstream driver recovery. For kernels
without AER, it falls back to resetting the root port directly.

> diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> index 2ce6f4b661334..363d3f970b10c 100644
> --- a/drivers/pci/controller/pci-host-common.c
> +++ b/drivers/pci/controller/pci-host-common.c

[ ... ]

> +static void pci_host_recover_root_port(struct pci_dev *port)
> +{
> +#if IS_ENABLED(CONFIG_PCIEAER)
> +	pcie_do_recovery(port, pci_channel_io_frozen, pci_host_reset_root_port);
> +#else
> +	pci_host_reset_root_port(port);

[Severity: High]
Without AER enabled, this falls back to calling pci_host_reset_root_port(),
which resets the downstream bus using pci_bus_error_reset().

Because pci_bus_error_reset() utilizes PCI_RESET_NO_RESTORE, does this wipe
the configuration space (BARs, MSI, Memory/Bus Master Enable) of all
downstream devices?

Since pcie_do_recovery() is bypassed in this branch, downstream drivers are
never notified via slot_reset callbacks to restore their configuration state.
Could those drivers remain active and continue attempting MMIO accesses to
unmapped or disabled memory regions, leading to Master Aborts, timeouts,
or kernel crashes?

> +#endif
> +}
> +
> +void pci_host_handle_link_down(struct pci_dev *port)
> +{
> +	pci_info(port, "Recovering Root Port due to Link Down\n");
> +	pci_host_recover_root_port(port);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-pci-port-reset-v9-0-53570b92064d@oss.qualcomm.com?part=3

[PATCH v9 4/5] PCI: qcom: Add support for resetting the Root Port due to link down event

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <mani@kernel.org>

The PCIe link can go down under circumstances such as the device firmware
crash, link instability, etc... When that happens, the PCIe Root Port needs
to be reset to make it operational again. Currently, the driver is not
handling the link down event, due to which the users have to restart the
machine to make PCIe link operational again. So fix it by detecting the
link down event and resetting the Root Port.

Since the Qcom PCIe controllers report the link down event through the
'global' IRQ, enable the link down event by setting PARF_INT_ALL_LINK_DOWN
bit in PARF_INT_ALL_MASK register.

In the case of the event, iterate through the available Root Ports and call
pci_host_handle_link_down() API with Root Port 'pci_dev' to let the PCI
core handle the link down condition. Since Qcom PCIe controllers only
support one Root Port per controller instance, the API will be called only
once. But the looping is necessary as there is no PCI API available to
fetch the Root Port instance without the child 'pci_dev'.

The API will internally call, 'pci_host_bridge::reset_root_port()' callback
to reset the Root Port in a platform specific way. So implement the
callback to reset the Root Port by first resetting the PCIe core, followed
by reinitializing the resources and then finally starting the link again.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Tested-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Reviewed-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 140 ++++++++++++++++++++++++++++++++-
 1 file changed, 139 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index d8eb52857f69..62b222c20464 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -56,6 +56,10 @@
 #define PARF_AXI_MSTR_WR_ADDR_HALT_V2		0x1a8
 #define PARF_Q2A_FLUSH				0x1ac
 #define PARF_LTSSM				0x1b0
+#define PARF_INT_ALL_STATUS			0x224
+#define PARF_INT_ALL_CLEAR			0x228
+#define PARF_INT_ALL_MASK			0x22c
+#define PARF_STATUS				0x230
 #define PARF_SID_OFFSET				0x234
 #define PARF_BDF_TRANSLATE_CFG			0x24c
 #define PARF_DBI_BASE_ADDR_V2			0x350
@@ -133,6 +137,13 @@
 /* PARF_LTSSM register fields */
 #define LTSSM_EN				BIT(8)
 #define PARF_LTSSM_STATE_MASK			GENMASK(5, 0)
+#define SW_CLEAR_FLUSH_MODE			BIT(10)
+#define FLUSH_MODE				BIT(11)
+
+/* PARF_INT_ALL_{STATUS/CLEAR/MASK} register fields */
+#define INT_ALL_LINK_DOWN			1
+#define PARF_INT_ALL_LINK_DOWN			BIT(INT_ALL_LINK_DOWN)
+#define PARF_INT_MSI_DEV_0_7			GENMASK(30, 23)
 
 /* PARF_NO_SNOOP_OVERRIDE register fields */
 #define WR_NO_SNOOP_OVERRIDE_EN			BIT(1)
@@ -144,6 +155,9 @@
 /* PARF_BDF_TO_SID_CFG fields */
 #define BDF_TO_SID_BYPASS			BIT(0)
 
+/* PARF_STATUS fields */
+#define FLUSH_COMPLETED				BIT(8)
+
 /* ELBI_SYS_CTRL register fields */
 #define ELBI_SYS_CTRL_LT_ENABLE			BIT(0)
 #define ELBI_SYS_CTRL_PME_TURNOFF_MSG		BIT(4)
@@ -172,6 +186,7 @@
 						PCIE_CAP_SLOT_POWER_LIMIT_SCALE)
 
 #define PERST_DELAY_US				1000
+#define FLUSH_TIMEOUT_US			100
 
 #define QCOM_PCIE_CRC8_POLYNOMIAL		(BIT(2) | BIT(1) | BIT(0))
 
@@ -291,10 +306,13 @@ struct qcom_pcie {
 	struct dentry *debugfs;
 	struct list_head ports;
 	struct gpio_desc *reset;
+	int global_irq;
 	bool use_pm_opp;
 };
 
 #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
+static int qcom_pcie_reset_root_port(struct pci_host_bridge *bridge,
+				  struct pci_dev *pdev);
 
 static void __qcom_pcie_perst_assert(struct qcom_pcie *pcie, bool assert)
 {
@@ -1406,6 +1424,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 			goto err_assert_reset;
 	}
 
+	pp->bridge->reset_root_port = qcom_pcie_reset_root_port;
+
 	return 0;
 
 err_assert_reset:
@@ -1734,6 +1754,75 @@ static int qcom_pcie_set_max_opp(struct device *dev)
 	return ret;
 }
 
+/*
+ * Qcom PCIe controllers only support one Root Port per controller instance. So
+ * this function ignores the 'pci_dev' associated with the Root Port and just
+ * resets the host bridge, which in turn resets the Root Port also.
+ */
+static int qcom_pcie_reset_root_port(struct pci_host_bridge *bridge,
+				  struct pci_dev *pdev)
+{
+	struct device *dev = bridge->dev.parent;
+	struct qcom_pcie *pcie = dev_get_drvdata(dev);
+	struct dw_pcie *pci = pcie->pci;
+	struct dw_pcie_rp *pp = &pci->pp;
+	u32 val;
+	int ret;
+
+	/* Wait for the pending transactions to be completed */
+	ret = readl_relaxed_poll_timeout(pcie->parf + PARF_STATUS, val,
+					 val & FLUSH_COMPLETED, 10,
+					 FLUSH_TIMEOUT_US);
+	if (ret) {
+		dev_err(dev, "Flush completion failed: %d\n", ret);
+		return ret;
+	}
+
+	/* Clear the FLUSH_MODE to allow the core to be reset */
+	val = readl(pcie->parf + PARF_LTSSM);
+	val |= SW_CLEAR_FLUSH_MODE;
+	writel(val, pcie->parf + PARF_LTSSM);
+
+	/* Wait for the FLUSH_MODE to clear */
+	ret = readl_relaxed_poll_timeout(pcie->parf + PARF_LTSSM, val,
+					 !(val & FLUSH_MODE), 10,
+					 FLUSH_TIMEOUT_US);
+	if (ret) {
+		dev_err(dev, "Flush mode clear failed: %d\n", ret);
+		return ret;
+	}
+
+	qcom_pcie_host_deinit(pp);
+
+	ret = qcom_pcie_host_init(pp);
+	if (ret) {
+		dev_err(dev, "Host init failed\n");
+		return ret;
+	}
+
+	ret = dw_pcie_setup_rc(pp);
+	if (ret)
+		return ret;
+
+	/*
+	 * Re-enable global IRQ events as the PARF_INT_ALL_MASK register is
+	 * non-sticky.
+	 */
+	if (pcie->global_irq)
+		writel_relaxed(PARF_INT_ALL_LINK_DOWN | PARF_INT_MSI_DEV_0_7,
+				pcie->parf + PARF_INT_ALL_MASK);
+
+	qcom_pcie_start_link(pci);
+
+	ret = dw_pcie_wait_for_link(pci);
+	if (ret)
+		return ret;
+
+	dev_dbg(dev, "Root Port reset completed\n");
+
+	return 0;
+}
+
 static int qcom_pcie_link_transition_count(struct seq_file *s, void *data)
 {
 	struct qcom_pcie *pcie = (struct qcom_pcie *)dev_get_drvdata(s->private);
@@ -1771,6 +1860,27 @@ static void qcom_pcie_init_debugfs(struct qcom_pcie *pcie)
 				    qcom_pcie_link_transition_count);
 }
 
+static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
+{
+	struct qcom_pcie *pcie = data;
+	struct dw_pcie_rp *pp = &pcie->pci->pp;
+	struct device *dev = pcie->pci->dev;
+	struct pci_dev *port;
+	unsigned long status = readl_relaxed(pcie->parf + PARF_INT_ALL_STATUS);
+
+	writel_relaxed(status, pcie->parf + PARF_INT_ALL_CLEAR);
+
+	if (test_and_clear_bit(INT_ALL_LINK_DOWN, &status)) {
+		dev_dbg(dev, "Received Link down event\n");
+		for_each_pci_bridge(port, pp->bridge->bus) {
+			if (pci_pcie_type(port) == PCI_EXP_TYPE_ROOT_PORT)
+				pci_host_handle_link_down(port);
+		}
+	}
+
+	return IRQ_HANDLED;
+}
+
 static void qcom_pci_free_msi(void *ptr)
 {
 	struct dw_pcie_rp *pp = (struct dw_pcie_rp *)ptr;
@@ -1990,7 +2100,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 	struct dw_pcie_rp *pp;
 	struct resource *res;
 	struct dw_pcie *pci;
-	int ret;
+	int ret, irq;
 
 	pcie_cfg = of_device_get_match_data(dev);
 	if (!pcie_cfg) {
@@ -2135,6 +2245,32 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 		goto err_phy_exit;
 	}
 
+	irq = platform_get_irq_byname_optional(pdev, "global");
+	if (irq > 0) {
+		const char *name;
+
+		name = devm_kasprintf(dev, GFP_KERNEL, "qcom_pcie_global_irq%d",
+				      pci_domain_nr(pp->bridge->bus));
+		if (!name) {
+			ret = -ENOMEM;
+			goto err_host_deinit;
+		}
+
+		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+						qcom_pcie_global_irq_thread,
+						IRQF_ONESHOT, name, pcie);
+		if (ret) {
+			dev_err_probe(&pdev->dev, ret,
+				      "Failed to request Global IRQ\n");
+			goto err_host_deinit;
+		}
+
+		writel_relaxed(PARF_INT_ALL_LINK_DOWN | PARF_INT_MSI_DEV_0_7,
+				pcie->parf + PARF_INT_ALL_MASK);
+
+		pcie->global_irq = irq;
+	}
+
 	qcom_pcie_icc_opp_update(pcie);
 
 	if (pcie->mhi)
@@ -2142,6 +2278,8 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 
 	return 0;
 
+err_host_deinit:
+	dw_pcie_host_deinit(pp);
 err_phy_exit:
 	list_for_each_entry_safe(port, tmp_port, &pcie->ports, list) {
 		list_for_each_entry_safe(perst, tmp_perst, &port->perst, list)

-- 
2.51.0

Re: [PATCH v9 4/5] PCI: qcom: Add support for resetting the Root Port due to link down event

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Unprotected iteration over PCI bus devices risks list corruption and use-after-free.
- [High] Accidental routing of MSI interrupts to the global IRQ thread causes an interrupt storm.
- [Medium] Platform devices created for power controllers are leaked if the newly added global IRQ request fails.
--

--- Patch [4]: [PATCH v9 4/5] PCI: qcom: Add support for resetting the Root Port due to link down event ---
commit 89d5e2336d3e173c1acc29f1a3ce080e06d174b3
Author: Manivannan Sadhasivam <mani@kernel.org>

PCI: qcom: Add support for resetting the Root Port due to link down event

This commit adds handling for PCIe link down events on Qcom controllers
by resetting the Root Port. It enables the link down event in the 
PARF_INT_ALL_MASK register and requests a global IRQ to detect and recover
from these events.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
[ ... ]
> @@ -1734,6 +1754,75 @@ static int qcom_pcie_set_max_opp(struct device *dev)
[ ... ]
> +static int qcom_pcie_reset_root_port(struct pci_host_bridge *bridge,
> +				  struct pci_dev *pdev)
> +{
[ ... ]
> +	/*
> +	 * Re-enable global IRQ events as the PARF_INT_ALL_MASK register is
> +	 * non-sticky.
> +	 */
> +	if (pcie->global_irq)
> +		writel_relaxed(PARF_INT_ALL_LINK_DOWN | PARF_INT_MSI_DEV_0_7,
> +				pcie->parf + PARF_INT_ALL_MASK);

[Severity: High]
Does unmasking PARF_INT_MSI_DEV_0_7 here (and during probe) inadvertently
route downstream MSI interrupts to the global IRQ thread instead of the
standard MSI handler?

Since modern endpoints can generate thousands of MSIs per second, could
this cause an interrupt storm that saturates the CPU when the global
threaded handler is woken up for every single MSI event?

> +
> +	qcom_pcie_start_link(pci);
[ ... ]
> +static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
> +{
> +	struct qcom_pcie *pcie = data;
> +	struct dw_pcie_rp *pp = &pcie->pci->pp;
> +	struct device *dev = pcie->pci->dev;
> +	struct pci_dev *port;
> +	unsigned long status = readl_relaxed(pcie->parf + PARF_INT_ALL_STATUS);
> +
> +	writel_relaxed(status, pcie->parf + PARF_INT_ALL_CLEAR);

[Severity: High]
Since PARF_INT_MSI_DEV_0_7 is unmasked, will this unconditional read and
clear of PARF_INT_ALL_STATUS steal the MSI events from the actual DWC MSI
handler? Might this cause MSIs to be silently dropped?

> +
> +	if (test_and_clear_bit(INT_ALL_LINK_DOWN, &status)) {
> +		dev_dbg(dev, "Received Link down event\n");
> +		for_each_pci_bridge(port, pp->bridge->bus) {

[Severity: High]
Is it safe to iterate over pp->bridge->bus->devices without holding the
pci_bus_sem reader lock?

If a user triggers the removal of the root port device via sysfs concurrently
with a link down event, could this cause list corruption or a use-after-free?

Also, does the loop need to take a reference to the port device via
pci_dev_get() before passing it to pci_host_handle_link_down() to ensure
the device structure isn't freed while being handled?

> +			if (pci_pcie_type(port) == PCI_EXP_TYPE_ROOT_PORT)
> +				pci_host_handle_link_down(port);
> +		}
> +	}
> +
> +	return IRQ_HANDLED;
> +}
[ ... ]
> @@ -2135,6 +2245,32 @@ static int qcom_pcie_probe(struct platform_device *pdev)
[ ... ]
> +		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> +						qcom_pcie_global_irq_thread,
> +						IRQF_ONESHOT, name, pcie);
> +		if (ret) {
> +			dev_err_probe(&pdev->dev, ret,
> +				      "Failed to request Global IRQ\n");
> +			goto err_host_deinit;

[Severity: Medium]
Can this error path cause a resource leak of platform devices?

When qcom_pcie_host_init() succeeds earlier in probe, it creates power
controller devices via pci_pwrctrl_create_devices(). If the newly added
global IRQ request then fails here, it jumps to err_host_deinit which
eventually calls qcom_pcie_host_deinit().

However, qcom_pcie_host_deinit() explicitly assumes it is only called
during system suspend and skips calling pci_pwrctrl_destroy_devices():

	if (!pci->pp.skip_pwrctrl_off) {
		/*
		 * No need to destroy pwrctrl devices as this function only
		 * gets called during system suspend as of now.
		 */
		pci_pwrctrl_power_off_devices(pci->dev);
	}

Will this leave the power controller devices permanently leaked on probe
failure?

> +		}
> +
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-pci-port-reset-v9-0-53570b92064d@oss.qualcomm.com?part=4

[PATCH v9 5/5] misc: pci_endpoint_test: Add AER error handlers

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

This Endpoint test driver doesn't need to do anything fancy in its error
handlers, but just restore the config space that was saved during probe and
report the correct result. This helps in making sure that the AER recovery
succeeds.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/misc/pci_endpoint_test.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index 3635741c3e7a..b7a149715700 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -1325,6 +1325,8 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
 	misc_device->parent = &pdev->dev;
 	misc_device->fops = &pci_endpoint_test_fops;
 
+	pci_save_state(pdev);
+
 	ret = misc_register(misc_device);
 	if (ret) {
 		dev_err(dev, "Failed to register device\n");
@@ -1452,12 +1454,33 @@ static const struct pci_device_id pci_endpoint_test_tbl[] = {
 };
 MODULE_DEVICE_TABLE(pci, pci_endpoint_test_tbl);
 
+static pci_ers_result_t pci_endpoint_test_error_detected(struct pci_dev *pdev,
+					      pci_channel_state_t state)
+{
+	if (state == pci_channel_io_perm_failure)
+		return PCI_ERS_RESULT_DISCONNECT;
+
+	return PCI_ERS_RESULT_NEED_RESET;
+}
+
+static pci_ers_result_t pci_endpoint_test_slot_reset(struct pci_dev *pdev)
+{
+	pci_restore_state(pdev);
+	return PCI_ERS_RESULT_RECOVERED;
+}
+
+static const struct pci_error_handlers pci_endpoint_test_err_handler = {
+	.error_detected = pci_endpoint_test_error_detected,
+	.slot_reset = pci_endpoint_test_slot_reset,
+};
+
 static struct pci_driver pci_endpoint_test_driver = {
 	.name		= DRV_MODULE_NAME,
 	.id_table	= pci_endpoint_test_tbl,
 	.probe		= pci_endpoint_test_probe,
 	.remove		= pci_endpoint_test_remove,
 	.sriov_configure = pci_sriov_configure_simple,
+	.err_handler    = &pci_endpoint_test_err_handler,
 };
 module_pci_driver(pci_endpoint_test_driver);
 

-- 
2.51.0

Re: [PATCH v9 5/5] misc: pci_endpoint_test: Add AER error handlers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] AER error handlers do not abort pending tests, causing ioctl threads to hang permanently in D state.

Pre-existing issues:
- [High] A use-after-free vulnerability exists in the ioctl handler if the device is removed while a file descriptor remains open.
--

--- Patch [5]: [PATCH v9 5/5] misc: pci_endpoint_test: Add AER error handlers ---
commit 1d34d50e68e455ba411f0fd8383353d65b5616cf
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

misc: pci_endpoint_test: Add AER error handlers

This commit introduces AER error handlers to the PCI endpoint test driver.
It adds functions to restore the configuration space and report the correct
recovery state when an error is detected.

> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> index 3635741c3e7a7..b7a1497157008 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -1325,6 +1325,8 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
>  	misc_device->parent = &pdev->dev;
>  	misc_device->fops = &pci_endpoint_test_fops;
>  
> +	pci_save_state(pdev);
> +
>  	ret = misc_register(misc_device);

[Severity: High]
This is a pre-existing issue, but does this code have a use-after-free
vulnerability if the device is removed while a file descriptor remains open?

The test structure is allocated earlier in pci_endpoint_test_probe() using
devm_kzalloc(). If a userspace process has the misc device open and the
PCI device is unbound, pci_endpoint_test_remove() calls misc_deregister()
which doesn't block waiting for open file descriptors to close.

When the device unbinds, devres will free the test structure. If the
userspace process then calls ioctl(), pci_endpoint_test_ioctl() will retrieve
the freed structure from file->private_data and dereference it when attempting
to acquire the mutex:

struct pci_endpoint_test *test = to_endpoint_test(file->private_data);
struct pci_dev *pdev = test->pdev;

mutex_lock(&test->mutex);

Could this be fixed by changing the lifetime management of the test structure
or ensuring the teardown synchronization waits for open file descriptors?

[ ... ]

> @@ -1452,12 +1454,33 @@ static const struct pci_device_id pci_endpoint_test_tbl[] = {
>  };
>  MODULE_DEVICE_TABLE(pci, pci_endpoint_test_tbl);
>  
> +static pci_ers_result_t pci_endpoint_test_error_detected(struct pci_dev *pdev,
> +					      pci_channel_state_t state)
> +{
> +	if (state == pci_channel_io_perm_failure)
> +		return PCI_ERS_RESULT_DISCONNECT;
> +
> +	return PCI_ERS_RESULT_NEED_RESET;
> +}

[Severity: Medium]
Will this error handler leave tasks permanently hanging in D state if an AER
error occurs during a test?

Test functions like pci_endpoint_test_write(), pci_endpoint_test_read(), or
pci_endpoint_test_copy() initiate DMA and sleep on
wait_for_completion(&test->irq_raised) without a timeout while holding
test->mutex:

pci_endpoint_test_write() {
    ...
    wait_for_completion(&test->irq_raised);
    ...
}

If an AER error occurs during this wait, the newly introduced error handlers
execute to recover the link but fail to complete the wait queue. Since the
device state is reset, the expected interrupt never arrives, leaving the ioctl
thread stuck forever.

Should the error handler complete test->irq_raised to wake up pending ioctl
threads and abort the tests?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-pci-port-reset-v9-0-53570b92064d@oss.qualcomm.com?part=5