[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
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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |