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

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

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

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.

Thanks,
Conor.

0: https://github.com/open-mpi/hwloc/issues/536

Conor Dooley (4):
  riscv: dts: sifive: Add fu540 topology information
  riscv: dts: sifive: Add fu740 topology information
  riscv: dts: microchip: Add mpfs' topology information
  riscv: dts: canaan: Add k210 topology information

Jonas Hahnfeld (1):
  riscv: dts: starfive: Add JH7100 CPU topology

 arch/riscv/boot/dts/canaan/k210.dtsi       | 12 +++++++++++
 arch/riscv/boot/dts/microchip/mpfs.dtsi    | 24 ++++++++++++++++++++++
 arch/riscv/boot/dts/sifive/fu540-c000.dtsi | 24 ++++++++++++++++++++++
 arch/riscv/boot/dts/sifive/fu740-c000.dtsi | 24 ++++++++++++++++++++++
 arch/riscv/boot/dts/starfive/jh7100.dtsi   | 16 +++++++++++++--
 5 files changed, 98 insertions(+), 2 deletions(-)


base-commit: b6f1f2fa2bddd69ff46a190b8120bd440fd50563
-- 
2.37.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 1/5] riscv: dts: starfive: Add JH7100 CPU topology

[Conor Dooley]

From: Jonas Hahnfeld <hahnjo@hahnjo.de>

Add cpu-map binding to inform the kernel about the hardware topology
of the CPU cores.

Before this change, lstopo would report 1 core with 2 threads:
Machine (7231MB total)
  Package L#0
    NUMANode L#0 (P#0 7231MB)
    L2 L#0 (2048KB) + Core L#0
      L1d L#0 (32KB) + L1i L#0 (32KB) + PU L#0 (P#0)
      L1d L#1 (32KB) + L1i L#1 (32KB) + PU L#1 (P#1)

After this change, it correctly identifies two cores:
Machine (7231MB total)
  Package L#0
    NUMANode L#0 (P#0 7231MB)
    L2 L#0 (2048KB)
      L1d L#0 (32KB) + L1i L#0 (32KB) + Core L#0 + PU L#0 (P#0)
      L1d L#1 (32KB) + L1i L#1 (32KB) + Core L#1 + PU L#1 (P#1)

Signed-off-by: Jonas Hahnfeld <hahnjo@hahnjo.de>
Co-developed-by: Emil Renner Berthing <kernel@esmil.dk>
Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/boot/dts/starfive/jh7100.dtsi | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
index 69f22f9aad9d..c617a61e26e2 100644
--- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
@@ -17,7 +17,7 @@ cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		cpu@0 {
+		U74_0: cpu@0 {
 			compatible = "sifive,u74-mc", "riscv";
 			reg = <0>;
 			d-cache-block-size = <64>;
@@ -42,7 +42,7 @@ cpu0_intc: interrupt-controller {
 			};
 		};
 
-		cpu@1 {
+		U74_1: cpu@1 {
 			compatible = "sifive,u74-mc", "riscv";
 			reg = <1>;
 			d-cache-block-size = <64>;
@@ -66,6 +66,18 @@ cpu1_intc: interrupt-controller {
 				#interrupt-cells = <1>;
 			};
 		};
+
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&U74_0>;
+				};
+
+				core1 {
+					cpu = <&U74_1>;
+				};
+			};
+		};
 	};
 
 	osc_sys: osc_sys {
-- 
2.37.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/5] riscv: dts: sifive: Add fu540 topology information

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

The fu540 has no cpu-map node, so tools like hwloc cannot correctly
parse the topology. Add the node using the existing node labels.

Reported-by: Brice Goglin <Brice.Goglin@inria.fr>
Link: https://github.com/open-mpi/hwloc/issues/536
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/boot/dts/sifive/fu540-c000.dtsi | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/riscv/boot/dts/sifive/fu540-c000.dtsi b/arch/riscv/boot/dts/sifive/fu540-c000.dtsi
index e3172d0ffac4..24bba83bec77 100644
--- a/arch/riscv/boot/dts/sifive/fu540-c000.dtsi
+++ b/arch/riscv/boot/dts/sifive/fu540-c000.dtsi
@@ -133,6 +133,30 @@ cpu4_intc: interrupt-controller {
 				interrupt-controller;
 			};
 		};
+
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&cpu0>;
+				};
+
+				core1 {
+					cpu = <&cpu1>;
+				};
+
+				core2 {
+					cpu = <&cpu2>;
+				};
+
+				core3 {
+					cpu = <&cpu3>;
+				};
+
+				core4 {
+					cpu = <&cpu4>;
+				};
+			};
+		};
 	};
 	soc {
 		#address-cells = <2>;
-- 
2.37.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 3/5] riscv: dts: sifive: Add fu740 topology information

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

The fu740 has no cpu-map node, so tools like hwloc cannot correctly
parse the topology. Add the node using the existing node labels.

Reported-by: Brice Goglin <Brice.Goglin@inria.fr>
Link: https://github.com/open-mpi/hwloc/issues/536
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/boot/dts/sifive/fu740-c000.dtsi | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/riscv/boot/dts/sifive/fu740-c000.dtsi b/arch/riscv/boot/dts/sifive/fu740-c000.dtsi
index 7b77c13496d8..43bed6c0a84f 100644
--- a/arch/riscv/boot/dts/sifive/fu740-c000.dtsi
+++ b/arch/riscv/boot/dts/sifive/fu740-c000.dtsi
@@ -134,6 +134,30 @@ cpu4_intc: interrupt-controller {
 				interrupt-controller;
 			};
 		};
+
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&cpu0>;
+				};
+
+				core1 {
+					cpu = <&cpu1>;
+				};
+
+				core2 {
+					cpu = <&cpu2>;
+				};
+
+				core3 {
+					cpu = <&cpu3>;
+				};
+
+				core4 {
+					cpu = <&cpu4>;
+				};
+			};
+		};
 	};
 	soc {
 		#address-cells = <2>;
-- 
2.37.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 4/5] riscv: dts: microchip: Add mpfs' topology information

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

The mpfs has no cpu-map node, so tools like hwloc cannot correctly
parse the topology. Add the node using the existing node labels.

Reported-by: Brice Goglin <Brice.Goglin@inria.fr>
Link: https://github.com/open-mpi/hwloc/issues/536
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/boot/dts/microchip/mpfs.dtsi | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/riscv/boot/dts/microchip/mpfs.dtsi b/arch/riscv/boot/dts/microchip/mpfs.dtsi
index 45efd35d50c5..0a17d30bb3f2 100644
--- a/arch/riscv/boot/dts/microchip/mpfs.dtsi
+++ b/arch/riscv/boot/dts/microchip/mpfs.dtsi
@@ -138,6 +138,30 @@ cpu4_intc: interrupt-controller {
 				interrupt-controller;
 			};
 		};
+
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&cpu0>;
+				};
+
+				core1 {
+					cpu = <&cpu1>;
+				};
+
+				core2 {
+					cpu = <&cpu2>;
+				};
+
+				core3 {
+					cpu = <&cpu3>;
+				};
+
+				core4 {
+					cpu = <&cpu4>;
+				};
+			};
+		};
 	};
 
 	refclk: mssrefclk {
-- 
2.37.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 4/5] riscv: dts: microchip: Add mpfs' topology information

[Palmer Dabbelt]

On Tue, 05 Jul 2022 12:04:35 PDT (-0700), mail@conchuod.ie wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> The mpfs has no cpu-map node, so tools like hwloc cannot correctly
> parse the topology. Add the node using the existing node labels.
>
> Reported-by: Brice Goglin <Brice.Goglin@inria.fr>
> Link: https://github.com/open-mpi/hwloc/issues/536
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  arch/riscv/boot/dts/microchip/mpfs.dtsi | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/microchip/mpfs.dtsi b/arch/riscv/boot/dts/microchip/mpfs.dtsi
> index 45efd35d50c5..0a17d30bb3f2 100644
> --- a/arch/riscv/boot/dts/microchip/mpfs.dtsi
> +++ b/arch/riscv/boot/dts/microchip/mpfs.dtsi
> @@ -138,6 +138,30 @@ cpu4_intc: interrupt-controller {
>  				interrupt-controller;
>  			};
>  		};
> +
> +		cpu-map {
> +			cluster0 {
> +				core0 {
> +					cpu = <&cpu0>;
> +				};
> +
> +				core1 {
> +					cpu = <&cpu1>;
> +				};
> +
> +				core2 {
> +					cpu = <&cpu2>;
> +				};
> +
> +				core3 {
> +					cpu = <&cpu3>;
> +				};
> +
> +				core4 {
> +					cpu = <&cpu4>;
> +				};
> +			};
> +		};
>  	};
>
>  	refclk: mssrefclk {

In case anyone is following along: this patch got split out from the 
rest of the series and ended up in the Microchip DT PR for 5.20.  
I've taken the other four into for-next.

Thanks!

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 5/5] riscv: dts: canaan: Add k210 topology information

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

The k210 has no cpu-map node, so tools like hwloc cannot correctly
parse the topology. Add the node using the existing node labels.

Reported-by: Brice Goglin <Brice.Goglin@inria.fr>
Link: https://github.com/open-mpi/hwloc/issues/536
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/boot/dts/canaan/k210.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/riscv/boot/dts/canaan/k210.dtsi b/arch/riscv/boot/dts/canaan/k210.dtsi
index 44d338514761..ec944d1537dc 100644
--- a/arch/riscv/boot/dts/canaan/k210.dtsi
+++ b/arch/riscv/boot/dts/canaan/k210.dtsi
@@ -65,6 +65,18 @@ cpu1_intc: interrupt-controller {
 				compatible = "riscv,cpu-intc";
 			};
 		};
+
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&cpu0>;
+				};
+
+				core1 {
+					cpu = <&cpu1>;
+				};
+			};
+		};
 	};
 
 	sram: memory@80000000 {
-- 
2.37.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 5/5] riscv: dts: canaan: Add k210 topology information

[Damien Le Moal]

On 7/6/22 04:04, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> The k210 has no cpu-map node, so tools like hwloc cannot correctly
> parse the topology. Add the node using the existing node labels.
> 
> Reported-by: Brice Goglin <Brice.Goglin@inria.fr>
> Link: https://github.com/open-mpi/hwloc/issues/536
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Looks good to me.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>

> ---
>  arch/riscv/boot/dts/canaan/k210.dtsi | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/canaan/k210.dtsi b/arch/riscv/boot/dts/canaan/k210.dtsi
> index 44d338514761..ec944d1537dc 100644
> --- a/arch/riscv/boot/dts/canaan/k210.dtsi
> +++ b/arch/riscv/boot/dts/canaan/k210.dtsi
> @@ -65,6 +65,18 @@ cpu1_intc: interrupt-controller {
>  				compatible = "riscv,cpu-intc";
>  			};
>  		};
> +
> +		cpu-map {
> +			cluster0 {
> +				core0 {
> +					cpu = <&cpu0>;
> +				};
> +
> +				core1 {
> +					cpu = <&cpu1>;
> +				};
> +			};
> +		};
>  	};
>  
>  	sram: memory@80000000 {


-- 
Damien Le Moal
Western Digital Research

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Sudeep Holla]

On Tue, Jul 05, 2022 at 08:04:31PM +0100, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> 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@arm.com>

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.

-- 
Regards,
Sudeep

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Conor.Dooley]

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@microchip.com>
>>
>> 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@arm.com>
> 
> 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.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Conor.Dooley]

On 05/07/2022 21:33, Conor.Dooley@microchip.com 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@microchip.com>
>>>
>>> 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@arm.com>
>>
>> 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@microchip.com>
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@microchip.com>

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);
+}

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Sudeep Holla]

On Tue, Jul 05, 2022 at 11:03:54PM +0000, Conor.Dooley@microchip.com wrote:
> 
> 
> On 05/07/2022 21:33, Conor.Dooley@microchip.com 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@microchip.com>
> >>>
> >>> 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@arm.com>
> >>
> >> 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@microchip.com>
> 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@microchip.com>
> 
> 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);
> +}
>

Looks good. Again package id is not cluster. This is what my series is
addressing. So update the log as Package instead of Cluster above. The
cluster id will be -1 unless you can get that for DT.


-- 
Regards,
Sudeep

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Conor.Dooley]

On 06/07/2022 10:21, Sudeep Holla wrote:
> On Tue, Jul 05, 2022 at 11:03:54PM +0000, Conor.Dooley@microchip.com wrote:
>>
>>
>> On 05/07/2022 21:33, Conor.Dooley@microchip.com 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@microchip.com>
>>>>>
>>>>> 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@arm.com>
>>>>
>>>> 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@microchip.com>
>> 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@microchip.com>
>>
>> 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);
>> +}
>>
> 
> Looks good. Again package id is not cluster. This is what my series is
> addressing. So update the log as Package instead of Cluster above. The
> cluster id will be -1 unless you can get that for DT.


Cool, I'll respin a v2 without this included then & get the series
backported since this is user visible & will fix all existing supported
platforms.


I'll send the topology code changes separately to avoid it going
forwards.
Thanks!
Conor.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Sudeep Holla]

On Wed, Jul 06, 2022 at 09:43:02AM +0000, Conor.Dooley@microchip.com wrote:
> 
> 
> On 06/07/2022 10:21, Sudeep Holla wrote:
> > On Tue, Jul 05, 2022 at 11:03:54PM +0000, Conor.Dooley@microchip.com wrote:
> >>
> >>
> >> On 05/07/2022 21:33, Conor.Dooley@microchip.com 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@microchip.com>
> >>>>>
> >>>>> 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@arm.com>
> >>>>
> >>>> 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@microchip.com>
> >> 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@microchip.com>
> >>
> >> 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);
> >> +}
> >>
> > 
> > Looks good. Again package id is not cluster. This is what my series is
> > addressing. So update the log as Package instead of Cluster above. The
> > cluster id will be -1 unless you can get that for DT.
> 
> 
> Cool, I'll respin a v2 without this included then & get the series
> backported since this is user visible & will fix all existing supported
> platforms.
>

The DT /cpu-map is optional, so I don't think it needs to be backported.

> 
> I'll send the topology code changes separately to avoid it going
> forwards.

This on the other hand can be backported to fix userspace.

-- 
Regards,
Sudeep

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Conor.Dooley]

On 06/07/2022 11:03, Sudeep Holla wrote:
> On Wed, Jul 06, 2022 at 09:43:02AM +0000, Conor.Dooley@microchip.com wrote:
>> Cool, I'll respin a v2 without this included then & get the series
>> backported since this is user visible & will fix all existing supported
>> platforms.
>>
> 
> The DT /cpu-map is optional, so I don't think it needs to be backported.
> 
>>
>> I'll send the topology code changes separately to avoid it going
>> forwards.
> 
> This on the other hand can be backported to fix userspace.
> 

Uh sure, that sounds like a better idea (but more work lol).
No need for a v2 of this series then.

Thanks again :)
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Conor.Dooley]

On 06/07/2022 10:21, Sudeep Holla wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Tue, Jul 05, 2022 at 11:03:54PM +0000, Conor.Dooley@microchip.com wrote:
>>
>>
>> On 05/07/2022 21:33, Conor.Dooley@microchip.com 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@microchip.com>
>>>>>
>>>>> 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@arm.com>
>>>>
>>>> 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@microchip.com>
>> 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@microchip.com>
>>
>> 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);
>> +}
>>
> 
> Looks good. Again package id is not cluster. This is what my series is
> addressing. So update the log as Package instead of Cluster above. The
> cluster id will be -1 unless you can get that for DT.
>

FYI I took that directly from arm64:
arch/arm64/kernel/topology.c:L57 (next-20220706)

	pr_debug("CPU%u: cluster %d core %d thread %d mpidr %#016llx\n",
		 cpuid, cpuid_topo->package_id, cpuid_topo->core_id,
		 cpuid_topo->thread_id, mpidr);

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Sudeep Holla]

On Wed, Jul 06, 2022 at 01:04:24PM +0000, Conor.Dooley@microchip.com wrote:
> On 06/07/2022 10:21, Sudeep Holla wrote:

[...]

> > Looks good. Again package id is not cluster. This is what my series is
> > addressing. So update the log as Package instead of Cluster above. The
> > cluster id will be -1 unless you can get that for DT.
> >
> 
> FYI I took that directly from arm64:
> arch/arm64/kernel/topology.c:L57 (next-20220706)
> 
> 	pr_debug("CPU%u: cluster %d core %d thread %d mpidr %#016llx\n",
> 		 cpuid, cpuid_topo->package_id, cpuid_topo->core_id,
> 		 cpuid_topo->thread_id, mpidr);
> 

Yikes, that needs to change. I will get that updates. Thanks for pointing
that out.

-- 
Regards,
Sudeep

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Sudeep Holla]

On Tue, Jul 05, 2022 at 08:33:39PM +0000, Conor.Dooley@microchip.com 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@microchip.com>
> >>
> >> 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@arm.com>
> > 
> > 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.

Sorry I didn't mean it to be part of this series. This series of DT changes
are just fine on their own.

-- 
Regards,
Sudeep

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

On Tue, 5 Jul 2022 20:04:31 +0100, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> 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.
> 
> [...]

Applied to dt-for-next, thanks!

[4/5] riscv: dts: microchip: Add mpfs' topology information
      https://git.kernel.org/conor/c/88d319c6abae

The rest is yours Palmer once reviewed :)

Thanks,
Conor.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv