[PATCH v6 38/44] x86/emulator: Refactor FXSAVE_AREA to use wrappers

[George Dunlap <gwd@xxxxxxxxxxxxxx>]

From: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>

The FXSAVE/FXRSTOR emulation scratch buffer is the calling vCPU's own
XSAVE area, reached today through a raw pointer dereference.  Route it
through the VCPU_MAP_XSAVE_AREA()/VCPU_UNMAP_XSAVE_AREA() seam
instead, so the buffer keeps working when guest XSAVE areas move
behind per-vCPU mappings.  This path always operates on `current`, for
which the map has a fastpath; the unmap is a no-op outside Xen builds.

The macro pair is named MAP_FXSAVE_AREA()/UNMAP_FXSAVE_AREA() rather
than keeping the bare FXSAVE_AREA name, so that the mapping operation
is visible at the use sites.

blk_fxsave() saves directly into the caller's buffer when it is large
enough, so it needs a second variable: the save destination and the
mapped area are not the same pointer, and only the latter may be
handed to the unmap.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
Assisted-by: Claude Code:claude-fable-5, Claude Code:claude-opus-5-5
Signed-off-by: George Dunlap <gwd@xxxxxxxxxxxxxx>

---
Revived from "x86: Address Space Isolation FPU preparations" v3 patch
08/12 (<20250110132823.24348-9-alejandro.vallejo@xxxxxxxxx>, Acked-by
Jan Beulich modulo the rename below; no v4 was ever posted).

Changes in v5:
- New in this version.

Changes since the previously posted version:
 - Renamed FXSAVE_AREA to MAP_FXSAVE_AREA(), as requested by Jan
   Beulich on the original thread ("the existing macro better would be
   adjusted to become MAP_FXSAVE_AREA()"), making the map operation
   visible at the use sites.
 - Introduced the separate save-destination variable in blk_fxsave().
   The original unmapped `fxsr` after that variable had been
   repointed at the caller's buffer, so the unmap received a pointer
   that was never mapped.  Harmless while the unmap was a stub, but a
   latent bug, and exactly the class of mistake Jan predicted in the
   message quoted above when he objected to the mapping being
   invisible at the use sites.
 - Reworded the commit message.
 - Unmap before raising #GP for a reserved MXCSR bit in blk_fxrstor:
   generate_exception_if() leaves through the done: label, past the
   unmap.
---
 xen/arch/x86/x86_emulate/blk.c | 36 +++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/x86_emulate/blk.c b/xen/arch/x86/x86_emulate/blk.c
index 9fddf649ee..b3055c8084 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 MAP_FXSAVE_AREA() ((void *)VCPU_MAP_XSAVE_AREA(current))
+#  define UNMAP_FXSAVE_AREA(x) VCPU_UNMAP_XSAVE_AREA(current, x)
 # else
-#  define FXSAVE_AREA get_fpu_save_area()
+#  define MAP_FXSAVE_AREA() get_fpu_save_area()
+#  define UNMAP_FXSAVE_AREA(x) ((void)(x))
 # endif
 #endif
 
@@ -254,7 +257,7 @@ int x86_emul_blk(
 
     case blk_fxrstor:
     {
-        struct x86_fxsr *fxsr = FXSAVE_AREA;
+        struct x86_fxsr *fxsr = MAP_FXSAVE_AREA();
 
         ASSERT(!data);
         ASSERT(bytes == sizeof(*fxsr));
@@ -277,18 +280,26 @@ int x86_emul_blk(
                               (unsigned int)offsetof(struct x86_fxsr, rsvd)));
         memset(fxsr->rsvd, 0, sizeof(*fxsr) - offsetof(struct x86_fxsr, rsvd));
 
-        generate_exception_if(fxsr->mxcsr & ~mxcsr_mask, X86_EXC_GP, 0);
+        if ( fxsr->mxcsr & ~mxcsr_mask )
+        {
+            UNMAP_FXSAVE_AREA(fxsr);
+            generate_exception(X86_EXC_GP, 0);
+        }
 
         if ( s->rex_prefix & REX_W )
             asm volatile ( "fxrstorq %0" :: "m" (*fxsr) );
         else
             asm volatile ( "fxrstor %0" :: "m" (*fxsr) );
+
+        UNMAP_FXSAVE_AREA(fxsr);
+
         break;
     }
 
     case blk_fxsave:
     {
-        struct x86_fxsr *fxsr = FXSAVE_AREA;
+        struct x86_fxsr *fxsr = MAP_FXSAVE_AREA();
+        struct x86_fxsr *dst = fxsr;
 
         ASSERT(!data);
         ASSERT(bytes == sizeof(*fxsr));
@@ -296,17 +307,20 @@ int x86_emul_blk(
 
         if ( s->op_bytes < sizeof(*fxsr) )
             /* Don't chance consuming uninitialized data. */
-            memset(fxsr, 0, s->op_bytes);
+            memset(dst, 0, s->op_bytes);
         else
-            fxsr = ptr;
+            dst = ptr;
 
         if ( s->rex_prefix & REX_W )
-            asm volatile ( "fxsaveq %0" : "=m" (*fxsr) );
+            asm volatile ( "fxsaveq %0" : "=m" (*dst) );
         else
-            asm volatile ( "fxsave %0" : "=m" (*fxsr) );
+            asm volatile ( "fxsave %0" : "=m" (*dst) );
+
+        if ( dst != ptr ) /* i.e. s->op_bytes < sizeof(*fxsr) */
+            memcpy(ptr, dst, s->op_bytes);
+
+        UNMAP_FXSAVE_AREA(fxsr);
 
-        if ( fxsr != ptr ) /* i.e. s->op_bytes < sizeof(*fxsr) */
-            memcpy(ptr, fxsr, s->op_bytes);
         break;
     }
 
-- 
2.56.0