Re: [Xen-devel] [PATCH for-next 4/8] x86/domain: push some code down to hvm_domain_initialise

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.04.17 at 15:27, <wei.liu2@xxxxxxxxxx> wrote:
> We want to have a single entry point to initialise hvm guest.  The
> timing to set hap bit and create per domain mapping is deferred, but
> that's not a problem.
> 
> No functional change.
> 
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
>  xen/arch/x86/domain.c         | 11 ++---------
>  xen/arch/x86/hvm/hvm.c        | 25 +++++++++++++++++--------
>  xen/include/asm-x86/hvm/hvm.h |  2 +-
>  3 files changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index ddebff6187..af060d8239 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -587,14 +587,7 @@ int arch_domain_create(struct domain *d, unsigned int 
> domcr_flags,
>          d->arch.emulation_flags = emflags;
>      }
>  
> -    if ( is_hvm_domain(d) )
> -    {
> -        d->arch.hvm_domain.hap_enabled =
> -            hvm_funcs.hap_supported && (domcr_flags & DOMCRF_hap);
> -
> -        rc = create_perdomain_mapping(d, PERDOMAIN_VIRT_START, 0, NULL, 
> NULL);

I'm not really certain it is a good idea to move this last one, as I can't
immediately spot uses of it from the hvm/ subtree.

> -    }
> -    else if ( is_idle_domain(d) )
> +    if ( is_idle_domain(d) )
>          rc = 0;
>      else

This now needs to be "else if ( !is_hvm_domain(d) )" afaict.

> @@ -615,10 +615,17 @@ int hvm_domain_initialise(struct domain *d)
>  
>      hvm_init_cacheattr_region_list(d);
>  
> -    rc = paging_enable(d, PG_refcounts|PG_translate|PG_external);
> +    d->arch.hvm_domain.hap_enabled =
> +        hvm_funcs.hap_supported && (domcr_flags & DOMCRF_hap);
> +
> +    rc = create_perdomain_mapping(d, PERDOMAIN_VIRT_START, 0, NULL, NULL);
>      if ( rc != 0 )
>          goto fail0;
>  
> +    rc = paging_enable(d, PG_refcounts|PG_translate|PG_external);
> +    if ( rc != 0 )
> +        goto fail1;

Is the order of these required to be that way? The various failN
labels would seem to not require re-numbering if you swapped
the last two actions.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel