This commit changes create_dom0 to no longer take the individual components and
take struct boot_info instead. Internally, it is changed to locate the kernel
and ramdisk details from struct boot_info.

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

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 0d2ee19998aa..c2bcddc50990 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -946,10 +946,8 @@ static unsigned int __init copy_bios_e820(struct e820entry *map, unsigned int li
     return n;
 }
 
-static struct domain *__init create_dom0(const module_t *image,
-                                         unsigned long headroom,
-                                         module_t *initrd, const char *kextra,
-                                         const char *loader)
+static struct domain *__init create_dom0(const struct boot_info *bi,
+                                         const char *kextra)
 {
     static char __initdata cmdline[MAX_GUEST_CMDLINE];
 
@@ -964,9 +962,21 @@ static struct domain *__init create_dom0(const module_t *image,
             .misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED : 0,
         },
     };
+    int headroom, mod_idx = first_boot_module_index(bi, BOOTMOD_RAMDISK);
+    module_t *image, *initrd;
     struct domain *d;
     domid_t domid;
 
+    /* Map boot_module to mb1 module for dom0 */
+    image = bi->mods[0].mod;
+    headroom = bi->mods[0].headroom;
+
+    /* Map boot_module to mb1 module for initrd */
+    if ( mod_idx < 0 )
+        initrd = NULL;
+    else
+        initrd = bi->mods[mod_idx].mod;
+
     if ( opt_dom0_pvh )
     {
         dom0_cfg.flags |= (XEN_DOMCTL_CDF_hvm |
@@ -995,7 +1005,7 @@ static struct domain *__init create_dom0(const module_t *image,
     if ( image->string || kextra )
     {
         if ( image->string )
-            safe_strcpy(cmdline, cmdline_cook(__va(image->string), loader));
+            safe_strcpy(cmdline, cmdline_cook(__va(image->string), bi->loader));
 
         if ( kextra )
             /* kextra always includes exactly one leading space. */
@@ -2106,10 +2116,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
      * We're going to setup domain0 using the module(s) that we stashed safely
      * above our heap. The second module, if present, is an initrd ramdisk.
      */
-    dom0 = create_dom0(bi->mods[0].mod, bi->mods[0].headroom,
-                       (initrdidx >= 0 && initrdidx < bi->nr_modules) ?
-                            bi->mods[initrdidx].mod : NULL,
-                       kextra, bi->loader);
+    dom0 = create_dom0(bi, kextra);
     if ( !dom0 )
         panic("Could not set up DOM0 guest OS\n");
 
-- 
2.30.2

On 2024-10-06 17:49, Daniel P. Smith wrote:
> This commit changes create_dom0 to no longer take the individual components and
> take struct boot_info instead. Internally, it is changed to locate the kernel
> and ramdisk details from struct boot_info.
> 
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> ---
>   xen/arch/x86/setup.c | 25 ++++++++++++++++---------
>   1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 0d2ee19998aa..c2bcddc50990 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -946,10 +946,8 @@ static unsigned int __init copy_bios_e820(struct e820entry *map, unsigned int li
>       return n;
>   }
>   
> -static struct domain *__init create_dom0(const module_t *image,
> -                                         unsigned long headroom,
> -                                         module_t *initrd, const char *kextra,
> -                                         const char *loader)
> +static struct domain *__init create_dom0(const struct boot_info *bi,
> +                                         const char *kextra)
>   {
>       static char __initdata cmdline[MAX_GUEST_CMDLINE];
>   
> @@ -964,9 +962,21 @@ static struct domain *__init create_dom0(const module_t *image,
>               .misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED : 0,
>           },
>       };
> +    int headroom, mod_idx = first_boot_module_index(bi, BOOTMOD_RAMDISK);

I think headroom should stay unsigned long, which matches struct 
boot_module.

With that

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

Regards,
Jason

On 10/8/24 12:52, Jason Andryuk wrote:
> On 2024-10-06 17:49, Daniel P. Smith wrote:
>> This commit changes create_dom0 to no longer take the individual 
>> components and
>> take struct boot_info instead. Internally, it is changed to locate the 
>> kernel
>> and ramdisk details from struct boot_info.
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>> ---
>>   xen/arch/x86/setup.c | 25 ++++++++++++++++---------
>>   1 file changed, 16 insertions(+), 9 deletions(-)
>>
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index 0d2ee19998aa..c2bcddc50990 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -946,10 +946,8 @@ static unsigned int __init copy_bios_e820(struct 
>> e820entry *map, unsigned int li
>>       return n;
>>   }
>> -static struct domain *__init create_dom0(const module_t *image,
>> -                                         unsigned long headroom,
>> -                                         module_t *initrd, const char 
>> *kextra,
>> -                                         const char *loader)
>> +static struct domain *__init create_dom0(const struct boot_info *bi,
>> +                                         const char *kextra)
>>   {
>>       static char __initdata cmdline[MAX_GUEST_CMDLINE];
>> @@ -964,9 +962,21 @@ static struct domain *__init create_dom0(const 
>> module_t *image,
>>               .misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED 
>> : 0,
>>           },
>>       };
>> +    int headroom, mod_idx = first_boot_module_index(bi, 
>> BOOTMOD_RAMDISK);
> 
> I think headroom should stay unsigned long, which matches struct 
> boot_module.

Yes, that was not intentional. Will preserve the size.

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

Thanks!

v/r,
dps

On 10.10.2024 01:02, Daniel P. Smith wrote:
> On 10/8/24 12:52, Jason Andryuk wrote:
>> On 2024-10-06 17:49, Daniel P. Smith wrote:
>>> This commit changes create_dom0 to no longer take the individual 
>>> components and
>>> take struct boot_info instead. Internally, it is changed to locate the 
>>> kernel
>>> and ramdisk details from struct boot_info.
>>>
>>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>>> ---
>>>   xen/arch/x86/setup.c | 25 ++++++++++++++++---------
>>>   1 file changed, 16 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>>> index 0d2ee19998aa..c2bcddc50990 100644
>>> --- a/xen/arch/x86/setup.c
>>> +++ b/xen/arch/x86/setup.c
>>> @@ -946,10 +946,8 @@ static unsigned int __init copy_bios_e820(struct 
>>> e820entry *map, unsigned int li
>>>       return n;
>>>   }
>>> -static struct domain *__init create_dom0(const module_t *image,
>>> -                                         unsigned long headroom,
>>> -                                         module_t *initrd, const char 
>>> *kextra,
>>> -                                         const char *loader)
>>> +static struct domain *__init create_dom0(const struct boot_info *bi,
>>> +                                         const char *kextra)
>>>   {
>>>       static char __initdata cmdline[MAX_GUEST_CMDLINE];
>>> @@ -964,9 +962,21 @@ static struct domain *__init create_dom0(const 
>>> module_t *image,
>>>               .misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED 
>>> : 0,
>>>           },
>>>       };
>>> +    int headroom, mod_idx = first_boot_module_index(bi, 
>>> BOOTMOD_RAMDISK);
>>
>> I think headroom should stay unsigned long, which matches struct 
>> boot_module.
> 
> Yes, that was not intentional. Will preserve the size.

And just to mention, with what was said on another patch mod_idx also
shouldn't be plain int.

Jan

On 10/10/24 04:03, Jan Beulich wrote:
> On 10.10.2024 01:02, Daniel P. Smith wrote:
>> On 10/8/24 12:52, Jason Andryuk wrote:
>>> On 2024-10-06 17:49, Daniel P. Smith wrote:
>>>> This commit changes create_dom0 to no longer take the individual
>>>> components and
>>>> take struct boot_info instead. Internally, it is changed to locate the
>>>> kernel
>>>> and ramdisk details from struct boot_info.
>>>>
>>>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>>>> ---
>>>>    xen/arch/x86/setup.c | 25 ++++++++++++++++---------
>>>>    1 file changed, 16 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>>>> index 0d2ee19998aa..c2bcddc50990 100644
>>>> --- a/xen/arch/x86/setup.c
>>>> +++ b/xen/arch/x86/setup.c
>>>> @@ -946,10 +946,8 @@ static unsigned int __init copy_bios_e820(struct
>>>> e820entry *map, unsigned int li
>>>>        return n;
>>>>    }
>>>> -static struct domain *__init create_dom0(const module_t *image,
>>>> -                                         unsigned long headroom,
>>>> -                                         module_t *initrd, const char
>>>> *kextra,
>>>> -                                         const char *loader)
>>>> +static struct domain *__init create_dom0(const struct boot_info *bi,
>>>> +                                         const char *kextra)
>>>>    {
>>>>        static char __initdata cmdline[MAX_GUEST_CMDLINE];
>>>> @@ -964,9 +962,21 @@ static struct domain *__init create_dom0(const
>>>> module_t *image,
>>>>                .misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED
>>>> : 0,
>>>>            },
>>>>        };
>>>> +    int headroom, mod_idx = first_boot_module_index(bi,
>>>> BOOTMOD_RAMDISK);
>>>
>>> I think headroom should stay unsigned long, which matches struct
>>> boot_module.
>>
>> Yes, that was not intentional. Will preserve the size.
> 
> And just to mention, with what was said on another patch mod_idx also
> shouldn't be plain int.

Correct, and I would like to think that the compiler would flag a type 
mismatch if I missed it.

v/r,
dps