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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 25/03/2020 13:41, Jan Beulich wrote:
> 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@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> 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.

No, but it is our normal hint that you shouldn't be using the field
directly, and should be using the accessors instead.

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

I did actually drop support in one version of my series, but put it back in.

These old microcode blobs are still around, including in some versions
of microcode.dat.  By dropping the ability to recognise them as
legitimate, we'd break the logic to search through a container of
multiple blobs to find the one which matches.

~Andrew