[PATCH v2 0/7] i386/cpu: Clean Up Reserved CPUID Leaves & Topology Overflow Fix

[PATCH v2 0/7] i386/cpu: Clean Up Reserved CPUID Leaves & Topology Overflow Fix

[Zhao Liu]

Hi,

To avoid the series being too trivial, I've merged the two previous
series together again.

This series is based on:

https://lore.kernel.org/qemu-devel/20250711102143.1622339-1-zhao1.liu@intel.com/

And you can find the codes at:

https://gitlab.com/zhao.liu/qemu/-/tree/cache-model-v3.0-rebase-07-10-2025?ref_type=heads


Part 1 (Patch 1-3)
==================

Since the previsor unified cache model series has already introduced a
new compat property "x-vendor-cpuid-only-v2", it's a chance to once
again consolidate more vendor-specific CPUIDs.

I also checked the CPUID leaves currently supported by Intel & AMD and
found that since the previous "x-vendor-cpuid-only," AMD has already
cleaned up the Intel-specific CPUIDs quite well.

As for Intel, the only cleanup needed is for the "extended function
CPUID" leaves (0x80000000~0x80000008). That's what these patches does.

Considerring the disscussion around AMD's arch capabilities MSR,
explicitly changing feature bits in QEMU that require host support is
inappropriate. CPUID adjustments based on the vendor should be limited
to parts that are fully emulated within QEMU (such as topology, vendor,
etc.).

Therefore, in v2, I dropped the modifications to 0x80000007 and narrowed
the changes to 0x80000008 to only the fields related to CPU topology.

Previous v1:

https://lore.kernel.org/qemu-devel/20250627035129.2755537-1-zhao1.liu@intel.com/


Part 2 (Patch 4-7)
==================

These patches collect and organize several topology-related cleanups and
fixes.

Comparing with v1, I provides the oveflow example for patch 6 & 7.

Note, In addition to the 0x1, 0x4, and 0x8000001d leaves involved in the
patch series, there is also the 0x1f leaf related to topology. However,
the upper limit for CPUID.1FH.EBX[bits 15:0] is 65,535 threads, which
provides enough room. Therefore, this field does not currently require
overflow checks.

PS: The previous patch 4 tried to fix overflow directly. But now we have
the "x-vendor-cpuid-only-v2", it's a chance to fix addressable ID for
new machines.

Previous v1:

https://lore.kernel.org/qemu-devel/20250227062523.124601-1-zhao1.liu@intel.com/

Thanks and Best Regards,
Zhao

---
Chuang Xu (1):
  i386/cpu: Fix number of addressable IDs field for CPUID.01H.EBX[23:16]

Qian Wen (2):
  i386/cpu: Fix cpu number overflow in CPUID.01H.EBX[23:16]
  i386/cpu: Fix overflow of cache topology fields in CPUID.04H

Zhao Liu (4):
  i386/cpu: Mark EBX/ECX/EDX in CPUID 0x80000000 leaf as reserved for
    Intel
  i386/cpu: Mark CPUID 0x80000008 ECX bits[0:7] & [12:15] as reserved
    for Intel/Zhaoxin
  i386/cpu: Reorder CPUID leaves in cpu_x86_cpuid()
  i386/cpu: Honor maximum value for CPUID.8000001DH.EAX[25:14]

 target/i386/cpu.c | 130 ++++++++++++++++++++++++++++++----------------
 1 file changed, 84 insertions(+), 46 deletions(-)

-- 
2.34.1

[PATCH v2 1/7] i386/cpu: Mark EBX/ECX/EDX in CPUID 0x80000000 leaf as reserved for Intel

[Zhao Liu]

Per SDM,

80000000H EAX Maximum Input Value for Extended Function CPUID Information.
          EBX Reserved.
          ECX Reserved.
          EDX Reserved.

EBX/ECX/EDX in CPUID 0x80000000 leaf are reserved. Intel and Zhaoxin are
already using 0x0 leaf to encode vendor.

Reviewed-by: Tao Su <tao1.su@linux.intel.com>
Tested-by: Yi Lai <yi1.lai@intel.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes Since v1:
 * Consider Zhaoxin CPU. (Ewan)
---
 target/i386/cpu.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 8d67cadec2f2..37e4bf51d890 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -8278,9 +8278,15 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         break;
     case 0x80000000:
         *eax = env->cpuid_xlevel;
-        *ebx = env->cpuid_vendor1;
-        *edx = env->cpuid_vendor2;
-        *ecx = env->cpuid_vendor3;
+
+        if (cpu->vendor_cpuid_only_v2 &&
+            (IS_INTEL_CPU(env) || IS_ZHAOXIN_CPU(env))) {
+            *ebx = *ecx = *edx = 0;
+        } else {
+            *ebx = env->cpuid_vendor1;
+            *edx = env->cpuid_vendor2;
+            *ecx = env->cpuid_vendor3;
+        }
         break;
     case 0x80000001:
         *eax = env->cpuid_version;
-- 
2.34.1

Re: [PATCH v2 1/7] i386/cpu: Mark EBX/ECX/EDX in CPUID 0x80000000 leaf as reserved for Intel

[Xiaoyao Li]

On 7/14/2025 4:08 PM, Zhao Liu wrote:
> Per SDM,
> 
> 80000000H EAX Maximum Input Value for Extended Function CPUID Information.
>            EBX Reserved.
>            ECX Reserved.
>            EDX Reserved.
> 
> EBX/ECX/EDX in CPUID 0x80000000 leaf are reserved. Intel and Zhaoxin are
> already using 0x0 leaf to encode vendor.
> 
> Reviewed-by: Tao Su <tao1.su@linux.intel.com>
> Tested-by: Yi Lai <yi1.lai@intel.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

[PATCH v2 2/7] i386/cpu: Mark CPUID 0x80000008 ECX bits[0:7] & [12:15] as reserved for Intel/Zhaoxin

[Zhao Liu]

Per SDM,

80000008H EAX Linear/Physical Address size.
              Bits 07-00: #Physical Address Bits*.
              Bits 15-08: #Linear Address Bits.
              Bits 31-16: Reserved = 0.
          EBX Bits 08-00: Reserved = 0.
              Bit 09: WBNOINVD is available if 1.
              Bits 31-10: Reserved = 0.
          ECX Reserved = 0.
          EDX Reserved = 0.

ECX/EDX in CPUID 0x80000008 leaf are reserved.

Currently, in QEMU, only ECX bits[0:7] and ECX bits[12:15] are encoded,
and both are emulated in QEMU.

Considering that Intel and Zhaoxin are already using the 0x1f leaf to
describe CPU topology, which includes similar information, Intel and
Zhaoxin will not implement ECX bits[0:7] and bits[12:15] of 0x80000008.

Therefore, mark these two fields as reserved and clear them for Intel
and Zhaoxin guests.

Reviewed-by: Tao Su <tao1.su@linux.intel.com>
Tested-by: Yi Lai <yi1.lai@intel.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes Since v1:
 * Consider Zhaoxin (Ewan).
 * Only clear ECX bits[0:7] and bits[12:15] for Intel/Zhaoxin, and do
   not cover other bits.
---
 target/i386/cpu.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 37e4bf51d890..abd529d587ba 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -8387,15 +8387,25 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
              *eax |= (cpu->guest_phys_bits << 16);
         }
         *ebx = env->features[FEAT_8000_0008_EBX];
+
         if (threads_per_pkg > 1) {
             /*
-             * Bits 15:12 is "The number of bits in the initial
-             * Core::X86::Apic::ApicId[ApicId] value that indicate
-             * thread ID within a package".
-             * Bits 7:0 is "The number of threads in the package is NC+1"
+             * Don't emulate Bits [7:0] & Bits [15:12] for Intel/Zhaoxin, since
+             * they're using 0x1f leaf.
              */
-            *ecx = (apicid_pkg_offset(topo_info) << 12) |
-                   (threads_per_pkg - 1);
+            if (cpu->vendor_cpuid_only_v2 &&
+                (IS_INTEL_CPU(env) || IS_ZHAOXIN_CPU(env))) {
+                    *ecx = 0;
+            } else {
+                /*
+                 * Bits 15:12 is "The number of bits in the initial
+                 * Core::X86::Apic::ApicId[ApicId] value that indicate
+                 * thread ID within a package".
+                 * Bits 7:0 is "The number of threads in the package is NC+1"
+                 */
+                *ecx = (apicid_pkg_offset(topo_info) << 12) |
+                       (threads_per_pkg - 1);
+            }
         } else {
             *ecx = 0;
         }
-- 
2.34.1

Re: [PATCH v2 2/7] i386/cpu: Mark CPUID 0x80000008 ECX bits[0:7] & [12:15] as reserved for Intel/Zhaoxin

[Xiaoyao Li]

On 7/14/2025 4:08 PM, Zhao Liu wrote:
> Per SDM,
> 
> 80000008H EAX Linear/Physical Address size.
>                Bits 07-00: #Physical Address Bits*.
>                Bits 15-08: #Linear Address Bits.
>                Bits 31-16: Reserved = 0.
>            EBX Bits 08-00: Reserved = 0.
>                Bit 09: WBNOINVD is available if 1.
>                Bits 31-10: Reserved = 0.
>            ECX Reserved = 0.
>            EDX Reserved = 0.
> 
> ECX/EDX in CPUID 0x80000008 leaf are reserved.
> 
> Currently, in QEMU, only ECX bits[0:7] and ECX bits[12:15] are encoded,
> and both are emulated in QEMU.
> 
> Considering that Intel and Zhaoxin are already using the 0x1f leaf to
> describe CPU topology, which includes similar information, Intel and
> Zhaoxin will not implement ECX bits[0:7] and bits[12:15] of 0x80000008.
> 
> Therefore, mark these two fields as reserved and clear them for Intel
> and Zhaoxin guests.
> 
> Reviewed-by: Tao Su <tao1.su@linux.intel.com>
> Tested-by: Yi Lai <yi1.lai@intel.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> Changes Since v1:
>   * Consider Zhaoxin (Ewan).
>   * Only clear ECX bits[0:7] and bits[12:15] for Intel/Zhaoxin, and do
>     not cover other bits.
> ---
>   target/i386/cpu.c | 22 ++++++++++++++++------
>   1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 37e4bf51d890..abd529d587ba 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -8387,15 +8387,25 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>                *eax |= (cpu->guest_phys_bits << 16);
>           }
>           *ebx = env->features[FEAT_8000_0008_EBX];
> +
>           if (threads_per_pkg > 1) {
>               /*
> -             * Bits 15:12 is "The number of bits in the initial
> -             * Core::X86::Apic::ApicId[ApicId] value that indicate
> -             * thread ID within a package".
> -             * Bits 7:0 is "The number of threads in the package is NC+1"
> +             * Don't emulate Bits [7:0] & Bits [15:12] for Intel/Zhaoxin, since
> +             * they're using 0x1f leaf.
>                */
> -            *ecx = (apicid_pkg_offset(topo_info) << 12) |
> -                   (threads_per_pkg - 1);
> +            if (cpu->vendor_cpuid_only_v2 &&
> +                (IS_INTEL_CPU(env) || IS_ZHAOXIN_CPU(env))) {
> +                    *ecx = 0;
> +            } else {
> +                /*
> +                 * Bits 15:12 is "The number of bits in the initial
> +                 * Core::X86::Apic::ApicId[ApicId] value that indicate
> +                 * thread ID within a package".
> +                 * Bits 7:0 is "The number of threads in the package is NC+1"
> +                 */
> +                *ecx = (apicid_pkg_offset(topo_info) << 12) |
> +                       (threads_per_pkg - 1);
> +            }
>           } else {
>               *ecx = 0;
>           }

I prefer below:

	if ((cpu->vendor_cpuid_only_v2 &&
	    (IS_INTEL_CPU(env) || IS_ZHAOXIN_CPU(env))) ||
	    threads_per_pkg < 2) {
		*ecx = 0;
	} else {
		...
	}

Re: [PATCH v2 2/7] i386/cpu: Mark CPUID 0x80000008 ECX bits[0:7] & [12:15] as reserved for Intel/Zhaoxin

[Zhao Liu]

> >           if (threads_per_pkg > 1) {
> >               /*
> > -             * Bits 15:12 is "The number of bits in the initial
> > -             * Core::X86::Apic::ApicId[ApicId] value that indicate
> > -             * thread ID within a package".
> > -             * Bits 7:0 is "The number of threads in the package is NC+1"
> > +             * Don't emulate Bits [7:0] & Bits [15:12] for Intel/Zhaoxin, since
> > +             * they're using 0x1f leaf.
> >                */
> > -            *ecx = (apicid_pkg_offset(topo_info) << 12) |
> > -                   (threads_per_pkg - 1);
> > +            if (cpu->vendor_cpuid_only_v2 &&
> > +                (IS_INTEL_CPU(env) || IS_ZHAOXIN_CPU(env))) {
> > +                    *ecx = 0;
> > +            } else {
> > +                /*
> > +                 * Bits 15:12 is "The number of bits in the initial
> > +                 * Core::X86::Apic::ApicId[ApicId] value that indicate
> > +                 * thread ID within a package".
> > +                 * Bits 7:0 is "The number of threads in the package is NC+1"
> > +                 */
> > +                *ecx = (apicid_pkg_offset(topo_info) << 12) |
> > +                       (threads_per_pkg - 1);
> > +            }
> >           } else {
> >               *ecx = 0;
> >           }
> 
> I prefer below:
> 
> 	if ((cpu->vendor_cpuid_only_v2 &&
> 	    (IS_INTEL_CPU(env) || IS_ZHAOXIN_CPU(env))) ||
> 	    threads_per_pkg < 2) {
> 		*ecx = 0;
> 	} else {
> 		...
> 	}

Yes, this works, but I would like to keep the vendor-related checks
separate from other logic - to avoid mixing them together.

Then it makes the code logic clearer and makes it easier for future
changes.

Thanks for your review!
Zhao

[PATCH v2 3/7] i386/cpu: Reorder CPUID leaves in cpu_x86_cpuid()

[Zhao Liu]

Sort the CPUID leaves strictly by index to facilitate checking and
changing.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 target/i386/cpu.c | 60 +++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index abd529d587ba..9e110e49ab8a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -8050,21 +8050,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         assert(!(*eax & ~0x1f));
         *ebx &= 0xffff; /* The count doesn't need to be reliable. */
         break;
-    case 0x1C:
-        if (cpu->enable_pmu && (env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_ARCH_LBR)) {
-            x86_cpu_get_supported_cpuid(0x1C, 0, eax, ebx, ecx, edx);
-            *edx = 0;
-        }
-        break;
-    case 0x1F:
-        /* V2 Extended Topology Enumeration Leaf */
-        if (!x86_has_cpuid_0x1f(cpu)) {
-            *eax = *ebx = *ecx = *edx = 0;
-            break;
-        }
-
-        encode_topo_cpuid1f(env, count, topo_info, eax, ebx, ecx, edx);
-        break;
     case 0xD: {
         /* Processor Extended State */
         *eax = 0;
@@ -8205,6 +8190,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         }
         break;
     }
+    case 0x1C:
+        if (cpu->enable_pmu && (env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_ARCH_LBR)) {
+            x86_cpu_get_supported_cpuid(0x1C, 0, eax, ebx, ecx, edx);
+            *edx = 0;
+        }
+        break;
     case 0x1D: {
         /* AMX TILE, for now hardcoded for Sapphire Rapids*/
         *eax = 0;
@@ -8242,6 +8233,15 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         }
         break;
     }
+    case 0x1F:
+        /* V2 Extended Topology Enumeration Leaf */
+        if (!x86_has_cpuid_0x1f(cpu)) {
+            *eax = *ebx = *ecx = *edx = 0;
+            break;
+        }
+
+        encode_topo_cpuid1f(env, count, topo_info, eax, ebx, ecx, edx);
+        break;
     case 0x24: {
         *eax = 0;
         *ebx = 0;
@@ -8465,6 +8465,21 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             *edx = 0;
         }
         break;
+    case 0x8000001F:
+        *eax = *ebx = *ecx = *edx = 0;
+        if (sev_enabled()) {
+            *eax = 0x2;
+            *eax |= sev_es_enabled() ? 0x8 : 0;
+            *eax |= sev_snp_enabled() ? 0x10 : 0;
+            *ebx = sev_get_cbit_position() & 0x3f; /* EBX[5:0] */
+            *ebx |= (sev_get_reduced_phys_bits() & 0x3f) << 6; /* EBX[11:6] */
+        }
+        break;
+    case 0x80000021:
+        *eax = *ebx = *ecx = *edx = 0;
+        *eax = env->features[FEAT_8000_0021_EAX];
+        *ebx = env->features[FEAT_8000_0021_EBX];
+        break;
     case 0x80000022:
         *eax = *ebx = *ecx = *edx = 0;
         /* AMD Extended Performance Monitoring and Debug */
@@ -8497,21 +8512,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *ecx = 0;
         *edx = 0;
         break;
-    case 0x8000001F:
-        *eax = *ebx = *ecx = *edx = 0;
-        if (sev_enabled()) {
-            *eax = 0x2;
-            *eax |= sev_es_enabled() ? 0x8 : 0;
-            *eax |= sev_snp_enabled() ? 0x10 : 0;
-            *ebx = sev_get_cbit_position() & 0x3f; /* EBX[5:0] */
-            *ebx |= (sev_get_reduced_phys_bits() & 0x3f) << 6; /* EBX[11:6] */
-        }
-        break;
-    case 0x80000021:
-        *eax = *ebx = *ecx = *edx = 0;
-        *eax = env->features[FEAT_8000_0021_EAX];
-        *ebx = env->features[FEAT_8000_0021_EBX];
-        break;
     default:
         /* reserved values: zero */
         *eax = 0;
-- 
2.34.1

[PATCH v2 4/7] i386/cpu: Fix number of addressable IDs field for CPUID.01H.EBX[23:16]

[Zhao Liu]

From: Chuang Xu <xuchuangxclwt@bytedance.com>

When QEMU is started with:
-cpu host,migratable=on,host-cache-info=on,l3-cache=off
-smp 180,sockets=2,dies=1,cores=45,threads=2

On Intel platform:
CPUID.01H.EBX[23:16] is defined as "max number of addressable IDs for
logical processors in the physical package".

When executing "cpuid -1 -l 1 -r" in the guest, we obtain a value of 90 for
CPUID.01H.EBX[23:16], whereas the expected value is 128. Additionally,
executing "cpuid -1 -l 4 -r" in the guest yields a value of 63 for
CPUID.04H.EAX[31:26], which matches the expected result.

As (1+CPUID.04H.EAX[31:26]) rounds up to the nearest power-of-2 integer,
it's necessary to round up CPUID.01H.EBX[23:16] to the nearest power-of-2
integer too. Otherwise there would be unexpected results in guest with
older kernel.

For example, when QEMU is started with CLI above and xtopology is disabled,
guest kernel 5.15.120 uses CPUID.01H.EBX[23:16]/(1+CPUID.04H.EAX[31:26]) to
calculate threads-per-core in detect_ht(). Then guest will get "90/(1+63)=1"
as the result, even though threads-per-core should actually be 2.

And on AMD platform:
CPUID.01H.EBX[23:16] is defined as "Logical processor count". Current
result meets our expectation.

So round up CPUID.01H.EBX[23:16] to the nearest power-of-2 integer only
for Intel platform to solve the unexpected result.

Use the "x-vendor-cpuid-only-v2" compat option to fix this issue.

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Guixiong Wei <weiguixiong@bytedance.com>
Signed-off-by: Yipeng Yin <yinyipeng@bytedance.com>
Signed-off-by: Chuang Xu <xuchuangxclwt@bytedance.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes Since New v1 [**]:
 * Drop Igor's Acked-by since this version uses the newly added
   x-vendor-cpuid-only-v2.
 * Add Zhaoxin since this is the behavior defined in SDM.

Changes Since original v6 [*] :
 * Rebase on the b69801dd6b1e ("Merge tag 'for_upstream' of
   https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging").
 * Polish the comment in code.
 * Explain the change doesn't need extra compat property.

[*] original v6: https://lore.kernel.org/qemu-devel/20241009035638.59330-1-xuchuangxclwt@bytedance.com/
[**] new v1: https://lore.kernel.org/qemu-devel/20250227062523.124601-2-zhao1.liu@intel.com/
---
 target/i386/cpu.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9e110e49ab8a..7fcb6c144d94 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7869,7 +7869,17 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         }
         *edx = env->features[FEAT_1_EDX];
         if (threads_per_pkg > 1) {
-            *ebx |= threads_per_pkg << 16;
+            /*
+             * For CPUID.01H.EBX[Bits 23-16], AMD requires logical processor
+             * count, but Intel needs maximum number of addressable IDs for
+             * logical processors per package.
+             */
+            if (cpu->vendor_cpuid_only_v2 &&
+                (IS_INTEL_CPU(env) || IS_ZHAOXIN_CPU(env))) {
+                *ebx |= 1 << apicid_pkg_offset(topo_info) << 16;
+            } else {
+                *ebx |= threads_per_pkg << 16;
+            }
         }
         break;
     case 2: { /* cache info: needed for Pentium Pro compatibility */
-- 
2.34.1

Re: [PATCH v2 4/7] i386/cpu: Fix number of addressable IDs field for CPUID.01H.EBX[23:16]

[Xiaoyao Li]

On 7/14/2025 4:08 PM, Zhao Liu wrote:
> From: Chuang Xu <xuchuangxclwt@bytedance.com>
> 
> When QEMU is started with:
> -cpu host,migratable=on,host-cache-info=on,l3-cache=off
> -smp 180,sockets=2,dies=1,cores=45,threads=2
> 
> On Intel platform:
> CPUID.01H.EBX[23:16] is defined as "max number of addressable IDs for
> logical processors in the physical package".
> 
> When executing "cpuid -1 -l 1 -r" in the guest, we obtain a value of 90 for
> CPUID.01H.EBX[23:16], whereas the expected value is 128. Additionally,
> executing "cpuid -1 -l 4 -r" in the guest yields a value of 63 for
> CPUID.04H.EAX[31:26], which matches the expected result.
> 
> As (1+CPUID.04H.EAX[31:26]) rounds up to the nearest power-of-2 integer,
> it's necessary to round up CPUID.01H.EBX[23:16] to the nearest power-of-2
> integer too. Otherwise there would be unexpected results in guest with
> older kernel.
> 
> For example, when QEMU is started with CLI above and xtopology is disabled,
> guest kernel 5.15.120 uses CPUID.01H.EBX[23:16]/(1+CPUID.04H.EAX[31:26]) to
> calculate threads-per-core in detect_ht(). Then guest will get "90/(1+63)=1"
> as the result, even though threads-per-core should actually be 2.
> 
> And on AMD platform:
> CPUID.01H.EBX[23:16] is defined as "Logical processor count". Current
> result meets our expectation.
> 
> So round up CPUID.01H.EBX[23:16] to the nearest power-of-2 integer only
> for Intel platform to solve the unexpected result.
> 
> Use the "x-vendor-cpuid-only-v2" compat option to fix this issue.
> 
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> Signed-off-by: Guixiong Wei <weiguixiong@bytedance.com>
> Signed-off-by: Yipeng Yin <yinyipeng@bytedance.com>
> Signed-off-by: Chuang Xu <xuchuangxclwt@bytedance.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

Re: [PATCH v2 4/7] i386/cpu: Fix number of addressable IDs field for CPUID.01H.EBX[23:16]

[Michael Tokarev]

On 14.07.2025 11:08, Zhao Liu wrote:
> From: Chuang Xu <xuchuangxclwt@bytedance.com>
> 
> When QEMU is started with:
> -cpu host,migratable=on,host-cache-info=on,l3-cache=off
> -smp 180,sockets=2,dies=1,cores=45,threads=2
> 
> On Intel platform:
> CPUID.01H.EBX[23:16] is defined as "max number of addressable IDs for
> logical processors in the physical package".
> 
> When executing "cpuid -1 -l 1 -r" in the guest, we obtain a value of 90 for
> CPUID.01H.EBX[23:16], whereas the expected value is 128. Additionally,
> executing "cpuid -1 -l 4 -r" in the guest yields a value of 63 for
> CPUID.04H.EAX[31:26], which matches the expected result.
> 
> As (1+CPUID.04H.EAX[31:26]) rounds up to the nearest power-of-2 integer,
> it's necessary to round up CPUID.01H.EBX[23:16] to the nearest power-of-2
> integer too. Otherwise there would be unexpected results in guest with
> older kernel.
> 
> For example, when QEMU is started with CLI above and xtopology is disabled,
> guest kernel 5.15.120 uses CPUID.01H.EBX[23:16]/(1+CPUID.04H.EAX[31:26]) to
> calculate threads-per-core in detect_ht(). Then guest will get "90/(1+63)=1"
> as the result, even though threads-per-core should actually be 2.
> 
> And on AMD platform:
> CPUID.01H.EBX[23:16] is defined as "Logical processor count". Current
> result meets our expectation.
> 
> So round up CPUID.01H.EBX[23:16] to the nearest power-of-2 integer only
> for Intel platform to solve the unexpected result.
> 
> Use the "x-vendor-cpuid-only-v2" compat option to fix this issue.
> 
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> Signed-off-by: Guixiong Wei <weiguixiong@bytedance.com>
> Signed-off-by: Yipeng Yin <yinyipeng@bytedance.com>
> Signed-off-by: Chuang Xu <xuchuangxclwt@bytedance.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> Changes Since New v1 [**]:
>   * Drop Igor's Acked-by since this version uses the newly added
>     x-vendor-cpuid-only-v2.
>   * Add Zhaoxin since this is the behavior defined in SDM.
> 
> Changes Since original v6 [*] :
>   * Rebase on the b69801dd6b1e ("Merge tag 'for_upstream' of
>     https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging").
>   * Polish the comment in code.
>   * Explain the change doesn't need extra compat property.
> 
> [*] original v6: https://lore.kernel.org/qemu-devel/20241009035638.59330-1-xuchuangxclwt@bytedance.com/
> [**] new v1: https://lore.kernel.org/qemu-devel/20250227062523.124601-2-zhao1.liu@intel.com/
> ---
>   target/i386/cpu.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 9e110e49ab8a..7fcb6c144d94 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -7869,7 +7869,17 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>           }
>           *edx = env->features[FEAT_1_EDX];
>           if (threads_per_pkg > 1) {
> -            *ebx |= threads_per_pkg << 16;
> +            /*
> +             * For CPUID.01H.EBX[Bits 23-16], AMD requires logical processor
> +             * count, but Intel needs maximum number of addressable IDs for
> +             * logical processors per package.
> +             */
> +            if (cpu->vendor_cpuid_only_v2 &&
> +                (IS_INTEL_CPU(env) || IS_ZHAOXIN_CPU(env))) {

Hi!

Previous incarnation of this patch were Cc'd qemu-stable@, as it were
supposed to be picked up for the stable qemu series.  However, this
incarnation is not Cc'd to stable, and, most importantly, it relies
on a feature which was introduced after all released qemu versions.
Namely, vendor_cpuid_only_v2 is past v10.0.0, which is commit
216d9bb6d7716 "i386/cpu: Add x-vendor-cpuid-only-v2 option for
compatibility".

Should I omit this change for stable-10.0 series, or should it be
modified to work in 10.0?

Thanks,

/mjt

Re: [PATCH v2 4/7] i386/cpu: Fix number of addressable IDs field for CPUID.01H.EBX[23:16]

[Zhao Liu]

> Hi!
> 
> Previous incarnation of this patch were Cc'd qemu-stable@, as it were
> supposed to be picked up for the stable qemu series.  However, this
> incarnation is not Cc'd to stable, and, most importantly, it relies
> on a feature which was introduced after all released qemu versions.
> Namely, vendor_cpuid_only_v2 is past v10.0.0, which is commit
> 216d9bb6d7716 "i386/cpu: Add x-vendor-cpuid-only-v2 option for
> compatibility".
> 
> Should I omit this change for stable-10.0 series, or should it be
> modified to work in 10.0?

Hi Michael, considerring current fix is covered by compat option,
it's not fit to be backported to the previous version.

This issue has existed for a very long time, and the feedback received
on this is currently one case and it based on a specific topology
configuration, so the impact is limited. Therefore, in this patch
version, I fixed it with the newly added compat option, which also
avoids the controversy about the impact of migration.

So I think you could omit this change for stable-10.0 series, if it's
also okay with Paolo.

Thanks,
Zhao

Re: [PATCH v2 4/7] i386/cpu: Fix number of addressable IDs field for CPUID.01H.EBX[23:16]

[Michael Tokarev]

On 17.07.2025 06:06, Zhao Liu wrote:
>> Hi!
>>
>> Previous incarnation of this patch were Cc'd qemu-stable@, as it were
>> supposed to be picked up for the stable qemu series.  However, this
>> incarnation is not Cc'd to stable, and, most importantly, it relies
>> on a feature which was introduced after all released qemu versions.
>> Namely, vendor_cpuid_only_v2 is past v10.0.0, which is commit
>> 216d9bb6d7716 "i386/cpu: Add x-vendor-cpuid-only-v2 option for
>> compatibility".
>>
>> Should I omit this change for stable-10.0 series, or should it be
>> modified to work in 10.0?
> 
> Hi Michael, considerring current fix is covered by compat option,
> it's not fit to be backported to the previous version.
> 
> This issue has existed for a very long time, and the feedback received
> on this is currently one case and it based on a specific topology
> configuration, so the impact is limited. Therefore, in this patch
> version, I fixed it with the newly added compat option, which also
> avoids the controversy about the impact of migration.
> 
> So I think you could omit this change for stable-10.0 series, if it's
> also okay with Paolo.

This makes sense.

In this case though, the next patch, a62fef5829956 "i386/cpu: Fix cpu
number overflow in CPUID.01H.EBX[23:16]", becomes a one-liner:

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 2c9517f56d..5e55dd9ee5 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6828,7 +6828,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t 
index, uint32_t count,
          }
          *edx = env->features[FEAT_1_EDX];
          if (threads_per_pkg > 1) {
-            *ebx |= threads_per_pkg << 16;
+            /* Fixup overflow: max value for bits 23-16 is 255. */
+            *ebx |= MIN(threads_per_pkg, 255) << 16;
          }
          if (!cpu->enable_pmu) {
              *ecx &= ~CPUID_EXT_PDCM;

Is it okay with you, or maybe should I drop this change for 10.0.x
too?

Thanks,

/mjt

Re: [PATCH v2 4/7] i386/cpu: Fix number of addressable IDs field for CPUID.01H.EBX[23:16]

[Zhao Liu]

> This makes sense.
> 
> In this case though, the next patch, a62fef5829956 "i386/cpu: Fix cpu
> number overflow in CPUID.01H.EBX[23:16]", becomes a one-liner:
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 2c9517f56d..5e55dd9ee5 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -6828,7 +6828,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
> uint32_t count,
>          }
>          *edx = env->features[FEAT_1_EDX];
>          if (threads_per_pkg > 1) {
> -            *ebx |= threads_per_pkg << 16;
> +            /* Fixup overflow: max value for bits 23-16 is 255. */
> +            *ebx |= MIN(threads_per_pkg, 255) << 16;
>          }
>          if (!cpu->enable_pmu) {
>              *ecx &= ~CPUID_EXT_PDCM;
> 
> Is it okay with you, or maybe should I drop this change for 10.0.x
> too?

Yes, it's okay. This commit I should have cc'd stable, as it is indeed a
reported bug.

And after this commit, there're another 2 commits to avoid overflow:

* commit 3e86124e7cb9 ("i386/cpu: Fix overflow of cache topology fields
  in CPUID.04H")
* commit 5d21ee453ad8 ("i386/cpu: Honor maximum value for CPUID.
  8000001DH.EAX[25:14]")

I think these two could also be stable v10.0.x materials.

Thanks,
Zhao

[PATCH v2 5/7] i386/cpu: Fix cpu number overflow in CPUID.01H.EBX[23:16]

[Zhao Liu]

From: Qian Wen <qian.wen@intel.com>

The legacy topology enumerated by CPUID.1.EBX[23:16] is defined in SDM
Vol2:

Bits 23-16: Maximum number of addressable IDs for logical processors in
this physical package.

When threads_per_socket > 255, it will 1) overwrite bits[31:24] which is
apic_id, 2) bits [23:16] get truncated.

Specifically, if launching the VM with -smp 256, the value written to
EBX[23:16] is 0 because of data overflow. If the guest only supports
legacy topology, without V2 Extended Topology enumerated by CPUID.0x1f
or Extended Topology enumerated by CPUID.0x0b to support over 255 CPUs,
the return of the kernel invoking cpu_smt_allowed() is false and APs
(application processors) will fail to bring up. Then only CPU 0 is online,
and others are offline.

For example, launch VM via:
qemu-system-x86_64 -M q35,accel=kvm,kernel-irqchip=split \
    -cpu qemu64,cpuid-0xb=off -smp 256 -m 32G \
    -drive file=guest.img,if=none,id=virtio-disk0,format=raw \
    -device virtio-blk-pci,drive=virtio-disk0,bootindex=1 --nographic

The guest shows:
    CPU(s):               256
    On-line CPU(s) list:  0
    Off-line CPU(s) list: 1-255

To avoid this issue caused by overflow, limit the max value written to
EBX[23:16] to 255 as the HW does.

Cc: qemu-stable@nongnu.org
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
Signed-off-by: Qian Wen <qian.wen@intel.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes Since New v1 [**]:
 * Rebase.

Changes Since Original v4 [*]:
 * Rebase on addressable ID fixup.
 * Drop R/b tags since the code base changes.

[*] original v4: https://lore.kernel.org/qemu-devel/20230829042405.932523-2-qian.wen@intel.com/
[**]: new v1: https://lore.kernel.org/qemu-devel/20250227062523.124601-3-zhao1.liu@intel.com/
---
 target/i386/cpu.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 7fcb6c144d94..67a371e23b22 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7869,6 +7869,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         }
         *edx = env->features[FEAT_1_EDX];
         if (threads_per_pkg > 1) {
+            uint32_t num;
+
             /*
              * For CPUID.01H.EBX[Bits 23-16], AMD requires logical processor
              * count, but Intel needs maximum number of addressable IDs for
@@ -7876,10 +7878,13 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
              */
             if (cpu->vendor_cpuid_only_v2 &&
                 (IS_INTEL_CPU(env) || IS_ZHAOXIN_CPU(env))) {
-                *ebx |= 1 << apicid_pkg_offset(topo_info) << 16;
+                num = 1 << apicid_pkg_offset(topo_info);
             } else {
-                *ebx |= threads_per_pkg << 16;
+                num = threads_per_pkg;
             }
+
+            /* Fixup overflow: max value for bits 23-16 is 255. */
+            *ebx |= MIN(num, 255) << 16;
         }
         break;
     case 2: { /* cache info: needed for Pentium Pro compatibility */
-- 
2.34.1

[PATCH v2 6/7] i386/cpu: Fix overflow of cache topology fields in CPUID.04H

[Zhao Liu]

From: Qian Wen <qian.wen@intel.com>

According to SDM, CPUID.0x4:EAX[31:26] indicates the Maximum number of
addressable IDs for processor cores in the physical package. If we
launch over 64 cores VM, the 6-bit field will overflow, and the wrong
core_id number will be reported.

Since the HW reports 0x3f when the intel processor has over 64 cores,
limit the max value written to EAX[31:26] to 63, so max num_cores should
be 64.

For EAX[14:25], though at present Q35 supports up to 4096 CPUs, by
constructing a specific topology, the width of the APIC ID can be
extended beyond 12 bits. For example, using `-smp threads=33,cores=9,
modules=9` results in a die level offset of 6 + 4 + 4 = 14 bits, which
can also cause overflow.  check and honor the maximum value for
EAX[14:25] as well.

In addition, for host-cache-info case, also apply the same checks and
fixes.

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
Signed-off-by: Qian Wen <qian.wen@intel.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes Since New v1 [**]:
 * Provide an overflow example of EAX[14:25].

Changes Since Original v4 [*]:
 * Rebase on addressable ID fixup.
 * Drop R/b tags since the code base changes.
 * Teak bits 25-14 as well and add the comment.
 * Fix overflow for host-cache-info case.

[*]: original v4: https://lore.kernel.org/qemu-devel/20230829042405.932523-3-qian.wen@intel.com/
[**]: new v1: https://lore.kernel.org/qemu-devel/20250227062523.124601-4-zhao1.liu@intel.com/
---
 target/i386/cpu.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 67a371e23b22..fedeeea151ee 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -347,11 +347,17 @@ static void encode_cache_cpuid4(CPUCacheInfo *cache,
     assert(cache->size == cache->line_size * cache->associativity *
                           cache->partitions * cache->sets);
 
+    /*
+     * The following fields have bit-width limitations, so consider the
+     * maximum values to avoid overflow:
+     * Bits 25-14: maximum 4095.
+     * Bits 31-26: maximum 63.
+     */
     *eax = CACHE_TYPE(cache->type) |
            CACHE_LEVEL(cache->level) |
            (cache->self_init ? CACHE_SELF_INIT_LEVEL : 0) |
-           (max_core_ids_in_package(topo_info) << 26) |
-           (max_thread_ids_for_cache(topo_info, cache->share_level) << 14);
+           (MIN(max_core_ids_in_package(topo_info), 63) << 26) |
+           (MIN(max_thread_ids_for_cache(topo_info, cache->share_level), 4095) << 14);
 
     assert(cache->line_size > 0);
     assert(cache->partitions > 0);
@@ -7928,13 +7934,13 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
                 int host_vcpus_per_cache = 1 + ((*eax & 0x3FFC000) >> 14);
 
                 *eax &= ~0xFC000000;
-                *eax |= max_core_ids_in_package(topo_info) << 26;
+                *eax |= MIN(max_core_ids_in_package(topo_info), 63) << 26;
                 if (host_vcpus_per_cache > threads_per_pkg) {
                     *eax &= ~0x3FFC000;
 
                     /* Share the cache at package level. */
-                    *eax |= max_thread_ids_for_cache(topo_info,
-                                CPU_TOPOLOGY_LEVEL_SOCKET) << 14;
+                    *eax |= MIN(max_thread_ids_for_cache(topo_info,
+                                CPU_TOPOLOGY_LEVEL_SOCKET), 4095) << 14;
                 }
             }
         } else if (cpu->vendor_cpuid_only && IS_AMD_CPU(env)) {
-- 
2.34.1

[PATCH v2 7/7] i386/cpu: Honor maximum value for CPUID.8000001DH.EAX[25:14]

[Zhao Liu]

CPUID.8000001DH:EAX[25:14] is "NumSharingCache", and the number of
logical processors sharing this cache is the value of this field
incremented by 1. Because of its width limitation, the maximum value
currently supported is 4095.

Though at present Q35 supports up to 4096 CPUs, by constructing a
specific topology, the width of the APIC ID can be extended beyond 12
bits. For example, using `-smp threads=33,cores=9,modules=9` results in
a die level offset of 6 + 4 + 4 = 14 bits, which can also cause
overflow. Check and honor the maximum value as CPUID.04H did.

Cc: Babu Moger <babu.moger@amd.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes Since RFC v1 [*]:
 * Correct the RFC's description, now there's the overflow case. Provide
   an overflow example.

RFC:
 * Although there are currently no overflow cases, to avoid any
   potential issue, add the overflow check, just as I did for Intel.

[*]: https://lore.kernel.org/qemu-devel/20250227062523.124601-5-zhao1.liu@intel.com/
---
 target/i386/cpu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index fedeeea151ee..eceda9865b8f 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -558,7 +558,8 @@ static void encode_cache_cpuid8000001d(CPUCacheInfo *cache,
 
     *eax = CACHE_TYPE(cache->type) | CACHE_LEVEL(cache->level) |
                (cache->self_init ? CACHE_SELF_INIT_LEVEL : 0);
-    *eax |= max_thread_ids_for_cache(topo_info, cache->share_level) << 14;
+    /* Bits 25:14 - NumSharingCache: maximum 4095. */
+    *eax |= MIN(max_thread_ids_for_cache(topo_info, cache->share_level), 4095) << 14;
 
     assert(cache->line_size > 0);
     assert(cache->partitions > 0);
-- 
2.34.1

Re: [PATCH v2 7/7] i386/cpu: Honor maximum value for CPUID.8000001DH.EAX[25:14]

[Moger, Babu]

Hi Zhao,

On 7/14/25 03:08, Zhao Liu wrote:
> CPUID.8000001DH:EAX[25:14] is "NumSharingCache", and the number of
> logical processors sharing this cache is the value of this field
> incremented by 1. Because of its width limitation, the maximum value
> currently supported is 4095.
> 
> Though at present Q35 supports up to 4096 CPUs, by constructing a
> specific topology, the width of the APIC ID can be extended beyond 12
> bits. For example, using `-smp threads=33,cores=9,modules=9` results in
> a die level offset of 6 + 4 + 4 = 14 bits, which can also cause
> overflow. Check and honor the maximum value as CPUID.04H did.
> 
> Cc: Babu Moger <babu.moger@amd.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> Changes Since RFC v1 [*]:
>  * Correct the RFC's description, now there's the overflow case. Provide
>    an overflow example.
> 
> RFC:
>  * Although there are currently no overflow cases, to avoid any
>    potential issue, add the overflow check, just as I did for Intel.
> 
> [*]: https://lore.kernel.org/qemu-devel/20250227062523.124601-5-zhao1.liu@intel.com/
> ---
>  target/i386/cpu.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index fedeeea151ee..eceda9865b8f 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -558,7 +558,8 @@ static void encode_cache_cpuid8000001d(CPUCacheInfo *cache,
>  
>      *eax = CACHE_TYPE(cache->type) | CACHE_LEVEL(cache->level) |
>                 (cache->self_init ? CACHE_SELF_INIT_LEVEL : 0);
> -    *eax |= max_thread_ids_for_cache(topo_info, cache->share_level) << 14;
> +    /* Bits 25:14 - NumSharingCache: maximum 4095. */
> +    *eax |= MIN(max_thread_ids_for_cache(topo_info, cache->share_level), 4095) << 14;

Will this be more meaningful?

*eax |=
 max_thread_ids_for_cache(topo_info, cache->share_level) & 0xFFF << 14

>  
>      assert(cache->line_size > 0);
>      assert(cache->partitions > 0);

-- 
Thanks
Babu Moger

Re: [PATCH v2 7/7] i386/cpu: Honor maximum value for CPUID.8000001DH.EAX[25:14]

[Zhao Liu]

On Mon, Jul 14, 2025 at 09:51:25AM -0500, Moger, Babu wrote:
> Date: Mon, 14 Jul 2025 09:51:25 -0500
> From: "Moger, Babu" <babu.moger@amd.com>
> Subject: Re: [PATCH v2 7/7] i386/cpu: Honor maximum value for
>  CPUID.8000001DH.EAX[25:14]
> 
> Hi Zhao,
> 
> On 7/14/25 03:08, Zhao Liu wrote:
> > CPUID.8000001DH:EAX[25:14] is "NumSharingCache", and the number of
> > logical processors sharing this cache is the value of this field
> > incremented by 1. Because of its width limitation, the maximum value
> > currently supported is 4095.
> > 
> > Though at present Q35 supports up to 4096 CPUs, by constructing a
> > specific topology, the width of the APIC ID can be extended beyond 12
> > bits. For example, using `-smp threads=33,cores=9,modules=9` results in
> > a die level offset of 6 + 4 + 4 = 14 bits, which can also cause
> > overflow. Check and honor the maximum value as CPUID.04H did.
> > 
> > Cc: Babu Moger <babu.moger@amd.com>
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > ---
> > Changes Since RFC v1 [*]:
> >  * Correct the RFC's description, now there's the overflow case. Provide
> >    an overflow example.
> > 
> > RFC:
> >  * Although there are currently no overflow cases, to avoid any
> >    potential issue, add the overflow check, just as I did for Intel.
> > 
> > [*]: https://lore.kernel.org/qemu-devel/20250227062523.124601-5-zhao1.liu@intel.com/
> > ---
> >  target/i386/cpu.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index fedeeea151ee..eceda9865b8f 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -558,7 +558,8 @@ static void encode_cache_cpuid8000001d(CPUCacheInfo *cache,
> >  
> >      *eax = CACHE_TYPE(cache->type) | CACHE_LEVEL(cache->level) |
> >                 (cache->self_init ? CACHE_SELF_INIT_LEVEL : 0);
> > -    *eax |= max_thread_ids_for_cache(topo_info, cache->share_level) << 14;
> > +    /* Bits 25:14 - NumSharingCache: maximum 4095. */
> > +    *eax |= MIN(max_thread_ids_for_cache(topo_info, cache->share_level), 4095) << 14;
> 
> Will this be more meaningful?
> 
> *eax |=
>  max_thread_ids_for_cache(topo_info, cache->share_level) & 0xFFF << 14

Hi Babu, thank you for your feedback! This approach depends on truncation,
which might lead to more erroneous conclusions. Currently, such cases
shouldn't exist on actual hardware; it's only QEMU that supports so many
CPUs and custom topologies.

Previously, when Intel handled similar cases (where the topology space
wasn't large enough), it would encode the maximum value rather than
truncate, as I'm doing now (you can refer to the description of 0x1 in
patch 5, and similar fixes in Intel's 0x4 leaf in patch 6). In the
future, if actual hardware CPUs reach such numbers and has special
behavior, we can update accordingly. I think at least for now, this
avoids overflow caused by special topology in QEMU emulation.

Thanks,
Zhao

Re: [PATCH v2 7/7] i386/cpu: Honor maximum value for CPUID.8000001DH.EAX[25:14]

[Moger, Babu]

On 7/14/25 10:41, Zhao Liu wrote:
> On Mon, Jul 14, 2025 at 09:51:25AM -0500, Moger, Babu wrote:
>> Date: Mon, 14 Jul 2025 09:51:25 -0500
>> From: "Moger, Babu" <babu.moger@amd.com>
>> Subject: Re: [PATCH v2 7/7] i386/cpu: Honor maximum value for
>>  CPUID.8000001DH.EAX[25:14]
>>
>> Hi Zhao,
>>
>> On 7/14/25 03:08, Zhao Liu wrote:
>>> CPUID.8000001DH:EAX[25:14] is "NumSharingCache", and the number of
>>> logical processors sharing this cache is the value of this field
>>> incremented by 1. Because of its width limitation, the maximum value
>>> currently supported is 4095.
>>>
>>> Though at present Q35 supports up to 4096 CPUs, by constructing a
>>> specific topology, the width of the APIC ID can be extended beyond 12
>>> bits. For example, using `-smp threads=33,cores=9,modules=9` results in
>>> a die level offset of 6 + 4 + 4 = 14 bits, which can also cause
>>> overflow. Check and honor the maximum value as CPUID.04H did.
>>>
>>> Cc: Babu Moger <babu.moger@amd.com>
>>> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Babu Moger <babu.moger@amd.com>

>>> ---
>>> Changes Since RFC v1 [*]:
>>>  * Correct the RFC's description, now there's the overflow case. Provide
>>>    an overflow example.
>>>
>>> RFC:
>>>  * Although there are currently no overflow cases, to avoid any
>>>    potential issue, add the overflow check, just as I did for Intel.
>>>
>>> [*]: https://lore.kernel.org/qemu-devel/20250227062523.124601-5-zhao1.liu@intel.com/
>>> ---
>>>  target/i386/cpu.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>>> index fedeeea151ee..eceda9865b8f 100644
>>> --- a/target/i386/cpu.c
>>> +++ b/target/i386/cpu.c
>>> @@ -558,7 +558,8 @@ static void encode_cache_cpuid8000001d(CPUCacheInfo *cache,
>>>  
>>>      *eax = CACHE_TYPE(cache->type) | CACHE_LEVEL(cache->level) |
>>>                 (cache->self_init ? CACHE_SELF_INIT_LEVEL : 0);
>>> -    *eax |= max_thread_ids_for_cache(topo_info, cache->share_level) << 14;
>>> +    /* Bits 25:14 - NumSharingCache: maximum 4095. */
>>> +    *eax |= MIN(max_thread_ids_for_cache(topo_info, cache->share_level), 4095) << 14;
>>
>> Will this be more meaningful?
>>
>> *eax |=
>>  max_thread_ids_for_cache(topo_info, cache->share_level) & 0xFFF << 14
> 
> Hi Babu, thank you for your feedback! This approach depends on truncation,
> which might lead to more erroneous conclusions. Currently, such cases
> shouldn't exist on actual hardware; it's only QEMU that supports so many
> CPUs and custom topologies.
> 
> Previously, when Intel handled similar cases (where the topology space
> wasn't large enough), it would encode the maximum value rather than
> truncate, as I'm doing now (you can refer to the description of 0x1 in
> patch 5, and similar fixes in Intel's 0x4 leaf in patch 6). In the
> future, if actual hardware CPUs reach such numbers and has special
> behavior, we can update accordingly. I think at least for now, this
> avoids overflow caused by special topology in QEMU emulation.
> 

Sure. Sounds good to me.

-- 
Thanks
Babu Moger

Re: [PATCH v2 0/7] i386/cpu: Clean Up Reserved CPUID Leaves & Topology Overflow Fix

[Paolo Bonzini]

Applied except for patch 3 (which has conflicts), thanks.

Paolo