| Message ID | 20241220213711.1892696-3-sohil.mehta@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Prepare for new Intel family models | expand |
On 12/20/2024 3:20 PM, Dave Hansen wrote: > On 12/20/24 13:36, Sohil Mehta wrote: >> The MP specification version 1.4 references the i486 and early Pentium >> processors in family 5. > > Can you please elaborate on how this reference is relevant to the patch > at hand? > The comment above UDELAY_10MS_DEFAULT which references the MP specification seems to be the basis of all the cpu_init_delay quirks. I wanted to use that reference to justify that family 15 doesn't need the 10 msec delay since it is not explicitly mentioned there. I realize now that it's moot since the Pentium 4 wasn't released until 3 years after it's publication. I referred the latest MP initialization specification but that doesn't say explicitly when the delay is needed vs not needed. However it does say that later Family 6 models and Family 15 models have similar initialization protocols. "The selection of the BSP and APs is handled through a special system bus cycle, without using BIPI and FIPI message arbitration (see Section 9.4.3, “MP Initialization Protocol Algorithm for MP Systems”). These processor generations have CPUID signatures of family=0FH with (model=0H, stepping>=0AH) or (model >0, all steppings); or family=06H, extended_model=0, model>=0EH." >> However, all processors starting with family 6 likely do not need the >> 10 msec INIT delay. The omission of the Pentium 4s (family 15) seems >> like an oversight in the original check. >> >> With some risk, choose a simpler check and extend the quirk to all >> recent and upcoming Intel processors. > > I'm struggling to follow this a bit. > Because my interpretation of the code and the related comments is opposite to yours. The usage of "quirk" in this context seems to be inverted due to how this check has evolved over time. > I think these are the facts that matter: > The code says this: /* if modern processor, use no delay */ init_udelay = 0; /* else, use legacy delay */ init_udelay = UDELAY_10MS_DEFAULT; The legacy/default delay is 10 msec and then the quirk applies to the modern processors to remove the delay. This is the comment above UDELAY_10MS_DEFAULT: * Modern processor families are quirked to remove the delay entirely. > * init_udelay=0 means "no quirk" > * Modern CPUs don't have the quirk > * The current check says "only family 6 is modern" > * Family 15 is _probably_ modern and just forgotten about > > And this is what you're doing in the end: > > Consider everything PPro and later to be modern, including all of > families 6, 15 and the new 18/19 CPUs. > That's right. We consider these CPUs to be modern and do not apply the 10 msec delay but the usage of "quirk" is confusing here. I'll clarify the changelog without using "quirk" to avoid confusion. Am I interpreting this wrong?
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c index b5a8f0891135..6c98e9178963 100644 --- a/arch/x86/kernel/smpboot.c +++ b/arch/x86/kernel/smpboot.c @@ -668,7 +668,7 @@ static void impress_friends(void) * But that slows boot and resume on modern processors, which include * many cores and don't require that delay. * - * Cmdline "init_cpu_udelay=" is available to over-ride this delay. + * Cmdline "cpu_init_udelay=" is available to override this delay. * Modern processor families are quirked to remove the delay entirely. */ #define UDELAY_10MS_DEFAULT 10000 @@ -690,9 +690,9 @@ static void __init smp_quirk_init_udelay(void) return; /* if modern processor, use no delay */ - if (((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) && (boot_cpu_data.x86 == 6)) || - ((boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) && (boot_cpu_data.x86 >= 0x18)) || - ((boot_cpu_data.x86_vendor == X86_VENDOR_AMD) && (boot_cpu_data.x86 >= 0xF))) { + if ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL && boot_cpu_data.x86_vfm >= INTEL_PENTIUM_PRO) || + (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON && boot_cpu_data.x86 >= 0x18) || + (boot_cpu_data.x86_vendor == X86_VENDOR_AMD && boot_cpu_data.x86 >= 0xF)) { init_udelay = 0; return; }
The MP specification version 1.4 references the i486 and early Pentium processors in family 5. However, all processors starting with family 6 likely do not need the 10 msec INIT delay. The omission of the Pentium 4s (family 15) seems like an oversight in the original check. With some risk, choose a simpler check and extend the quirk to all recent and upcoming Intel processors. While at it, fix the command line parameter comment to match with the actual name. Signed-off-by: Sohil Mehta <sohil.mehta@intel.com> --- arch/x86/kernel/smpboot.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)