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;
