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

[Paolo Bonzini <pbonzini@redhat.com>]

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