On Fri, 2022-04-29 at 21:26 +0200, Paolo Bonzini wrote:
> When cache_info_passthrough is requested, QEMU passes the host values
> of the cache information CPUID leaves down to the guest. However,
> it blindly assumes that the CPUID leaf exists on the host, and this
> cannot be guaranteed: for example, KVM has recently started to
> synthesize AMD leaves up to 0x80000021 in order to provide accurate
> CPU bug information to guests.
>
> Querying a nonexistent host leaf fills the output arguments of
> host_cpuid with data that (albeit deterministic) is nonsensical
> as cache information, namely the data in the highest Intel CPUID
> leaf. If said highest leaf is not ECX-dependent, this can even
> cause an infinite loop when kvm_arch_init_vcpu prepares the input
> to KVM_SET_CPUID2. The infinite loop is only terminated by an
> abort() when the array gets full.
>
> Reported-by: Maxim Levitsky <mlevi...@redhat.com>
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> target/i386/cpu.c | 41 ++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 36 insertions(+), 5 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 99343be926..c5461f7c0b 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -5022,6 +5022,37 @@ uint64_t
> x86_cpu_get_supported_feature_word(FeatureWord w,
> return r;
> }
>
> +static void x86_cpu_get_cache_cpuid(uint32_t func, uint32_t index,
> + uint32_t *eax, uint32_t *ebx,
> + uint32_t *ecx, uint32_t *edx)
> +{
> + uint32_t level, unused;
> +
> + /* Only return valid host leaves. */
> + switch (func) {
> + case 2:
> + case 4:
> + host_cpuid(0, 0, &level, &unused, &unused, &unused);
> + break;
> + case 0x80000005:
> + case 0x80000006:
> + case 0x8000001d:
> + host_cpuid(0x80000000, 0, &level, &unused, &unused, &unused);
> + break;
> + default:
> + return;
> + }
> +
> + if (func > level) {
> + *eax = 0;
> + *ebx = 0;
> + *ecx = 0;
> + *edx = 0;
> + } else {
> + host_cpuid(func, index, eax, ebx, ecx, edx);
> + }
> +}
> +
> /*
> * Only for builtin_x86_defs models initialized with
> x86_register_cpudef_types.
> */
> @@ -5280,7 +5311,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
> uint32_t count,
> case 2:
> /* cache info: needed for Pentium Pro compatibility */
> if (cpu->cache_info_passthrough) {
> - host_cpuid(index, 0, eax, ebx, ecx, edx);
> + x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
> break;
> } else if (cpu->vendor_cpuid_only && IS_AMD_CPU(env)) {
> *eax = *ebx = *ecx = *edx = 0;
> @@ -5300,7 +5331,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
> uint32_t count,
> case 4:
> /* cache info: needed for Core compatibility */
> if (cpu->cache_info_passthrough) {
> - host_cpuid(index, count, eax, ebx, ecx, edx);
> + x86_cpu_get_cache_cpuid(index, count, eax, ebx, ecx, edx);
> /* QEMU gives out its own APIC IDs, never pass down bits 31..26.
> */
> *eax &= ~0xFC000000;
> if ((*eax & 31) && cs->nr_cores > 1) {
> @@ -5702,7 +5733,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
> uint32_t count,
> case 0x80000005:
> /* cache info (L1 cache) */
> if (cpu->cache_info_passthrough) {
> - host_cpuid(index, 0, eax, ebx, ecx, edx);
> + x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
> break;
> }
> *eax = (L1_DTLB_2M_ASSOC << 24) | (L1_DTLB_2M_ENTRIES << 16) |
> @@ -5715,7 +5746,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
> uint32_t count,
> case 0x80000006:
> /* cache info (L2 cache) */
> if (cpu->cache_info_passthrough) {
> - host_cpuid(index, 0, eax, ebx, ecx, edx);
> + x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
> break;
> }
> *eax = (AMD_ENC_ASSOC(L2_DTLB_2M_ASSOC) << 28) |
> @@ -5775,7 +5806,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
> uint32_t count,
> case 0x8000001D:
> *eax = 0;
> if (cpu->cache_info_passthrough) {
> - host_cpuid(index, count, eax, ebx, ecx, edx);
> + x86_cpu_get_cache_cpuid(index, count, eax, ebx, ecx, edx);
> break;
> }
> switch (count) {
Makes sense.
Reviewed-by: Maxim Levitsky <mlev...@redhat.com>
Best regards,
Maxim Levitsky
