Re: [PATCH v5 02/10] xen: Refactor kernel_info to have a header like boot_domain

[Alejandro Vallejo <alejandro.garciavallejo@xxxxxxx>]

On Wed Jul 2, 2025 at 2:56 PM CEST, Jan Beulich wrote:
> On 01.07.2025 12:56, Alejandro Vallejo wrote:
>> --- a/xen/include/xen/bootfdt.h
>> +++ b/xen/include/xen/bootfdt.h
>> @@ -100,6 +100,16 @@ struct shared_meminfo {
>>      struct shmem_membank_extra extra[NR_SHMEM_BANKS];
>>  };
>>  
>> +
>
> Nit: No double blank lines please.
>
>> +struct boot_domain {
>> +    struct domain *d;
>> +
>> +    struct boot_module *kernel;
>> +    struct boot_module *initrd;
>> +
>> +    const char* cmdline;
>
> Nit: * and blank want to change places.
>
>> --- a/xen/include/xen/fdt-kernel.h
>> +++ b/xen/include/xen/fdt-kernel.h
>> @@ -16,7 +16,7 @@
>>  #endif
>>  
>>  struct kernel_info {
>> -    struct domain *d;
>> +    struct boot_domain hdr;
>>  
>>      void *fdt; /* flat device tree */
>>      paddr_t unassigned_mem; /* RAM not (yet) assigned to a bank */
>> @@ -34,8 +34,7 @@ struct kernel_info {
>>      paddr_t gnttab_size;
>>  
>>      /* boot blob load addresses */
>> -    const struct boot_module *kernel, *initrd, *dtb;
>
> Where did this "const" go?

x86 mutates the boot module to set the released flag, the headroom, etc.

>
>> -    const char* cmdline;
>> +    const struct boot_module *dtb;
>
> This one only retains the intended effect here.

Because x86 doesn't see or use the containing struct.

>
>>      paddr_t dtb_paddr;
>>      paddr_t initrd_paddr;
>
> "hdr" is an odd name here. That struct has no need to live at the beginning,
> afaict. How about "gen" for "generic" or "bd" for "boot_domain"?
>
> Jan

Sure. I'm fine with `bd`. Will adjust.

Cheers,
Alejandro