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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.03.2023 17:11, Andrew Cooper wrote:
> On 28/03/2023 3:06 pm, Jan Beulich wrote:
>> On 27.03.2023 21:41, Andrew Cooper wrote:
>>> --- 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.

Right - having seen the last patch, I'm certainly okay with this.

Jan