| Message ID | 20241001005234.61409-5-Smita.KoralahalliChannabasappa@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | acpi/ghes, cper, cxl: Trace FW-First CXL Protocol Errors | expand |
On 10/2/2024 5:16 PM, Dan Williams wrote: > Smita Koralahalli wrote: >> When PCIe AER is in FW-First, OS should process CXL Protocol errors from >> CPER records. >> >> Reuse the existing work queue cxl_cper_work registered with GHES to notify >> the CXL subsystem on a Protocol error. >> >> The defined trace events cxl_aer_uncorrectable_error and >> cxl_aer_correctable_error currently trace native CXL AER errors. Reuse >> them to trace FW-First Protocol Errors. >> >> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com> >> --- >> v2: >> Removed pr_warn for serial number. >> p_err -> rec/p_rec. >> --- >> drivers/acpi/apei/ghes.c | 14 ++++++++++++++ >> drivers/cxl/core/pci.c | 24 ++++++++++++++++++++++++ >> drivers/cxl/cxlpci.h | 3 +++ >> drivers/cxl/pci.c | 20 ++++++++++++++++++-- >> include/cxl/event.h | 1 + >> 5 files changed, 60 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c >> index 9dcf0f78458f..5082885e1f2c 100644 >> --- a/drivers/acpi/apei/ghes.c >> +++ b/drivers/acpi/apei/ghes.c >> @@ -723,6 +723,20 @@ static void cxl_cper_handle_prot_err(struct acpi_hest_generic_data *gdata) >> >> if (cxl_cper_handle_prot_err_info(gdata, &wd.p_rec)) >> return; >> + >> + guard(spinlock_irqsave)(&cxl_cper_work_lock); >> + >> + if (!cxl_cper_work) >> + return; >> + >> + wd.event_type = CXL_CPER_EVENT_PROT_ERR; >> + >> + if (!kfifo_put(&cxl_cper_fifo, wd)) { >> + pr_err_ratelimited("CXL CPER kfifo overflow\n"); >> + return; >> + } >> + >> + schedule_work(cxl_cper_work); > > The cxl_cper_work item is only for cases where the cxl_pci driver might care > about annotating an error report with driver specific details like the > impacted kernel object name, 'struct cxl_memdev', or address translation > for DPA data. > > Protocol errors that are not endpoint errors should never be placed in > the cxl_cper_fifo. That is exclusively for errors that cxl_pci needs to > consume. > > My expectation is that similar to aer_recover_queue for PCIe protocol > errors CXL needs to grow a cxl_recover_queue that at a minimum triggers > new trace events to dump these records to RAS daemon. > > I am struggling to think what useful information cxl_pci could ever > append to a protocol error event. > > What is more likely is that later when Terry adds port error handling a > CPER protocol error record could trigger a new cxl_do_recovery() to > react to CXL topology errors that might impact downstream CXL devices. > In that case the notification will come through something like a new > 'struct cxl_error_handlers *' hanging off 'struct pci_driver' since > accelerator drivers are going to have distinct error handling from > generic memory expanders. Hmm, I agree handling device and protocol errors separately. Unless Terry has something to add here, I can start working on a minimal version of cxl_recover_queue as of now. Thanks Smita >
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 9dcf0f78458f..5082885e1f2c 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -723,6 +723,20 @@ static void cxl_cper_handle_prot_err(struct acpi_hest_generic_data *gdata) if (cxl_cper_handle_prot_err_info(gdata, &wd.p_rec)) return; + + guard(spinlock_irqsave)(&cxl_cper_work_lock); + + if (!cxl_cper_work) + return; + + wd.event_type = CXL_CPER_EVENT_PROT_ERR; + + if (!kfifo_put(&cxl_cper_fifo, wd)) { + pr_err_ratelimited("CXL CPER kfifo overflow\n"); + return; + } + + schedule_work(cxl_cper_work); } int cxl_cper_register_work(struct work_struct *work) diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index 5b46bc46aaa9..39ef24c8991f 100644 --- a/drivers/cxl/core/pci.c +++ b/drivers/cxl/core/pci.c @@ -650,6 +650,30 @@ void read_cdat_data(struct cxl_port *port) } EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL); +void cxl_trace_prot_err(struct cxl_dev_state *cxlds, + struct cxl_cper_prot_err *rec) +{ + u32 status, fe; + + if (rec->severity == CXL_AER_CORRECTABLE) { + status = rec->cxl_ras.cor_status & ~rec->cxl_ras.cor_mask; + + trace_cxl_aer_correctable_error(cxlds->cxlmd, status); + } else { + status = rec->cxl_ras.uncor_status & ~rec->cxl_ras.uncor_mask; + + if (hweight32(status) > 1) + fe = BIT(FIELD_GET(CXL_RAS_CAP_CONTROL_FE_MASK, + rec->cxl_ras.cap_control)); + else + fe = status; + + trace_cxl_aer_uncorrectable_error(cxlds->cxlmd, status, fe, + rec->cxl_ras.header_log); + } +} +EXPORT_SYMBOL_NS_GPL(cxl_trace_prot_err, CXL); + static void __cxl_handle_cor_ras(struct cxl_dev_state *cxlds, void __iomem *ras_base) { diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h index 4da07727ab9c..8acd8f2c39c9 100644 --- a/drivers/cxl/cxlpci.h +++ b/drivers/cxl/cxlpci.h @@ -129,4 +129,7 @@ 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); +struct cxl_cper_prot_err; +void cxl_trace_prot_err(struct cxl_dev_state *cxlds, + struct cxl_cper_prot_err *rec); #endif /* __CXL_PCI_H__ */ diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index 915102f5113f..0a29321921a0 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -1064,12 +1064,28 @@ static void cxl_handle_cper_event(enum cxl_event_type ev_type, &uuid_null, &rec->event); } +static void cxl_handle_prot_err(struct cxl_cper_prot_err *rec) +{ + struct cxl_dev_state *cxlds; + + cxlds = get_cxl_devstate(rec->segment, rec->bus, + rec->device, rec->function); + if (!cxlds) + return; + + cxl_trace_prot_err(cxlds, rec); +} + static void cxl_cper_work_fn(struct work_struct *work) { struct cxl_cper_work_data wd; - while (cxl_cper_kfifo_get(&wd)) - cxl_handle_cper_event(wd.event_type, &wd.rec); + while (cxl_cper_kfifo_get(&wd)) { + if (wd.event_type == CXL_CPER_EVENT_PROT_ERR) + cxl_handle_prot_err(&wd.p_rec); + else + cxl_handle_cper_event(wd.event_type, &wd.rec); + } } static DECLARE_WORK(cxl_cper_work, cxl_cper_work_fn); diff --git a/include/cxl/event.h b/include/cxl/event.h index 5b316150556a..d854d8c435db 100644 --- a/include/cxl/event.h +++ b/include/cxl/event.h @@ -115,6 +115,7 @@ struct cxl_event_record_raw { } __packed; enum cxl_event_type { + CXL_CPER_EVENT_PROT_ERR, CXL_CPER_EVENT_GENERIC, CXL_CPER_EVENT_GEN_MEDIA, CXL_CPER_EVENT_DRAM,
When PCIe AER is in FW-First, OS should process CXL Protocol errors from CPER records. Reuse the existing work queue cxl_cper_work registered with GHES to notify the CXL subsystem on a Protocol error. The defined trace events cxl_aer_uncorrectable_error and cxl_aer_correctable_error currently trace native CXL AER errors. Reuse them to trace FW-First Protocol Errors. Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com> --- v2: Removed pr_warn for serial number. p_err -> rec/p_rec. --- drivers/acpi/apei/ghes.c | 14 ++++++++++++++ drivers/cxl/core/pci.c | 24 ++++++++++++++++++++++++ drivers/cxl/cxlpci.h | 3 +++ drivers/cxl/pci.c | 20 ++++++++++++++++++-- include/cxl/event.h | 1 + 5 files changed, 60 insertions(+), 2 deletions(-)