* [PATCH v5 0/4] x86emul: further misc improvements
@ 2017-01-12 14:08 Jan Beulich
2017-01-12 14:15 ` [PATCH v5 1/4] x86emul: conditionally clear BNDn for branches Jan Beulich
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Jan Beulich @ 2017-01-12 14:08 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
1: conditionally clear BNDn for branches
2: support VME and PVI
3: use switch()-wide local variable 'cr4'
4: improve CR/DR access handling
Signed-off-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 1/4] x86emul: conditionally clear BNDn for branches
2017-01-12 14:08 [PATCH v5 0/4] x86emul: further misc improvements Jan Beulich
@ 2017-01-12 14:15 ` Jan Beulich
2017-01-12 14:28 ` Andrew Cooper
2017-01-12 14:15 ` [PATCH v5 2/4] x86emul: support VME and PVI Jan Beulich
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2017-01-12 14:15 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 9449 bytes --]
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>
---
v5: Add cpu_has_mpx check to adjust_bnd().
v4: Re-base. Rename clear_bnd() to adjust_bnd(). Add
ASSERT_UNREACHABLE() to xstate_set_init(), and consistently use
set_xcr0() instead of xsetbv() there. Drop use of XSAVEOPT in
read_bndcfgu().
v3: Re-base.
v2: Re-base. Address all RFC reasons based on feedback from Intel.
Re-work the actual clearing of BNDn.
--- 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__
--- 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);
--- 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_t
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 */
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -496,15 +496,33 @@ bool_t xsave_enabled(const struct vcpu *
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
--- a/xen/include/asm-x86/xstate.h
+++ b/xen/include/asm-x86/xstate.h
@@ -99,13 +99,20 @@ struct __attribute__((aligned (64))) xsa
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 *);
[-- Attachment #2: x86emul-clear-bnd-on-branch.patch --]
[-- Type: text/plain, Size: 9495 bytes --]
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>
---
v5: Add cpu_has_mpx check to adjust_bnd().
v4: Re-base. Rename clear_bnd() to adjust_bnd(). Add
ASSERT_UNREACHABLE() to xstate_set_init(), and consistently use
set_xcr0() instead of xsetbv() there. Drop use of XSAVEOPT in
read_bndcfgu().
v3: Re-base.
v2: Re-base. Address all RFC reasons based on feedback from Intel.
Re-work the actual clearing of BNDn.
--- 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__
--- 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);
--- 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_t
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 */
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -496,15 +496,33 @@ bool_t xsave_enabled(const struct vcpu *
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
--- a/xen/include/asm-x86/xstate.h
+++ b/xen/include/asm-x86/xstate.h
@@ -99,13 +99,20 @@ struct __attribute__((aligned (64))) xsa
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 *);
[-- Attachment #3: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 2/4] x86emul: support VME and PVI
2017-01-12 14:08 [PATCH v5 0/4] x86emul: further misc improvements Jan Beulich
2017-01-12 14:15 ` [PATCH v5 1/4] x86emul: conditionally clear BNDn for branches Jan Beulich
@ 2017-01-12 14:15 ` Jan Beulich
2017-01-12 14:37 ` Andrew Cooper
2017-01-12 14:16 ` [PATCH v5 3/4] x86emul: use switch()-wide local variable 'cr4' Jan Beulich
2017-01-12 14:16 ` [PATCH v5 4/4] x86emul: improve CR/DR access handling Jan Beulich
3 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2017-01-12 14:15 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 5079 bytes --]
... affecting PUSHF, POPF, CLI, and STI.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v5: Add PUSHF adjustment. mode_pvi() -> mode_vif().
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -433,6 +433,8 @@ typedef union {
#define CR0_EM (1<<2)
#define CR0_TS (1<<3)
+#define CR4_VME (1<<0)
+#define CR4_PVI (1<<1)
#define CR4_TSD (1<<2)
#define CR4_OSFXSR (1<<9)
#define CR4_OSXMMEXCPT (1<<10)
@@ -1180,6 +1182,15 @@ _mode_iopl(
fail_if(_iopl < 0); \
_iopl; \
})
+#define mode_vif() ({ \
+ unsigned long cr4 = 0; \
+ if ( ops->read_cr && get_cpl(ctxt, ops) == 3 ) \
+ { \
+ rc = ops->read_cr(4, &cr4, ctxt); \
+ if ( rc != X86EMUL_OKAY ) goto done; \
+ } \
+ !!(cr4 & (_regs._eflags & EFLG_VM ? CR4_VME : CR4_PVI)); \
+})
static int ioport_access_check(
unsigned int first_port,
@@ -3267,20 +3278,44 @@ x86_emulate(
break;
case 0x9c: /* pushf */
- generate_exception_if((_regs._eflags & EFLG_VM) &&
- MASK_EXTR(_regs._eflags, EFLG_IOPL) != 3,
- EXC_GP, 0);
- src.val = _regs.r(flags) & ~(EFLG_VM | EFLG_RF);
+ if ( (_regs._eflags & EFLG_VM) &&
+ MASK_EXTR(_regs._eflags, EFLG_IOPL) != 3 )
+ {
+ unsigned long cr4 = 0;
+
+ if ( op_bytes == 2 && ops->read_cr )
+ {
+ rc = ops->read_cr(4, &cr4, ctxt);
+ if ( rc != X86EMUL_OKAY )
+ goto done;
+ }
+ generate_exception_if(!(cr4 & CR4_VME), EXC_GP, 0);
+ src.val = (_regs.flags & ~EFLG_IF) | EFLG_IOPL;
+ if ( _regs._eflags & EFLG_VIF )
+ src.val |= EFLG_IF;
+ }
+ else
+ src.val = _regs.r(flags) & ~(EFLG_VM | EFLG_RF);
goto push;
case 0x9d: /* popf */ {
uint32_t mask = EFLG_VIP | EFLG_VIF | EFLG_VM;
+ unsigned long cr4 = 0;
if ( !mode_ring0() )
{
- generate_exception_if((_regs._eflags & EFLG_VM) &&
- MASK_EXTR(_regs._eflags, EFLG_IOPL) != 3,
- EXC_GP, 0);
+ if ( _regs._eflags & EFLG_VM )
+ {
+ if ( op_bytes == 2 && ops->read_cr )
+ {
+ rc = ops->read_cr(4, &cr4, ctxt);
+ if ( rc != X86EMUL_OKAY )
+ goto done;
+ }
+ generate_exception_if(!(cr4 & CR4_VME) &&
+ MASK_EXTR(_regs._eflags, EFLG_IOPL) != 3,
+ EXC_GP, 0);
+ }
mask |= EFLG_IOPL;
if ( !mode_iopl() )
mask |= EFLG_IF;
@@ -3292,7 +3327,20 @@ x86_emulate(
&dst.val, op_bytes, ctxt, ops)) != 0 )
goto done;
if ( op_bytes == 2 )
+ {
dst.val = (uint16_t)dst.val | (_regs._eflags & 0xffff0000u);
+ if ( cr4 & CR4_VME )
+ {
+ if ( dst.val & EFLG_IF )
+ {
+ generate_exception_if(_regs._eflags & EFLG_VIP, EXC_GP, 0);
+ dst.val |= EFLG_VIF;
+ }
+ else
+ dst.val &= ~EFLG_VIF;
+ mask &= ~EFLG_VIF;
+ }
+ }
dst.val &= EFLAGS_MODIFIABLE;
_regs._eflags &= mask;
_regs._eflags |= (dst.val & ~mask) | EFLG_MBS;
@@ -4399,16 +4447,29 @@ x86_emulate(
break;
case 0xfa: /* cli */
- generate_exception_if(!mode_iopl(), EXC_GP, 0);
- _regs._eflags &= ~EFLG_IF;
+ if ( mode_iopl() )
+ _regs._eflags &= ~EFLG_IF;
+ else
+ {
+ generate_exception_if(!mode_vif(), EXC_GP, 0);
+ _regs._eflags &= ~EFLG_VIF;
+ }
break;
case 0xfb: /* sti */
- generate_exception_if(!mode_iopl(), EXC_GP, 0);
- if ( !(_regs._eflags & EFLG_IF) )
+ if ( mode_iopl() )
{
+ if ( !(_regs._eflags & EFLG_IF) )
+ ctxt->retire.sti = true;
_regs._eflags |= EFLG_IF;
- ctxt->retire.sti = true;
+ }
+ else
+ {
+ generate_exception_if((_regs._eflags & EFLG_VIP) || !mode_vif(),
+ EXC_GP, 0);
+ if ( !(_regs._eflags & EFLG_VIF) )
+ ctxt->retire.sti = true;
+ _regs._eflags |= EFLG_VIF;
}
break;
[-- Attachment #2: x86emul-VME-PVI.patch --]
[-- Type: text/plain, Size: 5107 bytes --]
x86emul: support VME and PVI
... affecting PUSHF, POPF, CLI, and STI.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v5: Add PUSHF adjustment. mode_pvi() -> mode_vif().
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -433,6 +433,8 @@ typedef union {
#define CR0_EM (1<<2)
#define CR0_TS (1<<3)
+#define CR4_VME (1<<0)
+#define CR4_PVI (1<<1)
#define CR4_TSD (1<<2)
#define CR4_OSFXSR (1<<9)
#define CR4_OSXMMEXCPT (1<<10)
@@ -1180,6 +1182,15 @@ _mode_iopl(
fail_if(_iopl < 0); \
_iopl; \
})
+#define mode_vif() ({ \
+ unsigned long cr4 = 0; \
+ if ( ops->read_cr && get_cpl(ctxt, ops) == 3 ) \
+ { \
+ rc = ops->read_cr(4, &cr4, ctxt); \
+ if ( rc != X86EMUL_OKAY ) goto done; \
+ } \
+ !!(cr4 & (_regs._eflags & EFLG_VM ? CR4_VME : CR4_PVI)); \
+})
static int ioport_access_check(
unsigned int first_port,
@@ -3267,20 +3278,44 @@ x86_emulate(
break;
case 0x9c: /* pushf */
- generate_exception_if((_regs._eflags & EFLG_VM) &&
- MASK_EXTR(_regs._eflags, EFLG_IOPL) != 3,
- EXC_GP, 0);
- src.val = _regs.r(flags) & ~(EFLG_VM | EFLG_RF);
+ if ( (_regs._eflags & EFLG_VM) &&
+ MASK_EXTR(_regs._eflags, EFLG_IOPL) != 3 )
+ {
+ unsigned long cr4 = 0;
+
+ if ( op_bytes == 2 && ops->read_cr )
+ {
+ rc = ops->read_cr(4, &cr4, ctxt);
+ if ( rc != X86EMUL_OKAY )
+ goto done;
+ }
+ generate_exception_if(!(cr4 & CR4_VME), EXC_GP, 0);
+ src.val = (_regs.flags & ~EFLG_IF) | EFLG_IOPL;
+ if ( _regs._eflags & EFLG_VIF )
+ src.val |= EFLG_IF;
+ }
+ else
+ src.val = _regs.r(flags) & ~(EFLG_VM | EFLG_RF);
goto push;
case 0x9d: /* popf */ {
uint32_t mask = EFLG_VIP | EFLG_VIF | EFLG_VM;
+ unsigned long cr4 = 0;
if ( !mode_ring0() )
{
- generate_exception_if((_regs._eflags & EFLG_VM) &&
- MASK_EXTR(_regs._eflags, EFLG_IOPL) != 3,
- EXC_GP, 0);
+ if ( _regs._eflags & EFLG_VM )
+ {
+ if ( op_bytes == 2 && ops->read_cr )
+ {
+ rc = ops->read_cr(4, &cr4, ctxt);
+ if ( rc != X86EMUL_OKAY )
+ goto done;
+ }
+ generate_exception_if(!(cr4 & CR4_VME) &&
+ MASK_EXTR(_regs._eflags, EFLG_IOPL) != 3,
+ EXC_GP, 0);
+ }
mask |= EFLG_IOPL;
if ( !mode_iopl() )
mask |= EFLG_IF;
@@ -3292,7 +3327,20 @@ x86_emulate(
&dst.val, op_bytes, ctxt, ops)) != 0 )
goto done;
if ( op_bytes == 2 )
+ {
dst.val = (uint16_t)dst.val | (_regs._eflags & 0xffff0000u);
+ if ( cr4 & CR4_VME )
+ {
+ if ( dst.val & EFLG_IF )
+ {
+ generate_exception_if(_regs._eflags & EFLG_VIP, EXC_GP, 0);
+ dst.val |= EFLG_VIF;
+ }
+ else
+ dst.val &= ~EFLG_VIF;
+ mask &= ~EFLG_VIF;
+ }
+ }
dst.val &= EFLAGS_MODIFIABLE;
_regs._eflags &= mask;
_regs._eflags |= (dst.val & ~mask) | EFLG_MBS;
@@ -4399,16 +4447,29 @@ x86_emulate(
break;
case 0xfa: /* cli */
- generate_exception_if(!mode_iopl(), EXC_GP, 0);
- _regs._eflags &= ~EFLG_IF;
+ if ( mode_iopl() )
+ _regs._eflags &= ~EFLG_IF;
+ else
+ {
+ generate_exception_if(!mode_vif(), EXC_GP, 0);
+ _regs._eflags &= ~EFLG_VIF;
+ }
break;
case 0xfb: /* sti */
- generate_exception_if(!mode_iopl(), EXC_GP, 0);
- if ( !(_regs._eflags & EFLG_IF) )
+ if ( mode_iopl() )
{
+ if ( !(_regs._eflags & EFLG_IF) )
+ ctxt->retire.sti = true;
_regs._eflags |= EFLG_IF;
- ctxt->retire.sti = true;
+ }
+ else
+ {
+ generate_exception_if((_regs._eflags & EFLG_VIP) || !mode_vif(),
+ EXC_GP, 0);
+ if ( !(_regs._eflags & EFLG_VIF) )
+ ctxt->retire.sti = true;
+ _regs._eflags |= EFLG_VIF;
}
break;
[-- Attachment #3: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 3/4] x86emul: use switch()-wide local variable 'cr4'
2017-01-12 14:08 [PATCH v5 0/4] x86emul: further misc improvements Jan Beulich
2017-01-12 14:15 ` [PATCH v5 1/4] x86emul: conditionally clear BNDn for branches Jan Beulich
2017-01-12 14:15 ` [PATCH v5 2/4] x86emul: support VME and PVI Jan Beulich
@ 2017-01-12 14:16 ` Jan Beulich
2017-01-12 14:16 ` [PATCH v5 4/4] x86emul: improve CR/DR access handling Jan Beulich
3 siblings, 0 replies; 7+ messages in thread
From: Jan Beulich @ 2017-01-12 14:16 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 4100 bytes --]
... rather than various smaller scope ones.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v2: Re-base over PUSHF adjustment in earlier patch.
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -862,13 +862,10 @@ do {
#define put_fpu(_fic) \
do { \
_put_fpu(); \
- if( (_fic)->exn_raised == EXC_XM && ops->read_cr ) \
- { \
- unsigned long cr4; \
- if ( (ops->read_cr(4, &cr4, ctxt) == X86EMUL_OKAY) && \
- !(cr4 & CR4_OSXMMEXCPT) ) \
- (_fic)->exn_raised = EXC_UD; \
- } \
+ if ( (_fic)->exn_raised == EXC_XM && ops->read_cr && \
+ ops->read_cr(4, &cr4, ctxt) == X86EMUL_OKAY && \
+ !(cr4 & CR4_OSXMMEXCPT) ) \
+ (_fic)->exn_raised = EXC_UD; \
generate_exception_if((_fic)->exn_raised >= 0, \
(_fic)->exn_raised); \
} while (0)
@@ -1183,7 +1180,7 @@ _mode_iopl(
_iopl; \
})
#define mode_vif() ({ \
- unsigned long cr4 = 0; \
+ cr4 = 0; \
if ( ops->read_cr && get_cpl(ctxt, ops) == 3 ) \
{ \
rc = ops->read_cr(4, &cr4, ctxt); \
@@ -2783,6 +2780,7 @@ x86_emulate(
{
enum x86_segment seg;
struct segment_register cs, sreg;
+ unsigned long cr4;
case 0x00 ... 0x05: add: /* add */
emulate_2op_SrcV("add", src, dst, _regs._eflags);
@@ -3281,8 +3279,7 @@ x86_emulate(
if ( (_regs._eflags & EFLG_VM) &&
MASK_EXTR(_regs._eflags, EFLG_IOPL) != 3 )
{
- unsigned long cr4 = 0;
-
+ cr4 = 0;
if ( op_bytes == 2 && ops->read_cr )
{
rc = ops->read_cr(4, &cr4, ctxt);
@@ -3300,8 +3297,8 @@ x86_emulate(
case 0x9d: /* popf */ {
uint32_t mask = EFLG_VIP | EFLG_VIF | EFLG_VM;
- unsigned long cr4 = 0;
+ cr4 = 0;
if ( !mode_ring0() )
{
if ( _regs._eflags & EFLG_VM )
@@ -4586,9 +4583,6 @@ x86_emulate(
#ifdef __XEN__
case 0xd1: /* xsetbv */
- {
- unsigned long cr4;
-
generate_exception_if(vex.pfx, EXC_UD);
if ( !ops->read_cr || ops->read_cr(4, &cr4, ctxt) != X86EMUL_OKAY )
cr4 = 0;
@@ -4598,7 +4592,6 @@ x86_emulate(
_regs._eax | (_regs.rdx << 32)),
EXC_GP, 0);
goto no_writeback;
- }
#endif
case 0xd4: /* vmfunc */
@@ -5126,8 +5119,8 @@ x86_emulate(
break;
case X86EMUL_OPC(0x0f, 0x31): rdtsc: /* rdtsc */ {
- unsigned long cr4;
uint64_t val;
+
if ( !mode_ring0() )
{
fail_if(ops->read_cr == NULL);
@@ -5499,9 +5492,6 @@ x86_emulate(
break;
case X86EMUL_OPC_F3(0x0f, 0xae): /* Grp15 */
- {
- unsigned long cr4;
-
fail_if(modrm_mod != 3);
generate_exception_if((modrm_reg & 4) || !mode_64bit(), EXC_UD);
fail_if(!ops->read_cr);
@@ -5536,7 +5526,6 @@ x86_emulate(
goto done;
}
break;
- }
case X86EMUL_OPC(0x0f, 0xaf): /* imul */
emulate_2op_SrcV_srcmem("imul", src, dst, _regs._eflags);
[-- Attachment #2: x86emul-fold-CR4.patch --]
[-- Type: text/plain, Size: 4147 bytes --]
x86emul: use switch()-wide local variable 'cr4'
... rather than various smaller scope ones.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v2: Re-base over PUSHF adjustment in earlier patch.
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -862,13 +862,10 @@ do {
#define put_fpu(_fic) \
do { \
_put_fpu(); \
- if( (_fic)->exn_raised == EXC_XM && ops->read_cr ) \
- { \
- unsigned long cr4; \
- if ( (ops->read_cr(4, &cr4, ctxt) == X86EMUL_OKAY) && \
- !(cr4 & CR4_OSXMMEXCPT) ) \
- (_fic)->exn_raised = EXC_UD; \
- } \
+ if ( (_fic)->exn_raised == EXC_XM && ops->read_cr && \
+ ops->read_cr(4, &cr4, ctxt) == X86EMUL_OKAY && \
+ !(cr4 & CR4_OSXMMEXCPT) ) \
+ (_fic)->exn_raised = EXC_UD; \
generate_exception_if((_fic)->exn_raised >= 0, \
(_fic)->exn_raised); \
} while (0)
@@ -1183,7 +1180,7 @@ _mode_iopl(
_iopl; \
})
#define mode_vif() ({ \
- unsigned long cr4 = 0; \
+ cr4 = 0; \
if ( ops->read_cr && get_cpl(ctxt, ops) == 3 ) \
{ \
rc = ops->read_cr(4, &cr4, ctxt); \
@@ -2783,6 +2780,7 @@ x86_emulate(
{
enum x86_segment seg;
struct segment_register cs, sreg;
+ unsigned long cr4;
case 0x00 ... 0x05: add: /* add */
emulate_2op_SrcV("add", src, dst, _regs._eflags);
@@ -3281,8 +3279,7 @@ x86_emulate(
if ( (_regs._eflags & EFLG_VM) &&
MASK_EXTR(_regs._eflags, EFLG_IOPL) != 3 )
{
- unsigned long cr4 = 0;
-
+ cr4 = 0;
if ( op_bytes == 2 && ops->read_cr )
{
rc = ops->read_cr(4, &cr4, ctxt);
@@ -3300,8 +3297,8 @@ x86_emulate(
case 0x9d: /* popf */ {
uint32_t mask = EFLG_VIP | EFLG_VIF | EFLG_VM;
- unsigned long cr4 = 0;
+ cr4 = 0;
if ( !mode_ring0() )
{
if ( _regs._eflags & EFLG_VM )
@@ -4586,9 +4583,6 @@ x86_emulate(
#ifdef __XEN__
case 0xd1: /* xsetbv */
- {
- unsigned long cr4;
-
generate_exception_if(vex.pfx, EXC_UD);
if ( !ops->read_cr || ops->read_cr(4, &cr4, ctxt) != X86EMUL_OKAY )
cr4 = 0;
@@ -4598,7 +4592,6 @@ x86_emulate(
_regs._eax | (_regs.rdx << 32)),
EXC_GP, 0);
goto no_writeback;
- }
#endif
case 0xd4: /* vmfunc */
@@ -5126,8 +5119,8 @@ x86_emulate(
break;
case X86EMUL_OPC(0x0f, 0x31): rdtsc: /* rdtsc */ {
- unsigned long cr4;
uint64_t val;
+
if ( !mode_ring0() )
{
fail_if(ops->read_cr == NULL);
@@ -5499,9 +5492,6 @@ x86_emulate(
break;
case X86EMUL_OPC_F3(0x0f, 0xae): /* Grp15 */
- {
- unsigned long cr4;
-
fail_if(modrm_mod != 3);
generate_exception_if((modrm_reg & 4) || !mode_64bit(), EXC_UD);
fail_if(!ops->read_cr);
@@ -5536,7 +5526,6 @@ x86_emulate(
goto done;
}
break;
- }
case X86EMUL_OPC(0x0f, 0xaf): /* imul */
emulate_2op_SrcV_srcmem("imul", src, dst, _regs._eflags);
[-- Attachment #3: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 4/4] x86emul: improve CR/DR access handling
2017-01-12 14:08 [PATCH v5 0/4] x86emul: further misc improvements Jan Beulich
` (2 preceding siblings ...)
2017-01-12 14:16 ` [PATCH v5 3/4] x86emul: use switch()-wide local variable 'cr4' Jan Beulich
@ 2017-01-12 14:16 ` Jan Beulich
3 siblings, 0 replies; 7+ messages in thread
From: Jan Beulich @ 2017-01-12 14:16 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 5380 bytes --]
- don't accept LOCK for DR accesses (it's undefined in the manuals)
- only accept LOCK for CR accesses when the respective feature flag is
set (which would not normally be the case for Intel)
- add (rather than or) 8 when LOCK is present; real hardware #UDs
when both REX.W and LOCK are present, implying that these would
rather access hypothetical CR16...23
- eliminate explicit decode_register() calls
- streamline remaining read/write code
No further functional change, i.e. not addressing the missing exception
generation (#UD for invalid CR/DR encodings, #GP(0) for invalid write
values, #DB for DR accesses with DR7.GD set).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -194,7 +194,8 @@ static const opcode_desc_t twobyte_table
ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM,
ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM,
/* 0x20 - 0x27 */
- ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM,
+ DstMem|SrcImplicit|ModRM, DstMem|SrcImplicit|ModRM,
+ DstImplicit|SrcMem|ModRM, DstImplicit|SrcMem|ModRM,
0, 0, 0, 0,
/* 0x28 - 0x2F */
ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM,
@@ -1320,6 +1321,7 @@ static bool vcpu_has(
#define vcpu_has_movbe() vcpu_has( 1, ECX, 22, ctxt, ops)
#define vcpu_has_avx() vcpu_has( 1, ECX, 28, ctxt, ops)
#define vcpu_has_lahf_lm() vcpu_has(0x80000001, ECX, 0, ctxt, ops)
+#define vcpu_has_cr8_legacy() vcpu_has(0x80000001, ECX, 4, ctxt, ops)
#define vcpu_has_lzcnt() vcpu_has(0x80000001, ECX, 5, ctxt, ops)
#define vcpu_has_misalignsse() vcpu_has(0x80000001, ECX, 7, ctxt, ops)
#define vcpu_has_bmi1() vcpu_has( 7, EBX, 3, ctxt, ops)
@@ -2047,6 +2049,19 @@ x86_decode_twobyte(
case 0xd0 ... 0xfe:
ctxt->opcode |= MASK_INSR(vex.pfx, X86EMUL_OPC_PFX_MASK);
break;
+
+ case 0x20: case 0x22: /* mov to/from cr */
+ if ( lock_prefix && vcpu_has_cr8_legacy() )
+ {
+ modrm_reg += 8;
+ lock_prefix = false;
+ }
+ /* fall through */
+ case 0x21: case 0x23: /* mov to/from dr */
+ generate_exception_if(lock_prefix || ea.type != OP_REG, EXC_UD);
+ op_bytes = mode_64bit() ? 8 : 4;
+ break;
+
/* Intentionally not handling here despite being modified by F3:
case 0xb8: jmpe / popcnt
case 0xbc: bsf / tzcnt
@@ -2683,14 +2698,10 @@ x86_emulate(
case DstNone: /* case DstImplicit: */
/*
* The only implicit-operands instructions allowed a LOCK prefix are
- * CMPXCHG{8,16}B, MOV CRn, MOV DRn.
+ * CMPXCHG{8,16}B (MOV CRn is being handled elsewhere).
*/
- generate_exception_if(
- lock_prefix &&
- (ext != ext_0f ||
- (((b < 0x20) || (b > 0x23)) && /* MOV CRn/DRn */
- (b != 0xc7))), /* CMPXCHG{8,16}B */
- EXC_UD);
+ generate_exception_if(lock_prefix && (ext != ext_0f || b != 0xc7),
+ EXC_UD);
dst.type = OP_NONE;
break;
@@ -5074,38 +5085,25 @@ x86_emulate(
case X86EMUL_OPC(0x0f, 0x21): /* mov dr,reg */
case X86EMUL_OPC(0x0f, 0x22): /* mov reg,cr */
case X86EMUL_OPC(0x0f, 0x23): /* mov reg,dr */
- generate_exception_if(ea.type != OP_REG, EXC_UD);
generate_exception_if(!mode_ring0(), EXC_GP, 0);
- modrm_reg |= lock_prefix << 3;
if ( b & 2 )
{
/* Write to CR/DR. */
- src.val = *(unsigned long *)decode_register(modrm_rm, &_regs, 0);
- if ( !mode_64bit() )
- src.val = (uint32_t)src.val;
- rc = ((b & 1)
- ? (ops->write_dr
- ? ops->write_dr(modrm_reg, src.val, ctxt)
- : X86EMUL_UNHANDLEABLE)
- : (ops->write_cr
- ? ops->write_cr(modrm_reg, src.val, ctxt)
- : X86EMUL_UNHANDLEABLE));
+ typeof(ops->write_cr) write = (b & 1) ? ops->write_dr
+ : ops->write_cr;
+
+ fail_if(!write);
+ rc = write(modrm_reg, src.val, ctxt);
}
else
{
/* Read from CR/DR. */
- dst.type = OP_REG;
- dst.bytes = mode_64bit() ? 8 : 4;
- dst.reg = decode_register(modrm_rm, &_regs, 0);
- rc = ((b & 1)
- ? (ops->read_dr
- ? ops->read_dr(modrm_reg, &dst.val, ctxt)
- : X86EMUL_UNHANDLEABLE)
- : (ops->read_cr
- ? ops->read_cr(modrm_reg, &dst.val, ctxt)
- : X86EMUL_UNHANDLEABLE));
+ typeof(ops->read_cr) read = (b & 1) ? ops->read_dr : ops->read_cr;
+
+ fail_if(!read);
+ rc = read(modrm_reg, &dst.val, ctxt);
}
- if ( rc != 0 )
+ if ( rc != X86EMUL_OKAY )
goto done;
break;
[-- Attachment #2: x86emul-CR-DR-improve.patch --]
[-- Type: text/plain, Size: 5418 bytes --]
x86emul: improve CR/DR access handling
- don't accept LOCK for DR accesses (it's undefined in the manuals)
- only accept LOCK for CR accesses when the respective feature flag is
set (which would not normally be the case for Intel)
- add (rather than or) 8 when LOCK is present; real hardware #UDs
when both REX.W and LOCK are present, implying that these would
rather access hypothetical CR16...23
- eliminate explicit decode_register() calls
- streamline remaining read/write code
No further functional change, i.e. not addressing the missing exception
generation (#UD for invalid CR/DR encodings, #GP(0) for invalid write
values, #DB for DR accesses with DR7.GD set).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -194,7 +194,8 @@ static const opcode_desc_t twobyte_table
ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM,
ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM,
/* 0x20 - 0x27 */
- ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM,
+ DstMem|SrcImplicit|ModRM, DstMem|SrcImplicit|ModRM,
+ DstImplicit|SrcMem|ModRM, DstImplicit|SrcMem|ModRM,
0, 0, 0, 0,
/* 0x28 - 0x2F */
ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM,
@@ -1320,6 +1321,7 @@ static bool vcpu_has(
#define vcpu_has_movbe() vcpu_has( 1, ECX, 22, ctxt, ops)
#define vcpu_has_avx() vcpu_has( 1, ECX, 28, ctxt, ops)
#define vcpu_has_lahf_lm() vcpu_has(0x80000001, ECX, 0, ctxt, ops)
+#define vcpu_has_cr8_legacy() vcpu_has(0x80000001, ECX, 4, ctxt, ops)
#define vcpu_has_lzcnt() vcpu_has(0x80000001, ECX, 5, ctxt, ops)
#define vcpu_has_misalignsse() vcpu_has(0x80000001, ECX, 7, ctxt, ops)
#define vcpu_has_bmi1() vcpu_has( 7, EBX, 3, ctxt, ops)
@@ -2047,6 +2049,19 @@ x86_decode_twobyte(
case 0xd0 ... 0xfe:
ctxt->opcode |= MASK_INSR(vex.pfx, X86EMUL_OPC_PFX_MASK);
break;
+
+ case 0x20: case 0x22: /* mov to/from cr */
+ if ( lock_prefix && vcpu_has_cr8_legacy() )
+ {
+ modrm_reg += 8;
+ lock_prefix = false;
+ }
+ /* fall through */
+ case 0x21: case 0x23: /* mov to/from dr */
+ generate_exception_if(lock_prefix || ea.type != OP_REG, EXC_UD);
+ op_bytes = mode_64bit() ? 8 : 4;
+ break;
+
/* Intentionally not handling here despite being modified by F3:
case 0xb8: jmpe / popcnt
case 0xbc: bsf / tzcnt
@@ -2683,14 +2698,10 @@ x86_emulate(
case DstNone: /* case DstImplicit: */
/*
* The only implicit-operands instructions allowed a LOCK prefix are
- * CMPXCHG{8,16}B, MOV CRn, MOV DRn.
+ * CMPXCHG{8,16}B (MOV CRn is being handled elsewhere).
*/
- generate_exception_if(
- lock_prefix &&
- (ext != ext_0f ||
- (((b < 0x20) || (b > 0x23)) && /* MOV CRn/DRn */
- (b != 0xc7))), /* CMPXCHG{8,16}B */
- EXC_UD);
+ generate_exception_if(lock_prefix && (ext != ext_0f || b != 0xc7),
+ EXC_UD);
dst.type = OP_NONE;
break;
@@ -5074,38 +5085,25 @@ x86_emulate(
case X86EMUL_OPC(0x0f, 0x21): /* mov dr,reg */
case X86EMUL_OPC(0x0f, 0x22): /* mov reg,cr */
case X86EMUL_OPC(0x0f, 0x23): /* mov reg,dr */
- generate_exception_if(ea.type != OP_REG, EXC_UD);
generate_exception_if(!mode_ring0(), EXC_GP, 0);
- modrm_reg |= lock_prefix << 3;
if ( b & 2 )
{
/* Write to CR/DR. */
- src.val = *(unsigned long *)decode_register(modrm_rm, &_regs, 0);
- if ( !mode_64bit() )
- src.val = (uint32_t)src.val;
- rc = ((b & 1)
- ? (ops->write_dr
- ? ops->write_dr(modrm_reg, src.val, ctxt)
- : X86EMUL_UNHANDLEABLE)
- : (ops->write_cr
- ? ops->write_cr(modrm_reg, src.val, ctxt)
- : X86EMUL_UNHANDLEABLE));
+ typeof(ops->write_cr) write = (b & 1) ? ops->write_dr
+ : ops->write_cr;
+
+ fail_if(!write);
+ rc = write(modrm_reg, src.val, ctxt);
}
else
{
/* Read from CR/DR. */
- dst.type = OP_REG;
- dst.bytes = mode_64bit() ? 8 : 4;
- dst.reg = decode_register(modrm_rm, &_regs, 0);
- rc = ((b & 1)
- ? (ops->read_dr
- ? ops->read_dr(modrm_reg, &dst.val, ctxt)
- : X86EMUL_UNHANDLEABLE)
- : (ops->read_cr
- ? ops->read_cr(modrm_reg, &dst.val, ctxt)
- : X86EMUL_UNHANDLEABLE));
+ typeof(ops->read_cr) read = (b & 1) ? ops->read_dr : ops->read_cr;
+
+ fail_if(!read);
+ rc = read(modrm_reg, &dst.val, ctxt);
}
- if ( rc != 0 )
+ if ( rc != X86EMUL_OKAY )
goto done;
break;
[-- Attachment #3: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 1/4] x86emul: conditionally clear BNDn for branches
2017-01-12 14:15 ` [PATCH v5 1/4] x86emul: conditionally clear BNDn for branches Jan Beulich
@ 2017-01-12 14:28 ` Andrew Cooper
0 siblings, 0 replies; 7+ messages in thread
From: Andrew Cooper @ 2017-01-12 14:28 UTC (permalink / raw)
To: Jan Beulich, xen-devel
On 12/01/17 14:15, Jan Beulich wrote:
> 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>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/4] x86emul: support VME and PVI
2017-01-12 14:15 ` [PATCH v5 2/4] x86emul: support VME and PVI Jan Beulich
@ 2017-01-12 14:37 ` Andrew Cooper
0 siblings, 0 replies; 7+ messages in thread
From: Andrew Cooper @ 2017-01-12 14:37 UTC (permalink / raw)
To: Jan Beulich, xen-devel
On 12/01/17 14:15, Jan Beulich wrote:
> ... affecting PUSHF, POPF, CLI, and STI.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-01-12 14:37 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-12 14:08 [PATCH v5 0/4] x86emul: further misc improvements Jan Beulich
2017-01-12 14:15 ` [PATCH v5 1/4] x86emul: conditionally clear BNDn for branches Jan Beulich
2017-01-12 14:28 ` Andrew Cooper
2017-01-12 14:15 ` [PATCH v5 2/4] x86emul: support VME and PVI Jan Beulich
2017-01-12 14:37 ` Andrew Cooper
2017-01-12 14:16 ` [PATCH v5 3/4] x86emul: use switch()-wide local variable 'cr4' Jan Beulich
2017-01-12 14:16 ` [PATCH v5 4/4] x86emul: improve CR/DR access handling Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.