[PATCH v2 00/13] x86: Address Space Isolation FPU preparations

[PATCH v2 00/13] x86: Address Space Isolation FPU preparations

[Alejandro Vallejo]

See original cover letter in v1

v1: https://lore.kernel.org/xen-devel/20241028154932.6797-1-alejandro.vallejo@cloud.com/
v1->v2:
  * Turned v1/patch1 into an assert removal
  * Dropped v1/patch11: "x86/mpx: Adjust read_bndcfgu() to clean after itself"
  * Other minor changes out of feedback. Explained in each patch.

Alejandro Vallejo (13):
  x86/xstate: Remove stale assertions in fpu_x{rstor,save}()
  x86/xstate: Create map/unmap primitives for xsave areas
  x86/hvm: Map/unmap xsave area in hvm_save_cpu_ctxt()
  x86/fpu: Map/umap xsave area in vcpu_{reset,setup}_fpu()
  x86/xstate: Map/unmap xsave area in xstate_set_init() and
    handle_setbv()
  x86/hvm: Map/unmap xsave area in hvmemul_{get,put}_fpu()
  x86/domctl: Map/unmap xsave area in arch_get_info_guest()
  x86/xstate: Map/unmap xsave area in {compress,expand}_xsave_states()
  x86/emulator: Refactor FXSAVE_AREA to use wrappers
  x86/mpx: Map/unmap xsave area in in read_bndcfgu()
  x86/fpu: Pass explicit xsave areas to fpu_(f)xsave()
  x86/fpu: Pass explicit xsave areas to fpu_(f)xrstor()
  x86/xstate: Make xstate_all() and vcpu_xsave_mask() take explicit
    xstate

 xen/arch/x86/domctl.c             |  9 +++--
 xen/arch/x86/hvm/emulate.c        | 12 +++++-
 xen/arch/x86/hvm/hvm.c            |  8 ++--
 xen/arch/x86/i387.c               | 65 +++++++++++++++++++------------
 xen/arch/x86/include/asm/xstate.h | 51 ++++++++++++++++++++++--
 xen/arch/x86/x86_emulate/blk.c    | 11 +++++-
 xen/arch/x86/xstate.c             | 47 +++++++++++++++-------
 7 files changed, 150 insertions(+), 53 deletions(-)

-- 
2.47.0

[PATCH v2 01/13] x86/xstate: Remove stale assertions in fpu_x{rstor,save}()

[Alejandro Vallejo]

After edb48e76458b("x86/fpu: Combine fpu_ctxt and xsave_area in arch_vcpu"),
v->arch.xsave_area is always present and we can just remove these asserts.

Fixes: edb48e76458b("x86/fpu: Combine fpu_ctxt and xsave_area in arch_vcpu")
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
v2:
  * Remove asserts rather than refactor them.
  * Trimmed and adjusted commit message
---
 xen/arch/x86/i387.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/xen/arch/x86/i387.c b/xen/arch/x86/i387.c
index 83f9b2502bff..3add0025e495 100644
--- a/xen/arch/x86/i387.c
+++ b/xen/arch/x86/i387.c
@@ -24,7 +24,6 @@ static inline void fpu_xrstor(struct vcpu *v, uint64_t mask)
 {
     bool ok;
 
-    ASSERT(v->arch.xsave_area);
     /*
      * XCR0 normally represents what guest OS set. In case of Xen itself,
      * we set the accumulated feature mask before doing save/restore.
@@ -136,7 +135,6 @@ static inline void fpu_xsave(struct vcpu *v)
     uint64_t mask = vcpu_xsave_mask(v);
 
     ASSERT(mask);
-    ASSERT(v->arch.xsave_area);
     /*
      * XCR0 normally represents what guest OS set. In case of Xen itself,
      * we set the accumulated feature mask before doing save/restore.
-- 
2.47.0

[PATCH v2 02/13] x86/xstate: Create map/unmap primitives for xsave areas

[Alejandro Vallejo]

Add infrastructure to simplify ASI handling. With ASI in the picture
we'll have several different means of accessing the XSAVE area of a
given vCPU, depending on whether a domain is covered by ASI or not and
whether the vCPU is question is scheduled on the current pCPU or not.

Having these complexities exposed at the call sites becomes unwieldy
very fast. These wrappers are intended to be used in a similar way to
map_domain_page() and unmap_domain_page(); The map operation will
dispatch the appropriate pointer for each case in a future patch, while
unmap will remain a no-op where no unmap is required (e.g: when there's
no ASI) and remove the transient maping if one was required.

Follow-up patches replace all uses of raw v->arch.xsave_area by this
mechanism in preparation to add the beforementioned dispatch logic to be
added at a later time.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
v2:
  * Comment macros more heavily to show their performance characteristics.
  * Addressed various nits in the macro comments.
  * Macro names to uppercase.
---
 xen/arch/x86/include/asm/xstate.h | 42 +++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/xen/arch/x86/include/asm/xstate.h b/xen/arch/x86/include/asm/xstate.h
index 07017cc4edfd..6b0daff0aeec 100644
--- a/xen/arch/x86/include/asm/xstate.h
+++ b/xen/arch/x86/include/asm/xstate.h
@@ -143,4 +143,46 @@ static inline bool xstate_all(const struct vcpu *v)
            (v->arch.xcr0_accum & XSTATE_LAZY & ~XSTATE_FP_SSE);
 }
 
+/*
+ * Fetch a pointer to a vCPU's XSAVE area
+ *
+ * TL;DR: If v == current, the mapping is guaranteed to already exist.
+ *
+ * Despite the name, this macro might not actually map anything. The only case
+ * in which a mutation of page tables is strictly required is when ASI==on &&
+ * v!=current. For everything else the mapping already exists and needs not
+ * be created nor destroyed.
+ *
+ *                         +-----------------+--------------+
+ *                         |   v == current  | v != current |
+ *          +--------------+-----------------+--------------+
+ *          | ASI  enabled | per-vCPU fixmap |  actual map  |
+ *          +--------------+-----------------+--------------+
+ *          | ASI disabled |             directmap          |
+ *          +--------------+--------------------------------+
+ *
+ * There MUST NOT be outstanding maps of XSAVE areas of the non-current vCPU
+ * at the point of context switch. Otherwise, the unmap operation will
+ * misbehave.
+ *
+ * TODO: Expand the macro to the ASI cases after infra to do so is in place.
+ *
+ * @param v Owner of the XSAVE area
+ */
+#define VCPU_MAP_XSAVE_AREA(v) ((v)->arch.xsave_area)
+
+/*
+ * Drops the mapping of a vCPU's XSAVE area and nullifies its pointer on exit
+ *
+ * See VCPU_MAP_XSAVE_AREA() for additional information on the persistence of
+ * these mappings. This macro only tears down the mappings in the ASI=on &&
+ * v!=current case.
+ *
+ * TODO: Expand the macro to the ASI cases after infra to do so is in place.
+ *
+ * @param v Owner of the XSAVE area
+ * @param x XSAVE blob of v
+ */
+#define VCPU_UNMAP_XSAVE_AREA(v, x) ({ (x) = NULL; })
+
 #endif /* __ASM_XSTATE_H */
-- 
2.47.0

[PATCH v2 03/13] x86/hvm: Map/unmap xsave area in hvm_save_cpu_ctxt()

[Alejandro Vallejo]

No functional change.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
v2:
  * No change
---
 xen/arch/x86/hvm/hvm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 018d44a08b6b..c90654697cb1 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -914,11 +914,11 @@ static int cf_check hvm_save_cpu_ctxt(struct vcpu *v, hvm_domain_context_t *h)
 
     if ( v->fpu_initialised )
     {
-        BUILD_BUG_ON(sizeof(ctxt.fpu_regs) !=
-                     sizeof(v->arch.xsave_area->fpu_sse));
-        memcpy(ctxt.fpu_regs, &v->arch.xsave_area->fpu_sse,
-               sizeof(ctxt.fpu_regs));
+        const struct xsave_struct *xsave_area = VCPU_MAP_XSAVE_AREA(v);
 
+        BUILD_BUG_ON(sizeof(ctxt.fpu_regs) != sizeof(xsave_area->fpu_sse));
+        memcpy(ctxt.fpu_regs, &xsave_area->fpu_sse, sizeof(ctxt.fpu_regs));
+        VCPU_UNMAP_XSAVE_AREA(v, xsave_area);
         ctxt.flags = XEN_X86_FPU_INITIALISED;
     }
 
-- 
2.47.0

[PATCH v2 04/13] x86/fpu: Map/umap xsave area in vcpu_{reset,setup}_fpu()

[Alejandro Vallejo]

No functional change.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
v2:
  * No change
---
 xen/arch/x86/i387.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/i387.c b/xen/arch/x86/i387.c
index 3add0025e495..a6ae323fa95f 100644
--- a/xen/arch/x86/i387.c
+++ b/xen/arch/x86/i387.c
@@ -304,8 +304,10 @@ int vcpu_init_fpu(struct vcpu *v)
 
 void vcpu_reset_fpu(struct vcpu *v)
 {
+    struct xsave_struct *xsave_area = VCPU_MAP_XSAVE_AREA(v);
+
     v->fpu_initialised = false;
-    *v->arch.xsave_area = (struct xsave_struct) {
+    *xsave_area = (struct xsave_struct) {
         .fpu_sse = {
             .mxcsr = MXCSR_DEFAULT,
             .fcw = FCW_RESET,
@@ -313,15 +315,21 @@ void vcpu_reset_fpu(struct vcpu *v)
         },
         .xsave_hdr.xstate_bv = X86_XCR0_X87,
     };
+
+    VCPU_UNMAP_XSAVE_AREA(v, xsave_area);
 }
 
 void vcpu_setup_fpu(struct vcpu *v, const void *data)
 {
+    struct xsave_struct *xsave_area = VCPU_MAP_XSAVE_AREA(v);
+
     v->fpu_initialised = true;
-    *v->arch.xsave_area = (struct xsave_struct) {
+    *xsave_area = (struct xsave_struct) {
         .fpu_sse = *(const fpusse_t*)data,
         .xsave_hdr.xstate_bv = XSTATE_FP_SSE,
     };
+
+    VCPU_UNMAP_XSAVE_AREA(v, xsave_area);
 }
 
 /* Free FPU's context save area */
-- 
2.47.0

[PATCH v2 05/13] x86/xstate: Map/unmap xsave area in xstate_set_init() and handle_setbv()

[Alejandro Vallejo]

No functional change.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
v2:
  * Added comment highlighting fastpath for current
---
 xen/arch/x86/xstate.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index af9e345a7ace..401bdad2eb0d 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -993,7 +993,13 @@ int handle_xsetbv(u32 index, u64 new_bv)
 
         clts();
         if ( curr->fpu_dirtied )
-            asm ( "stmxcsr %0" : "=m" (curr->arch.xsave_area->fpu_sse.mxcsr) );
+        {
+            /* has a fastpath for `current`, so there's no actual map */
+            struct xsave_struct *xsave_area = VCPU_MAP_XSAVE_AREA(curr);
+
+            asm ( "stmxcsr %0" : "=m" (xsave_area->fpu_sse.mxcsr) );
+            VCPU_UNMAP_XSAVE_AREA(curr, xsave_area);
+        }
         else if ( xstate_all(curr) )
         {
             /* See the comment in i387.c:vcpu_restore_fpu_eager(). */
@@ -1048,7 +1054,7 @@ void xstate_set_init(uint64_t mask)
     unsigned long cr0 = read_cr0();
     unsigned long xcr0 = this_cpu(xcr0);
     struct vcpu *v = idle_vcpu[smp_processor_id()];
-    struct xsave_struct *xstate = v->arch.xsave_area;
+    struct xsave_struct *xstate;
 
     if ( ~xfeature_mask & mask )
     {
@@ -1061,8 +1067,10 @@ void xstate_set_init(uint64_t mask)
 
     clts();
 
+    xstate = VCPU_MAP_XSAVE_AREA(v);
     memset(&xstate->xsave_hdr, 0, sizeof(xstate->xsave_hdr));
     xrstor(v, mask);
+    VCPU_UNMAP_XSAVE_AREA(v, xstate);
 
     if ( cr0 & X86_CR0_TS )
         write_cr0(cr0);
-- 
2.47.0

[PATCH v2 06/13] x86/hvm: Map/unmap xsave area in hvmemul_{get,put}_fpu()

[Alejandro Vallejo]

No functional change.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
v2:
  * Added comments highlighting fastpath for current
---
 xen/arch/x86/hvm/emulate.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index f2bc6967dfcb..04a3df420a59 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -2371,7 +2371,9 @@ static int cf_check hvmemul_get_fpu(
         alternative_vcall(hvm_funcs.fpu_dirty_intercept);
     else if ( type == X86EMUL_FPU_fpu )
     {
-        const fpusse_t *fpu_ctxt = &curr->arch.xsave_area->fpu_sse;
+        /* has a fastpath for `current`, so there's no actual map */
+        const struct xsave_struct *xsave_area = VCPU_MAP_XSAVE_AREA(curr);
+        const fpusse_t *fpu_ctxt = &xsave_area->fpu_sse;
 
         /*
          * Latch current register state so that we can back out changes
@@ -2397,6 +2399,8 @@ static int cf_check hvmemul_get_fpu(
             else
                 ASSERT(fcw == fpu_ctxt->fcw);
         }
+
+        VCPU_UNMAP_XSAVE_AREA(curr, xsave_area);
     }
 
     return X86EMUL_OKAY;
@@ -2411,7 +2415,9 @@ static void cf_check hvmemul_put_fpu(
 
     if ( aux )
     {
-        fpusse_t *fpu_ctxt = &curr->arch.xsave_area->fpu_sse;
+        /* has a fastpath for `current`, so there's no actual map */
+        struct xsave_struct *xsave_area = VCPU_MAP_XSAVE_AREA(curr);
+        fpusse_t *fpu_ctxt = &xsave_area->fpu_sse;
         bool dval = aux->dval;
         int mode = hvm_guest_x86_mode(curr);
 
@@ -2465,6 +2471,8 @@ static void cf_check hvmemul_put_fpu(
 
         fpu_ctxt->fop = aux->op;
 
+        VCPU_UNMAP_XSAVE_AREA(curr, xsave_area);
+
         /* Re-use backout code below. */
         backout = X86EMUL_FPU_fpu;
     }
-- 
2.47.0

[PATCH v2 07/13] x86/domctl: Map/unmap xsave area in arch_get_info_guest()

[Alejandro Vallejo]

No functional change.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
v2:
  * No change
---
 xen/arch/x86/domctl.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 5f01111619da..3044f706de1c 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1377,16 +1377,17 @@ void arch_get_info_guest(struct vcpu *v, vcpu_guest_context_u c)
     unsigned int i;
     const struct domain *d = v->domain;
     bool compat = is_pv_32bit_domain(d);
+    const struct xsave_struct *xsave_area;
 #ifdef CONFIG_COMPAT
 #define c(fld) (!compat ? (c.nat->fld) : (c.cmp->fld))
 #else
 #define c(fld) (c.nat->fld)
 #endif
 
-    BUILD_BUG_ON(sizeof(c.nat->fpu_ctxt) !=
-                 sizeof(v->arch.xsave_area->fpu_sse));
-    memcpy(&c.nat->fpu_ctxt, &v->arch.xsave_area->fpu_sse,
-           sizeof(c.nat->fpu_ctxt));
+    xsave_area = VCPU_MAP_XSAVE_AREA(v);
+    BUILD_BUG_ON(sizeof(c.nat->fpu_ctxt) != sizeof(xsave_area->fpu_sse));
+    memcpy(&c.nat->fpu_ctxt, &xsave_area->fpu_sse, sizeof(c.nat->fpu_ctxt));
+    VCPU_UNMAP_XSAVE_AREA(v, xsave_area);
 
     if ( is_pv_domain(d) )
         c(flags = v->arch.pv.vgc_flags & ~(VGCF_i387_valid|VGCF_in_kernel));
-- 
2.47.0

[PATCH v2 08/13] x86/xstate: Map/unmap xsave area in {compress,expand}_xsave_states()

[Alejandro Vallejo]

No functional change.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
v2:
  * No change
---
 xen/arch/x86/xstate.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 401bdad2eb0d..6db7ec2ea6a9 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -177,7 +177,7 @@ static void setup_xstate_comp(uint16_t *comp_offsets,
  */
 void expand_xsave_states(const struct vcpu *v, void *dest, unsigned int size)
 {
-    const struct xsave_struct *xstate = v->arch.xsave_area;
+    const struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);
     const void *src;
     uint16_t comp_offsets[sizeof(xfeature_mask)*8];
     u64 xstate_bv = xstate->xsave_hdr.xstate_bv;
@@ -228,6 +228,8 @@ void expand_xsave_states(const struct vcpu *v, void *dest, unsigned int size)
 
         valid &= ~feature;
     }
+
+    VCPU_UNMAP_XSAVE_AREA(v, xstate);
 }
 
 /*
@@ -242,7 +244,7 @@ void expand_xsave_states(const struct vcpu *v, void *dest, unsigned int size)
  */
 void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size)
 {
-    struct xsave_struct *xstate = v->arch.xsave_area;
+    struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);
     void *dest;
     uint16_t comp_offsets[sizeof(xfeature_mask)*8];
     u64 xstate_bv, valid;
@@ -294,6 +296,8 @@ void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size)
 
         valid &= ~feature;
     }
+
+    VCPU_UNMAP_XSAVE_AREA(v, xstate);
 }
 
 void xsave(struct vcpu *v, uint64_t mask)
-- 
2.47.0

[PATCH v2 09/13] x86/emulator: Refactor FXSAVE_AREA to use wrappers

[Alejandro Vallejo]

Adds an UNMAP primitive to make use of vcpu_unmap_xsave_area() when
linked into xen. unmap is a no-op during tests.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
v2:
  * Added comments highlighting fastpath on `current`
---
 xen/arch/x86/x86_emulate/blk.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/x86_emulate/blk.c b/xen/arch/x86/x86_emulate/blk.c
index 08a05f8453f7..76fd497ed8a3 100644
--- a/xen/arch/x86/x86_emulate/blk.c
+++ b/xen/arch/x86/x86_emulate/blk.c
@@ -11,9 +11,12 @@
     !defined(X86EMUL_NO_SIMD)
 # ifdef __XEN__
 #  include <asm/xstate.h>
-#  define FXSAVE_AREA ((void *)&current->arch.xsave_area->fpu_sse)
+/* has a fastpath for `current`, so there's no actual map */
+#  define FXSAVE_AREA ((void *)VCPU_MAP_XSAVE_AREA(current))
+#  define UNMAP_FXSAVE_AREA(x) VCPU_UNMAP_XSAVE_AREA(currt ent, x)
 # else
 #  define FXSAVE_AREA get_fpu_save_area()
+#  define UNMAP_FXSAVE_AREA(x) ((void)x)
 # endif
 #endif
 
@@ -292,6 +295,9 @@ int x86_emul_blk(
         }
         else
             asm volatile ( "fxrstor %0" :: "m" (*fxsr) );
+
+        UNMAP_FXSAVE_AREA(fxsr);
+
         break;
     }
 
@@ -320,6 +326,9 @@ int x86_emul_blk(
 
         if ( fxsr != ptr ) /* i.e. s->op_bytes < sizeof(*fxsr) */
             memcpy(ptr, fxsr, s->op_bytes);
+
+        UNMAP_FXSAVE_AREA(fxsr);
+
         break;
     }
 
-- 
2.47.0

[PATCH v2 10/13] x86/mpx: Map/unmap xsave area in in read_bndcfgu()

[Alejandro Vallejo]

No functional change.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
v2:
  * s/ret/bndcfgu
---
 xen/arch/x86/xstate.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 6db7ec2ea6a9..9ecbef760277 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -1022,9 +1022,10 @@ int handle_xsetbv(u32 index, u64 new_bv)
 
 uint64_t read_bndcfgu(void)
 {
+    uint64_t bndcfgu = 0;
     unsigned long cr0 = read_cr0();
-    struct xsave_struct *xstate
-        = idle_vcpu[smp_processor_id()]->arch.xsave_area;
+    struct vcpu *v = idle_vcpu[smp_processor_id()];
+    struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);
     const struct xstate_bndcsr *bndcsr;
 
     ASSERT(cpu_has_mpx);
@@ -1050,7 +1051,12 @@ uint64_t read_bndcfgu(void)
     if ( cr0 & X86_CR0_TS )
         write_cr0(cr0);
 
-    return xstate->xsave_hdr.xstate_bv & X86_XCR0_BNDCSR ? bndcsr->bndcfgu : 0;
+    if ( xstate->xsave_hdr.xstate_bv & X86_XCR0_BNDCSR )
+        bndcfgu = bndcsr->bndcfgu;
+
+    VCPU_UNMAP_XSAVE_AREA(v, xstate);
+
+    return bndcfgu;
 }
 
 void xstate_set_init(uint64_t mask)
-- 
2.47.0

[PATCH v2 11/13] x86/fpu: Pass explicit xsave areas to fpu_(f)xsave()

[Alejandro Vallejo]

No functional change.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
v2:
  * const-ified v
---
 xen/arch/x86/i387.c               | 16 ++++++++++------
 xen/arch/x86/include/asm/xstate.h |  2 +-
 xen/arch/x86/xstate.c             |  3 +--
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/i387.c b/xen/arch/x86/i387.c
index a6ae323fa95f..73c52ce2f577 100644
--- a/xen/arch/x86/i387.c
+++ b/xen/arch/x86/i387.c
@@ -129,7 +129,7 @@ static inline uint64_t vcpu_xsave_mask(const struct vcpu *v)
 }
 
 /* Save x87 extended state */
-static inline void fpu_xsave(struct vcpu *v)
+static inline void fpu_xsave(const struct vcpu *v, struct xsave_struct *xsave_area)
 {
     bool ok;
     uint64_t mask = vcpu_xsave_mask(v);
@@ -141,15 +141,14 @@ static inline void fpu_xsave(struct vcpu *v)
      */
     ok = set_xcr0(v->arch.xcr0_accum | XSTATE_FP_SSE);
     ASSERT(ok);
-    xsave(v, mask);
+    xsave(v, xsave_area, mask);
     ok = set_xcr0(v->arch.xcr0 ?: XSTATE_FP_SSE);
     ASSERT(ok);
 }
 
 /* Save x87 FPU, MMX, SSE and SSE2 state */
-static inline void fpu_fxsave(struct vcpu *v)
+static inline void fpu_fxsave(struct vcpu *v, fpusse_t *fpu_ctxt)
 {
-    fpusse_t *fpu_ctxt = &v->arch.xsave_area->fpu_sse;
     unsigned int fip_width = v->domain->arch.x87_fip_width;
 
     if ( fip_width != 4 )
@@ -264,6 +263,8 @@ void vcpu_restore_fpu_lazy(struct vcpu *v)
  */
 static bool _vcpu_save_fpu(struct vcpu *v)
 {
+    struct xsave_struct *xsave_area;
+
     if ( !v->fpu_dirtied && !v->arch.nonlazy_xstate_used )
         return false;
 
@@ -272,11 +273,14 @@ static bool _vcpu_save_fpu(struct vcpu *v)
     /* This can happen, if a paravirtualised guest OS has set its CR0.TS. */
     clts();
 
+    xsave_area = VCPU_MAP_XSAVE_AREA(v);
+
     if ( cpu_has_xsave )
-        fpu_xsave(v);
+        fpu_xsave(v, xsave_area);
     else
-        fpu_fxsave(v);
+        fpu_fxsave(v, &xsave_area->fpu_sse);
 
+    VCPU_UNMAP_XSAVE_AREA(v, xsave_area);
     v->fpu_dirtied = 0;
 
     return true;
diff --git a/xen/arch/x86/include/asm/xstate.h b/xen/arch/x86/include/asm/xstate.h
index 6b0daff0aeec..bd286123c735 100644
--- a/xen/arch/x86/include/asm/xstate.h
+++ b/xen/arch/x86/include/asm/xstate.h
@@ -97,7 +97,7 @@ uint64_t get_xcr0(void);
 void set_msr_xss(u64 xss);
 uint64_t get_msr_xss(void);
 uint64_t read_bndcfgu(void);
-void xsave(struct vcpu *v, uint64_t mask);
+void xsave(const struct vcpu *v, struct xsave_struct *ptr, uint64_t mask);
 void xrstor(struct vcpu *v, uint64_t mask);
 void xstate_set_init(uint64_t mask);
 bool xsave_enabled(const struct vcpu *v);
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 9ecbef760277..f3e41f742c3c 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -300,9 +300,8 @@ void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size)
     VCPU_UNMAP_XSAVE_AREA(v, xstate);
 }
 
-void xsave(struct vcpu *v, uint64_t mask)
+void xsave(const struct vcpu *v, struct xsave_struct *ptr, uint64_t mask)
 {
-    struct xsave_struct *ptr = v->arch.xsave_area;
     uint32_t hmask = mask >> 32;
     uint32_t lmask = mask;
     unsigned int fip_width = v->domain->arch.x87_fip_width;
-- 
2.47.0

[PATCH v2 12/13] x86/fpu: Pass explicit xsave areas to fpu_(f)xrstor()

[Alejandro Vallejo]

No functional change.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
v2:
  * const-ified v in fpu_xrstor()
---
 xen/arch/x86/i387.c               | 26 ++++++++++++++++----------
 xen/arch/x86/include/asm/xstate.h |  2 +-
 xen/arch/x86/xstate.c             | 10 ++++++----
 3 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/i387.c b/xen/arch/x86/i387.c
index 73c52ce2f577..c794367a3cc7 100644
--- a/xen/arch/x86/i387.c
+++ b/xen/arch/x86/i387.c
@@ -20,7 +20,8 @@
 /*     FPU Restore Functions   */
 /*******************************/
 /* Restore x87 extended state */
-static inline void fpu_xrstor(struct vcpu *v, uint64_t mask)
+static inline void fpu_xrstor(struct vcpu *v, struct xsave_struct *xsave_area,
+                              uint64_t mask)
 {
     bool ok;
 
@@ -30,16 +31,14 @@ static inline void fpu_xrstor(struct vcpu *v, uint64_t mask)
      */
     ok = set_xcr0(v->arch.xcr0_accum | XSTATE_FP_SSE);
     ASSERT(ok);
-    xrstor(v, mask);
+    xrstor(v, xsave_area, mask);
     ok = set_xcr0(v->arch.xcr0 ?: XSTATE_FP_SSE);
     ASSERT(ok);
 }
 
 /* Restore x87 FPU, MMX, SSE and SSE2 state */
-static inline void fpu_fxrstor(struct vcpu *v)
+static inline void fpu_fxrstor(struct vcpu *v, const fpusse_t *fpu_ctxt)
 {
-    const fpusse_t *fpu_ctxt = &v->arch.xsave_area->fpu_sse;
-
     /*
      * Some CPUs don't save/restore FDP/FIP/FOP unless an exception
      * is pending. Clear the x87 state here by setting it to fixed
@@ -195,6 +194,8 @@ static inline void fpu_fxsave(struct vcpu *v, fpusse_t *fpu_ctxt)
 /* Restore FPU state whenever VCPU is schduled in. */
 void vcpu_restore_fpu_nonlazy(struct vcpu *v, bool need_stts)
 {
+    struct xsave_struct *xsave_area;
+
     /* Restore nonlazy extended state (i.e. parts not tracked by CR0.TS). */
     if ( !v->arch.fully_eager_fpu && !v->arch.nonlazy_xstate_used )
         goto maybe_stts;
@@ -209,12 +210,13 @@ void vcpu_restore_fpu_nonlazy(struct vcpu *v, bool need_stts)
      * above) we also need to restore full state, to prevent subsequently
      * saving state belonging to another vCPU.
      */
+    xsave_area = VCPU_MAP_XSAVE_AREA(v);
     if ( v->arch.fully_eager_fpu || xstate_all(v) )
     {
         if ( cpu_has_xsave )
-            fpu_xrstor(v, XSTATE_ALL);
+            fpu_xrstor(v, xsave_area, XSTATE_ALL);
         else
-            fpu_fxrstor(v);
+            fpu_fxrstor(v, &xsave_area->fpu_sse);
 
         v->fpu_initialised = 1;
         v->fpu_dirtied = 1;
@@ -224,9 +226,10 @@ void vcpu_restore_fpu_nonlazy(struct vcpu *v, bool need_stts)
     }
     else
     {
-        fpu_xrstor(v, XSTATE_NONLAZY);
+        fpu_xrstor(v, xsave_area, XSTATE_NONLAZY);
         need_stts = true;
     }
+    VCPU_UNMAP_XSAVE_AREA(v, xsave_area);
 
  maybe_stts:
     if ( need_stts )
@@ -238,6 +241,7 @@ void vcpu_restore_fpu_nonlazy(struct vcpu *v, bool need_stts)
  */
 void vcpu_restore_fpu_lazy(struct vcpu *v)
 {
+    struct xsave_struct *xsave_area;
     ASSERT(!is_idle_vcpu(v));
 
     /* Avoid recursion. */
@@ -248,10 +252,12 @@ void vcpu_restore_fpu_lazy(struct vcpu *v)
 
     ASSERT(!v->arch.fully_eager_fpu);
 
+    xsave_area = VCPU_MAP_XSAVE_AREA(v);
     if ( cpu_has_xsave )
-        fpu_xrstor(v, XSTATE_LAZY);
+        fpu_xrstor(v, xsave_area, XSTATE_LAZY);
     else
-        fpu_fxrstor(v);
+        fpu_fxrstor(v, &xsave_area->fpu_sse);
+    VCPU_UNMAP_XSAVE_AREA(v, xsave_area);
 
     v->fpu_initialised = 1;
     v->fpu_dirtied = 1;
diff --git a/xen/arch/x86/include/asm/xstate.h b/xen/arch/x86/include/asm/xstate.h
index bd286123c735..d2ef4c0b25f0 100644
--- a/xen/arch/x86/include/asm/xstate.h
+++ b/xen/arch/x86/include/asm/xstate.h
@@ -98,7 +98,7 @@ void set_msr_xss(u64 xss);
 uint64_t get_msr_xss(void);
 uint64_t read_bndcfgu(void);
 void xsave(const struct vcpu *v, struct xsave_struct *ptr, uint64_t mask);
-void xrstor(struct vcpu *v, uint64_t mask);
+void xrstor(const struct vcpu *v, struct xsave_struct *ptr, uint64_t mask);
 void xstate_set_init(uint64_t mask);
 bool xsave_enabled(const struct vcpu *v);
 int __must_check validate_xstate(const struct domain *d,
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index f3e41f742c3c..b5e8d90ef600 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -374,11 +374,10 @@ void xsave(const struct vcpu *v, struct xsave_struct *ptr, uint64_t mask)
         ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET] = fip_width;
 }
 
-void xrstor(struct vcpu *v, uint64_t mask)
+void xrstor(const struct vcpu *v, struct xsave_struct *ptr, uint64_t mask)
 {
     uint32_t hmask = mask >> 32;
     uint32_t lmask = mask;
-    struct xsave_struct *ptr = v->arch.xsave_area;
     unsigned int faults, prev_faults;
 
     /*
@@ -992,6 +991,7 @@ int handle_xsetbv(u32 index, u64 new_bv)
     mask &= curr->fpu_dirtied ? ~XSTATE_FP_SSE : XSTATE_NONLAZY;
     if ( mask )
     {
+        struct xsave_struct *xsave_area = VCPU_MAP_XSAVE_AREA(curr);
         unsigned long cr0 = read_cr0();
 
         clts();
@@ -1011,7 +1011,9 @@ int handle_xsetbv(u32 index, u64 new_bv)
             curr->fpu_dirtied = 1;
             cr0 &= ~X86_CR0_TS;
         }
-        xrstor(curr, mask);
+        xrstor(curr, xsave_area, mask);
+        VCPU_UNMAP_XSAVE_AREA(curr, xsave_area);
+
         if ( cr0 & X86_CR0_TS )
             write_cr0(cr0);
     }
@@ -1078,7 +1080,7 @@ void xstate_set_init(uint64_t mask)
 
     xstate = VCPU_MAP_XSAVE_AREA(v);
     memset(&xstate->xsave_hdr, 0, sizeof(xstate->xsave_hdr));
-    xrstor(v, mask);
+    xrstor(v, xstate, mask);
     VCPU_UNMAP_XSAVE_AREA(v, xstate);
 
     if ( cr0 & X86_CR0_TS )
-- 
2.47.0

[PATCH v2 13/13] x86/xstate: Make xstate_all() and vcpu_xsave_mask() take explicit xstate

[Alejandro Vallejo]

No functional change.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
 xen/arch/x86/i387.c               | 9 +++++----
 xen/arch/x86/include/asm/xstate.h | 5 +++--
 xen/arch/x86/xstate.c             | 2 +-
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/i387.c b/xen/arch/x86/i387.c
index c794367a3cc7..36a6c8918162 100644
--- a/xen/arch/x86/i387.c
+++ b/xen/arch/x86/i387.c
@@ -107,7 +107,8 @@ static inline void fpu_fxrstor(struct vcpu *v, const fpusse_t *fpu_ctxt)
 /*      FPU Save Functions     */
 /*******************************/
 
-static inline uint64_t vcpu_xsave_mask(const struct vcpu *v)
+static inline uint64_t vcpu_xsave_mask(const struct vcpu *v,
+                                       const struct xsave_struct *xsave_area)
 {
     if ( v->fpu_dirtied )
         return v->arch.nonlazy_xstate_used ? XSTATE_ALL : XSTATE_LAZY;
@@ -124,14 +125,14 @@ static inline uint64_t vcpu_xsave_mask(const struct vcpu *v)
      * XSTATE_FP_SSE), vcpu_xsave_mask will return XSTATE_ALL. Otherwise
      * return XSTATE_NONLAZY.
      */
-    return xstate_all(v) ? XSTATE_ALL : XSTATE_NONLAZY;
+    return xstate_all(v, xsave_area) ? XSTATE_ALL : XSTATE_NONLAZY;
 }
 
 /* Save x87 extended state */
 static inline void fpu_xsave(const struct vcpu *v, struct xsave_struct *xsave_area)
 {
     bool ok;
-    uint64_t mask = vcpu_xsave_mask(v);
+    uint64_t mask = vcpu_xsave_mask(v, xsave_area);
 
     ASSERT(mask);
     /*
@@ -211,7 +212,7 @@ void vcpu_restore_fpu_nonlazy(struct vcpu *v, bool need_stts)
      * saving state belonging to another vCPU.
      */
     xsave_area = VCPU_MAP_XSAVE_AREA(v);
-    if ( v->arch.fully_eager_fpu || xstate_all(v) )
+    if ( v->arch.fully_eager_fpu || xstate_all(v, xsave_area) )
     {
         if ( cpu_has_xsave )
             fpu_xrstor(v, xsave_area, XSTATE_ALL);
diff --git a/xen/arch/x86/include/asm/xstate.h b/xen/arch/x86/include/asm/xstate.h
index d2ef4c0b25f0..e3e9c18239ed 100644
--- a/xen/arch/x86/include/asm/xstate.h
+++ b/xen/arch/x86/include/asm/xstate.h
@@ -132,14 +132,15 @@ xsave_area_compressed(const struct xsave_struct *xsave_area)
     return xsave_area->xsave_hdr.xcomp_bv & XSTATE_COMPACTION_ENABLED;
 }
 
-static inline bool xstate_all(const struct vcpu *v)
+static inline bool xstate_all(const struct vcpu *v,
+                              const struct xsave_struct *xsave_area)
 {
     /*
      * XSTATE_FP_SSE may be excluded, because the offsets of XSTATE_FP_SSE
      * (in the legacy region of xsave area) are fixed, so saving
      * XSTATE_FP_SSE will not cause overwriting problem with XSAVES/XSAVEC.
      */
-    return xsave_area_compressed(v->arch.xsave_area) &&
+    return xsave_area_compressed(xsave_area) &&
            (v->arch.xcr0_accum & XSTATE_LAZY & ~XSTATE_FP_SSE);
 }
 
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index b5e8d90ef600..26e460adfd79 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -1003,7 +1003,7 @@ int handle_xsetbv(u32 index, u64 new_bv)
             asm ( "stmxcsr %0" : "=m" (xsave_area->fpu_sse.mxcsr) );
             VCPU_UNMAP_XSAVE_AREA(curr, xsave_area);
         }
-        else if ( xstate_all(curr) )
+        else if ( xstate_all(curr, xsave_area) )
         {
             /* See the comment in i387.c:vcpu_restore_fpu_eager(). */
             mask |= XSTATE_LAZY;
-- 
2.47.0

Re: [PATCH v2 01/13] x86/xstate: Remove stale assertions in fpu_x{rstor,save}()

[Jan Beulich]

On 05.11.2024 15:32, Alejandro Vallejo wrote:
> After edb48e76458b("x86/fpu: Combine fpu_ctxt and xsave_area in arch_vcpu"),
> v->arch.xsave_area is always present and we can just remove these asserts.
> 
> Fixes: edb48e76458b("x86/fpu: Combine fpu_ctxt and xsave_area in arch_vcpu")
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH v2 02/13] x86/xstate: Create map/unmap primitives for xsave areas

[Jan Beulich]

On 05.11.2024 15:32, Alejandro Vallejo wrote:
> --- a/xen/arch/x86/include/asm/xstate.h
> +++ b/xen/arch/x86/include/asm/xstate.h
> @@ -143,4 +143,46 @@ static inline bool xstate_all(const struct vcpu *v)
>             (v->arch.xcr0_accum & XSTATE_LAZY & ~XSTATE_FP_SSE);
>  }
>  
> +/*
> + * Fetch a pointer to a vCPU's XSAVE area
> + *
> + * TL;DR: If v == current, the mapping is guaranteed to already exist.
> + *
> + * Despite the name, this macro might not actually map anything. The only case
> + * in which a mutation of page tables is strictly required is when ASI==on &&
> + * v!=current. For everything else the mapping already exists and needs not
> + * be created nor destroyed.
> + *
> + *                         +-----------------+--------------+
> + *                         |   v == current  | v != current |
> + *          +--------------+-----------------+--------------+
> + *          | ASI  enabled | per-vCPU fixmap |  actual map  |
> + *          +--------------+-----------------+--------------+
> + *          | ASI disabled |             directmap          |
> + *          +--------------+--------------------------------+
> + *
> + * There MUST NOT be outstanding maps of XSAVE areas of the non-current vCPU
> + * at the point of context switch. Otherwise, the unmap operation will
> + * misbehave.
> + *
> + * TODO: Expand the macro to the ASI cases after infra to do so is in place.
> + *
> + * @param v Owner of the XSAVE area
> + */
> +#define VCPU_MAP_XSAVE_AREA(v) ((v)->arch.xsave_area)

When this is fleshed out, I expect (hope) type safety (type of "return
value") will remain to be there. I think it would be nice ...

> +/*
> + * Drops the mapping of a vCPU's XSAVE area and nullifies its pointer on exit
> + *
> + * See VCPU_MAP_XSAVE_AREA() for additional information on the persistence of
> + * these mappings. This macro only tears down the mappings in the ASI=on &&
> + * v!=current case.
> + *
> + * TODO: Expand the macro to the ASI cases after infra to do so is in place.
> + *
> + * @param v Owner of the XSAVE area
> + * @param x XSAVE blob of v
> + */
> +#define VCPU_UNMAP_XSAVE_AREA(v, x) ({ (x) = NULL; })

... if this was typesafe (at least on x) from the very beginning as
well. Thoughts?

Jan

Re: [PATCH v2 03/13] x86/hvm: Map/unmap xsave area in hvm_save_cpu_ctxt()

[Jan Beulich]

On 05.11.2024 15:33, Alejandro Vallejo wrote:
> No functional change.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH v2 04/13] x86/fpu: Map/umap xsave area in vcpu_{reset,setup}_fpu()

[Jan Beulich]

On 05.11.2024 15:33, Alejandro Vallejo wrote:
> No functional change.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH v2 05/13] x86/xstate: Map/unmap xsave area in xstate_set_init() and handle_setbv()

[Jan Beulich]

On 05.11.2024 15:33, Alejandro Vallejo wrote:
> No functional change.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
with ...

> --- a/xen/arch/x86/xstate.c
> +++ b/xen/arch/x86/xstate.c
> @@ -993,7 +993,13 @@ int handle_xsetbv(u32 index, u64 new_bv)
>  
>          clts();
>          if ( curr->fpu_dirtied )
> -            asm ( "stmxcsr %0" : "=m" (curr->arch.xsave_area->fpu_sse.mxcsr) );
> +        {
> +            /* has a fastpath for `current`, so there's no actual map */
> +            struct xsave_struct *xsave_area = VCPU_MAP_XSAVE_AREA(curr);

... comment style adhered to (capital 'H').

Jan

Re: [PATCH v2 06/13] x86/hvm: Map/unmap xsave area in hvmemul_{get,put}_fpu()

[Jan Beulich]

On 05.11.2024 15:33, Alejandro Vallejo wrote:
> No functional change.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>

Again with comment style adjusted:
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan

Re: [PATCH v2 07/13] x86/domctl: Map/unmap xsave area in arch_get_info_guest()

[Jan Beulich]

On 05.11.2024 15:33, Alejandro Vallejo wrote:
> No functional change.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH v2 08/13] x86/xstate: Map/unmap xsave area in {compress,expand}_xsave_states()

[Jan Beulich]

On 05.11.2024 15:33, Alejandro Vallejo wrote:
> No functional change.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
> ---
> v2:
>   * No change
> ---
>  xen/arch/x86/xstate.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
> index 401bdad2eb0d..6db7ec2ea6a9 100644
> --- a/xen/arch/x86/xstate.c
> +++ b/xen/arch/x86/xstate.c
> @@ -177,7 +177,7 @@ static void setup_xstate_comp(uint16_t *comp_offsets,
>   */
>  void expand_xsave_states(const struct vcpu *v, void *dest, unsigned int size)
>  {
> -    const struct xsave_struct *xstate = v->arch.xsave_area;
> +    const struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);
>      const void *src;
>      uint16_t comp_offsets[sizeof(xfeature_mask)*8];
>      u64 xstate_bv = xstate->xsave_hdr.xstate_bv;
> @@ -228,6 +228,8 @@ void expand_xsave_states(const struct vcpu *v, void *dest, unsigned int size)
>  
>          valid &= ~feature;
>      }
> +
> +    VCPU_UNMAP_XSAVE_AREA(v, xstate);
>  }

In the middle of these two hunks there's an early return.

> @@ -242,7 +244,7 @@ void expand_xsave_states(const struct vcpu *v, void *dest, unsigned int size)
>   */
>  void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size)
>  {
> -    struct xsave_struct *xstate = v->arch.xsave_area;
> +    struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);
>      void *dest;
>      uint16_t comp_offsets[sizeof(xfeature_mask)*8];
>      u64 xstate_bv, valid;
> @@ -294,6 +296,8 @@ void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size)
>  
>          valid &= ~feature;
>      }
> +
> +    VCPU_UNMAP_XSAVE_AREA(v, xstate);
>  }

Same here.

Jan

Re: [PATCH v2 09/13] x86/emulator: Refactor FXSAVE_AREA to use wrappers

[Jan Beulich]

On 05.11.2024 15:33, Alejandro Vallejo wrote:
> --- a/xen/arch/x86/x86_emulate/blk.c
> +++ b/xen/arch/x86/x86_emulate/blk.c
> @@ -11,9 +11,12 @@
>      !defined(X86EMUL_NO_SIMD)
>  # ifdef __XEN__
>  #  include <asm/xstate.h>
> -#  define FXSAVE_AREA ((void *)&current->arch.xsave_area->fpu_sse)
> +/* has a fastpath for `current`, so there's no actual map */
> +#  define FXSAVE_AREA ((void *)VCPU_MAP_XSAVE_AREA(current))
> +#  define UNMAP_FXSAVE_AREA(x) VCPU_UNMAP_XSAVE_AREA(currt ent, x)

The typo of the first argument strongly suggests that the macro should
already now evaluate its parameters, also pleasing Misra.

>  # else
>  #  define FXSAVE_AREA get_fpu_save_area()
> +#  define UNMAP_FXSAVE_AREA(x) ((void)x)

If only for consistency and to avoid setting bad precedents - parentheses
please around x.

> @@ -292,6 +295,9 @@ int x86_emul_blk(
>          }
>          else
>              asm volatile ( "fxrstor %0" :: "m" (*fxsr) );
> +
> +        UNMAP_FXSAVE_AREA(fxsr);
> +
>          break;
>      }
>  
> @@ -320,6 +326,9 @@ int x86_emul_blk(
>  
>          if ( fxsr != ptr ) /* i.e. s->op_bytes < sizeof(*fxsr) */
>              memcpy(ptr, fxsr, s->op_bytes);
> +
> +        UNMAP_FXSAVE_AREA(fxsr);
> +
>          break;
>      }

So for now the emulator only supports FXSAVE / FXRSTOR. That'll need to change
sooner or later. Is it really appropriate in that light to name the new macro
after FXSAVE, when the underlying machinery uses all XSAVE?

Jan

Re: [PATCH v2 10/13] x86/mpx: Map/unmap xsave area in in read_bndcfgu()

[Jan Beulich]

On 05.11.2024 15:33, Alejandro Vallejo wrote:
> --- a/xen/arch/x86/xstate.c
> +++ b/xen/arch/x86/xstate.c
> @@ -1022,9 +1022,10 @@ int handle_xsetbv(u32 index, u64 new_bv)
>  
>  uint64_t read_bndcfgu(void)
>  {
> +    uint64_t bndcfgu = 0;
>      unsigned long cr0 = read_cr0();
> -    struct xsave_struct *xstate
> -        = idle_vcpu[smp_processor_id()]->arch.xsave_area;
> +    struct vcpu *v = idle_vcpu[smp_processor_id()];

Can this be pointer-to-const? Certainly right now, so the question is rather
meant to be forward looking.

> +    struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);

This certainly can be pointer-to-const, just like ...

>      const struct xstate_bndcsr *bndcsr;

... this is.

Jan

Re: [PATCH v2 11/13] x86/fpu: Pass explicit xsave areas to fpu_(f)xsave()

[Jan Beulich]

On 05.11.2024 15:33, Alejandro Vallejo wrote:
> No functional change.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
> ---
> v2:
>   * const-ified v

Hmm, yes, you did ...

> --- a/xen/arch/x86/i387.c
> +++ b/xen/arch/x86/i387.c
> @@ -129,7 +129,7 @@ static inline uint64_t vcpu_xsave_mask(const struct vcpu *v)
>  }
>  
>  /* Save x87 extended state */
> -static inline void fpu_xsave(struct vcpu *v)
> +static inline void fpu_xsave(const struct vcpu *v, struct xsave_struct *xsave_area)

... here.

> @@ -141,15 +141,14 @@ static inline void fpu_xsave(struct vcpu *v)
>       */
>      ok = set_xcr0(v->arch.xcr0_accum | XSTATE_FP_SSE);
>      ASSERT(ok);
> -    xsave(v, mask);
> +    xsave(v, xsave_area, mask);
>      ok = set_xcr0(v->arch.xcr0 ?: XSTATE_FP_SSE);
>      ASSERT(ok);
>  }
>  
>  /* Save x87 FPU, MMX, SSE and SSE2 state */
> -static inline void fpu_fxsave(struct vcpu *v)
> +static inline void fpu_fxsave(struct vcpu *v, fpusse_t *fpu_ctxt)

But then not here.

Jan

Re: [PATCH v2 12/13] x86/fpu: Pass explicit xsave areas to fpu_(f)xrstor()

[Jan Beulich]

On 05.11.2024 15:33, Alejandro Vallejo wrote:
> No functional change.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
> ---
> v2:
>   * const-ified v in fpu_xrstor()

Yet what about fpu_fxrstor()?

Jan

Re: [PATCH v2 13/13] x86/xstate: Make xstate_all() and vcpu_xsave_mask() take explicit xstate

[Jan Beulich]

On 05.11.2024 15:33, Alejandro Vallejo wrote:
> No functional change.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH v2 02/13] x86/xstate: Create map/unmap primitives for xsave areas

[Alejandro Vallejo]

On Mon Dec 9, 2024 at 4:11 PM GMT, Jan Beulich wrote:
> On 05.11.2024 15:32, Alejandro Vallejo wrote:
> > --- a/xen/arch/x86/include/asm/xstate.h
> > +++ b/xen/arch/x86/include/asm/xstate.h
> > @@ -143,4 +143,46 @@ static inline bool xstate_all(const struct vcpu *v)
> >             (v->arch.xcr0_accum & XSTATE_LAZY & ~XSTATE_FP_SSE);
> >  }
> >  
> > +/*
> > + * Fetch a pointer to a vCPU's XSAVE area
> > + *
> > + * TL;DR: If v == current, the mapping is guaranteed to already exist.
> > + *
> > + * Despite the name, this macro might not actually map anything. The only case
> > + * in which a mutation of page tables is strictly required is when ASI==on &&
> > + * v!=current. For everything else the mapping already exists and needs not
> > + * be created nor destroyed.
> > + *
> > + *                         +-----------------+--------------+
> > + *                         |   v == current  | v != current |
> > + *          +--------------+-----------------+--------------+
> > + *          | ASI  enabled | per-vCPU fixmap |  actual map  |
> > + *          +--------------+-----------------+--------------+
> > + *          | ASI disabled |             directmap          |
> > + *          +--------------+--------------------------------+
> > + *
> > + * There MUST NOT be outstanding maps of XSAVE areas of the non-current vCPU
> > + * at the point of context switch. Otherwise, the unmap operation will
> > + * misbehave.
> > + *
> > + * TODO: Expand the macro to the ASI cases after infra to do so is in place.
> > + *
> > + * @param v Owner of the XSAVE area
> > + */
> > +#define VCPU_MAP_XSAVE_AREA(v) ((v)->arch.xsave_area)
>
> When this is fleshed out, I expect (hope) type safety (type of "return
> value") will remain to be there. I think it would be nice ...

The return type will always be a pointer to `struct xsave_struct`, so that's
definitely type-safe.

>
> > +/*
> > + * Drops the mapping of a vCPU's XSAVE area and nullifies its pointer on exit
> > + *
> > + * See VCPU_MAP_XSAVE_AREA() for additional information on the persistence of
> > + * these mappings. This macro only tears down the mappings in the ASI=on &&
> > + * v!=current case.
> > + *
> > + * TODO: Expand the macro to the ASI cases after infra to do so is in place.
> > + *
> > + * @param v Owner of the XSAVE area
> > + * @param x XSAVE blob of v
> > + */
> > +#define VCPU_UNMAP_XSAVE_AREA(v, x) ({ (x) = NULL; })
>
> ... if this was typesafe (at least on x) from the very beginning as
> well. Thoughts?

I tentatively intend for both macros to involve a call to static inline
functions when the real infrastructure is in place. By the time everything is
fleshed out both will be definitely type-safe. Const-ness might suffer though,
as there's a tension between clarity of static inlines and flexible qualifiers
(i.e: having const outputs iff inputs are const).

>
> Jan

Cheers,
Alejandro

Re: [PATCH v2 05/13] x86/xstate: Map/unmap xsave area in xstate_set_init() and handle_setbv()

[Alejandro Vallejo]

On Mon Dec 9, 2024 at 4:16 PM GMT, Jan Beulich wrote:
> On 05.11.2024 15:33, Alejandro Vallejo wrote:
> > No functional change.
> > 
> > Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
>
> Acked-by: Jan Beulich <jbeulich@suse.com>

Thanks (for this one and the others).

> with ...
>
> > --- a/xen/arch/x86/xstate.c
> > +++ b/xen/arch/x86/xstate.c
> > @@ -993,7 +993,13 @@ int handle_xsetbv(u32 index, u64 new_bv)
> >  
> >          clts();
> >          if ( curr->fpu_dirtied )
> > -            asm ( "stmxcsr %0" : "=m" (curr->arch.xsave_area->fpu_sse.mxcsr) );
> > +        {
> > +            /* has a fastpath for `current`, so there's no actual map */
> > +            struct xsave_struct *xsave_area = VCPU_MAP_XSAVE_AREA(curr);
>
> ... comment style adhered to (capital 'H').
>
> Jan

Ah, yes. All those ought to be capitalized. My bad.

Cheers,
Alejandro

Re: [PATCH v2 08/13] x86/xstate: Map/unmap xsave area in {compress,expand}_xsave_states()

[Alejandro Vallejo]

On Mon Dec 9, 2024 at 4:20 PM GMT, Jan Beulich wrote:
> On 05.11.2024 15:33, Alejandro Vallejo wrote:
> > No functional change.
> > 
> > Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
> > ---
> > v2:
> >   * No change
> > ---
> >  xen/arch/x86/xstate.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
> > index 401bdad2eb0d..6db7ec2ea6a9 100644
> > --- a/xen/arch/x86/xstate.c
> > +++ b/xen/arch/x86/xstate.c
> > @@ -177,7 +177,7 @@ static void setup_xstate_comp(uint16_t *comp_offsets,
> >   */
> >  void expand_xsave_states(const struct vcpu *v, void *dest, unsigned int size)
> >  {
> > -    const struct xsave_struct *xstate = v->arch.xsave_area;
> > +    const struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);
> >      const void *src;
> >      uint16_t comp_offsets[sizeof(xfeature_mask)*8];
> >      u64 xstate_bv = xstate->xsave_hdr.xstate_bv;
> > @@ -228,6 +228,8 @@ void expand_xsave_states(const struct vcpu *v, void *dest, unsigned int size)
> >  
> >          valid &= ~feature;
> >      }
> > +
> > +    VCPU_UNMAP_XSAVE_AREA(v, xstate);
> >  }
>
> In the middle of these two hunks there's an early return.
>
> > @@ -242,7 +244,7 @@ void expand_xsave_states(const struct vcpu *v, void *dest, unsigned int size)
> >   */
> >  void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size)
> >  {
> > -    struct xsave_struct *xstate = v->arch.xsave_area;
> > +    struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);
> >      void *dest;
> >      uint16_t comp_offsets[sizeof(xfeature_mask)*8];
> >      u64 xstate_bv, valid;
> > @@ -294,6 +296,8 @@ void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size)
> >  
> >          valid &= ~feature;
> >      }
> > +
> > +    VCPU_UNMAP_XSAVE_AREA(v, xstate);
> >  }
>
> Same here.
>
> Jan

Doh! Yes, good catch. I'll "goto out" on both rather than the early exit to
ensure the unmap is invoked in the "already (de)compressed" cases.

Cheers,
Alejandro

Re: [PATCH v2 09/13] x86/emulator: Refactor FXSAVE_AREA to use wrappers

[Alejandro Vallejo]

On Mon Dec 9, 2024 at 4:26 PM GMT, Jan Beulich wrote:
> On 05.11.2024 15:33, Alejandro Vallejo wrote:
> > --- a/xen/arch/x86/x86_emulate/blk.c
> > +++ b/xen/arch/x86/x86_emulate/blk.c
> > @@ -11,9 +11,12 @@
> >      !defined(X86EMUL_NO_SIMD)
> >  # ifdef __XEN__
> >  #  include <asm/xstate.h>
> > -#  define FXSAVE_AREA ((void *)&current->arch.xsave_area->fpu_sse)
> > +/* has a fastpath for `current`, so there's no actual map */
> > +#  define FXSAVE_AREA ((void *)VCPU_MAP_XSAVE_AREA(current))
> > +#  define UNMAP_FXSAVE_AREA(x) VCPU_UNMAP_XSAVE_AREA(currt ent, x)
>
> The typo of the first argument strongly suggests that the macro should
> already now evaluate its parameters, also pleasing Misra.

Not an unreasonable takeaway. I can expand as follows instead:

#define VCPU_UNMAP_XSAVE_AREA(v, x) ({ (void)(v); x; })

Thoughts?

>
> >  # else
> >  #  define FXSAVE_AREA get_fpu_save_area()
> > +#  define UNMAP_FXSAVE_AREA(x) ((void)x)
>
> If only for consistency and to avoid setting bad precedents - parentheses
> please around x.

Ack.

>
> > @@ -292,6 +295,9 @@ int x86_emul_blk(
> >          }
> >          else
> >              asm volatile ( "fxrstor %0" :: "m" (*fxsr) );
> > +
> > +        UNMAP_FXSAVE_AREA(fxsr);
> > +
> >          break;
> >      }
> >  
> > @@ -320,6 +326,9 @@ int x86_emul_blk(
> >  
> >          if ( fxsr != ptr ) /* i.e. s->op_bytes < sizeof(*fxsr) */
> >              memcpy(ptr, fxsr, s->op_bytes);
> > +
> > +        UNMAP_FXSAVE_AREA(fxsr);
> > +
> >          break;
> >      }
>
> So for now the emulator only supports FXSAVE / FXRSTOR. That'll need to change
> sooner or later. Is it really appropriate in that light to name the new macro
> after FXSAVE, when the underlying machinery uses all XSAVE?
>
> Jan

I have no strong feeling one way or the other, except that it should mirror the
other macro's name. I'd say it makes more sense to rename _both_ after the
emulator is XSAVE-aware. That the internal machinery is actually XSAVE-based is
an implementation detail largely irrelevant at the call sites.

Cheers,
Alejandro

Re: [PATCH v2 10/13] x86/mpx: Map/unmap xsave area in in read_bndcfgu()

[Alejandro Vallejo]

On Mon Dec 9, 2024 at 4:30 PM GMT, Jan Beulich wrote:
> On 05.11.2024 15:33, Alejandro Vallejo wrote:
> > --- a/xen/arch/x86/xstate.c
> > +++ b/xen/arch/x86/xstate.c
> > @@ -1022,9 +1022,10 @@ int handle_xsetbv(u32 index, u64 new_bv)
> >  
> >  uint64_t read_bndcfgu(void)
> >  {
> > +    uint64_t bndcfgu = 0;
> >      unsigned long cr0 = read_cr0();
> > -    struct xsave_struct *xstate
> > -        = idle_vcpu[smp_processor_id()]->arch.xsave_area;
> > +    struct vcpu *v = idle_vcpu[smp_processor_id()];
>
> Can this be pointer-to-const? Certainly right now, so the question is rather
> meant to be forward looking.
>
> > +    struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);
>
> This certainly can be pointer-to-const, just like ...
>
> >      const struct xstate_bndcsr *bndcsr;
>
> ... this is.
>
> Jan

Yes, those retained non-const because of the now missing patch to zero-out
bndcfgu.

Cheers,
Alejandro

Re: [PATCH v2 09/13] x86/emulator: Refactor FXSAVE_AREA to use wrappers

[Jan Beulich]

On 16.12.2024 12:58, Alejandro Vallejo wrote:
> On Mon Dec 9, 2024 at 4:26 PM GMT, Jan Beulich wrote:
>> On 05.11.2024 15:33, Alejandro Vallejo wrote:
>>> --- a/xen/arch/x86/x86_emulate/blk.c
>>> +++ b/xen/arch/x86/x86_emulate/blk.c
>>> @@ -11,9 +11,12 @@
>>>      !defined(X86EMUL_NO_SIMD)
>>>  # ifdef __XEN__
>>>  #  include <asm/xstate.h>
>>> -#  define FXSAVE_AREA ((void *)&current->arch.xsave_area->fpu_sse)
>>> +/* has a fastpath for `current`, so there's no actual map */
>>> +#  define FXSAVE_AREA ((void *)VCPU_MAP_XSAVE_AREA(current))
>>> +#  define UNMAP_FXSAVE_AREA(x) VCPU_UNMAP_XSAVE_AREA(currt ent, x)
>>
>> The typo of the first argument strongly suggests that the macro should
>> already now evaluate its parameters, also pleasing Misra.
> 
> Not an unreasonable takeaway. I can expand as follows instead:
> 
> #define VCPU_UNMAP_XSAVE_AREA(v, x) ({ (void)(v); x; })
> 
> Thoughts?

Why would x not also be cast to void?

Jan

Re: [PATCH v2 10/13] x86/mpx: Map/unmap xsave area in in read_bndcfgu()

[Jan Beulich]

On 16.12.2024 13:00, Alejandro Vallejo wrote:
> On Mon Dec 9, 2024 at 4:30 PM GMT, Jan Beulich wrote:
>> On 05.11.2024 15:33, Alejandro Vallejo wrote:
>>> --- a/xen/arch/x86/xstate.c
>>> +++ b/xen/arch/x86/xstate.c
>>> @@ -1022,9 +1022,10 @@ int handle_xsetbv(u32 index, u64 new_bv)
>>>  
>>>  uint64_t read_bndcfgu(void)
>>>  {
>>> +    uint64_t bndcfgu = 0;
>>>      unsigned long cr0 = read_cr0();
>>> -    struct xsave_struct *xstate
>>> -        = idle_vcpu[smp_processor_id()]->arch.xsave_area;
>>> +    struct vcpu *v = idle_vcpu[smp_processor_id()];
>>
>> Can this be pointer-to-const? Certainly right now, so the question is rather
>> meant to be forward looking.
>>
>>> +    struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);
>>
>> This certainly can be pointer-to-const, just like ...
>>
>>>      const struct xstate_bndcsr *bndcsr;
>>
>> ... this is.
> 
> Yes, those retained non-const because of the now missing patch to zero-out
> bndcfgu.

I'm afraid this reply is ambiguous as to what's going to happen in the next
version. I can read both "will change" and "needs to stay" into it.

Jan

Re: [PATCH v2 10/13] x86/mpx: Map/unmap xsave area in in read_bndcfgu()

[Alejandro Vallejo]

On Mon Dec 16, 2024 at 12:03 PM GMT, Jan Beulich wrote:
> On 16.12.2024 13:00, Alejandro Vallejo wrote:
> > On Mon Dec 9, 2024 at 4:30 PM GMT, Jan Beulich wrote:
> >> On 05.11.2024 15:33, Alejandro Vallejo wrote:
> >>> --- a/xen/arch/x86/xstate.c
> >>> +++ b/xen/arch/x86/xstate.c
> >>> @@ -1022,9 +1022,10 @@ int handle_xsetbv(u32 index, u64 new_bv)
> >>>  
> >>>  uint64_t read_bndcfgu(void)
> >>>  {
> >>> +    uint64_t bndcfgu = 0;
> >>>      unsigned long cr0 = read_cr0();
> >>> -    struct xsave_struct *xstate
> >>> -        = idle_vcpu[smp_processor_id()]->arch.xsave_area;
> >>> +    struct vcpu *v = idle_vcpu[smp_processor_id()];
> >>
> >> Can this be pointer-to-const? Certainly right now, so the question is rather
> >> meant to be forward looking.
> >>
> >>> +    struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);
> >>
> >> This certainly can be pointer-to-const, just like ...
> >>
> >>>      const struct xstate_bndcsr *bndcsr;
> >>
> >> ... this is.
> > 
> > Yes, those retained non-const because of the now missing patch to zero-out
> > bndcfgu.
>
> I'm afraid this reply is ambiguous as to what's going to happen in the next
> version. I can read both "will change" and "needs to stay" into it.
>
> Jan

It was an answer to "Can this be pointer to const?". Yes, I'll put the const
back.

Cheers,
Alejandro

Re: [PATCH v2 09/13] x86/emulator: Refactor FXSAVE_AREA to use wrappers

[Alejandro Vallejo]

On Mon Dec 16, 2024 at 12:01 PM GMT, Jan Beulich wrote:
> On 16.12.2024 12:58, Alejandro Vallejo wrote:
> > On Mon Dec 9, 2024 at 4:26 PM GMT, Jan Beulich wrote:
> >> On 05.11.2024 15:33, Alejandro Vallejo wrote:
> >>> --- a/xen/arch/x86/x86_emulate/blk.c
> >>> +++ b/xen/arch/x86/x86_emulate/blk.c
> >>> @@ -11,9 +11,12 @@
> >>>      !defined(X86EMUL_NO_SIMD)
> >>>  # ifdef __XEN__
> >>>  #  include <asm/xstate.h>
> >>> -#  define FXSAVE_AREA ((void *)&current->arch.xsave_area->fpu_sse)
> >>> +/* has a fastpath for `current`, so there's no actual map */
> >>> +#  define FXSAVE_AREA ((void *)VCPU_MAP_XSAVE_AREA(current))
> >>> +#  define UNMAP_FXSAVE_AREA(x) VCPU_UNMAP_XSAVE_AREA(currt ent, x)
> >>
> >> The typo of the first argument strongly suggests that the macro should
> >> already now evaluate its parameters, also pleasing Misra.
> > 
> > Not an unreasonable takeaway. I can expand as follows instead:
> > 
> > #define VCPU_UNMAP_XSAVE_AREA(v, x) ({ (void)(v); x; })
> > 
> > Thoughts?
>
> Why would x not also be cast to void?
>
> Jan

it should have the "x = NULL", in fact.

  #define VCPU_UNMAP_XSAVE_AREA(v, x) do { (void)(v); (x) = NULL; } while(0)

Regardless, the important bit was adding a separate statement for (v) casted to
void. Leaving the rest as-is.

Cheers,
Alejandro

Re: [PATCH v2 10/13] x86/mpx: Map/unmap xsave area in in read_bndcfgu()

[Alejandro Vallejo]

On Mon Dec 16, 2024 at 2:02 PM GMT, Alejandro Vallejo wrote:
> On Mon Dec 16, 2024 at 12:03 PM GMT, Jan Beulich wrote:
> > On 16.12.2024 13:00, Alejandro Vallejo wrote:
> > > On Mon Dec 9, 2024 at 4:30 PM GMT, Jan Beulich wrote:
> > >> On 05.11.2024 15:33, Alejandro Vallejo wrote:
> > >>> --- a/xen/arch/x86/xstate.c
> > >>> +++ b/xen/arch/x86/xstate.c
> > >>> @@ -1022,9 +1022,10 @@ int handle_xsetbv(u32 index, u64 new_bv)
> > >>>  
> > >>>  uint64_t read_bndcfgu(void)
> > >>>  {
> > >>> +    uint64_t bndcfgu = 0;
> > >>>      unsigned long cr0 = read_cr0();
> > >>> -    struct xsave_struct *xstate
> > >>> -        = idle_vcpu[smp_processor_id()]->arch.xsave_area;
> > >>> +    struct vcpu *v = idle_vcpu[smp_processor_id()];
> > >>
> > >> Can this be pointer-to-const? Certainly right now, so the question is rather
> > >> meant to be forward looking.
> > >>
> > >>> +    struct xsave_struct *xstate = VCPU_MAP_XSAVE_AREA(v);
> > >>
> > >> This certainly can be pointer-to-const, just like ...
> > >>
> > >>>      const struct xstate_bndcsr *bndcsr;
> > >>
> > >> ... this is.
> > > 
> > > Yes, those retained non-const because of the now missing patch to zero-out
> > > bndcfgu.
> >
> > I'm afraid this reply is ambiguous as to what's going to happen in the next
> > version. I can read both "will change" and "needs to stay" into it.
> >
> > Jan
>
> It was an answer to "Can this be pointer to const?". Yes, I'll put the const
> back.
>
> Cheers,
> Alejandro

Actually, xstate is the target of the assembly, so it will be written to
(though not from C). It seems like asking for trouble modifying the target of a
pointer to non-volatile const from inline assembly.

I'll leave it as-is in the interest of safety.

Cheers,
Alejandro