| Message ID | 20240102150933.161009-5-Smita.KoralahalliChannabasappa@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | acpi/ghes, cper, cxl: Trace FW-First CXL Protocol Errors | expand |
Smita Koralahalli wrote: > When PCIe AER is in FW-First, OS should process CXL Protocol errors from > CPER records. These CPER records obtained from GHES module, will rely on > a registered callback to be notified to the CXL subsystem in order to be > processed. > > Call the existing cxl_cper_callback 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> [snip] > int cxl_cper_register_callback(cxl_cper_callback callback) > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c > index 37e1652afbc7..da516982a625 100644 > --- a/drivers/cxl/core/pci.c > +++ b/drivers/cxl/core/pci.c > @@ -6,6 +6,7 @@ > #include <linux/pci.h> > #include <linux/pci-doe.h> > #include <linux/aer.h> > +#include <linux/cper.h> > #include <cxlpci.h> > #include <cxlmem.h> > #include <cxl.h> > @@ -836,6 +837,51 @@ void cxl_setup_parent_dport(struct device *host, struct cxl_dport *dport) > } > EXPORT_SYMBOL_NS_GPL(cxl_setup_parent_dport, CXL); > > +#define CXL_AER_UNCORRECTABLE 0 > +#define CXL_AER_CORRECTABLE 1 Better defined as an enum? > + > +int cper_severity_cxl_aer(int cper_severity) My gut says that it would be better to hide this conversion in the GHES/CPER code and send a more generic defined CXL_AER_* severity through. > +{ > + switch (cper_severity) { > + case CPER_SEV_RECOVERABLE: > + case CPER_SEV_FATAL: > + return CXL_AER_UNCORRECTABLE; > + default: > + return CXL_AER_CORRECTABLE; > + } > +} > + > +void cxl_prot_err_trace_record(struct cxl_dev_state *cxlds, > + struct cxl_cper_rec_data *data) > +{ > + struct cper_cxl_event_sn *dev_serial_num = &data->rec.hdr.dev_serial_num; > + u32 status, fe; > + int severity; > + > + severity = cper_severity_cxl_aer(data->severity); > + > + cxlds->serial = (((u64)dev_serial_num->upper_dw << 32) | > + dev_serial_num->lower_dw); This permanently overwrites the serial number read from PCI... If the serial number does not match up or was not valid (per the check in the previous patch) lets add a warning. AFAICT they should match. Ira [snip]
On 1/2/2024 12:27 PM, Ira Weiny wrote: > Smita Koralahalli wrote: >> When PCIe AER is in FW-First, OS should process CXL Protocol errors from >> CPER records. These CPER records obtained from GHES module, will rely on >> a registered callback to be notified to the CXL subsystem in order to be >> processed. >> >> Call the existing cxl_cper_callback 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> > > [snip] > >> int cxl_cper_register_callback(cxl_cper_callback callback) >> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c >> index 37e1652afbc7..da516982a625 100644 >> --- a/drivers/cxl/core/pci.c >> +++ b/drivers/cxl/core/pci.c >> @@ -6,6 +6,7 @@ >> #include <linux/pci.h> >> #include <linux/pci-doe.h> >> #include <linux/aer.h> >> +#include <linux/cper.h> >> #include <cxlpci.h> >> #include <cxlmem.h> >> #include <cxl.h> >> @@ -836,6 +837,51 @@ void cxl_setup_parent_dport(struct device *host, struct cxl_dport *dport) >> } >> EXPORT_SYMBOL_NS_GPL(cxl_setup_parent_dport, CXL); >> >> +#define CXL_AER_UNCORRECTABLE 0 >> +#define CXL_AER_CORRECTABLE 1 > > Better defined as an enum? Will change. > >> + >> +int cper_severity_cxl_aer(int cper_severity) > > My gut says that it would be better to hide this conversion in the > GHES/CPER code and send a more generic defined CXL_AER_* severity through. Ok will change. > >> +{ >> + switch (cper_severity) { >> + case CPER_SEV_RECOVERABLE: >> + case CPER_SEV_FATAL: >> + return CXL_AER_UNCORRECTABLE; >> + default: >> + return CXL_AER_CORRECTABLE; >> + } >> +} >> + >> +void cxl_prot_err_trace_record(struct cxl_dev_state *cxlds, >> + struct cxl_cper_rec_data *data) >> +{ >> + struct cper_cxl_event_sn *dev_serial_num = &data->rec.hdr.dev_serial_num; >> + u32 status, fe; >> + int severity; >> + >> + severity = cper_severity_cxl_aer(data->severity); >> + >> + cxlds->serial = (((u64)dev_serial_num->upper_dw << 32) | >> + dev_serial_num->lower_dw); > > This permanently overwrites the serial number read from PCI... > > If the serial number does not match up or was not valid (per the check in > the previous patch) lets add a warning. Sure will add. Thanks, Smita > > AFAICT they should match. > > Ira > > [snip] >
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 6471584b2e79..217494c7c884 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -716,6 +716,7 @@ static void cxl_cper_post_event(enum cxl_event_type event_type, void cxl_cper_handle_prot_err(struct acpi_hest_generic_data *gdata) { + enum cxl_event_type event_type = CXL_CPER_EVENT_PROT_ERR; struct cxl_cper_rec_data data; memset(&data, 0, sizeof(data)); @@ -724,6 +725,10 @@ void cxl_cper_handle_prot_err(struct acpi_hest_generic_data *gdata) return; data.severity = gdata->error_severity; + + guard(rwsem_read)(&cxl_cper_rw_sem); + if (cper_callback) + cper_callback(event_type, &data); } int cxl_cper_register_callback(cxl_cper_callback callback) diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index 37e1652afbc7..da516982a625 100644 --- a/drivers/cxl/core/pci.c +++ b/drivers/cxl/core/pci.c @@ -6,6 +6,7 @@ #include <linux/pci.h> #include <linux/pci-doe.h> #include <linux/aer.h> +#include <linux/cper.h> #include <cxlpci.h> #include <cxlmem.h> #include <cxl.h> @@ -836,6 +837,51 @@ void cxl_setup_parent_dport(struct device *host, struct cxl_dport *dport) } EXPORT_SYMBOL_NS_GPL(cxl_setup_parent_dport, CXL); +#define CXL_AER_UNCORRECTABLE 0 +#define CXL_AER_CORRECTABLE 1 + +int cper_severity_cxl_aer(int cper_severity) +{ + switch (cper_severity) { + case CPER_SEV_RECOVERABLE: + case CPER_SEV_FATAL: + return CXL_AER_UNCORRECTABLE; + default: + return CXL_AER_CORRECTABLE; + } +} + +void cxl_prot_err_trace_record(struct cxl_dev_state *cxlds, + struct cxl_cper_rec_data *data) +{ + struct cper_cxl_event_sn *dev_serial_num = &data->rec.hdr.dev_serial_num; + u32 status, fe; + int severity; + + severity = cper_severity_cxl_aer(data->severity); + + cxlds->serial = (((u64)dev_serial_num->upper_dw << 32) | + dev_serial_num->lower_dw); + + if (severity == CXL_AER_CORRECTABLE) { + status = data->cxl_ras->cor_status & ~data->cxl_ras->cor_mask; + + trace_cxl_aer_correctable_error(cxlds->cxlmd, status); + } else { + status = data->cxl_ras->uncor_status & ~data->cxl_ras->uncor_mask; + + if (hweight32(status) > 1) + fe = BIT(FIELD_GET(CXL_RAS_CAP_CONTROL_FE_MASK, + data->cxl_ras->cap_control)); + else + fe = status; + + trace_cxl_aer_uncorrectable_error(cxlds->cxlmd, status, fe, + data->cxl_ras->header_log); + } +} +EXPORT_SYMBOL_NS_GPL(cxl_prot_err_trace_record, CXL); + static void cxl_handle_rdport_cor_ras(struct cxl_dev_state *cxlds, struct cxl_dport *dport) { diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h index 0fa4799ea316..462f1f9e82b0 100644 --- a/drivers/cxl/cxlpci.h +++ b/drivers/cxl/cxlpci.h @@ -93,4 +93,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_rec_data; +void cxl_prot_err_trace_record(struct cxl_dev_state *cxlds, + struct cxl_cper_rec_data *data); #endif /* __CXL_PCI_H__ */ diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index 7edbd53357e5..2b9f4dbf06c9 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -995,6 +995,11 @@ static void cxl_cper_event_call(enum cxl_event_type ev_type, if (!cxlds) return; + if (ev_type == CXL_CPER_EVENT_PROT_ERR) { + cxl_prot_err_trace_record(cxlds, data); + return; + } + /* Fabricate a log type */ hdr_flags = get_unaligned_le24(data->rec.event.generic.hdr.flags); log_type = FIELD_GET(CXL_EVENT_HDR_FLAGS_REC_SEVERITY, hdr_flags); diff --git a/include/linux/cxl-event.h b/include/linux/cxl-event.h index 7ba2dfd6619e..b4558f206f59 100644 --- a/include/linux/cxl-event.h +++ b/include/linux/cxl-event.h @@ -113,6 +113,7 @@ enum cxl_event_type { CXL_CPER_EVENT_GEN_MEDIA, CXL_CPER_EVENT_DRAM, CXL_CPER_EVENT_MEM_MODULE, + CXL_CPER_EVENT_PROT_ERR, }; #define CPER_CXL_DEVICE_ID_VALID BIT(0)
When PCIe AER is in FW-First, OS should process CXL Protocol errors from CPER records. These CPER records obtained from GHES module, will rely on a registered callback to be notified to the CXL subsystem in order to be processed. Call the existing cxl_cper_callback 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> --- drivers/acpi/apei/ghes.c | 5 +++++ drivers/cxl/core/pci.c | 46 +++++++++++++++++++++++++++++++++++++++ drivers/cxl/cxlpci.h | 3 +++ drivers/cxl/pci.c | 5 +++++ include/linux/cxl-event.h | 1 + 5 files changed, 60 insertions(+)