Re: [PATCH] Revert "i386/cpu: Set up CPUID_HT in x86_cpu_expand_features() instead of cpu_x86_cpuid()"

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

Hi Elisey,

Thank you for your patch! I'm sorry I previously noticed #2894 [*] but
missed your report.

Recently there's a fix (commit 5979f50fa9fd ("i386/tcg: Make CPUID_HT
and CPUID_EXT3_CMP_LEG supported")). Does that fix address this bug?

[*]: https://lore.kernel.org/qemu-devel/aCWdhIqZiu4q+UJi@intel.com/

Regards,
Zhao

On Tue, May 27, 2025 at 07:10:27PM +0300, elisey.konstantinov@icloud.com wrote:
> Date: Tue, 27 May 2025 19:10:27 +0300
> From: elisey.konstantinov@icloud.com
> Subject: [PATCH] Revert "i386/cpu: Set up CPUID_HT in
>  x86_cpu_expand_features() instead of cpu_x86_cpuid()"
> X-Mailer: Apple Mail (2.3826.600.51.1.1)
> 
> From e2f3eab60e9b9787c5c8f87bea5d1bd7079d982e Mon Sep 17 00:00:00 2001
> From: Elisey <elisey.konstantinov@icloud.com>
> Date: Tue, 27 May 2025 17:17:35 +0300
> Subject: [PATCH] Revert "i386/cpu: Set up CPUID_HT in x86_cpu_expand_features() instead of cpu_x86_cpuid()"
> 
> This reverts commit c6bd2dd634208ca717b6dc010064fe34d1359080.
> 
> The original change caused a regression where macOS guests (XNU kernel)
> would panic during boot with a divide error (type=0) when using SMP
> configuration. This affects multiple macOS versions from 10.6 to 10.14
> and possibly others.
> 
> The issue occurs during kernel TSC initialization and can be worked
> around by using single-core configuration (-smp 1), but reverting this
> change restores proper multi-core functionality.
> 
> Buglink: https://gitlab.com/qemu-project/qemu/-/issues/2933
> Tested-by: Elisey Konstantinov <elisey.konstantinov@icloud.com>
> Signed-off-by: Elisey Konstantinov <elisey.konstantinov@icloud.com>
> ---
> target/i386/cpu.c | 15 +--------------
> 1 file changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 9689f6374e..c648a1cf04 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -6859,6 +6859,7 @@ 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;
> +            *edx |= CPUID_HT;
>         }
>         if (!cpu->enable_pmu) {
>             *ecx &= ~CPUID_EXT_PDCM;
> @@ -7838,20 +7839,6 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
>         }
>     }
> 
> -    if (x86_threads_per_pkg(&env->topo_info) > 1) {
> -        env->features[FEAT_1_EDX] |= CPUID_HT;
> -
> -        /*
> -         * 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 (and other processors
> -         * following Intel's behavior) to make Linux guests happy.
> -         */
> -        if (!IS_INTEL_CPU(env) && !IS_ZHAOXIN_CPU(env)) {
> -            env->features[FEAT_8000_0001_ECX] |= CPUID_EXT3_CMP_LEG;
> -        }
> -    }
> -
>     for (i = 0; i < ARRAY_SIZE(feature_dependencies); i++) {
>         FeatureDep *d = &feature_dependencies[i];
>         if (!(env->features[d->from.index] & d->from.mask)) {
> -- 
> 2.39.5 (Apple Git-154)
> 
>