On 6/20/2025 5:27 PM, Zhao Liu wrote: > As preparation for merging cache_info_cpuid4 and cache_info_amd in > X86CPUState, set legacy cache model based on vendor in the CPUID 0x2 > leaf. For AMD CPU, select legacy AMD cache model (in cache_info_amd) as > the default cache model, otherwise, select legacy Intel cache model (in > cache_info_cpuid4) as before. > > To ensure compatibility is not broken, add an enable_legacy_vendor_cache > flag based on x-vendor-only-v2 to indicate cases where the legacy cache > model should be used regardless of the vendor. For CPUID 0x2 leaf, > enable_legacy_vendor_cache flag indicates to pick legacy Intel cache > model, which is for compatibility with the behavior of PC machine v10.0 > and older. > > The following explains how current vendor-based default legacy cache > model ensures correctness without breaking compatibility. > > * For the PC machine v6.0 and older, vendor_cpuid_only=false, and > vendor_cpuid_only_v2=false. > > - If the named CPU model has its own cache model, and doesn't use > legacy cache model (legacy_cache=false), then cache_info_cpuid4 and > cache_info_amd are same, so 0x2 leaf uses its own cache model > regardless of the vendor. > > - For max/host/named CPU (without its own cache model), then the flag > enable_legacy_vendor_cache is true, they will use legacy Intel cache > model just like their previous behavior. > > * For the PC machine v10.0 and older (to v6.1), vendor_cpuid_only=true, > and vendor_cpuid_only_v2=false. > > - If the named CPU model has its own cache model (legacy_cache=false), > then cache_info_cpuid4 & cache_info_amd both equal to its own cache > model, so it uses its own cache model in 0x2 leaf regardless of the > vendor. Only AMD CPUs have all-0 leaf due to vendor_cpuid_only=true, > and this is exactly the behavior of these old machines. > > - For max/host/named CPU (without its own cache model), then the flag > enable_legacy_vendor_cache is true, they will use legacy Intel cache > model. Similarly, only AMD CPUs have all-0 leaf, and this is exactly > the behavior of these old machines. > > * For the PC machine v10.1 and newer, vendor_cpuid_only=true, and > vendor_cpuid_only_v2=true. > > - If the named CPU model has its own cache model (legacy_cache=false), > then cache_info_cpuid4 & cache_info_amd both equal to its own cache > model, so it uses its own cache model in 0x2 leaf regardless of the > vendor. And AMD CPUs have all-0 leaf. Nothing will change. > > - For max/host/named CPU (without its own cache model), then the flag > enable_legacy_vendor_cache is false, the legacy cache model is > selected based on vendor. > > For AMD CPU, it will use legacy AMD cache but still get all-0 leaf > due to vendor_cpuid_only=true. > > For non-AMD (Intel/Zhaoxin) CPU, it will use legacy Intel cache as > expected. > > Here, selecting the legacy cache model based on the vendor does not > change the previous (before the change) behavior. > > Therefore, the above analysis proves that, with the help of the flag > enable_legacy_vendor_cache, it is acceptable to select the default > legacy cache model based on the vendor. > > For the CPUID 0x2 leaf, in X86CPUState, a unified cache_info is enough. > It only needs to be initialized and configured with the corresponding > legacy cache model based on the vendor. > > Signed-off-by: Zhao Liu <zhao1....@intel.com> > --- > target/i386/cpu.c | 47 +++++++++++++++++++++++++++++++++++++---------- > target/i386/cpu.h | 1 + > 2 files changed, 38 insertions(+), 10 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index bf8d7a19c88d..524d39de9ace 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -248,23 +248,17 @@ static const CPUCaches legacy_intel_cpuid2_cache_info; > > /* Encode cache info for CPUID[4] */ > static void encode_cache_cpuid2(X86CPU *cpu, > + const CPUCaches *caches, > uint32_t *eax, uint32_t *ebx, > uint32_t *ecx, uint32_t *edx) > { > CPUX86State *env = &cpu->env; > - const CPUCaches *caches; > int l1d, l1i, l2, l3; > bool unmatched = false; > > *eax = 1; /* Number of CPUID[EAX=2] calls required */ > *ebx = *ecx = *edx = 0; > > - if (env->enable_legacy_cpuid2_cache) { > - caches = &legacy_intel_cpuid2_cache_info; > - } else { > - caches = &env->cache_info_cpuid4; > - } > - > l1d = cpuid2_cache_descriptor(caches->l1d_cache, &unmatched); > l1i = cpuid2_cache_descriptor(caches->l1i_cache, &unmatched); > l2 = cpuid2_cache_descriptor(caches->l2_cache, &unmatched); > @@ -7482,8 +7476,37 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, > uint32_t count, > *ecx &= ~CPUID_EXT_PDCM; > } > break; > - case 2: > - /* cache info: needed for Pentium Pro compatibility */ > + case 2: { /* cache info: needed for Pentium Pro compatibility */ > + const CPUCaches *caches; > + > + if (env->enable_legacy_cpuid2_cache) { > + caches = &legacy_intel_cpuid2_cache_info; > + } else if (env->enable_legacy_vendor_cache) { > + caches = &legacy_intel_cache_info; > + } else { > + /* > + * FIXME: Temporarily select cache info model here based on > + * vendor, and merge these 2 cache info models later. > + * > + * This condition covers the following cases (with > + * enable_legacy_vendor_cache=false): > + * - When CPU model has its own cache model and doesn't use > legacy > + * cache model (legacy_model=off). Then cache_info_amd and > + * cache_info_cpuid4 are the same. > + * > + * - For v10.1 and newer machines, when CPU model uses legacy > cache > + * model. Non-AMD CPUs use cache_info_cpuid4 like before and > AMD > + * CPU will use cache_info_amd. But this doesn't matter for > AMD > + * CPU, because this leaf encodes all-0 for AMD whatever its > cache > + * model is. > + */ > + if (IS_AMD_CPU(env)) { > + caches = &env->cache_info_amd; > + } else { > + caches = &env->cache_info_cpuid4; > + } > + } > + > if (cpu->cache_info_passthrough) { > x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx); > break; > @@ -7491,8 +7514,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, > uint32_t count, > *eax = *ebx = *ecx = *edx = 0; > break; > } > - encode_cache_cpuid2(cpu, eax, ebx, ecx, edx); > + encode_cache_cpuid2(cpu, caches, eax, ebx, ecx, edx); > break; > + } > case 4: > /* cache info: needed for Core compatibility */ > if (cpu->cache_info_passthrough) { > @@ -8979,6 +9003,9 @@ static void x86_cpu_realizefn(DeviceState *dev, Error > **errp) > env->enable_legacy_cpuid2_cache = true; > } > > + if (!cpu->vendor_cpuid_only_v2) { > + env->enable_legacy_vendor_cache = true; > + } > env->cache_info_cpuid4 = legacy_intel_cache_info; > env->cache_info_amd = legacy_amd_cache_info; > } > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > index 02cda176798f..243383efd602 100644 > --- a/target/i386/cpu.h > +++ b/target/i386/cpu.h > @@ -2078,6 +2078,7 @@ typedef struct CPUArchState { > */ > CPUCaches cache_info_cpuid4, cache_info_amd; > bool enable_legacy_cpuid2_cache; > + bool enable_legacy_vendor_cache; > > /* MTRRs */ > uint64_t mtrr_fixed[11];
LGTM. Thanks. Reviewed-by: Dapeng Mi <dapeng1...@linux.intel.com>