diff mbox

[1/3] x86emul: honor guest CR4.OSFXSR, CR4.OSXSAVE, and CR0.PE/EFLAGS.VM

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

Commit Message

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

Comments

Andrew Cooper Oct. 4, 2016, 1:58 p.m. UTC | #1 
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
Jan Beulich Oct. 4, 2016, 2:18 p.m. UTC | #2 
>>> 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
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
@@ -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);