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

Zhao Liu posted 7 patches 5 months ago
[PATCH v2 2/7] i386/cpu: Mark CPUID 0x80000008 ECX bits[0:7] & [12:15] as reserved for Intel/Zhaoxin
Posted by Zhao Liu 5 months ago
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
Posted by Xiaoyao Li 5 months ago
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
Posted by Zhao Liu 5 months ago
> >           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