Re: [PATCH v9 1/6] x86/boot: convert domain construction to use boot info

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 15/11/2024 4:33 pm, Jason Andryuk wrote:
> On 2024-11-15 08:11, Daniel P. Smith wrote:
>> diff --git a/xen/arch/x86/hvm/dom0_build.c
>> b/xen/arch/x86/hvm/dom0_build.c
>> index 3dd913bdb029..d1bdf1b14601 100644
>> --- a/xen/arch/x86/hvm/dom0_build.c
>> +++ b/xen/arch/x86/hvm/dom0_build.c
>> @@ -1300,16 +1301,26 @@ static void __hwdom_init
>> pvh_setup_mmcfg(struct domain *d)
>>       }
>>   }
>>   -int __init dom0_construct_pvh(struct domain *d, const module_t
>> *image,
>> -                              unsigned long image_headroom,
>> -                              module_t *initrd,
>> -                              const char *cmdline)
>> +int __init dom0_construct_pvh(struct boot_info *bi, struct domain *d)
>>   {
>>       paddr_t entry, start_info;
>> +    struct boot_module *image;
>> +    struct boot_module *initrd = NULL;
>> +    unsigned int idx;
>>       int rc;
>>         printk(XENLOG_INFO "*** Building a PVH Dom%d ***\n",
>> d->domain_id);
>>   +    idx = first_boot_module_index(bi, BOOTMOD_KERNEL);
>> +    if ( idx >= bi->nr_modules )
>
> What do you think about introducing a new define:
>
>     #define BOOTMOD_NOT_FOUND (MAX_NR_BOOTMODS + 1)
>
> For first_boot_module_index() to return.  And then:
>
>     if ( idx == BOOTMOD_NOT_FOUND )
>
> ?

Care would need to be taken vs BOOTMOD_XEN, which could have the same
numeric value in a big HL configuration.

From a "reading the code" point of view, a range check against any
invalid value is better seeing as the next thing we do is index an
array, so I'm marginally on the side of "keep it as it is".

This particular logic can't trip because of earlier checks in
__start_xen(), and gets rewritten in patch 4 in the conversion to
boot_domains, so I'm also not overly fussed at extra polish on this
specific piece of logic.

~Andrew