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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 10/04/17 14:27, Wei Liu 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>

Two observations...

> ---
>  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
> @@ -663,7 +656,7 @@ int arch_domain_create(struct domain *d, unsigned int 
> domcr_flags,
>  
>      if ( is_hvm_domain(d) )
>      {
> -        if ( (rc = hvm_domain_initialise(d)) != 0 )
> +        if ( (rc = hvm_domain_initialise(d, domcr_flags)) != 0 )

If we are pushing these values down (which I agree is a good idea),
please can we push xen_arch_domainconfig down as well.

>              goto fail;
>      }
>      else
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index f50d15ff50..7fc49bb03d 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -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);

We really should fail with -EOPNOTSUPP.

The toolstack already knows whether HAP is available, and nothing good
can come from Xen falling silently back to shadow behind the toolstacks
back.

Probably worth being in a solo patch though.

~Andrew

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