Re: [Xen-devel] [PATCH 2/6] x86: save GUEST_BNDCFGS on context switch...

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Paul Durrant [mailto:paul.durrant@xxxxxxxxxx]
> Sent: Monday, January 7, 2019 8:03 PM
> 
> ...to avoid the need for a VMCS reload when the value of
> MSR_IA32_BNDCFGS is
> read by the tool-stack.

the frequency of context switch is much higher than the
one of reading by tool-stack (at least in general case), then
is it right thing to add a vmread for every context switch
to save a VMCS reload in tool stack path?

> 
> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> ---
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> Cc: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
> Cc: Jun Nakajima <jun.nakajima@xxxxxxxxx>
> Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
> ---
>  xen/arch/x86/hvm/hvm.c     | 18 +++++++++++++++---
>  xen/arch/x86/hvm/vmx/vmx.c |  3 +++
>  xen/include/asm-x86/msr.h  |  5 +++++
>  3 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 5fd5478b7d..b86aed7c24 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -311,6 +311,7 @@ int hvm_set_guest_pat(struct vcpu *v, u64
> guest_pat)
>  bool hvm_set_guest_bndcfgs(struct vcpu *v, uint64_t val)
>  {
>      const struct cpuid_policy *cp = v->domain->arch.cpuid;
> +    struct vcpu_msrs *msrs = v->arch.msrs;
> 
>      if ( !cp->feat.mpx )
>          return false;
> @@ -347,7 +348,8 @@ bool hvm_set_guest_bndcfgs(struct vcpu *v,
> uint64_t val)
>              /* nothing, best effort only */;
>      }
> 
> -    hvm_funcs.set_guest_bndcfgs(v, val);
> +    msrs->bndcfgs.raw = val;
> +    hvm_funcs.set_guest_bndcfgs(v, msrs->bndcfgs.raw);
> 
>      return true;
>  }
> @@ -355,12 +357,22 @@ bool hvm_set_guest_bndcfgs(struct vcpu *v,
> uint64_t val)
>  bool hvm_get_guest_bndcfgs(struct vcpu *v, uint64_t *val)
>  {
>      const struct cpuid_policy *cp = v->domain->arch.cpuid;
> +    struct vcpu_msrs *msrs = v->arch.msrs;
> 
>      if ( !cp->feat.mpx )
>          return false;
> 
> -    ASSERT(hvm_funcs.get_guest_bndcfgs);
> -    *val = hvm_funcs.get_guest_bndcfgs(v);
> +    /*
> +     * The value only need be read in current context as a context
> +     * switch will save the value into msrs->bndcfgs.
> +     */
> +    if ( v == current )
> +    {
> +        ASSERT(hvm_funcs.get_guest_bndcfgs);
> +        msrs->bndcfgs.raw = hvm_funcs.get_guest_bndcfgs(v);
> +    }
> +
> +    *val = msrs->bndcfgs.raw;
> 
>      return true;
>  }
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 4bfabe8d0e..7dba92da45 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -497,6 +497,9 @@ static void vmx_restore_host_msrs(void)
> 
>  static void vmx_save_guest_msrs(struct vcpu *v)
>  {
> +    if ( cpu_has_mpx && cpu_has_vmx_mpx )
> +        __vmread(GUEST_BNDCFGS, &v->arch.msrs->bndcfgs.raw);
> +
>      /*
>       * We cannot cache SHADOW_GS_BASE while the VCPU runs, as it can
>       * be updated at any time via SWAPGS, which we cannot trap.
> diff --git a/xen/include/asm-x86/msr.h b/xen/include/asm-x86/msr.h
> index ad8688a61f..c69ce56963 100644
> --- a/xen/include/asm-x86/msr.h
> +++ b/xen/include/asm-x86/msr.h
> @@ -305,6 +305,11 @@ struct vcpu_msrs
>       * values here may be stale in current context.
>       */
>      uint32_t dr_mask[4];
> +
> +    /* 0x00000d90 - MSR_IA32_BNDCFGS */
> +    struct {
> +        uint64_t raw;
> +    } bndcfgs;
>  };
> 
>  void init_guest_msr_policy(void);
> --
> 2.20.1

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel