| Message ID | 20190320234806.19748-1-atish.patra@wdc.com |
|---|---|
| Headers | show |
| Series | Unify CPU topology across ARM & RISC-V | expand |
On 3/20/19 4:48 PM, Atish Patra wrote: > The cpu-map DT entry in ARM can describe the CPU topology in much better > way compared to other existing approaches. RISC-V can easily adopt this > binding to represent its own CPU topology. Thus, both cpu-map DT > binding and topology parsing code can be moved to a common location so > that RISC-V or any other architecture can leverage that. > > The relevant discussion regarding unifying cpu topology can be found in > [1]. > > arch_topology seems to be a perfect place to move the common code. I > have not introduced any significant functional changes in the moved code. > The only downside in this approach is that the capacity code will be > executed for RISC-V as well. But, it will exit immediately after not > able to find the appropriate DT node. If the overhead is considered too > much, we can always compile out capacity related functions under a > different config for the architectures that do not support them. > > There was an opportunity to unify topology data structure for ARM32 done > by patch 3/4. But, I refrained from making any other changes as I am not > very well versed with original intention for some functions that > are present in arch_topology.c. I hope this patch series can be served > as a baseline for such changes in the future. > > The patches have been tested for RISC-V and compile tested for ARM64, > ARM32 & x86. > > The socket change[2] is also now part of this series. > > [1] https://lkml.org/lkml/2018/11/6/19 > [2] https://lkml.org/lkml/2018/11/7/918 > > QEMU changes for RISC-V topology are available at > > https://github.com/atishp04/qemu/tree/riscv_topology_dt > > HiFive Unleashed DT with topology node is available here. > https://github.com/atishp04/opensbi/tree/HiFive_unleashed_topology > > It can be verified with OpenSBI with following additional compile time > option. > > FW_PAYLOAD_FDT="unleashed_topology.dtb" > > Changes from v2->v3 > 1. Cover letter update with experiment DT for topology changes. > 2. Added the patch for [2]. > > Changes from v1->v2 > 1. ARM32 can now use the common code as well. > > Atish Patra (4): > dt-binding: cpu-topology: Move cpu-map to a common binding. > cpu-topology: Move cpu topology code to common code. > arm: Use common cpu_topology > RISC-V: Parse cpu topology during boot. > > Sudeep Holla (1): > Documentation: DT: arm: add support for sockets defining package > boundaries > > .../topology.txt => cpu/cpu-topology.txt} | 134 ++++++-- > arch/arm/include/asm/topology.h | 22 +- > arch/arm/kernel/topology.c | 10 +- > arch/arm64/include/asm/topology.h | 23 -- > arch/arm64/kernel/topology.c | 303 +----------------- > arch/riscv/Kconfig | 1 + > arch/riscv/kernel/smpboot.c | 3 + > drivers/base/arch_topology.c | 298 ++++++++++++++++- > drivers/base/topology.c | 1 + > include/linux/arch_topology.h | 36 +++ > 10 files changed, 453 insertions(+), 378 deletions(-) > rename Documentation/devicetree/bindings/{arm/topology.txt => cpu/cpu-topology.txt} (66%) > > -- > 2.21.0 > > Ping? Specifically, patch 3 & 4 affects ARM & ARM64. Any tests on real hardware would be great. Regards, Atish
On Wed, Apr 10, 2019 at 03:49:54PM -0700, Atish Patra wrote: [...] > > Ping? > Sorry for the delay(was off for a while), will take a look next week. > Specifically, patch 3 & 4 affects ARM & ARM64. Any tests on real hardware > would be great. > Sure. -- Regards, Sudeep
Hi Atish, Thanks again for doing this. Overall changes look good except a couple of minor nit, see below. On Wed, Mar 20, 2019 at 04:48:04PM -0700, Atish Patra wrote: > Both RISC-V & ARM64 are using cpu-map device tree to describe > their cpu topology. It's better to move the relevant code to > a common place instead of duplicate code. > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> > --- > arch/arm64/include/asm/topology.h | 23 --- > arch/arm64/kernel/topology.c | 303 +----------------------------- > drivers/base/arch_topology.c | 298 ++++++++++++++++++++++++++++- > drivers/base/topology.c | 1 + > include/linux/arch_topology.h | 28 +++ > 5 files changed, 330 insertions(+), 323 deletions(-) > [...] > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > index edfcf8d9..6cc6a860 100644 > --- a/drivers/base/arch_topology.c > +++ b/drivers/base/arch_topology.c > @@ -6,8 +6,8 @@ > * Written by: Juri Lelli, ARM Ltd. > */ > > -#include <linux/acpi.h> > #include <linux/arch_topology.h> > +#include <linux/acpi.h> > #include <linux/cpu.h> > #include <linux/cpufreq.h> > #include <linux/device.h> > @@ -16,6 +16,11 @@ > #include <linux/string.h> > #include <linux/sched/topology.h> > #include <linux/cpuset.h> > +#include <linux/cpumask.h> > +#include <linux/init.h> > +#include <linux/percpu.h> > +#include <linux/sched.h> > +#include <linux/smp.h> > > DEFINE_PER_CPU(unsigned long, freq_scale) = SCHED_CAPACITY_SCALE; > > @@ -278,3 +283,294 @@ static void parsing_done_workfn(struct work_struct *work) > #else > core_initcall(free_raw_capacity); > #endif > + > +#if defined(CONFIG_ARM64) || defined(CONFIG_RISCV) Why can't the above one be just GENERIC_ARCH_TOPOLOGY ? I may be missing to find it myself, but would like to know. > + > +#if defined(CONFIG_ARM64) || defined(CONFIG_RISCV) Ditto. -- Regards, Sudeep
On Wed, Mar 20, 2019 at 04:48:05PM -0700, Atish Patra wrote: > Currently, ARM32 and ARM64 uses different data structures to > represent their cpu toplogies. Since, we are moving the ARM64 > topology to common code to be used by other architectures, we > can reuse that for ARM32 as well. > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > --- > arch/arm/include/asm/topology.h | 22 +--------------------- > arch/arm/kernel/topology.c | 10 +++++----- > include/linux/arch_topology.h | 10 +++++++++- > 3 files changed, 15 insertions(+), 27 deletions(-) > [...] > diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h > index d4e76e0a..7c850611 100644 > --- a/include/linux/arch_topology.h > +++ b/include/linux/arch_topology.h > @@ -36,17 +36,25 @@ unsigned long topology_get_freq_scale(int cpu) > struct cpu_topology { > int thread_id; > int core_id; > +#ifdef CONFIG_ARM_CPU_TOPOLOGY > + int socket_id; Sorry, but I can't find any reason why we need to do this ifdef dance here, especially for socket_id vs package_id ? Other's I can understand as there are new, but I am sure we can find a way and get away with #ifdefery here completely. > +#else > int package_id; > int llc_id; > + cpumask_t llc_sibling; > +#endif > cpumask_t thread_sibling; > cpumask_t core_sibling; > - cpumask_t llc_sibling; > }; > > #ifdef CONFIG_GENERIC_ARCH_TOPOLOGY > extern struct cpu_topology cpu_topology[NR_CPUS]; > > +#ifdef CONFIG_ARM_CPU_TOPOLOGY > +#define topology_physical_package_id(cpu) (cpu_topology[cpu].socket_id) > +#else > #define topology_physical_package_id(cpu) (cpu_topology[cpu].package_id) > +#endif Since all callsites must use topology_physical_package_id, we should be able to rename socket_id to package_id easily. -- Regards, Sudeep
On 4/15/19 8:31 AM, Sudeep Holla wrote: > On Wed, Mar 20, 2019 at 04:48:05PM -0700, Atish Patra wrote: >> Currently, ARM32 and ARM64 uses different data structures to >> represent their cpu toplogies. Since, we are moving the ARM64 >> topology to common code to be used by other architectures, we >> can reuse that for ARM32 as well. >> >> Signed-off-by: Atish Patra <atish.patra@wdc.com> >> --- >> arch/arm/include/asm/topology.h | 22 +--------------------- >> arch/arm/kernel/topology.c | 10 +++++----- >> include/linux/arch_topology.h | 10 +++++++++- >> 3 files changed, 15 insertions(+), 27 deletions(-) >> > > [...] > >> diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h >> index d4e76e0a..7c850611 100644 >> --- a/include/linux/arch_topology.h >> +++ b/include/linux/arch_topology.h >> @@ -36,17 +36,25 @@ unsigned long topology_get_freq_scale(int cpu) >> struct cpu_topology { >> int thread_id; >> int core_id; >> +#ifdef CONFIG_ARM_CPU_TOPOLOGY >> + int socket_id; > > Sorry, but I can't find any reason why we need to do this ifdef dance > here, especially for socket_id vs package_id ? I was not sure if we can rename socket_id to package_id from a semantic point of view. If you are okay with it, I will change it to package_id and send a v4. Other's I can understand > as there are new, but I am sure we can find a way and get away with > #ifdefery here completely. > That would be good. Any suggestions on how to do that? >> +#else >> int package_id; >> int llc_id; >> + cpumask_t llc_sibling; >> +#endif >> cpumask_t thread_sibling; >> cpumask_t core_sibling; >> - cpumask_t llc_sibling; >> }; >> >> #ifdef CONFIG_GENERIC_ARCH_TOPOLOGY >> extern struct cpu_topology cpu_topology[NR_CPUS]; >> >> +#ifdef CONFIG_ARM_CPU_TOPOLOGY >> +#define topology_physical_package_id(cpu) (cpu_topology[cpu].socket_id) >> +#else >> #define topology_physical_package_id(cpu) (cpu_topology[cpu].package_id) >> +#endif > > Since all callsites must use topology_physical_package_id, we should be > able to rename socket_id to package_id easily. > Sure. Regards, Atish > -- > Regards, > Sudeep >
On 4/15/19 8:27 AM, Sudeep Holla wrote: > Hi Atish, > > Thanks again for doing this. Overall changes look good except a couple > of minor nit, see below. > > On Wed, Mar 20, 2019 at 04:48:04PM -0700, Atish Patra wrote: >> Both RISC-V & ARM64 are using cpu-map device tree to describe >> their cpu topology. It's better to move the relevant code to >> a common place instead of duplicate code. >> >> Signed-off-by: Atish Patra <atish.patra@wdc.com> >> Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> >> --- >> arch/arm64/include/asm/topology.h | 23 --- >> arch/arm64/kernel/topology.c | 303 +----------------------------- >> drivers/base/arch_topology.c | 298 ++++++++++++++++++++++++++++- >> drivers/base/topology.c | 1 + >> include/linux/arch_topology.h | 28 +++ >> 5 files changed, 330 insertions(+), 323 deletions(-) >> > > [...] > >> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c >> index edfcf8d9..6cc6a860 100644 >> --- a/drivers/base/arch_topology.c >> +++ b/drivers/base/arch_topology.c >> @@ -6,8 +6,8 @@ >> * Written by: Juri Lelli, ARM Ltd. >> */ >> >> -#include <linux/acpi.h> >> #include <linux/arch_topology.h> >> +#include <linux/acpi.h> >> #include <linux/cpu.h> >> #include <linux/cpufreq.h> >> #include <linux/device.h> >> @@ -16,6 +16,11 @@ >> #include <linux/string.h> >> #include <linux/sched/topology.h> >> #include <linux/cpuset.h> >> +#include <linux/cpumask.h> >> +#include <linux/init.h> >> +#include <linux/percpu.h> >> +#include <linux/sched.h> >> +#include <linux/smp.h> >> >> DEFINE_PER_CPU(unsigned long, freq_scale) = SCHED_CAPACITY_SCALE; >> >> @@ -278,3 +283,294 @@ static void parsing_done_workfn(struct work_struct *work) >> #else >> core_initcall(free_raw_capacity); >> #endif >> + >> +#if defined(CONFIG_ARM64) || defined(CONFIG_RISCV) > > Why can't the above one be just GENERIC_ARCH_TOPOLOGY ? > I may be missing to find it myself, but would like to know. > GENERIC_ARCH_TOPOLOGY is now used for both RISCV, ARM & ARM64. The below functions under this #ifdef have different implementation for ARM and ARM64. parse_dt_topology cpu_coregroup_mask update_siblings_masks While we can combine the later two functions and move them to common code as well, parse_dt_topology is significantly different. That's why we need some kind of #ifdef or renaming of parse_dt_topology for ARM32 code. Thanks for the review!! Regards, Atish >> + >> +#if defined(CONFIG_ARM64) || defined(CONFIG_RISCV) > > Ditto. > > -- > Regards, > Sudeep >
On Mon, Apr 15, 2019 at 03:08:45PM -0700, Atish Patra wrote: > On 4/15/19 8:27 AM, Sudeep Holla wrote: > > Hi Atish, > > > > Thanks again for doing this. Overall changes look good except a couple > > of minor nit, see below. > > > > On Wed, Mar 20, 2019 at 04:48:04PM -0700, Atish Patra wrote: > > > Both RISC-V & ARM64 are using cpu-map device tree to describe > > > their cpu topology. It's better to move the relevant code to > > > a common place instead of duplicate code. > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > > Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> > > > --- > > > arch/arm64/include/asm/topology.h | 23 --- > > > arch/arm64/kernel/topology.c | 303 +----------------------------- > > > drivers/base/arch_topology.c | 298 ++++++++++++++++++++++++++++- > > > drivers/base/topology.c | 1 + > > > include/linux/arch_topology.h | 28 +++ > > > 5 files changed, 330 insertions(+), 323 deletions(-) > > > > > > > [...] > > > > > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > > > index edfcf8d9..6cc6a860 100644 > > > --- a/drivers/base/arch_topology.c > > > +++ b/drivers/base/arch_topology.c > > > @@ -6,8 +6,8 @@ > > > * Written by: Juri Lelli, ARM Ltd. > > > */ > > > -#include <linux/acpi.h> > > > #include <linux/arch_topology.h> > > > +#include <linux/acpi.h> > > > #include <linux/cpu.h> > > > #include <linux/cpufreq.h> > > > #include <linux/device.h> > > > @@ -16,6 +16,11 @@ > > > #include <linux/string.h> > > > #include <linux/sched/topology.h> > > > #include <linux/cpuset.h> > > > +#include <linux/cpumask.h> > > > +#include <linux/init.h> > > > +#include <linux/percpu.h> > > > +#include <linux/sched.h> > > > +#include <linux/smp.h> > > > DEFINE_PER_CPU(unsigned long, freq_scale) = SCHED_CAPACITY_SCALE; > > > @@ -278,3 +283,294 @@ static void parsing_done_workfn(struct work_struct *work) > > > #else > > > core_initcall(free_raw_capacity); > > > #endif > > > + > > > +#if defined(CONFIG_ARM64) || defined(CONFIG_RISCV) > > > > Why can't the above one be just GENERIC_ARCH_TOPOLOGY ? > > I may be missing to find it myself, but would like to know. > > > GENERIC_ARCH_TOPOLOGY is now used for both RISCV, ARM & ARM64. > The below functions under this #ifdef have different implementation for ARM > and ARM64. > > parse_dt_topology > cpu_coregroup_mask > update_siblings_masks > > While we can combine the later two functions and move them to common code as > well, parse_dt_topology is significantly different. > Sure, had a quick glance and indeed they may look different, but won't it defeat the purpose of this binding consolidation ? > That's why we need some kind of #ifdef or renaming of parse_dt_topology for > ARM32 code. > I am fine if we want to take this up later to keep the impact minimum. But cpu_coregroup_mask and update_siblings_masks can and must be unified. In fact the existing generic version must work on ARM32 too. > Thanks for the review!! > You are welcome. -- Regards, Sudeep
On 4/16/19 6:23 AM, Sudeep Holla wrote: > On Mon, Apr 15, 2019 at 03:08:45PM -0700, Atish Patra wrote: >> On 4/15/19 8:27 AM, Sudeep Holla wrote: >>> Hi Atish, >>> >>> Thanks again for doing this. Overall changes look good except a couple >>> of minor nit, see below. >>> >>> On Wed, Mar 20, 2019 at 04:48:04PM -0700, Atish Patra wrote: >>>> Both RISC-V & ARM64 are using cpu-map device tree to describe >>>> their cpu topology. It's better to move the relevant code to >>>> a common place instead of duplicate code. >>>> >>>> Signed-off-by: Atish Patra <atish.patra@wdc.com> >>>> Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> >>>> --- >>>> arch/arm64/include/asm/topology.h | 23 --- >>>> arch/arm64/kernel/topology.c | 303 +----------------------------- >>>> drivers/base/arch_topology.c | 298 ++++++++++++++++++++++++++++- >>>> drivers/base/topology.c | 1 + >>>> include/linux/arch_topology.h | 28 +++ >>>> 5 files changed, 330 insertions(+), 323 deletions(-) >>>> >>> >>> [...] >>> >>>> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c >>>> index edfcf8d9..6cc6a860 100644 >>>> --- a/drivers/base/arch_topology.c >>>> +++ b/drivers/base/arch_topology.c >>>> @@ -6,8 +6,8 @@ >>>> * Written by: Juri Lelli, ARM Ltd. >>>> */ >>>> -#include <linux/acpi.h> >>>> #include <linux/arch_topology.h> >>>> +#include <linux/acpi.h> >>>> #include <linux/cpu.h> >>>> #include <linux/cpufreq.h> >>>> #include <linux/device.h> >>>> @@ -16,6 +16,11 @@ >>>> #include <linux/string.h> >>>> #include <linux/sched/topology.h> >>>> #include <linux/cpuset.h> >>>> +#include <linux/cpumask.h> >>>> +#include <linux/init.h> >>>> +#include <linux/percpu.h> >>>> +#include <linux/sched.h> >>>> +#include <linux/smp.h> >>>> DEFINE_PER_CPU(unsigned long, freq_scale) = SCHED_CAPACITY_SCALE; >>>> @@ -278,3 +283,294 @@ static void parsing_done_workfn(struct work_struct *work) >>>> #else >>>> core_initcall(free_raw_capacity); >>>> #endif >>>> + >>>> +#if defined(CONFIG_ARM64) || defined(CONFIG_RISCV) >>> >>> Why can't the above one be just GENERIC_ARCH_TOPOLOGY ? >>> I may be missing to find it myself, but would like to know. >>> >> GENERIC_ARCH_TOPOLOGY is now used for both RISCV, ARM & ARM64. >> The below functions under this #ifdef have different implementation for ARM >> and ARM64. >> >> parse_dt_topology >> cpu_coregroup_mask >> update_siblings_masks >> >> While we can combine the later two functions and move them to common code as >> well, parse_dt_topology is significantly different. >> > > Sure, had a quick glance and indeed they may look different, but won't > it defeat the purpose of this binding consolidation ? > I didn't want change too much at first go. >> That's why we need some kind of #ifdef or renaming of parse_dt_topology for >> ARM32 code. >> > > I am fine if we want to take this up later to keep the impact minimum. > But cpu_coregroup_mask and update_siblings_masks can and must be unified. Sure. I will just leave parse_dt_topology as it is for now and unify other two functions. I think we should unify parse_dt_topology in separate series. Regards, Atish > In fact the existing generic version must work on ARM32 too. > >> Thanks for the review!! >> > > You are welcome. > > -- > Regards, > Sudeep >
On 4/16/19 6:09 AM, Sudeep Holla wrote: > On Mon, Apr 15, 2019 at 02:16:43PM -0700, Atish Patra wrote: >> On 4/15/19 8:31 AM, Sudeep Holla wrote: >>> On Wed, Mar 20, 2019 at 04:48:05PM -0700, Atish Patra wrote: >>>> Currently, ARM32 and ARM64 uses different data structures to >>>> represent their cpu toplogies. Since, we are moving the ARM64 >>>> topology to common code to be used by other architectures, we >>>> can reuse that for ARM32 as well. >>>> >>>> Signed-off-by: Atish Patra <atish.patra@wdc.com> >>>> --- >>>> arch/arm/include/asm/topology.h | 22 +--------------------- >>>> arch/arm/kernel/topology.c | 10 +++++----- >>>> include/linux/arch_topology.h | 10 +++++++++- >>>> 3 files changed, 15 insertions(+), 27 deletions(-) >>>> >>> >>> [...] >>> >>>> diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h >>>> index d4e76e0a..7c850611 100644 >>>> --- a/include/linux/arch_topology.h >>>> +++ b/include/linux/arch_topology.h >>>> @@ -36,17 +36,25 @@ unsigned long topology_get_freq_scale(int cpu) >>>> struct cpu_topology { >>>> int thread_id; >>>> int core_id; >>>> +#ifdef CONFIG_ARM_CPU_TOPOLOGY >>>> + int socket_id; >>> >>> Sorry, but I can't find any reason why we need to do this ifdef dance >>> here, especially for socket_id vs package_id ? >> >> I was not sure if we can rename socket_id to package_id from a semantic >> point of view. If you are okay with it, I will change it to package_id and >> send a v4. >> > > Thanks, all make sure to cc linux-arm-kernel@lists.infradead.org, > just noticed that's missing and you are asking for testing on ARM > platforms :) > My bad!! I didn't realize that linux-arm-kernel is not included. Thanks for pointing that out. >> Other's I can understand >>> as there are new, but I am sure we can find a way and get away with >>> #ifdefery here completely. >>> >> That would be good. Any suggestions on how to do that? >> > > Do you see any issues having extra structure members for ARM ? > Something like below seem to compile + boot fine on my 32-bit TC2 with > proper topology info on top of your series. Of course, more testing is > better, but I don't see any issue keeping llc_{id,sibling} around for > ARM eliminating the need for #ifdefs > I thought adding unused members for ARM32 might be unacceptable :). I will update my v4 with this. Regards, Atish > Let me know if I am missing something. > > -->8 > > diff --git i/arch/arm/kernel/topology.c w/arch/arm/kernel/topology.c > index 0ddb24c76c17..f2aa942e0cfa 100644 > --- i/arch/arm/kernel/topology.c > +++ w/arch/arm/kernel/topology.c > @@ -206,7 +206,7 @@ void update_siblings_masks(unsigned int cpuid) > for_each_possible_cpu(cpu) { > cpu_topo = &cpu_topology[cpu]; > > - if (cpuid_topo->socket_id != cpu_topo->socket_id) > + if (cpuid_topo->package_id != cpu_topo->package_id) > continue; > > cpumask_set_cpu(cpuid, &cpu_topo->core_sibling); > @@ -250,12 +250,12 @@ void store_cpu_topology(unsigned int cpuid) > /* core performance interdependency */ > cpuid_topo->thread_id = MPIDR_AFFINITY_LEVEL(mpidr, 0); > cpuid_topo->core_id = MPIDR_AFFINITY_LEVEL(mpidr, 1); > - cpuid_topo->socket_id = MPIDR_AFFINITY_LEVEL(mpidr, 2); > + cpuid_topo->package_id = MPIDR_AFFINITY_LEVEL(mpidr, 2); > } else { > /* largely independent cores */ > cpuid_topo->thread_id = -1; > cpuid_topo->core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0); > - cpuid_topo->socket_id = MPIDR_AFFINITY_LEVEL(mpidr, 1); > + cpuid_topo->package_id = MPIDR_AFFINITY_LEVEL(mpidr, 1); > } > } else { > /* > @@ -265,7 +265,7 @@ void store_cpu_topology(unsigned int cpuid) > */ > cpuid_topo->thread_id = -1; > cpuid_topo->core_id = 0; > - cpuid_topo->socket_id = -1; > + cpuid_topo->package_id = -1; > } > > update_siblings_masks(cpuid); > @@ -275,7 +275,7 @@ void store_cpu_topology(unsigned int cpuid) > pr_info("CPU%u: thread %d, cpu %d, socket %d, mpidr %x\n", > cpuid, cpu_topology[cpuid].thread_id, > cpu_topology[cpuid].core_id, > - cpu_topology[cpuid].socket_id, mpidr); > + cpu_topology[cpuid].package_id, mpidr); > } > > static inline int cpu_corepower_flags(void) > @@ -306,7 +306,7 @@ void __init init_cpu_topology(void) > > cpu_topo->thread_id = -1; > cpu_topo->core_id = -1; > - cpu_topo->socket_id = -1; > + cpu_topo->package_id = -1; > cpumask_clear(&cpu_topo->core_sibling); > cpumask_clear(&cpu_topo->thread_sibling); > } > diff --git i/include/linux/arch_topology.h w/include/linux/arch_topology.h > index 7c850611986d..8e82389c2bed 100644 > --- i/include/linux/arch_topology.h > +++ w/include/linux/arch_topology.h > @@ -36,13 +36,9 @@ unsigned long topology_get_freq_scale(int cpu) > struct cpu_topology { > int thread_id; > int core_id; > -#ifdef CONFIG_ARM_CPU_TOPOLOGY > - int socket_id; > -#else > int package_id; > int llc_id; > cpumask_t llc_sibling; > -#endif > cpumask_t thread_sibling; > cpumask_t core_sibling; > }; > @@ -50,11 +46,7 @@ struct cpu_topology { > #ifdef CONFIG_GENERIC_ARCH_TOPOLOGY > extern struct cpu_topology cpu_topology[NR_CPUS]; > > -#ifdef CONFIG_ARM_CPU_TOPOLOGY > -#define topology_physical_package_id(cpu) (cpu_topology[cpu].socket_id) > -#else > #define topology_physical_package_id(cpu) (cpu_topology[cpu].package_id) > -#endif > #define topology_core_id(cpu) (cpu_topology[cpu].core_id) > #define topology_core_cpumask(cpu) (&cpu_topology[cpu].core_sibling) > #define topology_sibling_cpumask(cpu) (&cpu_topology[cpu].thread_sibling) >
The cpu-map DT entry in ARM can describe the CPU topology in much better way compared to other existing approaches. RISC-V can easily adopt this binding to represent its own CPU topology. Thus, both cpu-map DT binding and topology parsing code can be moved to a common location so that RISC-V or any other architecture can leverage that. The relevant discussion regarding unifying cpu topology can be found in [1]. arch_topology seems to be a perfect place to move the common code. I have not introduced any significant functional changes in the moved code. The only downside in this approach is that the capacity code will be executed for RISC-V as well. But, it will exit immediately after not able to find the appropriate DT node. If the overhead is considered too much, we can always compile out capacity related functions under a different config for the architectures that do not support them. There was an opportunity to unify topology data structure for ARM32 done by patch 3/4. But, I refrained from making any other changes as I am not very well versed with original intention for some functions that are present in arch_topology.c. I hope this patch series can be served as a baseline for such changes in the future. The patches have been tested for RISC-V and compile tested for ARM64, ARM32 & x86. The socket change[2] is also now part of this series. [1] https://lkml.org/lkml/2018/11/6/19 [2] https://lkml.org/lkml/2018/11/7/918 QEMU changes for RISC-V topology are available at https://github.com/atishp04/qemu/tree/riscv_topology_dt HiFive Unleashed DT with topology node is available here. https://github.com/atishp04/opensbi/tree/HiFive_unleashed_topology It can be verified with OpenSBI with following additional compile time option. FW_PAYLOAD_FDT="unleashed_topology.dtb" Changes from v2->v3 1. Cover letter update with experiment DT for topology changes. 2. Added the patch for [2]. Changes from v1->v2 1. ARM32 can now use the common code as well. Atish Patra (4): dt-binding: cpu-topology: Move cpu-map to a common binding. cpu-topology: Move cpu topology code to common code. arm: Use common cpu_topology RISC-V: Parse cpu topology during boot. Sudeep Holla (1): Documentation: DT: arm: add support for sockets defining package boundaries .../topology.txt => cpu/cpu-topology.txt} | 134 ++++++-- arch/arm/include/asm/topology.h | 22 +- arch/arm/kernel/topology.c | 10 +- arch/arm64/include/asm/topology.h | 23 -- arch/arm64/kernel/topology.c | 303 +----------------- arch/riscv/Kconfig | 1 + arch/riscv/kernel/smpboot.c | 3 + drivers/base/arch_topology.c | 298 ++++++++++++++++- drivers/base/topology.c | 1 + include/linux/arch_topology.h | 36 +++ 10 files changed, 453 insertions(+), 378 deletions(-) rename Documentation/devicetree/bindings/{arm/topology.txt => cpu/cpu-topology.txt} (66%) -- 2.21.0