Re: [PATCH 3/5] x86/ucode: Fold early_microcode_update_cpu() into early_microcode_init()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 28/03/2023 3:06 pm, Jan Beulich wrote:
> On 27.03.2023 21:41, Andrew Cooper wrote:
>> Begin to address this by folding early_update_cache() into it's single 
>> caller,
>> rearranging the exit path to always invalidate the mapping.
> ... this looks to lack editing after copy-and-paste from the earlier patch.

Will fix.

>> --- a/xen/arch/x86/cpu/microcode/core.c
>> +++ b/xen/arch/x86/cpu/microcode/core.c
>> @@ -868,8 +835,37 @@ int __init early_microcode_init(unsigned long 
>> *module_map,
>>  
>>      ucode_ops.collect_cpu_info();
>>  
>> -    if ( ucode_mod.mod_end || ucode_blob.size )
>> -        rc = early_microcode_update_cpu();
>> +    if ( ucode_blob.data )
>> +    {
>> +        blob = ucode_blob;
>> +    }
>> +    else if ( ucode_mod.mod_end )
>> +    {
>> +        blob.data = bootstrap_map(&ucode_mod);
>> +        blob.size = ucode_mod.mod_end;
>> +    }
> I wonder whether the order of if/else-if being different between
> microcode_init_cache() and here (also before your change) is meaningful
> in any way. I would prefer if the checking was always done in the same
> order, if I can talk you into re-arranging here and/or in the earlier
> patch.

It does matter, yes (well - certainly in patch 2).  (Although I see a
.size -> .data typo in the moved code, which I need to fix.)

However, both these chains are deleted in patch 5, so I'm going to keep
patches 2 and 3 as close to pure code movement as I can.

~Andrew