| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 19 Oct 2017 16:56:22 +0100
From: Lorenzo Pieralisi <lorenzo.pieralisi@....com>
To: Jeremy Linton <jeremy.linton@....com>
Cc: linux-acpi@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
sudeep.holla@....com, hanjun.guo@...aro.org, rjw@...ysocki.net,
will.deacon@....com, catalin.marinas@....com,
gregkh@...uxfoundation.org, viresh.kumar@...aro.org,
mark.rutland@....com, linux-kernel@...r.kernel.org,
linux-pm@...r.kernel.org, jhugo@...eaurora.org,
wangxiongfeng2@...wei.com, Jonathan.Zhang@...ium.com,
ahs3@...hat.com, Jayachandran.Nair@...ium.com,
austinwc@...eaurora.org
Subject: Re: [PATCH v3 6/7] arm64: topology: Enable ACPI/PPTT based CPU
topology.
On Thu, Oct 12, 2017 at 02:48:55PM -0500, 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 passing an arbitrary large level to setup_acpi_cpu_topology()
> 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 thread/core/cod/package then the package
> id will be used for the missing levels.
>
> Since server/ACPI machines are more likely to be multisocket and NUMA,
I think this stuff is vague enough already so to start with I would drop
patch 4 and 5 and stop assuming what machines are more likely to ship
with ACPI than DT.
I am just saying, for the umpteenth time, that these levels have no
architectural meaning _whatsoever_, level is a hierarchy concept
with no architectural meaning attached.
The only consistent thing PPTT is bringing about is the hierarchy
levels/grouping (and _possibly_ - what a package boundary is), let's
stick to that for the time being.
> this patch also modifies the default clusters=sockets behavior
> for ACPI machines to sockets=sockets. DT machines continue to
> represent sockets as clusters. For ACPI machines, this results in a
> more normalized view of the topology. Cluster level scheduler decisions
> are still being made due to the "MC" level in the scheduler which has
> knowledge of cache sharing domains.
>
> This code is loosely based on a combination of code from:
> Xiongfeng Wang <wangxiongfeng2@...wei.com>
> John Garry <john.garry@...wei.com>
> Jeffrey Hugo <jhugo@...eaurora.org>
>
> Signed-off-by: Jeremy Linton <jeremy.linton@....com>
> ---
> arch/arm64/kernel/topology.c | 54 +++++++++++++++++++++++++++++++++++++++++++-
> include/linux/topology.h | 1 +
> 2 files changed, 54 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index 9147e5b6326d..42f3e7f28b2b 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>
> @@ -304,6 +306,54 @@ 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)
> +{
> + u64 is_threaded;
> + int cpu;
> + int topology_id;
> + /* set a large depth, to hit ACPI_PPTT_PHYSICAL_PACKAGE if one exists */
> + const int max_topo = 0xFF;
> +
> + is_threaded = read_cpuid_mpidr() & MPIDR_MT_BITMASK;
> +
> + for_each_possible_cpu(cpu) {
> + topology_id = setup_acpi_cpu_topology(cpu, 0);
> + if (topology_id < 0)
> + return topology_id;
> +
> + if (is_threaded) {
> + cpu_topology[cpu].thread_id = topology_id;
> + topology_id = setup_acpi_cpu_topology(cpu, 1);
Nit: you can move setup_acpi_cpu_topology() to include/linux/acpi.h,
provide an empty inline function for the !ACPI case and remove
this function ACPI ifdeffery.
> + cpu_topology[cpu].core_id = topology_id;
> + topology_id = setup_acpi_cpu_topology(cpu, 2);
> + cpu_topology[cpu].cluster_id = topology_id;
> + topology_id = setup_acpi_cpu_topology(cpu, max_topo);
If you want a package id (that's just a package tag to group cores), you
should not use a large level because you know how setup_acpi_cpu_topology()works, you should add an API that allows you to retrieve the package id
(so that you can use th ACPI_PPTT_PHYSICAL_PACKAGE flag consistenly,
whatever it represents).
Lorenzo
> + cpu_topology[cpu].package_id = topology_id;
> + } else {
> + cpu_topology[cpu].thread_id = -1;
> + cpu_topology[cpu].core_id = topology_id;
> + topology_id = setup_acpi_cpu_topology(cpu, 1);
> + cpu_topology[cpu].cluster_id = topology_id;
> + topology_id = setup_acpi_cpu_topology(cpu, max_topo);
> + cpu_topology[cpu].package_id = topology_id;
> + }
> + }
> + return 0;
> +}
> +
> +#else
> +static int __init parse_acpi_topology(void)
> +{
> + /*ACPI kernels should be built with PPTT support*/
> + return -EINVAL;
> +}
> +#endif
> +
> void __init init_cpu_topology(void)
> {
> reset_cpu_topology();
> @@ -312,6 +362,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();
> }
> diff --git a/include/linux/topology.h b/include/linux/topology.h
> index 4660749a7303..cbf2fb13bf92 100644
> --- a/include/linux/topology.h
> +++ b/include/linux/topology.h
> @@ -43,6 +43,7 @@
> if (nr_cpus_node(node))
>
> int arch_update_cpu_topology(void);
> +int setup_acpi_cpu_topology(unsigned int cpu, int level);
>
> /* Conform to ACPI 2.0 SLIT distance definitions */
> #define LOCAL_DISTANCE 10
> --
> 2.13.5
>
Powered by blists - more mailing lists