* [PATCH] target/i386: do not consult nonexistent host leaves
@ 2022-04-29 19:26 Paolo Bonzini
2022-05-01 11:18 ` Maxim Levitsky
0 siblings, 1 reply; 2+ messages in thread
From: Paolo Bonzini @ 2022-04-29 19:26 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Maxim Levitsky
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 <mlevitsk@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@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) {
--
2.35.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] target/i386: do not consult nonexistent host leaves
2022-04-29 19:26 [PATCH] target/i386: do not consult nonexistent host leaves Paolo Bonzini
@ 2022-05-01 11:18 ` Maxim Levitsky
0 siblings, 0 replies; 2+ messages in thread
From: Maxim Levitsky @ 2022-05-01 11:18 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: qemu-stable
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 <mlevitsk@redhat.com>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Paolo Bonzini <pbonzini@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 <mlevisk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-05-01 11:20 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-29 19:26 [PATCH] target/i386: do not consult nonexistent host leaves Paolo Bonzini
2022-05-01 11:18 ` Maxim Levitsky
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.