Re: [PATCH v4 6/7] firmware/efi: Process CXL Component Events

From: Dan Williams
Date: Mon Dec 18 2023 - 15:24:39 EST


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@xxxxxxxxxx>
> > Signed-off-by: Ira Weiny <ira.weiny@xxxxxxxxx>
> >
>
> [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;