Thanks Zhao, Igor and Xiaoyao, I have sent v1 for this https://patchwork.ozlabs.org/project/qemu-devel/patch/20240724075226.212882-1-manish.mis...@nutanix.com/.
On 23/07/24 8:33 pm, Igor Mammedov wrote: !-------------------------------------------------------------------| CAUTION: External Email |-------------------------------------------------------------------! On Mon, 22 Jul 2024 10:18:59 +0000 "manish.mishra" <manish.mis...@nutanix.com><mailto:manish.mis...@nutanix.com> wrote: QEMU does not set 0x1f in case VM does not have extended CPU topology and expects guests to fallback to 0xb. Some versions of windows i.e. windows 10, 11 does not like this behavior and expects this leaf to be ^^^^^^^^^^^^^ be more clear about Done now, please let me know if i need to add more details. populated. This is observed with windows VMs with secure boot, uefi and HyperV role enabled. add here exact QME CLI and necessary guest OS details to reproduce the issue. Done Leaf 0x1f is superset of 0xb, so it makes sense to set 0x1f equivalent to 0xb by default and workaround windows issue. This change adds a new property 'cpuid-0x1f-enforce' to set leaf 0x1f equivalent to 0xb in case extended CPU topology is not present and behave as before otherwise. --- maybe instead of adding workaround it would be better to enable 1F leaf on CPU models that supposed to have it? Windows expects it only when we have set max cpuid level greater than or equal to 0x1f. I mean if it is exposed it should not be all zeros. SapphireRapids CPU definition raised cpuid level to 0x20, so we starting seeing it with SapphireRapids. hw/i386/pc.c | 1 + target/i386/cpu.c | 71 +++++++++++++++++++++++++++---------------- target/i386/cpu.h | 5 +++ target/i386/kvm/kvm.c | 4 ++- 4 files changed, 53 insertions(+), 28 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index c74931d577..4cab04e443 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -85,6 +85,7 @@ GlobalProperty pc_compat_9_0[] = { { TYPE_X86_CPU, "guest-phys-bits", "0" }, { "sev-guest", "legacy-vm-type", "on" }, { TYPE_X86_CPU, "legacy-multi-node", "on" }, + { TYPE_X86_CPU, "cpuid-0x1f-enforce", "false" }, }; const size_t pc_compat_9_0_len = G_N_ELEMENTS(pc_compat_9_0); diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 4688d140c2..f89b2ef335 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -416,6 +416,43 @@ static void encode_topo_cpuid1f(CPUX86State *env, uint32_t count, assert(!(*eax & ~0x1f)); } +static void encode_topo_cpuid_b(CPUX86State *env, uint32_t count, + X86CPUTopoInfo *topo_info, + uint32_t threads_per_pkg, + uint32_t *eax, uint32_t *ebx, + uint32_t *ecx, uint32_t *edx) +{ + X86CPU *cpu = env_archcpu(env); + + if (!cpu->enable_cpuid_0xb) { + *eax = *ebx = *ecx = *edx = 0; + return; + } + + *ecx = count & 0xff; + *edx = cpu->apic_id; + + switch (count) { + case 0: + *eax = apicid_core_offset(topo_info); + *ebx = topo_info->threads_per_core; + *ecx |= CPUID_B_ECX_TOPO_LEVEL_SMT << 8; + break; + case 1: + *eax = apicid_pkg_offset(topo_info); + *ebx = threads_per_pkg; + *ecx |= CPUID_B_ECX_TOPO_LEVEL_CORE << 8; + break; + default: + *eax = 0; + *ebx = 0; + *ecx |= CPUID_B_ECX_TOPO_LEVEL_INVALID << 8; + } + + assert(!(*eax & ~0x1f)); + *ebx &= 0xffff; /* The count doesn't need to be reliable. */ +} + /* Encode cache info for CPUID[0x80000005].ECX or CPUID[0x80000005].EDX */ static uint32_t encode_cache_cpuid80000005(CPUCacheInfo *cache) { @@ -6601,33 +6638,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, break; case 0xB: /* Extended Topology Enumeration Leaf */ - if (!cpu->enable_cpuid_0xb) { - *eax = *ebx = *ecx = *edx = 0; - break; - } - - *ecx = count & 0xff; - *edx = cpu->apic_id; - - switch (count) { - case 0: - *eax = apicid_core_offset(&topo_info); - *ebx = topo_info.threads_per_core; - *ecx |= CPUID_B_ECX_TOPO_LEVEL_SMT << 8; - break; - case 1: - *eax = apicid_pkg_offset(&topo_info); - *ebx = threads_per_pkg; - *ecx |= CPUID_B_ECX_TOPO_LEVEL_CORE << 8; - break; - default: - *eax = 0; - *ebx = 0; - *ecx |= CPUID_B_ECX_TOPO_LEVEL_INVALID << 8; - } - - assert(!(*eax & ~0x1f)); - *ebx &= 0xffff; /* The count doesn't need to be reliable. */ + encode_topo_cpuid_b(env, count, &topo_info, threads_per_pkg, + eax, ebx, ecx, edx); break; case 0x1C: if (cpu->enable_pmu && (env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_ARCH_LBR)) { @@ -6639,6 +6651,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, /* V2 Extended Topology Enumeration Leaf */ if (!x86_has_extended_topo(env->avail_cpu_topo)) { *eax = *ebx = *ecx = *edx = 0; + if (cpu->enable_cpuid_0x1f_enforce) { + encode_topo_cpuid_b(env, count, &topo_info, threads_per_pkg, + eax, ebx, ecx, edx); + } break; } @@ -8316,6 +8332,7 @@ static Property x86_cpu_properties[] = { DEFINE_PROP_BOOL("full-cpuid-auto-level", X86CPU, full_cpuid_auto_level, true), DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor), DEFINE_PROP_BOOL("cpuid-0xb", X86CPU, enable_cpuid_0xb, true), + DEFINE_PROP_BOOL("cpuid-0x1f-enforce", X86CPU, enable_cpuid_0x1f_enforce, true), DEFINE_PROP_BOOL("x-vendor-cpuid-only", X86CPU, vendor_cpuid_only, true), DEFINE_PROP_BOOL("x-amd-topoext-features-only", X86CPU, amd_topoext_features_only, true), DEFINE_PROP_BOOL("lmce", X86CPU, enable_lmce, false), diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 1e121acef5..718b9f2b0b 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -2102,6 +2102,11 @@ struct ArchCPU { /* Compatibility bits for old machine types: */ bool enable_cpuid_0xb; + /* Always return values for 0x1f leaf. In cases where extended CPU topology + * is not supported, return values equivalent of leaf 0xb. + */ + bool enable_cpuid_0x1f_enforce; + /* Enable auto level-increase for all CPUID leaves */ bool full_cpuid_auto_level; diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index becca2efa5..a9c6f02900 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -1799,6 +1799,7 @@ static uint32_t kvm_x86_build_cpuid(CPUX86State *env, uint32_t limit, i, j; uint32_t unused; struct kvm_cpuid_entry2 *c; + X86CPU *cpu = env_archcpu(env); cpu_x86_cpuid(env, 0, 0, &limit, &unused, &unused, &unused); @@ -1831,7 +1832,8 @@ static uint32_t kvm_x86_build_cpuid(CPUX86State *env, break; } case 0x1f: - if (!x86_has_extended_topo(env->avail_cpu_topo)) { + if (!x86_has_extended_topo(env->avail_cpu_topo) && + !cpu->enable_cpuid_0x1f_enforce) { cpuid_i--; break; }
