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 > 0x80000005 leaf. For AMD CPU, select legacy AMD cache model (in > cache_info_amd) as the default cache model like before, otherwise, > select legacy Intel cache model (in cache_info_cpuid4). > > 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 0x80000005 > leaf, enable_legacy_vendor_cache flag indicates to pick legacy AMD 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 0x80000005 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 AMD 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. > > - No change, since this leaf doesn't aware vendor_cpuid_only. > > * 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 0x80000005 leaf regardless > of the vendor. Only Intel CPUs have all-0 leaf due to > vendor_cpuid_only_2=true, and this is exactly the expected behavior. > > - 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 as expected. > > For Intel CPU, it will use legacy Intel cache but still get all-0 > leaf due to vendor_cpuid_only_2=true as expected. > > (Note) And for Zhaoxin CPU, it will use legacy Intel cache model > instead of AMD's. This is the difference brought by this change! But > it's correct since then Zhaoxin could have the consistent cache info > in CPUID 0x2, 0x4 and 0x80000005 leaves. > > Here, except Zhaoxin, selecting the legacy cache model based on the > vendor does not change the previous (before the change) behavior. > And the change for Zhaoxin is also a good improvement. > > 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 0x80000005 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. > > Cc: EwanHai <ewanhai...@zhaoxin.com> > Signed-off-by: Zhao Liu <zhao1....@intel.com> > --- > Note, side effect of this patch: fix the inconsistency cache info for > Zhaoxin. For more details, see the commit message above. > --- > target/i386/cpu.c | 36 ++++++++++++++++++++++++++++++++---- > 1 file changed, 32 insertions(+), 4 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index afbf11569ab4..16b4ecb76113 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -7945,8 +7945,35 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, > uint32_t count, > *ecx = env->cpuid_model[(index - 0x80000002) * 4 + 2]; > *edx = env->cpuid_model[(index - 0x80000002) * 4 + 3]; > break; > - case 0x80000005: > - /* cache info (L1 cache/TLB Associativity Field) */ > + case 0x80000005: { /* cache info (L1 cache/TLB Associativity Field) */ Better put the comment into an independent line.
> + const CPUCaches *caches; > + > + if (env->enable_legacy_vendor_cache) { > + caches = &legacy_amd_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 uses > 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. AMD CPUs use cache_info_amd like before and non-AMD > + * CPU will use cache_info_cpuid4. But this doesn't matter, > + * because for Intel CPU, it will get all-0 leaf, and Zhaoxin > CPU > + * will get correct cache info. Both are expected. > + */ > + 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; > @@ -7961,9 +7988,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, > uint32_t count, > (L1_ITLB_2M_ASSOC << 8) | (L1_ITLB_2M_ENTRIES); > *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) | > (L1_ITLB_4K_ASSOC << 8) | (L1_ITLB_4K_ENTRIES); > - *ecx = encode_cache_cpuid80000005(env->cache_info_amd.l1d_cache); > - *edx = encode_cache_cpuid80000005(env->cache_info_amd.l1i_cache); > + *ecx = encode_cache_cpuid80000005(caches->l1d_cache); > + *edx = encode_cache_cpuid80000005(caches->l1i_cache); > break; > + } > case 0x80000006: > /* cache info (L2 cache/TLB/L3 cache) */ > if (cpu->cache_info_passthrough) {