From mboxrd@z Thu Jan  1 00:00:00 1970
From: Julien Grall <julien.grall@citrix.com>
Subject: Re: [PATCH V5 08/10] xen: Call arch_domain_create()
 before evtchn_init()
Date: Sun, 31 May 2015 14:29:09 +0100
Message-ID: <556B0CA5.3020103@citrix.com>
References: <1432984051-10838-1-git-send-email-cbz@baozis.org>
	<1432984051-10838-9-git-send-email-cbz@baozis.org>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta3.messagelabs.com ([195.245.230.39])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <prvs=586dd3b0e=julien.grall@citrix.com>)
	id 1Yz3J2-000623-Mk
	for xen-devel@lists.xenproject.org; Sun, 31 May 2015 13:29:36 +0000
In-Reply-To: <1432984051-10838-9-git-send-email-cbz@baozis.org>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Chen Baozi <cbz@baozis.org>, xen-devel@lists.xenproject.org
Cc: Keir Fraser <keir@xen.org>, Ian Campbell <ian.campbell@citrix.com>, Andrew Cooper <andrew.cooper3@citrix.com>, Tim Deegan <tim@xen.org>, Julien Grall <julien.grall@citrix.com>, Jan Beulich <jbeulich@suse.com>, Chen Baozi <baozich@gmail.com>
List-Id: xen-devel@lists.xenproject.org

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