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

Zhao Liu posted 10 patches 6 months, 3 weeks ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Zhao Liu <zhao1.liu@intel.com>, Marcelo Tosatti <mtosatti@redhat.com>
[RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel
Posted by Zhao Liu 6 months, 3 weeks ago
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.

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))) {
+            *eax = *ebx = *ecx = *edx = 0;
+            break;
+        } else if (cpu->cache_info_passthrough) {
             x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
             break;
         }
+
         *eax = (L1_DTLB_2M_ASSOC << 24) | (L1_DTLB_2M_ENTRIES << 16) |
                (L1_ITLB_2M_ASSOC <<  8) | (L1_ITLB_2M_ENTRIES);
         *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) |
-- 
2.34.1
Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel
Posted by Ewan Hai 5 months, 3 weeks ago

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.
> 
> 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))) {
> +            *eax = *ebx = *ecx = *edx = 0;
> +            break;
> +        } else if (cpu->cache_info_passthrough) {
>               x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
>               break;
>           }
> +
>           *eax = (L1_DTLB_2M_ASSOC << 24) | (L1_DTLB_2M_ENTRIES << 16) |
>                  (L1_ITLB_2M_ASSOC <<  8) | (L1_ITLB_2M_ENTRIES);
>           *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) |

I've reviewed the cache-related CPUID path and noticed an oddity: every AMD vCPU 
model still reports identical hard-coded values for the L1 ITLB and L1 DTLB 
fields in leaf 0x8000_0005. Your patch fixes this for Intel(and Zhaoxin), but 
all AMD models continue to receive the same constants in EAX/EBX.

Do you know the reason for this choice? Is the guest expected to ignore those L1 
TLB numbers? If so, I'll prepare a patch that adjusts only the Zhaoxin defaults 
in leaf 0x8000_0005 like below, matching real YongFeng behaviour in ecx and edx, 
but keep eax and ebx following AMD's behaviour.

## Notes
1. Changes tied to "-machine smp-cache xxx" (mainly 
x86_cpu_update_smp_cache_topo()) are not included here.
2. Do you think I need Zhaoxin-specific legacy_l1d/l1i/l2/l3_cache helpers? If 
yes, I'll add them with YongFeng sillicon topology data.

--- patch prototype start ---

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 7b223642ba..8a17e5ffe9 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2726,6 +2726,66 @@ static const CPUCaches xeon_srf_cache_info = {
      },
  };

+static const CPUCaches yongfeng_cache_info = {
+    .l1d_cache = &(CPUCacheInfo) {
+        .type = DATA_CACHE,
+        .level = 1,
+        .size = 32 * KiB,
+        .line_size = 64,
+        .associativity = 8,
+        .partitions = 1,
+        .sets = 64,
+        .lines_per_tag = 1,
+        .inclusive = false,
+        .self_init = true,
+        .no_invd_sharing = false,
+        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
+    },
+    .l1i_cache = &(CPUCacheInfo) {
+        .type = INSTRUCTION_CACHE,
+        .level = 1,
+        .size = 64 * KiB,
+        .line_size = 64,
+        .associativity = 16,
+        .partitions = 1,
+        .sets = 64,
+        .lines_per_tag = 1,
+        .inclusive = false,
+        .self_init = true,
+        .no_invd_sharing = false,
+        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
+    },
+    .l2_cache = &(CPUCacheInfo) {
+        .type = UNIFIED_CACHE,
+        .level = 2,
+        .size = 256 * KiB,
+        .line_size = 64,
+        .associativity = 8,
+        .partitions = 1,
+        .sets = 512,
+        .lines_per_tag = 1,
+        .inclusive = true,
+        .self_init = true,
+        .no_invd_sharing = false,
+        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
+    },
+    .l3_cache = &(CPUCacheInfo) {
+        .type = UNIFIED_CACHE,
+        .level = 3,
+        .size = 8 * MiB,
+        .line_size = 64,
+        .associativity = 16,
+        .partitions = 1,
+        .sets = 8192,
+        .lines_per_tag = 1,
+        .self_init = true,
+        .inclusive = true,
+        .no_invd_sharing = true,
+        .complex_indexing = false,
+        .share_level = CPU_TOPOLOGY_LEVEL_DIE,
+    },
+};
+
  /* The following VMX features are not supported by KVM and are left out in the
   * CPU definitions:
   *
@@ -5928,6 +5988,15 @@ static const X86CPUDefinition builtin_x86_defs[] = {
                      { /* end of list */ }
                  }
              },
+            {
+                .version = 3,
+                .note = "wiith the correct model number and cache info",
+                .props = (PropValue[]) {
+                    { "model", "0x5b" },
+                    { /* end of list */ }
+                },
+                .cache_info = &yongfeng_cache_info
+            },
              { /* end of list */ }
          }
      },
@@ -7565,8 +7634,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
           * 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))) {
+        if (cpu->vendor_cpuid_only && IS_INTEL_CPU(env)) {
              *eax = *ebx = *ecx = *edx = 0;
              break;
          } else if (cpu->cache_info_passthrough) {
@@ -7578,8 +7646,21 @@ 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);
+
+        if (IS_AMD_CPU(env)) {
+            *ecx = encode_cache_cpuid80000005(env->cache_info_amd.l1d_cache);
+            *edx = encode_cache_cpuid80000005(env->cache_info_amd.l1i_cache);
+            break;
+        }
+        /* Zhaoxin follows AMD behaviour on leaf 0x8000_0005 */
+        if (IS_ZHAOXIN_CPU(env)) {
+            *ecx = encode_cache_cpuid80000005(env->cache_info_zhaoxin.l1d_cache);
+            *edx = encode_cache_cpuid80000005(env->cache_info_zhaoxin.l1i_cache);
+            break;
+        }
+
+        /* Other vendors */
+        *eax = *ebx = *ecx = *edx = 0;
          break;
      case 0x80000006:
          /* cache info (L2 cache) */
@@ -8638,7 +8719,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
              return;
          }
          env->cache_info_cpuid2 = env->cache_info_cpuid4 = env->cache_info_amd =
-            *cache_info;
+            env->cache_info_zhaoxin = *cache_info;
      } else {
          /* Build legacy cache information */
          env->cache_info_cpuid2.l1d_cache = &legacy_l1d_cache;
@@ -8655,6 +8736,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
          env->cache_info_amd.l1i_cache = &legacy_l1i_cache_amd;
          env->cache_info_amd.l2_cache = &legacy_l2_cache_amd;
          env->cache_info_amd.l3_cache = &legacy_l3_cache;
+
+        env->cache_info_zhaoxin.l1d_cache = &legacy_l1d_cache;
+        env->cache_info_zhaoxin.l1i_cache = &legacy_l1i_cache;
+        env->cache_info_zhaoxin.l2_cache = &legacy_l2_cache;
+        env->cache_info_zhaoxin.l3_cache = &legacy_l3_cache;
      }

  #ifndef CONFIG_USER_ONLY
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index d98c9ba282..46bfd6f6b0 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2062,6 +2062,7 @@ typedef struct CPUArchState {
       * with old QEMU versions.
       */
      CPUCaches cache_info_cpuid2, cache_info_cpuid4, cache_info_amd;
+    CPUCaches cache_info_zhaoxin;

      /* MTRRs */
      uint64_t mtrr_fixed[11];
Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel
Posted by Zhao Liu 4 months, 3 weeks ago
Just want to confirm with the "lines_per_tag" field, which is related
about how to handle current "assert(lines_per_tag > 0)":

> --- patch prototype start ---
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 7b223642ba..8a17e5ffe9 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -2726,6 +2726,66 @@ static const CPUCaches xeon_srf_cache_info = {
>      },
>  };
> 
> +static const CPUCaches yongfeng_cache_info = {
> +    .l1d_cache = &(CPUCacheInfo) {
> +        .type = DATA_CACHE,
> +        .level = 1,
> +        .size = 32 * KiB,
> +        .line_size = 64,
> +        .associativity = 8,
> +        .partitions = 1,
> +        .sets = 64,
> +        .lines_per_tag = 1,

This fits AMD APM, and is fine.

> +        .inclusive = false,
> +        .self_init = true,
> +        .no_invd_sharing = false,
> +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> +    },
> +    .l1i_cache = &(CPUCacheInfo) {
> +        .type = INSTRUCTION_CACHE,
> +        .level = 1,
> +        .size = 64 * KiB,
> +        .line_size = 64,
> +        .associativity = 16,
> +        .partitions = 1,
> +        .sets = 64,
> +        .lines_per_tag = 1,

Fine, too.

> +        .inclusive = false,
> +        .self_init = true,
> +        .no_invd_sharing = false,
> +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> +    },
> +    .l2_cache = &(CPUCacheInfo) {
> +        .type = UNIFIED_CACHE,
> +        .level = 2,
> +        .size = 256 * KiB,
> +        .line_size = 64,
> +        .associativity = 8,
> +        .partitions = 1,
> +        .sets = 512,
> +        .lines_per_tag = 1,

SDM reserves this field:

For 0x80000006 ECX:

Bits 11-08: Reserved.

So I think this field should be 0, to align with "Reserved".

In this patch:

https://lore.kernel.org/qemu-devel/20250620092734.1576677-9-zhao1.liu@intel.com/

I add an argument (lines_per_tag_supported) in encode_cache_cpuid80000006(),
and for the case that lines_per_tag_supported=false, I assert
"lines_per_tag == 0" to align with "Reserved".

> +        .inclusive = true,
> +        .self_init = true,
> +        .no_invd_sharing = false,
> +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> +    },
> +    .l3_cache = &(CPUCacheInfo) {
> +        .type = UNIFIED_CACHE,
> +        .level = 3,
> +        .size = 8 * MiB,
> +        .line_size = 64,
> +        .associativity = 16,
> +        .partitions = 1,
> +        .sets = 8192,
> +        .lines_per_tag = 1,

The 0x80000006 EDX is also reserved in SDM. So I think this field should
be 0, too.

Do you agree?

> +        .self_init = true,
> +        .inclusive = true,
> +        .no_invd_sharing = true,
> +        .complex_indexing = false,
> +        .share_level = CPU_TOPOLOGY_LEVEL_DIE,
> +    },
> +};
> +
Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel
Posted by Ewan Hai 4 months, 3 weeks ago

On 6/25/25 5:19 PM, Zhao Liu wrote:
> 
> 
> Just want to confirm with the "lines_per_tag" field, which is related
> about how to handle current "assert(lines_per_tag > 0)":
> 
>> --- patch prototype start ---
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 7b223642ba..8a17e5ffe9 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -2726,6 +2726,66 @@ static const CPUCaches xeon_srf_cache_info = {
>>       },
>>   };
>>
>> +static const CPUCaches yongfeng_cache_info = {
>> +    .l1d_cache = &(CPUCacheInfo) {
>> +        .type = DATA_CACHE,
>> +        .level = 1,
>> +        .size = 32 * KiB,
>> +        .line_size = 64,
>> +        .associativity = 8,
>> +        .partitions = 1,
>> +        .sets = 64,
>> +        .lines_per_tag = 1,
> 
> This fits AMD APM, and is fine.
> 
>> +        .inclusive = false,
>> +        .self_init = true,
>> +        .no_invd_sharing = false,
>> +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
>> +    },
>> +    .l1i_cache = &(CPUCacheInfo) {
>> +        .type = INSTRUCTION_CACHE,
>> +        .level = 1,
>> +        .size = 64 * KiB,
>> +        .line_size = 64,
>> +        .associativity = 16,
>> +        .partitions = 1,
>> +        .sets = 64,
>> +        .lines_per_tag = 1,
> 
> Fine, too.
> 
>> +        .inclusive = false,
>> +        .self_init = true,
>> +        .no_invd_sharing = false,
>> +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
>> +    },
>> +    .l2_cache = &(CPUCacheInfo) {
>> +        .type = UNIFIED_CACHE,
>> +        .level = 2,
>> +        .size = 256 * KiB,
>> +        .line_size = 64,
>> +        .associativity = 8,
>> +        .partitions = 1,
>> +        .sets = 512,
>> +        .lines_per_tag = 1,
> 
> SDM reserves this field:
> 
> For 0x80000006 ECX:
> 
> Bits 11-08: Reserved.
> 
> So I think this field should be 0, to align with "Reserved".

I agree. For Zhaoxin, the "lines-per-tag" field appears only in CPUID leaf 
0x80000005. Because Zhaoxin follows AMD behavior on this leaf, and the AMD 
manual states that it reports L1 cache/TLB information, so any "lines-per-tag" 
value for levels other than L1 should be omitted or set to zero.

> 
> In this patch:
> 
> https://lore.kernel.org/qemu-devel/20250620092734.1576677-9-zhao1.liu@intel.com/
> 
> I add an argument (lines_per_tag_supported) in encode_cache_cpuid80000006(),
> and for the case that lines_per_tag_supported=false, I assert
> "lines_per_tag == 0" to align with "Reserved".
> 
>> +        .inclusive = true,
>> +        .self_init = true,
>> +        .no_invd_sharing = false,
>> +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
>> +    },
>> +    .l3_cache = &(CPUCacheInfo) {
>> +        .type = UNIFIED_CACHE,
>> +        .level = 3,
>> +        .size = 8 * MiB,
>> +        .line_size = 64,
>> +        .associativity = 16,
>> +        .partitions = 1,
>> +        .sets = 8192,
>> +        .lines_per_tag = 1,
> 
> The 0x80000006 EDX is also reserved in SDM. So I think this field should
> be 0, too.
> 
> Do you agree?

Ditto.>
>> +        .self_init = true,
>> +        .inclusive = true,
>> +        .no_invd_sharing = true,
>> +        .complex_indexing = false,
>> +        .share_level = CPU_TOPOLOGY_LEVEL_DIE,
>> +    },
>> +};
>> +
Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel
Posted by Zhao Liu 5 months, 3 weeks ago
> 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.
> > 
> > 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))) {
> > +            *eax = *ebx = *ecx = *edx = 0;
> > +            break;
> > +        } else if (cpu->cache_info_passthrough) {
> >               x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
> >               break;
> >           }
> > +
> >           *eax = (L1_DTLB_2M_ASSOC << 24) | (L1_DTLB_2M_ENTRIES << 16) |
> >                  (L1_ITLB_2M_ASSOC <<  8) | (L1_ITLB_2M_ENTRIES);
> >           *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) |
> 
> I've reviewed the cache-related CPUID path and noticed an oddity: every AMD
> vCPU model still reports identical hard-coded values for the L1 ITLB and L1
> DTLB fields in leaf 0x8000_0005. Your patch fixes this for Intel(and
> Zhaoxin), but all AMD models continue to receive the same constants in
> EAX/EBX.

Yes, TLB info is hardcoded here. Previously, Babu and Eduardo cleaned up
the cache info but didn't cover TLB [*]. I guess one reason would there
are very few use cases related to TLB's info, and people are more
concerned about the cache itself.

[*]: https://lore.kernel.org/qemu-devel/20180510204148.11687-2-babu.moger@amd.com/

> Do you know the reason for this choice? Is the guest expected to ignore
> those L1 TLB numbers? If so, I'll prepare a patch that adjusts only the
> Zhaoxin defaults in leaf 0x8000_0005 like below, matching real YongFeng
> behaviour in ecx and edx, but keep eax and ebx following AMD's behaviour.

This way is fine for me.

> ## Notes
> 1. Changes tied to "-machine smp-cache xxx" (mainly
> x86_cpu_update_smp_cache_topo()) are not included here.
> 2. Do you think I need Zhaoxin-specific legacy_l1d/l1i/l2/l3_cache helpers?
> If yes, I'll add them with YongFeng sillicon topology data.

Legacy cache info stands for information on very old machines. So I
think your yongfeng_cache_info is enough for Yongfeng CPU.

> --- patch prototype start ---

Thank you for your patch!

> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 7b223642ba..8a17e5ffe9 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -2726,6 +2726,66 @@ static const CPUCaches xeon_srf_cache_info = {
>      },
>  };
> 
> +static const CPUCaches yongfeng_cache_info = {
> +    .l1d_cache = &(CPUCacheInfo) {
> +        .type = DATA_CACHE,
> +        .level = 1,
> +        .size = 32 * KiB,
> +        .line_size = 64,
> +        .associativity = 8,
> +        .partitions = 1,
> +        .sets = 64,
> +        .lines_per_tag = 1,
> +        .inclusive = false,
> +        .self_init = true,
> +        .no_invd_sharing = false,
> +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> +    },
> +    .l1i_cache = &(CPUCacheInfo) {
> +        .type = INSTRUCTION_CACHE,
> +        .level = 1,
> +        .size = 64 * KiB,
> +        .line_size = 64,
> +        .associativity = 16,
> +        .partitions = 1,
> +        .sets = 64,
> +        .lines_per_tag = 1,
> +        .inclusive = false,
> +        .self_init = true,
> +        .no_invd_sharing = false,
> +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> +    },
> +    .l2_cache = &(CPUCacheInfo) {
> +        .type = UNIFIED_CACHE,
> +        .level = 2,
> +        .size = 256 * KiB,
> +        .line_size = 64,
> +        .associativity = 8,
> +        .partitions = 1,
> +        .sets = 512,
> +        .lines_per_tag = 1,
> +        .inclusive = true,
> +        .self_init = true,
> +        .no_invd_sharing = false,
> +        .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> +    },
> +    .l3_cache = &(CPUCacheInfo) {
> +        .type = UNIFIED_CACHE,
> +        .level = 3,
> +        .size = 8 * MiB,
> +        .line_size = 64,
> +        .associativity = 16,
> +        .partitions = 1,
> +        .sets = 8192,
> +        .lines_per_tag = 1,
> +        .self_init = true,
> +        .inclusive = true,
> +        .no_invd_sharing = true,
> +        .complex_indexing = false,
> +        .share_level = CPU_TOPOLOGY_LEVEL_DIE,

Just to confirm:

Is the topology of cache on your physical machines per-die instead of
per-socket?

> +    },
> +};
> +
>  /* The following VMX features are not supported by KVM and are left out in the
>   * CPU definitions:
>   *
> @@ -5928,6 +5988,15 @@ static const X86CPUDefinition builtin_x86_defs[] = {
>                      { /* end of list */ }
>                  }
>              },
> +            {
> +                .version = 3,
> +                .note = "wiith the correct model number and cache info",
> +                .props = (PropValue[]) {
> +                    { "model", "0x5b" },
> +                    { /* end of list */ }
> +                },
> +                .cache_info = &yongfeng_cache_info
> +            },
>              { /* end of list */ }
>          }
>      },

Adding a cache model can be done as a separate patch. :-)

> @@ -7565,8 +7634,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
> uint32_t count,
>           * 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))) {
> +        if (cpu->vendor_cpuid_only && IS_INTEL_CPU(env)) {
>              *eax = *ebx = *ecx = *edx = 0;
>              break;
>          } else if (cpu->cache_info_passthrough) {
> @@ -7578,8 +7646,21 @@ 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);
> +
> +        if (IS_AMD_CPU(env)) {
> +            *ecx = encode_cache_cpuid80000005(env->cache_info_amd.l1d_cache);
> +            *edx = encode_cache_cpuid80000005(env->cache_info_amd.l1i_cache);
> +            break;
> +        }

AMD uses 0x80000005 and doesn't use 0x4 leaf. Does Zhaoxin use 0x4?
If not, you can fix 0x4 for Zhaoxin, too.

> +        /* Zhaoxin follows AMD behaviour on leaf 0x8000_0005 */
> +        if (IS_ZHAOXIN_CPU(env)) {
> +            *ecx = encode_cache_cpuid80000005(env->cache_info_zhaoxin.l1d_cache);
> +            *edx = encode_cache_cpuid80000005(env->cache_info_zhaoxin.l1i_cache);

Maybe it's not necessary to add cache_info_zhaoxin? Zhaoxin could use
legacy cache_info_cpuid4 with Intel, because it seems cache_info_zhaoxin
is same as cache_info_cpuid4.

For this case, you can add a comment like "This is a special case where
Zhaoxin follows AMD. Elsewhere, Zhaoxin follows Intel's behavior."

> +            break;
> +        }
> +
> +        /* Other vendors */
> +        *eax = *ebx = *ecx = *edx = 0;
>          break;
>      case 0x80000006:
>          /* cache info (L2 cache) */
> @@ -8638,7 +8719,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>              return;
>          }
>          env->cache_info_cpuid2 = env->cache_info_cpuid4 = env->cache_info_amd =
> -            *cache_info;
> +            env->cache_info_zhaoxin = *cache_info;
>      } else {
>          /* Build legacy cache information */
>          env->cache_info_cpuid2.l1d_cache = &legacy_l1d_cache;
> @@ -8655,6 +8736,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>          env->cache_info_amd.l1i_cache = &legacy_l1i_cache_amd;
>          env->cache_info_amd.l2_cache = &legacy_l2_cache_amd;
>          env->cache_info_amd.l3_cache = &legacy_l3_cache;
> +
> +        env->cache_info_zhaoxin.l1d_cache = &legacy_l1d_cache;
> +        env->cache_info_zhaoxin.l1i_cache = &legacy_l1i_cache;
> +        env->cache_info_zhaoxin.l2_cache = &legacy_l2_cache;
> +        env->cache_info_zhaoxin.l3_cache = &legacy_l3_cache;

As I said above, we can apply cache_info_cpuid4 for Zhaoxin too.

>      }
> 
>  #ifndef CONFIG_USER_ONLY
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index d98c9ba282..46bfd6f6b0 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -2062,6 +2062,7 @@ typedef struct CPUArchState {
>       * with old QEMU versions.
>       */
>      CPUCaches cache_info_cpuid2, cache_info_cpuid4, cache_info_amd;
> +    CPUCaches cache_info_zhaoxin;
> 
>      /* MTRRs */
>      uint64_t mtrr_fixed[11];
> 
> 
> 
>
Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel
Posted by Ewan Hai 5 months, 3 weeks ago

On 5/27/25 5:15 PM, Zhao Liu wrote:
> 
>> 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.
>>>
>>> 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
[snip]>>> +
>>>            *eax = (L1_DTLB_2M_ASSOC << 24) | (L1_DTLB_2M_ENTRIES << 16) |
>>>                   (L1_ITLB_2M_ASSOC <<  8) | (L1_ITLB_2M_ENTRIES);
>>>            *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) |
>>
>> I've reviewed the cache-related CPUID path and noticed an oddity: every AMD
>> vCPU model still reports identical hard-coded values for the L1 ITLB and L1
>> DTLB fields in leaf 0x8000_0005. Your patch fixes this for Intel(and
>> Zhaoxin), but all AMD models continue to receive the same constants in
>> EAX/EBX.
> 
> Yes, TLB info is hardcoded here. Previously, Babu and Eduardo cleaned up
> the cache info but didn't cover TLB [*]. I guess one reason would there
> are very few use cases related to TLB's info, and people are more
> concerned about the cache itself.
> 
> [*]: https://lore.kernel.org/qemu-devel/20180510204148.11687-2-babu.moger@amd.com/

Understood. Keeping the L1 I/D-TLB fields hard-coded for every vCPU model is 
acceptable.

>> Do you know the reason for this choice? Is the guest expected to ignore
>> those L1 TLB numbers? If so, I'll prepare a patch that adjusts only the
>> Zhaoxin defaults in leaf 0x8000_0005 like below, matching real YongFeng
>> behaviour in ecx and edx, but keep eax and ebx following AMD's behaviour.
> 
> This way is fine for me.
> 

Thanks for confirming. I'll post the YongFeng cache-info series once your 
refactor lands.

>> ## Notes
>> 1. Changes tied to "-machine smp-cache xxx" (mainly
>> x86_cpu_update_smp_cache_topo()) are not included here.
>> 2. Do you think I need Zhaoxin-specific legacy_l1d/l1i/l2/l3_cache helpers?
>> If yes, I'll add them with YongFeng sillicon topology data.
> 
> Legacy cache info stands for information on very old machines. So I
> think your yongfeng_cache_info is enough for Yongfeng CPU.
> 
>> --- patch prototype start ---
> 
> Thank you for your patch!
> 
You're welcome!

>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 7b223642ba..8a17e5ffe9 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -2726,6 +2726,66 @@ static const CPUCaches xeon_srf_cache_info = {
>>       },
>>   };
>>
>> +static const CPUCaches yongfeng_cache_info = {
[snip]>> +    .l3_cache = &(CPUCacheInfo) {
>> +        .type = UNIFIED_CACHE,
>> +        .level = 3,
>> +        .size = 8 * MiB,
>> +        .line_size = 64,
>> +        .associativity = 16,
>> +        .partitions = 1,
>> +        .sets = 8192,
>> +        .lines_per_tag = 1,
>> +        .self_init = true,
>> +        .inclusive = true,
>> +        .no_invd_sharing = true,
>> +        .complex_indexing = false,
>> +        .share_level = CPU_TOPOLOGY_LEVEL_DIE,
> 
> Just to confirm:
> 
> Is the topology of cache on your physical machines per-die instead of
> per-socket?
> 
Apologies for the confusion, the code I shared was only a prototype.
Before submitting the real patch I'll verify every value in yongfeng_cache_info 
against silicon.

For YongFeng the hierarchy is: L1/L2 shared per core, L3 shared per die (four 
cores per L3).

>> +    },
>> +};
>> +
>>   /* The following VMX features are not supported by KVM and are left out in the
>>    * CPU definitions:
>>    *
>> @@ -5928,6 +5988,15 @@ static const X86CPUDefinition builtin_x86_defs[] = {
>>                       { /* end of list */ }
>>                   }
>>               },
>> +            {
>> +                .version = 3,
>> +                .note = "wiith the correct model number and cache info",
>> +                .props = (PropValue[]) {
>> +                    { "model", "0x5b" },
>> +                    { /* end of list */ }
>> +                },
>> +                .cache_info = &yongfeng_cache_info
>> +            },
>>               { /* end of list */ }
>>           }
>>       },
> 
> Adding a cache model can be done as a separate patch. :-)
Ok, the current blob was for review only; the formal submission will be split 
into logical patches.>
>> @@ -7565,8 +7634,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
>> uint32_t count,
>>            * 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))) {
>> +        if (cpu->vendor_cpuid_only && IS_INTEL_CPU(env)) {
>>               *eax = *ebx = *ecx = *edx = 0;
>>               break;
>>           } else if (cpu->cache_info_passthrough) {
>> @@ -7578,8 +7646,21 @@ 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);
>> +
>> +        if (IS_AMD_CPU(env)) {
>> +            *ecx = encode_cache_cpuid80000005(env->cache_info_amd.l1d_cache);
>> +            *edx = encode_cache_cpuid80000005(env->cache_info_amd.l1i_cache);
>> +            break;
>> +        }
> 
> AMD uses 0x80000005 and doesn't use 0x4 leaf. Does Zhaoxin use 0x4?
> If not, you can fix 0x4 for Zhaoxin, too.
Zhaoxin uses 0x4 like Intel. We don't need to fix cpuid 0x4 leaf behaviour.>
>> +        /* Zhaoxin follows AMD behaviour on leaf 0x8000_0005 */
>> +        if (IS_ZHAOXIN_CPU(env)) {
>> +            *ecx = encode_cache_cpuid80000005(env->cache_info_zhaoxin.l1d_cache);
>> +            *edx = encode_cache_cpuid80000005(env->cache_info_zhaoxin.l1i_cache);
> 
> Maybe it's not necessary to add cache_info_zhaoxin? Zhaoxin could use
> legacy cache_info_cpuid4 with Intel, because it seems cache_info_zhaoxin
> is same as cache_info_cpuid4.
> 
> For this case, you can add a comment like "This is a special case where
> Zhaoxin follows AMD. Elsewhere, Zhaoxin follows Intel's behavior."
I agree! I'll drop the cache_info_zhaoxin stub and reuse cache_info_cpuid4 in 
the final patch set.>
>> +            break;
>> +        }
>> +
>> +        /* Other vendors */
>> +        *eax = *ebx = *ecx = *edx = 0;
>>           break;
>>       case 0x80000006:
>>           /* cache info (L2 cache) */
>> @@ -8638,7 +8719,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>>               return;
>>           }
>>           env->cache_info_cpuid2 = env->cache_info_cpuid4 = env->cache_info_amd =
>> -            *cache_info;
>> +            env->cache_info_zhaoxin = *cache_info;
>>       } else {
>>           /* Build legacy cache information */
>>           env->cache_info_cpuid2.l1d_cache = &legacy_l1d_cache;
>> @@ -8655,6 +8736,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>>           env->cache_info_amd.l1i_cache = &legacy_l1i_cache_amd;
>>           env->cache_info_amd.l2_cache = &legacy_l2_cache_amd;
>>           env->cache_info_amd.l3_cache = &legacy_l3_cache;
>> +
>> +        env->cache_info_zhaoxin.l1d_cache = &legacy_l1d_cache;
>> +        env->cache_info_zhaoxin.l1i_cache = &legacy_l1i_cache;
>> +        env->cache_info_zhaoxin.l2_cache = &legacy_l2_cache;
>> +        env->cache_info_zhaoxin.l3_cache = &legacy_l3_cache;
> 
> As I said above, we can apply cache_info_cpuid4 for Zhaoxin too.
Yep!> >>       }
[snip]
Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel
Posted by Zhao Liu 4 months, 3 weeks ago
On Tue, May 27, 2025 at 05:56:07PM +0800, Ewan Hai wrote:
> Date: Tue, 27 May 2025 17:56:07 +0800
> From: Ewan Hai <ewanhai-oc@zhaoxin.com>
> Subject: Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for
>  Intel
> 
> 
> 
> On 5/27/25 5:15 PM, Zhao Liu wrote:
> > 
> > > 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.
> > > > 
> > > > 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
> [snip]>>> +
> > > >            *eax = (L1_DTLB_2M_ASSOC << 24) | (L1_DTLB_2M_ENTRIES << 16) |
> > > >                   (L1_ITLB_2M_ASSOC <<  8) | (L1_ITLB_2M_ENTRIES);
> > > >            *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) |
> > > 
> > > I've reviewed the cache-related CPUID path and noticed an oddity: every AMD
> > > vCPU model still reports identical hard-coded values for the L1 ITLB and L1
> > > DTLB fields in leaf 0x8000_0005. Your patch fixes this for Intel(and
> > > Zhaoxin), but all AMD models continue to receive the same constants in
> > > EAX/EBX.
> > 
> > Yes, TLB info is hardcoded here. Previously, Babu and Eduardo cleaned up
> > the cache info but didn't cover TLB [*]. I guess one reason would there
> > are very few use cases related to TLB's info, and people are more
> > concerned about the cache itself.
> > 
> > [*]: https://lore.kernel.org/qemu-devel/20180510204148.11687-2-babu.moger@amd.com/
> 
> Understood. Keeping the L1 I/D-TLB fields hard-coded for every vCPU model is
> acceptable.
> 
> > > Do you know the reason for this choice? Is the guest expected to ignore
> > > those L1 TLB numbers? If so, I'll prepare a patch that adjusts only the
> > > Zhaoxin defaults in leaf 0x8000_0005 like below, matching real YongFeng
> > > behaviour in ecx and edx, but keep eax and ebx following AMD's behaviour.
> > 
> > This way is fine for me.
> > 
> 
> Thanks for confirming. I'll post the YongFeng cache-info series once your
> refactor lands.

Hi Ewan,

By this patch:

https://lore.kernel.org/qemu-devel/20250620092734.1576677-14-zhao1.liu@intel.com/

I fixed the 0x80000005 leaf for Intel and Zhaoxin based on your feedback
by the way.

It looks like the unified cache_info would be very compatible with
various vendor needs and corner cases. So I'll respin this series based
on that cache_info series.

Before sending the patch, I was thinking that maybe I could help you
rebase and include your Yongfeng cache model patch you added into my v2
series, or you could rebase and send it yourself afterward. Which way do
you like?

And for TLB, we can wait and see what maintainers think. Maybe it is
possible, considering that the cache also transitioned from hardcoding
to a cache model (since commit 7e3482f82480).

Thanks,
Zhao
Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel
Posted by Ewan Hai 4 months, 3 weeks ago

On 6/24/25 3:22 PM, Zhao Liu wrote:
> 
> On Tue, May 27, 2025 at 05:56:07PM +0800, Ewan Hai wrote:
>> Date: Tue, 27 May 2025 17:56:07 +0800
>> From: Ewan Hai <ewanhai-oc@zhaoxin.com>
>> Subject: Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for
>>   Intel
>>
>>
>>
>> On 5/27/25 5:15 PM, Zhao Liu wrote:
>>>
>>>> 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.
>>>>>
>>>>> 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
>> [snip]>>> +
>>>>>             *eax = (L1_DTLB_2M_ASSOC << 24) | (L1_DTLB_2M_ENTRIES << 16) |
>>>>>                    (L1_ITLB_2M_ASSOC <<  8) | (L1_ITLB_2M_ENTRIES);
>>>>>             *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) |
>>>>
>>>> I've reviewed the cache-related CPUID path and noticed an oddity: every AMD
>>>> vCPU model still reports identical hard-coded values for the L1 ITLB and L1
>>>> DTLB fields in leaf 0x8000_0005. Your patch fixes this for Intel(and
>>>> Zhaoxin), but all AMD models continue to receive the same constants in
>>>> EAX/EBX.
>>>
>>> Yes, TLB info is hardcoded here. Previously, Babu and Eduardo cleaned up
>>> the cache info but didn't cover TLB [*]. I guess one reason would there
>>> are very few use cases related to TLB's info, and people are more
>>> concerned about the cache itself.
>>>
>>> [*]: https://lore.kernel.org/qemu-devel/20180510204148.11687-2-babu.moger@amd.com/
>>
>> Understood. Keeping the L1 I/D-TLB fields hard-coded for every vCPU model is
>> acceptable.
>>
>>>> Do you know the reason for this choice? Is the guest expected to ignore
>>>> those L1 TLB numbers? If so, I'll prepare a patch that adjusts only the
>>>> Zhaoxin defaults in leaf 0x8000_0005 like below, matching real YongFeng
>>>> behaviour in ecx and edx, but keep eax and ebx following AMD's behaviour.
>>>
>>> This way is fine for me.
>>>
>>
>> Thanks for confirming. I'll post the YongFeng cache-info series once your
>> refactor lands.
> 
> Hi Ewan,
> 
> By this patch:
> 
> https://lore.kernel.org/qemu-devel/20250620092734.1576677-14-zhao1.liu@intel.com/
> 
> I fixed the 0x80000005 leaf for Intel and Zhaoxin based on your feedback
> by the way.
> 
> It looks like the unified cache_info would be very compatible with
> various vendor needs and corner cases. So I'll respin this series based
> on that cache_info series.
> 
> Before sending the patch, I was thinking that maybe I could help you
> rebase and include your Yongfeng cache model patch you added into my v2
> series, or you could rebase and send it yourself afterward. Which way do
> you like?

It would be great if you could include the Yongfeng cache-model patch in your v2 
series. Let me know if you need any more information about the Yongfeng cache 
model. After you submit v2, I can review the Zhaoxin parts and make any 
necessary code changes if needed.

And thanks again for taking Zhaoxin into account.

> 
> And for TLB, we can wait and see what maintainers think. Maybe it is
> possible, considering that the cache also transitioned from hardcoding
> to a cache model (since commit 7e3482f82480).
> 
Let's wait.> Thanks,
> Zhao
>
Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel
Posted by Zhao Liu 4 months, 3 weeks ago
On Tue, Jun 24, 2025 at 07:04:02PM +0800, Ewan Hai wrote:
> Date: Tue, 24 Jun 2025 19:04:02 +0800
> From: Ewan Hai <ewanhai-oc@zhaoxin.com>
> Subject: Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for
>  Intel
> 
> 
> 
> On 6/24/25 3:22 PM, Zhao Liu wrote:
> > 
> > On Tue, May 27, 2025 at 05:56:07PM +0800, Ewan Hai wrote:
> > > Date: Tue, 27 May 2025 17:56:07 +0800
> > > From: Ewan Hai <ewanhai-oc@zhaoxin.com>
> > > Subject: Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for
> > >   Intel
> > > 
> > > 
> > > 
> > > On 5/27/25 5:15 PM, Zhao Liu wrote:
> > > > 
> > > > > 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.
> > > > > > 
> > > > > > 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
> > > [snip]>>> +
> > > > > >             *eax = (L1_DTLB_2M_ASSOC << 24) | (L1_DTLB_2M_ENTRIES << 16) |
> > > > > >                    (L1_ITLB_2M_ASSOC <<  8) | (L1_ITLB_2M_ENTRIES);
> > > > > >             *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) |
> > > > > 
> > > > > I've reviewed the cache-related CPUID path and noticed an oddity: every AMD
> > > > > vCPU model still reports identical hard-coded values for the L1 ITLB and L1
> > > > > DTLB fields in leaf 0x8000_0005. Your patch fixes this for Intel(and
> > > > > Zhaoxin), but all AMD models continue to receive the same constants in
> > > > > EAX/EBX.
> > > > 
> > > > Yes, TLB info is hardcoded here. Previously, Babu and Eduardo cleaned up
> > > > the cache info but didn't cover TLB [*]. I guess one reason would there
> > > > are very few use cases related to TLB's info, and people are more
> > > > concerned about the cache itself.
> > > > 
> > > > [*]: https://lore.kernel.org/qemu-devel/20180510204148.11687-2-babu.moger@amd.com/
> > > 
> > > Understood. Keeping the L1 I/D-TLB fields hard-coded for every vCPU model is
> > > acceptable.
> > > 
> > > > > Do you know the reason for this choice? Is the guest expected to ignore
> > > > > those L1 TLB numbers? If so, I'll prepare a patch that adjusts only the
> > > > > Zhaoxin defaults in leaf 0x8000_0005 like below, matching real YongFeng
> > > > > behaviour in ecx and edx, but keep eax and ebx following AMD's behaviour.
> > > > 
> > > > This way is fine for me.
> > > > 
> > > 
> > > Thanks for confirming. I'll post the YongFeng cache-info series once your
> > > refactor lands.
> > 
> > Hi Ewan,
> > 
> > By this patch:
> > 
> > https://lore.kernel.org/qemu-devel/20250620092734.1576677-14-zhao1.liu@intel.com/
> > 
> > I fixed the 0x80000005 leaf for Intel and Zhaoxin based on your feedback
> > by the way.
> > 
> > It looks like the unified cache_info would be very compatible with
> > various vendor needs and corner cases. So I'll respin this series based
> > on that cache_info series.
> > 
> > Before sending the patch, I was thinking that maybe I could help you
> > rebase and include your Yongfeng cache model patch you added into my v2
> > series, or you could rebase and send it yourself afterward. Which way do
> > you like?
> 
> It would be great if you could include the Yongfeng cache-model patch in
> your v2 series. Let me know if you need any more information about the
> Yongfeng cache model. After you submit v2, I can review the Zhaoxin parts
> and make any necessary code changes if needed.
> 
> And thanks again for taking Zhaoxin into account.

Welcome; it's something I can easily help with. If possible, when v2 is
out, hope you could help test it on your platform to ensure everything
is fine. :-) And I've verified it myself through TCG.

Thanks,
Zhao
Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel
Posted by Ewan Hai 4 months, 3 weeks ago

On 6/25/25 11:03 AM, Zhao Liu wrote:
> 
> 
> On Tue, Jun 24, 2025 at 07:04:02PM +0800, Ewan Hai wrote:
>> Date: Tue, 24 Jun 2025 19:04:02 +0800
>> From: Ewan Hai <ewanhai-oc@zhaoxin.com>
>> Subject: Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for
>>   Intel
>>
>>
>>
>> On 6/24/25 3:22 PM, Zhao Liu wrote:
>>>
>>> On Tue, May 27, 2025 at 05:56:07PM +0800, Ewan Hai wrote:
>>>> Date: Tue, 27 May 2025 17:56:07 +0800
>>>> From: Ewan Hai <ewanhai-oc@zhaoxin.com>
>>>> Subject: Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for
>>>>    Intel
>>>>
>>>>
>>>>
>>>> On 5/27/25 5:15 PM, Zhao Liu wrote:
>>>>>
>>>>>> 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.
>>>>>>>
>>>>>>> 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
>>>> [snip]>>> +
>>>>>>>              *eax = (L1_DTLB_2M_ASSOC << 24) | (L1_DTLB_2M_ENTRIES << 16) |
>>>>>>>                     (L1_ITLB_2M_ASSOC <<  8) | (L1_ITLB_2M_ENTRIES);
>>>>>>>              *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) |
>>>>>>
>>>>>> I've reviewed the cache-related CPUID path and noticed an oddity: every AMD
>>>>>> vCPU model still reports identical hard-coded values for the L1 ITLB and L1
>>>>>> DTLB fields in leaf 0x8000_0005. Your patch fixes this for Intel(and
>>>>>> Zhaoxin), but all AMD models continue to receive the same constants in
>>>>>> EAX/EBX.
>>>>>
>>>>> Yes, TLB info is hardcoded here. Previously, Babu and Eduardo cleaned up
>>>>> the cache info but didn't cover TLB [*]. I guess one reason would there
>>>>> are very few use cases related to TLB's info, and people are more
>>>>> concerned about the cache itself.
>>>>>
>>>>> [*]: https://lore.kernel.org/qemu-devel/20180510204148.11687-2-babu.moger@amd.com/
>>>>
>>>> Understood. Keeping the L1 I/D-TLB fields hard-coded for every vCPU model is
>>>> acceptable.
>>>>
>>>>>> Do you know the reason for this choice? Is the guest expected to ignore
>>>>>> those L1 TLB numbers? If so, I'll prepare a patch that adjusts only the
>>>>>> Zhaoxin defaults in leaf 0x8000_0005 like below, matching real YongFeng
>>>>>> behaviour in ecx and edx, but keep eax and ebx following AMD's behaviour.
>>>>>
>>>>> This way is fine for me.
>>>>>
>>>>
>>>> Thanks for confirming. I'll post the YongFeng cache-info series once your
>>>> refactor lands.
>>>
>>> Hi Ewan,
>>>
>>> By this patch:
>>>
>>> https://lore.kernel.org/qemu-devel/20250620092734.1576677-14-zhao1.liu@intel.com/
>>>
>>> I fixed the 0x80000005 leaf for Intel and Zhaoxin based on your feedback
>>> by the way.
>>>
>>> It looks like the unified cache_info would be very compatible with
>>> various vendor needs and corner cases. So I'll respin this series based
>>> on that cache_info series.
>>>
>>> Before sending the patch, I was thinking that maybe I could help you
>>> rebase and include your Yongfeng cache model patch you added into my v2
>>> series, or you could rebase and send it yourself afterward. Which way do
>>> you like?
>>
>> It would be great if you could include the Yongfeng cache-model patch in
>> your v2 series. Let me know if you need any more information about the
>> Yongfeng cache model. After you submit v2, I can review the Zhaoxin parts
>> and make any necessary code changes if needed.
>>
>> And thanks again for taking Zhaoxin into account.
> 
> Welcome; it's something I can easily help with. If possible, when v2 is
> out, hope you could help test it on your platform to ensure everything
> is fine. :-) And I've verified it myself through TCG.
> 
There's no problem!> Thanks,
> Zhao
>
Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel
Posted by Ewan Hai 6 months, 3 weeks ago

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.

> 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.

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

Thanks again for your work.
Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel
Posted by Zhao Liu 6 months, 3 weeks ago
> 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!


Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel
Posted by Xiaoyao Li 6 months, 3 weeks ago
On 4/23/2025 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.
> 
> 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.
> +         */

We need to define a new compat property for it other than 
vendor_cpuid_only, for 10.1.

I proposed some change to leaf FEAT_8000_0001_EDX[1], and I was told by 
Paolo (privately) that vendor_cpuid_only doesn't suffice.

  On Fri, Oct 11, 2024 at 6:22 PM Xiaoyao Li <xiaoyao.li@intel.com> wrote:
  >
  > On 10/11/2024 11:30 PM, Paolo Bonzini wrote:
  > > On Fri, Oct 11, 2024 at 4:55 PM Xiaoyao Li <xiaoyao.li@intel.com> 
wrote:
  > >>
  > >> I think patch 8 is also a general issue> Without it, the
  > >> CPUID_EXT2_AMD_ALIASES bits are exposed to Intel VMs which are
  > >> reserved bits for Intel.
  > >
  > > Yes but you'd have to add compat properties for these. If you can do
  > > it for TDX only, that's easier.
  >
  > Does vendor_cpuid_only suffice?

  Unfortunately not, because it is turned off only for <=6.0 machine
  types. Here you'd have to turn it off for <=9.1 machine types.


[1] 
https://lore.kernel.org/qemu-devel/20240814075431.339209-9-xiaoyao.li@intel.com/


> +        if (cpu->vendor_cpuid_only &&
> +            (IS_INTEL_CPU(env) || IS_ZHAOXIN_CPU(env))) {
> +            *eax = *ebx = *ecx = *edx = 0;
> +            break;
> +        } else if (cpu->cache_info_passthrough) {
>               x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
>               break;
>           }
> +
>           *eax = (L1_DTLB_2M_ASSOC << 24) | (L1_DTLB_2M_ENTRIES << 16) |
>                  (L1_ITLB_2M_ASSOC <<  8) | (L1_ITLB_2M_ENTRIES);
>           *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) |


Re: [RFC 01/10] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel
Posted by Zhao Liu 6 months, 3 weeks ago
> > +        /*
> > +         * 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.
> > +         */
> 
> We need to define a new compat property for it other than vendor_cpuid_only,
> for 10.1.
> 
> I proposed some change to leaf FEAT_8000_0001_EDX[1], and I was told by
> Paolo (privately) that vendor_cpuid_only doesn't suffice.
> 
>  On Fri, Oct 11, 2024 at 6:22 PM Xiaoyao Li <xiaoyao.li@intel.com> wrote:
>  >
>  > On 10/11/2024 11:30 PM, Paolo Bonzini wrote:
>  > > On Fri, Oct 11, 2024 at 4:55 PM Xiaoyao Li <xiaoyao.li@intel.com>
> wrote:
>  > >>
>  > >> I think patch 8 is also a general issue> Without it, the
>  > >> CPUID_EXT2_AMD_ALIASES bits are exposed to Intel VMs which are
>  > >> reserved bits for Intel.
>  > >
>  > > Yes but you'd have to add compat properties for these. If you can do
>  > > it for TDX only, that's easier.
>  >
>  > Does vendor_cpuid_only suffice?
> 
>  Unfortunately not, because it is turned off only for <=6.0 machine
>  types. Here you'd have to turn it off for <=9.1 machine types.
> 
> 
> [1] https://lore.kernel.org/qemu-devel/20240814075431.339209-9-xiaoyao.li@intel.com/

For the patch link, you wanted to mark it as unavailiable but it would
break the machine <= 6.0 (with vendor_cpuid_only turned off), correct?

For this patch:
 * vendor_cpuid_only turns off for <= 6.0 machine, no change.
 * vendor_cpuid_only turns on for > 6.0 machine, I treated it as a
   "direct" fix because original codes encode the AMD cache model info
   on non-AMD platform (in ecx & edx). This doesn't make sense. Non-AMD
   platform should use cache_info_cpuid4 or 0 here. If it is considered
   a fix, it may be more appropriate to use cache_info_cpuid4.

I think it's somehow similar to the “trade-offs” Daniel indicated [2].

This case can also be fixed by compat option. Then we don't  need to
encode cache_info_cpuid4 info for non-AMD platforms in this leaf.

Do you still need the patches in your links? (I didn't find the related
patch merged.) If so, I can add the compat option in the next version
which could also help you land your previous patches v10.1 as well.

[2]: https://lore.kernel.org/qemu-devel/Z08j2Ii-QuZk3lTY@redhat.com/

> > +        if (cpu->vendor_cpuid_only &&
> > +            (IS_INTEL_CPU(env) || IS_ZHAOXIN_CPU(env))) {
> > +            *eax = *ebx = *ecx = *edx = 0;
> > +            break;
> > +        } else if (cpu->cache_info_passthrough) {
> >               x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
> >               break;
> >           }
> > +
> >           *eax = (L1_DTLB_2M_ASSOC << 24) | (L1_DTLB_2M_ENTRIES << 16) |
> >                  (L1_ITLB_2M_ASSOC <<  8) | (L1_ITLB_2M_ENTRIES);
> >           *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) |
>