Re: [PATCH v9 2/3] xen/domain: adjust domain ID allocation for Arm

[Julien Grall <julien@xxxxxxx>]

Hi Denis,

On 05/06/2025 23:05, Julien Grall wrote:
> Hi Denis,
>
> On 28/05/2025 23:50, dmkhn@xxxxxxxxx wrote:
> > From: Denis Mukhin <dmkhn@xxxxxxxxx>
> >
> > From: Denis Mukhin <dmukhin@xxxxxxxx>
> >
> > Remove the hardcoded domain ID 0 allocation for hardware domain and 
> > replace it
> > with a call to get_initial_domain_id() (returns the value of 
> > hardware_domid on
> > Arm).
> I am not entirely why this is done. Are you intending to pass a 
> different domain ID? If so...
> > Update domid_alloc(DOMID_INVALID) case to ensure that 
> > get_initial_domain_id()
> > ID is skipped during domain ID allocation to cover domU case in dom0less
> > configuration. That also fixes a potential issue with re-using ID#0 
> > for domUs
> > when get_initial_domain_id() returns non-zero.
> >
> > Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>
> > ---
> > Changes since v8:
> > - rebased
> > ---
> >   xen/arch/arm/domain_build.c             | 4 ++--
> >   xen/common/device-tree/dom0less-build.c | 9 +++------
> >   xen/common/domain.c                     | 4 ++--
> >   3 files changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index e9d563c269..0ad80b020a 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -2035,9 +2035,9 @@ void __init create_dom0(void)
> ... naming like create_dom0() probably wants to be renamed.
>
> That said, I am not convinced a domain other than 0 should have full 
> privilege by default. So I would argue it should stay as ...
> >       if ( !llc_coloring_enabled )
> >           flags |= CDF_directmap;
> > -    domid = domid_alloc(0);
> > +    domid = domid_alloc(get_initial_domain_id());
> ... 0.
Looking at the implementation of get_initial_domain_id(), I noticed the 
behavior was changed for x86 by [1].
Before, get_initial_domain_id() was returning 0 except for the PV shim.
But now, it would could return the domain ID specified on the command 
line (via hardware_dom).
From my understanding, the goal of the command line was to create the 
hardware domain *after* boot. So initially we create dom0 and then 
initialize the hardware domain. With the patch below, this has changed.
However, from the commit message, I don't understand why. It seems like 
we broke late hwdom?
For instance, late_hwdom_init() has the following assert:

    dom0 = rcu_lock_domain_by_id(0);
    ASSERT(dom0 != NULL);

Jan, I saw you were involved in the review of the series. Any idea why 
this was changed?
Cheers,

[1] https://lore.kernel.org/all/20250306075819.154361-1-dmkhn@xxxxxxxxx/

--
Julien Grall