[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <434b4b74-54ab-68a3-4a81-9cc02ea75e39@bytedance.com>
Date: Sun, 5 Feb 2023 22:13:42 +0000
From: Usama Arif <usama.arif@...edance.com>
To: David Woodhouse <dwmw2@...radead.org>,
Arjan van de Ven <arjan@...ux.intel.com>,
Kim Phillips <kim.phillips@....com>, tglx@...utronix.de,
rja@....com
Cc: mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
hpa@...or.com, x86@...nel.org, pbonzini@...hat.com,
paulmck@...nel.org, linux-kernel@...r.kernel.org,
kvm@...r.kernel.org, rcu@...r.kernel.org, mimoja@...oja.de,
hewenliang4@...wei.com, thomas.lendacky@....com, seanjc@...gle.com,
pmenzel@...gen.mpg.de, fam.zheng@...edance.com,
punit.agrawal@...edance.com, simon.evans@...edance.com,
liangma@...ngbit.com, Mario Limonciello <Mario.Limonciello@....com>
Subject: Re: [External] Re: [PATCH v6 07/11] x86/smpboot: Disable parallel
boot for AMD CPUs
On 04/02/2023 22:31, David Woodhouse wrote:
>
>
> On 4 February 2023 18:18:55 GMT, Arjan van de Ven <arjan@...ux.intel.com> wrote:
>>>
>>> However...
>>>
>>> Even though we *can* support non-X2APIC processors, we *might* want to
>>> play it safe and not go back that far; only enabling parallel bringup
>>> on machines with X2APIC which roughly correlates with "lots of CPUs"
>>> since that's where the benefit is.
>>
>> I think that this is the right approach, at least on the initial patch series.
>> KISS principle; do all the easy-but-important cases first, get it stable and working
>> and in later series/kernels the range can be expanded.... if it matters.
>
> Agreed. I'll split it to do it only with X2APIC for the initial series, and then hold the CPUID 0x1 part back for the next phase.
This was an interesting find! I tested the latest branch
parallel-6.2-rc6 and it works well. The numbers from Russ makes the
patch series look so much better! :)
If we do it with x2apic only and not support non-x2apic CPUID 0x1 case,
maybe we apply the following diff to part 1?
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index f53a060a899b..f6b89cf40076 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1564,7 +1564,7 @@ void __init native_smp_prepare_cpus(unsigned int
max_cpus)
* sufficient). Otherwise it's too hard. And not for SEV-ES
* guests because they can't use CPUID that early.
*/
- if (IS_ENABLED(CONFIG_X86_32) || boot_cpu_data.cpuid_level < 1 ||
+ if (IS_ENABLED(CONFIG_X86_32) || !x2apic_mode ||
(x2apic_mode && boot_cpu_data.cpuid_level < 0xb) ||
cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
do_parallel_bringup = false;
For reusing timer calibration, calibrate_delay ends up being used in
start_kernel, smp_callin, debug_calc_bogomips and check_cx686_slop. I
think reusing timer calibration would be ok in the first 2 uses? but not
really sure about the other 2. cx686 seems to be quite old so not sure
if anyone will have it to test or will ever run 6.2 kernel on it :). I
guess if unsure, better to leave out initially and try and get part1 merged?
Thanks,
Usama
Powered by blists - more mailing lists