lkml.org 
[lkml]   [2023]   [Oct]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRE: [PATCH RFC v2 2/3] firmware/efi: Process CXL Component Events
    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 only difference is the UUID is
    > passed via the Section Type as a GUID and not included as part of the
    > record data.
    >
    > Add EFI support to detect CXL CPER records and call a notifier chain
    > with the record data blobs.
    >
    > Note that the format of a GUID and UUID are not the same. Therefore the
    > Section Type GUID defines are duplicated from the CXL code.
    >
    > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
    >
    > ---
    > Changes from RFC v1
    > [iweiny: use an enum for know record types and skip converting GUID to UUID]
    > [iweiny: commit to the UUID not being part of the event record data]
    > [iweiny: use defines for GUID definitions]
    > ---
    > drivers/firmware/efi/cper.c | 16 +++++++++++
    > drivers/firmware/efi/cper_cxl.c | 40 ++++++++++++++++++++++++++++
    > drivers/firmware/efi/cper_cxl.h | 29 ++++++++++++++++++++
    > include/linux/efi.h | 59 +++++++++++++++++++++++++++++++++++++++++
    > 4 files changed, 144 insertions(+)
    >
    > diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
    > index 35c37f667781..d6415c94d584 100644
    > --- a/drivers/firmware/efi/cper.c
    > +++ b/drivers/firmware/efi/cper.c
    > @@ -607,6 +607,22 @@ 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_equal(sec_type, &CPER_SEC_CXL_DRAM) ||
    > + guid_equal(sec_type, &CPER_SEC_CXL_MEM_MODULE)) {
    > + struct cper_cxl_event_rec *rec = acpi_hest_get_payload(gdata);
    > +
    > + printk("%ssection type: CXL Event\n", newpfx);

    I would say that since this is going to be hanlded elsewhere the kernel
    log can stay silent.

    > +
    > + 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;
    > + }
    > +
    > + cper_post_cxl_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..04234884898d 100644
    > --- a/drivers/firmware/efi/cper_cxl.c
    > +++ b/drivers/firmware/efi/cper_cxl.c
    > @@ -187,3 +187,43 @@ void cper_print_prot_err(const char *pfx, const struct cper_sec_prot_err *prot_e
    > sizeof(cxl_ras->header_log), 0);
    > }
    > }
    > +
    > +/* CXL CPER notifier chain */
    > +static BLOCKING_NOTIFIER_HEAD(cxl_cper_chain_head);
    > +
    > +void cper_post_cxl_event(const char *pfx, guid_t *sec_type,
    > + struct cper_cxl_event_rec *rec)
    > +{
    > + struct cxl_cper_notifier_data nd = {
    > + .rec = rec,
    > + };
    > +
    > + 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))
    > + nd.cper_event = CXL_CPER_EVENT_GEN_MEDIA;
    > + else if (guid_equal(sec_type, &CPER_SEC_CXL_DRAM))
    > + nd.cper_event = CXL_CPER_EVENT_DRAM;
    > + else if (guid_equal(sec_type, &CPER_SEC_CXL_MEM_MODULE))
    > + nd.cper_event = CXL_CPER_EVENT_MEM_MODULE;
    > +
    > + if (blocking_notifier_call_chain(&cxl_cper_chain_head, 0, (void *)&nd)
    > + == NOTIFY_BAD)
    > + pr_err(FW_WARN "cxl event notifier chain failed\n");
    > +}
    > +
    > +int register_cxl_cper_notifier(struct notifier_block *nb)
    > +{
    > + return blocking_notifier_chain_register(&cxl_cper_chain_head, nb);
    > +}
    > +EXPORT_SYMBOL(register_cxl_cper_notifier);

    I think this should be EXPORT_SYMBOL_NS_GPL(..., CXL) since I can't
    imagine a third-party driver use case for this.

    > +
    > +void unregister_cxl_cper_notifier(struct notifier_block *nb)
    > +{
    > + blocking_notifier_chain_unregister(&cxl_cper_chain_head, nb);
    > +}
    > +EXPORT_SYMBOL(unregister_cxl_cper_notifier);
    > +
    > diff --git a/drivers/firmware/efi/cper_cxl.h b/drivers/firmware/efi/cper_cxl.h
    > index 86bfcf7909ec..ca26126cd9b8 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/efi.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 used as the 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_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_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_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 cper_post_cxl_event(const char *pfx, guid_t *sec_type,
    > + struct cper_cxl_event_rec *rec);
    >
    > #endif //__CPER_CXL_
    > diff --git a/include/linux/efi.h b/include/linux/efi.h
    > index 80b21d1c6eaf..b5b8b46c8deb 100644
    > --- a/include/linux/efi.h
    > +++ b/include/linux/efi.h
    > @@ -1355,4 +1355,63 @@ bool efi_config_table_is_usable(const efi_guid_t *guid, unsigned long table)
    >
    > umode_t efi_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n);
    >
    > +/*
    > + * Event log size adjusted for CPER
    > + *
    > + * Base table from CXL r3.0 Table 8-42: (30h + 50h)
    > + * For lack of UUID: - 10h
    > + *
    > + * (30h + 50h) - 10h = 70h
    > + */
    > +#define CPER_CXL_COMP_EVENT_LOG_SIZE 0x70
    > +#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 cper_cxl_event_rec {
    > + struct {
    > + u32 length;
    > + u64 validation_bits;
    > + struct {
    > + 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;
    > + struct {
    > + u32 lower_dw;
    > + u32 upper_dw;
    > + } dev_serial_num;
    > + } hdr;
    > +
    > + u8 comp_event_log[CPER_CXL_COMP_EVENT_LOG_SIZE];

    Rather than define CPER_CXL_COMP_EVENT_LOG_SIZE I would prefer that CXL
    and EFI share a common struct definition for these common fields.

    That would also remove the need for BUILD_BUG_ON() since they literally
    can not disagree on the size in that case.

    > +};
    > +#define CPER_CXL_REC_LEN(rec) (rec->hdr.length - sizeof(rec->hdr))
    > +
    > +enum cxl_cper_event {
    > + CXL_CPER_EVENT_GEN_MEDIA,
    > + CXL_CPER_EVENT_DRAM,
    > + CXL_CPER_EVENT_MEM_MODULE,
    > +};

    It follows from defining that common data structure above that this enum
    would be a generic CXL namespace that drops "_CPER_". I.e. the CPER
    notification handler and the native driver translate the event to this
    common generic sub-structure that gets emitted.

    > +
    > +struct cxl_cper_notifier_data {
    > + enum cxl_cper_event cper_event;
    > + struct cper_cxl_event_rec *rec;
    > +};
    > +
    > +#ifdef CONFIG_EFI
    > +int register_cxl_cper_notifier(struct notifier_block *nb);
    > +void unregister_cxl_cper_notifier(struct notifier_block *nb);
    > +#else
    > +static inline int register_cxl_cper_notifier(struct notifier_block *nb)
    > +{
    > + return 0;
    > +}
    > +
    > +static inline void unregister_cxl_cper_notifier(struct notifier_block *nb) { }
    > +#endif
    > +
    > #endif /* _LINUX_EFI_H */
    >
    > --
    > 2.41.0
    >


    \
     
     \ /
      Last update: 2023-10-26 23:03    [W:4.673 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site