[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Julien Grall <julien@xxxxxxx>
  • From: Luca Fancellu <luca.fancellu@xxxxxxx>
  • Date: Wed, 17 Mar 2021 17:04:57 +0000
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=sjoirN9hJ9lCOrBH9wWVcqrLpaZmOEcCPlOyfu+EaLM=; b=WxQpho7J4vqgv5TVBaYHEukDLZxsnsRte4E9m4+3oHagrKg07UhhgAvB0JhWVDQ34AJyyxWA+4xuviwjaY3PnNKmO7BzpnkoTTzrmBKsdtOaUEzX7IleIuKOTkcJx5X18oWBp7txNpyAprUEE8ncW2EMfGuEME/2npCLLioPbjAgZuUtFhguvIqGTXOas3dP5kF2E/+qR69Buvx11JwRMwRGJjNF+JetLdJj+nN2/cQHIxXdaFV76m7XBfxGzAwgqrIH2Vh5B4A1iyDDSbtVEhnORLntxiIvBETz/REfp7qGL5xvo/BOTZeqtIUwj98GeapL1fPHF9gmD3E0WnLm1w==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=VRUJjgLgj8kxG5NOYqBxYhQnr1dULhf5gOFTNzN9Ylm+hYzQhk0UEMm3HS5XhQLSpnnWSEkIsBGioOW9ggKvi2E3MXzVHVU7Aumrm24CjjAf7Xb81DMdR4IdpLSDHTuvk8YF5lYTFoIJVju48jpg9q+/PM9sph5qHZziGy3os/VCCjWhw0ZnAxy1G6JPAAPEQGRR86Ou4zdBucEMjxTmyonQT2irCG+1txptYSa3Sh5BHkBJP9dG6gJ6KLdAqPXEYngI7ATitEWRgM1kEP5+d12Y969zrM7cXnQbvKXjSbrZmCNYfr31y6H/14NMC6aU2CnOg7dyUOIRi5cNwXWEEg==
  • Authentication-results-original: xen.org; dkim=none (message not signed) header.d=none;xen.org; dmarc=none action=none header.from=arm.com;
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, wei.chen@xxxxxxx
  • Delivery-date: Wed, 17 Mar 2021 17:05:40 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: xen.org; dkim=none (message not signed) header.d=none;xen.org; dmarc=none action=none header.from=arm.com;

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




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.