On Wed, 26 Mar 2025 20:47:04 -0500 Terry Bowman <terry.bowman@xxxxxxx> wrote: > CXL error handling will soon be moved from the AER driver into the CXL > driver. This requires a notification mechanism for the AER driver to share > the AER interrupt details with CXL driver. The notification is required for > the CXL drivers to then handle CXL RAS errors. > > Add a kfifo work queue to be used by the AER driver and CXL driver. The AER > driver will be the sole kfifo producer adding work. The cxl_core will be > the sole kfifo consumer removing work. Add the boilerplate kfifo support. > > Add CXL work queue handler registration functions in the AER driver. Export > the functions allowing CXL driver to access. Implement the registration > functions for the CXL driver to assign or clear the work handler function. > > Create a work queue handler function, cxl_prot_err_work_fn(), as a stub for > now. The CXL specific handling will be added in future patch. > > Introduce 'struct cxl_prot_err_info'. This structure caches CXL error > details used in completing error handling. This avoid duplicating some > function calls and allows the error to be treated generically when > possible. > > Signed-off-by: Terry Bowman <terry.bowman@xxxxxxx> It's been a while since I messed around with work queues, but don't we need to initialize the work_struct somewhere? DECLARE_WORK() probably similar to cxl_cper_prot_err_work. Having both the pointer and the instance of the work struct called cxl_prot_err_work is confusing me... > --- > drivers/cxl/core/ras.c | 54 +++++++++++++++++++++++++++++++++++++++++- > drivers/cxl/cxlpci.h | 3 +++ > drivers/pci/pcie/aer.c | 39 ++++++++++++++++++++++++++++++ > include/linux/aer.h | 37 +++++++++++++++++++++++++++++ > 4 files changed, 132 insertions(+), 1 deletion(-) > > diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c > index 485a831695c7..ecb60a5962de 100644 > --- a/drivers/cxl/core/ras.c > +++ b/drivers/cxl/core/ras.c > @@ -5,6 +5,7 @@ > #include <linux/aer.h> > #include <cxl/event.h> > #include <cxlmem.h> > +#include <cxlpci.h> > #include "trace.h" > > static void cxl_cper_trace_corr_port_prot_err(struct pci_dev *pdev, > @@ -107,13 +108,64 @@ 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_create_prot_err_info(struct pci_dev *_pdev, int severity, > + struct cxl_prot_error_info *err_info) > +{ > + struct pci_dev *pdev __free(pci_dev_put) = pci_dev_get(_pdev); > + struct cxl_dev_state *cxlds; > + > + if (!pdev || !err_info) { > + pr_warn_once("Error: parameter is NULL"); > + return -ENODEV; > + } > + > + if ((pci_pcie_type(pdev) != PCI_EXP_TYPE_ENDPOINT) && > + (pci_pcie_type(pdev) != PCI_EXP_TYPE_RC_END)) { > + pci_warn_once(pdev, "Error: Unsupported device type (%X)", pci_pcie_type(pdev)); > + return -ENODEV; > + } > + > + cxlds = pci_get_drvdata(pdev); > + struct device *dev __free(put_device) = get_device(&cxlds->cxlmd->dev); > + > + if (!dev) > + return -ENODEV; > + > + *err_info = (struct cxl_prot_error_info){ 0 }; > + err_info->ras_base = cxlds->regs.ras; > + err_info->severity = severity; > + err_info->pdev = pdev; > + err_info->dev = dev; > + > + return 0; > +} > + > +struct work_struct cxl_prot_err_work; This is the one I'm not seeing initialized anywhere... > + > int cxl_ras_init(void) > { > - return cxl_cper_register_prot_err_work(&cxl_cper_prot_err_work); > + int rc; > + > + rc = cxl_cper_register_prot_err_work(&cxl_cper_prot_err_work); > + if (rc) { > + pr_err("Failed to register CPER kfifo with AER driver"); > + return rc; > + } > + > + rc = cxl_register_prot_err_work(&cxl_prot_err_work, cxl_create_prot_err_info); > + if (rc) { > + pr_err("Failed to register kfifo with AER driver"); > + return rc; > + } > + > + return rc; > } > > 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_prot_err_work(); > + cancel_work_sync(&cxl_prot_err_work); > } > diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h > index 54e219b0049e..92d72c0423ab 100644 > --- a/drivers/cxl/cxlpci.h > +++ b/drivers/cxl/cxlpci.h > @@ -4,6 +4,7 @@ > #define __CXL_PCI_H__ > #include <linux/pci.h> > #include "cxl.h" > +#include "linux/aer.h" > > #define CXL_MEMORY_PROGIF 0x10 > > @@ -135,4 +136,6 @@ void read_cdat_data(struct cxl_port *port); > void cxl_cor_error_detected(struct pci_dev *pdev); > pci_ers_result_t cxl_error_detected(struct pci_dev *pdev, > pci_channel_state_t state); > +int cxl_create_prot_err_info(struct pci_dev *_pdev, int severity, > + struct cxl_prot_error_info *err_info); > #endif /* __CXL_PCI_H__ */ > diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c > index 83f2069f111e..46123b70f496 100644 > --- a/drivers/pci/pcie/aer.c > +++ b/drivers/pci/pcie/aer.c > @@ -110,6 +110,16 @@ struct aer_stats { > static int pcie_aer_disable; > static pci_ers_result_t aer_root_reset(struct pci_dev *dev); > > +#if defined(CONFIG_PCIEAER_CXL) > +#define CXL_ERROR_SOURCES_MAX 128 > +static DEFINE_KFIFO(cxl_prot_err_fifo, struct cxl_prot_err_work_data, > + CXL_ERROR_SOURCES_MAX); > +static DEFINE_SPINLOCK(cxl_prot_err_fifo_lock); > +struct work_struct *cxl_prot_err_work; > +static int (*cxl_create_prot_err_info)(struct pci_dev*, int severity, > + struct cxl_prot_error_info*); > +#endif > + > void pci_no_aer(void) > { > pcie_aer_disable = 1; > @@ -1577,6 +1587,35 @@ static pci_ers_result_t aer_root_reset(struct pci_dev *dev) > return rc ? PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_RECOVERED; > } > > + > +#if defined(CONFIG_PCIEAER_CXL) > +int cxl_register_prot_err_work(struct work_struct *work, > + int (*_cxl_create_prot_err_info)(struct pci_dev*, int, > + struct cxl_prot_error_info*)) > +{ > + guard(spinlock)(&cxl_prot_err_fifo_lock); > + cxl_prot_err_work = work; > + cxl_create_prot_err_info = _cxl_create_prot_err_info; > + return 0; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_register_prot_err_work, "CXL"); > + > +int cxl_unregister_prot_err_work(void) > +{ > + guard(spinlock)(&cxl_prot_err_fifo_lock); > + cxl_prot_err_work = NULL; > + cxl_create_prot_err_info = NULL; > + return 0; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_unregister_prot_err_work, "CXL"); > + > +int cxl_prot_err_kfifo_get(struct cxl_prot_err_work_data *wd) > +{ > + return kfifo_get(&cxl_prot_err_fifo, wd); > +} > +EXPORT_SYMBOL_NS_GPL(cxl_prot_err_kfifo_get, "CXL"); > +#endif > + > static struct pcie_port_service_driver aerdriver = { > .name = "aer", > .port_type = PCIE_ANY_PORT, > diff --git a/include/linux/aer.h b/include/linux/aer.h > index 947b63091902..761d6f5cd792 100644 > --- a/include/linux/aer.h > +++ b/include/linux/aer.h > @@ -10,6 +10,7 @@ > > #include <linux/errno.h> > #include <linux/types.h> > +#include <linux/workqueue_types.h> > > #define AER_NONFATAL 0 > #define AER_FATAL 1 > @@ -45,6 +46,24 @@ struct aer_capability_regs { > u16 uncor_err_source; > }; > > +/** > + * struct cxl_prot_err_info - Error information used in CXL error handling > + * @pdev: PCI device with CXL error > + * @dev: CXL device with error. From CXL topology using ACPI/platform discovery > + * @ras_base: Mapped address of CXL RAS registers > + * @severity: CXL AER/RAS severity: AER_CORRECTABLE, AER_FATAL, AER_NONFATAL > + */ > +struct cxl_prot_error_info { > + struct pci_dev *pdev; > + struct device *dev; > + void __iomem *ras_base; > + int severity; > +}; > + > +struct cxl_prot_err_work_data { > + struct cxl_prot_error_info err_info; > +}; > + > #if defined(CONFIG_PCIEAER) > int pci_aer_clear_nonfatal_status(struct pci_dev *dev); > int pcie_aer_is_native(struct pci_dev *dev); > @@ -56,6 +75,24 @@ static inline int pci_aer_clear_nonfatal_status(struct pci_dev *dev) > static inline int pcie_aer_is_native(struct pci_dev *dev) { return 0; } > #endif > > +#if defined(CONFIG_PCIEAER_CXL) > +int cxl_register_prot_err_work(struct work_struct *work, > + int (*_cxl_create_proto_err_info)(struct pci_dev*, int, > + struct cxl_prot_error_info*)); > +int cxl_unregister_prot_err_work(void); > +int cxl_prot_err_kfifo_get(struct cxl_prot_err_work_data *wd); > +#else > +static inline int > +cxl_register_prot_err_work(struct work_struct *work, > + int (*_cxl_create_proto_err_info)(struct pci_dev*, int, > + struct cxl_prot_error_info*)) > +{ > + return 0; > +} > +static inline int cxl_unregister_prot_err_work(void) { return 0; } > +static inline int cxl_prot_err_kfifo_get(struct cxl_prot_err_work_data *wd) { return 0; } > +#endif > + > void pci_print_aer(struct pci_dev *dev, int aer_severity, > struct aer_capability_regs *aer); > int cper_severity_to_aer(int cper_severity);