Re: [Xen-devel] [PATCH v3 1/2] x86/xstate: Use a guests CPUID policy, rather than allowing all features

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.07.18 at 11:37, <andrew.cooper3@xxxxxxxxxx> wrote:
> Before 4.9, the calls to {hvm,pv}_cpuid() are needed to because the
> information can't be read directly out of d->arch.cpuids[].  The restore
> boolean is needed because this array will be empty at the time it is
> accessed on the restore path.

Would you mind looking over the 4.8 backport below, where I think I've
got away without such a boolean?

Thanks, Jan

--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1177,7 +1177,7 @@ long arch_do_domctl(
             if ( _xcr0_accum )
             {
                 if ( evc->size >= PV_XSAVE_HDR_SIZE + XSTATE_AREA_MIN_SIZE )
-                    ret = validate_xstate(_xcr0, _xcr0_accum,
+                    ret = validate_xstate(d, _xcr0, _xcr0_accum,
                                           &_xsave_area->xsave_hdr);
             }
             else if ( !_xcr0 )
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1356,7 +1356,7 @@ static int hvm_load_cpu_xsave_states(str
     ctxt = (struct hvm_hw_cpu_xsave *)&h->data[h->cur];
     h->cur += desc->length;
 
-    err = validate_xstate(ctxt->xcr0, ctxt->xcr0_accum,
+    err = validate_xstate(d, ctxt->xcr0, ctxt->xcr0_accum,
                           (const void *)&ctxt->save_area.xsave_hdr);
     if ( err )
     {
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -652,12 +652,47 @@ static bool_t valid_xcr0(u64 xcr0)
     return !(xcr0 & XSTATE_BNDREGS) == !(xcr0 & XSTATE_BNDCSR);
 }
 
-int validate_xstate(u64 xcr0, u64 xcr0_accum, const struct xsave_hdr *hdr)
+static uint64_t guest_xcr0_max(const struct domain *d)
 {
+    if ( has_hvm_container_domain(d) )
+    {
+        uint32_t eax, ecx = 0, edx;
+
+        hvm_cpuid(XSTATE_CPUID, &eax, NULL, &ecx, &edx);
+
+        return ((uint64_t)edx << 32) | eax;
+    }
+    else
+    {
+        struct cpu_user_regs regs = { };
+
+        regs._eax = XSTATE_CPUID;
+        regs._ecx = 0;
+        pv_cpuid(&regs);
+
+        return (regs.rdx << 32) | regs._eax;
+    }
+}
+
+int validate_xstate(const struct domain *d, uint64_t xcr0, uint64_t xcr0_accum,
+                    const struct xsave_hdr *hdr)
+{
+    uint64_t xcr0_max;
     unsigned int i;
 
+    if ( d == current->domain )
+        xcr0_max = guest_xcr0_max(d);
+    else
+    {
+        xcr0_max = xfeature_mask;
+        if ( !has_hvm_container_domain(d) )
+            xcr0_max &= ~(XSTATE_BNDREGS | XSTATE_BNDCSR |
+                          XSTATE_PKRU | XSTATE_LWP);
+    }
+
     if ( (hdr->xstate_bv & ~xcr0_accum) ||
          (xcr0 & ~xcr0_accum) ||
+         (xcr0_accum & ~xcr0_max) ||
          !valid_xcr0(xcr0) ||
          !valid_xcr0(xcr0_accum) )
         return -EINVAL;
@@ -676,18 +711,16 @@ int validate_xstate(u64 xcr0, u64 xcr0_a
 int handle_xsetbv(u32 index, u64 new_bv)
 {
     struct vcpu *curr = current;
+    uint64_t xcr0_max = guest_xcr0_max(curr->domain);
     u64 mask;
 
     if ( index != XCR_XFEATURE_ENABLED_MASK )
         return -EOPNOTSUPP;
 
-    if ( (new_bv & ~xfeature_mask) || !valid_xcr0(new_bv) )
+    if ( (new_bv & ~xcr0_max) ||
+         (new_bv & ~xfeature_mask) || !valid_xcr0(new_bv) )
         return -EINVAL;
 
-    /* XCR0.PKRU is disabled on PV mode. */
-    if ( is_pv_vcpu(curr) && (new_bv & XSTATE_PKRU) )
-        return -EOPNOTSUPP;
-
     if ( !set_xcr0(new_bv) )
         return -EFAULT;
 
--- a/xen/include/asm-x86/xstate.h
+++ b/xen/include/asm-x86/xstate.h
@@ -107,8 +107,9 @@ uint64_t get_msr_xss(void);
 void xsave(struct vcpu *v, uint64_t mask);
 void xrstor(struct vcpu *v, uint64_t mask);
 bool_t xsave_enabled(const struct vcpu *v);
-int __must_check validate_xstate(u64 xcr0, u64 xcr0_accum,
-                                 const struct xsave_hdr *);
+int __must_check validate_xstate(const struct domain *d,
+                                 uint64_t xcr0, uint64_t xcr0_accum,
+                                 const struct xsave_hdr *hdr);
 int __must_check handle_xsetbv(u32 index, u64 new_bv);
 void expand_xsave_states(struct vcpu *v, void *dest, unsigned int size);
 void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size);



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