[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5df3cca321782fecdfcfac9f4139da5a7fdf3277.camel@intel.com>
Date: Fri, 23 Jun 2023 15:36:57 +0000
From: "Zhang, Rui" <rui.zhang@...el.com>
To: "tglx@...utronix.de" <tglx@...utronix.de>,
"peterz@...radead.org" <peterz@...radead.org>,
"Tang, Feng" <feng.tang@...el.com>
CC: "Chen, Tim C" <tim.c.chen@...el.com>,
"bp@...en8.de" <bp@...en8.de>, "x86@...nel.org" <x86@...nel.org>,
"hpa@...or.com" <hpa@...or.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"mingo@...hat.com" <mingo@...hat.com>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"paulmck@...nel.org" <paulmck@...nel.org>,
"Woodhouse, David" <dwmw@...zon.co.uk>
Subject: Re: [Patch v2 2/2] x86/tsc: use logical_packages as a better
estimation of socket numbers
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
Powered by blists - more mailing lists