[Xen-devel] [PATCH v2] x86/xsave: Remove xfeat_mask checking from validate_xstate()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

validate_xsave() is called codepaths which load new vcpu xsave state from
XEN_DOMCTL_{setvcpuextstate,sethvmcontext}, usually as part of migration.  In
both cases, this is the xfeature_mask of the saving Xen rather than the
restoring Xen.

Given that the xsave state itself is checked for consistency and validity on
the current cpu, checking whether it was valid for the cpu before migration is
not interesting (or indeed relevant, as the error can't be distinguished from
the other validity checking).

This change removes the need to pass the saving Xen's xfeature_mask,
simplifying the toolstack code and migration stream format in this area.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CC: Keir Fraser <keir@xxxxxxx>
CC: Jan Beulich <JBeulich@xxxxxxxx>

--
v2:

 * Reword commit message.  This change only applies to the toolstack paths for
   loading xsave state, and does not affect the regular guest/xen xsave
   interaction.
---
 xen/arch/x86/domctl.c                  |    3 +--
 xen/arch/x86/hvm/hvm.c                 |    3 +--
 xen/arch/x86/xstate.c                  |    5 ++---
 xen/include/asm-x86/xstate.h           |    3 +--
 xen/include/public/arch-x86/hvm/save.h |    2 +-
 xen/include/public/domctl.h            |    2 +-
 6 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index f8b0a79..1285dd0 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1247,8 +1247,7 @@ long arch_do_domctl(
             {
                 if ( evc->size >= 2 * sizeof(uint64_t) + XSTATE_AREA_MIN_SIZE )
                     ret = validate_xstate(_xcr0, _xcr0_accum,
-                                          _xsave_area->xsave_hdr.xstate_bv,
-                                          evc->xfeature_mask);
+                                          _xsave_area->xsave_hdr.xstate_bv);
             }
             else if ( !_xcr0 )
                 ret = 0;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 59836a1..163e62a 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1992,8 +1992,7 @@ static int hvm_load_cpu_xsave_states(struct domain *d, 
hvm_domain_context_t *h)
     h->cur += desc->length;
 
     err = validate_xstate(ctxt->xcr0, ctxt->xcr0_accum,
-                          ctxt->save_area.xsave_hdr.xstate_bv,
-                          ctxt->xfeature_mask);
+                          ctxt->save_area.xsave_hdr.xstate_bv);
     if ( err )
     {
         printk(XENLOG_G_WARNING
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 7d9b92b..e202344 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -346,10 +346,9 @@ static bool_t valid_xcr0(u64 xcr0)
     return !(xcr0 & XSTATE_BNDREGS) == !(xcr0 & XSTATE_BNDCSR);
 }
 
-int validate_xstate(u64 xcr0, u64 xcr0_accum, u64 xstate_bv, u64 xfeat_mask)
+int validate_xstate(u64 xcr0, u64 xcr0_accum, u64 xstate_bv)
 {
-    if ( (xcr0_accum & ~xfeat_mask) ||
-         (xstate_bv & ~xcr0_accum) ||
+    if ( (xstate_bv & ~xcr0_accum) ||
          (xcr0 & ~xcr0_accum) ||
          !valid_xcr0(xcr0) ||
          !valid_xcr0(xcr0_accum) )
diff --git a/xen/include/asm-x86/xstate.h b/xen/include/asm-x86/xstate.h
index 1a92ac3..8d21349 100644
--- a/xen/include/asm-x86/xstate.h
+++ b/xen/include/asm-x86/xstate.h
@@ -81,8 +81,7 @@ uint64_t get_xcr0(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, u64 xstate_bv,
-                                 u64 xfeat_mask);
+int __must_check validate_xstate(u64 xcr0, u64 xcr0_accum, u64 xstate_bv);
 int __must_check handle_xsetbv(u32 index, u64 new_bv);
 
 /* extended state init and cleanup functions */
diff --git a/xen/include/public/arch-x86/hvm/save.h 
b/xen/include/public/arch-x86/hvm/save.h
index 5a13795..16d85a3 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -544,7 +544,7 @@ DECLARE_HVM_SAVE_TYPE(MTRR, 14, struct hvm_hw_mtrr);
  */
 
 struct hvm_hw_cpu_xsave {
-    uint64_t xfeature_mask;
+    uint64_t xfeature_mask;        /* Ignored */
     uint64_t xcr0;                 /* Updated by XSETBV */
     uint64_t xcr0_accum;           /* Updated by XSETBV */
     struct {
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 565fa4c..385b053 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -839,7 +839,7 @@ struct xen_domctl_vcpuextstate {
     /* IN: VCPU that this call applies to. */
     uint32_t         vcpu;
     /*
-     * SET: xfeature support mask of struct (IN)
+     * SET: Ignored.
      * GET: xfeature support mask of struct (IN/OUT)
      * xfeature mask is served as identifications of the saving format
      * so that compatible CPUs can have a check on format to decide
-- 
1.7.10.4


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