[PATCH/RFC 0/4] Fix RISC-V's arch-topology reporting

[PATCH/RFC 0/4] Fix RISC-V's arch-topology reporting

[Conor Dooley]

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

Hey all,
It's my first time messing around with arch/ code at all, let alone
more than one arch, so forgive me if I have screwed up how to do a
migration like this.

The goal here is the fix the incorrectly reported arch topology on
RISC-V which seems to have been broken since it was added.
cpu, package and thread IDs are all currently reported as -1, so tools
like lstopo think systems have multiple threads on the same core when
this is not true:
https://github.com/open-mpi/hwloc/issues/536

Because I want to backport that fix, I implemented store_cpu_topology
for RISC-V before migrating to the generic version which seems to have
just complicated things, but was Sudeep's preferred way of doing it.

Palmer, I have not marked the first patch as RFC because I would like
it to be taken as a fix for 5.19-rc(late) independently of the rest if
possible.

The rest of the series is RFC b/c I don't know what I am doing ;)

I only built tested for arm64, so hopefully it is not broken when used.

Thanks,
Conor.

Conor Dooley (4):
  riscv: arch-topology: fix default topology reporting
  arch-topology: add a default implementation of store_cpu_topology()
  riscv: arch-topology: move riscv to the generic store_cpu_topology()
  arm64: arch-topology move arm64 to the generic store_cpu_topology()

 arch/arm64/kernel/smp.c       | 16 ++++++++++++--
 arch/arm64/kernel/topology.c  | 40 -----------------------------------
 arch/riscv/Kconfig            |  2 +-
 arch/riscv/kernel/smpboot.c   |  4 +++-
 drivers/base/arch_topology.c  | 19 +++++++++++++++++
 include/linux/arch_topology.h |  1 +
 6 files changed, 38 insertions(+), 44 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/RFC 0/4] Fix RISC-V's arch-topology reporting

[Conor Dooley]

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

Hey all,
It's my first time messing around with arch/ code at all, let alone
more than one arch, so forgive me if I have screwed up how to do a
migration like this.

The goal here is the fix the incorrectly reported arch topology on
RISC-V which seems to have been broken since it was added.
cpu, package and thread IDs are all currently reported as -1, so tools
like lstopo think systems have multiple threads on the same core when
this is not true:
https://github.com/open-mpi/hwloc/issues/536

Because I want to backport that fix, I implemented store_cpu_topology
for RISC-V before migrating to the generic version which seems to have
just complicated things, but was Sudeep's preferred way of doing it.

Palmer, I have not marked the first patch as RFC because I would like
it to be taken as a fix for 5.19-rc(late) independently of the rest if
possible.

The rest of the series is RFC b/c I don't know what I am doing ;)

I only built tested for arm64, so hopefully it is not broken when used.

Thanks,
Conor.

Conor Dooley (4):
  riscv: arch-topology: fix default topology reporting
  arch-topology: add a default implementation of store_cpu_topology()
  riscv: arch-topology: move riscv to the generic store_cpu_topology()
  arm64: arch-topology move arm64 to the generic store_cpu_topology()

 arch/arm64/kernel/smp.c       | 16 ++++++++++++--
 arch/arm64/kernel/topology.c  | 40 -----------------------------------
 arch/riscv/Kconfig            |  2 +-
 arch/riscv/kernel/smpboot.c   |  4 +++-
 drivers/base/arch_topology.c  | 19 +++++++++++++++++
 include/linux/arch_topology.h |  1 +
 6 files changed, 38 insertions(+), 44 deletions(-)


base-commit: b6f1f2fa2bddd69ff46a190b8120bd440fd50563
-- 
2.37.0

[PATCH/RFC 0/4] Fix RISC-V's arch-topology reporting

[Conor Dooley]

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

Hey all,
It's my first time messing around with arch/ code at all, let alone
more than one arch, so forgive me if I have screwed up how to do a
migration like this.

The goal here is the fix the incorrectly reported arch topology on
RISC-V which seems to have been broken since it was added.
cpu, package and thread IDs are all currently reported as -1, so tools
like lstopo think systems have multiple threads on the same core when
this is not true:
https://github.com/open-mpi/hwloc/issues/536

Because I want to backport that fix, I implemented store_cpu_topology
for RISC-V before migrating to the generic version which seems to have
just complicated things, but was Sudeep's preferred way of doing it.

Palmer, I have not marked the first patch as RFC because I would like
it to be taken as a fix for 5.19-rc(late) independently of the rest if
possible.

The rest of the series is RFC b/c I don't know what I am doing ;)

I only built tested for arm64, so hopefully it is not broken when used.

Thanks,
Conor.

Conor Dooley (4):
  riscv: arch-topology: fix default topology reporting
  arch-topology: add a default implementation of store_cpu_topology()
  riscv: arch-topology: move riscv to the generic store_cpu_topology()
  arm64: arch-topology move arm64 to the generic store_cpu_topology()

 arch/arm64/kernel/smp.c       | 16 ++++++++++++--
 arch/arm64/kernel/topology.c  | 40 -----------------------------------
 arch/riscv/Kconfig            |  2 +-
 arch/riscv/kernel/smpboot.c   |  4 +++-
 drivers/base/arch_topology.c  | 19 +++++++++++++++++
 include/linux/arch_topology.h |  1 +
 6 files changed, 38 insertions(+), 44 deletions(-)


base-commit: b6f1f2fa2bddd69ff46a190b8120bd440fd50563
-- 
2.37.0


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

[RFC 1/4] riscv: arch-topology: fix default topology reporting

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.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@vger.kernel.org
Fixes: 03f11f03dbfe ("RISC-V: Parse cpu topology during boot.")
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>
---
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.

Changes since v1:
- removed the GENERIC_ARCH_TOPOLOGY dependancy on SMP
- removed a duplicate call to update_siblings_masks()
---
 arch/riscv/Kconfig                |  2 +-
 arch/riscv/include/asm/topology.h | 13 +++++++++++++
 arch/riscv/kernel/Makefile        |  1 +
 arch/riscv/kernel/smpboot.c       |  5 ++++-
 arch/riscv/kernel/topology.c      | 32 +++++++++++++++++++++++++++++++
 5 files changed, 51 insertions(+), 2 deletions(-)
 create mode 100644 arch/riscv/include/asm/topology.h
 create mode 100644 arch/riscv/kernel/topology.c

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 2af0701b7518..4b6c2fdbb57c 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -52,7 +52,7 @@ config RISCV
 	select COMMON_CLK
 	select CPU_PM if CPU_IDLE
 	select EDAC_SUPPORT
-	select GENERIC_ARCH_TOPOLOGY if SMP
+	select GENERIC_ARCH_TOPOLOGY
 	select GENERIC_ATOMIC64 if !64BIT
 	select GENERIC_CLOCKEVENTS_BROADCAST if SMP
 	select GENERIC_EARLY_IOREMAP
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..a8239b4b61f3 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,9 +164,9 @@ 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);
 	set_cpu_online(curr_cpuid, 1);
 
 	/*
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);
+}
-- 
2.37.0


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

[RFC 1/4] riscv: arch-topology: fix default topology reporting

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.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@vger.kernel.org
Fixes: 03f11f03dbfe ("RISC-V: Parse cpu topology during boot.")
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>
---
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.

Changes since v1:
- removed the GENERIC_ARCH_TOPOLOGY dependancy on SMP
- removed a duplicate call to update_siblings_masks()
---
 arch/riscv/Kconfig                |  2 +-
 arch/riscv/include/asm/topology.h | 13 +++++++++++++
 arch/riscv/kernel/Makefile        |  1 +
 arch/riscv/kernel/smpboot.c       |  5 ++++-
 arch/riscv/kernel/topology.c      | 32 +++++++++++++++++++++++++++++++
 5 files changed, 51 insertions(+), 2 deletions(-)
 create mode 100644 arch/riscv/include/asm/topology.h
 create mode 100644 arch/riscv/kernel/topology.c

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 2af0701b7518..4b6c2fdbb57c 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -52,7 +52,7 @@ config RISCV
 	select COMMON_CLK
 	select CPU_PM if CPU_IDLE
 	select EDAC_SUPPORT
-	select GENERIC_ARCH_TOPOLOGY if SMP
+	select GENERIC_ARCH_TOPOLOGY
 	select GENERIC_ATOMIC64 if !64BIT
 	select GENERIC_CLOCKEVENTS_BROADCAST if SMP
 	select GENERIC_EARLY_IOREMAP
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..a8239b4b61f3 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,9 +164,9 @@ 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);
 	set_cpu_online(curr_cpuid, 1);
 
 	/*
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);
+}
-- 
2.37.0

[RFC 1/4] riscv: arch-topology: fix default topology reporting

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.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@vger.kernel.org
Fixes: 03f11f03dbfe ("RISC-V: Parse cpu topology during boot.")
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>
---
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.

Changes since v1:
- removed the GENERIC_ARCH_TOPOLOGY dependancy on SMP
- removed a duplicate call to update_siblings_masks()
---
 arch/riscv/Kconfig                |  2 +-
 arch/riscv/include/asm/topology.h | 13 +++++++++++++
 arch/riscv/kernel/Makefile        |  1 +
 arch/riscv/kernel/smpboot.c       |  5 ++++-
 arch/riscv/kernel/topology.c      | 32 +++++++++++++++++++++++++++++++
 5 files changed, 51 insertions(+), 2 deletions(-)
 create mode 100644 arch/riscv/include/asm/topology.h
 create mode 100644 arch/riscv/kernel/topology.c

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 2af0701b7518..4b6c2fdbb57c 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -52,7 +52,7 @@ config RISCV
 	select COMMON_CLK
 	select CPU_PM if CPU_IDLE
 	select EDAC_SUPPORT
-	select GENERIC_ARCH_TOPOLOGY if SMP
+	select GENERIC_ARCH_TOPOLOGY
 	select GENERIC_ATOMIC64 if !64BIT
 	select GENERIC_CLOCKEVENTS_BROADCAST if SMP
 	select GENERIC_EARLY_IOREMAP
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..a8239b4b61f3 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,9 +164,9 @@ 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);
 	set_cpu_online(curr_cpuid, 1);
 
 	/*
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);
+}
-- 
2.37.0


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

[RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Conor Dooley]

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

RISC-V & arm64 both use an almost identical method of filling in
default vales for arch topology. Create a weakly defined default
implementation with the intent of migrating both archs to use it.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/base/arch_topology.c  | 19 +++++++++++++++++++
 include/linux/arch_topology.h |  1 +
 2 files changed, 20 insertions(+)

diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 441e14ac33a4..07e84c6ac5c2 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
 	}
 }
 
+void __weak 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);
+}
+
 static void clear_cpu_topology(int cpu)
 {
 	struct cpu_topology *cpu_topo = &cpu_topology[cpu];
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
index a07b510e7dc5..fee306b8a541 100644
--- a/include/linux/arch_topology.h
+++ b/include/linux/arch_topology.h
@@ -92,6 +92,7 @@ void update_siblings_masks(unsigned int cpu);
 void remove_cpu_topology(unsigned int cpuid);
 void reset_cpu_topology(void);
 int parse_acpi_topology(void);
+
 #endif
 
 #endif /* _LINUX_ARCH_TOPOLOGY_H_ */
-- 
2.37.0


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

[RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Conor Dooley]

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

RISC-V & arm64 both use an almost identical method of filling in
default vales for arch topology. Create a weakly defined default
implementation with the intent of migrating both archs to use it.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/base/arch_topology.c  | 19 +++++++++++++++++++
 include/linux/arch_topology.h |  1 +
 2 files changed, 20 insertions(+)

diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 441e14ac33a4..07e84c6ac5c2 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
 	}
 }
 
+void __weak 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);
+}
+
 static void clear_cpu_topology(int cpu)
 {
 	struct cpu_topology *cpu_topo = &cpu_topology[cpu];
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
index a07b510e7dc5..fee306b8a541 100644
--- a/include/linux/arch_topology.h
+++ b/include/linux/arch_topology.h
@@ -92,6 +92,7 @@ void update_siblings_masks(unsigned int cpu);
 void remove_cpu_topology(unsigned int cpuid);
 void reset_cpu_topology(void);
 int parse_acpi_topology(void);
+
 #endif
 
 #endif /* _LINUX_ARCH_TOPOLOGY_H_ */
-- 
2.37.0

[RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Conor Dooley]

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

RISC-V & arm64 both use an almost identical method of filling in
default vales for arch topology. Create a weakly defined default
implementation with the intent of migrating both archs to use it.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/base/arch_topology.c  | 19 +++++++++++++++++++
 include/linux/arch_topology.h |  1 +
 2 files changed, 20 insertions(+)

diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 441e14ac33a4..07e84c6ac5c2 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
 	}
 }
 
+void __weak 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);
+}
+
 static void clear_cpu_topology(int cpu)
 {
 	struct cpu_topology *cpu_topo = &cpu_topology[cpu];
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
index a07b510e7dc5..fee306b8a541 100644
--- a/include/linux/arch_topology.h
+++ b/include/linux/arch_topology.h
@@ -92,6 +92,7 @@ void update_siblings_masks(unsigned int cpu);
 void remove_cpu_topology(unsigned int cpuid);
 void reset_cpu_topology(void);
 int parse_acpi_topology(void);
+
 #endif
 
 #endif /* _LINUX_ARCH_TOPOLOGY_H_ */
-- 
2.37.0


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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Sudeep Holla]

On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> RISC-V & arm64 both use an almost identical method of filling in
> default vales for arch topology. Create a weakly defined default
> implementation with the intent of migrating both archs to use it.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  drivers/base/arch_topology.c  | 19 +++++++++++++++++++
>  include/linux/arch_topology.h |  1 +
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index 441e14ac33a4..07e84c6ac5c2 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
>  	}
>  }
>  
> +void __weak store_cpu_topology(unsigned int cpuid)

I prefer to have this as default implementation. So just get the risc-v
one pushed to upstream first(for v5.20) and get all the backports if required.
Next cycle(i.e. v5.21), you can move both RISC-V and arm64.

-- 
Regards,
Sudeep

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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Sudeep Holla]

On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> RISC-V & arm64 both use an almost identical method of filling in
> default vales for arch topology. Create a weakly defined default
> implementation with the intent of migrating both archs to use it.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  drivers/base/arch_topology.c  | 19 +++++++++++++++++++
>  include/linux/arch_topology.h |  1 +
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index 441e14ac33a4..07e84c6ac5c2 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
>  	}
>  }
>  
> +void __weak store_cpu_topology(unsigned int cpuid)

I prefer to have this as default implementation. So just get the risc-v
one pushed to upstream first(for v5.20) and get all the backports if required.
Next cycle(i.e. v5.21), you can move both RISC-V and arm64.

-- 
Regards,
Sudeep

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Sudeep Holla]

On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> RISC-V & arm64 both use an almost identical method of filling in
> default vales for arch topology. Create a weakly defined default
> implementation with the intent of migrating both archs to use it.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  drivers/base/arch_topology.c  | 19 +++++++++++++++++++
>  include/linux/arch_topology.h |  1 +
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index 441e14ac33a4..07e84c6ac5c2 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
>  	}
>  }
>  
> +void __weak store_cpu_topology(unsigned int cpuid)

I prefer to have this as default implementation. So just get the risc-v
one pushed to upstream first(for v5.20) and get all the backports if required.
Next cycle(i.e. v5.21), you can move both RISC-V and arm64.

-- 
Regards,
Sudeep

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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Conor.Dooley]

On 08/07/2022 09:24, Sudeep Holla wrote:
> On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
>> From: Conor Dooley <conor.dooley@microchip.com>
>>
>> RISC-V & arm64 both use an almost identical method of filling in
>> default vales for arch topology. Create a weakly defined default
>> implementation with the intent of migrating both archs to use it.
>>
>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>> ---
>>   drivers/base/arch_topology.c  | 19 +++++++++++++++++++
>>   include/linux/arch_topology.h |  1 +
>>   2 files changed, 20 insertions(+)
>>
>> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
>> index 441e14ac33a4..07e84c6ac5c2 100644
>> --- a/drivers/base/arch_topology.c
>> +++ b/drivers/base/arch_topology.c
>> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
>>   	}
>>   }
>>   
>> +void __weak store_cpu_topology(unsigned int cpuid)

Does using __weak here make sense to you?

> 
> I prefer to have this as default implementation. So just get the risc-v
> one pushed to upstream first(for v5.20) and get all the backports if required.
> Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
> 

Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
the risc-v impl. and then migrate both on IRC & he seemed happy with
it.

If you're okay with patch 1/4, I'll resubmit it as a standalone v2.

Thanks,
Conor.

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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Conor.Dooley]

On 08/07/2022 09:24, Sudeep Holla wrote:
> On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
>> From: Conor Dooley <conor.dooley@microchip.com>
>>
>> RISC-V & arm64 both use an almost identical method of filling in
>> default vales for arch topology. Create a weakly defined default
>> implementation with the intent of migrating both archs to use it.
>>
>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>> ---
>>   drivers/base/arch_topology.c  | 19 +++++++++++++++++++
>>   include/linux/arch_topology.h |  1 +
>>   2 files changed, 20 insertions(+)
>>
>> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
>> index 441e14ac33a4..07e84c6ac5c2 100644
>> --- a/drivers/base/arch_topology.c
>> +++ b/drivers/base/arch_topology.c
>> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
>>   	}
>>   }
>>   
>> +void __weak store_cpu_topology(unsigned int cpuid)

Does using __weak here make sense to you?

> 
> I prefer to have this as default implementation. So just get the risc-v
> one pushed to upstream first(for v5.20) and get all the backports if required.
> Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
> 

Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
the risc-v impl. and then migrate both on IRC & he seemed happy with
it.

If you're okay with patch 1/4, I'll resubmit it as a standalone v2.

Thanks,
Conor.

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Conor.Dooley]

On 08/07/2022 09:24, Sudeep Holla wrote:
> On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
>> From: Conor Dooley <conor.dooley@microchip.com>
>>
>> RISC-V & arm64 both use an almost identical method of filling in
>> default vales for arch topology. Create a weakly defined default
>> implementation with the intent of migrating both archs to use it.
>>
>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>> ---
>>   drivers/base/arch_topology.c  | 19 +++++++++++++++++++
>>   include/linux/arch_topology.h |  1 +
>>   2 files changed, 20 insertions(+)
>>
>> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
>> index 441e14ac33a4..07e84c6ac5c2 100644
>> --- a/drivers/base/arch_topology.c
>> +++ b/drivers/base/arch_topology.c
>> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
>>   	}
>>   }
>>   
>> +void __weak store_cpu_topology(unsigned int cpuid)

Does using __weak here make sense to you?

> 
> I prefer to have this as default implementation. So just get the risc-v
> one pushed to upstream first(for v5.20) and get all the backports if required.
> Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
> 

Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
the risc-v impl. and then migrate both on IRC & he seemed happy with
it.

If you're okay with patch 1/4, I'll resubmit it as a standalone v2.

Thanks,
Conor.

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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Sudeep Holla]

On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
> On 08/07/2022 09:24, Sudeep Holla wrote:
> > On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
> >> From: Conor Dooley <conor.dooley@microchip.com>
> >>
> >> RISC-V & arm64 both use an almost identical method of filling in
> >> default vales for arch topology. Create a weakly defined default
> >> implementation with the intent of migrating both archs to use it.
> >>
> >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> >> ---
> >>   drivers/base/arch_topology.c  | 19 +++++++++++++++++++
> >>   include/linux/arch_topology.h |  1 +
> >>   2 files changed, 20 insertions(+)
> >>
> >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> >> index 441e14ac33a4..07e84c6ac5c2 100644
> >> --- a/drivers/base/arch_topology.c
> >> +++ b/drivers/base/arch_topology.c
> >> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
> >>   	}
> >>   }
> >>   
> >> +void __weak store_cpu_topology(unsigned int cpuid)
> 
> Does using __weak here make sense to you?
>

I don't want any weak definition and arch to override as we know only
arm64 and RISC-V are the only users and they are aligned to have same
implementation. So weak definition doesn't make sense to me.

> > 
> > I prefer to have this as default implementation. So just get the risc-v
> > one pushed to upstream first(for v5.20) and get all the backports if required.
> > Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
> > 
> 
> Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
> and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
> the risc-v impl. and then migrate both on IRC & he seemed happy with
> it.
>

Ah OK, good.

> If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
>

That would be great, thanks. You can most the code to move to generic from
both arm64 and risc-v once we have this in v5.20-rc1

-- 
Regards,
Sudeep

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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Sudeep Holla]

On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
> On 08/07/2022 09:24, Sudeep Holla wrote:
> > On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
> >> From: Conor Dooley <conor.dooley@microchip.com>
> >>
> >> RISC-V & arm64 both use an almost identical method of filling in
> >> default vales for arch topology. Create a weakly defined default
> >> implementation with the intent of migrating both archs to use it.
> >>
> >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> >> ---
> >>   drivers/base/arch_topology.c  | 19 +++++++++++++++++++
> >>   include/linux/arch_topology.h |  1 +
> >>   2 files changed, 20 insertions(+)
> >>
> >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> >> index 441e14ac33a4..07e84c6ac5c2 100644
> >> --- a/drivers/base/arch_topology.c
> >> +++ b/drivers/base/arch_topology.c
> >> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
> >>   	}
> >>   }
> >>   
> >> +void __weak store_cpu_topology(unsigned int cpuid)
> 
> Does using __weak here make sense to you?
>

I don't want any weak definition and arch to override as we know only
arm64 and RISC-V are the only users and they are aligned to have same
implementation. So weak definition doesn't make sense to me.

> > 
> > I prefer to have this as default implementation. So just get the risc-v
> > one pushed to upstream first(for v5.20) and get all the backports if required.
> > Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
> > 
> 
> Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
> and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
> the risc-v impl. and then migrate both on IRC & he seemed happy with
> it.
>

Ah OK, good.

> If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
>

That would be great, thanks. You can most the code to move to generic from
both arm64 and risc-v once we have this in v5.20-rc1

-- 
Regards,
Sudeep

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Sudeep Holla]

On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
> On 08/07/2022 09:24, Sudeep Holla wrote:
> > On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
> >> From: Conor Dooley <conor.dooley@microchip.com>
> >>
> >> RISC-V & arm64 both use an almost identical method of filling in
> >> default vales for arch topology. Create a weakly defined default
> >> implementation with the intent of migrating both archs to use it.
> >>
> >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> >> ---
> >>   drivers/base/arch_topology.c  | 19 +++++++++++++++++++
> >>   include/linux/arch_topology.h |  1 +
> >>   2 files changed, 20 insertions(+)
> >>
> >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> >> index 441e14ac33a4..07e84c6ac5c2 100644
> >> --- a/drivers/base/arch_topology.c
> >> +++ b/drivers/base/arch_topology.c
> >> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
> >>   	}
> >>   }
> >>   
> >> +void __weak store_cpu_topology(unsigned int cpuid)
> 
> Does using __weak here make sense to you?
>

I don't want any weak definition and arch to override as we know only
arm64 and RISC-V are the only users and they are aligned to have same
implementation. So weak definition doesn't make sense to me.

> > 
> > I prefer to have this as default implementation. So just get the risc-v
> > one pushed to upstream first(for v5.20) and get all the backports if required.
> > Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
> > 
> 
> Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
> and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
> the risc-v impl. and then migrate both on IRC & he seemed happy with
> it.
>

Ah OK, good.

> If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
>

That would be great, thanks. You can most the code to move to generic from
both arm64 and risc-v once we have this in v5.20-rc1

-- 
Regards,
Sudeep

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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Geert Uytterhoeven]

Hi Sudeep,

On Fri, Jul 8, 2022 at 11:22 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
> > On 08/07/2022 09:24, Sudeep Holla wrote:
> > > On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
> > >> From: Conor Dooley <conor.dooley@microchip.com>
> > >>
> > >> RISC-V & arm64 both use an almost identical method of filling in
> > >> default vales for arch topology. Create a weakly defined default
> > >> implementation with the intent of migrating both archs to use it.
> > >>
> > >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > >> ---
> > >>   drivers/base/arch_topology.c  | 19 +++++++++++++++++++
> > >>   include/linux/arch_topology.h |  1 +
> > >>   2 files changed, 20 insertions(+)
> > >>
> > >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> > >> index 441e14ac33a4..07e84c6ac5c2 100644
> > >> --- a/drivers/base/arch_topology.c
> > >> +++ b/drivers/base/arch_topology.c
> > >> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
> > >>    }
> > >>   }
> > >>
> > >> +void __weak store_cpu_topology(unsigned int cpuid)
> >
> > Does using __weak here make sense to you?
> >
>
> I don't want any weak definition and arch to override as we know only
> arm64 and RISC-V are the only users and they are aligned to have same
> implementation. So weak definition doesn't make sense to me.
>
> > >
> > > I prefer to have this as default implementation. So just get the risc-v
> > > one pushed to upstream first(for v5.20) and get all the backports if required.
> > > Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
> > >
> >
> > Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
> > and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
> > the risc-v impl. and then migrate both on IRC & he seemed happy with
> > it.
> >
>
> Ah OK, good.
>
> > If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
> >
>
> That would be great, thanks. You can most the code to move to generic from
> both arm64 and risc-v once we have this in v5.20-rc1

Why not ignore risc-v for now, and move the arm64 implementation to
the generic code for v5.20, so every arch will have it at once?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Geert Uytterhoeven]

Hi Sudeep,

On Fri, Jul 8, 2022 at 11:22 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
> > On 08/07/2022 09:24, Sudeep Holla wrote:
> > > On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
> > >> From: Conor Dooley <conor.dooley@microchip.com>
> > >>
> > >> RISC-V & arm64 both use an almost identical method of filling in
> > >> default vales for arch topology. Create a weakly defined default
> > >> implementation with the intent of migrating both archs to use it.
> > >>
> > >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > >> ---
> > >>   drivers/base/arch_topology.c  | 19 +++++++++++++++++++
> > >>   include/linux/arch_topology.h |  1 +
> > >>   2 files changed, 20 insertions(+)
> > >>
> > >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> > >> index 441e14ac33a4..07e84c6ac5c2 100644
> > >> --- a/drivers/base/arch_topology.c
> > >> +++ b/drivers/base/arch_topology.c
> > >> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
> > >>    }
> > >>   }
> > >>
> > >> +void __weak store_cpu_topology(unsigned int cpuid)
> >
> > Does using __weak here make sense to you?
> >
>
> I don't want any weak definition and arch to override as we know only
> arm64 and RISC-V are the only users and they are aligned to have same
> implementation. So weak definition doesn't make sense to me.
>
> > >
> > > I prefer to have this as default implementation. So just get the risc-v
> > > one pushed to upstream first(for v5.20) and get all the backports if required.
> > > Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
> > >
> >
> > Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
> > and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
> > the risc-v impl. and then migrate both on IRC & he seemed happy with
> > it.
> >
>
> Ah OK, good.
>
> > If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
> >
>
> That would be great, thanks. You can most the code to move to generic from
> both arm64 and risc-v once we have this in v5.20-rc1

Why not ignore risc-v for now, and move the arm64 implementation to
the generic code for v5.20, so every arch will have it at once?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Geert Uytterhoeven]

Hi Sudeep,

On Fri, Jul 8, 2022 at 11:22 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
> > On 08/07/2022 09:24, Sudeep Holla wrote:
> > > On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
> > >> From: Conor Dooley <conor.dooley@microchip.com>
> > >>
> > >> RISC-V & arm64 both use an almost identical method of filling in
> > >> default vales for arch topology. Create a weakly defined default
> > >> implementation with the intent of migrating both archs to use it.
> > >>
> > >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > >> ---
> > >>   drivers/base/arch_topology.c  | 19 +++++++++++++++++++
> > >>   include/linux/arch_topology.h |  1 +
> > >>   2 files changed, 20 insertions(+)
> > >>
> > >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> > >> index 441e14ac33a4..07e84c6ac5c2 100644
> > >> --- a/drivers/base/arch_topology.c
> > >> +++ b/drivers/base/arch_topology.c
> > >> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
> > >>    }
> > >>   }
> > >>
> > >> +void __weak store_cpu_topology(unsigned int cpuid)
> >
> > Does using __weak here make sense to you?
> >
>
> I don't want any weak definition and arch to override as we know only
> arm64 and RISC-V are the only users and they are aligned to have same
> implementation. So weak definition doesn't make sense to me.
>
> > >
> > > I prefer to have this as default implementation. So just get the risc-v
> > > one pushed to upstream first(for v5.20) and get all the backports if required.
> > > Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
> > >
> >
> > Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
> > and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
> > the risc-v impl. and then migrate both on IRC & he seemed happy with
> > it.
> >
>
> Ah OK, good.
>
> > If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
> >
>
> That would be great, thanks. You can most the code to move to generic from
> both arm64 and risc-v once we have this in v5.20-rc1

Why not ignore risc-v for now, and move the arm64 implementation to
the generic code for v5.20, so every arch will have it at once?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Sudeep Holla]

On Fri, Jul 08, 2022 at 11:28:19AM +0200, Geert Uytterhoeven wrote:
> Hi Sudeep,
> 
> On Fri, Jul 8, 2022 at 11:22 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
> > > On 08/07/2022 09:24, Sudeep Holla wrote:
> > > > On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
> > > >> From: Conor Dooley <conor.dooley@microchip.com>
> > > >>
> > > >> RISC-V & arm64 both use an almost identical method of filling in
> > > >> default vales for arch topology. Create a weakly defined default
> > > >> implementation with the intent of migrating both archs to use it.
> > > >>
> > > >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > >> ---
> > > >>   drivers/base/arch_topology.c  | 19 +++++++++++++++++++
> > > >>   include/linux/arch_topology.h |  1 +
> > > >>   2 files changed, 20 insertions(+)
> > > >>
> > > >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> > > >> index 441e14ac33a4..07e84c6ac5c2 100644
> > > >> --- a/drivers/base/arch_topology.c
> > > >> +++ b/drivers/base/arch_topology.c
> > > >> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
> > > >>    }
> > > >>   }
> > > >>
> > > >> +void __weak store_cpu_topology(unsigned int cpuid)
> > >
> > > Does using __weak here make sense to you?
> > >
> >
> > I don't want any weak definition and arch to override as we know only
> > arm64 and RISC-V are the only users and they are aligned to have same
> > implementation. So weak definition doesn't make sense to me.
> >
> > > >
> > > > I prefer to have this as default implementation. So just get the risc-v
> > > > one pushed to upstream first(for v5.20) and get all the backports if required.
> > > > Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
> > > >
> > >
> > > Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
> > > and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
> > > the risc-v impl. and then migrate both on IRC & he seemed happy with
> > > it.
> > >
> >
> > Ah OK, good.
> >
> > > If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
> > >
> >
> > That would be great, thanks. You can most the code to move to generic from
> > both arm64 and risc-v once we have this in v5.20-rc1
> 
> Why not ignore risc-v for now, and move the arm64 implementation to
> the generic code for v5.20, so every arch will have it at once?
>

We could but,
1. This arch_topology is new and has been going through lot of changes
   recently and having code there might make it difficult to backport
   changes that are required for RISC-V(my guess)

2. May be too late for v5.20, I would like to see if we can even drop tiny
   arm64 bit in the code. It may be risky to try that this late and also
   with other topology changes we already have queued.

Let me know if that makes sense.

-- 
Regards,
Sudeep

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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Sudeep Holla]

On Fri, Jul 08, 2022 at 11:28:19AM +0200, Geert Uytterhoeven wrote:
> Hi Sudeep,
> 
> On Fri, Jul 8, 2022 at 11:22 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
> > > On 08/07/2022 09:24, Sudeep Holla wrote:
> > > > On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
> > > >> From: Conor Dooley <conor.dooley@microchip.com>
> > > >>
> > > >> RISC-V & arm64 both use an almost identical method of filling in
> > > >> default vales for arch topology. Create a weakly defined default
> > > >> implementation with the intent of migrating both archs to use it.
> > > >>
> > > >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > >> ---
> > > >>   drivers/base/arch_topology.c  | 19 +++++++++++++++++++
> > > >>   include/linux/arch_topology.h |  1 +
> > > >>   2 files changed, 20 insertions(+)
> > > >>
> > > >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> > > >> index 441e14ac33a4..07e84c6ac5c2 100644
> > > >> --- a/drivers/base/arch_topology.c
> > > >> +++ b/drivers/base/arch_topology.c
> > > >> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
> > > >>    }
> > > >>   }
> > > >>
> > > >> +void __weak store_cpu_topology(unsigned int cpuid)
> > >
> > > Does using __weak here make sense to you?
> > >
> >
> > I don't want any weak definition and arch to override as we know only
> > arm64 and RISC-V are the only users and they are aligned to have same
> > implementation. So weak definition doesn't make sense to me.
> >
> > > >
> > > > I prefer to have this as default implementation. So just get the risc-v
> > > > one pushed to upstream first(for v5.20) and get all the backports if required.
> > > > Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
> > > >
> > >
> > > Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
> > > and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
> > > the risc-v impl. and then migrate both on IRC & he seemed happy with
> > > it.
> > >
> >
> > Ah OK, good.
> >
> > > If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
> > >
> >
> > That would be great, thanks. You can most the code to move to generic from
> > both arm64 and risc-v once we have this in v5.20-rc1
> 
> Why not ignore risc-v for now, and move the arm64 implementation to
> the generic code for v5.20, so every arch will have it at once?
>

We could but,
1. This arch_topology is new and has been going through lot of changes
   recently and having code there might make it difficult to backport
   changes that are required for RISC-V(my guess)

2. May be too late for v5.20, I would like to see if we can even drop tiny
   arm64 bit in the code. It may be risky to try that this late and also
   with other topology changes we already have queued.

Let me know if that makes sense.

-- 
Regards,
Sudeep

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Sudeep Holla]

On Fri, Jul 08, 2022 at 11:28:19AM +0200, Geert Uytterhoeven wrote:
> Hi Sudeep,
> 
> On Fri, Jul 8, 2022 at 11:22 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
> > > On 08/07/2022 09:24, Sudeep Holla wrote:
> > > > On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
> > > >> From: Conor Dooley <conor.dooley@microchip.com>
> > > >>
> > > >> RISC-V & arm64 both use an almost identical method of filling in
> > > >> default vales for arch topology. Create a weakly defined default
> > > >> implementation with the intent of migrating both archs to use it.
> > > >>
> > > >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > >> ---
> > > >>   drivers/base/arch_topology.c  | 19 +++++++++++++++++++
> > > >>   include/linux/arch_topology.h |  1 +
> > > >>   2 files changed, 20 insertions(+)
> > > >>
> > > >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> > > >> index 441e14ac33a4..07e84c6ac5c2 100644
> > > >> --- a/drivers/base/arch_topology.c
> > > >> +++ b/drivers/base/arch_topology.c
> > > >> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
> > > >>    }
> > > >>   }
> > > >>
> > > >> +void __weak store_cpu_topology(unsigned int cpuid)
> > >
> > > Does using __weak here make sense to you?
> > >
> >
> > I don't want any weak definition and arch to override as we know only
> > arm64 and RISC-V are the only users and they are aligned to have same
> > implementation. So weak definition doesn't make sense to me.
> >
> > > >
> > > > I prefer to have this as default implementation. So just get the risc-v
> > > > one pushed to upstream first(for v5.20) and get all the backports if required.
> > > > Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
> > > >
> > >
> > > Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
> > > and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
> > > the risc-v impl. and then migrate both on IRC & he seemed happy with
> > > it.
> > >
> >
> > Ah OK, good.
> >
> > > If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
> > >
> >
> > That would be great, thanks. You can most the code to move to generic from
> > both arm64 and risc-v once we have this in v5.20-rc1
> 
> Why not ignore risc-v for now, and move the arm64 implementation to
> the generic code for v5.20, so every arch will have it at once?
>

We could but,
1. This arch_topology is new and has been going through lot of changes
   recently and having code there might make it difficult to backport
   changes that are required for RISC-V(my guess)

2. May be too late for v5.20, I would like to see if we can even drop tiny
   arm64 bit in the code. It may be risky to try that this late and also
   with other topology changes we already have queued.

Let me know if that makes sense.

-- 
Regards,
Sudeep

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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Greg KH]

On Fri, Jul 08, 2022 at 10:47:10AM +0100, Sudeep Holla wrote:
> On Fri, Jul 08, 2022 at 11:28:19AM +0200, Geert Uytterhoeven wrote:
> > Hi Sudeep,
> > 
> > On Fri, Jul 8, 2022 at 11:22 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
> > > > On 08/07/2022 09:24, Sudeep Holla wrote:
> > > > > On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
> > > > >> From: Conor Dooley <conor.dooley@microchip.com>
> > > > >>
> > > > >> RISC-V & arm64 both use an almost identical method of filling in
> > > > >> default vales for arch topology. Create a weakly defined default
> > > > >> implementation with the intent of migrating both archs to use it.
> > > > >>
> > > > >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > > >> ---
> > > > >>   drivers/base/arch_topology.c  | 19 +++++++++++++++++++
> > > > >>   include/linux/arch_topology.h |  1 +
> > > > >>   2 files changed, 20 insertions(+)
> > > > >>
> > > > >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> > > > >> index 441e14ac33a4..07e84c6ac5c2 100644
> > > > >> --- a/drivers/base/arch_topology.c
> > > > >> +++ b/drivers/base/arch_topology.c
> > > > >> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
> > > > >>    }
> > > > >>   }
> > > > >>
> > > > >> +void __weak store_cpu_topology(unsigned int cpuid)
> > > >
> > > > Does using __weak here make sense to you?
> > > >
> > >
> > > I don't want any weak definition and arch to override as we know only
> > > arm64 and RISC-V are the only users and they are aligned to have same
> > > implementation. So weak definition doesn't make sense to me.
> > >
> > > > >
> > > > > I prefer to have this as default implementation. So just get the risc-v
> > > > > one pushed to upstream first(for v5.20) and get all the backports if required.
> > > > > Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
> > > > >
> > > >
> > > > Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
> > > > and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
> > > > the risc-v impl. and then migrate both on IRC & he seemed happy with
> > > > it.
> > > >
> > >
> > > Ah OK, good.
> > >
> > > > If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
> > > >
> > >
> > > That would be great, thanks. You can most the code to move to generic from
> > > both arm64 and risc-v once we have this in v5.20-rc1
> > 
> > Why not ignore risc-v for now, and move the arm64 implementation to
> > the generic code for v5.20, so every arch will have it at once?
> >
> 
> We could but,
> 1. This arch_topology is new and has been going through lot of changes
>    recently and having code there might make it difficult to backport
>    changes that are required for RISC-V(my guess)

Worry about future issues in the future.  Make it simple now as you know
what you are dealing with at the moment.

thanks,

greg k-h

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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Greg KH]

On Fri, Jul 08, 2022 at 10:47:10AM +0100, Sudeep Holla wrote:
> On Fri, Jul 08, 2022 at 11:28:19AM +0200, Geert Uytterhoeven wrote:
> > Hi Sudeep,
> > 
> > On Fri, Jul 8, 2022 at 11:22 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
> > > > On 08/07/2022 09:24, Sudeep Holla wrote:
> > > > > On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
> > > > >> From: Conor Dooley <conor.dooley@microchip.com>
> > > > >>
> > > > >> RISC-V & arm64 both use an almost identical method of filling in
> > > > >> default vales for arch topology. Create a weakly defined default
> > > > >> implementation with the intent of migrating both archs to use it.
> > > > >>
> > > > >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > > >> ---
> > > > >>   drivers/base/arch_topology.c  | 19 +++++++++++++++++++
> > > > >>   include/linux/arch_topology.h |  1 +
> > > > >>   2 files changed, 20 insertions(+)
> > > > >>
> > > > >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> > > > >> index 441e14ac33a4..07e84c6ac5c2 100644
> > > > >> --- a/drivers/base/arch_topology.c
> > > > >> +++ b/drivers/base/arch_topology.c
> > > > >> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
> > > > >>    }
> > > > >>   }
> > > > >>
> > > > >> +void __weak store_cpu_topology(unsigned int cpuid)
> > > >
> > > > Does using __weak here make sense to you?
> > > >
> > >
> > > I don't want any weak definition and arch to override as we know only
> > > arm64 and RISC-V are the only users and they are aligned to have same
> > > implementation. So weak definition doesn't make sense to me.
> > >
> > > > >
> > > > > I prefer to have this as default implementation. So just get the risc-v
> > > > > one pushed to upstream first(for v5.20) and get all the backports if required.
> > > > > Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
> > > > >
> > > >
> > > > Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
> > > > and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
> > > > the risc-v impl. and then migrate both on IRC & he seemed happy with
> > > > it.
> > > >
> > >
> > > Ah OK, good.
> > >
> > > > If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
> > > >
> > >
> > > That would be great, thanks. You can most the code to move to generic from
> > > both arm64 and risc-v once we have this in v5.20-rc1
> > 
> > Why not ignore risc-v for now, and move the arm64 implementation to
> > the generic code for v5.20, so every arch will have it at once?
> >
> 
> We could but,
> 1. This arch_topology is new and has been going through lot of changes
>    recently and having code there might make it difficult to backport
>    changes that are required for RISC-V(my guess)

Worry about future issues in the future.  Make it simple now as you know
what you are dealing with at the moment.

thanks,

greg k-h

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Greg KH]

On Fri, Jul 08, 2022 at 10:47:10AM +0100, Sudeep Holla wrote:
> On Fri, Jul 08, 2022 at 11:28:19AM +0200, Geert Uytterhoeven wrote:
> > Hi Sudeep,
> > 
> > On Fri, Jul 8, 2022 at 11:22 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
> > > > On 08/07/2022 09:24, Sudeep Holla wrote:
> > > > > On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
> > > > >> From: Conor Dooley <conor.dooley@microchip.com>
> > > > >>
> > > > >> RISC-V & arm64 both use an almost identical method of filling in
> > > > >> default vales for arch topology. Create a weakly defined default
> > > > >> implementation with the intent of migrating both archs to use it.
> > > > >>
> > > > >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > > >> ---
> > > > >>   drivers/base/arch_topology.c  | 19 +++++++++++++++++++
> > > > >>   include/linux/arch_topology.h |  1 +
> > > > >>   2 files changed, 20 insertions(+)
> > > > >>
> > > > >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> > > > >> index 441e14ac33a4..07e84c6ac5c2 100644
> > > > >> --- a/drivers/base/arch_topology.c
> > > > >> +++ b/drivers/base/arch_topology.c
> > > > >> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
> > > > >>    }
> > > > >>   }
> > > > >>
> > > > >> +void __weak store_cpu_topology(unsigned int cpuid)
> > > >
> > > > Does using __weak here make sense to you?
> > > >
> > >
> > > I don't want any weak definition and arch to override as we know only
> > > arm64 and RISC-V are the only users and they are aligned to have same
> > > implementation. So weak definition doesn't make sense to me.
> > >
> > > > >
> > > > > I prefer to have this as default implementation. So just get the risc-v
> > > > > one pushed to upstream first(for v5.20) and get all the backports if required.
> > > > > Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
> > > > >
> > > >
> > > > Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
> > > > and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
> > > > the risc-v impl. and then migrate both on IRC & he seemed happy with
> > > > it.
> > > >
> > >
> > > Ah OK, good.
> > >
> > > > If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
> > > >
> > >
> > > That would be great, thanks. You can most the code to move to generic from
> > > both arm64 and risc-v once we have this in v5.20-rc1
> > 
> > Why not ignore risc-v for now, and move the arm64 implementation to
> > the generic code for v5.20, so every arch will have it at once?
> >
> 
> We could but,
> 1. This arch_topology is new and has been going through lot of changes
>    recently and having code there might make it difficult to backport
>    changes that are required for RISC-V(my guess)

Worry about future issues in the future.  Make it simple now as you know
what you are dealing with at the moment.

thanks,

greg k-h

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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Sudeep Holla]

On Fri, Jul 08, 2022 at 12:03:41PM +0200, Greg KH wrote:
> On Fri, Jul 08, 2022 at 10:47:10AM +0100, Sudeep Holla wrote:
> > On Fri, Jul 08, 2022 at 11:28:19AM +0200, Geert Uytterhoeven wrote:
> > > Hi Sudeep,
> > > 
> > > On Fri, Jul 8, 2022 at 11:22 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > > On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
> > > > > On 08/07/2022 09:24, Sudeep Holla wrote:
> > > > > > On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
> > > > > >> From: Conor Dooley <conor.dooley@microchip.com>
> > > > > >>
> > > > > >> RISC-V & arm64 both use an almost identical method of filling in
> > > > > >> default vales for arch topology. Create a weakly defined default
> > > > > >> implementation with the intent of migrating both archs to use it.
> > > > > >>
> > > > > >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > > > >> ---
> > > > > >>   drivers/base/arch_topology.c  | 19 +++++++++++++++++++
> > > > > >>   include/linux/arch_topology.h |  1 +
> > > > > >>   2 files changed, 20 insertions(+)
> > > > > >>
> > > > > >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> > > > > >> index 441e14ac33a4..07e84c6ac5c2 100644
> > > > > >> --- a/drivers/base/arch_topology.c
> > > > > >> +++ b/drivers/base/arch_topology.c
> > > > > >> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
> > > > > >>    }
> > > > > >>   }
> > > > > >>
> > > > > >> +void __weak store_cpu_topology(unsigned int cpuid)
> > > > >
> > > > > Does using __weak here make sense to you?
> > > > >
> > > >
> > > > I don't want any weak definition and arch to override as we know only
> > > > arm64 and RISC-V are the only users and they are aligned to have same
> > > > implementation. So weak definition doesn't make sense to me.
> > > >
> > > > > >
> > > > > > I prefer to have this as default implementation. So just get the risc-v
> > > > > > one pushed to upstream first(for v5.20) and get all the backports if required.
> > > > > > Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
> > > > > >
> > > > >
> > > > > Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
> > > > > and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
> > > > > the risc-v impl. and then migrate both on IRC & he seemed happy with
> > > > > it.
> > > > >
> > > >
> > > > Ah OK, good.
> > > >
> > > > > If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
> > > > >
> > > >
> > > > That would be great, thanks. You can most the code to move to generic from
> > > > both arm64 and risc-v once we have this in v5.20-rc1
> > > 
> > > Why not ignore risc-v for now, and move the arm64 implementation to
> > > the generic code for v5.20, so every arch will have it at once?
> > >
> > 
> > We could but,
> > 1. This arch_topology is new and has been going through lot of changes
> >    recently and having code there might make it difficult to backport
> >    changes that are required for RISC-V(my guess)
> 
> Worry about future issues in the future.  Make it simple now as you know
> what you are dealing with at the moment.
>

Sure, I was just suggesting and expecting someone from RISC-V community or
maintainers to make a call. As I said it is based on my understanding.
hence I have mentioned as guess. So I am not against it as such.


-- 
Regards,
Sudeep

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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Sudeep Holla]

On Fri, Jul 08, 2022 at 12:03:41PM +0200, Greg KH wrote:
> On Fri, Jul 08, 2022 at 10:47:10AM +0100, Sudeep Holla wrote:
> > On Fri, Jul 08, 2022 at 11:28:19AM +0200, Geert Uytterhoeven wrote:
> > > Hi Sudeep,
> > > 
> > > On Fri, Jul 8, 2022 at 11:22 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > > On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
> > > > > On 08/07/2022 09:24, Sudeep Holla wrote:
> > > > > > On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
> > > > > >> From: Conor Dooley <conor.dooley@microchip.com>
> > > > > >>
> > > > > >> RISC-V & arm64 both use an almost identical method of filling in
> > > > > >> default vales for arch topology. Create a weakly defined default
> > > > > >> implementation with the intent of migrating both archs to use it.
> > > > > >>
> > > > > >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > > > >> ---
> > > > > >>   drivers/base/arch_topology.c  | 19 +++++++++++++++++++
> > > > > >>   include/linux/arch_topology.h |  1 +
> > > > > >>   2 files changed, 20 insertions(+)
> > > > > >>
> > > > > >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> > > > > >> index 441e14ac33a4..07e84c6ac5c2 100644
> > > > > >> --- a/drivers/base/arch_topology.c
> > > > > >> +++ b/drivers/base/arch_topology.c
> > > > > >> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
> > > > > >>    }
> > > > > >>   }
> > > > > >>
> > > > > >> +void __weak store_cpu_topology(unsigned int cpuid)
> > > > >
> > > > > Does using __weak here make sense to you?
> > > > >
> > > >
> > > > I don't want any weak definition and arch to override as we know only
> > > > arm64 and RISC-V are the only users and they are aligned to have same
> > > > implementation. So weak definition doesn't make sense to me.
> > > >
> > > > > >
> > > > > > I prefer to have this as default implementation. So just get the risc-v
> > > > > > one pushed to upstream first(for v5.20) and get all the backports if required.
> > > > > > Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
> > > > > >
> > > > >
> > > > > Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
> > > > > and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
> > > > > the risc-v impl. and then migrate both on IRC & he seemed happy with
> > > > > it.
> > > > >
> > > >
> > > > Ah OK, good.
> > > >
> > > > > If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
> > > > >
> > > >
> > > > That would be great, thanks. You can most the code to move to generic from
> > > > both arm64 and risc-v once we have this in v5.20-rc1
> > > 
> > > Why not ignore risc-v for now, and move the arm64 implementation to
> > > the generic code for v5.20, so every arch will have it at once?
> > >
> > 
> > We could but,
> > 1. This arch_topology is new and has been going through lot of changes
> >    recently and having code there might make it difficult to backport
> >    changes that are required for RISC-V(my guess)
> 
> Worry about future issues in the future.  Make it simple now as you know
> what you are dealing with at the moment.
>

Sure, I was just suggesting and expecting someone from RISC-V community or
maintainers to make a call. As I said it is based on my understanding.
hence I have mentioned as guess. So I am not against it as such.


-- 
Regards,
Sudeep

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Sudeep Holla]

On Fri, Jul 08, 2022 at 12:03:41PM +0200, Greg KH wrote:
> On Fri, Jul 08, 2022 at 10:47:10AM +0100, Sudeep Holla wrote:
> > On Fri, Jul 08, 2022 at 11:28:19AM +0200, Geert Uytterhoeven wrote:
> > > Hi Sudeep,
> > > 
> > > On Fri, Jul 8, 2022 at 11:22 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > > On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
> > > > > On 08/07/2022 09:24, Sudeep Holla wrote:
> > > > > > On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
> > > > > >> From: Conor Dooley <conor.dooley@microchip.com>
> > > > > >>
> > > > > >> RISC-V & arm64 both use an almost identical method of filling in
> > > > > >> default vales for arch topology. Create a weakly defined default
> > > > > >> implementation with the intent of migrating both archs to use it.
> > > > > >>
> > > > > >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > > > >> ---
> > > > > >>   drivers/base/arch_topology.c  | 19 +++++++++++++++++++
> > > > > >>   include/linux/arch_topology.h |  1 +
> > > > > >>   2 files changed, 20 insertions(+)
> > > > > >>
> > > > > >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> > > > > >> index 441e14ac33a4..07e84c6ac5c2 100644
> > > > > >> --- a/drivers/base/arch_topology.c
> > > > > >> +++ b/drivers/base/arch_topology.c
> > > > > >> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
> > > > > >>    }
> > > > > >>   }
> > > > > >>
> > > > > >> +void __weak store_cpu_topology(unsigned int cpuid)
> > > > >
> > > > > Does using __weak here make sense to you?
> > > > >
> > > >
> > > > I don't want any weak definition and arch to override as we know only
> > > > arm64 and RISC-V are the only users and they are aligned to have same
> > > > implementation. So weak definition doesn't make sense to me.
> > > >
> > > > > >
> > > > > > I prefer to have this as default implementation. So just get the risc-v
> > > > > > one pushed to upstream first(for v5.20) and get all the backports if required.
> > > > > > Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
> > > > > >
> > > > >
> > > > > Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
> > > > > and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
> > > > > the risc-v impl. and then migrate both on IRC & he seemed happy with
> > > > > it.
> > > > >
> > > >
> > > > Ah OK, good.
> > > >
> > > > > If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
> > > > >
> > > >
> > > > That would be great, thanks. You can most the code to move to generic from
> > > > both arm64 and risc-v once we have this in v5.20-rc1
> > > 
> > > Why not ignore risc-v for now, and move the arm64 implementation to
> > > the generic code for v5.20, so every arch will have it at once?
> > >
> > 
> > We could but,
> > 1. This arch_topology is new and has been going through lot of changes
> >    recently and having code there might make it difficult to backport
> >    changes that are required for RISC-V(my guess)
> 
> Worry about future issues in the future.  Make it simple now as you know
> what you are dealing with at the moment.
>

Sure, I was just suggesting and expecting someone from RISC-V community or
maintainers to make a call. As I said it is based on my understanding.
hence I have mentioned as guess. So I am not against it as such.


-- 
Regards,
Sudeep

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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Conor.Dooley]

On 08/07/2022 12:39, Sudeep Holla wrote:
> On Fri, Jul 08, 2022 at 12:03:41PM +0200, Greg KH wrote:
>> On Fri, Jul 08, 2022 at 10:47:10AM +0100, Sudeep Holla wrote:
>>> On Fri, Jul 08, 2022 at 11:28:19AM +0200, Geert Uytterhoeven wrote:
>>>> Hi Sudeep,
>>>>
>>>> On Fri, Jul 8, 2022 at 11:22 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>>> On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
>>>>>> If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
>>>>>>
>>>>>
>>>>> That would be great, thanks. You can most the code to move to generic from
>>>>> both arm64 and risc-v once we have this in v5.20-rc1
>>>>
>>>> Why not ignore risc-v for now, and move the arm64 implementation to
>>>> the generic code for v5.20, so every arch will have it at once?
>>>>
>>>
>>> We could but,
>>> 1. This arch_topology is new and has been going through lot of changes
>>>     recently and having code there might make it difficult to backport
>>>     changes that are required for RISC-V(my guess)
>>
>> Worry about future issues in the future.  Make it simple now as you know
>> what you are dealing with at the moment.
>>
> 
> Sure, I was just suggesting and expecting someone from RISC-V community or
> maintainers to make a call. As I said it is based on my understanding.
> hence I have mentioned as guess. So I am not against it as such.

I did a little bit of poking in the git history.
The last code touching the arm implementation was:
3102bc0e6ac7 ("arm64: topology: Stop using MPIDR for topology information")
on Fri Oct 2 12:01:41 2020 +0100

The introduction of arch-topology stuff to RISC-V was:
03f11f03dbfe ("RISC-V: Parse cpu topology during boot.")
on Thu Jun 27 12:53:00 2019 -0700

Backporting as far as v5.10 should be no real effort and I don't think
to v5.4 that should be meaninfully harder. If 3102bc0e6ac7 hasn't been
backported already, maybe it should be since it appears to have been
fixing a problem too.

Based on that, I think doing this the straightforward way in the first
place is a better idea.

I'll respin the series as:
patch 1: Move arm64 to the generic implementation
patch 2: Make RISC-V use the generic implementation

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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Conor.Dooley]

On 08/07/2022 12:39, Sudeep Holla wrote:
> On Fri, Jul 08, 2022 at 12:03:41PM +0200, Greg KH wrote:
>> On Fri, Jul 08, 2022 at 10:47:10AM +0100, Sudeep Holla wrote:
>>> On Fri, Jul 08, 2022 at 11:28:19AM +0200, Geert Uytterhoeven wrote:
>>>> Hi Sudeep,
>>>>
>>>> On Fri, Jul 8, 2022 at 11:22 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>>> On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
>>>>>> If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
>>>>>>
>>>>>
>>>>> That would be great, thanks. You can most the code to move to generic from
>>>>> both arm64 and risc-v once we have this in v5.20-rc1
>>>>
>>>> Why not ignore risc-v for now, and move the arm64 implementation to
>>>> the generic code for v5.20, so every arch will have it at once?
>>>>
>>>
>>> We could but,
>>> 1. This arch_topology is new and has been going through lot of changes
>>>     recently and having code there might make it difficult to backport
>>>     changes that are required for RISC-V(my guess)
>>
>> Worry about future issues in the future.  Make it simple now as you know
>> what you are dealing with at the moment.
>>
> 
> Sure, I was just suggesting and expecting someone from RISC-V community or
> maintainers to make a call. As I said it is based on my understanding.
> hence I have mentioned as guess. So I am not against it as such.

I did a little bit of poking in the git history.
The last code touching the arm implementation was:
3102bc0e6ac7 ("arm64: topology: Stop using MPIDR for topology information")
on Fri Oct 2 12:01:41 2020 +0100

The introduction of arch-topology stuff to RISC-V was:
03f11f03dbfe ("RISC-V: Parse cpu topology during boot.")
on Thu Jun 27 12:53:00 2019 -0700

Backporting as far as v5.10 should be no real effort and I don't think
to v5.4 that should be meaninfully harder. If 3102bc0e6ac7 hasn't been
backported already, maybe it should be since it appears to have been
fixing a problem too.

Based on that, I think doing this the straightforward way in the first
place is a better idea.

I'll respin the series as:
patch 1: Move arm64 to the generic implementation
patch 2: Make RISC-V use the generic implementation

Thanks,
Conor.

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Conor.Dooley]

On 08/07/2022 12:39, Sudeep Holla wrote:
> On Fri, Jul 08, 2022 at 12:03:41PM +0200, Greg KH wrote:
>> On Fri, Jul 08, 2022 at 10:47:10AM +0100, Sudeep Holla wrote:
>>> On Fri, Jul 08, 2022 at 11:28:19AM +0200, Geert Uytterhoeven wrote:
>>>> Hi Sudeep,
>>>>
>>>> On Fri, Jul 8, 2022 at 11:22 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>>> On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
>>>>>> If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
>>>>>>
>>>>>
>>>>> That would be great, thanks. You can most the code to move to generic from
>>>>> both arm64 and risc-v once we have this in v5.20-rc1
>>>>
>>>> Why not ignore risc-v for now, and move the arm64 implementation to
>>>> the generic code for v5.20, so every arch will have it at once?
>>>>
>>>
>>> We could but,
>>> 1. This arch_topology is new and has been going through lot of changes
>>>     recently and having code there might make it difficult to backport
>>>     changes that are required for RISC-V(my guess)
>>
>> Worry about future issues in the future.  Make it simple now as you know
>> what you are dealing with at the moment.
>>
> 
> Sure, I was just suggesting and expecting someone from RISC-V community or
> maintainers to make a call. As I said it is based on my understanding.
> hence I have mentioned as guess. So I am not against it as such.

I did a little bit of poking in the git history.
The last code touching the arm implementation was:
3102bc0e6ac7 ("arm64: topology: Stop using MPIDR for topology information")
on Fri Oct 2 12:01:41 2020 +0100

The introduction of arch-topology stuff to RISC-V was:
03f11f03dbfe ("RISC-V: Parse cpu topology during boot.")
on Thu Jun 27 12:53:00 2019 -0700

Backporting as far as v5.10 should be no real effort and I don't think
to v5.4 that should be meaninfully harder. If 3102bc0e6ac7 hasn't been
backported already, maybe it should be since it appears to have been
fixing a problem too.

Based on that, I think doing this the straightforward way in the first
place is a better idea.

I'll respin the series as:
patch 1: Move arm64 to the generic implementation
patch 2: Make RISC-V use the generic implementation

Thanks,
Conor.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Sudeep Holla]

On Fri, Jul 08, 2022 at 11:57:05AM +0000, Conor.Dooley@microchip.com wrote:
> 
> I did a little bit of poking in the git history.
> The last code touching the arm implementation was:
> 3102bc0e6ac7 ("arm64: topology: Stop using MPIDR for topology information")
> on Fri Oct 2 12:01:41 2020 +0100
> 
> The introduction of arch-topology stuff to RISC-V was:
> 03f11f03dbfe ("RISC-V: Parse cpu topology during boot.")
> on Thu Jun 27 12:53:00 2019 -0700
> 
> Backporting as far as v5.10 should be no real effort and I don't think
> to v5.4 that should be meaninfully harder. If 3102bc0e6ac7 hasn't been
> backported already, maybe it should be since it appears to have been
> fixing a problem too.
>

Thanks for doing the research and sorry for the noise earlier.

> Based on that, I think doing this the straightforward way in the first
> place is a better idea.
> 
> I'll respin the series as:
> patch 1: Move arm64 to the generic implementation

I don't think the mpidr check we have there is of much use IMO. You can
drop that and see if arm64 maintainers and/or others agree. As you have
already figured, since 3102bc0e6ac7 we are not using MPIDR and the one
check we have is optional IMO. So you can either drop it or keep it as
in your RFC and then post updates.

-- 
Regards,
Sudeep

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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Sudeep Holla]

On Fri, Jul 08, 2022 at 11:57:05AM +0000, Conor.Dooley@microchip.com wrote:
> 
> I did a little bit of poking in the git history.
> The last code touching the arm implementation was:
> 3102bc0e6ac7 ("arm64: topology: Stop using MPIDR for topology information")
> on Fri Oct 2 12:01:41 2020 +0100
> 
> The introduction of arch-topology stuff to RISC-V was:
> 03f11f03dbfe ("RISC-V: Parse cpu topology during boot.")
> on Thu Jun 27 12:53:00 2019 -0700
> 
> Backporting as far as v5.10 should be no real effort and I don't think
> to v5.4 that should be meaninfully harder. If 3102bc0e6ac7 hasn't been
> backported already, maybe it should be since it appears to have been
> fixing a problem too.
>

Thanks for doing the research and sorry for the noise earlier.

> Based on that, I think doing this the straightforward way in the first
> place is a better idea.
> 
> I'll respin the series as:
> patch 1: Move arm64 to the generic implementation

I don't think the mpidr check we have there is of much use IMO. You can
drop that and see if arm64 maintainers and/or others agree. As you have
already figured, since 3102bc0e6ac7 we are not using MPIDR and the one
check we have is optional IMO. So you can either drop it or keep it as
in your RFC and then post updates.

-- 
Regards,
Sudeep

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Sudeep Holla]

On Fri, Jul 08, 2022 at 11:57:05AM +0000, Conor.Dooley@microchip.com wrote:
> 
> I did a little bit of poking in the git history.
> The last code touching the arm implementation was:
> 3102bc0e6ac7 ("arm64: topology: Stop using MPIDR for topology information")
> on Fri Oct 2 12:01:41 2020 +0100
> 
> The introduction of arch-topology stuff to RISC-V was:
> 03f11f03dbfe ("RISC-V: Parse cpu topology during boot.")
> on Thu Jun 27 12:53:00 2019 -0700
> 
> Backporting as far as v5.10 should be no real effort and I don't think
> to v5.4 that should be meaninfully harder. If 3102bc0e6ac7 hasn't been
> backported already, maybe it should be since it appears to have been
> fixing a problem too.
>

Thanks for doing the research and sorry for the noise earlier.

> Based on that, I think doing this the straightforward way in the first
> place is a better idea.
> 
> I'll respin the series as:
> patch 1: Move arm64 to the generic implementation

I don't think the mpidr check we have there is of much use IMO. You can
drop that and see if arm64 maintainers and/or others agree. As you have
already figured, since 3102bc0e6ac7 we are not using MPIDR and the one
check we have is optional IMO. So you can either drop it or keep it as
in your RFC and then post updates.

-- 
Regards,
Sudeep

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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Conor.Dooley]

On 08/07/2022 10:21, Sudeep Holla wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
>> On 08/07/2022 09:24, Sudeep Holla wrote:
>>> On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
>>>> From: Conor Dooley <conor.dooley@microchip.com>
>>>>
>>>> RISC-V & arm64 both use an almost identical method of filling in
>>>> default vales for arch topology. Create a weakly defined default
>>>> implementation with the intent of migrating both archs to use it.
>>>>
>>>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>>>> ---
>>>>    drivers/base/arch_topology.c  | 19 +++++++++++++++++++
>>>>    include/linux/arch_topology.h |  1 +
>>>>    2 files changed, 20 insertions(+)
>>>>
>>>> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
>>>> index 441e14ac33a4..07e84c6ac5c2 100644
>>>> --- a/drivers/base/arch_topology.c
>>>> +++ b/drivers/base/arch_topology.c
>>>> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
>>>>     }
>>>>    }
>>>>
>>>> +void __weak store_cpu_topology(unsigned int cpuid)
>>
>> Does using __weak here make sense to you?
>>
> 
> I don't want any weak definition and arch to override as we know only
> arm64 and RISC-V are the only users and they are aligned to have same
> implementation. So weak definition doesn't make sense to me.

Right. I had used __weak b/c I didn't know how to split the migration
into smaller patches per arch without breaking the build due to
multiple definitions of store_cpu_topology().

> 
>>>
>>> I prefer to have this as default implementation. So just get the risc-v
>>> one pushed to upstream first(for v5.20) and get all the backports if required.
>>> Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
>>>
>>
>> Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
>> and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
>> the risc-v impl. and then migrate both on IRC & he seemed happy with
>> it.
>>
> 
> Ah OK, good.
> 
>> If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
>>
> 
> That would be great, thanks. You can most the code to move to generic from
> both arm64 and risc-v once we have this in v5.20-rc1

Right, that sounds like a plan (well, pending geert's concerns).
Could I have your R-b on patch 1? The comments you made about
removing the duplicate function should be resolved.

Thanks,
Conor.

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

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Conor.Dooley]

On 08/07/2022 10:21, Sudeep Holla wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
>> On 08/07/2022 09:24, Sudeep Holla wrote:
>>> On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
>>>> From: Conor Dooley <conor.dooley@microchip.com>
>>>>
>>>> RISC-V & arm64 both use an almost identical method of filling in
>>>> default vales for arch topology. Create a weakly defined default
>>>> implementation with the intent of migrating both archs to use it.
>>>>
>>>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>>>> ---
>>>>    drivers/base/arch_topology.c  | 19 +++++++++++++++++++
>>>>    include/linux/arch_topology.h |  1 +
>>>>    2 files changed, 20 insertions(+)
>>>>
>>>> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
>>>> index 441e14ac33a4..07e84c6ac5c2 100644
>>>> --- a/drivers/base/arch_topology.c
>>>> +++ b/drivers/base/arch_topology.c
>>>> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
>>>>     }
>>>>    }
>>>>
>>>> +void __weak store_cpu_topology(unsigned int cpuid)
>>
>> Does using __weak here make sense to you?
>>
> 
> I don't want any weak definition and arch to override as we know only
> arm64 and RISC-V are the only users and they are aligned to have same
> implementation. So weak definition doesn't make sense to me.

Right. I had used __weak b/c I didn't know how to split the migration
into smaller patches per arch without breaking the build due to
multiple definitions of store_cpu_topology().

> 
>>>
>>> I prefer to have this as default implementation. So just get the risc-v
>>> one pushed to upstream first(for v5.20) and get all the backports if required.
>>> Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
>>>
>>
>> Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
>> and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
>> the risc-v impl. and then migrate both on IRC & he seemed happy with
>> it.
>>
> 
> Ah OK, good.
> 
>> If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
>>
> 
> That would be great, thanks. You can most the code to move to generic from
> both arm64 and risc-v once we have this in v5.20-rc1

Right, that sounds like a plan (well, pending geert's concerns).
Could I have your R-b on patch 1? The comments you made about
removing the duplicate function should be resolved.

Thanks,
Conor.

Re: [RFC 2/4] arch-topology: add a default implementation of store_cpu_topology()

[Conor.Dooley]

On 08/07/2022 10:21, Sudeep Holla wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Fri, Jul 08, 2022 at 08:35:57AM +0000, Conor.Dooley@microchip.com wrote:
>> On 08/07/2022 09:24, Sudeep Holla wrote:
>>> On Thu, Jul 07, 2022 at 11:04:35PM +0100, Conor Dooley wrote:
>>>> From: Conor Dooley <conor.dooley@microchip.com>
>>>>
>>>> RISC-V & arm64 both use an almost identical method of filling in
>>>> default vales for arch topology. Create a weakly defined default
>>>> implementation with the intent of migrating both archs to use it.
>>>>
>>>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>>>> ---
>>>>    drivers/base/arch_topology.c  | 19 +++++++++++++++++++
>>>>    include/linux/arch_topology.h |  1 +
>>>>    2 files changed, 20 insertions(+)
>>>>
>>>> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
>>>> index 441e14ac33a4..07e84c6ac5c2 100644
>>>> --- a/drivers/base/arch_topology.c
>>>> +++ b/drivers/base/arch_topology.c
>>>> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
>>>>     }
>>>>    }
>>>>
>>>> +void __weak store_cpu_topology(unsigned int cpuid)
>>
>> Does using __weak here make sense to you?
>>
> 
> I don't want any weak definition and arch to override as we know only
> arm64 and RISC-V are the only users and they are aligned to have same
> implementation. So weak definition doesn't make sense to me.

Right. I had used __weak b/c I didn't know how to split the migration
into smaller patches per arch without breaking the build due to
multiple definitions of store_cpu_topology().

> 
>>>
>>> I prefer to have this as default implementation. So just get the risc-v
>>> one pushed to upstream first(for v5.20) and get all the backports if required.
>>> Next cycle(i.e. v5.21), you can move both RISC-V and arm64.
>>>
>>
>> Yeah, that was my intention. I meant to label patch 1/4 as "PATCH"
>> and (2,3,4)/4 as RFC but forgot. I talked with Palmer about doing
>> the risc-v impl. and then migrate both on IRC & he seemed happy with
>> it.
>>
> 
> Ah OK, good.
> 
>> If you're okay with patch 1/4, I'll resubmit it as a standalone v2.
>>
> 
> That would be great, thanks. You can most the code to move to generic from
> both arm64 and risc-v once we have this in v5.20-rc1

Right, that sounds like a plan (well, pending geert's concerns).
Could I have your R-b on patch 1? The comments you made about
removing the duplicate function should be resolved.

Thanks,
Conor.

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

[RFC 3/4] riscv: arch-topology: move riscv to the generic store_cpu_topology()

[Conor Dooley]

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

The default implementation of store_cpu_topology() is exactly that
used by RISC-V so revert the portions of aaaabbbbccccdddd ("riscv:
arch-topology: fix default topology reporting") which add the arch
specific version.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/include/asm/topology.h | 13 -------------
 arch/riscv/kernel/Makefile        |  1 -
 arch/riscv/kernel/smpboot.c       |  1 -
 arch/riscv/kernel/topology.c      | 32 -------------------------------
 4 files changed, 47 deletions(-)
 delete mode 100644 arch/riscv/include/asm/topology.h
 delete mode 100644 arch/riscv/kernel/topology.c

diff --git a/arch/riscv/include/asm/topology.h b/arch/riscv/include/asm/topology.h
deleted file mode 100644
index 36bc6ecda898..000000000000
--- a/arch/riscv/include/asm/topology.h
+++ /dev/null
@@ -1,13 +0,0 @@
-/* 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 9518882ba6f9..c71d6591d539 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -50,7 +50,6 @@ 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 a8239b4b61f3..a1c861f84fe2 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -32,7 +32,6 @@
 #include <asm/sections.h>
 #include <asm/sbi.h>
 #include <asm/smp.h>
-#include <asm/topology.h>
 
 #include "head.h"
 
diff --git a/arch/riscv/kernel/topology.c b/arch/riscv/kernel/topology.c
deleted file mode 100644
index db72862bd5b5..000000000000
--- a/arch/riscv/kernel/topology.c
+++ /dev/null
@@ -1,32 +0,0 @@
-// 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);
-}
-- 
2.37.0


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

[RFC 3/4] riscv: arch-topology: move riscv to the generic store_cpu_topology()

[Conor Dooley]

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

The default implementation of store_cpu_topology() is exactly that
used by RISC-V so revert the portions of aaaabbbbccccdddd ("riscv:
arch-topology: fix default topology reporting") which add the arch
specific version.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/include/asm/topology.h | 13 -------------
 arch/riscv/kernel/Makefile        |  1 -
 arch/riscv/kernel/smpboot.c       |  1 -
 arch/riscv/kernel/topology.c      | 32 -------------------------------
 4 files changed, 47 deletions(-)
 delete mode 100644 arch/riscv/include/asm/topology.h
 delete mode 100644 arch/riscv/kernel/topology.c

diff --git a/arch/riscv/include/asm/topology.h b/arch/riscv/include/asm/topology.h
deleted file mode 100644
index 36bc6ecda898..000000000000
--- a/arch/riscv/include/asm/topology.h
+++ /dev/null
@@ -1,13 +0,0 @@
-/* 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 9518882ba6f9..c71d6591d539 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -50,7 +50,6 @@ 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 a8239b4b61f3..a1c861f84fe2 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -32,7 +32,6 @@
 #include <asm/sections.h>
 #include <asm/sbi.h>
 #include <asm/smp.h>
-#include <asm/topology.h>
 
 #include "head.h"
 
diff --git a/arch/riscv/kernel/topology.c b/arch/riscv/kernel/topology.c
deleted file mode 100644
index db72862bd5b5..000000000000
--- a/arch/riscv/kernel/topology.c
+++ /dev/null
@@ -1,32 +0,0 @@
-// 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);
-}
-- 
2.37.0

[RFC 3/4] riscv: arch-topology: move riscv to the generic store_cpu_topology()

[Conor Dooley]

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

The default implementation of store_cpu_topology() is exactly that
used by RISC-V so revert the portions of aaaabbbbccccdddd ("riscv:
arch-topology: fix default topology reporting") which add the arch
specific version.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/include/asm/topology.h | 13 -------------
 arch/riscv/kernel/Makefile        |  1 -
 arch/riscv/kernel/smpboot.c       |  1 -
 arch/riscv/kernel/topology.c      | 32 -------------------------------
 4 files changed, 47 deletions(-)
 delete mode 100644 arch/riscv/include/asm/topology.h
 delete mode 100644 arch/riscv/kernel/topology.c

diff --git a/arch/riscv/include/asm/topology.h b/arch/riscv/include/asm/topology.h
deleted file mode 100644
index 36bc6ecda898..000000000000
--- a/arch/riscv/include/asm/topology.h
+++ /dev/null
@@ -1,13 +0,0 @@
-/* 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 9518882ba6f9..c71d6591d539 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -50,7 +50,6 @@ 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 a8239b4b61f3..a1c861f84fe2 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -32,7 +32,6 @@
 #include <asm/sections.h>
 #include <asm/sbi.h>
 #include <asm/smp.h>
-#include <asm/topology.h>
 
 #include "head.h"
 
diff --git a/arch/riscv/kernel/topology.c b/arch/riscv/kernel/topology.c
deleted file mode 100644
index db72862bd5b5..000000000000
--- a/arch/riscv/kernel/topology.c
+++ /dev/null
@@ -1,32 +0,0 @@
-// 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);
-}
-- 
2.37.0


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

[RFC 4/4] arm64: arch-topology move arm64 to the generic store_cpu_topology()

[Conor Dooley]

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

The default implementation of store_cpu_topology() was derived from the
arm64 implementation, but with the mpidr bits removed. Extract the mpidr
bits from the arch implementation to the callsites & use the generic
version.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/arm64/kernel/smp.c      | 16 +++++++++++++--
 arch/arm64/kernel/topology.c | 40 ------------------------------------
 2 files changed, 14 insertions(+), 42 deletions(-)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 62ed361a4376..9e8acaa4c2f7 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -234,7 +234,12 @@ asmlinkage notrace void secondary_start_kernel(void)
 	 * Log the CPU info before it is marked online and might get read.
 	 */
 	cpuinfo_store_cpu();
-	store_cpu_topology(cpu);
+
+	/*
+	 * Uniprocessor systems can rely on default topology values
+	 */
+	if (!(mpidr & MPIDR_UP_BITMASK))
+		store_cpu_topology(cpu);
 
 	/*
 	 * Enable GIC and timers.
@@ -719,6 +724,7 @@ void __init smp_init_cpus(void)
 void __init smp_prepare_cpus(unsigned int max_cpus)
 {
 	const struct cpu_operations *ops;
+	u64 mpidr;
 	int err;
 	unsigned int cpu;
 	unsigned int this_cpu;
@@ -726,7 +732,13 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 	init_cpu_topology();
 
 	this_cpu = smp_processor_id();
-	store_cpu_topology(this_cpu);
+	mpidr = read_cpuid_mpidr();
+
+	/*
+	 * Uniprocessor systems can rely on default topology values
+	 */
+	if (!(mpidr & MPIDR_UP_BITMASK))
+		store_cpu_topology(this_cpu);
 	numa_store_cpu_info(this_cpu);
 	numa_add_cpu(this_cpu);
 
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 869ffc4d4484..7889a00f5487 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -22,46 +22,6 @@
 #include <asm/cputype.h>
 #include <asm/topology.h>
 
-void store_cpu_topology(unsigned int cpuid)
-{
-	struct cpu_topology *cpuid_topo = &cpu_topology[cpuid];
-	u64 mpidr;
-
-	if (cpuid_topo->package_id != -1)
-		goto topology_populated;
-
-	mpidr = read_cpuid_mpidr();
-
-	/* Uniprocessor systems can rely on default topology values */
-	if (mpidr & MPIDR_UP_BITMASK)
-		return;
-
-	/*
-	 * This would be the place to create cpu topology based on MPIDR.
-	 *
-	 * However, it cannot be trusted to depict the actual topology; some
-	 * pieces of the architecture enforce an artificial cap on Aff0 values
-	 * (e.g. GICv3's ICC_SGI1R_EL1 limits it to 15), leading to an
-	 * artificial cycling of Aff1, Aff2 and Aff3 values. IOW, these end up
-	 * having absolutely no relationship to the actual underlying system
-	 * topology, and cannot be reasonably used as core / package ID.
-	 *
-	 * If the MT bit is set, Aff0 *could* be used to define a thread ID, but
-	 * we still wouldn't be able to obtain a sane core ID. This means we
-	 * need to entirely ignore MPIDR for any topology deduction.
-	 */
-	cpuid_topo->thread_id  = -1;
-	cpuid_topo->core_id    = cpuid;
-	cpuid_topo->package_id = cpu_to_node(cpuid);
-
-	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);
-
-topology_populated:
-	update_siblings_masks(cpuid);
-}
-
 #ifdef CONFIG_ACPI
 static bool __init acpi_cpu_is_threaded(int cpu)
 {
-- 
2.37.0


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

[RFC 4/4] arm64: arch-topology move arm64 to the generic store_cpu_topology()

[Conor Dooley]

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

The default implementation of store_cpu_topology() was derived from the
arm64 implementation, but with the mpidr bits removed. Extract the mpidr
bits from the arch implementation to the callsites & use the generic
version.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/arm64/kernel/smp.c      | 16 +++++++++++++--
 arch/arm64/kernel/topology.c | 40 ------------------------------------
 2 files changed, 14 insertions(+), 42 deletions(-)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 62ed361a4376..9e8acaa4c2f7 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -234,7 +234,12 @@ asmlinkage notrace void secondary_start_kernel(void)
 	 * Log the CPU info before it is marked online and might get read.
 	 */
 	cpuinfo_store_cpu();
-	store_cpu_topology(cpu);
+
+	/*
+	 * Uniprocessor systems can rely on default topology values
+	 */
+	if (!(mpidr & MPIDR_UP_BITMASK))
+		store_cpu_topology(cpu);
 
 	/*
 	 * Enable GIC and timers.
@@ -719,6 +724,7 @@ void __init smp_init_cpus(void)
 void __init smp_prepare_cpus(unsigned int max_cpus)
 {
 	const struct cpu_operations *ops;
+	u64 mpidr;
 	int err;
 	unsigned int cpu;
 	unsigned int this_cpu;
@@ -726,7 +732,13 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 	init_cpu_topology();
 
 	this_cpu = smp_processor_id();
-	store_cpu_topology(this_cpu);
+	mpidr = read_cpuid_mpidr();
+
+	/*
+	 * Uniprocessor systems can rely on default topology values
+	 */
+	if (!(mpidr & MPIDR_UP_BITMASK))
+		store_cpu_topology(this_cpu);
 	numa_store_cpu_info(this_cpu);
 	numa_add_cpu(this_cpu);
 
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 869ffc4d4484..7889a00f5487 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -22,46 +22,6 @@
 #include <asm/cputype.h>
 #include <asm/topology.h>
 
-void store_cpu_topology(unsigned int cpuid)
-{
-	struct cpu_topology *cpuid_topo = &cpu_topology[cpuid];
-	u64 mpidr;
-
-	if (cpuid_topo->package_id != -1)
-		goto topology_populated;
-
-	mpidr = read_cpuid_mpidr();
-
-	/* Uniprocessor systems can rely on default topology values */
-	if (mpidr & MPIDR_UP_BITMASK)
-		return;
-
-	/*
-	 * This would be the place to create cpu topology based on MPIDR.
-	 *
-	 * However, it cannot be trusted to depict the actual topology; some
-	 * pieces of the architecture enforce an artificial cap on Aff0 values
-	 * (e.g. GICv3's ICC_SGI1R_EL1 limits it to 15), leading to an
-	 * artificial cycling of Aff1, Aff2 and Aff3 values. IOW, these end up
-	 * having absolutely no relationship to the actual underlying system
-	 * topology, and cannot be reasonably used as core / package ID.
-	 *
-	 * If the MT bit is set, Aff0 *could* be used to define a thread ID, but
-	 * we still wouldn't be able to obtain a sane core ID. This means we
-	 * need to entirely ignore MPIDR for any topology deduction.
-	 */
-	cpuid_topo->thread_id  = -1;
-	cpuid_topo->core_id    = cpuid;
-	cpuid_topo->package_id = cpu_to_node(cpuid);
-
-	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);
-
-topology_populated:
-	update_siblings_masks(cpuid);
-}
-
 #ifdef CONFIG_ACPI
 static bool __init acpi_cpu_is_threaded(int cpu)
 {
-- 
2.37.0

[RFC 4/4] arm64: arch-topology move arm64 to the generic store_cpu_topology()

[Conor Dooley]

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

The default implementation of store_cpu_topology() was derived from the
arm64 implementation, but with the mpidr bits removed. Extract the mpidr
bits from the arch implementation to the callsites & use the generic
version.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/arm64/kernel/smp.c      | 16 +++++++++++++--
 arch/arm64/kernel/topology.c | 40 ------------------------------------
 2 files changed, 14 insertions(+), 42 deletions(-)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 62ed361a4376..9e8acaa4c2f7 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -234,7 +234,12 @@ asmlinkage notrace void secondary_start_kernel(void)
 	 * Log the CPU info before it is marked online and might get read.
 	 */
 	cpuinfo_store_cpu();
-	store_cpu_topology(cpu);
+
+	/*
+	 * Uniprocessor systems can rely on default topology values
+	 */
+	if (!(mpidr & MPIDR_UP_BITMASK))
+		store_cpu_topology(cpu);
 
 	/*
 	 * Enable GIC and timers.
@@ -719,6 +724,7 @@ void __init smp_init_cpus(void)
 void __init smp_prepare_cpus(unsigned int max_cpus)
 {
 	const struct cpu_operations *ops;
+	u64 mpidr;
 	int err;
 	unsigned int cpu;
 	unsigned int this_cpu;
@@ -726,7 +732,13 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 	init_cpu_topology();
 
 	this_cpu = smp_processor_id();
-	store_cpu_topology(this_cpu);
+	mpidr = read_cpuid_mpidr();
+
+	/*
+	 * Uniprocessor systems can rely on default topology values
+	 */
+	if (!(mpidr & MPIDR_UP_BITMASK))
+		store_cpu_topology(this_cpu);
 	numa_store_cpu_info(this_cpu);
 	numa_add_cpu(this_cpu);
 
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 869ffc4d4484..7889a00f5487 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -22,46 +22,6 @@
 #include <asm/cputype.h>
 #include <asm/topology.h>
 
-void store_cpu_topology(unsigned int cpuid)
-{
-	struct cpu_topology *cpuid_topo = &cpu_topology[cpuid];
-	u64 mpidr;
-
-	if (cpuid_topo->package_id != -1)
-		goto topology_populated;
-
-	mpidr = read_cpuid_mpidr();
-
-	/* Uniprocessor systems can rely on default topology values */
-	if (mpidr & MPIDR_UP_BITMASK)
-		return;
-
-	/*
-	 * This would be the place to create cpu topology based on MPIDR.
-	 *
-	 * However, it cannot be trusted to depict the actual topology; some
-	 * pieces of the architecture enforce an artificial cap on Aff0 values
-	 * (e.g. GICv3's ICC_SGI1R_EL1 limits it to 15), leading to an
-	 * artificial cycling of Aff1, Aff2 and Aff3 values. IOW, these end up
-	 * having absolutely no relationship to the actual underlying system
-	 * topology, and cannot be reasonably used as core / package ID.
-	 *
-	 * If the MT bit is set, Aff0 *could* be used to define a thread ID, but
-	 * we still wouldn't be able to obtain a sane core ID. This means we
-	 * need to entirely ignore MPIDR for any topology deduction.
-	 */
-	cpuid_topo->thread_id  = -1;
-	cpuid_topo->core_id    = cpuid;
-	cpuid_topo->package_id = cpu_to_node(cpuid);
-
-	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);
-
-topology_populated:
-	update_siblings_masks(cpuid);
-}
-
 #ifdef CONFIG_ACPI
 static bool __init acpi_cpu_is_threaded(int cpu)
 {
-- 
2.37.0


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

Re: [PATCH/RFC 0/4] Fix RISC-V's arch-topology reporting

[Conor.Dooley]

On 07/07/2022 23:04, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> Hey all,
> It's my first time messing around with arch/ code at all, let alone
> more than one arch, so forgive me if I have screwed up how to do a
> migration like this.
> 
> The goal here is the fix the incorrectly reported arch topology on
> RISC-V which seems to have been broken since it was added.
> cpu, package and thread IDs are all currently reported as -1, so tools
> like lstopo think systems have multiple threads on the same core when
> this is not true:
> https://github.com/open-mpi/hwloc/issues/536
> 
> Because I want to backport that fix, I implemented store_cpu_topology
> for RISC-V before migrating to the generic version which seems to have
> just complicated things, but was Sudeep's preferred way of doing it.
> 
> Palmer, I have not marked the first patch as RFC because I would like
> it to be taken as a fix for 5.19-rc(late) independently of the rest if
> possible.

Apparently I forgot to swap RFC for PATCH on it, but point still stands.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH/RFC 0/4] Fix RISC-V's arch-topology reporting

[Conor.Dooley]

On 07/07/2022 23:04, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> Hey all,
> It's my first time messing around with arch/ code at all, let alone
> more than one arch, so forgive me if I have screwed up how to do a
> migration like this.
> 
> The goal here is the fix the incorrectly reported arch topology on
> RISC-V which seems to have been broken since it was added.
> cpu, package and thread IDs are all currently reported as -1, so tools
> like lstopo think systems have multiple threads on the same core when
> this is not true:
> https://github.com/open-mpi/hwloc/issues/536
> 
> Because I want to backport that fix, I implemented store_cpu_topology
> for RISC-V before migrating to the generic version which seems to have
> just complicated things, but was Sudeep's preferred way of doing it.
> 
> Palmer, I have not marked the first patch as RFC because I would like
> it to be taken as a fix for 5.19-rc(late) independently of the rest if
> possible.

Apparently I forgot to swap RFC for PATCH on it, but point still stands.

Re: [PATCH/RFC 0/4] Fix RISC-V's arch-topology reporting

[Conor.Dooley]

On 07/07/2022 23:04, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> Hey all,
> It's my first time messing around with arch/ code at all, let alone
> more than one arch, so forgive me if I have screwed up how to do a
> migration like this.
> 
> The goal here is the fix the incorrectly reported arch topology on
> RISC-V which seems to have been broken since it was added.
> cpu, package and thread IDs are all currently reported as -1, so tools
> like lstopo think systems have multiple threads on the same core when
> this is not true:
> https://github.com/open-mpi/hwloc/issues/536
> 
> Because I want to backport that fix, I implemented store_cpu_topology
> for RISC-V before migrating to the generic version which seems to have
> just complicated things, but was Sudeep's preferred way of doing it.
> 
> Palmer, I have not marked the first patch as RFC because I would like
> it to be taken as a fix for 5.19-rc(late) independently of the rest if
> possible.

Apparently I forgot to swap RFC for PATCH on it, but point still stands.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel