diff mbox series

[v1,3/3] hvf: Support AVX512 guests on capable hardware

Message ID 1da0fc0a4f119e951ae11b29ff26ee587f4748aa.1585607927.git.dirty@apple.com (mailing list archive)
State New, archived
Headers show
Series hvf: Support AVX512 guests and cleanup | expand

Commit Message

Zhijian Li (Fujitsu)" via March 31, 2020, 12:16 a.m. UTC
macOS lazily enables AVX512.  Explicitly enable it if the processor
supports it.

cpu_x86_cpuid() tries to handle OSXSAVE but refers to env->cr[4] for the
guest copy of CR4.  HVF doesn't support caching CPUID values like KVM,
so we need to track it ourselves.

Signed-off-by: Cameron Esfahani <dirty@apple.com>
---
 target/i386/cpu.h        |  1 +
 target/i386/hvf/hvf.c    | 68 ++++++++++++++++++++++++++++++++++++++--
 target/i386/hvf/vmx.h    |  9 +++++-
 target/i386/hvf/x86hvf.c |  2 +-
 4 files changed, 76 insertions(+), 4 deletions(-)

Comments

Paolo Bonzini April 6, 2020, 10 a.m. UTC | #1
On 31/03/20 02:16, Cameron Esfahani wrote:
> @@ -458,7 +459,7 @@ void hvf_reset_vcpu(CPUState *cpu) {
>      macvm_set_cr0(cpu->hvf_fd, CR0_CD_MASK | CR0_NW_MASK | CR0_ET_MASK);
>      macvm_set_cr0(cpu->hvf_fd, 0x60000010);
>  
> -    wvmcs(cpu->hvf_fd, VMCS_CR4_MASK, CR4_VMXE_MASK);
> +    wvmcs(cpu->hvf_fd, VMCS_CR4_MASK, CR4_VMXE_MASK | CR4_OSXSAVE_MASK);
>      wvmcs(cpu->hvf_fd, VMCS_CR4_SHADOW, 0x0);
>      wvmcs(cpu->hvf_fd, VMCS_GUEST_CR4, CR4_VMXE_MASK);
>  
> diff --git a/target/i386/hvf/vmx.h b/target/i386/hvf/vmx.h
> index 1a1b150c97..dccd5ceb0f 100644
> --- a/target/i386/hvf/vmx.h
> +++ b/target/i386/hvf/vmx.h
> @@ -157,13 +157,20 @@ static inline void macvm_set_cr0(hv_vcpuid_t vcpu, uint64_t cr0)
>      hv_vcpu_flush(vcpu);
>  }
>  
> -static inline void macvm_set_cr4(hv_vcpuid_t vcpu, uint64_t cr4)
> +static inline void macvm_set_cr4(CPUX86State *env, hv_vcpuid_t vcpu,
> +                                 uint64_t cr4)
>  {
>      uint64_t guest_cr4 = cr4 | CR4_VMXE_MASK;

I think you need to add the host CR4.OSXSAVE bit here too?  (You can
read it from CPUID).

>      wvmcs(vcpu, VMCS_GUEST_CR4, guest_cr4);
>      wvmcs(vcpu, VMCS_CR4_SHADOW, cr4);
>  
> +    /*
> +     * Track whether OSXSAVE is enabled so we can properly return it
> +     * for CPUID 1.
> +     */
> +    env->osxsave_enabled = ((cr4 & CR4_OSXSAVE_MASK) != 0);

This new variable doesn't seem necessary.  Instead you can just set
env->cr[4] here, and everything should work fine.

Paolo
Roman Bolshakov April 8, 2020, 1:56 p.m. UTC | #2
On Mon, Mar 30, 2020 at 05:16:06PM -0700, Cameron Esfahani wrote:
> macOS lazily enables AVX512.  Explicitly enable it if the processor
> supports it.
> 
> cpu_x86_cpuid() tries to handle OSXSAVE but refers to env->cr[4] for the
> guest copy of CR4.  HVF doesn't support caching CPUID values like KVM,
> so we need to track it ourselves.
> 

Hi Cameron,

Side question, how did you test it? I tried the latest ubuntu and
archlinux iso with -cpu host and noticed that kernel complains about TSC
misbehaviour early during boot.

Also, I didn't look thoroughly into it but do you happen to know if
there could be a case that we're getting AVX512 support without AVX2
support? Wouldn't that be an issue?

Thanks,
Roman
diff mbox series

Patch

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 1286ec6e7a..f3864d0fac 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1591,6 +1591,7 @@  typedef struct CPUX86State {
     struct kvm_nested_state *nested_state;
 #endif
 #if defined(CONFIG_HVF)
+    bool osxsave_enabled;
     HVFX86EmulatorState *hvf_emul;
 #endif
 
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index fef1ee7d70..68a85c3b9b 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -65,6 +65,7 @@ 
 
 #include <Hypervisor/hv.h>
 #include <Hypervisor/hv_vmx.h>
+#include <mach/mach.h>
 
 #include "exec/address-spaces.h"
 #include "hw/i386/apic_internal.h"
@@ -458,7 +459,7 @@  void hvf_reset_vcpu(CPUState *cpu) {
     macvm_set_cr0(cpu->hvf_fd, CR0_CD_MASK | CR0_NW_MASK | CR0_ET_MASK);
     macvm_set_cr0(cpu->hvf_fd, 0x60000010);
 
-    wvmcs(cpu->hvf_fd, VMCS_CR4_MASK, CR4_VMXE_MASK);
+    wvmcs(cpu->hvf_fd, VMCS_CR4_MASK, CR4_VMXE_MASK | CR4_OSXSAVE_MASK);
     wvmcs(cpu->hvf_fd, VMCS_CR4_SHADOW, 0x0);
     wvmcs(cpu->hvf_fd, VMCS_GUEST_CR4, CR4_VMXE_MASK);
 
@@ -541,6 +542,55 @@  static void dummy_signal(int sig)
 {
 }
 
+static bool enable_avx512_thread_state(void)
+{
+    x86_avx512_state_t state;
+    uint32_t ebx;
+    kern_return_t ret;
+    unsigned int count;
+
+    /*
+     * macOS lazily enables AVX512 support.  Enable it explicitly if the
+     * processor supports it.
+     */
+
+    host_cpuid(7, 0, NULL, &ebx, NULL, NULL);
+    if ((ebx & CPUID_7_0_EBX_AVX512F) == 0) {
+        return false;
+    }
+
+    memset(&state, 0, sizeof(x86_avx512_state_t));
+
+    /* Get AVX state */
+    count = x86_AVX_STATE_COUNT;
+    ret = thread_get_state(mach_thread_self(),
+                           x86_AVX_STATE,
+                           (thread_state_t) &state,
+                           &count);
+    if (ret != KERN_SUCCESS) {
+        return false;
+    }
+    if (count != x86_AVX_STATE_COUNT) {
+        return false;
+    }
+    if (state.ash.flavor != x86_AVX_STATE64) {
+        return false;
+    }
+    state.ash.flavor = x86_AVX512_STATE64;
+    state.ash.count = x86_AVX512_STATE64_COUNT;
+
+    /* Now set as AVX512 */
+    ret = thread_set_state(mach_thread_self(),
+                           state.ash.flavor,
+                           (thread_state_t) &state.ufs.as64,
+                           state.ash.count);
+    if (ret != KERN_SUCCESS) {
+        return false;
+    }
+
+    return true;
+}
+
 int hvf_init_vcpu(CPUState *cpu)
 {
 
@@ -826,6 +876,18 @@  int hvf_vcpu_exec(CPUState *cpu)
 
             cpu_x86_cpuid(env, rax, rcx, &rax, &rbx, &rcx, &rdx);
 
+            if (rax == 1) {
+                /*
+                 * cpu_x86_cpuid tries to handle OSXSAVE but refers to
+                 * env->cr[4] for the guest copy of CR4.  This isn't
+                 * updated regularly so we track it ourselves in
+                 * env->osxsave_enabled.
+                 */
+                if ((rcx & CPUID_EXT_XSAVE) && env->osxsave_enabled) {
+                    rcx |= CPUID_EXT_OSXSAVE;
+                }
+            }
+
             wreg(cpu->hvf_fd, HV_X86_RAX, rax);
             wreg(cpu->hvf_fd, HV_X86_RBX, rbx);
             wreg(cpu->hvf_fd, HV_X86_RCX, rcx);
@@ -889,7 +951,7 @@  int hvf_vcpu_exec(CPUState *cpu)
                 break;
             }
             case 4: {
-                macvm_set_cr4(cpu->hvf_fd, RRX(env, reg));
+                macvm_set_cr4(env, cpu->hvf_fd, RRX(env, reg));
                 break;
             }
             case 8: {
@@ -966,6 +1028,8 @@  static int hvf_accel_init(MachineState *ms)
     hv_return_t ret;
     HVFState *s;
 
+    enable_avx512_thread_state();
+
     ret = hv_vm_create(HV_VM_DEFAULT);
     assert_hvf_ok(ret);
 
diff --git a/target/i386/hvf/vmx.h b/target/i386/hvf/vmx.h
index 1a1b150c97..dccd5ceb0f 100644
--- a/target/i386/hvf/vmx.h
+++ b/target/i386/hvf/vmx.h
@@ -157,13 +157,20 @@  static inline void macvm_set_cr0(hv_vcpuid_t vcpu, uint64_t cr0)
     hv_vcpu_flush(vcpu);
 }
 
-static inline void macvm_set_cr4(hv_vcpuid_t vcpu, uint64_t cr4)
+static inline void macvm_set_cr4(CPUX86State *env, hv_vcpuid_t vcpu,
+                                 uint64_t cr4)
 {
     uint64_t guest_cr4 = cr4 | CR4_VMXE_MASK;
 
     wvmcs(vcpu, VMCS_GUEST_CR4, guest_cr4);
     wvmcs(vcpu, VMCS_CR4_SHADOW, cr4);
 
+    /*
+     * Track whether OSXSAVE is enabled so we can properly return it
+     * for CPUID 1.
+     */
+    env->osxsave_enabled = ((cr4 & CR4_OSXSAVE_MASK) != 0);
+
     hv_vcpu_invalidate_tlb(vcpu);
     hv_vcpu_flush(vcpu);
 }
diff --git a/target/i386/hvf/x86hvf.c b/target/i386/hvf/x86hvf.c
index edefe5319a..bd25bf19c4 100644
--- a/target/i386/hvf/x86hvf.c
+++ b/target/i386/hvf/x86hvf.c
@@ -100,7 +100,7 @@  void hvf_put_segments(CPUState *cpu_state)
     vmx_update_tpr(cpu_state);
     wvmcs(cpu_state->hvf_fd, VMCS_GUEST_IA32_EFER, env->efer);
 
-    macvm_set_cr4(cpu_state->hvf_fd, env->cr[4]);
+    macvm_set_cr4(env, cpu_state->hvf_fd, env->cr[4]);
     macvm_set_cr0(cpu_state->hvf_fd, env->cr[0]);
 
     hvf_set_segment(cpu_state, &seg, &env->segs[R_CS], false);