Re: [PATCH v4 06/44] x86/boot: convert consider_modules to struct boot_module

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.08.2024 23:46, Daniel P. Smith wrote:
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -632,7 +632,7 @@ static void __init noinline move_xen(void)
>  #undef BOOTSTRAP_MAP_LIMIT
>  
>  static uint64_t __init consider_modules(
> -    uint64_t s, uint64_t e, uint32_t size, const module_t *mod,
> +    uint64_t s, uint64_t e, uint32_t size, const struct boot_module *mods,

As an array is meant, may I ask to switch to mods[] at this occasion?

> @@ -642,20 +642,20 @@ static uint64_t __init consider_modules(
>  
>      for ( i = 0; i < nr_mods ; ++i )
>      {
> -        uint64_t start = (uint64_t)mod[i].mod_start << PAGE_SHIFT;
> -        uint64_t end = start + PAGE_ALIGN(mod[i].mod_end);
> +        uint64_t start = (uint64_t)mods[i].early_mod->mod_start << 
> PAGE_SHIFT;

Similarly, may I ask to stop open-coding {,__}pfn_to_paddr() while
transforming this?

> @@ -1447,7 +1447,7 @@ void asmlinkage __init noreturn __start_xen(unsigned 
> long mbi_p)
>          {
>              /* Don't overlap with modules. */
>              end = consider_modules(s, e, reloc_size + mask,
> -                                   mod, boot_info->nr_mods, -1);
> +                                   boot_info->mods, boot_info->nr_mods, -1);
>              end &= ~mask;
>          }
>          else
> @@ -1482,7 +1482,7 @@ void asmlinkage __init noreturn __start_xen(unsigned 
> long mbi_p)
>                  continue;
>  
>              /* Don't overlap with other modules (or Xen itself). */
> -            end = consider_modules(s, e, size, mod,
> +            end = consider_modules(s, e, size, boot_info->mods,
>                                     boot_info->nr_mods + relocated, j);
>  
>              if ( highmem_start && end > highmem_start )
> @@ -1509,7 +1509,7 @@ void asmlinkage __init noreturn __start_xen(unsigned 
> long mbi_p)
>          while ( !kexec_crash_area.start )
>          {
>              /* Don't overlap with modules (or Xen itself). */
> -            e = consider_modules(s, e, PAGE_ALIGN(kexec_crash_area.size), 
> mod,
> +            e = consider_modules(s, e, PAGE_ALIGN(kexec_crash_area.size), 
> boot_info->mods,
>                                   boot_info->nr_mods + relocated, -1);

All of these show a meaningful increase of line lengths, up to the point of
ending up with too long a line here. I really wonder if the variable name
"boot_info" isn't too long for something that's going to be used quite
frequently. Just "bi" maybe?

Jan