Introduce CXL Port protocol error handling callbacks to unify detection,
logging, and recovery across CXL Ports and Endpoints, including RCH
downstream ports. Establish a consistent flow for correctable and
uncorrectable CXL protocol errors.

Provide the solution by adding cxl_port_cor_error_detected() and
cxl_port_error_detected() to handle correctable and uncorrectable handling
through CXL RAS helpers, coordinating uncorrectable recovery in
cxl_do_recovery(), and panicking when the handler returns PCI_ERS_RESULT_PANIC
to preserve fatal cachemem behavior. Gate endpoint handling on the endpoint
driver being bound to avoid processing errors on disabled devices.

Centralize the RAS base lookup in cxl_get_ras_base(), selecting the
downstream-port dport->regs.ras for Root/Downstream Ports and port->regs.ras
for Upstream Ports/Endpoints.

Export pcie_clear_device_status() and pci_aer_clear_fatal_status() to enable
cxl_core to clear PCIe/AER state in these flows.

Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Dave Jiang dave.jiang@intel.com

---

Changes in v14->v15:
- Update commit message and title. Added Bjorn's ack.
- Move CE and UCE handling logic here

Changes in v13->v14:
- Add Dave Jiang's review-by
- Update commit message & headline (Bjorn)
- Refactor cxl_port_error_detected()/cxl_port_cor_error_detected() to
  one line (Jonathan)
- Remove cxl_walk_port() (Dan)
- Remove cxl_pci_drv_bound(). Check for 'is_cxl' parent port is
  sufficient (Dan)
- Remove device_lock_if()
- Combined CE and UCE here (Terry)

Changes in v12->v13:
- Move get_pci_cxl_host_dev() and cxl_handle_proto_error() to Dequeue
  patch (Terry)
- Remove EP case in cxl_get_ras_base(), not used. (Terry)
- Remove check for dport->dport_dev (Dave)
- Remove whitespace (Terry)

Changes in v11->v12:
- Add call to cxl_pci_drv_bound() in cxl_handle_proto_error() and
  pci_to_cxl_dev()
- Change cxl_error_detected() -> cxl_cor_error_detected()
- Remove NULL variable assignments
- Replace bus_find_device() with find_cxl_port_by_uport() for upstream
  port searches.

Changes in v10->v11:
- None
---
 drivers/cxl/core/ras.c | 134 +++++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.c      |   1 +
 drivers/pci/pci.h      |   2 -
 drivers/pci/pcie/aer.c |   1 +
 include/linux/aer.h    |   2 +
 include/linux/pci.h    |   2 +
 6 files changed, 140 insertions(+), 2 deletions(-)

diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
index a6c0bc6d7203..0216dafa6118 100644
--- a/drivers/cxl/core/ras.c
+++ b/drivers/cxl/core/ras.c
@@ -218,6 +218,68 @@ static struct cxl_port *get_cxl_port(struct pci_dev *pdev)
 	return NULL;
 }
 
+static void __iomem *cxl_get_ras_base(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+
+	switch (pci_pcie_type(pdev)) {
+	case PCI_EXP_TYPE_ROOT_PORT:
+	case PCI_EXP_TYPE_DOWNSTREAM:
+	{
+		struct cxl_dport *dport;
+		struct cxl_port *port __free(put_cxl_port) = find_cxl_port(&pdev->dev, &dport);
+
+		if (!dport) {
+			pci_err(pdev, "Failed to find the CXL device");
+			return NULL;
+		}
+		return dport->regs.ras;
+	}
+	case PCI_EXP_TYPE_UPSTREAM:
+	case PCI_EXP_TYPE_ENDPOINT:
+	{
+		struct cxl_port *port __free(put_cxl_port) = find_cxl_port_by_uport(&pdev->dev);
+
+		if (!port) {
+			pci_err(pdev, "Failed to find the CXL device");
+			return NULL;
+		}
+		return port->regs.ras;
+	}
+	}
+	dev_warn_once(dev, "Error: Unsupported device type (%#x)", pci_pcie_type(pdev));
+	return NULL;
+}
+
+static pci_ers_result_t cxl_port_error_detected(struct device *dev);
+
+static void cxl_do_recovery(struct pci_dev *pdev)
+{
+	struct cxl_port *port __free(put_cxl_port) = get_cxl_port(pdev);
+	pci_ers_result_t status;
+
+	if (!port) {
+		pci_err(pdev, "Failed to find the CXL device\n");
+		return;
+	}
+
+	status = cxl_port_error_detected(&pdev->dev);
+	if (status == PCI_ERS_RESULT_PANIC)
+		panic("CXL cachemem error.");
+
+	/*
+	 * If we have native control of AER, clear error status in the device
+	 * that detected the error.  If the platform retained control of AER,
+	 * it is responsible for clearing this status.  In that case, the
+	 * signaling device may not even be visible to the OS.
+	 */
+	if (pcie_aer_is_native(pdev)) {
+		pcie_clear_device_status(pdev);
+		pci_aer_clear_nonfatal_status(pdev);
+		pci_aer_clear_fatal_status(pdev);
+	}
+}
+
 void cxl_handle_cor_ras(struct device *dev, u64 serial, void __iomem *ras_base)
 {
 	void __iomem *addr;
@@ -288,6 +350,60 @@ bool cxl_handle_ras(struct device *dev, u64 serial, void __iomem *ras_base)
 	return true;
 }
 
+static void cxl_port_cor_error_detected(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct cxl_port *port __free(put_cxl_port) = get_cxl_port(pdev);
+
+	if (is_cxl_endpoint(port)) {
+		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
+		struct cxl_dev_state *cxlds = cxlmd->cxlds;
+
+		guard(device)(&cxlmd->dev);
+
+		if (!dev->driver) {
+			dev_warn(&pdev->dev,
+				 "%s: memdev disabled, abort error handling\n",
+				 dev_name(dev));
+			return;
+		}
+
+		if (cxlds->rcd)
+			cxl_handle_rdport_errors(cxlds);
+
+		cxl_handle_cor_ras(dev, cxlds->serial, cxl_get_ras_base(dev));
+	} else {
+		cxl_handle_cor_ras(dev, 0, cxl_get_ras_base(dev));
+	}
+}
+
+static pci_ers_result_t cxl_port_error_detected(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct cxl_port *port __free(put_cxl_port) = get_cxl_port(pdev);
+
+	if (is_cxl_endpoint(port)) {
+		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
+		struct cxl_dev_state *cxlds = cxlmd->cxlds;
+
+		guard(device)(&cxlmd->dev);
+
+		if (!dev->driver) {
+			dev_warn(&pdev->dev,
+				 "%s: memdev disabled, abort error handling\n",
+				 dev_name(dev));
+			return PCI_ERS_RESULT_NONE;
+		}
+
+		if (cxlds->rcd)
+			cxl_handle_rdport_errors(cxlds);
+
+		return cxl_handle_ras(dev, cxlds->serial, cxl_get_ras_base(dev));
+	} else {
+		return cxl_handle_ras(dev, 0, cxl_get_ras_base(dev));
+	}
+}
+
 void cxl_cor_error_detected(struct pci_dev *pdev)
 {
 	struct cxl_dev_state *cxlds = pci_get_drvdata(pdev);
@@ -363,6 +479,24 @@ EXPORT_SYMBOL_NS_GPL(cxl_error_detected, "CXL");
 
 static void cxl_handle_proto_error(struct cxl_proto_err_work_data *err_info)
 {
+	struct pci_dev *pdev = err_info->pdev;
+
+	if (err_info->severity == AER_CORRECTABLE) {
+
+		if (!pcie_aer_is_native(pdev))
+			return;
+
+		if (pdev->aer_cap)
+			pci_clear_and_set_config_dword(pdev,
+						       pdev->aer_cap + PCI_ERR_COR_STATUS,
+						       0, PCI_ERR_COR_INTERNAL);
+
+		cxl_port_cor_error_detected(&pdev->dev);
+
+		pcie_clear_device_status(pdev);
+	} else {
+		cxl_do_recovery(pdev);
+	}
 }
 
 static void cxl_proto_err_work_fn(struct work_struct *work)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 13dbb405dc31..b7bfefdaf990 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2248,6 +2248,7 @@ void pcie_clear_device_status(struct pci_dev *dev)
 	pcie_capability_read_word(dev, PCI_EXP_DEVSTA, &sta);
 	pcie_capability_write_word(dev, PCI_EXP_DEVSTA, sta);
 }
+EXPORT_SYMBOL_GPL(pcie_clear_device_status);
 #endif
 
 /**
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 8ccb3ba61e11..d81c4170f595 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -229,7 +229,6 @@ void pci_refresh_power_state(struct pci_dev *dev);
 int pci_power_up(struct pci_dev *dev);
 void pci_disable_enabled_device(struct pci_dev *dev);
 int pci_finish_runtime_suspend(struct pci_dev *dev);
-void pcie_clear_device_status(struct pci_dev *dev);
 void pcie_clear_root_pme_status(struct pci_dev *dev);
 bool pci_check_pme_status(struct pci_dev *dev);
 void pci_pme_wakeup_bus(struct pci_bus *bus);
@@ -1198,7 +1197,6 @@ void pci_restore_aer_state(struct pci_dev *dev);
 static inline void pci_no_aer(void) { }
 static inline void pci_aer_init(struct pci_dev *d) { }
 static inline void pci_aer_exit(struct pci_dev *d) { }
-static inline void pci_aer_clear_fatal_status(struct pci_dev *dev) { }
 static inline int pci_aer_clear_status(struct pci_dev *dev) { return -EINVAL; }
 static inline int pci_aer_raw_clear_status(struct pci_dev *dev) { return -EINVAL; }
 static inline void pci_save_aer_state(struct pci_dev *dev) { }
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 7af10a74da34..4fc9de4c78f8 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -298,6 +298,7 @@ void pci_aer_clear_fatal_status(struct pci_dev *dev)
 	if (status)
 		pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, status);
 }
+EXPORT_SYMBOL_GPL(pci_aer_clear_fatal_status);
 
 /**
  * pci_aer_raw_clear_status - Clear AER error registers.
diff --git a/include/linux/aer.h b/include/linux/aer.h
index f351e41dd979..c1aef7859d0a 100644
--- a/include/linux/aer.h
+++ b/include/linux/aer.h
@@ -65,6 +65,7 @@ struct cxl_proto_err_work_data {
 
 #if defined(CONFIG_PCIEAER)
 int pci_aer_clear_nonfatal_status(struct pci_dev *dev);
+void pci_aer_clear_fatal_status(struct pci_dev *dev);
 int pcie_aer_is_native(struct pci_dev *dev);
 void pci_aer_unmask_internal_errors(struct pci_dev *dev);
 #else
@@ -72,6 +73,7 @@ static inline int pci_aer_clear_nonfatal_status(struct pci_dev *dev)
 {
 	return -EINVAL;
 }
+static inline void pci_aer_clear_fatal_status(struct pci_dev *dev) { }
 static inline int pcie_aer_is_native(struct pci_dev *dev) { return 0; }
 static inline void pci_aer_unmask_internal_errors(struct pci_dev *dev) { }
 #endif
diff --git a/include/linux/pci.h b/include/linux/pci.h
index ee05d5925b13..1ef4743bf151 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1921,8 +1921,10 @@ static inline void pci_hp_unignore_link_change(struct pci_dev *pdev) { }
 
 #ifdef CONFIG_PCIEAER
 bool pci_aer_available(void);
+void pcie_clear_device_status(struct pci_dev *dev);
 #else
 static inline bool pci_aer_available(void) { return false; }
+static inline void pcie_clear_device_status(struct pci_dev *dev) { }
 #endif
 
 bool pci_ats_disabled(void);
-- 
2.34.1

Terry Bowman wrote:
> Introduce CXL Port protocol error handling callbacks to unify detection,
> logging, and recovery across CXL Ports and Endpoints, including RCH
> downstream ports. Establish a consistent flow for correctable and
> uncorrectable CXL protocol errors.
> 
> Provide the solution by adding cxl_port_cor_error_detected() and
> cxl_port_error_detected() to handle correctable and uncorrectable handling
> through CXL RAS helpers, coordinating uncorrectable recovery in
> cxl_do_recovery(), and panicking when the handler returns PCI_ERS_RESULT_PANIC
> to preserve fatal cachemem behavior. Gate endpoint handling on the endpoint
> driver being bound to avoid processing errors on disabled devices.
> 
> Centralize the RAS base lookup in cxl_get_ras_base(), selecting the
> downstream-port dport->regs.ras for Root/Downstream Ports and port->regs.ras
> for Upstream Ports/Endpoints.
> 
> Export pcie_clear_device_status() and pci_aer_clear_fatal_status() to enable
> cxl_core to clear PCIe/AER state in these flows.
> 
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> Reviewed-by: Dave Jiang dave.jiang@intel.com
> 
> ---
> 
> Changes in v14->v15:
> - Update commit message and title. Added Bjorn's ack.
> - Move CE and UCE handling logic here
> 
> Changes in v13->v14:
> - Add Dave Jiang's review-by
> - Update commit message & headline (Bjorn)
> - Refactor cxl_port_error_detected()/cxl_port_cor_error_detected() to
>   one line (Jonathan)
> - Remove cxl_walk_port() (Dan)
> - Remove cxl_pci_drv_bound(). Check for 'is_cxl' parent port is
>   sufficient (Dan)
> - Remove device_lock_if()
> - Combined CE and UCE here (Terry)
> 
> Changes in v12->v13:
> - Move get_pci_cxl_host_dev() and cxl_handle_proto_error() to Dequeue
>   patch (Terry)
> - Remove EP case in cxl_get_ras_base(), not used. (Terry)
> - Remove check for dport->dport_dev (Dave)
> - Remove whitespace (Terry)
> 
> Changes in v11->v12:
> - Add call to cxl_pci_drv_bound() in cxl_handle_proto_error() and
>   pci_to_cxl_dev()
> - Change cxl_error_detected() -> cxl_cor_error_detected()
> - Remove NULL variable assignments
> - Replace bus_find_device() with find_cxl_port_by_uport() for upstream
>   port searches.
> 
> Changes in v10->v11:
> - None
> ---
>  drivers/cxl/core/ras.c | 134 +++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/pci.c      |   1 +
>  drivers/pci/pci.h      |   2 -
>  drivers/pci/pcie/aer.c |   1 +
>  include/linux/aer.h    |   2 +
>  include/linux/pci.h    |   2 +
>  6 files changed, 140 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
> index a6c0bc6d7203..0216dafa6118 100644
> --- a/drivers/cxl/core/ras.c
> +++ b/drivers/cxl/core/ras.c
> @@ -218,6 +218,68 @@ static struct cxl_port *get_cxl_port(struct pci_dev *pdev)
>  	return NULL;
>  }
>  
> +static void __iomem *cxl_get_ras_base(struct device *dev)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +
> +	switch (pci_pcie_type(pdev)) {
> +	case PCI_EXP_TYPE_ROOT_PORT:
> +	case PCI_EXP_TYPE_DOWNSTREAM:
> +	{

Nit, clang-format puts that { on the same line because coding style says
only functions get newlines for open brackets.

> +		struct cxl_dport *dport;
> +		struct cxl_port *port __free(put_cxl_port) = find_cxl_port(&pdev->dev, &dport);
> +
> +		if (!dport) {
> +			pci_err(pdev, "Failed to find the CXL device");
> +			return NULL;
> +		}
> +		return dport->regs.ras;
> +	}
> +	case PCI_EXP_TYPE_UPSTREAM:
> +	case PCI_EXP_TYPE_ENDPOINT:
> +	{
> +		struct cxl_port *port __free(put_cxl_port) = find_cxl_port_by_uport(&pdev->dev);
> +
> +		if (!port) {
> +			pci_err(pdev, "Failed to find the CXL device");
> +			return NULL;
> +		}
> +		return port->regs.ras;
> +	}
> +	}
> +	dev_warn_once(dev, "Error: Unsupported device type (%#x)", pci_pcie_type(pdev));
> +	return NULL;
> +}
> +
> +static pci_ers_result_t cxl_port_error_detected(struct device *dev);
> +
> +static void cxl_do_recovery(struct pci_dev *pdev)
> +{
> +	struct cxl_port *port __free(put_cxl_port) = get_cxl_port(pdev);
> +	pci_ers_result_t status;
> +
> +	if (!port) {
> +		pci_err(pdev, "Failed to find the CXL device\n");
> +		return;
> +	}
> +
> +	status = cxl_port_error_detected(&pdev->dev);
> +	if (status == PCI_ERS_RESULT_PANIC)
> +		panic("CXL cachemem error.");
> +
> +	/*
> +	 * If we have native control of AER, clear error status in the device
> +	 * that detected the error.  If the platform retained control of AER,
> +	 * it is responsible for clearing this status.  In that case, the
> +	 * signaling device may not even be visible to the OS.
> +	 */

This comment feels more appropriate as documentation for
pcie_aer_is_native(). CXL is just using for the same purpose as all the
other callers. You can maybe reference "See pcie_aer_is_native() for
expecations on clearing errors", but I otherwise would not expect CXL to
carry its own paragraph.

> +	if (pcie_aer_is_native(pdev)) {
> +		pcie_clear_device_status(pdev);
> +		pci_aer_clear_nonfatal_status(pdev);
> +		pci_aer_clear_fatal_status(pdev);
> +	}
> +}
> +
>  void cxl_handle_cor_ras(struct device *dev, u64 serial, void __iomem *ras_base)
>  {
>  	void __iomem *addr;
> @@ -288,6 +350,60 @@ bool cxl_handle_ras(struct device *dev, u64 serial, void __iomem *ras_base)
>  	return true;
>  }
>  
> +static void cxl_port_cor_error_detected(struct device *dev)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct cxl_port *port __free(put_cxl_port) = get_cxl_port(pdev);
> +
> +	if (is_cxl_endpoint(port)) {
> +		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
> +		struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +
> +		guard(device)(&cxlmd->dev);
> +
> +		if (!dev->driver) {
> +			dev_warn(&pdev->dev,
> +				 "%s: memdev disabled, abort error handling\n",
> +				 dev_name(dev));
> +			return;
> +		}
> +
> +		if (cxlds->rcd)
> +			cxl_handle_rdport_errors(cxlds);

Isn't this dead code? Only VH topologies will ever get a forwarded CXL
error, right? I realize it gets deleted in a future patch, but then why
leave dead code in the git history?

> +
> +		cxl_handle_cor_ras(dev, cxlds->serial, cxl_get_ras_base(dev));
> +	} else {
> +		cxl_handle_cor_ras(dev, 0, cxl_get_ras_base(dev));
> +	}
> +}
> +
> +static pci_ers_result_t cxl_port_error_detected(struct device *dev)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct cxl_port *port __free(put_cxl_port) = get_cxl_port(pdev);
> +
> +	if (is_cxl_endpoint(port)) {
> +		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
> +		struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +
> +		guard(device)(&cxlmd->dev);
> +
> +		if (!dev->driver) {
> +			dev_warn(&pdev->dev,
> +				 "%s: memdev disabled, abort error handling\n",
> +				 dev_name(dev));
> +			return PCI_ERS_RESULT_NONE;
> +		}
> +
> +		if (cxlds->rcd)
> +			cxl_handle_rdport_errors(cxlds);
> +
> +		return cxl_handle_ras(dev, cxlds->serial, cxl_get_ras_base(dev));
> +	} else {
> +		return cxl_handle_ras(dev, 0, cxl_get_ras_base(dev));
> +	}
> +}
> +
>  void cxl_cor_error_detected(struct pci_dev *pdev)
>  {
>  	struct cxl_dev_state *cxlds = pci_get_drvdata(pdev);
> @@ -363,6 +479,24 @@ EXPORT_SYMBOL_NS_GPL(cxl_error_detected, "CXL");
>  
>  static void cxl_handle_proto_error(struct cxl_proto_err_work_data *err_info)
>  {
> +	struct pci_dev *pdev = err_info->pdev;
> +
> +	if (err_info->severity == AER_CORRECTABLE) {
> +
> +		if (!pcie_aer_is_native(pdev))
> +			return;
> +
> +		if (pdev->aer_cap)
> +			pci_clear_and_set_config_dword(pdev,
> +						       pdev->aer_cap + PCI_ERR_COR_STATUS,
> +						       0, PCI_ERR_COR_INTERNAL);
> +
> +		cxl_port_cor_error_detected(&pdev->dev);
> +
> +		pcie_clear_device_status(pdev);
> +	} else {
> +		cxl_do_recovery(pdev);
> +	}
>  }
>  
>  static void cxl_proto_err_work_fn(struct work_struct *work)
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 13dbb405dc31..b7bfefdaf990 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2248,6 +2248,7 @@ void pcie_clear_device_status(struct pci_dev *dev)
>  	pcie_capability_read_word(dev, PCI_EXP_DEVSTA, &sta);
>  	pcie_capability_write_word(dev, PCI_EXP_DEVSTA, sta);
>  }
> +EXPORT_SYMBOL_GPL(pcie_clear_device_status);

No reason to open up this symbol to the world. Only cxl_core.ko needs
this exported, and hopefully we never see another bus that abuses PCI
like CXL does ever again.

[..]
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 7af10a74da34..4fc9de4c78f8 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -298,6 +298,7 @@ void pci_aer_clear_fatal_status(struct pci_dev *dev)
>  	if (status)
>  		pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, status);
>  }
> +EXPORT_SYMBOL_GPL(pci_aer_clear_fatal_status);

ditto, too wide of an export.

On 2/3/2026 11:08 PM, dan.j.williams@intel.com wrote:
> Terry Bowman wrote:
>> Introduce CXL Port protocol error handling callbacks to unify detection,
>> logging, and recovery across CXL Ports and Endpoints, including RCH
>> downstream ports. Establish a consistent flow for correctable and
>> uncorrectable CXL protocol errors.
>>
>> Provide the solution by adding cxl_port_cor_error_detected() and
>> cxl_port_error_detected() to handle correctable and uncorrectable handling
>> through CXL RAS helpers, coordinating uncorrectable recovery in
>> cxl_do_recovery(), and panicking when the handler returns PCI_ERS_RESULT_PANIC
>> to preserve fatal cachemem behavior. Gate endpoint handling on the endpoint
>> driver being bound to avoid processing errors on disabled devices.
>>
>> Centralize the RAS base lookup in cxl_get_ras_base(), selecting the
>> downstream-port dport->regs.ras for Root/Downstream Ports and port->regs.ras
>> for Upstream Ports/Endpoints.
>>
>> Export pcie_clear_device_status() and pci_aer_clear_fatal_status() to enable
>> cxl_core to clear PCIe/AER state in these flows.
>>
>> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>> Reviewed-by: Dave Jiang dave.jiang@intel.com
>>
>> ---
>>
>> Changes in v14->v15:
>> - Update commit message and title. Added Bjorn's ack.
>> - Move CE and UCE handling logic here
>>
>> Changes in v13->v14:
>> - Add Dave Jiang's review-by
>> - Update commit message & headline (Bjorn)
>> - Refactor cxl_port_error_detected()/cxl_port_cor_error_detected() to
>>   one line (Jonathan)
>> - Remove cxl_walk_port() (Dan)
>> - Remove cxl_pci_drv_bound(). Check for 'is_cxl' parent port is
>>   sufficient (Dan)
>> - Remove device_lock_if()
>> - Combined CE and UCE here (Terry)
>>
>> Changes in v12->v13:
>> - Move get_pci_cxl_host_dev() and cxl_handle_proto_error() to Dequeue
>>   patch (Terry)
>> - Remove EP case in cxl_get_ras_base(), not used. (Terry)
>> - Remove check for dport->dport_dev (Dave)
>> - Remove whitespace (Terry)
>>
>> Changes in v11->v12:
>> - Add call to cxl_pci_drv_bound() in cxl_handle_proto_error() and
>>   pci_to_cxl_dev()
>> - Change cxl_error_detected() -> cxl_cor_error_detected()
>> - Remove NULL variable assignments
>> - Replace bus_find_device() with find_cxl_port_by_uport() for upstream
>>   port searches.
>>
>> Changes in v10->v11:
>> - None
>> ---
>>  drivers/cxl/core/ras.c | 134 +++++++++++++++++++++++++++++++++++++++++
>>  drivers/pci/pci.c      |   1 +
>>  drivers/pci/pci.h      |   2 -
>>  drivers/pci/pcie/aer.c |   1 +
>>  include/linux/aer.h    |   2 +
>>  include/linux/pci.h    |   2 +
>>  6 files changed, 140 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
>> index a6c0bc6d7203..0216dafa6118 100644
>> --- a/drivers/cxl/core/ras.c
>> +++ b/drivers/cxl/core/ras.c
>> @@ -218,6 +218,68 @@ static struct cxl_port *get_cxl_port(struct pci_dev *pdev)
>>  	return NULL;
>>  }
>>  
>> +static void __iomem *cxl_get_ras_base(struct device *dev)
>> +{
>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +
>> +	switch (pci_pcie_type(pdev)) {
>> +	case PCI_EXP_TYPE_ROOT_PORT:
>> +	case PCI_EXP_TYPE_DOWNSTREAM:
>> +	{
> 
> Nit, clang-format puts that { on the same line because coding style says
> only functions get newlines for open brackets.
> 

Hi Dan,

Thanks for the note. Would you like every switch-case to be upodated to match the clang 
recommended format?

>> +		struct cxl_dport *dport;
>> +		struct cxl_port *port __free(put_cxl_port) = find_cxl_port(&pdev->dev, &dport);
>> +
>> +		if (!dport) {
>> +			pci_err(pdev, "Failed to find the CXL device");
>> +			return NULL;
>> +		}
>> +		return dport->regs.ras;
>> +	}
>> +	case PCI_EXP_TYPE_UPSTREAM:
>> +	case PCI_EXP_TYPE_ENDPOINT:
>> +	{
>> +		struct cxl_port *port __free(put_cxl_port) = find_cxl_port_by_uport(&pdev->dev);
>> +
>> +		if (!port) {
>> +			pci_err(pdev, "Failed to find the CXL device");
>> +			return NULL;
>> +		}
>> +		return port->regs.ras;
>> +	}
>> +	}
>> +	dev_warn_once(dev, "Error: Unsupported device type (%#x)", pci_pcie_type(pdev));
>> +	return NULL;
>> +}
>> +
>> +static pci_ers_result_t cxl_port_error_detected(struct device *dev);
>> +
>> +static void cxl_do_recovery(struct pci_dev *pdev)
>> +{
>> +	struct cxl_port *port __free(put_cxl_port) = get_cxl_port(pdev);
>> +	pci_ers_result_t status;
>> +
>> +	if (!port) {
>> +		pci_err(pdev, "Failed to find the CXL device\n");
>> +		return;
>> +	}
>> +
>> +	status = cxl_port_error_detected(&pdev->dev);
>> +	if (status == PCI_ERS_RESULT_PANIC)
>> +		panic("CXL cachemem error.");
>> +
>> +	/*
>> +	 * If we have native control of AER, clear error status in the device
>> +	 * that detected the error.  If the platform retained control of AER,
>> +	 * it is responsible for clearing this status.  In that case, the
>> +	 * signaling device may not even be visible to the OS.
>> +	 */
> 
> This comment feels more appropriate as documentation for
> pcie_aer_is_native(). CXL is just using for the same purpose as all the
> other callers. You can maybe reference "See pcie_aer_is_native() for
> expecations on clearing errors", but I otherwise would not expect CXL to
> carry its own paragraph.
> 

Agreed. I’ll drop the local comment and rely on pcie_aer_is_native() semantics,
with a brief reference if needed.


>> +	if (pcie_aer_is_native(pdev)) {
>> +		pcie_clear_device_status(pdev);
>> +		pci_aer_clear_nonfatal_status(pdev);
>> +		pci_aer_clear_fatal_status(pdev);
>> +	}
>> +}
>> +
>>  void cxl_handle_cor_ras(struct device *dev, u64 serial, void __iomem *ras_base)
>>  {
>>  	void __iomem *addr;
>> @@ -288,6 +350,60 @@ bool cxl_handle_ras(struct device *dev, u64 serial, void __iomem *ras_base)
>>  	return true;
>>  }
>>  
>> +static void cxl_port_cor_error_detected(struct device *dev)
>> +{
>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +	struct cxl_port *port __free(put_cxl_port) = get_cxl_port(pdev);
>> +
>> +	if (is_cxl_endpoint(port)) {
>> +		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
>> +		struct cxl_dev_state *cxlds = cxlmd->cxlds;
>> +
>> +		guard(device)(&cxlmd->dev);
>> +
>> +		if (!dev->driver) {
>> +			dev_warn(&pdev->dev,
>> +				 "%s: memdev disabled, abort error handling\n",
>> +				 dev_name(dev));
>> +			return;
>> +		}
>> +
>> +		if (cxlds->rcd)
>> +			cxl_handle_rdport_errors(cxlds);
> 
> Isn't this dead code? Only VH topologies will ever get a forwarded CXL
> error, right? I realize it gets deleted in a future patch, but then why
> leave dead code in the git history?
> 
                            
Yes, agreed - I'll remove. Correct, only VH is forwarded. My understanding is the 
cxl_memdev guard and driver check are no longer required here. The memdev is only 
used to source the serial number, so I’ll refactor accordingly. Please correct
me if Im wrong.

I see an additional fix needed: cxl_rch_handle_error_iter() in pci/pcie/aer.c
also needs its callbacks updated. The RCH/RCD path previously invoked the EP
PCIe handlers, but with RAS now handled at the port level, those callbacks no
longer reach the correct logic.

I had a coupled ideas. One options is for the CXL logic to make a callback into a 
cxl_core exported function such as cxl_handle_rdport_errors(). BTW, the CXL logic in 
AER and the CXL driver's RAS are both built with the CONFIG_CXL_RAS config.

Another option is updating the CXL PCIe callbacks. The cxl_pci PCI error callbacks 
currently support only AER and could be updated to also support RCH/RCD (no VH) with 
something along the lines of below?

static bool cxl_pci_detected(struct pci_dev *pdev)                                                                                                      
{                                                                                                                                                     
 ...
     if (pci_pcie_type(pdev) == PCI_EXP_TYPE_RC_EC &&
            is_aer_internal_error(info))
		cxl_handle_rdport_errors();

In this case we would also need a cxl_pci_cor_detected(). 


>> +
>> +		cxl_handle_cor_ras(dev, cxlds->serial, cxl_get_ras_base(dev));
>> +	} else {
>> +		cxl_handle_cor_ras(dev, 0, cxl_get_ras_base(dev));
>> +	}
>> +}
>> +
>> +static pci_ers_result_t cxl_port_error_detected(struct device *dev)
>> +{
>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +	struct cxl_port *port __free(put_cxl_port) = get_cxl_port(pdev);
>> +
>> +	if (is_cxl_endpoint(port)) {
>> +		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
>> +		struct cxl_dev_state *cxlds = cxlmd->cxlds;
>> +
>> +		guard(device)(&cxlmd->dev);
>> +
>> +		if (!dev->driver) {
>> +			dev_warn(&pdev->dev,
>> +				 "%s: memdev disabled, abort error handling\n",
>> +				 dev_name(dev));
>> +			return PCI_ERS_RESULT_NONE;
>> +		}
>> +
>> +		if (cxlds->rcd)
>> +			cxl_handle_rdport_errors(cxlds);
>> +
>> +		return cxl_handle_ras(dev, cxlds->serial, cxl_get_ras_base(dev));
>> +	} else {
>> +		return cxl_handle_ras(dev, 0, cxl_get_ras_base(dev));
>> +	}
>> +}
>> +
>>  void cxl_cor_error_detected(struct pci_dev *pdev)
>>  {
>>  	struct cxl_dev_state *cxlds = pci_get_drvdata(pdev);
>> @@ -363,6 +479,24 @@ EXPORT_SYMBOL_NS_GPL(cxl_error_detected, "CXL");
>>  
>>  static void cxl_handle_proto_error(struct cxl_proto_err_work_data *err_info)
>>  {
>> +	struct pci_dev *pdev = err_info->pdev;
>> +
>> +	if (err_info->severity == AER_CORRECTABLE) {
>> +
>> +		if (!pcie_aer_is_native(pdev))
>> +			return;
>> +
>> +		if (pdev->aer_cap)
>> +			pci_clear_and_set_config_dword(pdev,
>> +						       pdev->aer_cap + PCI_ERR_COR_STATUS,
>> +						       0, PCI_ERR_COR_INTERNAL);
>> +
>> +		cxl_port_cor_error_detected(&pdev->dev);
>> +
>> +		pcie_clear_device_status(pdev);
>> +	} else {
>> +		cxl_do_recovery(pdev);
>> +	}
>>  }
>>  
>>  static void cxl_proto_err_work_fn(struct work_struct *work)
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 13dbb405dc31..b7bfefdaf990 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -2248,6 +2248,7 @@ void pcie_clear_device_status(struct pci_dev *dev)
>>  	pcie_capability_read_word(dev, PCI_EXP_DEVSTA, &sta);
>>  	pcie_capability_write_word(dev, PCI_EXP_DEVSTA, sta);
>>  }
>> +EXPORT_SYMBOL_GPL(pcie_clear_device_status);
> 
> No reason to open up this symbol to the world. Only cxl_core.ko needs
> this exported, and hopefully we never see another bus that abuses PCI
> like CXL does ever again.
> 
> [..]

Understood. I’ll switch this to a CXL‑scoped export.

>> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
>> index 7af10a74da34..4fc9de4c78f8 100644
>> --- a/drivers/pci/pcie/aer.c
>> +++ b/drivers/pci/pcie/aer.c
>> @@ -298,6 +298,7 @@ void pci_aer_clear_fatal_status(struct pci_dev *dev)
>>  	if (status)
>>  		pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, status);
>>  }
>> +EXPORT_SYMBOL_GPL(pci_aer_clear_fatal_status);
> 
> ditto, too wide of an export.
OK

-Terry

Bowman, Terry wrote:
[..]
> >> +static void __iomem *cxl_get_ras_base(struct device *dev)
> >> +{
> >> +	struct pci_dev *pdev = to_pci_dev(dev);
> >> +
> >> +	switch (pci_pcie_type(pdev)) {
> >> +	case PCI_EXP_TYPE_ROOT_PORT:
> >> +	case PCI_EXP_TYPE_DOWNSTREAM:
> >> +	{
> > 
> > Nit, clang-format puts that { on the same line because coding style says
> > only functions get newlines for open brackets.
> > 
> 
> Hi Dan,
> 
> Thanks for the note. Would you like every switch-case to be upodated
> to match the clang recommended format?

Yes, please. See "git grep case.*:\ {" for all the other examples.

[..]
> > Isn't this dead code? Only VH topologies will ever get a forwarded CXL
> > error, right? I realize it gets deleted in a future patch, but then why
> > leave dead code in the git history?
> > 
>                             
> Yes, agreed - I'll remove. Correct, only VH is forwarded. My
> understanding is the cxl_memdev guard and driver check are no longer
> required here. The memdev is only used to source the serial number, so
> I’ll refactor accordingly. Please correct me if Im wrong.

You do not need the memdev to get the serial number, and I note that the
serial number is only mandated for CXL memory class devices. I would
rather stop worrying about serial / pass 0 then add endpoint special
casing. The consumer of the tracepoint can always get the serial number
from sysfs, or this can call "pci_get_dsn(pdev)".

Overall, I expect that this generic error handling is device-type
indepdendent. The aim is it does not need to be touched again when/if
Linux ever sees CXL.cache devices without CXL.mem or the "serial is
mandated" edict for memory class devices.

> I see an additional fix needed: cxl_rch_handle_error_iter() in pci/pcie/aer.c
> also needs its callbacks updated. The RCH/RCD path previously invoked the EP
> PCIe handlers, but with RAS now handled at the port level, those callbacks no
> longer reach the correct logic.
> 
> I had a coupled ideas. One options is for the CXL logic to make a
> callback into a cxl_core exported function such as
> cxl_handle_rdport_errors(). BTW, the CXL logic in AER and the CXL
> driver's RAS are both built with the CONFIG_CXL_RAS config.

That destroys the modularity of cxl_core.ko.

> Another option is updating the CXL PCIe callbacks. The cxl_pci PCI
> error callbacks currently support only AER and could be updated to
> also support RCH/RCD (no VH) with something along the lines of below?

This continues the abuse of PCI error handlers for what is an odd CXL
aberration.

The answer that feels consistent with unburdening the PCI core with the
vagaries CXL is to include RCH errors in the class of notifications that
get forwarded. Arrange for cxl_proto_err_work_data to carry whether it
is an RCH or VH error and then dispatch either
cxl_handle_rdport_errors() or cxl_handle_proto_error().

On 2/4/2026 3:22 PM, dan.j.williams@intel.com wrote:
> Bowman, Terry wrote:
> [..]
>>>> +static void __iomem *cxl_get_ras_base(struct device *dev)
>>>> +{
>>>> +	struct pci_dev *pdev = to_pci_dev(dev);
>>>> +
>>>> +	switch (pci_pcie_type(pdev)) {
>>>> +	case PCI_EXP_TYPE_ROOT_PORT:
>>>> +	case PCI_EXP_TYPE_DOWNSTREAM:
>>>> +	{
>>>
>>> Nit, clang-format puts that { on the same line because coding style says
>>> only functions get newlines for open brackets.
>>>
>>
>> Hi Dan,
>>
>> Thanks for the note. Would you like every switch-case to be upodated
>> to match the clang recommended format?
> 
> Yes, please. See "git grep case.*:\ {" for all the other examples.
> 
> [..]
>>> Isn't this dead code? Only VH topologies will ever get a forwarded CXL
>>> error, right? I realize it gets deleted in a future patch, but then why
>>> leave dead code in the git history?
>>>
>>                             
>> Yes, agreed - I'll remove. Correct, only VH is forwarded. My
>> understanding is the cxl_memdev guard and driver check are no longer
>> required here. The memdev is only used to source the serial number, so
>> I’ll refactor accordingly. Please correct me if Im wrong.
> 
> You do not need the memdev to get the serial number, and I note that the
> serial number is only mandated for CXL memory class devices. I would
> rather stop worrying about serial / pass 0 then add endpoint special
> casing. The consumer of the tracepoint can always get the serial number
> from sysfs, or this can call "pci_get_dsn(pdev)".
> 
> Overall, I expect that this generic error handling is device-type
> indepdendent. The aim is it does not need to be touched again when/if
> Linux ever sees CXL.cache devices without CXL.mem or the "serial is
> mandated" edict for memory class devices.
> 
>> I see an additional fix needed: cxl_rch_handle_error_iter() in pci/pcie/aer.c
>> also needs its callbacks updated. The RCH/RCD path previously invoked the EP
>> PCIe handlers, but with RAS now handled at the port level, those callbacks no
>> longer reach the correct logic.
>>
>> I had a coupled ideas. One options is for the CXL logic to make a
>> callback into a cxl_core exported function such as
>> cxl_handle_rdport_errors(). BTW, the CXL logic in AER and the CXL
>> driver's RAS are both built with the CONFIG_CXL_RAS config.
> 
> That destroys the modularity of cxl_core.ko.
> 
>> Another option is updating the CXL PCIe callbacks. The cxl_pci PCI
>> error callbacks currently support only AER and could be updated to
>> also support RCH/RCD (no VH) with something along the lines of below?
> 
> This continues the abuse of PCI error handlers for what is an odd CXL
> aberration.
> 
> The answer that feels consistent with unburdening the PCI core with the
> vagaries CXL is to include RCH errors in the class of notifications that
> get forwarded. Arrange for cxl_proto_err_work_data to carry whether it
> is an RCH or VH error and then dispatch either
> cxl_handle_rdport_errors() or cxl_handle_proto_error().


That approach makes sense to me.

Would you like to keep the RCH's RCiEP traversal in the AER driver for now? In
that model, the RCiEP PCI device ID would be passed via cxl_proto_err_work_data. 
This would be a relatively small change — updating cxl_rch_handle_error_iter() 
and pcie/aer_cxl_rch.c to call cxl_forward_error().

A cleaner long-term approach would be to move all of the logic in aer_cxl_rch.c 
into cxl/core/rch_ras.c. In that case, an RCEC (reporting on behalf of the RCH 
error) would be passed in cxl_proto_err_work_data, and RCiEP iteration would be 
handled by the CXL driver after the work item surfaces from the kfifo.

The second approach improves PCI/CXL separation, but it may be harder to land 
late in the series. Would it be acceptable to proceed with the first approach 
initially, followed immediately by a cleanup series moving pcie/aer_cxl_rch.c 
into cxl/core/rch_ras.c?

- Terry

Bowman, Terry wrote:
[..]
> > The answer that feels consistent with unburdening the PCI core with the
> > vagaries CXL is to include RCH errors in the class of notifications that
> > get forwarded. Arrange for cxl_proto_err_work_data to carry whether it
> > is an RCH or VH error and then dispatch either
> > cxl_handle_rdport_errors() or cxl_handle_proto_error().
> 
> 
> That approach makes sense to me.
> 
> Would you like to keep the RCH's RCiEP traversal in the AER driver for now? In
> that model, the RCiEP PCI device ID would be passed via cxl_proto_err_work_data. 
> This would be a relatively small change — updating cxl_rch_handle_error_iter() 
> and pcie/aer_cxl_rch.c to call cxl_forward_error().
> 
> A cleaner long-term approach would be to move all of the logic in aer_cxl_rch.c 
> into cxl/core/rch_ras.c. In that case, an RCEC (reporting on behalf of the RCH 
> error) would be passed in cxl_proto_err_work_data, and RCiEP iteration would be 
> handled by the CXL driver after the work item surfaces from the kfifo.
> 
> The second approach improves PCI/CXL separation, but it may be harder to land 
> late in the series. Would it be acceptable to proceed with the first approach 
> initially, followed immediately by a cleanup series moving pcie/aer_cxl_rch.c 
> into cxl/core/rch_ras.c?

I think it is fine to do this incrementally. Keep RCiEP traversal in AER
for now. Later move more of that logic to the CXL core so PCI does not
need to worry about that complication. That later move makes it easier
to add consideration for details like the "RCEC Downstream Port
Association Structure" (RDPAS) without thrashing the PCI core.

On Mon, 2 Feb 2026 20:52:40 -0600
Terry Bowman <terry.bowman@amd.com> wrote:

> Introduce CXL Port protocol error handling callbacks to unify detection,
> logging, and recovery across CXL Ports and Endpoints, including RCH
> downstream ports. Establish a consistent flow for correctable and
> uncorrectable CXL protocol errors.
> 
> Provide the solution by adding cxl_port_cor_error_detected() and
> cxl_port_error_detected() to handle correctable and uncorrectable handling
> through CXL RAS helpers, coordinating uncorrectable recovery in
> cxl_do_recovery(), and panicking when the handler returns PCI_ERS_RESULT_PANIC
> to preserve fatal cachemem behavior. Gate endpoint handling on the endpoint
> driver being bound to avoid processing errors on disabled devices.
> 
> Centralize the RAS base lookup in cxl_get_ras_base(), selecting the
> downstream-port dport->regs.ras for Root/Downstream Ports and port->regs.ras
> for Upstream Ports/Endpoints.
> 
> Export pcie_clear_device_status() and pci_aer_clear_fatal_status() to enable
> cxl_core to clear PCIe/AER state in these flows.
> 
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> Reviewed-by: Dave Jiang dave.jiang@intel.com

Hi Terry,

A few comments inline.

Thanks,

Jonathan

> 
> ---
> 
> Changes in v14->v15:
> - Update commit message and title. Added Bjorn's ack.
> - Move CE and UCE handling logic here
> 
> Changes in v13->v14:
> - Add Dave Jiang's review-by
> - Update commit message & headline (Bjorn)
> - Refactor cxl_port_error_detected()/cxl_port_cor_error_detected() to
>   one line (Jonathan)
> - Remove cxl_walk_port() (Dan)
> - Remove cxl_pci_drv_bound(). Check for 'is_cxl' parent port is
>   sufficient (Dan)
> - Remove device_lock_if()
> - Combined CE and UCE here (Terry)
> 
> Changes in v12->v13:
> - Move get_pci_cxl_host_dev() and cxl_handle_proto_error() to Dequeue
>   patch (Terry)
> - Remove EP case in cxl_get_ras_base(), not used. (Terry)
> - Remove check for dport->dport_dev (Dave)
> - Remove whitespace (Terry)
> 
> Changes in v11->v12:
> - Add call to cxl_pci_drv_bound() in cxl_handle_proto_error() and
>   pci_to_cxl_dev()
> - Change cxl_error_detected() -> cxl_cor_error_detected()
> - Remove NULL variable assignments
> - Replace bus_find_device() with find_cxl_port_by_uport() for upstream
>   port searches.
> 
> Changes in v10->v11:
> - None
> ---
>  drivers/cxl/core/ras.c | 134 +++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/pci.c      |   1 +
>  drivers/pci/pci.h      |   2 -
>  drivers/pci/pcie/aer.c |   1 +
>  include/linux/aer.h    |   2 +
>  include/linux/pci.h    |   2 +
>  6 files changed, 140 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
> index a6c0bc6d7203..0216dafa6118 100644
> --- a/drivers/cxl/core/ras.c
> +++ b/drivers/cxl/core/ras.c
> @@ -218,6 +218,68 @@ static struct cxl_port *get_cxl_port(struct pci_dev *pdev)
>  	return NULL;
>  }
>  
> +static void __iomem *cxl_get_ras_base(struct device *dev)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +
> +	switch (pci_pcie_type(pdev)) {
> +	case PCI_EXP_TYPE_ROOT_PORT:
> +	case PCI_EXP_TYPE_DOWNSTREAM:
> +	{
> +		struct cxl_dport *dport;

		struct cxl_dport *dport = NULL;

> +		struct cxl_port *port __free(put_cxl_port) = find_cxl_port(&pdev->dev, &dport);

as if this failed, dport is not written. Alternative is check port, not dport as port
will always be initialized whether or not failure occurs in find_cxl_port()


> +
> +		if (!dport) {
> +			pci_err(pdev, "Failed to find the CXL device");
> +			return NULL;
> +		}
> +		return dport->regs.ras;
> +	}
> +	case PCI_EXP_TYPE_UPSTREAM:
> +	case PCI_EXP_TYPE_ENDPOINT:
> +	{
> +		struct cxl_port *port __free(put_cxl_port) = find_cxl_port_by_uport(&pdev->dev);
> +
> +		if (!port) {
> +			pci_err(pdev, "Failed to find the CXL device");
> +			return NULL;
> +		}
> +		return port->regs.ras;
> +	}
> +	}
> +	dev_warn_once(dev, "Error: Unsupported device type (%#x)", pci_pcie_type(pdev));
> +	return NULL;
> +}

>  void cxl_handle_cor_ras(struct device *dev, u64 serial, void __iomem *ras_base)
>  {
>  	void __iomem *addr;
> @@ -288,6 +350,60 @@ bool cxl_handle_ras(struct device *dev, u64 serial, void __iomem *ras_base)
>  	return true;
>  }
>  
> +static void cxl_port_cor_error_detected(struct device *dev)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct cxl_port *port __free(put_cxl_port) = get_cxl_port(pdev);
> +
> +	if (is_cxl_endpoint(port)) {
> +		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
> +		struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +
> +		guard(device)(&cxlmd->dev);

Maybe add a comment on why this lock needs to be held and then why the dev->drvier
below needs to be true.

> +
> +		if (!dev->driver) {
> +			dev_warn(&pdev->dev,
> +				 "%s: memdev disabled, abort error handling\n",
> +				 dev_name(dev));

Same question as below on why pdev->dev / dev_name(dev) here.
Maybe pci_warn() is more appropriate.

> +			return;
> +		}
> +
> +		if (cxlds->rcd)
> +			cxl_handle_rdport_errors(cxlds);
> +
> +		cxl_handle_cor_ras(dev, cxlds->serial, cxl_get_ras_base(dev));
> +	} else {
> +		cxl_handle_cor_ras(dev, 0, cxl_get_ras_base(dev));
> +	}
> +}
> +
> +static pci_ers_result_t cxl_port_error_detected(struct device *dev)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct cxl_port *port __free(put_cxl_port) = get_cxl_port(pdev);
> +
> +	if (is_cxl_endpoint(port)) {
> +		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
> +		struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +
> +		guard(device)(&cxlmd->dev);
> +
> +		if (!dev->driver) {
> +			dev_warn(&pdev->dev,

Somewhat circular.  dev_warn() will print the device name anyway I think and
this pdev->dev == dev here so might as well use that.

Or was intent to use different devices?

> +				 "%s: memdev disabled, abort error handling\n",
> +				 dev_name(dev));
> +			return PCI_ERS_RESULT_NONE;
> +		}
> +
> +		if (cxlds->rcd)
> +			cxl_handle_rdport_errors(cxlds);
> +
> +		return cxl_handle_ras(dev, cxlds->serial, cxl_get_ras_base(dev));
> +	} else {
> +		return cxl_handle_ras(dev, 0, cxl_get_ras_base(dev));
> +	}
> +}

>  
>  static void cxl_proto_err_work_fn(struct work_struct *work)

On 2/3/2026 9:40 AM, Jonathan Cameron wrote:
> On Mon, 2 Feb 2026 20:52:40 -0600
> Terry Bowman <terry.bowman@amd.com> wrote:
> 
>> Introduce CXL Port protocol error handling callbacks to unify detection,
>> logging, and recovery across CXL Ports and Endpoints, including RCH
>> downstream ports. Establish a consistent flow for correctable and
>> uncorrectable CXL protocol errors.
>>
>> Provide the solution by adding cxl_port_cor_error_detected() and
>> cxl_port_error_detected() to handle correctable and uncorrectable handling
>> through CXL RAS helpers, coordinating uncorrectable recovery in
>> cxl_do_recovery(), and panicking when the handler returns PCI_ERS_RESULT_PANIC
>> to preserve fatal cachemem behavior. Gate endpoint handling on the endpoint
>> driver being bound to avoid processing errors on disabled devices.
>>
>> Centralize the RAS base lookup in cxl_get_ras_base(), selecting the
>> downstream-port dport->regs.ras for Root/Downstream Ports and port->regs.ras
>> for Upstream Ports/Endpoints.
>>
>> Export pcie_clear_device_status() and pci_aer_clear_fatal_status() to enable
>> cxl_core to clear PCIe/AER state in these flows.
>>
>> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>> Reviewed-by: Dave Jiang dave.jiang@intel.com
> 
> Hi Terry,
> 
> A few comments inline.
> 
> Thanks,
> 
> Jonathan
> 

Thanks for reviewing.

>>
>> ---
>>
>> Changes in v14->v15:
>> - Update commit message and title. Added Bjorn's ack.
>> - Move CE and UCE handling logic here
>>
>> Changes in v13->v14:
>> - Add Dave Jiang's review-by
>> - Update commit message & headline (Bjorn)
>> - Refactor cxl_port_error_detected()/cxl_port_cor_error_detected() to
>>   one line (Jonathan)
>> - Remove cxl_walk_port() (Dan)
>> - Remove cxl_pci_drv_bound(). Check for 'is_cxl' parent port is
>>   sufficient (Dan)
>> - Remove device_lock_if()
>> - Combined CE and UCE here (Terry)
>>
>> Changes in v12->v13:
>> - Move get_pci_cxl_host_dev() and cxl_handle_proto_error() to Dequeue
>>   patch (Terry)
>> - Remove EP case in cxl_get_ras_base(), not used. (Terry)
>> - Remove check for dport->dport_dev (Dave)
>> - Remove whitespace (Terry)
>>
>> Changes in v11->v12:
>> - Add call to cxl_pci_drv_bound() in cxl_handle_proto_error() and
>>   pci_to_cxl_dev()
>> - Change cxl_error_detected() -> cxl_cor_error_detected()
>> - Remove NULL variable assignments
>> - Replace bus_find_device() with find_cxl_port_by_uport() for upstream
>>   port searches.
>>
>> Changes in v10->v11:
>> - None
>> ---
>>  drivers/cxl/core/ras.c | 134 +++++++++++++++++++++++++++++++++++++++++
>>  drivers/pci/pci.c      |   1 +
>>  drivers/pci/pci.h      |   2 -
>>  drivers/pci/pcie/aer.c |   1 +
>>  include/linux/aer.h    |   2 +
>>  include/linux/pci.h    |   2 +
>>  6 files changed, 140 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
>> index a6c0bc6d7203..0216dafa6118 100644
>> --- a/drivers/cxl/core/ras.c
>> +++ b/drivers/cxl/core/ras.c
>> @@ -218,6 +218,68 @@ static struct cxl_port *get_cxl_port(struct pci_dev *pdev)
>>  	return NULL;
>>  }
>>  
>> +static void __iomem *cxl_get_ras_base(struct device *dev)
>> +{
>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +
>> +	switch (pci_pcie_type(pdev)) {
>> +	case PCI_EXP_TYPE_ROOT_PORT:
>> +	case PCI_EXP_TYPE_DOWNSTREAM:
>> +	{
>> +		struct cxl_dport *dport;
> 
> 		struct cxl_dport *dport = NULL;
> 
>> +		struct cxl_port *port __free(put_cxl_port) = find_cxl_port(&pdev->dev, &dport);
> 
> as if this failed, dport is not written. Alternative is check port, not dport as port
> will always be initialized whether or not failure occurs in find_cxl_port()
> 

Ok.

> 
>> +
>> +		if (!dport) {
>> +			pci_err(pdev, "Failed to find the CXL device");
>> +			return NULL;
>> +		}
>> +		return dport->regs.ras;
>> +	}
>> +	case PCI_EXP_TYPE_UPSTREAM:
>> +	case PCI_EXP_TYPE_ENDPOINT:
>> +	{
>> +		struct cxl_port *port __free(put_cxl_port) = find_cxl_port_by_uport(&pdev->dev);
>> +
>> +		if (!port) {
>> +			pci_err(pdev, "Failed to find the CXL device");
>> +			return NULL;
>> +		}
>> +		return port->regs.ras;
>> +	}
>> +	}
>> +	dev_warn_once(dev, "Error: Unsupported device type (%#x)", pci_pcie_type(pdev));
>> +	return NULL;
>> +}
> 
>>  void cxl_handle_cor_ras(struct device *dev, u64 serial, void __iomem *ras_base)
>>  {
>>  	void __iomem *addr;
>> @@ -288,6 +350,60 @@ bool cxl_handle_ras(struct device *dev, u64 serial, void __iomem *ras_base)
>>  	return true;
>>  }
>>  
>> +static void cxl_port_cor_error_detected(struct device *dev)
>> +{
>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +	struct cxl_port *port __free(put_cxl_port) = get_cxl_port(pdev);
>> +
>> +	if (is_cxl_endpoint(port)) {
>> +		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
>> +		struct cxl_dev_state *cxlds = cxlmd->cxlds;
>> +
>> +		guard(device)(&cxlmd->dev);
> 
> Maybe add a comment on why this lock needs to be held and then why the dev->drvier
> below needs to be true.
> 
This can be removed. This was added to ensure the EP's RAS registers remained accessible 
during handling. This was when when the mapped RAS registers were owned by the CXL memory 
device. This has changed such that the EP RAS registers are now owned by the EP Port. And, 
the Endpoint Port is already locked in cxl_proto_err_work_fn() before calling this funcion.

>> +
>> +		if (!dev->driver) {
>> +			dev_warn(&pdev->dev,
>> +				 "%s: memdev disabled, abort error handling\n",
>> +				 dev_name(dev));
> 
> Same question as below on why pdev->dev / dev_name(dev) here.
> Maybe pci_warn() is more appropriate.
> 

I believe the driver check can be removed but would like your input. The check 
for the driver is another piece of code specifically for when the handler was accessing 
the memdev's RAS registers. It was a last check to make certain the device is bound 
to a driver before accessing. EP RAS is now owned by the Endpoint Port.

Ok, I'll make these a pci_warn().

>> +			return;
>> +		}
>> +
>> +		if (cxlds->rcd)
>> +			cxl_handle_rdport_errors(cxlds);
>> +
>> +		cxl_handle_cor_ras(dev, cxlds->serial, cxl_get_ras_base(dev));
>> +	} else {
>> +		cxl_handle_cor_ras(dev, 0, cxl_get_ras_base(dev));
>> +	}
>> +}
>> +
>> +static pci_ers_result_t cxl_port_error_detected(struct device *dev)
>> +{
>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +	struct cxl_port *port __free(put_cxl_port) = get_cxl_port(pdev);
>> +
>> +	if (is_cxl_endpoint(port)) {
>> +		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
>> +		struct cxl_dev_state *cxlds = cxlmd->cxlds;
>> +
>> +		guard(device)(&cxlmd->dev);
>> +
>> +		if (!dev->driver) {
>> +			dev_warn(&pdev->dev,
> 
> Somewhat circular.  dev_warn() will print the device name anyway I think and
> this pdev->dev == dev here so might as well use that.
> 
> Or was intent to use different devices?
> 

No. Same device. I made a last minute change to "make logging more useful with 
device name" and the change wasn't necessary here. I'll use pci_warn() as you 
mentioned above.

-Terry

>> +				 "%s: memdev disabled, abort error handling\n",
>> +				 dev_name(dev));
>> +			return PCI_ERS_RESULT_NONE;
>> +		}
>> +
>> +		if (cxlds->rcd)
>> +			cxl_handle_rdport_errors(cxlds);
>> +
>> +		return cxl_handle_ras(dev, cxlds->serial, cxl_get_ras_base(dev));
>> +	} else {
>> +		return cxl_handle_ras(dev, 0, cxl_get_ras_base(dev));
>> +	}
>> +}
> 
>>  
>>  static void cxl_proto_err_work_fn(struct work_struct *work)

On Tue, 3 Feb 2026 12:21:56 -0600
"Bowman, Terry" <terry.bowman@amd.com> wrote:

> On 2/3/2026 9:40 AM, Jonathan Cameron wrote:
> > On Mon, 2 Feb 2026 20:52:40 -0600
> > Terry Bowman <terry.bowman@amd.com> wrote:
> >   
> >> Introduce CXL Port protocol error handling callbacks to unify detection,
> >> logging, and recovery across CXL Ports and Endpoints, including RCH
> >> downstream ports. Establish a consistent flow for correctable and
> >> uncorrectable CXL protocol errors.
> >>
> >> Provide the solution by adding cxl_port_cor_error_detected() and
> >> cxl_port_error_detected() to handle correctable and uncorrectable handling
> >> through CXL RAS helpers, coordinating uncorrectable recovery in
> >> cxl_do_recovery(), and panicking when the handler returns PCI_ERS_RESULT_PANIC
> >> to preserve fatal cachemem behavior. Gate endpoint handling on the endpoint
> >> driver being bound to avoid processing errors on disabled devices.
> >>
> >> Centralize the RAS base lookup in cxl_get_ras_base(), selecting the
> >> downstream-port dport->regs.ras for Root/Downstream Ports and port->regs.ras
> >> for Upstream Ports/Endpoints.
> >>
> >> Export pcie_clear_device_status() and pci_aer_clear_fatal_status() to enable
> >> cxl_core to clear PCIe/AER state in these flows.
> >>
> >> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> >> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> >> Reviewed-by: Dave Jiang dave.jiang@intel.com  
> > 
> > Hi Terry,
> > 
> > A few comments inline.
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> 
> Thanks for reviewing.
> 
> >>
> >> ---
> >>
> >> Changes in v14->v15:
> >> - Update commit message and title. Added Bjorn's ack.
> >> - Move CE and UCE handling logic here
> >>
> >> Changes in v13->v14:
> >> - Add Dave Jiang's review-by
> >> - Update commit message & headline (Bjorn)
> >> - Refactor cxl_port_error_detected()/cxl_port_cor_error_detected() to
> >>   one line (Jonathan)
> >> - Remove cxl_walk_port() (Dan)
> >> - Remove cxl_pci_drv_bound(). Check for 'is_cxl' parent port is
> >>   sufficient (Dan)
> >> - Remove device_lock_if()
> >> - Combined CE and UCE here (Terry)
> >>
> >> Changes in v12->v13:
> >> - Move get_pci_cxl_host_dev() and cxl_handle_proto_error() to Dequeue
> >>   patch (Terry)
> >> - Remove EP case in cxl_get_ras_base(), not used. (Terry)
> >> - Remove check for dport->dport_dev (Dave)
> >> - Remove whitespace (Terry)
> >>
> >> Changes in v11->v12:
> >> - Add call to cxl_pci_drv_bound() in cxl_handle_proto_error() and
> >>   pci_to_cxl_dev()
> >> - Change cxl_error_detected() -> cxl_cor_error_detected()
> >> - Remove NULL variable assignments
> >> - Replace bus_find_device() with find_cxl_port_by_uport() for upstream
> >>   port searches.
> >>
> >> Changes in v10->v11:
> >> - None
> >> ---
> >>  drivers/cxl/core/ras.c | 134 +++++++++++++++++++++++++++++++++++++++++
> >>  drivers/pci/pci.c      |   1 +
> >>  drivers/pci/pci.h      |   2 -
> >>  drivers/pci/pcie/aer.c |   1 +
> >>  include/linux/aer.h    |   2 +
> >>  include/linux/pci.h    |   2 +
> >>  6 files changed, 140 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
> >> index a6c0bc6d7203..0216dafa6118 100644
> >> --- a/drivers/cxl/core/ras.c
> >> +++ b/drivers/cxl/core/ras.c
> >> @@ -218,6 +218,68 @@ static struct cxl_port *get_cxl_port(struct pci_dev *pdev)
> >>  	return NULL;
> >>  }
> >>  
> >> +static void __iomem *cxl_get_ras_base(struct device *dev)
> >> +{
> >> +	struct pci_dev *pdev = to_pci_dev(dev);
> >> +
> >> +	switch (pci_pcie_type(pdev)) {
> >> +	case PCI_EXP_TYPE_ROOT_PORT:
> >> +	case PCI_EXP_TYPE_DOWNSTREAM:
> >> +	{
> >> +		struct cxl_dport *dport;  
> > 
> > 		struct cxl_dport *dport = NULL;
> >   
> >> +		struct cxl_port *port __free(put_cxl_port) = find_cxl_port(&pdev->dev, &dport);  
> > 
> > as if this failed, dport is not written. Alternative is check port, not dport as port
> > will always be initialized whether or not failure occurs in find_cxl_port()
> >   
> 
> Ok.
> 
> >   
> >> +
> >> +		if (!dport) {
> >> +			pci_err(pdev, "Failed to find the CXL device");
> >> +			return NULL;
> >> +		}
> >> +		return dport->regs.ras;
> >> +	}
> >> +	case PCI_EXP_TYPE_UPSTREAM:
> >> +	case PCI_EXP_TYPE_ENDPOINT:
> >> +	{
> >> +		struct cxl_port *port __free(put_cxl_port) = find_cxl_port_by_uport(&pdev->dev);
> >> +
> >> +		if (!port) {
> >> +			pci_err(pdev, "Failed to find the CXL device");
> >> +			return NULL;
> >> +		}
> >> +		return port->regs.ras;
> >> +	}
> >> +	}
> >> +	dev_warn_once(dev, "Error: Unsupported device type (%#x)", pci_pcie_type(pdev));
> >> +	return NULL;
> >> +}  
> >   
> >>  void cxl_handle_cor_ras(struct device *dev, u64 serial, void __iomem *ras_base)
> >>  {
> >>  	void __iomem *addr;
> >> @@ -288,6 +350,60 @@ bool cxl_handle_ras(struct device *dev, u64 serial, void __iomem *ras_base)
> >>  	return true;
> >>  }
> >>  
> >> +static void cxl_port_cor_error_detected(struct device *dev)
> >> +{
> >> +	struct pci_dev *pdev = to_pci_dev(dev);
> >> +	struct cxl_port *port __free(put_cxl_port) = get_cxl_port(pdev);
> >> +
> >> +	if (is_cxl_endpoint(port)) {
> >> +		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
> >> +		struct cxl_dev_state *cxlds = cxlmd->cxlds;
> >> +
> >> +		guard(device)(&cxlmd->dev);  
> > 
> > Maybe add a comment on why this lock needs to be held and then why the dev->drvier
> > below needs to be true.
> >   
> This can be removed. This was added to ensure the EP's RAS registers remained accessible 
> during handling. This was when when the mapped RAS registers were owned by the CXL memory 
> device. This has changed such that the EP RAS registers are now owned by the EP Port. And, 
> the Endpoint Port is already locked in cxl_proto_err_work_fn() before calling this funcion.
> 
> >> +
> >> +		if (!dev->driver) {
> >> +			dev_warn(&pdev->dev,
> >> +				 "%s: memdev disabled, abort error handling\n",
> >> +				 dev_name(dev));  
> > 
> > Same question as below on why pdev->dev / dev_name(dev) here.
> > Maybe pci_warn() is more appropriate.
> >   
> 
> I believe the driver check can be removed but would like your input. The check 
> for the driver is another piece of code specifically for when the handler was accessing 
> the memdev's RAS registers. It was a last check to make certain the device is bound 
> to a driver before accessing. EP RAS is now owned by the Endpoint Port.

Sounds fine to me to drop this check if we don't need anything that
belongs to that driver any more.

Jonathan