Re: [PATCH V5 08/10] xen: Call arch_domain_create() before evtchn_init()

[Julien Grall <julien.grall@citrix.com>]

Hi Chen,

On 30/05/2015 12:07, Chen Baozi wrote:
> From: Chen Baozi <baozich@gmail.com>
>
> evtchn_init() will call domain_max_vcpus() to allocate poll_mask, which
> needs the max vCPU number returned by domain_max_vcpus(). On arm/arm64
> platform, this number is determined by the vGIC the guest is going to
> use, which won't be initialised until arch_domain_create() is finished.
>
> Signed-off-by: Chen Baozi <baozich@gmail.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Julien Grall <julien.grall@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Tim Deegan <tim@xen.org>
> ---
>   xen/common/domain.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 6803c4d..5b98f69 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -316,6 +316,10 @@ struct domain *domain_create(domid_t domid, unsigned int domcr_flags,
>       if ( domcr_flags & DOMCRF_dummy )
>           return d;
>
> +    if ( (err = arch_domain_create(d, domcr_flags, config)) != 0 )
> +        goto fail;
> +    init_status |= INIT_arch;
> +
>       if ( !is_idle_domain(d) )
>       {
>           if ( (err = xsm_domain_create(XSM_HOOK, d, ssidref)) != 0 )
> @@ -354,10 +358,6 @@ struct domain *domain_create(domid_t domid, unsigned int domcr_flags,
>               goto fail;
>       }
>
> -    if ( (err = arch_domain_create(d, domcr_flags, config)) != 0 )
> -        goto fail;
> -    init_status |= INIT_arch;
> -

NACK. Moving arch_domain_create means that we will allocate memory 
before checking the XSM policy. I don't think we want to execute an 
expensive function if the domain is not allowed to boot.

Furthermore, did you audit all the code to make sure that 
arch_domain_create (on both x86 and ARM) doesn't use the allocation done 
before (i.e evtch, grant table,...)?

Depending on this question, we may want to create a preinit domain 
creation with set the different value (such as the vGIC callback) 
without allocation memory.

Regards,

-- 
Julien Grall