Re: [PATCH] KVM: x86: Advertise AVX10.1 CPUID to userspace

[Sean Christopherson <seanjc@google.com>]

On Mon, May 20, 2024, Tao Su wrote:
> @@ -1162,6 +1162,22 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
>  			break;
>  		}
>  		break;
> +	case 0x24: {
> +		u8 avx10_version;
> +		u32 vector_support;
> +
> +		if (!kvm_cpu_cap_has(X86_FEATURE_AVX10)) {
> +			entry->eax = entry->ebx = entry->ecx = entry->edx = 0;
> +			break;
> +		}
> +		avx10_version = min(entry->ebx & 0xff, 1);

Taking the min() of '1' and anything else is pointless.  Per the spec, the version
can never be 0.

  CPUID.(EAX=24H, ECX=00H):EBX[bits 7:0]  Reports the Intel AVX10 Converged Vector ISA version. Integer (≥ 1)

And it's probably too late, but why on earth is there an AVX10 version number?
Version numbers are _awful_ for virtualization; see the constant vPMU problems
that arise from bundling things under a single version number..  Y'all carved out
room for sub-leafs, i.e. there's a ton of room for "discrete feature bits", so
why oh why is there a version number?

> +		vector_support = entry->ebx & GENMASK(18, 16);

Please add proper defines somewhere, this this can be something like:

		/* EBX[7:0] hold the AVX10 version; KVM supports version '1'. */
		entry->eax = 0;
		entry->ebx = (entry->ebx & AVX10_VECTOR_SIZES_MASK) | 1;
		entry->ecx = 0;
		entry->edx = 0;

Or perhaps we should have feature bits for the vector sizes, because that's really
what they are.  Mixing feature bits in with a version number makes for painful
code, but there's nothing KVM can do about that.  With proper features, this then
becomes something like:

		entry->eax = 0;
		cpuid_entry_override(entry, CPUID_24_0_EBX);
		/* EBX[7:0] hold the AVX10 version; KVM supports version '1'. */
		entry->ebx |= 1;
		entry->ecx = 0;
		entry->edx = 0;