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@xxxxxxxxxx>
> Signed-off-by: Aaron Lewis <aaronlewis@xxxxxxxxxx>
> ---
> 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.
