| Message ID | 20231215-cxl-cper-v4-6-01b6dab44fcd@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | efi/cxl-cper: Report CPER CXL component events through trace events | expand |
On 12/15/2023 3:26 PM, Ira Weiny wrote: > BIOS can configure memory devices as firmware first. This will send CXL > events to the firmware instead of the OS. The firmware can then send > these events to the OS via UEFI. > > UEFI v2.10 section N.2.14 defines a Common Platform Error Record (CPER) > format for CXL Component Events. The format is mostly the same as the > CXL Common Event Record Format. The difference is the use of a GUID in > the Section Type rather than a UUID as part of the event itself. > > Add EFI support to detect CXL CPER records and call a registered > notifier with the event. Enforce that only one notifier call can be > registered at any time. > > Cc: Ard Biesheuvel <ardb@kernel.org> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > [snip] > + struct { > + u32 length; > + u64 validation_bits; > + struct cper_cxl_event_devid { > + u16 vendor_id; > + u16 device_id; > + u8 func_num; > + u8 device_num; > + u8 bus_num; > + u16 segment_num; > + u16 slot_num; /* bits 2:0 reserved */ > + u8 reserved; > + } device_id __packed; > + struct cper_cxl_event_sn { > + u32 lower_dw; > + u32 upper_dw; > + } dev_serial_num __packed; > + } hdr __packed; > + > + union cxl_event event; > +} __packed; > + For some reason, prefixing the struct name with __packed attribute seems to do the job. ("__packed device_id" and "__packed dev_serial_num"). Thanks, Smita
Smita Koralahalli wrote: > On 12/15/2023 3:26 PM, Ira Weiny wrote: > > BIOS can configure memory devices as firmware first. This will send CXL > > events to the firmware instead of the OS. The firmware can then send > > these events to the OS via UEFI. > > > > UEFI v2.10 section N.2.14 defines a Common Platform Error Record (CPER) > > format for CXL Component Events. The format is mostly the same as the > > CXL Common Event Record Format. The difference is the use of a GUID in > > the Section Type rather than a UUID as part of the event itself. > > > > Add EFI support to detect CXL CPER records and call a registered > > notifier with the event. Enforce that only one notifier call can be > > registered at any time. > > > > Cc: Ard Biesheuvel <ardb@kernel.org> > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > > > > [snip] > > > + struct { > > + u32 length; > > + u64 validation_bits; > > + struct cper_cxl_event_devid { > > + u16 vendor_id; > > + u16 device_id; > > + u8 func_num; > > + u8 device_num; > > + u8 bus_num; > > + u16 segment_num; > > + u16 slot_num; /* bits 2:0 reserved */ > > + u8 reserved; > > + } device_id __packed; > > + struct cper_cxl_event_sn { > > + u32 lower_dw; > > + u32 upper_dw; > > + } dev_serial_num __packed; > > + } hdr __packed; > > + > > + union cxl_event event; > > +} __packed; > > + > > For some reason, prefixing the struct name with __packed attribute seems > to do the job. ("__packed device_id" and "__packed dev_serial_num"). Good catch, yeah, the expectation is that follows the closing brace not only to match the predominant style in the kernel, but gcc appears to not honor it otherwise. Looks better with this on top: diff --git a/include/linux/cxl-event.h b/include/linux/cxl-event.h index 2b137aead750..975925029f6d 100644 --- a/include/linux/cxl-event.h +++ b/include/linux/cxl-event.h @@ -130,12 +130,12 @@ struct cxl_cper_event_rec { u16 segment_num; u16 slot_num; /* bits 2:0 reserved */ u8 reserved; - } device_id __packed; + } __packed device_id; struct cper_cxl_event_sn { u32 lower_dw; u32 upper_dw; - } dev_serial_num __packed; - } hdr __packed; + } __packed dev_serial_num; + } __packed hdr; union cxl_event event; } __packed;
On Fri, 15 Dec 2023 15:26:32 -0800 Ira Weiny <ira.weiny@intel.com> wrote: > BIOS can configure memory devices as firmware first. This will send CXL > events to the firmware instead of the OS. The firmware can then send > these events to the OS via UEFI. > > UEFI v2.10 section N.2.14 defines a Common Platform Error Record (CPER) > format for CXL Component Events. The format is mostly the same as the > CXL Common Event Record Format. The difference is the use of a GUID in > the Section Type rather than a UUID as part of the event itself. > > Add EFI support to detect CXL CPER records and call a registered > notifier with the event. Enforce that only one notifier call can be > registered at any time. > > Cc: Ard Biesheuvel <ardb@kernel.org> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> One trivial thing inline. + Agree that notifier naming is unwise given what that means elsewhere in the kernel. > diff --git a/drivers/firmware/efi/cper_cxl.h b/drivers/firmware/efi/cper_cxl.h > index 86bfcf7909ec..71f27b3e2810 100644 > --- a/drivers/firmware/efi/cper_cxl.h > +++ b/drivers/firmware/efi/cper_cxl.h > @@ -10,11 +10,38 @@ > #ifndef LINUX_CPER_CXL_H > #define LINUX_CPER_CXL_H > > +#include <linux/cxl-event.h> > + > /* CXL Protocol Error Section */ > #define CPER_SEC_CXL_PROT_ERR \ > GUID_INIT(0x80B9EFB4, 0x52B5, 0x4DE3, 0xA7, 0x77, 0x68, 0x78, \ > 0x4B, 0x77, 0x10, 0x48) > > +/* CXL Event record UUIDs are formated at GUIDs and reported in section type */ as GUIDs > +/* > + * General Media Event Record > + * CXL rev 3.0 Section 8.2.9.2.1.1; Table 8-43 > + */ > +#define CPER_SEC_CXL_GEN_MEDIA_GUID \ >
Dan Williams wrote: > Smita Koralahalli wrote: > > On 12/15/2023 3:26 PM, Ira Weiny wrote: [snip] > > > + struct { > > > + u32 length; > > > + u64 validation_bits; > > > + struct cper_cxl_event_devid { > > > + u16 vendor_id; > > > + u16 device_id; > > > + u8 func_num; > > > + u8 device_num; > > > + u8 bus_num; > > > + u16 segment_num; > > > + u16 slot_num; /* bits 2:0 reserved */ > > > + u8 reserved; > > > + } device_id __packed; > > > + struct cper_cxl_event_sn { > > > + u32 lower_dw; > > > + u32 upper_dw; > > > + } dev_serial_num __packed; > > > + } hdr __packed; > > > + > > > + union cxl_event event; > > > +} __packed; > > > + > > > > For some reason, prefixing the struct name with __packed attribute seems > > to do the job. ("__packed device_id" and "__packed dev_serial_num"). > > Good catch, yeah, the expectation is that follows the closing brace not > only to match the predominant style in the kernel, but gcc appears to > not honor it otherwise. Looks better with this on top: Very good catch. I did not mean to do this at all... :-( > > diff --git a/include/linux/cxl-event.h b/include/linux/cxl-event.h > index 2b137aead750..975925029f6d 100644 > --- a/include/linux/cxl-event.h > +++ b/include/linux/cxl-event.h > @@ -130,12 +130,12 @@ struct cxl_cper_event_rec { > u16 segment_num; > u16 slot_num; /* bits 2:0 reserved */ > u8 reserved; > - } device_id __packed; > + } __packed device_id; > struct cper_cxl_event_sn { > u32 lower_dw; > u32 upper_dw; > - } dev_serial_num __packed; > - } hdr __packed; > + } __packed dev_serial_num; > + } __packed hdr; > > union cxl_event event; > } __packed; > Yes thanks, Ira
Jonathan Cameron wrote: > On Fri, 15 Dec 2023 15:26:32 -0800 > Ira Weiny <ira.weiny@intel.com> wrote: [snip] > > > > Cc: Ard Biesheuvel <ardb@kernel.org> > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > One trivial thing inline. > > + Agree that notifier naming is unwise given what that means elsewhere in the > kernel. Changing that. > > > diff --git a/drivers/firmware/efi/cper_cxl.h b/drivers/firmware/efi/cper_cxl.h > > index 86bfcf7909ec..71f27b3e2810 100644 > > --- a/drivers/firmware/efi/cper_cxl.h > > +++ b/drivers/firmware/efi/cper_cxl.h > > @@ -10,11 +10,38 @@ > > #ifndef LINUX_CPER_CXL_H > > #define LINUX_CPER_CXL_H > > > > +#include <linux/cxl-event.h> > > + > > /* CXL Protocol Error Section */ > > #define CPER_SEC_CXL_PROT_ERR \ > > GUID_INIT(0x80B9EFB4, 0x52B5, 0x4DE3, 0xA7, 0x77, 0x68, 0x78, \ > > 0x4B, 0x77, 0x10, 0x48) > > > > +/* CXL Event record UUIDs are formated at GUIDs and reported in section type */ > > as GUIDs > Fixed, thanks, Ira
diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c index 35c37f667781..2ee5011c4a8e 100644 --- a/drivers/firmware/efi/cper.c +++ b/drivers/firmware/efi/cper.c @@ -22,6 +22,7 @@ #include <linux/aer.h> #include <linux/printk.h> #include <linux/bcd.h> +#include <linux/cxl-event.h> #include <acpi/ghes.h> #include <ras/ras_event.h> #include "cper_cxl.h" @@ -607,6 +608,20 @@ cper_estatus_print_section(const char *pfx, struct acpi_hest_generic_data *gdata cper_print_prot_err(newpfx, prot_err); else goto err_section_too_small; + } else if (guid_equal(sec_type, &CPER_SEC_CXL_GEN_MEDIA_GUID) || + guid_equal(sec_type, &CPER_SEC_CXL_DRAM_GUID) || + guid_equal(sec_type, &CPER_SEC_CXL_MEM_MODULE_GUID)) { + struct cxl_cper_event_rec *rec = acpi_hest_get_payload(gdata); + + if (rec->hdr.length <= sizeof(rec->hdr)) + goto err_section_too_small; + + if (rec->hdr.length > sizeof(*rec)) { + pr_err(FW_WARN "error section length is too big\n"); + return; + } + + cxl_cper_post_event(newpfx, sec_type, rec); } else { const void *err = acpi_hest_get_payload(gdata); diff --git a/drivers/firmware/efi/cper_cxl.c b/drivers/firmware/efi/cper_cxl.c index a55771b99a97..29388a0825ef 100644 --- a/drivers/firmware/efi/cper_cxl.c +++ b/drivers/firmware/efi/cper_cxl.c @@ -8,6 +8,7 @@ */ #include <linux/cper.h> +#include <linux/cxl-event.h> #include "cper_cxl.h" #define PROT_ERR_VALID_AGENT_TYPE BIT_ULL(0) @@ -187,3 +188,48 @@ void cper_print_prot_err(const char *pfx, const struct cper_sec_prot_err *prot_e sizeof(cxl_ras->header_log), 0); } } + +static DECLARE_RWSEM(cxl_cper_rw_sem); +static cxl_cper_notifier cper_notifier; + +void cxl_cper_post_event(const char *pfx, guid_t *sec_type, + struct cxl_cper_event_rec *rec) +{ + enum cxl_event_type event_type; + + if (!(rec->hdr.validation_bits & CPER_CXL_COMP_EVENT_LOG_VALID)) { + pr_err(FW_WARN "cxl event no Component Event Log present\n"); + return; + } + + if (guid_equal(sec_type, &CPER_SEC_CXL_GEN_MEDIA_GUID)) + event_type = CXL_CPER_EVENT_GEN_MEDIA; + else if (guid_equal(sec_type, &CPER_SEC_CXL_DRAM_GUID)) + event_type = CXL_CPER_EVENT_DRAM; + else if (guid_equal(sec_type, &CPER_SEC_CXL_MEM_MODULE_GUID)) + event_type = CXL_CPER_EVENT_MEM_MODULE; + + guard(rwsem_read)(&cxl_cper_rw_sem); + if (cper_notifier) + cper_notifier(event_type, rec); +} + +int cxl_cper_register_notifier(cxl_cper_notifier notifier) +{ + guard(rwsem_write)(&cxl_cper_rw_sem); + if (cper_notifier) + return -EINVAL; + cper_notifier = notifier; + return 0; +} +EXPORT_SYMBOL_NS_GPL(cxl_cper_register_notifier, CXL); + +int cxl_cper_unregister_notifier(cxl_cper_notifier notifier) +{ + guard(rwsem_write)(&cxl_cper_rw_sem); + if (notifier != cper_notifier) + return -EINVAL; + cper_notifier = NULL; + return 0; +} +EXPORT_SYMBOL_NS_GPL(cxl_cper_unregister_notifier, CXL); diff --git a/drivers/firmware/efi/cper_cxl.h b/drivers/firmware/efi/cper_cxl.h index 86bfcf7909ec..71f27b3e2810 100644 --- a/drivers/firmware/efi/cper_cxl.h +++ b/drivers/firmware/efi/cper_cxl.h @@ -10,11 +10,38 @@ #ifndef LINUX_CPER_CXL_H #define LINUX_CPER_CXL_H +#include <linux/cxl-event.h> + /* CXL Protocol Error Section */ #define CPER_SEC_CXL_PROT_ERR \ GUID_INIT(0x80B9EFB4, 0x52B5, 0x4DE3, 0xA7, 0x77, 0x68, 0x78, \ 0x4B, 0x77, 0x10, 0x48) +/* CXL Event record UUIDs are formated at GUIDs and reported in section type */ +/* + * General Media Event Record + * CXL rev 3.0 Section 8.2.9.2.1.1; Table 8-43 + */ +#define CPER_SEC_CXL_GEN_MEDIA_GUID \ + GUID_INIT(0xfbcd0a77, 0xc260, 0x417f, \ + 0x85, 0xa9, 0x08, 0x8b, 0x16, 0x21, 0xeb, 0xa6) + +/* + * DRAM Event Record + * CXL rev 3.0 section 8.2.9.2.1.2; Table 8-44 + */ +#define CPER_SEC_CXL_DRAM_GUID \ + GUID_INIT(0x601dcbb3, 0x9c06, 0x4eab, \ + 0xb8, 0xaf, 0x4e, 0x9b, 0xfb, 0x5c, 0x96, 0x24) + +/* + * Memory Module Event Record + * CXL rev 3.0 section 8.2.9.2.1.3; Table 8-45 + */ +#define CPER_SEC_CXL_MEM_MODULE_GUID \ + GUID_INIT(0xfe927475, 0xdd59, 0x4339, \ + 0xa5, 0x86, 0x79, 0xba, 0xb1, 0x13, 0xb7, 0x74) + #pragma pack(1) /* Compute Express Link Protocol Error Section, UEFI v2.10 sec N.2.13 */ @@ -62,5 +89,7 @@ struct cper_sec_prot_err { #pragma pack() void cper_print_prot_err(const char *pfx, const struct cper_sec_prot_err *prot_err); +void cxl_cper_post_event(const char *pfx, guid_t *sec_type, + struct cxl_cper_event_rec *rec); #endif //__CPER_CXL_ diff --git a/include/linux/cxl-event.h b/include/linux/cxl-event.h index 18dab4d90dc8..2b137aead750 100644 --- a/include/linux/cxl-event.h +++ b/include/linux/cxl-event.h @@ -108,4 +108,54 @@ struct cxl_event_record_raw { union cxl_event event; } __packed; +enum cxl_event_type { + CXL_CPER_EVENT_GEN_MEDIA, + CXL_CPER_EVENT_DRAM, + CXL_CPER_EVENT_MEM_MODULE, +}; + +#define CPER_CXL_DEVICE_ID_VALID BIT(0) +#define CPER_CXL_DEVICE_SN_VALID BIT(1) +#define CPER_CXL_COMP_EVENT_LOG_VALID BIT(2) +struct cxl_cper_event_rec { + struct { + u32 length; + u64 validation_bits; + struct cper_cxl_event_devid { + u16 vendor_id; + u16 device_id; + u8 func_num; + u8 device_num; + u8 bus_num; + u16 segment_num; + u16 slot_num; /* bits 2:0 reserved */ + u8 reserved; + } device_id __packed; + struct cper_cxl_event_sn { + u32 lower_dw; + u32 upper_dw; + } dev_serial_num __packed; + } hdr __packed; + + union cxl_event event; +} __packed; + +typedef void (*cxl_cper_notifier)(enum cxl_event_type type, + struct cxl_cper_event_rec *rec); + +#ifdef CONFIG_UEFI_CPER +int cxl_cper_register_notifier(cxl_cper_notifier notifier); +int cxl_cper_unregister_notifier(cxl_cper_notifier notifier); +#else +static inline int cxl_cper_register_notifier(cxl_cper_notifier notifier) +{ + return 0; +} + +static inline int cxl_cper_unregister_notifier(cxl_cper_notifier notifier) +{ + return 0; +} +#endif + #endif /* _LINUX_CXL_EVENT_H */
BIOS can configure memory devices as firmware first. This will send CXL events to the firmware instead of the OS. The firmware can then send these events to the OS via UEFI. UEFI v2.10 section N.2.14 defines a Common Platform Error Record (CPER) format for CXL Component Events. The format is mostly the same as the CXL Common Event Record Format. The difference is the use of a GUID in the Section Type rather than a UUID as part of the event itself. Add EFI support to detect CXL CPER records and call a registered notifier with the event. Enforce that only one notifier call can be registered at any time. Cc: Ard Biesheuvel <ardb@kernel.org> Signed-off-by: Ira Weiny <ira.weiny@intel.com> --- Changes for v4 [kernel test robot: fix CONFIG_UEFI_CPER=n build] [kernel test robot: use static for cxl_cper_rw_sem] Changes for v3: [djbw: check notifier vs registered and error if not the same] [djbw: check for previous notifier and fail if already set] [djbw: use guard for rw sem] [djbw: pass the event type and record data as 2 parameters] [djbw: prefer __packed over pragma packed] [djbw: clean up commit message] [iweiny: clean structure names] --- drivers/firmware/efi/cper.c | 15 +++++++++++++ drivers/firmware/efi/cper_cxl.c | 46 +++++++++++++++++++++++++++++++++++++ drivers/firmware/efi/cper_cxl.h | 29 ++++++++++++++++++++++++ include/linux/cxl-event.h | 50 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 140 insertions(+)