Re: [PATCH 0/5] RISC-V: Add cpu-map topology information nodes

[Conor.Dooley]

On 05/07/2022 21:33, Conor.Dooley@xxxxxxxxxxxxx wrote:
> 
> 
> On 05/07/2022 21:19, Sudeep Holla wrote:
>> On Tue, Jul 05, 2022 at 08:04:31PM +0100, Conor Dooley wrote:
>>> From: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
>>>
>>> It was reported to me that the Hive Unmatched incorrectly reports
>>> its topology to hwloc, but the StarFive VisionFive did in [0] &
>>> a subsequent off-list email from Brice (the hwloc maintainer).
>>> This turned out not to be entirely true, the /downstream/ version
>>> of the VisionFive does work correctly but not upstream, as the
>>> downstream devicetree has a cpu-map node that was added recently.
>>>
>>> This series adds a cpu-map node to all upstream devicetrees, which
>>> I have tested on mpfs & fu540. The first patch is lifted directly
>>> from the downstream StarFive devicetree.
>>>
>>
>> Reviewed-by: Sudeep Holla <sudeep.holla@xxxxxxx>
>>
>> I would recommend to have sane defaults in core risc-v code in case of
>> absence of /cpu-map node as it is optional. The reason I mentioned is that
>> Conor mentioned how the default values in absence of the node looked quite
>> wrong. I don't know if it is possible on RISC-V but on ARM64 we do have
>> default values if arch_topology fails to set based on DT/ACPI.
>>
> 
> Yeah the defaults are all -1. I'll add some sane defaults for a v2.
> Thanks,
> Conor.

I shamelessly stole from arm64... Seems to work, but have done minimal
testing (only PolarFire SoC).

Author: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
Date:   Wed Jul 6 00:00:34 2022 +0100

    riscv: arch-topology: add sane defaults
    
    RISC-V has no sane defaults to fall back on where there is no cpu-map
    in the devicetree. Add sane defaults in ~the exact same way as ARM64.
    
    Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>

diff --git a/arch/riscv/include/asm/topology.h b/arch/riscv/include/asm/topology.h
new file mode 100644
index 000000000000..71c80710f00e
--- /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 */
\ No newline at end of file
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..799b3423e0bc
--- /dev/null
+++ b/arch/riscv/kernel/topology.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * 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_info("CPU%u: cluster %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);
+}