Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel

[Zhao Liu <zhao1.liu@intel.com>]

> On 4/23/25 7:46 PM, Zhao Liu wrote:
> > 
> > Per SDM, 0x80000005 leaf is reserved for Intel CPU, and its current
> > "assert" check blocks adding new cache model for non-AMD CPUs.
> > 
> > Therefore, check the vendor and encode this leaf as all-0 for Intel
> > CPU. And since Zhaoxin mostly follows Intel behavior, apply the vendor
> > check for Zhaoxin as well.
> 
> Thanks for taking Zhaoxin CPUs into account.
> 
> Zhaoxin follows AMD's definition for CPUID leaf 0x80000005, so this leaf is
> valid on our CPUs rather than reserved. We do, however, follow Intel's
> definition for leaf 0x80000006.

Thank you! I'll revert the change.

> > Note, for !vendor_cpuid_only case, non-AMD CPU would get the wrong
> > information, i.e., get AMD's cache model for Intel or Zhaoxin CPUs.
> > For this case, there is no need to tweak for non-AMD CPUs, because
> > vendor_cpuid_only has been turned on by default since PC machine v6.1.
> > 
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > ---
> >   target/i386/cpu.c | 16 ++++++++++++++--
> >   1 file changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 1b64ceaaba46..8fdafa8aedaf 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -7248,11 +7248,23 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> >           *edx = env->cpuid_model[(index - 0x80000002) * 4 + 3];
> >           break;
> >       case 0x80000005:
> > -        /* cache info (L1 cache) */
> > -        if (cpu->cache_info_passthrough) {
> > +        /*
> > +         * cache info (L1 cache)
> > +         *
> > +         * For !vendor_cpuid_only case, non-AMD CPU would get the wrong
> > +         * information, i.e., get AMD's cache model. It doesn't matter,
> > +         * vendor_cpuid_only has been turned on by default since
> > +         * PC machine v6.1.
> > +         */
> > +        if (cpu->vendor_cpuid_only &&
> > +            (IS_INTEL_CPU(env) || IS_ZHAOXIN_CPU(env))) {
> 
> Given that, there is no need to add IS_ZHAOXIN_CPU(env) to the 0x80000005
> path. Note that the L1 TLB constants for the YongFeng core differ from the
> current values in target/i386/cpu.c(YongFeng defaults shown in brackets):
> 
> #define L1_DTLB_2M_ASSOC       1 (4)
> #define L1_DTLB_2M_ENTRIES   255 (32)
> #define L1_DTLB_4K_ASSOC       1 (6)
> #define L1_DTLB_4K_ENTRIES   255 (96)
> 
> #define L1_ITLB_2M_ASSOC       1 (4)
> #define L1_ITLB_2M_ENTRIES   255 (32)
> #define L1_ITLB_4K_ASSOC       1 (6)
> #define L1_ITLB_4K_ENTRIES   255 (96)
> 
> I am still reviewing how these constants flow through cpu_x86_cpuid() for
> leaf 0x80000005, so I'm not yet certain whether they are overridden.
> 
> For now, the patchset can ignore Zhaoxin in leaf 0x80000005. Once I have
> traced the code path, I will send an update if needed. Please include
> Zhaoxin in the handling for leaf 0x80000006.

Sure! And you can add more comment for the special cases.

If possible, could you please help check if there are any other cases :-)
without spec reference, it's easy to cause regression when refactor. As
per Xiaoyao's comment, we would like to further clean up the Intel/AMD
features by minimizing the “incorrect” AMD features in Intel Guests
when vendor_cpuid_only (or another enhanced compat option) is turned on.

> I should have sent this after completing my review, but I did not want to
> delay your work. Sorry for the noise.

No problem. And your info really helps me. It will take me a little
while until the next version. It makes it easier to decouple our work
and review them separately in the community!

> Thanks again for your work.

You're welcome!