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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6580aa72ca0d1_269bd294e8@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Date: Mon, 18 Dec 2023 12:24:18 -0800
From: Dan Williams <dan.j.williams@...el.com>
To: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>, Ira Weiny
	<ira.weiny@...el.com>, Dan Williams <dan.j.williams@...el.com>, "Jonathan
 Cameron" <jonathan.cameron@...wei.com>, Shiju Jose <shiju.jose@...wei.com>
CC: Yazen Ghannam <yazen.ghannam@....com>, Davidlohr Bueso
	<dave@...olabs.net>, Dave Jiang <dave.jiang@...el.com>, Alison Schofield
	<alison.schofield@...el.com>, Vishal Verma <vishal.l.verma@...el.com>, "Ard
 Biesheuvel" <ardb@...nel.org>, <linux-efi@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linux-cxl@...r.kernel.org>
Subject: Re: [PATCH v4 6/7] firmware/efi: Process CXL Component Events

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@...nel.org>
> > Signed-off-by: Ira Weiny <ira.weiny@...el.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;