On 26/03/2020 14:56, Jan Beulich wrote:
> On 26.03.2020 15:35, Andrew Cooper wrote:
>> On 25/03/2020 13:41, Jan Beulich wrote:
>>> On 23.03.2020 11:17, Andrew Cooper wrote:
>>>> --- 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.
> Yet in patch 5 you do. Perhaps for an understandable reason, but
> that way you at least partly invalidate what you say above.
The net result of of patch 5 is three adjacent helpers, which are the
only code which use the fields themselves.
I can drop if you really insist. We're only talking about 400 lines or
code, or thereabouts.
>>> 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.
> Oh, good point.
Shame I only came to that realisation after having reworked the series
to take it out...
I'm constructing companion document to
https://xenbits.xen.org/docs/sphinx-unstable/admin-guide/microcode-loading.html
which will live in hypervisor-guide section, and cover a whole range of
topics like this.
~Andrew
