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 <jul...@xen.org> 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
