Hi Jeremy, I have tested the patch with the newest UEFI. It prints the below error:
[ 4.017371] BUG: arch topology borken [ 4.021069] BUG: arch topology borken [ 4.024764] BUG: arch topology borken [ 4.028460] BUG: arch topology borken [ 4.032153] BUG: arch topology borken [ 4.035849] BUG: arch topology borken [ 4.039543] BUG: arch topology borken [ 4.043239] BUG: arch topology borken [ 4.046932] BUG: arch topology borken [ 4.050629] BUG: arch topology borken [ 4.054322] BUG: arch topology borken I checked the code and found that the newest UEFI set PPTT physical_package_flag on a physical package node and the NUMA domain (SRAT domains) starts from the layer of DIE. (The topology of our board is core->cluster->die->package). When the kernel starts to build sched_domain, the multi-core sched_domain contains all the cores within a package, and the lowest NUMA sched_domain contains all the cores within a die. But the kernel requires that the multi-core sched_domain should be a subset of the lowest NUMA sched_domain, so the BUG info is printed. If we modify the UEFI to make NUMA sched_domain start from the layer of package, then all the topology information within the package will be discarded. I think we need to build the multi-core sched_domain using the cores within the cluster instead of the cores within the package. I think that's what 'multi-core' means. Multi cores form a cluster. I guess. If we build the multi-core sched_domain using the cores within a cluster, I think we need to add fields in struct cpu_topology to record which cores are in each cluster. Thanks, Xiongfeng On 2018/1/13 8:59, Jeremy Linton wrote: > Propagate the topology information from the PPTT tree to the > cpu_topology array. We can get the thread id, core_id and > cluster_id by assuming certain levels of the PPTT tree correspond > to those concepts. The package_id is flagged in the tree and can be > found by calling find_acpi_cpu_topology_package() which terminates > its search when it finds an ACPI node flagged as the physical > package. If the tree doesn't contain enough levels to represent > all of the requested levels then the root node will be returned > for all subsequent levels. > > Cc: Juri Lelli <[email protected]> > Signed-off-by: Jeremy Linton <[email protected]> > --- > arch/arm64/kernel/topology.c | 46 > +++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 45 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c > index 7b06e263fdd1..ce8ec7fd6b32 100644 > --- a/arch/arm64/kernel/topology.c > +++ b/arch/arm64/kernel/topology.c > @@ -11,6 +11,7 @@ > * for more details. > */ > > +#include <linux/acpi.h> > #include <linux/arch_topology.h> > #include <linux/cpu.h> > #include <linux/cpumask.h> > @@ -22,6 +23,7 @@ > #include <linux/sched.h> > #include <linux/sched/topology.h> > #include <linux/slab.h> > +#include <linux/smp.h> > #include <linux/string.h> > > #include <asm/cpu.h> > @@ -300,6 +302,46 @@ static void __init reset_cpu_topology(void) > } > } > > +#ifdef CONFIG_ACPI > +/* > + * Propagate the topology information of the processor_topology_node tree to > the > + * cpu_topology array. > + */ > +static int __init parse_acpi_topology(void) > +{ > + bool is_threaded; > + int cpu, topology_id; > + > + is_threaded = read_cpuid_mpidr() & MPIDR_MT_BITMASK; > + > + for_each_possible_cpu(cpu) { > + topology_id = find_acpi_cpu_topology(cpu, 0); > + if (topology_id < 0) > + return topology_id; > + > + if (is_threaded) { > + cpu_topology[cpu].thread_id = topology_id; > + topology_id = find_acpi_cpu_topology(cpu, 1); > + cpu_topology[cpu].core_id = topology_id; > + topology_id = find_acpi_cpu_topology_package(cpu); > + cpu_topology[cpu].package_id = topology_id; > + } else { > + cpu_topology[cpu].thread_id = -1; > + cpu_topology[cpu].core_id = topology_id; > + topology_id = find_acpi_cpu_topology_package(cpu); > + cpu_topology[cpu].package_id = topology_id; > + } > + } > + > + return 0; > +} > + > +#else > +static inline int __init parse_acpi_topology(void) > +{ > + return -EINVAL; > +} > +#endif > > void __init init_cpu_topology(void) > { > @@ -309,6 +351,8 @@ void __init init_cpu_topology(void) > * Discard anything that was parsed if we hit an error so we > * don't use partial information. > */ > - if (of_have_populated_dt() && parse_dt_topology()) > + if ((!acpi_disabled) && parse_acpi_topology()) > + reset_cpu_topology(); > + else if (of_have_populated_dt() && parse_dt_topology()) > reset_cpu_topology(); > } >
