Messages in this thread | | | From | "Zhang, Rui" <> | Subject | Re: [Patch v2 2/2] x86/tsc: use logical_packages as a better estimation of socket numbers | Date | Fri, 23 Jun 2023 15:36:57 +0000 |
| |
Hi, Thomas,
On Thu, 2023-06-22 at 16:27 +0200, Thomas Gleixner wrote: > On Fri, Jun 16 2023 at 15:18, Feng Tang wrote: > > On Thu, Jun 15, 2023 at 11:20:21AM +0200, Peter Zijlstra wrote: > > Yes. Rui is working on a MADT based parsing which may take a while > > before being stable, given all kinds of fancy firmware out there. > > Please not yet another mad table parser. > > The topology can be evaluated during early boot via: > > 1) The APIC IDs of the possible CPUs. > > 2) CPUID leaf 0xb or 0x1f where the topmost subleaf gives the > number > of bits to shift the APIC ID right for the package/socket > exactly the same in my proposal.
The difference is that I also 1. get the bitshift of the core id and count the number of cores in a package. On Intel hybrid platforms, using nr_package_cpus / nr_core_cpus to get the number of cores in a package (x86_max_cores) is broken. e.g. for a 6Pcore + 8Ecore system, package has 20 CPUs and 14 cores. 2. get the maximum number of SMT siblings in each core to set correct smp_num_siblings. On future hybrid platforms, it is possible that Ecore is used as boot CPU. This could result in smp_num_siblings set to 1 during boot cpu startup, and cpu_smt_control set to CPU_SMT_NOT_SUPPORTED.
> Trying to accomodate for anything else than the documented > enumeration > is crazy. If fancy firmware is broken then they can keep the pieces. > > So something like the below should just work. > > I fundamentally hate the hackery in topology.c, but cleaning this > mess > up is a completely different problem and already worked on. > > Thanks, > > tglx > --- > --- a/arch/x86/include/asm/apic.h > +++ b/arch/x86/include/asm/apic.h > @@ -509,9 +509,12 @@ extern int default_check_phys_apicid_pre > #ifdef CONFIG_SMP > bool apic_id_is_primary_thread(unsigned int id); > void apic_smt_update(void); > +extern unsigned int apic_to_pkg_shift; > +bool logical_packages_update(u32 apicid); > #else > static inline bool apic_id_is_primary_thread(unsigned int id) { > return false; } > static inline void apic_smt_update(void) { } > +static inline bool logical_packages_update(u32 apicid) { return > true; } > #endif > > struct msi_msg; > --- a/arch/x86/kernel/acpi/boot.c > +++ b/arch/x86/kernel/acpi/boot.c > @@ -177,6 +177,9 @@ static int acpi_register_lapic(int id, u > return -EINVAL; > } > > + if (!logical_packages_update(acpiid)) > + return -EINVAL; > + > if (!enabled) { > ++disabled_cpus; > return -EINVAL; > --- a/arch/x86/kernel/cpu/amd.c > +++ b/arch/x86/kernel/cpu/amd.c > @@ -692,6 +692,8 @@ static void early_init_amd(struct cpuinf > } > } > > + detect_extended_topology_early(c); > + > if (cpu_has(c, X86_FEATURE_TOPOEXT)) > smp_num_siblings = ((cpuid_ebx(0x8000001e) >> 8) & > 0xff) + 1; > } > --- a/arch/x86/kernel/cpu/topology.c > +++ b/arch/x86/kernel/cpu/topology.c > @@ -29,6 +29,8 @@ unsigned int __max_die_per_package __rea > EXPORT_SYMBOL(__max_die_per_package); > > #ifdef CONFIG_SMP > +unsigned int apic_to_pkg_shift __ro_after_init; > + > /* > * Check if given CPUID extended topology "leaf" is implemented > */ > @@ -66,7 +68,7 @@ int detect_extended_topology_early(struc > { > #ifdef CONFIG_SMP > unsigned int eax, ebx, ecx, edx; > - int leaf; > + int leaf, subleaf; > > leaf = detect_extended_topology_leaf(c); > if (leaf < 0) > @@ -80,6 +82,14 @@ int detect_extended_topology_early(struc > */ > c->initial_apicid = edx; > smp_num_siblings = max_t(int, smp_num_siblings, > LEVEL_MAX_SIBLINGS(ebx)); > + > + for (subleaf = 1; subleaf < 8; subleaf++) { > + cpuid_count(leaf, subleaf, &eax, &ebx, &ecx, &edx); > + > + if (ebx == 0 || !LEAFB_SUBTYPE(ecx))
Do we ever see ebx == 0 for a valid subtype? When decoding CPUID.0B/1F, we check for invalid subtype only.
thanks, rui
| |