[v3] Add support for Zhaoxin Yongfeng CPU model and

[PATCH v3 4/4] target/i386: Mask CMPLegacy bit in CPUID[0x80000001].ECX for Zhaoxin CPUs

Posted by EwanHai 3 months, 2 weeks ago

Zhaoxin CPUs (including vendors "Shanghai" and "Centaurhauls") handle the
CMPLegacy bit similarly to Intel CPUs. Therefore, this commit masks the
CMPLegacy bit in CPUID[0x80000001].ECX for Zhaoxin CPUs, just as it is done
for Intel CPUs.

AMD uses the CMPLegacy bit (CPUID[0x80000001].ECX.bit1) along with other CPUID
information to enumerate platform topology (e.g., the number of logical
processors per package). However, for Intel and other CPUs that follow Intel's
behavior, CPUID[0x80000001].ECX.bit1 is reserved.

- Impact on Intel and similar CPUs:
This change has no effect on Intel and similar CPUs, as the goal is to
accurately emulate CPU CPUID information.

- Impact on Linux Guests running on Intel (and similar) vCPUs:
During boot, Linux checks if the CPU supports Hyper-Threading. If it detects
X86_FEATURE_CMP_LEGACY, it assumes Hyper-Threading is not supported. For Intel
and similar vCPUs, if the CMPLegacy bit is not masked in CPUID[0x80000001].ECX,
Linux will incorrectly assume that Hyper-Threading is not supported, even if
the vCPU does support it.

Signed-off-by: EwanHai <ewanhai-oc@zhaoxin.com>
---
 target/i386/cpu.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 95849c40ad..eb55d92e8a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6995,12 +6995,11 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
 
         /* The Linux kernel checks for the CMPLegacy bit and
          * discards multiple thread information if it is set.
-         * So don't set it here for Intel to make Linux guests happy.
+         * So don't set it here for Intel(and other processors
+         * following Intel's behavior) to make Linux guests happy.
          */
         if (threads_per_pkg > 1) {
-            if (env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1 ||
-                env->cpuid_vendor2 != CPUID_VENDOR_INTEL_2 ||
-                env->cpuid_vendor3 != CPUID_VENDOR_INTEL_3) {
+            if (!IS_INTEL_CPU(env) && !IS_ZHAOXIN_CPU(env)) {
                 *ecx |= 1 << 1;    /* CmpLegacy bit */
             }
         }
-- 
2.34.1

Re: [PATCH v3 4/4] target/i386: Mask CMPLegacy bit in CPUID[0x80000001].ECX for Zhaoxin CPUs

Posted by Zhao Liu 3 months, 2 weeks ago

On Fri, Aug 09, 2024 at 05:42:59AM -0400, EwanHai wrote:
> Date: Fri, 9 Aug 2024 05:42:59 -0400
> From: EwanHai <ewanhai-oc@zhaoxin.com>
> Subject: [PATCH v3 4/4] target/i386: Mask CMPLegacy bit in
>  CPUID[0x80000001].ECX for Zhaoxin CPUs
> X-Mailer: git-send-email 2.34.1
> 
> Zhaoxin CPUs (including vendors "Shanghai" and "Centaurhauls") handle the
> CMPLegacy bit similarly to Intel CPUs. Therefore, this commit masks the
> CMPLegacy bit in CPUID[0x80000001].ECX for Zhaoxin CPUs, just as it is done
> for Intel CPUs.
> 
> AMD uses the CMPLegacy bit (CPUID[0x80000001].ECX.bit1) along with other CPUID
> information to enumerate platform topology (e.g., the number of logical
> processors per package). However, for Intel and other CPUs that follow Intel's
> behavior, CPUID[0x80000001].ECX.bit1 is reserved.
> 
> - Impact on Intel and similar CPUs:
> This change has no effect on Intel and similar CPUs, as the goal is to
> accurately emulate CPU CPUID information.
> 
> - Impact on Linux Guests running on Intel (and similar) vCPUs:
> During boot, Linux checks if the CPU supports Hyper-Threading.
> If it detects

Maybe "For the kernel before v6.9, if it detects"? About this change,
see the below comment...

> X86_FEATURE_CMP_LEGACY, it assumes Hyper-Threading is not supported. For Intel
> and similar vCPUs, if the CMPLegacy bit is not masked in CPUID[0x80000001].ECX,
> Linux will incorrectly assume that Hyper-Threading is not supported, even if
> the vCPU does support it.

...It seems this issue exists in the kernel before v6.9. Thomas'
topology refactoring has fixed this behavior:
* commit 22d63660c35e ("x86/cpu: Use common topology code for Intel")
* commit 598e719c40d6 ("x86/cpu: Use common topology code for Centaur
  and Zhaoxin")

> Signed-off-by: EwanHai <ewanhai-oc@zhaoxin.com>
> ---
>  target/i386/cpu.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)

Just the above nit. Otherwise, LGTM,

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH v3 4/4] target/i386: Mask CMPLegacy bit in CPUID[0x80000001].ECX for Zhaoxin CPUs

Posted by Ewan Hai 3 months, 2 weeks ago

Thank you for your review!, I will udpate the commit message according
to your suggestions to ensure it provides the most accurate information.

On 8/12/24 05:52, Zhao Liu wrote:
> On Fri, Aug 09, 2024 at 05:42:59AM -0400, EwanHai wrote:
>> Date: Fri, 9 Aug 2024 05:42:59 -0400
>> From: EwanHai<ewanhai-oc@zhaoxin.com>
>> Subject: [PATCH v3 4/4] target/i386: Mask CMPLegacy bit in
>>   CPUID[0x80000001].ECX for Zhaoxin CPUs
>> X-Mailer: git-send-email 2.34.1
>>
>> Zhaoxin CPUs (including vendors "Shanghai" and "Centaurhauls") handle the
>> CMPLegacy bit similarly to Intel CPUs. Therefore, this commit masks the
>> CMPLegacy bit in CPUID[0x80000001].ECX for Zhaoxin CPUs, just as it is done
>> for Intel CPUs.
>>
>> AMD uses the CMPLegacy bit (CPUID[0x80000001].ECX.bit1) along with other CPUID
>> information to enumerate platform topology (e.g., the number of logical
>> processors per package). However, for Intel and other CPUs that follow Intel's
>> behavior, CPUID[0x80000001].ECX.bit1 is reserved.
>>
>> - Impact on Intel and similar CPUs:
>> This change has no effect on Intel and similar CPUs, as the goal is to
>> accurately emulate CPU CPUID information.
>>
>> - Impact on Linux Guests running on Intel (and similar) vCPUs:
>> During boot, Linux checks if the CPU supports Hyper-Threading.
>> If it detects
> Maybe "For the kernel before v6.9, if it detects"? About this change,
> see the below comment...
>
>> X86_FEATURE_CMP_LEGACY, it assumes Hyper-Threading is not supported. For Intel
>> and similar vCPUs, if the CMPLegacy bit is not masked in CPUID[0x80000001].ECX,
>> Linux will incorrectly assume that Hyper-Threading is not supported, even if
>> the vCPU does support it.
> ...It seems this issue exists in the kernel before v6.9. Thomas'
> topology refactoring has fixed this behavior:
> * commit 22d63660c35e ("x86/cpu: Use common topology code for Intel")
> * commit 598e719c40d6 ("x86/cpu: Use common topology code for Centaur
>    and Zhaoxin")
>
>> Signed-off-by: EwanHai<ewanhai-oc@zhaoxin.com>
>> ---
>>   target/i386/cpu.c | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
> Just the above nit. Otherwise, LGTM,
>
> Reviewed-by: Zhao Liu<zhao1.liu@intel.com>
>
>