[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9acc229e3d4931fff9106d60b57e0f46941bfb50.camel@infradead.org>
Date: Tue, 07 Feb 2023 10:04:44 +0000
From: David Woodhouse <dwmw2@...radead.org>
To: Thomas Gleixner <tglx@...utronix.de>,
Kim Phillips <kim.phillips@....com>,
Usama Arif <usama.arif@...edance.com>, arjan@...ux.intel.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: [PATCH v6 07/11] x86/smpboot: Disable parallel boot for AMD CPUs
On Tue, 2023-02-07 at 01:23 +0100, Thomas Gleixner wrote:
> On Sat, Feb 04 2023 at 15:40, David Woodhouse wrote:
> > On Fri, 2023-02-03 at 13:48 -0600, Kim Phillips wrote:
> > > Then:
> > >
> > > - a Ryzen 3000 (Picasso A1/Zen+) notebook I have access to fails to boot.
> > > - Zen 2,3,4-based servers boot fine
> > > - a Zen1-based server doesn't boot.
> >
> > I've changed it to use CPUID 0xb only if we're actually in x2apic mode,
> > which Boris tells me won't be the case on Zen1 because that doesn't
> > support X2APIC.
>
> Correct.
>
> > When we're not in x2apic mode, we can use CPUID 0x1 because the 8 bits
> > of APIC ID we find there are perfectly sufficient.
>
> Is that worth the trouble?
Well, that's what was being debated. I think the conclusion that was
bring reached was that it *is* worth the trouble, because there will be
a number of physical and especially virtual machines which have a high
CPU count but which don't actually use X2APIC mode. And which might not
even *support* CPUID 0xb.
So using CPUID 0x1 when there is no APIC ID greater than 254 does seem
to make sense.
> > 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.
>
> The parallel bringup code is complex enough already, so please don't
> optimize for the non-interesting case in the first place. When this has
> stabilized then the CPUID 0x1 mechanism can be added if anyone thinks
> it's interesting. KISS is still the best engineering principle.
Actually it ends up being trivial. It probably makes sense to keep it
in there even if it can only be exercised by a deliberate opt-in on
older CPUs. I reworked the register usage from your original anyway,
which helps a little.
testl $STARTUP_APICID_CPUID_0B, %edx
jnz .Luse_cpuid_0b
testl $STARTUP_APICID_CPUID_01, %edx
jnz .Luse_cpuid_01
andl $0x0FFFFFFF, %edx
jmp .Lsetup_AP
.Luse_cpuid_01:
mov $0x01, %eax
cpuid
mov %ebx, %edx
shr $24, %edx
jmp .Lsetup_AP
.Luse_cpuid_0b:
mov $0x0B, %eax
xorl %ecx, %ecx
cpuid
.Lsetup_AP:
/* EDX contains the APICID of the current CPU */
Download attachment "smime.p7s" of type "application/pkcs7-signature" (5965 bytes)
Powered by blists - more mailing lists