Re: [Xen-devel] [PATCH 2/2] x86/svm: Add virtual GIF support

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/11/17 15:32, Brian Woods wrote:
> This patch detects and enables Virtual GIF if available.  This allows
> a nested hypervisor to perform STGIs and CLGIs without having to be
> intercepted by host hypervisor.
>
> Signed-off-by: Brian Woods <brian.woods@xxxxxxx>
> ---
>  xen/arch/x86/hvm/svm/nestedsvm.c | 7 ++++++-
>  xen/arch/x86/hvm/svm/svm.c       | 1 +
>  xen/arch/x86/hvm/svm/vmcb.c      | 9 +++++++++
>  3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c 
> b/xen/arch/x86/hvm/svm/nestedsvm.c
> index 1de896e456..de5d022815 100644
> --- a/xen/arch/x86/hvm/svm/nestedsvm.c
> +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
> @@ -1597,8 +1597,13 @@ bool_t
>  nestedsvm_gif_isset(struct vcpu *v)

bool_t used to be char, which is why this function had !! semantics.

Since 4.7, bool_t switched to being a C99 bool, and we've been slowly
updating the codebase to match.

Please would you update the prototype to use bool, and drop the
now-unnecessary !!'s

>  {
>      struct nestedsvm *svm = &vcpu_nestedsvm(v);
> +    struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
>  
> -    return (!!svm->ns_gif);
> +    /* get the vmcb gif value if using vgif */
> +    if ( cpu_has_svm_vgif )

vmcb->_vintr.fields.vgif_enabled ?

While the two are synonymous at the moment, this would be more
accurate.  Also, in some copious free time, I'd like to add the ability
to control the L0 hypervisor features on a per-guest basis, so you can
sensibly test a domain with and without (in this case) VGIF support on
the same host without hacking at the hypervisor.

~Andrew

> +        return (!!vmcb->_vintr.fields.vgif);
> +    else
> +        return (!!svm->ns_gif);
>  }
>  
>  void svm_vmexit_do_stgi(struct cpu_user_regs *regs, struct vcpu *v)
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index b9cf423fd9..6b7a462fcb 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1669,6 +1669,7 @@ const struct hvm_function_table * __init start_svm(void)
>      P(cpu_has_svm_nrips, "Next-RIP Saved on #VMEXIT");
>      P(cpu_has_svm_cleanbits, "VMCB Clean Bits");
>      P(cpu_has_svm_decode, "DecodeAssists");
> +    P(cpu_has_svm_vgif, "Virtual GIF");
>      P(cpu_has_pause_filter, "Pause-Intercept Filter");
>      P(cpu_has_tsc_ratio, "TSC Rate MSR");
>  #undef P
> diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
> index 997e7597e0..5a714be8f2 100644
> --- a/xen/arch/x86/hvm/svm/vmcb.c
> +++ b/xen/arch/x86/hvm/svm/vmcb.c
> @@ -206,6 +206,15 @@ static int construct_vmcb(struct vcpu *v)
>          vmcb->_exception_intercepts |= (1U << TRAP_page_fault);
>      }
>  
> +    /* if available, enable and configure virtual gif */
> +    if ( cpu_has_svm_vgif )
> +    {
> +        vmcb->_vintr.fields.vgif = 1;
> +        vmcb->_vintr.fields.vgif_enable = 1;
> +        vmcb->_general2_intercepts &= ~GENERAL2_INTERCEPT_STGI;
> +        vmcb->_general2_intercepts &= ~GENERAL2_INTERCEPT_CLGI;
> +    }
> +
>      if ( cpu_has_pause_filter )
>      {
>          vmcb->_pause_filter_count = SVM_PAUSEFILTER_INIT;


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