Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 11 Nov 2022 13:29:13 -0800
From:   Ashok Raj <ashok.raj@intel.com>
To:     Dave Hansen <dave.hansen@intel.com>
CC:     Borislav Petkov <bp@alien8.de>,
        Jithu Joseph <jithu.joseph@intel.com>, <hdegoede@redhat.com>,
        <markgross@kernel.org>, <tglx@linutronix.de>, <mingo@redhat.com>,
        <dave.hansen@linux.intel.com>, <x86@kernel.org>, <hpa@zytor.com>,
        <gregkh@linuxfoundation.org>, <tony.luck@intel.com>,
        <linux-kernel@vger.kernel.org>,
        <platform-driver-x86@vger.kernel.org>, <patches@lists.linux.dev>,
        <ravi.v.shankar@intel.com>, <thiago.macieira@intel.com>,
        <athenas.jimenez.gonzalez@intel.com>, <sohil.mehta@intel.com>,
        Ashok Raj <ashok.raj@intel.com>
Subject: Re: [PATCH v2 10/14] platform/x86/intel/ifs: Add metadata validation
Message-ID: <Y26+qclls38ugbub@a4bf019067fa.jf.intel.com>
References: <20221021203413.1220137-1-jithu.joseph@intel.com>
 <20221107225323.2733518-1-jithu.joseph@intel.com>
 <20221107225323.2733518-11-jithu.joseph@intel.com>
 <Y26W+emSSL5Xik0G@zn.tnic>
 <82d6137f-4183-d994-f8b1-56b4be3d620f@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <82d6137f-4183-d994-f8b1-56b4be3d620f@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?xcPpPMQAyY4Z0h/O88nccx/L3Bfj9ZOoTXB+JG8+a3V0obMG6xz2N+zuMaII?=
 =?us-ascii?Q?KdzIv1uI1vTtYlqZxUYboimfug6JRSTwqUnmyRYYtlr/e5f8nVFjtstybw1m?=
 =?us-ascii?Q?sQTRdp8++PAWCcSPm+n8d5S0fOqjrQVN1twR3DiFCmjKeKUPRNPgeYYdFMe5?=
 =?us-ascii?Q?uwdHpu27vcHmOSv4OwYPEMc46shaT0lVGSLaVioID/cq/kKz+46zWxynPqfA?=
 =?us-ascii?Q?iCwShyOqX6EOYzxISWHX+z/a2m6if08NIuoinutNHTM57yUmN5skH2nWIHM4?=
 =?us-ascii?Q?7LmwCn519M5EJ2x61r2XOMpZoYtCBZXpW3SVa6b+6TIFEd3VKilAXqg5DKPf?=
 =?us-ascii?Q?ZmeIGSDgrBty2+fVEQUSB+T7y8sO1zseULrGDBFc8KTgEVFoBLTO2so4a2X2?=
 =?us-ascii?Q?zTEa7EoX6jyA0LEPrUduiQGDVR4/YyXCEIoyuul+2UkMR/uq0bUIjvCCNk4a?=
 =?us-ascii?Q?WnmcrKi8SKpycLwUoDhZRDVyfkMzr6JiR7rYbc7W8iWYH3Hyej6H36Wttg6M?=
 =?us-ascii?Q?FgU+Lc3FfrznOj47VlC3BQAW3NrQgMrntBsHLyya3zxNVVgrn5OCLzjXYQpn?=
 =?us-ascii?Q?WrfUcbAUsqjSwwIF0h0PCJbJDiMkYY6Mb3g3aaNtjfRA73jPs5jYIq6wqjwt?=
 =?us-ascii?Q?7hBmAqCLE7chc7GfKhilOyGmYRSOmfYu6vwTlh6sao1XKcGh6TK+TJvGQVcO?=
 =?us-ascii?Q?f4UtyRg2OuZwIZdIzXHq2gTVsSdq5uHC7fSMLetE/wqhWbDC+dq3kajVTdgE?=
 =?us-ascii?Q?jvEBYiLCKuepZOAFZyV04LASGJ8M3/aB7gI1121svZWo6i2I8QCvIYlEE6P4?=
 =?us-ascii?Q?ce7iVSqBm4MftZnhhe5OyJLel786b+xSiGzXsD9ZT8LUoHIFcStEcEmEgD3F?=
 =?us-ascii?Q?v5Eom1ccwU/zDnXlfQcwzIFE6OAASbcZYN5lBZSDGmq8ekFc9klcssRN9Jot?=
 =?us-ascii?Q?Ob/VPOlN6WNx/d67JRV8oYjkdP3Nr2x1Q9do5xAplES1G2goiJBa7eS9Lqir?=
 =?us-ascii?Q?pT1P8ZTS9xw81vj3YfTyN6zhkh36z+rwuwtTGsQlEZNJRQFal3WF4goBJIpG?=
 =?us-ascii?Q?Gwk12oZVVFq+ye3gTM8yikJ6J2+D/DOZ8GPrWpGglIgJXsVMuEYhs7j4Qrp/?=
 =?us-ascii?Q?PbUoSc9guAnrZCQi9K5NvCQYR4y8HIoOi/tkS8kagQ0gzYXei2jFK5MzHg7K?=
 =?us-ascii?Q?oXc+p2AYgmLKM8Gz9XIuZnyP9M/OJnP754sl36E8X3DNS2N7FXWpZcu+bQzU?=
 =?us-ascii?Q?8mA1Ae09h650/NlmY/kd2RQrsTaBde+8tOOPDbXsaq+IhHXvl+TI+Hi4kOZv?=
 =?us-ascii?Q?nuK4D+7ysDRW/2EOvh2Y9HgR+VNf7nV1s8OnseFEZS0x3LkLocGfF8a/Vqrr?=
 =?us-ascii?Q?8iuBLMF/XkY99nzZwFF446uHwODMjoRkKJbIP6nMi+x7kXO0xXDaoEykZ9bw?=
 =?us-ascii?Q?fn+XLmB03v067tFpCyqc0xEbdd9WE8IrxLW9Gr1Scr+NRgG/aArWGSXSq9bP?=
 =?us-ascii?Q?oJwUyUuwLkDEsdgVwJ5hhnoeVfsg6nk1pIimtITja/6PANEaL5eKQrXUV/S/?=
 =?us-ascii?Q?luxO5aJQaIjrybXq3edvoe1tZhFRvSOWhHr/Nkiv?=
X-MS-Exchange-CrossTenant-Network-Message-Id: ba9f567a-9ce8-4598-7a21-08dac42bcaea
X-MS-Exchange-CrossTenant-AuthSource: SJ1PR11MB6201.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Nov 2022 21:29:19.4910
 (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: xb+uXfRGFf/nHj90DcmoOaMmMME8jD/J73PkK70YyHVgKKugJI4vgQm6NXu2UIBMkEvMQ/GcaQfYlzi/AAc7mg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB7147
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI,
        SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Nov 11, 2022 at 10:48:49AM -0800, Dave Hansen wrote:
> On 11/11/22 10:39, Borislav Petkov wrote:
> >> +struct meta_data {
> >> +	unsigned int meta_type;		// metadata type
> >> +	unsigned int meta_size;		// size of this entire struct including hdrs.
> >> +	unsigned int test_type;		// IFS test type
> >> +	unsigned int fusa_info;		// Fusa info
> >> +	unsigned int total_images;	// Total number of images
> >> +	unsigned int current_image;	// Current Image #
> >> +	unsigned int total_chunks;	// Total number of chunks in this image
> >> +	unsigned int starting_chunk;	// Starting chunk number in this image
> >> +	unsigned int size_per_chunk;	// size of each chunk
> >> +	unsigned int chunks_per_stride;	// number of chunks in a stride
> >> +	unsigned int reserved[54];	// Align to 256 bytes for chunk alignment.
> > That looks weird.
> > 
> > __packed and __aligned doesn't work?
> 
> ... and don't we try to use fixed-size typed in hardware structures,
> like u32?

Maybe should we do a cleanup patch to move struct microcode_header_intel
also to u32 format then add meta_data to follow the same style?

When we defined struct meta_data followed the format and style that was
already in place.

The suggestion below is a lot nicer, let the compiler do te job.

> 
> There are also much nicer ways to do this:
> 
> union meta_data {
> 	struct {
> 		u32 meta_type;		// metadata type
> 		u32 meta_size;		// size of ...
> 	};
> 	u8 padding[IFS_CHUNK_ALIGNMENT];
> }
> 
> That doesn't have any magic linkage between the magic "54" (times 4) and
> IFS_CHUNK_ALIGNMENT.  It makes the compiler do the hard work for you.
> 
> Voila, you have a union that's always IFS_CHUNK_ALIGNMENT in size,  No
> magic 54's necessary.