| Message ID | 20240809094259.119221-5-ewanhai-oc@zhaoxin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for Zhaoxin Yongfeng CPU model and | expand |
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>
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> > >
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 */ } }
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(-)