[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 05/30] x86emul: replace earlier relaxation of decode_gpr()


  • To: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Wed, 11 Nov 2026 15:22:31 +0100
  • Authentication-results: eu.smtp.expurgate.cloud; dkim=pass header.s=google header.d=suse.com header.i="@suse.com" header.h="Content-Transfer-Encoding:Content-Type:In-Reply-To:Autocrypt:Content-Language:References:Cc:To:From:Subject:User-Agent:MIME-Version:Date:Message-ID"
  • Autocrypt: addr=jbeulich@xxxxxxxx; keydata= xsDiBFk3nEQRBADAEaSw6zC/EJkiwGPXbWtPxl2xCdSoeepS07jW8UgcHNurfHvUzogEq5xk hu507c3BarVjyWCJOylMNR98Yd8VqD9UfmX0Hb8/BrA+Hl6/DB/eqGptrf4BSRwcZQM32aZK 7Pj2XbGWIUrZrd70x1eAP9QE3P79Y2oLrsCgbZJfEwCgvz9JjGmQqQkRiTVzlZVCJYcyGGsD /0tbFCzD2h20ahe8rC1gbb3K3qk+LpBtvjBu1RY9drYk0NymiGbJWZgab6t1jM7sk2vuf0Py O9Hf9XBmK0uE9IgMaiCpc32XV9oASz6UJebwkX+zF2jG5I1BfnO9g7KlotcA/v5ClMjgo6Gl MDY4HxoSRu3i1cqqSDtVlt+AOVBJBACrZcnHAUSuCXBPy0jOlBhxPqRWv6ND4c9PH1xjQ3NP nxJuMBS8rnNg22uyfAgmBKNLpLgAGVRMZGaGoJObGf72s6TeIqKJo/LtggAS9qAUiuKVnygo 3wjfkS9A3DRO+SpU7JqWdsveeIQyeyEJ/8PTowmSQLakF+3fote9ybzd880fSmFuIEJldWxp Y2ggPGpiZXVsaWNoQHN1c2UuY29tPsJgBBMRAgAgBQJZN5xEAhsDBgsJCAcDAgQVAggDBBYC AwECHgECF4AACgkQoDSui/t3IH4J+wCfQ5jHdEjCRHj23O/5ttg9r9OIruwAn3103WUITZee e7Sbg12UgcQ5lv7SzsFNBFk3nEQQCACCuTjCjFOUdi5Nm244F+78kLghRcin/awv+IrTcIWF hUpSs1Y91iQQ7KItirz5uwCPlwejSJDQJLIS+QtJHaXDXeV6NI0Uef1hP20+y8qydDiVkv6l IreXjTb7DvksRgJNvCkWtYnlS3mYvQ9NzS9PhyALWbXnH6sIJd2O9lKS1Mrfq+y0IXCP10eS FFGg+Av3IQeFatkJAyju0PPthyTqxSI4lZYuJVPknzgaeuJv/2NccrPvmeDg6Coe7ZIeQ8Yj t0ARxu2xytAkkLCel1Lz1WLmwLstV30g80nkgZf/wr+/BXJW/oIvRlonUkxv+IbBM3dX2OV8 AmRv1ySWPTP7AAMFB/9PQK/VtlNUJvg8GXj9ootzrteGfVZVVT4XBJkfwBcpC/XcPzldjv+3 HYudvpdNK3lLujXeA5fLOH+Z/G9WBc5pFVSMocI71I8bT8lIAzreg0WvkWg5V2WZsUMlnDL9 mpwIGFhlbM3gfDMs7MPMu8YQRFVdUvtSpaAs8OFfGQ0ia3LGZcjA6Ik2+xcqscEJzNH+qh8V m5jjp28yZgaqTaRbg3M/+MTbMpicpZuqF4rnB0AQD12/3BNWDR6bmh+EkYSMcEIpQmBM51qM EKYTQGybRCjpnKHGOxG0rfFY1085mBDZCH5Kx0cl0HVJuQKC+dV2ZY5AqjcKwAxpE75MLFkr wkkEGBECAAkFAlk3nEQCGwwACgkQoDSui/t3IH7nnwCfcJWUDUFKdCsBH/E5d+0ZnMQi+G0A nAuWpQkjM1ASeQwSHEeAWPgskBQL
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Teddy Astie <teddy.astie@xxxxxxxxxx>, Roger Pau Monné <roger@xxxxxxxxxxxxxx>
  • Delivery-date: Wed, 11 Nov 2026 14:22:37 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Handle eGPR-s in a new out-of-line helper, while restoring some code to
the state before commit e8af03bf9a32 ("x86emul: prepare for AVX512F S/G
insns"). Some of the function invocations are "just in case"; to allow
these to continue to work without adding logic to tell whether for a
particular insn a particular operand is a GPR or another knd of
register, record register numbers in the new structure in all cases,
while fetching register values only when eGPR-s are actually enabled.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
The almost-pure C variant of the register latching would be something
like

           switch ( reg )
           {
#define CASE(regno, basereg)                                          \
           case regno:                                                \
               /* REX2 prefixes with W, R4 and possibly R3 set. */    \
               asm ( ".byte 0xd5, 0x48 | %c1; mov %%" #basereg ", %0" \
                     : "+m" (egpr->val[i])                            \
                     : "i" ((regno & 8) >> 1) )

           CASE(16, eax); break;
           CASE(17, ecx); break;
           CASE(18, edx); break;
           CASE(19, ebx); break;
           CASE(20, esp); break;
           CASE(21, ebp); break;
           CASE(22, esi); break;
           CASE(23, edi); break;
           CASE(24, eax); break;
           CASE(25, ecx); break;
           CASE(26, edx); break;
           CASE(27, ebx); break;
           CASE(28, esp); break;
           CASE(29, ebp); break;
           CASE(30, esi); break;
           CASE(31, edi); break;
#undef CASE

           default:
               ASSERT_UNREACHABLE();
               break;
           }

which, because of the way the compiler translates the switch(), ends up
larger without (imo) being that much easier to read.

--- a/tools/fuzz/x86_instruction_emulator/Makefile
+++ b/tools/fuzz/x86_instruction_emulator/Makefile
@@ -32,7 +32,7 @@ GCOV_FLAGS := --coverage
 
 OBJS := fuzz-emul.o x86-emulate.o
 OBJS += x86_emulate/0f01.o x86_emulate/0fae.o x86_emulate/0fc7.o
-OBJS += x86_emulate/decode.o x86_emulate/fpu.o
+OBJS += x86_emulate/decode.o x86_emulate/fpu.o x86_emulate/util.o
 
 WRAPPED = $(shell sed -n 's,^ *WRAP(\([[:alnum:]_]*\));,\1,p' \
                       ../../tests/x86_emulator/x86-emulate.h)
--- a/tools/tests/x86_emulator/x86-emulate.h
+++ b/tools/tests/x86_emulator/x86-emulate.h
@@ -143,6 +143,8 @@ static inline uint64_t xgetbv(uint32_t x
     return ((uint64_t)hi << 32) | lo;
 }
 
+#define get_xcr0() xgetbv(0)
+
 /* Intentionally checking OSXSAVE here. */
 #define cpu_has_xsave     (cpu_policy.basic.raw[1].c & (1u << 27))
 
@@ -225,6 +227,8 @@ void wrpkru(unsigned int val);
                                      xcr0_mask(0xe6))
 #define cpu_has_avx10_2_aux         (cpu_policy.avx10.avx10_v2_aux && \
                                      xcr0_mask(0xe6))
+#define cpu_has_apx_f               (cpu_policy.feat.apx_f && \
+                                     xcr0_mask(X86_XCR0_APX))
 
 #define cpu_has_xgetbv1             (cpu_has_xsave && 
cpu_policy.xstate.xgetbv1)
 
--- a/xen/arch/x86/x86_emulate/util.c
+++ b/xen/arch/x86/x86_emulate/util.c
@@ -7,6 +7,74 @@
  */
 
 #include "private.h"
+#ifdef __XEN__
+#include <asm/xstate.h>
+#endif
+
+unsigned long *decode_egpr(struct egpr *egpr, unsigned int reg)
+{
+    unsigned int i;
+
+    for ( i = 0; i < ARRAY_SIZE(egpr->regno); ++i )
+    {
+       unsigned int xcr0 = 0;
+
+       if ( egpr->regno[i] == reg )
+           break;
+
+       if ( egpr->regno[i] )
+           continue;
+
+       egpr->regno[i] = reg;
+
+       /*
+        * Obtain the active XCR0 here, for not having emulation context or an
+        * ops structure available, as this is a utility function used also
+        * from outside of the emulator.
+        */
+       if ( cpu_has_apx_f )
+           xcr0 = get_xcr0();
+
+       /* Fetch the register value if accessible. */
+       if ( xcr0 & X86_XCR0_APX )
+       {
+#ifdef __x86_64__
+           asm ( "sub  $17, %[reg]\n\t"
+
+                 /* mov %r16, (%rdi) */
+                 ".byte 0xd5, 0x48; mov %%eax, (%%rdi)\n\t"
+
+                 /* cfcmovz %r17, (%rdi) */
+                 ".byte 0x62, 0xe4, 0xfc, 0x0c, 0x44, 0x0f\n\t"
+
+                 ".irp n, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 
30\n\t"
+
+                 "dec  %[reg]\n\t"
+
+                 /* cfcmovz %r\n, (%rdi) */
+                 ".byte 0x62, 0x64 + ((~\\n & 8) << 4), 0xfc, 0x0c\n\t"
+                 ".byte 0x44, 0x07 + ((\\n & 7) << 3)\n\t"
+
+                 ".endr\n\t"
+
+                 /* cfcmovg %r31, (%rdi) */
+                 ".byte 0x62, 0x64, 0xfc, 0x0c, 0x4f, 0x3f"
+
+                 : [reg] "+r" (reg), "=m" (egpr->val[i])
+                 : "D" (&egpr->val[i]) );
+
+           egpr->orig[i] = egpr->val[i];
+#endif
+       }
+
+       break;
+    }
+
+    ASSERT(i < ARRAY_SIZE(egpr->val));
+
+    /* Note that this also acts as array_access_nospec() stand-in. */
+    return &egpr->val[i & (ARRAY_SIZE(egpr->val) - 1)];
+}
 
 unsigned int x86_insn_length(const struct x86_emulate_state *s,
                              const struct x86_emulate_ctxt *ctxt)
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -634,27 +634,36 @@ int x86_emulate_wrapper(
 /* Map GPRs by ModRM encoding to their offset within struct cpu_user_regs. */
 extern const uint8_t cpu_user_regs_gpr_offsets[X86_NR_GPRS];
 
+/*
+ * APX insn can reference up to 4 GPRs: Either up to three register operands
+ * or up to two register operands and one memory operand, the latter using
+ * potentially both base and index register.
+ */
 struct egpr {
+       unsigned char regno[4];
+       unsigned long val[4], orig[4];
 };
 
+unsigned long *decode_egpr(struct egpr *egpr, unsigned int reg);
+
 /*
- * Given the 'reg' portion of a ModRM byte, and a register block, return a
- * pointer into the block that addresses the relevant register.
+ * Given the 'reg' portion of a ModRM byte, and a pair of register blocks,
+ * return a pointer into either block that addresses the relevant register.
  */
 static inline unsigned long *decode_gpr(struct cpu_user_regs *regs,
                                         struct egpr *egpr,
                                         unsigned int reg)
 {
+    if ( reg & 0x10 )
+        return decode_egpr(egpr, reg);
+
     /* Check that the array is a power of two. */
     BUILD_BUG_ON(ARRAY_SIZE(cpu_user_regs_gpr_offsets) &
                  (ARRAY_SIZE(cpu_user_regs_gpr_offsets) - 1));
 
-    /*
-     * Note that this also acts as array_access_nospec() stand-in.  Higher
-     * bits may legitimately come in set here, from EVEX decoding, and
-     * hence truncation is what we want (bits not ignored will get checked
-     * elsewhere).
-     */
+    ASSERT(reg < ARRAY_SIZE(cpu_user_regs_gpr_offsets));
+
+    /* Note that this also acts as array_access_nospec() stand-in. */
     reg &= ARRAY_SIZE(cpu_user_regs_gpr_offsets) - 1;
 
     return (void *)regs + cpu_user_regs_gpr_offsets[reg];




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.