x86emul: conditionally clear BNDn for branches

Considering that we surface MPX to HVM guests, instructions we emulate
should also correctly deal with MPX state. While for now BND*
instructions don't get emulated, the effect of branches (which we do
emulate) without BND prefix should be taken care of.

No need to alter XABORT behavior: While not mentioned in the SDM so
far, this restores BNDn as they were at the XBEGIN, and since we make
XBEGIN abort right away, XABORT in the emulator is only a no-op.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff --git a/tools/tests/x86_emulator/x86_emulate.c b/tools/tests/x86_emulator/x86_emulate.c
index 2e8dfbfea0eacd8fabb67e549c0547871a4efd11..48f5d64877a9d144c9f4c07ba2ef3d36de7d895c 100644 (file)
--- a/tools/tests/x86_emulator/x86_emulate.c
+++ b/tools/tests/x86_emulator/x86_emulate.c
@@ -7,6 +7,9 @@
 
 #define cpu_has_amd_erratum(nr) 0
 #define mark_regs_dirty(r) ((void)(r))
+#define cpu_has_mpx false
+#define read_bndcfgu() 0
+#define xstate_set_init(what)
 
 /* For generic assembly code: use macros to define operation/operand sizes. */
 #ifdef __i386__

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 369a83aebd987a9160d7076f36f1ac681ab0814d..56669fccc7f0bf1670c882c653b9155c29d08336 100644 (file)
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -391,6 +391,8 @@ int vcpu_initialise(struct vcpu *v)
 
         vmce_init_vcpu(v);
     }
+    else if ( (rc = xstate_alloc_save_area(v)) != 0 )
+        return rc;
 
     spin_lock_init(&v->arch.vpmu.vpmu_lock);
 

diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index 45cdc248668a6b3c13445f09691ab045868dd753..490eec99023301591520804d2c7875edad20d7fa 100644 (file)
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -417,6 +417,9 @@ typedef union {
 #define MSR_SYSENTER_EIP 0x00000176
 #define MSR_DEBUGCTL     0x000001d9
 #define DEBUGCTL_BTF     (1 << 1)
+#define MSR_BNDCFGS      0x00000d90
+#define BNDCFG_ENABLE    (1 << 0)
+#define BNDCFG_PRESERVE  (1 << 1)
 #define MSR_EFER         0xc0000080
 #define MSR_STAR         0xc0000081
 #define MSR_LSTAR        0xc0000082
@@ -1314,6 +1317,7 @@ static bool vcpu_has(
 #define vcpu_has_bmi1()        vcpu_has(         7, EBX,  3, ctxt, ops)
 #define vcpu_has_hle()         vcpu_has(         7, EBX,  4, ctxt, ops)
 #define vcpu_has_rtm()         vcpu_has(         7, EBX, 11, ctxt, ops)
+#define vcpu_has_mpx()         vcpu_has(         7, EBX, 14, ctxt, ops)
 #define vcpu_has_smap()        vcpu_has(         7, EBX, 20, ctxt, ops)
 #define vcpu_has_clflushopt()  vcpu_has(         7, EBX, 23, ctxt, ops)
 #define vcpu_has_clwb()        vcpu_has(         7, EBX, 24, ctxt, ops)
@@ -1836,6 +1840,34 @@ static int inject_swint(enum x86_swint_type type,
     generate_exception(fault_type, error_code);
 }
 
+static void adjust_bnd(struct x86_emulate_ctxt *ctxt,
+                       const struct x86_emulate_ops *ops, enum vex_pfx pfx)
+{
+    uint64_t bndcfg;
+    int rc;
+
+    if ( pfx == vex_f2 || !cpu_has_mpx || !vcpu_has_mpx() )
+        return;
+
+    if ( !mode_ring0() )
+        bndcfg = read_bndcfgu();
+    else if ( !ops->read_msr ||
+              ops->read_msr(MSR_BNDCFGS, &bndcfg, ctxt) != X86EMUL_OKAY )
+        return;
+    if ( (bndcfg & BNDCFG_ENABLE) && !(bndcfg & BNDCFG_PRESERVE) )
+    {
+        /*
+         * Using BNDMK or any other MPX instruction here is pointless, as
+         * we run with MPX disabled ourselves, and hence they're all no-ops.
+         * Therefore we have two ways to clear BNDn: Enable MPX temporarily
+         * (in which case executing any suitable non-prefixed branch
+         * instruction would do), or use XRSTOR.
+         */
+        xstate_set_init(XSTATE_BNDREGS);
+    }
+ done:;
+}
+
 int x86emul_unhandleable_rw(
     enum x86_segment seg,
     unsigned long offset,
@@ -3072,6 +3104,7 @@ x86_emulate(
     case 0x70 ... 0x7f: /* jcc (short) */
         if ( test_cc(b, _regs._eflags) )
             jmp_rel((int32_t)src.val);
+        adjust_bnd(ctxt, ops, vex.pfx);
         break;
 
     case 0x82: /* Grp1 (x86/32 only) */
@@ -3424,6 +3457,7 @@ x86_emulate(
              (rc = ops->insn_fetch(x86_seg_cs, dst.val, NULL, 0, ctxt)) )
             goto done;
         _regs.r(ip) = dst.val;
+        adjust_bnd(ctxt, ops, vex.pfx);
         break;
 
     case 0xc4: /* les */
@@ -4137,12 +4171,15 @@ x86_emulate(
         op_bytes = ((op_bytes == 4) && mode_64bit()) ? 8 : op_bytes;
         src.val = _regs.r(ip);
         jmp_rel(rel);
+        adjust_bnd(ctxt, ops, vex.pfx);
         goto push;
     }
 
     case 0xe9: /* jmp (near) */
     case 0xeb: /* jmp (short) */
         jmp_rel((int32_t)src.val);
+        if ( !(b & 2) )
+            adjust_bnd(ctxt, ops, vex.pfx);
         break;
 
     case 0xea: /* jmp (far, absolute) */
@@ -4402,12 +4439,14 @@ x86_emulate(
                 goto done;
             _regs.r(ip) = src.val;
             src.val = dst.val;
+            adjust_bnd(ctxt, ops, vex.pfx);
             goto push;
         case 4: /* jmp (near) */
             if ( (rc = ops->insn_fetch(x86_seg_cs, src.val, NULL, 0, ctxt)) )
                 goto done;
             _regs.r(ip) = src.val;
             dst.type = OP_NONE;
+            adjust_bnd(ctxt, ops, vex.pfx);
             break;
         case 3: /* call (far, absolute indirect) */
         case 5: /* jmp (far, absolute indirect) */
@@ -5281,6 +5320,7 @@ x86_emulate(
     case X86EMUL_OPC(0x0f, 0x80) ... X86EMUL_OPC(0x0f, 0x8f): /* jcc (near) */
         if ( test_cc(b, _regs._eflags) )
             jmp_rel((int32_t)src.val);
+        adjust_bnd(ctxt, ops, vex.pfx);
         break;
 
     case X86EMUL_OPC(0x0f, 0x90) ... X86EMUL_OPC(0x0f, 0x9f): /* setcc */

diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 85a0116f5ec55af8d4361f2151b95a2dfd0e08ce..6073e1d3ace985905154e9d434557210c433f642 100644 (file)
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -496,15 +496,33 @@ bool_t xsave_enabled(const struct vcpu *v)
 int xstate_alloc_save_area(struct vcpu *v)
 {
     struct xsave_struct *save_area;
+    unsigned int size;
 
-    if ( !cpu_has_xsave || is_idle_vcpu(v) )
+    if ( !cpu_has_xsave )
         return 0;
 
-    BUG_ON(xsave_cntxt_size < XSTATE_AREA_MIN_SIZE);
+    if ( !is_idle_vcpu(v) || !cpu_has_xsavec )
+    {
+        size = xsave_cntxt_size;
+        BUG_ON(size < XSTATE_AREA_MIN_SIZE);
+    }
+    else
+    {
+        /*
+         * For idle vcpus on XSAVEC-capable CPUs allocate an area large
+         * enough to save any individual extended state.
+         */
+        unsigned int i;
+
+        for ( size = 0, i = 2; i < xstate_features; ++i )
+            if ( size < xstate_sizes[i] )
+                size = xstate_sizes[i];
+        size += XSTATE_AREA_MIN_SIZE;
+    }
 
     /* XSAVE/XRSTOR requires the save area be 64-byte-boundary aligned. */
     BUILD_BUG_ON(__alignof(*save_area) < 64);
-    save_area = _xzalloc(xsave_cntxt_size, __alignof(*save_area));
+    save_area = _xzalloc(size, __alignof(*save_area));
     if ( save_area == NULL )
         return -ENOMEM;
 
@@ -723,6 +741,67 @@ int handle_xsetbv(u32 index, u64 new_bv)
     return 0;
 }
 
+uint64_t read_bndcfgu(void)
+{
+    unsigned long cr0 = read_cr0();
+    struct xsave_struct *xstate
+        = idle_vcpu[smp_processor_id()]->arch.xsave_area;
+    const struct xstate_bndcsr *bndcsr;
+
+    ASSERT(cpu_has_mpx);
+    clts();
+
+    if ( cpu_has_xsavec )
+    {
+        asm ( ".byte 0x0f,0xc7,0x27\n" /* xsavec */
+              : "=m" (*xstate)
+              : "a" (XSTATE_BNDCSR), "d" (0), "D" (xstate) );
+
+        bndcsr = (void *)(xstate + 1);
+    }
+    else
+    {
+        asm ( ".byte 0x0f,0xae,0x27\n" /* xsave */
+              : "=m" (*xstate)
+              : "a" (XSTATE_BNDCSR), "d" (0), "D" (xstate) );
+
+        bndcsr = (void *)xstate + xstate_offsets[_XSTATE_BNDCSR];
+    }
+
+    if ( cr0 & X86_CR0_TS )
+        write_cr0(cr0);
+
+    return xstate->xsave_hdr.xstate_bv & XSTATE_BNDCSR ? bndcsr->bndcfgu : 0;
+}
+
+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;
+
+    if ( ~xfeature_mask & mask )
+    {
+        ASSERT_UNREACHABLE();
+        return;
+    }
+
+    if ( (~xcr0 & mask) && !set_xcr0(xcr0 | mask) )
+        return;
+
+    clts();
+
+    memset(&xstate->xsave_hdr, 0, sizeof(xstate->xsave_hdr));
+    xrstor(v, mask);
+
+    if ( cr0 & X86_CR0_TS )
+        write_cr0(cr0);
+
+    if ( (~xcr0 & mask) && !set_xcr0(xcr0) )
+        BUG();
+}
+
 /*
  * Local variables:
  * mode: C

diff --git a/xen/include/asm-x86/xstate.h b/xen/include/asm-x86/xstate.h
index 45b2a9bf03305afb70504e5bb30aabcb0557e760..a3d37b8b5e9dafcc8c844784ae54ee350ca71dd6 100644 (file)
--- a/xen/include/asm-x86/xstate.h
+++ b/xen/include/asm-x86/xstate.h
@@ -99,13 +99,20 @@ struct __attribute__((aligned (64))) xsave_struct
     char data[];                             /* Variable layout states */
 };
 
+struct xstate_bndcsr {
+    uint64_t bndcfgu;
+    uint64_t bndstatus;
+};
+
 /* extended state operations */
 bool_t __must_check set_xcr0(u64 xfeatures);
 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 xrstor(struct vcpu *v, uint64_t mask);
+void xstate_set_init(uint64_t mask);
 bool_t xsave_enabled(const struct vcpu *v);
 int __must_check validate_xstate(u64 xcr0, u64 xcr0_accum,
                                  const struct xsave_hdr *);