Re: [PATCH] x86/ucode: Trivial further cleanup

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 08/10/2020 08:49, Roger Pau Monné wrote:
> On Wed, Oct 07, 2020 at 07:01:20PM +0100, Andrew Cooper wrote:
>>  * Drop unused include in private.h.
>>  * Used explicit width integers for Intel header fields.
>>  * Adjust comment to better describe the extended header.
>>  * Drop unnecessary __packed attribute for AMD header.
>>  * Switch mc_patch_data_id to being uint16_t, which is how it is more 
>> commonly
>>    referred to.
>>  * Fix types and style.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks,

>> ---
>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> CC: Wei Liu <wl@xxxxxxx>
>> ---
>>  xen/arch/x86/cpu/microcode/amd.c     | 10 +++++-----
>>  xen/arch/x86/cpu/microcode/intel.c   | 34 +++++++++++++++++-----------------
>>  xen/arch/x86/cpu/microcode/private.h |  2 --
>>  3 files changed, 22 insertions(+), 24 deletions(-)
>>
>> diff --git a/xen/arch/x86/cpu/microcode/amd.c 
>> b/xen/arch/x86/cpu/microcode/amd.c
>> index cd532321e8..e913232067 100644
>> --- a/xen/arch/x86/cpu/microcode/amd.c
>> +++ b/xen/arch/x86/cpu/microcode/amd.c
>> @@ -24,7 +24,7 @@
>>  
>>  #define pr_debug(x...) ((void)0)
>>  
>> -struct __packed equiv_cpu_entry {
>> +struct equiv_cpu_entry {
>>      uint32_t installed_cpu;
>>      uint32_t fixed_errata_mask;
>>      uint32_t fixed_errata_compare;
>> @@ -35,7 +35,7 @@ struct __packed equiv_cpu_entry {
>>  struct microcode_patch {
>>      uint32_t data_code;
>>      uint32_t patch_id;
>> -    uint8_t  mc_patch_data_id[2];
>> +    uint16_t mc_patch_data_id;
>>      uint8_t  mc_patch_data_len;
> I think you could also drop the mc_patch_ prefixes from a couple of
> fields in this structure, since they serve no purpose AFAICT.

Actually, I'll drop this change and leave the field names alone. 
Stripping that prefix will make the field names logically wrong (e.g.
data_len isn't the length of the header, or of the entire patch), and
I've got other work planned to clean this area up.

~Andrew