Re: [Xen-devel] [PATCH 4/7] x86/ucode/intel: Reimplement get_{data, total}size() helpers

From: Jan Beulich <jbeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Xen-devel <xen-devel@lists.xenproject.org>,
	"Wei Liu" <wl@xen.org>, "Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH 4/7] x86/ucode/intel: Reimplement get_{data, total}size() helpers
Date: Wed, 25 Mar 2020 14:41:05 +0100	[thread overview]
Message-ID: <52481e12-eb98-eead-be74-d48815e5a41f@suse.com> (raw)
In-Reply-To: <20200323101724.15655-5-andrew.cooper3@citrix.com>

On 23.03.2020 11:17, Andrew Cooper wrote:
> Every caller actually passes a struct microcode_header_intel.  Implement the
> helpers with proper types, and leave a comment explaining the Pentium Pro/II
> behaviour with empty {data,total}size fields.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with...

> --- a/xen/arch/x86/cpu/microcode/intel.c
> +++ b/xen/arch/x86/cpu/microcode/intel.c
> @@ -46,9 +46,16 @@ struct microcode_header_intel {
>      unsigned int sig;
>      unsigned int cksum;
>      unsigned int ldrver;
> +
> +    /*
> +     * Microcode for the Pentium Pro and II had all further fields in the
> +     * header reserved, had a fixed datasize of 2000 and totalsize of 2048,
> +     * and didn't use platform flags despite the availability of the MSR.
> +     */
> +
>      unsigned int pf;
> -    unsigned int datasize;
> -    unsigned int totalsize;
> +    unsigned int _datasize;
> +    unsigned int _totalsize;

... the underscores here dropped again. Or else - why did you add
them? This (to me at least) doesn't e.g. make any more clear that
the fields may be zero on old hardware.

Furthermore - do we really need this PPro/PentiumII logic seeing
that these aren't 64-bit capable CPUs?

Jan