Re: [PATCH 1/3] x86: don't expose XSAVES capability to PV guests

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 19/01/15 15:27, Jan Beulich wrote:
> As done by the recent Linux commit b65d6e17fe ("kvm: x86: mask out
> XSAVES") for KVM, we should also mask out XSAVES from what PV guests
> get to see as long as we don't emulate accesses to MSR_IA32_XSS.
>
> Actually, go beyond that: Just like for leaf 7, switch from
> blacklisting to whitelisting, i.e. only allow XSAVEOPT and XSAVEC for
> the time being. And do these overrides consistently for both Dom0 and
> DomU-s.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -837,7 +837,7 @@ void pv_cpuid(struct cpu_user_regs *regs
>  
>          switch ( cpuid_leaf )
>          {
> -        case 0xd:
> +        case XSTATE_CPUID:
>          {
>              unsigned int _eax, _ebx, _ecx, _edx;
>              /* EBX value of main leaf 0 depends on enabled xsave features */
> @@ -855,7 +855,7 @@ void pv_cpuid(struct cpu_user_regs *regs
>                          b = _eax + _ebx;
>                  }
>              }
> -        break;
> +            goto xstate;
>          }
>          }
>          goto out;
> @@ -931,9 +931,19 @@ void pv_cpuid(struct cpu_user_regs *regs
>          a = c = d = 0;
>          break;
>  
> -    case 0x0000000d: /* XSAVE */
> +    case XSTATE_CPUID:
> +    xstate:
>          if ( !cpu_has_xsave )
>              goto unsupported;
> +        if ( regs->_ecx == 1 )
> +        {
> +            a &= XSTATE_FEATURE_XSAVEOPT |
> +                 XSTATE_FEATURE_XSAVEC |
> +                 (cpu_has_xgetbv1 ? XSTATE_FEATURE_XGETBV1 : 0) |
> +                 (cpu_has_xsaves ? XSTATE_FEATURE_XSAVES : 0);
> +            if ( !cpu_has_xsaves )
> +                b = c = d = 0;
> +        }
>          break;
>  
>      case 0x80000001:
> --- a/xen/arch/x86/xstate.c
> +++ b/xen/arch/x86/xstate.c
> @@ -14,7 +14,10 @@
>  #include <asm/xstate.h>
>  #include <asm/asm_defns.h>
>  
> -bool_t __read_mostly cpu_has_xsaveopt;
> +static bool_t __read_mostly cpu_has_xsaveopt;
> +static bool_t __read_mostly cpu_has_xsavec;
> +bool_t __read_mostly cpu_has_xgetbv1;
> +bool_t __read_mostly cpu_has_xsaves;
>  
>  /*
>   * Maximum size (in byte) of the XSAVE/XRSTOR save area required by all
> @@ -320,12 +323,22 @@ void xstate_init(bool_t bsp)
>          BUG_ON(xsave_cntxt_size != _xstate_ctxt_size(feature_mask));
>      }
>  
> -    /* Check XSAVEOPT feature. */
> +    /* Check extended XSAVE features. */
>      cpuid_count(XSTATE_CPUID, 1, &eax, &ebx, &ecx, &edx);
>      if ( bsp )
> +    {
>          cpu_has_xsaveopt = !!(eax & XSTATE_FEATURE_XSAVEOPT);
> +        cpu_has_xsavec = !!(eax & XSTATE_FEATURE_XSAVEC);
> +        /* XXX cpu_has_xgetbv1 = !!(eax & XSTATE_FEATURE_XGETBV1); */
> +        /* XXX cpu_has_xsaves = !!(eax & XSTATE_FEATURE_XSAVES); */
> +    }
>      else
> +    {
>          BUG_ON(!cpu_has_xsaveopt != !(eax & XSTATE_FEATURE_XSAVEOPT));
> +        BUG_ON(!cpu_has_xsavec != !(eax & XSTATE_FEATURE_XSAVEC));
> +        /* XXX BUG_ON(!cpu_has_xgetbv1 != !(eax & XSTATE_FEATURE_XGETBV1)); */
> +        /* XXX BUG_ON(!cpu_has_xsaves != !(eax & XSTATE_FEATURE_XSAVES)); */
> +    }
>  }
>  
>  static bool_t valid_xcr0(u64 xcr0)
> --- a/xen/include/asm-x86/xstate.h
> +++ b/xen/include/asm-x86/xstate.h
> @@ -16,6 +16,9 @@
>  
>  #define XSTATE_CPUID              0x0000000d
>  #define XSTATE_FEATURE_XSAVEOPT   (1 << 0)    /* sub-leaf 1, eax[bit 0] */
> +#define XSTATE_FEATURE_XSAVEC     (1 << 1)    /* sub-leaf 1, eax[bit 1] */
> +#define XSTATE_FEATURE_XGETBV1    (1 << 2)    /* sub-leaf 1, eax[bit 2] */
> +#define XSTATE_FEATURE_XSAVES     (1 << 3)    /* sub-leaf 1, eax[bit 3] */
>  
>  #define XCR_XFEATURE_ENABLED_MASK 0x00000000  /* index of XCR0 */
>  
> @@ -40,6 +43,7 @@
>  #define XSTATE_LAZY    (XSTATE_ALL & ~XSTATE_NONLAZY)
>  
>  extern u64 xfeature_mask;
> +extern bool_t cpu_has_xsaves, cpu_has_xgetbv1;
>  
>  /* extended state save area */
>  struct __packed __attribute__((aligned (64))) xsave_struct
>
>
>