| 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, 7 Jul 2022 10:12:48 +0000
From: <Conor.Dooley@...rochip.com>
To: <sudeep.holla@....com>, <atishp@...shpatra.org>
CC: <paul.walmsley@...ive.com>, <palmer@...belt.com>,
<palmer@...osinc.com>, <aou@...s.berkeley.edu>,
<Daire.McNamara@...rochip.com>, <Conor.Dooley@...rochip.com>,
<niklas.cassel@....com>, <damien.lemoal@...nsource.wdc.com>,
<geert@...ux-m68k.org>, <zong.li@...ive.com>, <kernel@...il.dk>,
<hahnjo@...njo.de>, <guoren@...nel.org>, <anup@...infault.org>,
<changbin.du@...el.com>, <heiko@...ech.de>,
<philipp.tomsich@...ll.eu>, <linux-riscv@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>, <Brice.Goglin@...ia.fr>
Subject: Re: [PATCH] riscv: arch-topology: fix default topology reporting
On 07/07/2022 10:47, Sudeep Holla wrote:
> On Wed, Jul 06, 2022 at 02:38:01PM -0700, Atish Patra wrote:
>> On Wed, Jul 6, 2022 at 11:46 AM Conor Dooley <mail@...chuod.ie> wrote:
>>>
>>> From: Conor Dooley <conor.dooley@...rochip.com>
>>>
>>> RISC-V has no sane defaults to fall back on where there is no cpu-map
>>> in the devicetree.
>>> Without sane defaults, the package, core and thread IDs are all set to
>>> -1. This causes user-visible inaccuracies for tools like hwloc/lstopo
>>> which rely on the sysfs cpu topology files to detect a system's
>>> topology.
>>>
>>> Add sane defaults in ~the exact same way as ARM64.
>>>
>>> CC: stable@...r.kernel.org
>>> Fixes: 03f11f03dbfe ("RISC-V: Parse cpu topology during boot.")
>>> Reported-by: Brice Goglin <Brice.Goglin@...ia.fr>
>>> Link: https://github.com/open-mpi/hwloc/issues/536
>>> Signed-off-by: Conor Dooley <conor.dooley@...rochip.com>
>>> ---
>>>
>>> Sudeep suggested that this be backported rather than the changes to
>>> the devicetrees adding cpu-map since that property is optional.
>>> That patchset is still valid in it's own right.
>>>
>>> arch/riscv/include/asm/topology.h | 13 +++++++++++++
>>> arch/riscv/kernel/Makefile | 1 +
>>> arch/riscv/kernel/smpboot.c | 4 ++++
>>> arch/riscv/kernel/topology.c | 32 +++++++++++++++++++++++++++++++
>>> 4 files changed, 50 insertions(+)
>>> create mode 100644 arch/riscv/include/asm/topology.h
>>> create mode 100644 arch/riscv/kernel/topology.c
>>>
>>> diff --git a/arch/riscv/include/asm/topology.h b/arch/riscv/include/asm/topology.h
>>> new file mode 100644
>>> index 000000000000..36bc6ecda898
>>> --- /dev/null
>>> +++ b/arch/riscv/include/asm/topology.h
>>> @@ -0,0 +1,13 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +/*
>>> + * Copyright (c) 2022 Microchip Technology Inc. and its subsidiaries
>>> + */
>>> +
>>> +#ifndef _ASM_RISCV_TOPOLOGY_H
>>> +#define _ASM_RISCV_TOPOLOGY_H
>>> +
>>> +#include <asm-generic/topology.h>
>>> +
>>> +void store_cpu_topology(unsigned int cpuid);
>>> +
>>> +#endif /* _ASM_RISCV_TOPOLOGY_H */
>>> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
>>> index c71d6591d539..9518882ba6f9 100644
>>> --- a/arch/riscv/kernel/Makefile
>>> +++ b/arch/riscv/kernel/Makefile
>>> @@ -50,6 +50,7 @@ obj-y += riscv_ksyms.o
>>> obj-y += stacktrace.o
>>> obj-y += cacheinfo.o
>>> obj-y += patch.o
>>> +obj-y += topology.o
>>> obj-y += probes/
>>> obj-$(CONFIG_MMU) += vdso.o vdso/
>>>
>>> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
>>> index f1e4948a4b52..d551c7f452d4 100644
>>> --- a/arch/riscv/kernel/smpboot.c
>>> +++ b/arch/riscv/kernel/smpboot.c
>>> @@ -32,6 +32,7 @@
>>> #include <asm/sections.h>
>>> #include <asm/sbi.h>
>>> #include <asm/smp.h>
>>> +#include <asm/topology.h>
>>>
>>> #include "head.h"
>>>
>>> @@ -40,6 +41,8 @@ static DECLARE_COMPLETION(cpu_running);
>>> void __init smp_prepare_boot_cpu(void)
>>> {
>>> init_cpu_topology();
>>> +
>>> + store_cpu_topology(smp_processor_id());
>>> }
>>>
>>> void __init smp_prepare_cpus(unsigned int max_cpus)
>>> @@ -161,6 +164,7 @@ asmlinkage __visible void smp_callin(void)
>>> mmgrab(mm);
>>> current->active_mm = mm;
>>>
>>> + store_cpu_topology(curr_cpuid);
>>> notify_cpu_starting(curr_cpuid);
>>> numa_add_cpu(curr_cpuid);
>>> update_siblings_masks(curr_cpuid);
>>> diff --git a/arch/riscv/kernel/topology.c b/arch/riscv/kernel/topology.c
>>> new file mode 100644
>>> index 000000000000..db72862bd5b5
>>> --- /dev/null
>>> +++ b/arch/riscv/kernel/topology.c
>>> @@ -0,0 +1,32 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * Copyright (c) 2022 Microchip Technology Inc. and its subsidiaries
>>> + *
>>> + * Based on the arm64 version, which was in turn based on arm32, which was
>>> + * ultimately based on sh's.
>>> + * The arm64 version was listed as:
>>> + * Copyright (C) 2011,2013,2014 Linaro Limited.
>>> + */
>>> +
>>> +#include <linux/arch_topology.h>
>>> +#include <linux/topology.h>
>>> +#include <asm/topology.h>
>>> +
>>> +void store_cpu_topology(unsigned int cpuid)
>>> +{
>>> + struct cpu_topology *cpuid_topo = &cpu_topology[cpuid];
>>> +
>>> + if (cpuid_topo->package_id != -1)
>>> + goto topology_populated;
>>> +
>>> + cpuid_topo->thread_id = -1;
>>> + cpuid_topo->core_id = cpuid;
>>> + cpuid_topo->package_id = cpu_to_node(cpuid);
>>> +
>>> + pr_debug("CPU%u: package %d core %d thread %d\n",
>>> + cpuid, cpuid_topo->package_id, cpuid_topo->core_id,
>>> + cpuid_topo->thread_id);
>>> +
>>> +topology_populated:
>>> + update_siblings_masks(cpuid);
>>> +}
>>>
>>
>> This function is pretty much the same as the arm64 one except the
>> UP/mpidr check.
>> Can we move this to the common code as well ?
>>
>
> While I completely agree with the idea, not sure if that makes backports
> (if required) any difficult. If so, I would rather keep this way for a
> release and then move both to the common place in arch_topology.
Yeah, that seems like a good idea. I'll let this patch just touch
RISC-V for the sake of backporting & create a second patch to move
to a common implementation.
Since I've not modified any real arch code before, I'd rather make
that a separate patch/series too for the sake of getting this
patch applied as a v5.19-rc(late) fix.
Powered by blists - more mailing lists