* [PATCH 0/3] x86emul: XSA-190 follow-ups
@ 2016-10-04 13:23 Jan Beulich
2016-10-04 13:39 ` [PATCH 1/3] x86emul: honor guest CR4.OSFXSR, CR4.OSXSAVE, and CR0.PE/EFLAGS.VM Jan Beulich
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Jan Beulich @ 2016-10-04 13:23 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
1: honor guest CR4.OSFXSR, CR4.OSXSAVE, and CR0.PE/EFLAGS.VM
2: deliver correct math exceptions
3: check for FPU availability
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] 9+ messages in thread
* [PATCH 1/3] x86emul: honor guest CR4.OSFXSR, CR4.OSXSAVE, and CR0.PE/EFLAGS.VM
2016-10-04 13:23 [PATCH 0/3] x86emul: XSA-190 follow-ups Jan Beulich
@ 2016-10-04 13:39 ` Jan Beulich
2016-10-04 13:58 ` Andrew Cooper
2016-10-04 13:39 ` [PATCH 2/3] x86emul: deliver correct math exceptions Jan Beulich
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2016-10-04 13:39 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 3336 bytes --]
These checks belong into the emulator instead of hvmemul_get_fpu().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Eventually the XCR0 checks should also move into the insn emulator, but
that'll require a new hook (or an extension to the read_cr()
semantics).
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -129,6 +129,13 @@ static int cpuid(
(edx & (1U << 26)) != 0; \
})
+#define cpu_has_xsave ({ \
+ unsigned int eax = 1, ecx = 0; \
+ cpuid(&eax, &eax, &ecx, &eax, NULL); \
+ /* Intentionally checking OSXSAVE here. */ \
+ (ecx & (1U << 27)) != 0; \
+})
+
static inline uint64_t xgetbv(uint32_t xcr)
{
uint32_t lo, hi;
@@ -169,6 +176,11 @@ static int read_cr(
case 0:
*val = 0x00000001; /* PE */
return X86EMUL_OKAY;
+
+ case 4:
+ /* OSFXSR, OSXMMEXCPT, and maybe OSXSAVE */
+ *val = 0x00000600 | (cpu_has_xsave ? 0x00040000 : 0);
+ return X86EMUL_OKAY;
}
return X86EMUL_UNHANDLEABLE;
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1631,20 +1631,11 @@ static int hvmemul_get_fpu(
{
case X86EMUL_FPU_fpu:
case X86EMUL_FPU_wait:
- break;
case X86EMUL_FPU_mmx:
- if ( !cpu_has_mmx )
- return X86EMUL_UNHANDLEABLE;
- break;
case X86EMUL_FPU_xmm:
- if ( !(curr->arch.hvm_vcpu.guest_cr[4] & X86_CR4_OSFXSR) )
- return X86EMUL_UNHANDLEABLE;
break;
case X86EMUL_FPU_ymm:
- if ( !(curr->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PE) ||
- (ctxt->regs->eflags & X86_EFLAGS_VM) ||
- !(curr->arch.hvm_vcpu.guest_cr[4] & X86_CR4_OSXSAVE) ||
- !(curr->arch.xcr0 & XSTATE_SSE) ||
+ if ( !(curr->arch.xcr0 & XSTATE_SSE) ||
!(curr->arch.xcr0 & XSTATE_YMM) )
return X86EMUL_UNHANDLEABLE;
break;
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -424,8 +424,11 @@ typedef union {
#define CR0_MP (1<<1)
#define CR0_EM (1<<2)
#define CR0_TS (1<<3)
-#define CR4_TSD (1<<2)
-#define CR4_UMIP (1<<11)
+
+#define CR4_TSD (1<<2)
+#define CR4_OSFXSR (1<<9)
+#define CR4_UMIP (1<<11)
+#define CR4_OSXSAVE (1<<18)
/* EFLAGS bit definitions. */
#define EFLG_VIP (1<<20)
@@ -770,9 +773,23 @@ static int _get_fpu(
unsigned long cr0;
fail_if(!ops->read_cr);
+ if ( type >= X86EMUL_FPU_xmm )
+ {
+ unsigned long cr4;
+
+ rc = ops->read_cr(4, &cr4, ctxt);
+ if ( rc != X86EMUL_OKAY )
+ return rc;
+ generate_exception_if(!(cr4 & ((type == X86EMUL_FPU_xmm)
+ ? CR4_OSFXSR : CR4_OSXSAVE)),
+ EXC_UD, -1);
+ }
+
rc = ops->read_cr(0, &cr0, ctxt);
if ( rc != X86EMUL_OKAY )
return rc;
+ if ( !(cr0 & CR0_PE) || (ctxt->regs->eflags & EFLG_VM) )
+ generate_exception_if(type >= X86EMUL_FPU_ymm, EXC_UD, -1);
if ( cr0 & CR0_EM )
{
generate_exception_if(type == X86EMUL_FPU_fpu, EXC_NM, -1);
[-- Attachment #2: x86emul-honor-CR4-OS-flags.patch --]
[-- Type: text/plain, Size: 3400 bytes --]
x86emul: honor guest CR4.OSFXSR, CR4.OSXSAVE, and CR0.PE/EFLAGS.VM
These checks belong into the emulator instead of hvmemul_get_fpu().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Eventually the XCR0 checks should also move into the insn emulator, but
that'll require a new hook (or an extension to the read_cr()
semantics).
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -129,6 +129,13 @@ static int cpuid(
(edx & (1U << 26)) != 0; \
})
+#define cpu_has_xsave ({ \
+ unsigned int eax = 1, ecx = 0; \
+ cpuid(&eax, &eax, &ecx, &eax, NULL); \
+ /* Intentionally checking OSXSAVE here. */ \
+ (ecx & (1U << 27)) != 0; \
+})
+
static inline uint64_t xgetbv(uint32_t xcr)
{
uint32_t lo, hi;
@@ -169,6 +176,11 @@ static int read_cr(
case 0:
*val = 0x00000001; /* PE */
return X86EMUL_OKAY;
+
+ case 4:
+ /* OSFXSR, OSXMMEXCPT, and maybe OSXSAVE */
+ *val = 0x00000600 | (cpu_has_xsave ? 0x00040000 : 0);
+ return X86EMUL_OKAY;
}
return X86EMUL_UNHANDLEABLE;
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1631,20 +1631,11 @@ static int hvmemul_get_fpu(
{
case X86EMUL_FPU_fpu:
case X86EMUL_FPU_wait:
- break;
case X86EMUL_FPU_mmx:
- if ( !cpu_has_mmx )
- return X86EMUL_UNHANDLEABLE;
- break;
case X86EMUL_FPU_xmm:
- if ( !(curr->arch.hvm_vcpu.guest_cr[4] & X86_CR4_OSFXSR) )
- return X86EMUL_UNHANDLEABLE;
break;
case X86EMUL_FPU_ymm:
- if ( !(curr->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PE) ||
- (ctxt->regs->eflags & X86_EFLAGS_VM) ||
- !(curr->arch.hvm_vcpu.guest_cr[4] & X86_CR4_OSXSAVE) ||
- !(curr->arch.xcr0 & XSTATE_SSE) ||
+ if ( !(curr->arch.xcr0 & XSTATE_SSE) ||
!(curr->arch.xcr0 & XSTATE_YMM) )
return X86EMUL_UNHANDLEABLE;
break;
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -424,8 +424,11 @@ typedef union {
#define CR0_MP (1<<1)
#define CR0_EM (1<<2)
#define CR0_TS (1<<3)
-#define CR4_TSD (1<<2)
-#define CR4_UMIP (1<<11)
+
+#define CR4_TSD (1<<2)
+#define CR4_OSFXSR (1<<9)
+#define CR4_UMIP (1<<11)
+#define CR4_OSXSAVE (1<<18)
/* EFLAGS bit definitions. */
#define EFLG_VIP (1<<20)
@@ -770,9 +773,23 @@ static int _get_fpu(
unsigned long cr0;
fail_if(!ops->read_cr);
+ if ( type >= X86EMUL_FPU_xmm )
+ {
+ unsigned long cr4;
+
+ rc = ops->read_cr(4, &cr4, ctxt);
+ if ( rc != X86EMUL_OKAY )
+ return rc;
+ generate_exception_if(!(cr4 & ((type == X86EMUL_FPU_xmm)
+ ? CR4_OSFXSR : CR4_OSXSAVE)),
+ EXC_UD, -1);
+ }
+
rc = ops->read_cr(0, &cr0, ctxt);
if ( rc != X86EMUL_OKAY )
return rc;
+ if ( !(cr0 & CR0_PE) || (ctxt->regs->eflags & EFLG_VM) )
+ generate_exception_if(type >= X86EMUL_FPU_ymm, EXC_UD, -1);
if ( cr0 & CR0_EM )
{
generate_exception_if(type == X86EMUL_FPU_fpu, EXC_NM, -1);
[-- 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] 9+ messages in thread
* [PATCH 2/3] x86emul: deliver correct math exceptions
2016-10-04 13:23 [PATCH 0/3] x86emul: XSA-190 follow-ups Jan Beulich
2016-10-04 13:39 ` [PATCH 1/3] x86emul: honor guest CR4.OSFXSR, CR4.OSXSAVE, and CR0.PE/EFLAGS.VM Jan Beulich
@ 2016-10-04 13:39 ` Jan Beulich
2016-10-04 14:09 ` Andrew Cooper
2016-10-04 13:40 ` [PATCH 3/3] x86emul: check for FPU availability Jan Beulich
2016-10-04 15:10 ` [PATCH v2 1/3] x86emul: honor guest CR4.OSFXSR and CR4.OSXSAVE Jan Beulich
3 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2016-10-04 13:39 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 2436 bytes --]
#MF only applies to x87 instructions. SSE and AVX ones need #XM to be
raised instead, unless CR4.OSXMMEXCPT is clear, in which case #UD needs
to result. (But note that this is only a latent issue - we don't
emulate any instructions so far which could result in #XM.)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -427,6 +427,7 @@ typedef union {
#define CR4_TSD (1<<2)
#define CR4_OSFXSR (1<<9)
+#define CR4_OSXMMEXCPT (1<<10)
#define CR4_UMIP (1<<11)
#define CR4_OSXSAVE (1<<18)
@@ -462,6 +463,7 @@ typedef union {
#define EXC_GP 13
#define EXC_PF 14
#define EXC_MF 16
+#define EXC_XM 19
/* Segment selector error code bits. */
#define ECODE_EXT (1 << 0)
@@ -745,13 +747,14 @@ do {
struct fpu_insn_ctxt {
uint8_t insn_bytes;
- uint8_t exn_raised;
+ int8_t exn_raised;
};
static void fpu_handle_exception(void *_fic, struct cpu_user_regs *regs)
{
struct fpu_insn_ctxt *fic = _fic;
- fic->exn_raised = 1;
+ ASSERT(regs->entry_vector < 0x20);
+ fic->exn_raised = regs->entry_vector;
regs->eip += fic->insn_bytes;
}
@@ -763,7 +766,7 @@ static int _get_fpu(
{
int rc;
- fic->exn_raised = 0;
+ fic->exn_raised = -1;
fail_if(!ops->get_fpu);
rc = ops->get_fpu(fpu_handle_exception, fic, type, ctxt);
@@ -818,7 +821,15 @@ do {
#define put_fpu(_fic) \
do { \
_put_fpu(); \
- generate_exception_if((_fic)->exn_raised, EXC_MF, -1); \
+ 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; \
+ } \
+ generate_exception_if((_fic)->exn_raised >= 0, \
+ (_fic)->exn_raised, -1); \
} while (0)
#define emulate_fpu_insn(_op) \
[-- Attachment #2: x86emul-math-exceptions.patch --]
[-- Type: text/plain, Size: 2474 bytes --]
x86emul: deliver correct math exceptions
#MF only applies to x87 instructions. SSE and AVX ones need #XM to be
raised instead, unless CR4.OSXMMEXCPT is clear, in which case #UD needs
to result. (But note that this is only a latent issue - we don't
emulate any instructions so far which could result in #XM.)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -427,6 +427,7 @@ typedef union {
#define CR4_TSD (1<<2)
#define CR4_OSFXSR (1<<9)
+#define CR4_OSXMMEXCPT (1<<10)
#define CR4_UMIP (1<<11)
#define CR4_OSXSAVE (1<<18)
@@ -462,6 +463,7 @@ typedef union {
#define EXC_GP 13
#define EXC_PF 14
#define EXC_MF 16
+#define EXC_XM 19
/* Segment selector error code bits. */
#define ECODE_EXT (1 << 0)
@@ -745,13 +747,14 @@ do {
struct fpu_insn_ctxt {
uint8_t insn_bytes;
- uint8_t exn_raised;
+ int8_t exn_raised;
};
static void fpu_handle_exception(void *_fic, struct cpu_user_regs *regs)
{
struct fpu_insn_ctxt *fic = _fic;
- fic->exn_raised = 1;
+ ASSERT(regs->entry_vector < 0x20);
+ fic->exn_raised = regs->entry_vector;
regs->eip += fic->insn_bytes;
}
@@ -763,7 +766,7 @@ static int _get_fpu(
{
int rc;
- fic->exn_raised = 0;
+ fic->exn_raised = -1;
fail_if(!ops->get_fpu);
rc = ops->get_fpu(fpu_handle_exception, fic, type, ctxt);
@@ -818,7 +821,15 @@ do {
#define put_fpu(_fic) \
do { \
_put_fpu(); \
- generate_exception_if((_fic)->exn_raised, EXC_MF, -1); \
+ 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; \
+ } \
+ generate_exception_if((_fic)->exn_raised >= 0, \
+ (_fic)->exn_raised, -1); \
} while (0)
#define emulate_fpu_insn(_op) \
[-- 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] 9+ messages in thread
* [PATCH 3/3] x86emul: check for FPU availability
2016-10-04 13:23 [PATCH 0/3] x86emul: XSA-190 follow-ups Jan Beulich
2016-10-04 13:39 ` [PATCH 1/3] x86emul: honor guest CR4.OSFXSR, CR4.OSXSAVE, and CR0.PE/EFLAGS.VM Jan Beulich
2016-10-04 13:39 ` [PATCH 2/3] x86emul: deliver correct math exceptions Jan Beulich
@ 2016-10-04 13:40 ` Jan Beulich
2016-10-04 15:10 ` [PATCH v2 1/3] x86emul: honor guest CR4.OSFXSR and CR4.OSXSAVE Jan Beulich
3 siblings, 0 replies; 9+ messages in thread
From: Jan Beulich @ 2016-10-04 13:40 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 2643 bytes --]
We can't exclude someone wanting to hide the FPU from guests.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper@citrix.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1246,6 +1246,7 @@ static bool_t vcpu_has(
#define vcpu_must_have(leaf, reg, bit) \
generate_exception_if(!vcpu_has(leaf, reg, bit, ctxt, ops), EXC_UD, -1)
+#define vcpu_must_have_fpu() vcpu_must_have(0x00000001, EDX, 0)
#define vcpu_must_have_mmx() vcpu_must_have(0x00000001, EDX, 23)
#define vcpu_must_have_sse() vcpu_must_have(0x00000001, EDX, 25)
#define vcpu_must_have_sse2() vcpu_must_have(0x00000001, EDX, 26)
@@ -3107,6 +3108,7 @@ x86_emulate(
{
struct fpu_insn_ctxt fic = { .insn_bytes = 1 };
+ host_and_vcpu_must_have(fpu);
get_fpu(X86EMUL_FPU_wait, &fic);
asm volatile ( "fwait" ::: "memory" );
put_fpu(&fic);
@@ -3479,6 +3481,7 @@ x86_emulate(
}
case 0xd8: /* FPU 0xd8 */
+ host_and_vcpu_must_have(fpu);
switch ( modrm )
{
case 0xc0 ... 0xc7: /* fadd %stN,%stN */
@@ -3529,6 +3532,7 @@ x86_emulate(
break;
case 0xd9: /* FPU 0xd9 */
+ host_and_vcpu_must_have(fpu);
switch ( modrm )
{
case 0xfb: /* fsincos */
@@ -3612,6 +3616,7 @@ x86_emulate(
break;
case 0xda: /* FPU 0xda */
+ host_and_vcpu_must_have(fpu);
switch ( modrm )
{
case 0xc0 ... 0xc7: /* fcmovb %stN */
@@ -3659,6 +3664,7 @@ x86_emulate(
break;
case 0xdb: /* FPU 0xdb */
+ host_and_vcpu_must_have(fpu);
switch ( modrm )
{
case 0xc0 ... 0xc7: /* fcmovnb %stN */
@@ -3731,6 +3737,7 @@ x86_emulate(
break;
case 0xdc: /* FPU 0xdc */
+ host_and_vcpu_must_have(fpu);
switch ( modrm )
{
case 0xc0 ... 0xc7: /* fadd %stN */
@@ -3779,6 +3786,7 @@ x86_emulate(
break;
case 0xdd: /* FPU 0xdd */
+ host_and_vcpu_must_have(fpu);
switch ( modrm )
{
case 0xc0 ... 0xc7: /* ffree %stN */
@@ -3832,6 +3840,7 @@ x86_emulate(
break;
case 0xde: /* FPU 0xde */
+ host_and_vcpu_must_have(fpu);
switch ( modrm )
{
case 0xc0 ... 0xc7: /* faddp %stN */
@@ -3881,6 +3890,7 @@ x86_emulate(
break;
case 0xdf: /* FPU 0xdf */
+ host_and_vcpu_must_have(fpu);
switch ( modrm )
{
case 0xe0:
[-- Attachment #2: x86emul-check-CPUID-FPU.patch --]
[-- Type: text/plain, Size: 2676 bytes --]
x86emul: check for FPU availability
We can't exclude someone wanting to hide the FPU from guests.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper@citrix.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1246,6 +1246,7 @@ static bool_t vcpu_has(
#define vcpu_must_have(leaf, reg, bit) \
generate_exception_if(!vcpu_has(leaf, reg, bit, ctxt, ops), EXC_UD, -1)
+#define vcpu_must_have_fpu() vcpu_must_have(0x00000001, EDX, 0)
#define vcpu_must_have_mmx() vcpu_must_have(0x00000001, EDX, 23)
#define vcpu_must_have_sse() vcpu_must_have(0x00000001, EDX, 25)
#define vcpu_must_have_sse2() vcpu_must_have(0x00000001, EDX, 26)
@@ -3107,6 +3108,7 @@ x86_emulate(
{
struct fpu_insn_ctxt fic = { .insn_bytes = 1 };
+ host_and_vcpu_must_have(fpu);
get_fpu(X86EMUL_FPU_wait, &fic);
asm volatile ( "fwait" ::: "memory" );
put_fpu(&fic);
@@ -3479,6 +3481,7 @@ x86_emulate(
}
case 0xd8: /* FPU 0xd8 */
+ host_and_vcpu_must_have(fpu);
switch ( modrm )
{
case 0xc0 ... 0xc7: /* fadd %stN,%stN */
@@ -3529,6 +3532,7 @@ x86_emulate(
break;
case 0xd9: /* FPU 0xd9 */
+ host_and_vcpu_must_have(fpu);
switch ( modrm )
{
case 0xfb: /* fsincos */
@@ -3612,6 +3616,7 @@ x86_emulate(
break;
case 0xda: /* FPU 0xda */
+ host_and_vcpu_must_have(fpu);
switch ( modrm )
{
case 0xc0 ... 0xc7: /* fcmovb %stN */
@@ -3659,6 +3664,7 @@ x86_emulate(
break;
case 0xdb: /* FPU 0xdb */
+ host_and_vcpu_must_have(fpu);
switch ( modrm )
{
case 0xc0 ... 0xc7: /* fcmovnb %stN */
@@ -3731,6 +3737,7 @@ x86_emulate(
break;
case 0xdc: /* FPU 0xdc */
+ host_and_vcpu_must_have(fpu);
switch ( modrm )
{
case 0xc0 ... 0xc7: /* fadd %stN */
@@ -3779,6 +3786,7 @@ x86_emulate(
break;
case 0xdd: /* FPU 0xdd */
+ host_and_vcpu_must_have(fpu);
switch ( modrm )
{
case 0xc0 ... 0xc7: /* ffree %stN */
@@ -3832,6 +3840,7 @@ x86_emulate(
break;
case 0xde: /* FPU 0xde */
+ host_and_vcpu_must_have(fpu);
switch ( modrm )
{
case 0xc0 ... 0xc7: /* faddp %stN */
@@ -3881,6 +3890,7 @@ x86_emulate(
break;
case 0xdf: /* FPU 0xdf */
+ host_and_vcpu_must_have(fpu);
switch ( modrm )
{
case 0xe0:
[-- 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] 9+ messages in thread
* Re: [PATCH 1/3] x86emul: honor guest CR4.OSFXSR, CR4.OSXSAVE, and CR0.PE/EFLAGS.VM
2016-10-04 13:39 ` [PATCH 1/3] x86emul: honor guest CR4.OSFXSR, CR4.OSXSAVE, and CR0.PE/EFLAGS.VM Jan Beulich
@ 2016-10-04 13:58 ` Andrew Cooper
2016-10-04 14:18 ` Jan Beulich
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Cooper @ 2016-10-04 13:58 UTC (permalink / raw)
To: Jan Beulich, xen-devel
On 04/10/16 14:39, Jan Beulich wrote:
> @@ -770,9 +773,23 @@ static int _get_fpu(
> unsigned long cr0;
>
> fail_if(!ops->read_cr);
> + if ( type >= X86EMUL_FPU_xmm )
> + {
> + unsigned long cr4;
> +
> + rc = ops->read_cr(4, &cr4, ctxt);
> + if ( rc != X86EMUL_OKAY )
> + return rc;
> + generate_exception_if(!(cr4 & ((type == X86EMUL_FPU_xmm)
> + ? CR4_OSFXSR : CR4_OSXSAVE)),
> + EXC_UD, -1);
> + }
> +
> rc = ops->read_cr(0, &cr0, ctxt);
> if ( rc != X86EMUL_OKAY )
> return rc;
> + if ( !(cr0 & CR0_PE) || (ctxt->regs->eflags & EFLG_VM) )
> + generate_exception_if(type >= X86EMUL_FPU_ymm, EXC_UD, -1);
Is this an appropriate check to do here? This restriction is because
the VEX prefix isn't permitted in real/vm86 mode.
Instead of a generate_exception_if(), I would instead have an ASSERT()
that we don't actually reach this point.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] x86emul: deliver correct math exceptions
2016-10-04 13:39 ` [PATCH 2/3] x86emul: deliver correct math exceptions Jan Beulich
@ 2016-10-04 14:09 ` Andrew Cooper
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Cooper @ 2016-10-04 14:09 UTC (permalink / raw)
To: Jan Beulich, xen-devel
On 04/10/16 14:39, Jan Beulich wrote:
> #MF only applies to x87 instructions. SSE and AVX ones need #XM to be
> raised instead, unless CR4.OSXMMEXCPT is clear, in which case #UD needs
> to result. (But note that this is only a latent issue - we don't
> emulate any instructions so far which could result in #XM.)
>
> 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] 9+ messages in thread
* Re: [PATCH 1/3] x86emul: honor guest CR4.OSFXSR, CR4.OSXSAVE, and CR0.PE/EFLAGS.VM
2016-10-04 13:58 ` Andrew Cooper
@ 2016-10-04 14:18 ` Jan Beulich
0 siblings, 0 replies; 9+ messages in thread
From: Jan Beulich @ 2016-10-04 14:18 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
>>> On 04.10.16 at 15:58, <andrew.cooper3@citrix.com> wrote:
> On 04/10/16 14:39, Jan Beulich wrote:
>> @@ -770,9 +773,23 @@ static int _get_fpu(
>> unsigned long cr0;
>>
>> fail_if(!ops->read_cr);
>> + if ( type >= X86EMUL_FPU_xmm )
>> + {
>> + unsigned long cr4;
>> +
>> + rc = ops->read_cr(4, &cr4, ctxt);
>> + if ( rc != X86EMUL_OKAY )
>> + return rc;
>> + generate_exception_if(!(cr4 & ((type == X86EMUL_FPU_xmm)
>> + ? CR4_OSFXSR : CR4_OSXSAVE)),
>> + EXC_UD, -1);
>> + }
>> +
>> rc = ops->read_cr(0, &cr0, ctxt);
>> if ( rc != X86EMUL_OKAY )
>> return rc;
>> + if ( !(cr0 & CR0_PE) || (ctxt->regs->eflags & EFLG_VM) )
>> + generate_exception_if(type >= X86EMUL_FPU_ymm, EXC_UD, -1);
>
> Is this an appropriate check to do here? This restriction is because
> the VEX prefix isn't permitted in real/vm86 mode.
>
> Instead of a generate_exception_if(), I would instead have an ASSERT()
> that we don't actually reach this point.
Hmm, that's right.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/3] x86emul: honor guest CR4.OSFXSR and CR4.OSXSAVE
2016-10-04 13:23 [PATCH 0/3] x86emul: XSA-190 follow-ups Jan Beulich
` (2 preceding siblings ...)
2016-10-04 13:40 ` [PATCH 3/3] x86emul: check for FPU availability Jan Beulich
@ 2016-10-04 15:10 ` Jan Beulich
2016-10-04 15:30 ` Andrew Cooper
3 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2016-10-04 15:10 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Paul Durrant
[-- Attachment #1: Type: text/plain, Size: 3519 bytes --]
These checks belong into the emulator instead of hvmemul_get_fpu().
The CR0.PE/EFLAGS.VM ones can actually just be ASSERT()ed, as decoding
should make it impossible to get into get_fpu() with them in the wrong
state.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Convert CR0.PE / EFLAGS.VM check to ASSERT().
---
Eventually the XCR0 checks should also move into the insn emulator, but
that'll require a new hook (or an extension to the read_cr()
semantics).
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -129,6 +129,13 @@ static int cpuid(
(edx & (1U << 26)) != 0; \
})
+#define cpu_has_xsave ({ \
+ unsigned int eax = 1, ecx = 0; \
+ cpuid(&eax, &eax, &ecx, &eax, NULL); \
+ /* Intentionally checking OSXSAVE here. */ \
+ (ecx & (1U << 27)) != 0; \
+})
+
static inline uint64_t xgetbv(uint32_t xcr)
{
uint32_t lo, hi;
@@ -169,6 +176,11 @@ static int read_cr(
case 0:
*val = 0x00000001; /* PE */
return X86EMUL_OKAY;
+
+ case 4:
+ /* OSFXSR, OSXMMEXCPT, and maybe OSXSAVE */
+ *val = 0x00000600 | (cpu_has_xsave ? 0x00040000 : 0);
+ return X86EMUL_OKAY;
}
return X86EMUL_UNHANDLEABLE;
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1629,20 +1629,11 @@ static int hvmemul_get_fpu(
{
case X86EMUL_FPU_fpu:
case X86EMUL_FPU_wait:
- break;
case X86EMUL_FPU_mmx:
- if ( !cpu_has_mmx )
- return X86EMUL_UNHANDLEABLE;
- break;
case X86EMUL_FPU_xmm:
- if ( !(curr->arch.hvm_vcpu.guest_cr[4] & X86_CR4_OSFXSR) )
- return X86EMUL_UNHANDLEABLE;
break;
case X86EMUL_FPU_ymm:
- if ( !(curr->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PE) ||
- (ctxt->regs->eflags & X86_EFLAGS_VM) ||
- !(curr->arch.hvm_vcpu.guest_cr[4] & X86_CR4_OSXSAVE) ||
- !(curr->arch.xcr0 & XSTATE_SSE) ||
+ if ( !(curr->arch.xcr0 & XSTATE_SSE) ||
!(curr->arch.xcr0 & XSTATE_YMM) )
return X86EMUL_UNHANDLEABLE;
break;
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -421,8 +421,11 @@ typedef union {
#define CR0_MP (1<<1)
#define CR0_EM (1<<2)
#define CR0_TS (1<<3)
-#define CR4_TSD (1<<2)
-#define CR4_UMIP (1<<11)
+
+#define CR4_TSD (1<<2)
+#define CR4_OSFXSR (1<<9)
+#define CR4_UMIP (1<<11)
+#define CR4_OSXSAVE (1<<18)
/* EFLAGS bit definitions. */
#define EFLG_VIP (1<<20)
@@ -767,9 +770,23 @@ static int _get_fpu(
unsigned long cr0;
fail_if(!ops->read_cr);
+ if ( type >= X86EMUL_FPU_xmm )
+ {
+ unsigned long cr4;
+
+ rc = ops->read_cr(4, &cr4, ctxt);
+ if ( rc != X86EMUL_OKAY )
+ return rc;
+ generate_exception_if(!(cr4 & ((type == X86EMUL_FPU_xmm)
+ ? CR4_OSFXSR : CR4_OSXSAVE)),
+ EXC_UD, -1);
+ }
+
rc = ops->read_cr(0, &cr0, ctxt);
if ( rc != X86EMUL_OKAY )
return rc;
+ if ( type >= X86EMUL_FPU_ymm )
+ ASSERT((cr0 & CR0_PE) && !(ctxt->regs->eflags & EFLG_VM));
if ( cr0 & CR0_EM )
{
generate_exception_if(type == X86EMUL_FPU_fpu, EXC_NM, -1);
[-- Attachment #2: x86emul-honor-CR4-OS-flags.patch --]
[-- Type: text/plain, Size: 3564 bytes --]
x86emul: honor guest CR4.OSFXSR and CR4.OSXSAVE
These checks belong into the emulator instead of hvmemul_get_fpu().
The CR0.PE/EFLAGS.VM ones can actually just be ASSERT()ed, as decoding
should make it impossible to get into get_fpu() with them in the wrong
state.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Convert CR0.PE / EFLAGS.VM check to ASSERT().
---
Eventually the XCR0 checks should also move into the insn emulator, but
that'll require a new hook (or an extension to the read_cr()
semantics).
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -129,6 +129,13 @@ static int cpuid(
(edx & (1U << 26)) != 0; \
})
+#define cpu_has_xsave ({ \
+ unsigned int eax = 1, ecx = 0; \
+ cpuid(&eax, &eax, &ecx, &eax, NULL); \
+ /* Intentionally checking OSXSAVE here. */ \
+ (ecx & (1U << 27)) != 0; \
+})
+
static inline uint64_t xgetbv(uint32_t xcr)
{
uint32_t lo, hi;
@@ -169,6 +176,11 @@ static int read_cr(
case 0:
*val = 0x00000001; /* PE */
return X86EMUL_OKAY;
+
+ case 4:
+ /* OSFXSR, OSXMMEXCPT, and maybe OSXSAVE */
+ *val = 0x00000600 | (cpu_has_xsave ? 0x00040000 : 0);
+ return X86EMUL_OKAY;
}
return X86EMUL_UNHANDLEABLE;
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1629,20 +1629,11 @@ static int hvmemul_get_fpu(
{
case X86EMUL_FPU_fpu:
case X86EMUL_FPU_wait:
- break;
case X86EMUL_FPU_mmx:
- if ( !cpu_has_mmx )
- return X86EMUL_UNHANDLEABLE;
- break;
case X86EMUL_FPU_xmm:
- if ( !(curr->arch.hvm_vcpu.guest_cr[4] & X86_CR4_OSFXSR) )
- return X86EMUL_UNHANDLEABLE;
break;
case X86EMUL_FPU_ymm:
- if ( !(curr->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PE) ||
- (ctxt->regs->eflags & X86_EFLAGS_VM) ||
- !(curr->arch.hvm_vcpu.guest_cr[4] & X86_CR4_OSXSAVE) ||
- !(curr->arch.xcr0 & XSTATE_SSE) ||
+ if ( !(curr->arch.xcr0 & XSTATE_SSE) ||
!(curr->arch.xcr0 & XSTATE_YMM) )
return X86EMUL_UNHANDLEABLE;
break;
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -421,8 +421,11 @@ typedef union {
#define CR0_MP (1<<1)
#define CR0_EM (1<<2)
#define CR0_TS (1<<3)
-#define CR4_TSD (1<<2)
-#define CR4_UMIP (1<<11)
+
+#define CR4_TSD (1<<2)
+#define CR4_OSFXSR (1<<9)
+#define CR4_UMIP (1<<11)
+#define CR4_OSXSAVE (1<<18)
/* EFLAGS bit definitions. */
#define EFLG_VIP (1<<20)
@@ -767,9 +770,23 @@ static int _get_fpu(
unsigned long cr0;
fail_if(!ops->read_cr);
+ if ( type >= X86EMUL_FPU_xmm )
+ {
+ unsigned long cr4;
+
+ rc = ops->read_cr(4, &cr4, ctxt);
+ if ( rc != X86EMUL_OKAY )
+ return rc;
+ generate_exception_if(!(cr4 & ((type == X86EMUL_FPU_xmm)
+ ? CR4_OSFXSR : CR4_OSXSAVE)),
+ EXC_UD, -1);
+ }
+
rc = ops->read_cr(0, &cr0, ctxt);
if ( rc != X86EMUL_OKAY )
return rc;
+ if ( type >= X86EMUL_FPU_ymm )
+ ASSERT((cr0 & CR0_PE) && !(ctxt->regs->eflags & EFLG_VM));
if ( cr0 & CR0_EM )
{
generate_exception_if(type == X86EMUL_FPU_fpu, EXC_NM, -1);
[-- 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] 9+ messages in thread
* Re: [PATCH v2 1/3] x86emul: honor guest CR4.OSFXSR and CR4.OSXSAVE
2016-10-04 15:10 ` [PATCH v2 1/3] x86emul: honor guest CR4.OSFXSR and CR4.OSXSAVE Jan Beulich
@ 2016-10-04 15:30 ` Andrew Cooper
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Cooper @ 2016-10-04 15:30 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Paul Durrant
On 04/10/16 16:10, Jan Beulich wrote:
> @@ -767,9 +770,23 @@ static int _get_fpu(
> unsigned long cr0;
>
> fail_if(!ops->read_cr);
> + if ( type >= X86EMUL_FPU_xmm )
> + {
> + unsigned long cr4;
> +
> + rc = ops->read_cr(4, &cr4, ctxt);
> + if ( rc != X86EMUL_OKAY )
> + return rc;
> + generate_exception_if(!(cr4 & ((type == X86EMUL_FPU_xmm)
> + ? CR4_OSFXSR : CR4_OSXSAVE)),
> + EXC_UD, -1);
> + }
> +
> rc = ops->read_cr(0, &cr0, ctxt);
> if ( rc != X86EMUL_OKAY )
> return rc;
> + if ( type >= X86EMUL_FPU_ymm )
I would be tempted to add here
/* Should not be reachable if VEX decoding is working correctly. */
Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> + ASSERT((cr0 & CR0_PE) && !(ctxt->regs->eflags & EFLG_VM));
> if ( cr0 & CR0_EM )
> {
> generate_exception_if(type == X86EMUL_FPU_fpu, EXC_NM, -1);
>
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-10-04 15:39 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-04 13:23 [PATCH 0/3] x86emul: XSA-190 follow-ups Jan Beulich
2016-10-04 13:39 ` [PATCH 1/3] x86emul: honor guest CR4.OSFXSR, CR4.OSXSAVE, and CR0.PE/EFLAGS.VM Jan Beulich
2016-10-04 13:58 ` Andrew Cooper
2016-10-04 14:18 ` Jan Beulich
2016-10-04 13:39 ` [PATCH 2/3] x86emul: deliver correct math exceptions Jan Beulich
2016-10-04 14:09 ` Andrew Cooper
2016-10-04 13:40 ` [PATCH 3/3] x86emul: check for FPU availability Jan Beulich
2016-10-04 15:10 ` [PATCH v2 1/3] x86emul: honor guest CR4.OSFXSR and CR4.OSXSAVE Jan Beulich
2016-10-04 15:30 ` Andrew Cooper
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.