Re: [PATCH v2 2/6] KVM: x86: Clear all supported AVX-512 xfeatures if they are not all set

[Sean Christopherson <seanjc@google.com>]

On Fri, Dec 30, 2022, Aaron Lewis wrote:
> Be a good citizen and don't allow any of the supported AVX-512
> xfeatures[1] to be set if they can't all be set.  That way userspace or
> a guest doesn't fail if it attempts to set them in XCR0.

The form letter shortlog+changelo+code doesn't fit AVX-512.  There's only one
AVX512 flag, SSE and AVX and are pure prerequisites and exist independently of
AVX512.

> It's important to note that in order to set any of the AVX-512
> xfeatures, the SSE[bit-1] and AVX[bit-2] must also be set.
> 
> [1] CPUID.(EAX=0DH,ECX=0):EAX.OPMASK[bit-5]
>     CPUID.(EAX=0DH,ECX=0):EAX.ZMM_Hi256[bit-6]
>     CPUID.(EAX=0DH,ECX=0):EAX.ZMM_Hi16_ZMM[bit-7]
> 
> Suggested-by: Jim Mattson <jmattson@google.com>
> Signed-off-by: Aaron Lewis <aaronlewis@google.com>
> ---
>  arch/x86/kvm/cpuid.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 2431c46d456b4..89ad8cd865173 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -862,6 +862,10 @@ static u64 sanitize_xcr0(u64 xcr0) {
>  	if ((xcr0 & mask) != mask)
>  		xcr0 &= ~mask;
>  
> +	mask = XFEATURE_MASK_SSE | XFEATURE_MASK_YMM | XFEATURE_MASK_AVX512;

Checking AVX512 is unnecessary.  If it's not set, the AND-NOT is a nop.

> +	if ((xcr0 & mask) != mask)
> +		xcr0 &= ~XFEATURE_MASK_AVX512;

This can be:

	if (!(xcr0 & XFEATURE_MASK_SSE) || !(xcr0 & XFEATURE_MASK_YMM))
		xcr0 &= ~XFEATURE_MASK_AVX512

to better capture the dependency.