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

[Luca Fancellu <luca.fancellu@xxxxxxx>]

Hi,

I’ve checked the common code and the arm part, I can confirm that the domid 0 
is never allocated even if the domain 0 is not present, here the only places 
where domain_create(…) is called using a variable value:

1) xen/arch/arm/domain_build.c
d = domain_create(++max_init_domid, &d_cfg, false);
Where max_init_domid has value 0 and it is defined in setup.c

2) xen/common/domctl.c
d = domain_create(dom, &op->u.createdomain, false);
For me seems that the dom variable won’t take the 0 value, if someone could 
give another feedback it would be great.

On every other part where domain_create(…) is used, it is called with a 
constant value different from 0.

For the hardware_domain being NULL and not handled in some situation, it seems 
that it’s not directly related to this patch, but I can handle it on a next 
serie, from a quick look it seems that many cases can be handled by checking if 
the domain is NULL in is_hardware_domain(…).

So, if the community agree, I can push a v2 patch with all the discussed things 
(moving dom0 creation code)

Cheers,

Luca

> On 12 Mar 2021, at 11:31, Julien Grall <julien@xxxxxxx> wrote:
> 
> Hi Luca,
> 
> On 12/03/2021 09:38, Luca Fancellu wrote:
>>>> +
>>>>  size_t __read_mostly dcache_line_bytes;
>>>>    /* C entry point for boot CPU */
>>>> @@ -804,7 +833,7 @@ void __init start_xen(unsigned long boot_phys_offset,
>>>>      int cpus, i;
>>>>      const char *cmdline;
>>>>      struct bootmodule *xen_bootmodule;
>>>> -    struct domain *dom0;
>>>> +    struct domain *dom0 = NULL;
>>>>      struct xen_domctl_createdomain dom0_cfg = {
>>>>          .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
>>>>          .max_evtchn_port = -1,
>>>> @@ -964,28 +993,33 @@ void __init start_xen(unsigned long boot_phys_offset,
>>>>      apply_alternatives_all();
>>>>      enable_errata_workarounds();
>>>>  -    /* Create initial domain 0. */
>>>> -    /* The vGIC for DOM0 is exactly emulating the hardware GIC */
>>>> -    dom0_cfg.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
>>>> -    /*
>>>> -     * Xen vGIC supports a maximum of 992 interrupt lines.
>>>> -     * 32 are substracted to cover local IRQs.
>>>> -     */
>>>> -    dom0_cfg.arch.nr_spis = min(gic_number_lines(), (unsigned int) 992) - 
>>>> 32;
>>>> -    if ( gic_number_lines() > 992 )
>>>> -        printk(XENLOG_WARNING "Maximum number of vGIC IRQs exceeded.\n");
>>>> -    dom0_cfg.arch.tee_type = tee_get_type();
>>>> -    dom0_cfg.max_vcpus = dom0_max_vcpus();
>>>> -
>>>> -    if ( iommu_enabled )
>>>> -        dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
>>>> -
>>>> -    dom0 = domain_create(0, &dom0_cfg, true);
>>>> -    if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
>>>> -        panic("Error creating domain 0\n");
>>>> -
>>>> -    if ( construct_dom0(dom0) != 0)
>>>> -        panic("Could not set up DOM0 guest OS\n");
>>>> +    if ( !is_dom0less_mode() )
>>>> +    {
>>>> +        /* Create initial domain 0. */
>>>> +        /* The vGIC for DOM0 is exactly emulating the hardware GIC */
>>>> +        dom0_cfg.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
>>>> +        /*
>>>> +        * Xen vGIC supports a maximum of 992 interrupt lines.
>>>> +        * 32 are substracted to cover local IRQs.
>>>> +        */
>>>> +        dom0_cfg.arch.nr_spis = min(gic_number_lines(), (unsigned int) 
>>>> 992) - 32;
>>>> +        if ( gic_number_lines() > 992 )
>>>> +            printk(XENLOG_WARNING "Maximum number of vGIC IRQs 
>>>> exceeded.\n");
>>>> +        dom0_cfg.arch.tee_type = tee_get_type();
>>>> +        dom0_cfg.max_vcpus = dom0_max_vcpus();
>>>> +
>>>> +        if ( iommu_enabled )
>>>> +            dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
>>>> +
>>>> +        dom0 = domain_create(0, &dom0_cfg, true);
>>>> +        if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
>>>> +            panic("Error creating domain 0\n");
>>>> +
>>>> +        if ( construct_dom0(dom0) != 0)
>>>> +            panic("Could not set up DOM0 guest OS\n");
>>>> +    }
>>> 
>>> It always felt a bit strange the dom0 creation is partly happening in 
>>> setup.c when for domU everythink will happen in domain_build.c.
>>> 
>>> Woule you be able to create a patch that will first move the code in a new 
>>> function (maybe create_dom0())? The function would return NULL in case of 
>>> an error or the domain.
>> Yes I will create a new patch with this change and I will put on top the v2 
>> dom0less patch
> 
> I think it would be better to put it first. This will avoid some churn if the 
> code movmement comes second (you would first indent and then move the code).
> 
> Cheers,
> 
> -- 
> Julien Grall