Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/setup.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 30a139074833..b3b6e6f38622 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -276,8 +276,6 @@ custom_param("acpi", parse_acpi_param);
 
 static const char *cmdline_cook(const char *p, const char *loader_name);
 
-static const struct boot_module *__initdata initial_images;
-
 struct boot_info __initdata xen_boot_info;
 
 static struct boot_info __init *multiboot_fill_boot_info(unsigned long mbi_p)
@@ -336,9 +334,9 @@ unsigned long __init initial_images_nrpages(nodeid_t node)
 
     for ( nr = i = 0; i < bi->nr_modules; ++i )
     {
-        unsigned long start = initial_images[i].mod->mod_start;
+        unsigned long start = bi->mods[i].mod->mod_start;
         unsigned long end = start +
-                            PFN_UP(initial_images[i].mod->mod_end);
+                            PFN_UP(bi->mods[i].mod->mod_end);
 
         if ( end > node_start && node_end > start )
             nr += min(node_end, end) - max(node_start, start);
@@ -355,15 +353,14 @@ void __init discard_initial_images(void)
     for ( i = 0; i < bi->nr_modules; ++i )
     {
         uint64_t start =
-            (uint64_t)initial_images[i].mod->mod_start << PAGE_SHIFT;
+            (uint64_t)bi->mods[i].mod->mod_start << PAGE_SHIFT;
 
         init_domheap_pages(start,
                            start +
-                           PAGE_ALIGN(initial_images[i].mod->mod_end));
+                           PAGE_ALIGN(bi->mods[i].mod->mod_end));
     }
 
     bi->nr_modules = 0;
-    initial_images = NULL;
 }
 
 static void __init init_idle_domain(void)
@@ -1383,8 +1380,6 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
     set_kexec_crash_area_size((u64)nr_pages << PAGE_SHIFT);
     kexec_reserve_area();
 
-    initial_images = bi->mods;
-
     for ( i = 0; !efi_enabled(EFI_LOADER) && i < bi->nr_modules; i++ )
     {
         if ( bi->mods[i].mod->mod_start & (PAGE_SIZE - 1) )
-- 
2.30.2

On 2024-10-06 17:49, Daniel P. Smith wrote:
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> ---
>   xen/arch/x86/setup.c | 13 ++++---------
>   1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 30a139074833..b3b6e6f38622 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -276,8 +276,6 @@ custom_param("acpi", parse_acpi_param);
>   
>   static const char *cmdline_cook(const char *p, const char *loader_name);
>   
> -static const struct boot_module *__initdata initial_images;
> -
>   struct boot_info __initdata xen_boot_info;
>   
>   static struct boot_info __init *multiboot_fill_boot_info(unsigned long mbi_p)
> @@ -336,9 +334,9 @@ unsigned long __init initial_images_nrpages(nodeid_t node)
>   
>       for ( nr = i = 0; i < bi->nr_modules; ++i )
>       {
> -        unsigned long start = initial_images[i].mod->mod_start;
> +        unsigned long start = bi->mods[i].mod->mod_start;
>           unsigned long end = start +
> -                            PFN_UP(initial_images[i].mod->mod_end);
> +                            PFN_UP(bi->mods[i].mod->mod_end);

Fits on a single line.

>   
>           if ( end > node_start && node_end > start )
>               nr += min(node_end, end) - max(node_start, start);
> @@ -355,15 +353,14 @@ void __init discard_initial_images(void)
>       for ( i = 0; i < bi->nr_modules; ++i )
>       {
>           uint64_t start =
> -            (uint64_t)initial_images[i].mod->mod_start << PAGE_SHIFT;
> +            (uint64_t)bi->mods[i].mod->mod_start << PAGE_SHIFT;

Fits on one line.  Can also be pfn_to_paddr(), which applies to earlier 
patches.  Having said that, maybe it's okay to skip pfn_to_paddr as at 
the end of the series mods[i].start is used without a shift.  i.e. fewer 
transformations in these "mechanical" changes make review easier. 
Unless someone else wants pfn_to_addr(), I am okay without that conversion.

>   
>           init_domheap_pages(start,
>                              start +
> -                           PAGE_ALIGN(initial_images[i].mod->mod_end));
> +                           PAGE_ALIGN(bi->mods[i].mod->mod_end));

One line.

>       }
>   
>       bi->nr_modules = 0;
> -    initial_images = NULL;
>   }
>   
>   static void __init init_idle_domain(void)

With the line fixups:

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

On 10/8/24 15:04, Jason Andryuk wrote:
> On 2024-10-06 17:49, Daniel P. Smith wrote:
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>> ---
>>   xen/arch/x86/setup.c | 13 ++++---------
>>   1 file changed, 4 insertions(+), 9 deletions(-)
>>
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index 30a139074833..b3b6e6f38622 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -276,8 +276,6 @@ custom_param("acpi", parse_acpi_param);
>>   static const char *cmdline_cook(const char *p, const char 
>> *loader_name);
>> -static const struct boot_module *__initdata initial_images;
>> -
>>   struct boot_info __initdata xen_boot_info;
>>   static struct boot_info __init *multiboot_fill_boot_info(unsigned 
>> long mbi_p)
>> @@ -336,9 +334,9 @@ unsigned long __init 
>> initial_images_nrpages(nodeid_t node)
>>       for ( nr = i = 0; i < bi->nr_modules; ++i )
>>       {
>> -        unsigned long start = initial_images[i].mod->mod_start;
>> +        unsigned long start = bi->mods[i].mod->mod_start;
>>           unsigned long end = start +
>> -                            PFN_UP(initial_images[i].mod->mod_end);
>> +                            PFN_UP(bi->mods[i].mod->mod_end);
> 
> Fits on a single line.

Ack.

>>           if ( end > node_start && node_end > start )
>>               nr += min(node_end, end) - max(node_start, start);
>> @@ -355,15 +353,14 @@ void __init discard_initial_images(void)
>>       for ( i = 0; i < bi->nr_modules; ++i )
>>       {
>>           uint64_t start =
>> -            (uint64_t)initial_images[i].mod->mod_start << PAGE_SHIFT;
>> +            (uint64_t)bi->mods[i].mod->mod_start << PAGE_SHIFT;
> 
> Fits on one line.  Can also be pfn_to_paddr(), which applies to earlier 
> patches.  Having said that, maybe it's okay to skip pfn_to_paddr as at 
> the end of the series mods[i].start is used without a shift.  i.e. fewer 
> transformations in these "mechanical" changes make review easier. Unless 
> someone else wants pfn_to_addr(), I am okay without that conversion.

After I did a few based on a comment from Jan on v4, I realized I am 
making changes that just go away or shift to a different conversion 
macro. I would prefer to just leave them to the end and make sure that 
when arriving at the final form, none of them are using any open coded 
forms.

I will move it up to a single line.

>>           init_domheap_pages(start,
>>                              start +
>> -                           PAGE_ALIGN(initial_images[i].mod->mod_end));
>> +                           PAGE_ALIGN(bi->mods[i].mod->mod_end));
> 
> One line.

Ack.

>>       }
>>       bi->nr_modules = 0;
>> -    initial_images = NULL;
>>   }
>>   static void __init init_idle_domain(void)
> 
> With the line fixups:
> 
> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Thanks!

v/r,
dps