On 8/26/25 6:35 PM, Terry Bowman wrote:
> The AER driver is now designed to forward CXL protocol errors to the CXL
I would rephrase it to:
The AER driver enqueues the CXL protocol error info to the created kfifo for the CXL driver to consume.
> driver. Update the CXL driver with functionality to dequeue the forwarded
> CXL error from the kfifo. Also, update the CXL driver to begin the protocol
> error handling processing using the work received from the FIFO.
>
> Update function cxl_proto_err_work_fn() to dequeue work forwarded by the
> AER service driver. This will begin the CXL protocol error processing with
> a call to cxl_handle_proto_error().
>
> Introduce logic to take the SBDF values from 'struct cxl_proto_error_info'
> and use in discovering the erring PCI device. The call to pci_get_domain_bus_and_slot()
> will return a reference counted 'struct pci_dev *'. This will serve as
> reference count to prevent releasing the CXL Endpoint's mapped RAS while
> handling the error. Use scope base __free() to put the reference count.
> This will change when adding support for CXL port devices in the future.
>
> Implement cxl_handle_proto_error() to differentiate between Restricted CXL
> Host (RCH) protocol errors and CXL virtual host (VH) protocol errors.
> Maintain the existing RCH handling. Export the AER driver's pcie_walk_rcec()
> allowing the CXL driver to walk the RCEC's secondary bus.
>
> VH correctable error (CE) processing will call the CXL CE handler. VH
> uncorrectable errors (UCE) will call cxl_do_recovery(), implemented as a
> stub for now and to be updated in future patch. Export pci_aer_clean_fatal_status()
> and pci_clean_device_status() used to clean up AER status after handling.
>
> Signed-off-by: Terry Bowman <terry.bowman@xxxxxxx>
> Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
>
> ---
> Changes in v10->v11:
> - Reword patch commit message to remove RCiEP details (Jonathan)
> - Add #include <linux/bitfield.h> (Terry)
> - is_cxl_rcd() - Fix short comment message wrap (Jonathan)
> - is_cxl_rcd() - Combine return calls into 1 (Jonathan)
> - cxl_handle_proto_error() - Move comment earlier (Jonathan)
> - Usse FIELD_GET() in discovering class code (Jonathan)
> - Remove BDF from cxl_proto_err_work_data. Use 'struct pci_dev *' (Dan)
> ---
> drivers/cxl/core/ras.c | 68 ++++++++++++++++++++++++++++++++++-------
> drivers/pci/pci.c | 1 +
> drivers/pci/pci.h | 7 -----
> drivers/pci/pcie/aer.c | 1 +
> drivers/pci/pcie/rcec.c | 1 +
> include/linux/aer.h | 2 ++
> include/linux/pci.h | 10 ++++++
> 7 files changed, 72 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
> index b285448c2d9c..a2e95c49f965 100644
> --- a/drivers/cxl/core/ras.c
> +++ b/drivers/cxl/core/ras.c
> @@ -118,17 +118,6 @@ static void cxl_cper_prot_err_work_fn(struct work_struct *work)
> }
> static DECLARE_WORK(cxl_cper_prot_err_work, cxl_cper_prot_err_work_fn);
>
> -int cxl_ras_init(void)
> -{
> - return cxl_cper_register_prot_err_work(&cxl_cper_prot_err_work);
> -}
> -
> -void cxl_ras_exit(void)
> -{
> - cxl_cper_unregister_prot_err_work(&cxl_cper_prot_err_work);
> - cancel_work_sync(&cxl_cper_prot_err_work);
> -}
> -
> static pci_ers_result_t cxl_handle_ras(struct device *dev, u64 serial, void __iomem *ras_base);
> static void cxl_handle_cor_ras(struct device *dev, u64 serial, void __iomem *ras_base);
>
> @@ -331,6 +320,10 @@ void cxl_endpoint_port_init_ras(struct cxl_port *ep)
> }
> EXPORT_SYMBOL_NS_GPL(cxl_endpoint_port_init_ras, "CXL");
>
> +static void cxl_do_recovery(struct device *dev)
> +{
> +}
> +
> static void cxl_handle_cor_ras(struct device *dev, u64 serial, void __iomem *ras_base)
> {
> void __iomem *addr;
> @@ -472,3 +465,56 @@ pci_ers_result_t pci_error_detected(struct pci_dev *pdev,
> return rc;
> }
> EXPORT_SYMBOL_NS_GPL(pci_error_detected, "CXL");
> +
> +static void cxl_handle_proto_error(struct cxl_proto_err_work_data *err_info)
> +{
> + struct pci_dev *pdev = err_info->pdev;
> + struct cxl_dev_state *cxlds = pci_get_drvdata(pdev);
So this function is called from the workqueue thread to consume data from the kfifo right? Do we need to take the device lock of the pdev to ensure that a driver is bound to the device before we attempt to retrieve the data? And do we also need to verify that the driver bound is the cxl_pci driver (and not something like vfio_pci)? Otherwise I think assuming the drv data is cxl_dev_state may cause crash.
DJ
> + struct cxl_memdev *cxlmd = cxlds->cxlmd;
> + struct device *host_dev __free(put_device) = get_device(&cxlmd->dev);
> +> + if (err_info->severity == AER_CORRECTABLE) {
> + int aer = pdev->aer_cap;
> +
> + if (aer)
> + pci_clear_and_set_config_dword(pdev,
> + aer + PCI_ERR_COR_STATUS,
> + 0, PCI_ERR_COR_INTERNAL);
> +
> + cxl_cor_error_detected(&cxlmd->dev);
> +
> + pcie_clear_device_status(pdev);
> + } else {
> + cxl_do_recovery(&cxlmd->dev);
> + }
> +}
> +
> +static void cxl_proto_err_work_fn(struct work_struct *work)
> +{
> + struct cxl_proto_err_work_data wd;
> +
> + while (cxl_proto_err_kfifo_get(&wd))
> + cxl_handle_proto_error(&wd);
> +}
> +
> +static struct work_struct cxl_proto_err_work;
> +static DECLARE_WORK(cxl_proto_err_work, cxl_proto_err_work_fn);
> +
> +int cxl_ras_init(void)
> +{
> + if (cxl_cper_register_prot_err_work(&cxl_cper_prot_err_work))
> + pr_err("Failed to initialize CXL RAS CPER\n");
> +
> + cxl_register_proto_err_work(&cxl_proto_err_work);
> +
> + return 0;
> +}
> +
> +void cxl_ras_exit(void)
> +{
> + cxl_cper_unregister_prot_err_work(&cxl_cper_prot_err_work);
> + cancel_work_sync(&cxl_cper_prot_err_work);
> +
> + cxl_unregister_proto_err_work();
> + cancel_work_sync(&cxl_proto_err_work);
> +}
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index d775ed37a79b..2c9827690cb3 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2328,6 +2328,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_NS_GPL(pcie_clear_device_status, "CXL");
> #endif
>
> /**
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index cfa75903dd3f..69ff7c2d214f 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -671,16 +671,10 @@ static inline bool pci_dpc_recovered(struct pci_dev *pdev) { return false; }
> void pci_rcec_init(struct pci_dev *dev);
> void pci_rcec_exit(struct pci_dev *dev);
> void pcie_link_rcec(struct pci_dev *rcec);
> -void pcie_walk_rcec(struct pci_dev *rcec,
> - int (*cb)(struct pci_dev *, void *),
> - void *userdata);
> #else
> static inline void pci_rcec_init(struct pci_dev *dev) { }
> static inline void pci_rcec_exit(struct pci_dev *dev) { }
> static inline void pcie_link_rcec(struct pci_dev *rcec) { }
> -static inline void pcie_walk_rcec(struct pci_dev *rcec,
> - int (*cb)(struct pci_dev *, void *),
> - void *userdata) { }
> #endif
>
> #ifdef CONFIG_PCI_ATS
> @@ -1022,7 +1016,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 627d89ccea9c..45abe1622316 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -288,6 +288,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/drivers/pci/pcie/rcec.c b/drivers/pci/pcie/rcec.c
> index d0bcd141ac9c..fb6cf6449a1d 100644
> --- a/drivers/pci/pcie/rcec.c
> +++ b/drivers/pci/pcie/rcec.c
> @@ -145,6 +145,7 @@ void pcie_walk_rcec(struct pci_dev *rcec, int (*cb)(struct pci_dev *, void *),
>
> walk_rcec(walk_rcec_helper, &rcec_data);
> }
> +EXPORT_SYMBOL_NS_GPL(pcie_walk_rcec, "CXL");
>
> void pci_rcec_init(struct pci_dev *dev)
> {
> diff --git a/include/linux/aer.h b/include/linux/aer.h
> index f8eb32805957..1f79f0be4bf7 100644
> --- a/include/linux/aer.h
> +++ b/include/linux/aer.h
> @@ -66,12 +66,14 @@ 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);
> #else
> 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; }
> #endif
>
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 3dcab36c437f..3407d687459d 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1804,6 +1804,9 @@ extern bool pcie_ports_native;
>
> int pcie_set_target_speed(struct pci_dev *port, enum pci_bus_speed speed_req,
> bool use_lt);
> +void pcie_walk_rcec(struct pci_dev *rcec,
> + int (*cb)(struct pci_dev *, void *),
> + void *userdata);
> #else
> #define pcie_ports_disabled true
> #define pcie_ports_native false
> @@ -1814,8 +1817,15 @@ static inline int pcie_set_target_speed(struct pci_dev *port,
> {
> return -EOPNOTSUPP;
> }
> +
> +static inline void pcie_walk_rcec(struct pci_dev *rcec,
> + int (*cb)(struct pci_dev *, void *),
> + void *userdata) { }
> +
> #endif
>
> +void pcie_clear_device_status(struct pci_dev *dev);
> +
> #define PCIE_LINK_STATE_L0S (BIT(0) | BIT(1)) /* Upstr/dwnstr L0s */
> #define PCIE_LINK_STATE_L1 BIT(2) /* L1 state */
> #define PCIE_LINK_STATE_L1_1 BIT(3) /* ASPM L1.1 state */
