| Message ID | 20240102150933.161009-2-Smita.KoralahalliChannabasappa@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | acpi/ghes, cper, cxl: Trace FW-First CXL Protocol Errors | expand |
On 1/2/2024 8:23 AM, Ira Weiny wrote: > Smita Koralahalli wrote: >> Currently defined cxl_cper_callback interface between CXL subsystem and >> GHES module is just confined to handling CXL Component errors only. >> >> Extend this callback to process CXL Protocol errors as well. Achieve >> by defining a new struct cxl_cper_rec_data to include cxl_cper_event_rec >> and other fields of CXL protocol errors which will be defined in future >> patches. >> >> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com> > > [snip] > >> >> static int __init cxl_pci_driver_init(void) >> diff --git a/include/linux/cxl-event.h b/include/linux/cxl-event.h >> index 17eadee819b6..afa71ee0437c 100644 >> --- a/include/linux/cxl-event.h >> +++ b/include/linux/cxl-event.h >> @@ -141,8 +141,12 @@ struct cxl_cper_event_rec { >> union cxl_event event; >> } __packed; >> >> +struct cxl_cper_rec_data { >> + struct cxl_cper_event_rec rec; > > NIT: I would call this something like event to distinguish it from other > record data. What do you think of the below? struct cxl_cper_event_info { struct cxl_cper_event_rec rec; struct cxl_cper_prot_err { struct cxl_ras_capability_regs cxl_ras; int severity; } p_err; }; Addressed changing to sub-struct and copying the struct rather than pointer comments in patch 3.. > > Other than that this seems reasonable to me. > > Reviewed-by: Ira Weiny <ira.weiny@intel.com> > > [snip] >
Smita Koralahalli wrote: > On 1/2/2024 8:23 AM, Ira Weiny wrote: > > Smita Koralahalli wrote: [snip] > >> diff --git a/include/linux/cxl-event.h b/include/linux/cxl-event.h > >> index 17eadee819b6..afa71ee0437c 100644 > >> --- a/include/linux/cxl-event.h > >> +++ b/include/linux/cxl-event.h > >> @@ -141,8 +141,12 @@ struct cxl_cper_event_rec { > >> union cxl_event event; > >> } __packed; > >> > >> +struct cxl_cper_rec_data { > >> + struct cxl_cper_event_rec rec; > > > > NIT: I would call this something like event to distinguish it from other > > record data. > > What do you think of the below? > > struct cxl_cper_event_info { > struct cxl_cper_event_rec rec; > struct cxl_cper_prot_err { > struct cxl_ras_capability_regs cxl_ras; > int severity; > } p_err; > }; > > Addressed changing to sub-struct and copying the struct rather than > pointer comments in patch 3.. That is much better. Thanks! Ira
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index aed465d2fd68..d6a85fbc0a8b 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -693,6 +693,10 @@ static cxl_cper_callback cper_callback; static void cxl_cper_post_event(enum cxl_event_type event_type, struct cxl_cper_event_rec *rec) { + struct cxl_cper_rec_data data; + + data.rec = *(struct cxl_cper_event_rec *)rec; + if (rec->hdr.length <= sizeof(rec->hdr) || rec->hdr.length > sizeof(*rec)) { pr_err(FW_WARN "CXL CPER Invalid section length (%u)\n", @@ -707,7 +711,7 @@ static void cxl_cper_post_event(enum cxl_event_type event_type, guard(rwsem_read)(&cxl_cper_rw_sem); if (cper_callback) - cper_callback(event_type, rec); + cper_callback(event_type, &data); } int cxl_cper_register_callback(cxl_cper_callback callback) diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index b14237f824cf..7edbd53357e5 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -972,9 +972,9 @@ static struct pci_driver cxl_pci_driver = { #define CXL_EVENT_HDR_FLAGS_REC_SEVERITY GENMASK(1, 0) static void cxl_cper_event_call(enum cxl_event_type ev_type, - struct cxl_cper_event_rec *rec) + struct cxl_cper_rec_data *data) { - struct cper_cxl_event_devid *device_id = &rec->hdr.device_id; + struct cper_cxl_event_devid *device_id = &data->rec.hdr.device_id; struct pci_dev *pdev __free(pci_dev_put) = NULL; enum cxl_event_log_type log_type; struct cxl_dev_state *cxlds; @@ -996,11 +996,11 @@ static void cxl_cper_event_call(enum cxl_event_type ev_type, return; /* Fabricate a log type */ - hdr_flags = get_unaligned_le24(rec->event.generic.hdr.flags); + hdr_flags = get_unaligned_le24(data->rec.event.generic.hdr.flags); log_type = FIELD_GET(CXL_EVENT_HDR_FLAGS_REC_SEVERITY, hdr_flags); cxl_event_trace_record(cxlds->cxlmd, log_type, ev_type, - &uuid_null, &rec->event); + &uuid_null, &data->rec.event); } static int __init cxl_pci_driver_init(void) diff --git a/include/linux/cxl-event.h b/include/linux/cxl-event.h index 17eadee819b6..afa71ee0437c 100644 --- a/include/linux/cxl-event.h +++ b/include/linux/cxl-event.h @@ -141,8 +141,12 @@ struct cxl_cper_event_rec { union cxl_event event; } __packed; +struct cxl_cper_rec_data { + struct cxl_cper_event_rec rec; +}; + typedef void (*cxl_cper_callback)(enum cxl_event_type type, - struct cxl_cper_event_rec *rec); + struct cxl_cper_rec_data *data); #ifdef CONFIG_ACPI_APEI_GHES int cxl_cper_register_callback(cxl_cper_callback callback);
Currently defined cxl_cper_callback interface between CXL subsystem and GHES module is just confined to handling CXL Component errors only. Extend this callback to process CXL Protocol errors as well. Achieve by defining a new struct cxl_cper_rec_data to include cxl_cper_event_rec and other fields of CXL protocol errors which will be defined in future patches. Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com> --- drivers/acpi/apei/ghes.c | 6 +++++- drivers/cxl/pci.c | 8 ++++---- include/linux/cxl-event.h | 6 +++++- 3 files changed, 14 insertions(+), 6 deletions(-)