On 4/9/24 07:02, Richard Henderson wrote:
Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
---
  linux-user/i386/signal.c | 19 ++++++++++++++++++-
  1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/linux-user/i386/signal.c b/linux-user/i386/signal.c
index d015fe520a..fd09c973d4 100644
--- a/linux-user/i386/signal.c
+++ b/linux-user/i386/signal.c
@@ -612,6 +612,7 @@ static bool xrstor_sigcontext(CPUX86State *env, FPStateKind 
fpkind,
      struct target_fpx_sw_bytes *sw = (void *)&fxstate->sw_reserved;
      uint32_t magic1, magic2;
      uint32_t extended_size, xstate_size, min_size, max_size;
+    uint64_t xfeatures;
switch (fpkind) {
      case FPSTATE_XSAVE:
@@ -628,10 +629,25 @@ static bool xrstor_sigcontext(CPUX86State *env, 
FPStateKind fpkind,
              xstate_size > extended_size) {
              break;
          }
+
+        /*
+         * Restore the features indicated in the frame, masked by
+         * those currently enabled.  Re-check the frame size.
+         * ??? It is not clear where the kernel does this, but it
+         * is not in check_xstate_in_sigframe, and so (probably)
+         * does not fall back to fxrstor.
+         */

I think you're referring to this in __fpu_restore_sig?

        if (use_xsave()) {
                /*
                 * Remove all UABI feature bits not set in user_xfeatures
                 * from the memory xstate header which makes the full
                 * restore below bring them into init state. This works for
                 * fx_only mode as well because that has only FP and SSE
                 * set in user_xfeatures.
                 *
                 * Preserve supervisor states!
                 */
                u64 mask = user_xfeatures | xfeatures_mask_supervisor();

                fpregs->xsave.header.xfeatures &= mask;
                success = !os_xrstor_safe(fpu->fpstate,
                                          fpu_kernel_cfg.max_features);

It is not masking against the user process's xcr0, but qemu-user's xcr0
is effectively user_xfeatures (it's computed in x86_cpu_reset_hold() and
will never change afterwards since XSETBV is privileged).

Paolo

+        xfeatures = tswap64(sw->xfeatures) & env->xcr0;
+        min_size = xsave_area_size(xfeatures, false);
+        if (xstate_size < min_size) {
+            return false;
+        }
+
          if (!access_ok(env_cpu(env), VERIFY_READ, fxstate_addr,
                         xstate_size + TARGET_FP_XSTATE_MAGIC2_SIZE)) {
              return false;
          }
+
          /*
           * Check for the presence of second magic word at the end of memory
           * layout. This detects the case where the user just copied the legacy
@@ -644,7 +660,8 @@ static bool xrstor_sigcontext(CPUX86State *env, FPStateKind 
fpkind,
          if (magic2 != FP_XSTATE_MAGIC2) {
              break;
          }
-        cpu_x86_xrstor(env, fxstate_addr, -1);
+
+        cpu_x86_xrstor(env, fxstate_addr, xfeatures);
          return true;
default:


Reply via email to