From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Google-Smtp-Source: AH8x226ZKs88c3RYV+opf2ONvp/IFMl6ghWUSjRV97PpKs1JOTBJsZ7O4Z/3j3wsJejMW2X5Sj8g ARC-Seal: i=1; a=rsa-sha256; t=1516882545; cv=none; d=google.com; s=arc-20160816; b=REgzJpyC9faxDZ4uvT3xcsrAt6aEAvP+aVjBli4gXF40oN3agXpPOIQk6YG1wH8UoZ tGLL700NmJgTGujU+4i+utom/7KqgZGt3EY6zS3bqVINQXjkQjMTb+zj9jZhrlwAM2Mw PGOpNIvNRkjbeeTCukfVg0x6hTHNVw1UjDLJXZcsTTSmLDqnvLbvEYApWrg1lbEq7q9c gevF07RWlsJTDko1WG4siuUqy/Mf2FJ2J2V6f0jSHyr5qHdGGDAEWK+knHua661zLXJ2 U+zno4foxlrLs5UK0UNqBjKWz+tHOby+B4sc4mqUlWDEBRvjc7y5TfDWj35KybI45Wa6 NRxA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:in-reply-to:mime-version:user-agent:date :message-id:from:cc:references:to:subject:arc-authentication-results; bh=w9GO8oInx9Eg59s2ToOUB8x/lirq/mXn18EnnQUOjmg=; b=Shc25+PmP7sbc22rlUkpSESHTPvv1AK0vL7VzGpK92aJVuWHOCymGhNY2UMQ/6Gmd6 72mMut7SDREz4U4tySRrBQu/UpqjPtU+nGepBnjDfuo4LWyn1LROhQZBRRTZBvkKIxpQ 05MkGctLv58Xa04r8TmIcBTDgjysti8Dj8mjtVK05vE37on5MrLuR6dRFN7Tgfb2TWB6 sKvOaui65Qvts+KO3b150qtasnDrBDI7dGG3DIyVGYaduYQNpGD2jK6GccYeRbZehRp0 /yeuMinAlEbTp+w7ctOO/wuTUTx/r8YDcBDFPjRVz5zQEKbW7+X8Pws5NaKu+L1RG+1n O6gw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of wangxiongfeng2@huawei.com designates 45.249.212.32 as permitted sender) smtp.mailfrom=wangxiongfeng2@huawei.com Authentication-Results: mx.google.com; spf=pass (google.com: domain of wangxiongfeng2@huawei.com designates 45.249.212.32 as permitted sender) smtp.mailfrom=wangxiongfeng2@huawei.com Subject: Re: [PATCH v6 11/12] arm64: topology: enable ACPI/PPTT based CPU topology To: Jeremy Linton , References: <20180113005920.28658-1-jeremy.linton@arm.com> <20180113005920.28658-12-jeremy.linton@arm.com> CC: , , , , , , , , , , , , , , , , , , , , Juri Lelli From: Xiongfeng Wang Message-ID: <928cb0c9-1d7a-3d0c-0538-a8bb0e2a86b1@huawei.com> Date: Thu, 25 Jan 2018 20:15:21 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20180113005920.28658-12-jeremy.linton@arm.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.177.32.209] X-CFilter-Loop: Reflected X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1589437013330936926?= X-GMAIL-MSGID: =?utf-8?q?1590566632189753013?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: 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 > Signed-off-by: Jeremy Linton > --- > 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 > #include > #include > #include > @@ -22,6 +23,7 @@ > #include > #include > #include > +#include > #include > > #include > @@ -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(); > } >