Received-SPF: pass (google.com: domain of linux-kernel+bounces-5507-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Tue, 19 Dec 2023 07:43:45 -0800
From: Ira Weiny <ira.weiny@intel.com>
To: Dan Williams <dan.j.williams@intel.com>, Smita Koralahalli
	<Smita.KoralahalliChannabasappa@amd.com>, Ira Weiny <ira.weiny@intel.com>,
	Jonathan Cameron <jonathan.cameron@huawei.com>, Shiju Jose
	<shiju.jose@huawei.com>
CC: Yazen Ghannam <yazen.ghannam@amd.com>, Davidlohr Bueso
	<dave@stgolabs.net>, Dave Jiang <dave.jiang@intel.com>, Alison Schofield
	<alison.schofield@intel.com>, Vishal Verma <vishal.l.verma@intel.com>, "Ard
 Biesheuvel" <ardb@kernel.org>, <linux-efi@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-cxl@vger.kernel.org>
Subject: Re: [PATCH v4 6/7] firmware/efi: Process CXL Component Events
Message-ID: <6581ba314ee16_24f6b29431@iweiny-mobl.notmuch>
References: <20231215-cxl-cper-v4-0-01b6dab44fcd@intel.com>
 <20231215-cxl-cper-v4-6-01b6dab44fcd@intel.com>
 <a32464b2-9810-ca9d-73e5-4ed4cf69fea0@amd.com>
 <6580aa72ca0d1_269bd294e8@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <6580aa72ca0d1_269bd294e8@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?vrUqwtze+z8w2rYO3huwf1JAaJ4+tZD2Nt+WaBACqcFC1pwIxUgNUFzJ+bYS?=
 =?us-ascii?Q?lPCoFjZsXeSNNzMuaQ9tB7ZpB5TZuafXuQ5alkjg5cMVYNz4SeTrw9iA5arP?=
 =?us-ascii?Q?iyy6D29bhXBj/dCY70Xz5vwWCmjDJDECQrh/MxkvUItBl18PZePm4401hkgz?=
 =?us-ascii?Q?Y7TRi3LoFCxUb6caRQX/ci3/2x9B9Qsux1upGry/tR6R2cjDMAUYAYggOsTN?=
 =?us-ascii?Q?QMeFYiNWiN74mpjYHxOUxvuoCjrU5KONXZnV61e3iLJ3Lj2jp859WzH6K9nT?=
 =?us-ascii?Q?jq0aXqKQmRCeKIxY9KENaj8X+ZNwGo6qKHbzdwqytro9KumUGUeoEk1Ntl2R?=
 =?us-ascii?Q?T9RnvSEpDPFC9QKt2rRACVtr7Itg9v+Kzl93MiGs3wEBoVYfd/wMY+m6k0C3?=
 =?us-ascii?Q?/k4V72JuPM56WBJUmarInOYt23osClT9AN0Tvpwf4aVDMBgHjt0AKIVc75Qf?=
 =?us-ascii?Q?WPyyUeePK3Hm7Tw8zUNkCLMMfN5pKlqPqui6mWip2++6AFF+k1wmDorHLBTh?=
 =?us-ascii?Q?IaBU+zLz+S1x27cU9LnSK0fhRqX9oWqRl/IsLogy9FoBJy2k8gEpu/VSotGw?=
 =?us-ascii?Q?GO7GVGfsboBhOywAoybnZm53b+Rc1p2Xhm7iFYZc3sD/22sA7PQLg2tqB15I?=
 =?us-ascii?Q?xB9hwzGHv2MT08vQ3YbN53EdOeXqyMU2aDQY2D0RHfWWkFF6B0OkZ0NBLEWU?=
 =?us-ascii?Q?Jip4Hma8GMz+/eVlaDA5BnUSAIARx5oHx34z0OXtkfAGwhztwrfCt74OI65I?=
 =?us-ascii?Q?Zw8bdfhcunbg36durQQL1EZgQ1nA17Ji3MRXYlYShjuqp3bQwgOyukIbyLEC?=
 =?us-ascii?Q?t6wF3H/FL1nNP+vU54z9tj/yYtlvaMUXMYgZEWLvoZN4ne6i2MKcq0M/8dW2?=
 =?us-ascii?Q?Y+vN11z0MZPub1+jSsfxlyLuHpoVf6tTa/q85ikt9PmKwBhfzu6K9N7NA/n6?=
 =?us-ascii?Q?PyvhndBmxu46Qb+AT9Y2eB6r/hpGjsQuDw2TtZ2C9u/2LL2KsbTCWlqdYgVi?=
 =?us-ascii?Q?+crD6An5Ebxom09c2JuNgLyobT3/eoBEQoAzE739vshvdguTadcj30uq9Bdg?=
 =?us-ascii?Q?UtMcavyEEXxm1soK+j/SDPPluy/98bxwzFSght3uuiPjMSMPiEeHypK+glh+?=
 =?us-ascii?Q?Nx26HA+lLFH0fuKyPEAs5R3TxCEh18W9Ae7CpTwcIfpOdszE7ER2WGd6JlDN?=
 =?us-ascii?Q?vMJUuNBtM2bE4Cmtb5QMU3nntwJnIwD3aMDfJsdFiodTTmjZPq3/JWr7eMIF?=
 =?us-ascii?Q?WPyNwJh4bM0yDVcm+STaaZP/SaqOMWDS85SwkvH5oZ8CgYw3dgQlSfct8zzf?=
 =?us-ascii?Q?uNJXzXB84uIBizsH2qRlzmvrWqDrDENdf4XvgSt4ILmj8RJl1pB6ej4t7wo9?=
 =?us-ascii?Q?fOJBTctuJifPPt2eSOip+KWygDKZZpcUgmvLtO9oPIKBN8pamWqdhL6bY3fy?=
 =?us-ascii?Q?+OUUZ19phzlExTpSZ6hxsWNj7uxhtx/86ZDxW0RHgp/k3INsfj2SxJ1tEmdD?=
 =?us-ascii?Q?BgiiJVgxoTPlsu6swES0tJgbtHb8hfXLbi4AZ5sY/tJ3Ihw/aCukUc0+u/PM?=
 =?us-ascii?Q?ajw32g5L66cS+gluzSGZwrkdbfpJP8Cc5wnSl2IS9RFjdUuGDS+A8DouCXBH?=
 =?us-ascii?Q?uJFYWggI0/UPmGt+jI0H6WM0TRP2aPkifn97BKbuBk99?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 274a748e-e9d1-4613-edfc-08dc00a94be3
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Dec 2023 15:43:50.4004
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: FZ2t7StMacCXv3mSjbCh7HEnO5suus4jAq1cEUrBI+gxAeYiJis8YyftgWz927Op9BVrmUfJ5f3npYb4Po8kNQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB7970
X-OriginatorOrg: intel.com

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