Re: [PATCH] KVM: x86: Enable Intel AVX-512 for guest

[Paolo Bonzini <pbonzini@redhat.com>]

On 10/22/2014 11:35 AM, Chao Peng wrote:
> Expose Intel AVX-512 feature bits to guest. Also add checks for
> xcr0 AVX512 related bits according to spec:
> http://download-software.intel.com/sites/default/files/managed/71/2e/319433-017.pdf
> 
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>

The patch looks good, but you also have to patch QEMU in order to
save/restore the values of the registers.  IIRC the manual already
details where the registers are in the XSAVE area, so it should be easy
to get them in and out.  You can look at the MPX patches for an example.

In the meanwhile, kernel bits are

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

> ---
>  arch/x86/include/asm/xsave.h |    1 +
>  arch/x86/kvm/cpuid.c         |    3 ++-
>  arch/x86/kvm/x86.c           |    6 ++++++
>  arch/x86/kvm/x86.h           |    3 ++-
>  4 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/xsave.h b/arch/x86/include/asm/xsave.h
> index 7e7a79a..5fa9770 100644
> --- a/arch/x86/include/asm/xsave.h
> +++ b/arch/x86/include/asm/xsave.h
> @@ -16,6 +16,7 @@
>  #define XSTATE_Hi16_ZMM		0x80
>  
>  #define XSTATE_FPSSE	(XSTATE_FP | XSTATE_SSE)
> +#define XSTATE_AVX512	(XSTATE_OPMASK | XSTATE_ZMM_Hi256 | XSTATE_Hi16_ZMM)
>  /* Bit 63 of XCR0 is reserved for future expansion */
>  #define XSTATE_EXTEND_MASK	(~(XSTATE_FPSSE | (1ULL << 63)))
>  
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 976e3a5..20d8321 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -317,7 +317,8 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  	const u32 kvm_supported_word9_x86_features =
>  		F(FSGSBASE) | F(BMI1) | F(HLE) | F(AVX2) | F(SMEP) |
>  		F(BMI2) | F(ERMS) | f_invpcid | F(RTM) | f_mpx | F(RDSEED) |
> -		F(ADX) | F(SMAP);
> +		F(ADX) | F(SMAP) | F(AVX512F) | F(AVX512PF) | F(AVX512ER) |
> +		F(AVX512CD);
>  
>  	/* all calls to cpuid_count() should be made on the same cpu */
>  	get_cpu();
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 5430e4b..3d77b88 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -651,6 +651,12 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
>  	if ((!(xcr0 & XSTATE_BNDREGS)) != (!(xcr0 & XSTATE_BNDCSR)))
>  		return 1;
>  
> +	if (xcr0 & XSTATE_AVX512) {
> +		if (!(xcr0 & XSTATE_YMM))
> +			return 1;
> +		if ((xcr0 & XSTATE_AVX512) != XSTATE_AVX512)
> +			return 1;
> +	}
>  	kvm_put_guest_xcr0(vcpu);
>  	vcpu->arch.xcr0 = xcr0;
>  
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index 7cb9c45..cc1d61a 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -162,7 +162,8 @@ int kvm_write_guest_virt_system(struct x86_emulate_ctxt *ctxt,
>  bool kvm_mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data);
>  
>  #define KVM_SUPPORTED_XCR0     (XSTATE_FP | XSTATE_SSE | XSTATE_YMM \
> -				| XSTATE_BNDREGS | XSTATE_BNDCSR)
> +				| XSTATE_BNDREGS | XSTATE_BNDCSR \
> +				| XSTATE_AVX512)
>  extern u64 host_xcr0;
>  
>  extern u64 kvm_supported_xcr0(void);
>