Re: [PATCH v5 03/10] x86: Replace arch-specific boot_domain with the common one

[Alejandro Vallejo <agarciav@xxxxxxx>]

On Wed Jul 2, 2025 at 3:15 PM CEST, Jan Beulich wrote:
> On 01.07.2025 12:56, Alejandro Vallejo wrote:
>> --- a/xen/arch/x86/include/asm/bootfdt.h
>> +++ b/xen/arch/x86/include/asm/bootfdt.h
>> @@ -3,6 +3,12 @@
>>  #define X86_BOOTFDT_H
>>  
>>  #include <xen/types.h>
>> +#include <public/xen.h>
>> +
>> +struct arch_boot_domain
>> +{
>> +    domid_t domid;
>> +};
>>  
>>  struct arch_boot_module
>>  {
>>[...]
>> @@ -1048,11 +1050,11 @@ static struct domain *__init create_dom0(struct 
>> boot_info *bi)
>>          dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
>>  
>>      /* Create initial domain.  Not d0 for pvshim. */
>> -    bd->domid = get_initial_domain_id();
>> -    d = domain_create(bd->domid, &dom0_cfg,
>> +    bd->arch.domid = get_initial_domain_id();
>> +    d = domain_create(bd->arch.domid, &dom0_cfg,
>>                        pv_shim ? 0 : CDF_privileged | CDF_hardware);
>>      if ( IS_ERR(d) )
>> -        panic("Error creating d%u: %ld\n", bd->domid, PTR_ERR(d));
>> +        panic("Error creating d%u: %ld\n", bd->arch.domid, PTR_ERR(d));
>
> This being the only place where the (now) arch-specific field is used, why
> does it exist? A local variable would do? And if it's needed for
> (supposedly arch-agnostic) hyperlaunch, then it probably shouldn't be
> arch-specific? Daniel, Jason?
>
> Jan

As for the arch-agnostic side of things, arm needs some extra work to be
able to do it safely. dom0less currently constructs domains immediately after
parsing them, which is problematic for cases where some domains have the prop
and others don't. The domid allocation strategy may preclude further otherwise
good domains from being created just because their domid was stolen by a domain
that didn't actually care about which domid it got.

It'll eventually want to leave the arch-specific area, but I don't want to do
that work now.

Cheers,
Alejandro