Re: [PATCH 02/12] x86/boot: eliminate module_map

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.11.2024 15:34, Andrew Cooper wrote:
> On 02/11/2024 5:25 pm, Daniel P. Smith wrote:
>> With all boot modules now labeled by type, it is no longer necessary to
>> track whether a boot module was identified via the module_map bitmap.
>>
>> Introduce a set of helpers to search the list of boot modules based on type 
>> and
>> the reference type, pointer or array index, desired. Then drop all uses of
>> setting a bit in module_map and replace its use for looping with the helpers.
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
>> ---
>> Changes since v7:
>> - collapse the three module_map patches into one,
>>   - x86/boot: remove module_map usage from microcode loading
>>   - x86/boot: remove module_map usage from xsm policy loading
>>   - x86/boot: remove module_map usage by ramdisk loading
> 
> Definitely nicer for having been collapsed together.
> 
>> ---
>>  xen/arch/x86/cpu/microcode/core.c   | 12 ++++-----
>>  xen/arch/x86/include/asm/bootinfo.h | 42 ++++++++++++++++++++++++++++-
>>  xen/arch/x86/setup.c                | 28 +++++++++++--------
>>  xen/xsm/xsm_policy.c                | 19 +++++--------
>>  4 files changed, 70 insertions(+), 31 deletions(-)
>>
>> diff --git a/xen/arch/x86/cpu/microcode/core.c 
>> b/xen/arch/x86/cpu/microcode/core.c
>> index f46464241557..b09cf83249f6 100644
>> --- a/xen/arch/x86/cpu/microcode/core.c
>> +++ b/xen/arch/x86/cpu/microcode/core.c
>> @@ -790,15 +790,13 @@ static int __init early_microcode_load(struct 
>> boot_info *bi)
>>  
>>      if ( opt_scan ) /* Scan for a CPIO archive */
>>      {
>> -        for ( idx = 1; idx < bi->nr_modules; ++idx )
>> +        for_each_boot_module_by_type(idx, bi, BOOTMOD_UNKNOWN)
> 
> Minor, but we treat for_each_* as if they were for loops, so this either
> wants to be
> 
> for_each_boot_module_by_type ( idx, bi, BOOTMOD_UNKNOWN )
> 
> or
> 
> for_each_boot_module_by_type (idx, bi, BOOTMOD_UNKNOWN)
> 
> spacing wise.  There's no agreement between maintainers on the extra
> spaces inside brackets or not.

Just to clarify - no, the latter form you suggest is not okay to use.
Daniel's form is, as is the first one you suggest. The choice is between
"like a for() loop" (your 1st form) and "just another macro invocation"
(Daniel's form).

Jan