Re: [PATCH 4/8] i386/cpu: Introduce cache model for YongFeng

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

> > +            {
> > +                .version = 3,
> > +                .note = "with the cache info",
> 
> I realize that my previous use of "cache info" was not precise; "cache
> model" is more appropriate. Please help me adjust accordingly, thank you.

Nope, will fix.

> > +                .cache_info = &yongfeng_cache_info
> > +            },
> >               { /* end of list */ }
> >           }
> >       },
> > --
> > 2.34.1
> > 
> 
> Hi Zhao,
> 
> I tested the patchsets you provided on different hosts, and here are the results:
> 
> 1. On an Intel host with KVM enabled
> The CPUID leaves 0x2 and 0x4 reported inside the YongFeng-V3 VM match our
> expected cache details exactly. However, CPUID leaf 0x80000005 returns all
> zeros. This is because when KVM is in use, QEMU uses the host's vendor for
> the IS_INTEL_CPU(env), IS_ZHAOXIN_CPU(env), and IS_AMD_CPU(env) checks.

This is a bug:

https://lore.kernel.org/qemu-devel/d429b6f5-b59c-4884-b18f-8db71cb8dc7b@oracle.com/

And we expect we can change the vendor with KVM.

> Given that behavior, a zeroed 0x80000005 leaf in the guest is expected and,
> to me, acceptable. What are your thoughts?

Well, (with this bug) since VM is "Intel" vendor, so this is correct.

> 2. On a YongFeng host (with or without KVM)
> The CPUID leaves 0x2, 0x4, and 0x80000006 inside the VM all return the
> values we want, and the L1D/L1I cache info in leaf 0x80000005 is also
> correct.

Nice!

> 3. TLB info in leaf 0x80000005
> On both Intel and YongFeng hosts, the L1 TLB fields in leaf 0x80000005
> remain constant, as we discussed. As you mentioned before, "we can wait and
> see what maintainers think" about this.

Yes. I suppose Zhaoxin also uses 0x2 to present TLB info like Intel does.
To support TLB, I feel like there is still some work to be done, and it
depends on if it's worth it...

> In summary, both patchsets look good for Zhaoxin support, I don't see any
> issues so far.

Thanks!

> Btw, YongFeng host also support 0x1F, does YongFeng need to turn on
> "x-force-cpuid-0x1f" default ? I think maybe yes.

OK, will add it.

BTW...my colleague reports a bug that Intel/Zhaoxin CPUs with cache
model will meet assertion failure on the v10.0 or old machine.

So I think it's necessary to drop all the assert() checks on
lines_per_tag directly:

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 18bb0e9cf9f6..f73943a46945 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -491,7 +491,6 @@ static void encode_topo_cpuid1f(CPUX86State *env, uint32_t count,
 static uint32_t encode_cache_cpuid80000005(CPUCacheInfo *cache)
 {
     assert(cache->size % 1024 == 0);
-    assert(cache->lines_per_tag > 0);
     assert(cache->associativity > 0);
     assert(cache->line_size > 0);
     return ((cache->size / 1024) << 24) | (cache->associativity << 16) |
@@ -520,13 +519,10 @@ static uint32_t encode_cache_cpuid80000005(CPUCacheInfo *cache)
  */
 static void encode_cache_cpuid80000006(CPUCacheInfo *l2,
                                        CPUCacheInfo *l3,
-                                       uint32_t *ecx, uint32_t *edx,
-                                       bool lines_per_tag_supported)
+                                       uint32_t *ecx, uint32_t *edx)
 {
     assert(l2->size % 1024 == 0);
     assert(l2->associativity > 0);
-    assert(lines_per_tag_supported ?
-           l2->lines_per_tag > 0 : l2->lines_per_tag == 0);
     *ecx = ((l2->size / 1024) << 16) |
            (X86_ENC_ASSOC(l2->associativity) << 12) |
            (l2->lines_per_tag << 8) | (l2->line_size);
@@ -535,8 +531,6 @@ static void encode_cache_cpuid80000006(CPUCacheInfo *l2,
     if (l3) {
         assert(l3->size % (512 * 1024) == 0);
         assert(l3->associativity > 0);
-        assert(lines_per_tag_supported ?
-               l3->lines_per_tag > 0 : l3->lines_per_tag == 0);
         assert(l3->line_size > 0);
         *edx = ((l3->size / (512 * 1024)) << 18) |
                (X86_ENC_ASSOC(l3->associativity) << 12) |
@@ -8353,7 +8347,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             (IS_INTEL_CPU(env) || IS_ZHAOXIN_CPU(env))) {
             *eax = *ebx = 0;
             encode_cache_cpuid80000006(caches->l2_cache,
-                                       NULL, ecx, edx, false);
+                                       NULL, ecx, edx);
             break;
         }

@@ -8369,7 +8363,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         encode_cache_cpuid80000006(caches->l2_cache,
                                    cpu->enable_l3_cache ?
                                    caches->l3_cache : NULL,
-                                   ecx, edx, true);
+                                   ecx, edx);
         break;
     }
     case 0x80000007: