Re: [PATCH v2 4/4] xen/arm: Prevent Dom0 to be loaded when using dom0less

[Julien Grall <julien@xxxxxxx>]

On 09/04/2021 10:56, Luca Fancellu wrote:
> > On 9 Apr 2021, at 10:12, Julien Grall <julien@xxxxxxx> wrote:
> >
> > Hi Luca,
> >
> > On 08/04/2021 10:48, Luca Fancellu wrote:
> > > This patch prevents the dom0 to be loaded skipping its
> > > building and going forward to build domUs when the dom0
> > > kernel is not found and at least one domU is present.
> > > Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> > > ---
> > >   docs/features/dom0less.pandoc |  7 +++---
> > >   xen/arch/arm/setup.c          | 42 ++++++++++++++++++++++++++++++++---
> > >   2 files changed, 43 insertions(+), 6 deletions(-)
> > > diff --git a/docs/features/dom0less.pandoc b/docs/features/dom0less.pandoc
> > > index d798596cdf..a5eb5bcda0 100644
> > > --- a/docs/features/dom0less.pandoc
> > > +++ b/docs/features/dom0less.pandoc
> > > @@ -16,9 +16,10 @@ Multiboot specification has been extended to allow for 
> > > multiple domains
> > >   to be passed to Xen. See docs/misc/arm/device-tree/booting.txt for more
> > >   information about the Multiboot specification and how to use it.
> > >   -Currently, a control domain ("dom0") is still required, but in the
> > > -future it will become unnecessary when all domains are created
> > > -directly from Xen. Instead of waiting for the control domain to be fully
> > > +Currently, a control domain ("dom0") is still required to manage the DomU
> > > +domains, but the system can start also without dom0 if the hypervisor
> > "hypervisor Device Tree" sounds a bit strange to me. I would either drop "hypervisor" or 
> > say "host Devicet Tree".
> >
> > > +Device Tree doesn't specify it and it declares one or more domUs.
> > AFAICT, the first "it" refer to dom0 but it is not clear what exact property 
> > will used to do the decision.
> >
> > Also you have two 'it' in a row that refers to two different entities. I would 
> > name it to avoid confusion.
> Yes I will rephrase it, what about:
>
> Currently, a control domain ("dom0") is still required to manage the DomU
> domains, but the system can start also without dom0 if the Device Tree
> doesn't specify the dom0 kernel and it declares one or more domUs.
Sounds good to me.

> > > +Instead of waiting for the control domain (when declared) to be fully
> > >   booted and the Xen tools to become available, domains created by Xen
> > >   this way are started right away in parallel. Hence, their boot time is
> > >   typically much shorter.
> > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > > index b405f58996..ecc4f0ae98 100644
> > > --- a/xen/arch/arm/setup.c
> > > +++ b/xen/arch/arm/setup.c
> > > @@ -793,6 +793,38 @@ static void __init setup_mm(void)
> > >   }
> > >   #endif
> > >   +static bool __init is_dom0less_mode(void)
> > > +{
> > > +    struct bootmodules *mods = &bootinfo.modules;
> > > +    struct bootmodule *mod;
> > > +    unsigned int i;
> > > +    bool dom0found = false;
> > > +    bool domUfound = false;
> > > +
> > > +    /* Look into the bootmodules */
> > > +    for ( i = 0 ; i < mods->nr_mods ; i++ )
> > > +    {
> > > +        mod = &mods->module[i];
> > > +        /* Find if dom0 and domU kernels are present */
> > > +        if ( mod->kind == BOOTMOD_KERNEL )
> > > +        {
> > > +            if ( mod->domU == false )
> > > +            {
> > > +                dom0found = true;
> > > +                break;
> > > +            }
> > NIT: You can directly return false here because if you have dom0 the it can't 
> > be dom0less.
> When I can I try to have just one exit point from a function, do you think here 
> it can cause
> issues?
I don't think so. I was only asking that because:
  - It is clearer to me that when you find dom0 then it must not a 
dom0less configuration.
  - It removes dom0found and reduce the code

But this is a non-important things to me (hence the NIT). If you prefer 
your version, then I am happy with it :).
Cheers,

--
Julien Grall