[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Thu, 26 Mar 2020 15:09:56 +0000
  • Authentication-results: esa1.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=andrew.cooper3@xxxxxxxxxx; spf=Pass smtp.mailfrom=Andrew.Cooper3@xxxxxxxxxx; spf=None smtp.helo=postmaster@xxxxxxxxxxxxxxx
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Thu, 26 Mar 2020 15:10:14 +0000
  • Ironport-sdr: i31MZBr5Fvmu1NA6Gaxf16kKyFGlnzbcdEmZ89RfjMsw023uTl131oHvc3K3zcVuubjqI/MXj0 SNmxx6sgLlv+uJ1nZNvTNAm7KUgFhppiRZT9qsfFOSCN8DO1vn3qgn8P6JhfrePlPiTBGza+2A /KtyxjBZfFiZ9K1diJ6E9bdsH5bm4ihk43xIc3Q7jWv51DCvIdFOvBf138et2FZ75xWP4tIBmC opmp1laIud3+AtyxobrSaip3vsLT7NyRPkCLtPUmc/WTK39ltoBy7j0VPWRPGv2NpujWRGnFlW qGY=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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
which will live in hypervisor-guide section, and cover a whole range of
topics like this.




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.