diff mbox

[v2,1/3] x86emul: honor guest CR4.OSFXSR and CR4.OSXSAVE

Message ID 57F3E2720200007800114F5A@prv-mh.provo.novell.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Beulich Oct. 4, 2016, 3:10 p.m. UTC 
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).
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);

Comments

Andrew Cooper Oct. 4, 2016, 3:30 p.m. UTC | #1 
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);
>
>
>
diff mbox

Patch

--- 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);