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

[Zhao Liu <zhao1.liu@intel.com>]

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>