[RFT PATCH v1 0/4] Unify CPU topology across ARM64 & RISC-V

[RFT PATCH v1 0/4] Unify CPU topology across ARM64 & RISC-V

[Atish Patra]

The cpu-map DT entry in ARM64 can describe the CPU topology in
much better way compared to other existing approaches. RISC-V can
easily adopt this binding to represent it's 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 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.

The patches have been tested for RISC-V and compile tested for
ARM64 & 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/riscv-qemu/tree/cpu_topo

Apologies for the previous patch series with incorrect title and
was sent only to kernel mailing list due to a bug in my config.
Please ignore that.

Atish Patra (3):
dt-binding: cpu-topology: Move cpu-map to a common binding.
cpu-topology: Move cpu topology code to common code.
RISC-V: Parse cpu topology during boot.

Sudeep Holla (1):
Documentation: DT: arm: add support for sockets defining package
boundaries

.../{arm/topology.txt => cpu/cpu-topology.txt}     | 133 +++++++--
arch/arm64/include/asm/topology.h                  |  22 --
arch/arm64/kernel/topology.c                       | 303 +--------------------
arch/riscv/Kconfig                                 |   1 +
arch/riscv/kernel/smpboot.c                        |   3 +
drivers/base/arch_topology.c                       | 294 ++++++++++++++++++++
include/linux/arch_topology.h                      |  26 ++
include/linux/topology.h                           |   1 +
8 files changed, 435 insertions(+), 348 deletions(-)
rename Documentation/devicetree/bindings/{arm/topology.txt => cpu/cpu-topology.txt} (66%)

--
2.7.4

[RFT PATCH v1 1/4] Documentation: DT: arm: add support for sockets defining package boundaries

[Atish Patra]

From: Sudeep Holla <sudeep.holla@arm.com>

The current ARM DT topology description provides the operating system
with a topological view of the system that is based on leaf nodes
representing either cores or threads (in an SMT system) and a
hierarchical set of cluster nodes that creates a hierarchical topology
view of how those cores and threads are grouped.

However this hierarchical representation of clusters does not allow to
describe what topology level actually represents the physical package or
the socket boundary, which is a key piece of information to be used by
an operating system to optimize resource allocation and scheduling.

Lets add a new "socket" node type in the cpu-map node to describe the
same.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 Documentation/devicetree/bindings/arm/topology.txt | 52 ++++++++++++++++------
 1 file changed, 39 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/arm/topology.txt
index de9eb048..66848355 100644
--- a/Documentation/devicetree/bindings/arm/topology.txt
+++ b/Documentation/devicetree/bindings/arm/topology.txt
@@ -9,6 +9,7 @@ ARM topology binding description
 In an ARM system, the hierarchy of CPUs is defined through three entities that
 are used to describe the layout of physical CPUs in the system:
 
+- socket
 - cluster
 - core
 - thread
@@ -63,21 +64,23 @@ nodes are listed.
 
 	The cpu-map node's child nodes can be:
 
-	- one or more cluster nodes
+	- one or more cluster nodes or
+	- one or more socket nodes in a multi-socket system
 
 	Any other configuration is considered invalid.
 
-The cpu-map node can only contain three types of child nodes:
+The cpu-map node can only contain 4 types of child nodes:
 
+- socket node
 - cluster node
 - core node
 - thread node
 
 whose bindings are described in paragraph 3.
 
-The nodes describing the CPU topology (cluster/core/thread) can only
-be defined within the cpu-map node and every core/thread in the system
-must be defined within the topology.  Any other configuration is
+The nodes describing the CPU topology (socket/cluster/core/thread) can
+only be defined within the cpu-map node and every core/thread in the
+system must be defined within the topology.  Any other configuration is
 invalid and therefore must be ignored.
 
 ===========================================
@@ -85,26 +88,44 @@ invalid and therefore must be ignored.
 ===========================================
 
 cpu-map child nodes must follow a naming convention where the node name
-must be "clusterN", "coreN", "threadN" depending on the node type (ie
-cluster/core/thread) (where N = {0, 1, ...} is the node number; nodes which
-are siblings within a single common parent node must be given a unique and
+must be "socketN", "clusterN", "coreN", "threadN" depending on the node type
+(ie socket/cluster/core/thread) (where N = {0, 1, ...} is the node number; nodes
+which are siblings within a single common parent node must be given a unique and
 sequential N value, starting from 0).
 cpu-map child nodes which do not share a common parent node can have the same
 name (ie same number N as other cpu-map child nodes at different device tree
 levels) since name uniqueness will be guaranteed by the device tree hierarchy.
 
 ===========================================
-3 - cluster/core/thread node bindings
+3 - socket/cluster/core/thread node bindings
 ===========================================
 
-Bindings for cluster/cpu/thread nodes are defined as follows:
+Bindings for socket/cluster/cpu/thread nodes are defined as follows:
+
+- socket node
+
+	 Description: must be declared within a cpu-map node, one node
+		      per physical socket in the system. A system can
+		      contain single or multiple physical socket.
+		      The association of sockets and NUMA nodes is beyond
+		      the scope of this bindings, please refer [2] for
+		      NUMA bindings.
+
+	This node is optional for a single socket system.
+
+	The socket node name must be "socketN" as described in 2.1 above.
+	A socket node can not be a leaf node.
+
+	A socket node's child nodes must be one or more cluster nodes.
+
+	Any other configuration is considered invalid.
 
 - cluster node
 
 	 Description: must be declared within a cpu-map node, one node
 		      per cluster. A system can contain several layers of
-		      clustering and cluster nodes can be contained in parent
-		      cluster nodes.
+		      clustering within a single physical socket and cluster
+		      nodes can be contained in parent cluster nodes.
 
 	The cluster node name must be "clusterN" as described in 2.1 above.
 	A cluster node can not be a leaf node.
@@ -164,13 +185,15 @@ Bindings for cluster/cpu/thread nodes are defined as follows:
 4 - Example dts
 ===========================================
 
-Example 1 (ARM 64-bit, 16-cpu system, two clusters of clusters):
+Example 1 (ARM 64-bit, 16-cpu system, two clusters of clusters in a single
+physical socket):
 
 cpus {
 	#size-cells = <0>;
 	#address-cells = <2>;
 
 	cpu-map {
+		socket0 {
 			cluster0 {
 				cluster0 {
 					core0 {
@@ -253,6 +276,7 @@ cpus {
 				};
 			};
 		};
+	};
 
 	CPU0: cpu@0 {
 		device_type = "cpu";
@@ -473,3 +497,5 @@ cpus {
 ===============================================================================
 [1] ARM Linux kernel documentation
     Documentation/devicetree/bindings/arm/cpus.txt
+[2] Devicetree NUMA binding description
+    Documentation/devicetree/bindings/numa.txt
-- 
2.7.4

Re: [RFT PATCH v1 1/4] Documentation: DT: arm: add support for sockets defining package boundaries

[Sudeep Holla]

On Thu, Nov 29, 2018 at 03:28:17PM -0800, Atish Patra wrote:
> From: Sudeep Holla <sudeep.holla@arm.com>
> 
> The current ARM DT topology description provides the operating system
> with a topological view of the system that is based on leaf nodes
> representing either cores or threads (in an SMT system) and a
> hierarchical set of cluster nodes that creates a hierarchical topology
> view of how those cores and threads are grouped.
> 
> However this hierarchical representation of clusters does not allow to
> describe what topology level actually represents the physical package or
> the socket boundary, which is a key piece of information to be used by
> an operating system to optimize resource allocation and scheduling.
> 
> Lets add a new "socket" node type in the cpu-map node to describe the
> same.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Thanks for picking this up and putting it as part of your unification
work. I will try to test the series and review it soon.

--
Regards,
Sudeep

Re: [RFT PATCH v1 1/4] Documentation: DT: arm: add support for sockets defining package boundaries

[Rob Herring]

On Thu, 29 Nov 2018 15:28:17 -0800, Atish Patra wrote:
> From: Sudeep Holla <sudeep.holla@arm.com>
> 
> The current ARM DT topology description provides the operating system
> with a topological view of the system that is based on leaf nodes
> representing either cores or threads (in an SMT system) and a
> hierarchical set of cluster nodes that creates a hierarchical topology
> view of how those cores and threads are grouped.
> 
> However this hierarchical representation of clusters does not allow to
> describe what topology level actually represents the physical package or
> the socket boundary, which is a key piece of information to be used by
> an operating system to optimize resource allocation and scheduling.
> 
> Lets add a new "socket" node type in the cpu-map node to describe the
> same.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  Documentation/devicetree/bindings/arm/topology.txt | 52 ++++++++++++++++------
>  1 file changed, 39 insertions(+), 13 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[RFT PATCH v1 2/4] dt-binding: cpu-topology: Move cpu-map to a common binding.

[Atish Patra]

cpu-map binding can be used to described cpu topology for both
RISC-V & ARM. It makes more sense to move the binding to document
to a common place.

The relevant discussion can be found here.
https://lkml.org/lkml/2018/11/6/19

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 .../{arm/topology.txt => cpu/cpu-topology.txt}     | 81 ++++++++++++++++++----
 1 file changed, 67 insertions(+), 14 deletions(-)
 rename Documentation/devicetree/bindings/{arm/topology.txt => cpu/cpu-topology.txt} (86%)

diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/cpu/cpu-topology.txt
similarity index 86%
rename from Documentation/devicetree/bindings/arm/topology.txt
rename to Documentation/devicetree/bindings/cpu/cpu-topology.txt
index 66848355..1de6fbce 100644
--- a/Documentation/devicetree/bindings/arm/topology.txt
+++ b/Documentation/devicetree/bindings/cpu/cpu-topology.txt
@@ -1,12 +1,12 @@
 ===========================================
-ARM topology binding description
+CPU topology binding description
 ===========================================
 
 ===========================================
 1 - Introduction
 ===========================================
 
-In an ARM system, the hierarchy of CPUs is defined through three entities that
+In a SMP system, the hierarchy of CPUs is defined through three entities that
 are used to describe the layout of physical CPUs in the system:
 
 - socket
@@ -14,9 +14,6 @@ are used to describe the layout of physical CPUs in the system:
 - core
 - thread
 
-The cpu nodes (bindings defined in [1]) represent the devices that
-correspond to physical CPUs and are to be mapped to the hierarchy levels.
-
 The bottom hierarchy level sits at core or thread level depending on whether
 symmetric multi-threading (SMT) is supported or not.
 
@@ -25,33 +22,37 @@ threads existing in the system and map to the hierarchy level "thread" above.
 In systems where SMT is not supported "cpu" nodes represent all cores present
 in the system and map to the hierarchy level "core" above.
 
-ARM topology bindings allow one to associate cpu nodes with hierarchical groups
+CPU topology bindings allow one to associate cpu nodes with hierarchical groups
 corresponding to the system hierarchy; syntactically they are defined as device
 tree nodes.
 
-The remainder of this document provides the topology bindings for ARM, based
-on the Devicetree Specification, available from:
+Currently, only ARM/RISC-V intend to use this cpu topology binding but it may be
+used for any other architecture as well.
 
-https://www.devicetree.org/specifications/
+The remainder of this document provides the topology bindings for ARM/RISC-V, based
+on the Devicetree Specification, available at [4].
+
+The cpu nodes (bindings defined in [1] for ARM or [2] for RISC-V) represent the devices that
+correspond to physical CPUs and are to be mapped to the hierarchy levels.
 
 If not stated otherwise, whenever a reference to a cpu node phandle is made its
 value must point to a cpu node compliant with the cpu node bindings as
-documented in [1].
+documented in [1] or [3] for respective ISA.
 A topology description containing phandles to cpu nodes that are not compliant
-with bindings standardized in [1] is therefore considered invalid.
+with bindings standardized in [1] or [3] is therefore considered invalid.
 
 ===========================================
 2 - cpu-map node
 ===========================================
 
-The ARM CPU topology is defined within the cpu-map node, which is a direct
+The ARM/RISC-V CPU topology is defined within the cpu-map node, which is a direct
 child of the cpus node and provides a container where the actual topology
 nodes are listed.
 
 - cpu-map node
 
-	Usage: Optional - On ARM SMP systems provide CPUs topology to the OS.
-			  ARM uniprocessor systems do not require a topology
+	Usage: Optional - On SMP systems provide CPUs topology to the OS.
+			  Uniprocessor systems do not require a topology
 			  description and therefore should not define a
 			  cpu-map node.
 
@@ -494,8 +495,60 @@ cpus {
 	};
 };
 
+Example 3: HiFive Unleashed (RISC-V 64 bit, 4 core system)
+
+cpus {
+	#address-cells = <2>;
+	#size-cells = <2>;
+	compatible = "sifive,fu540g", "sifive,fu500";
+	model = "sifive,hifive-unleashed-a00";
+
+	...
+
+	cpu-map {
+		cluster0 {
+			core0 {
+				cpu = <&L12>;
+		 	};
+			core1 {
+				cpu = <&L15>;
+			};
+			core2 {
+				cpu0 = <&L18>;
+			};
+			core3 {
+				cpu0 = <&L21>;
+			};
+		};
+ 	};
+
+	L12: cpu@1 {
+		device_type = "cpu";
+		compatible = "sifive,rocket0", "riscv";
+		reg = <0x1>;
+	}
+
+	L15: cpu@2 {
+		device_type = "cpu";
+		compatible = "sifive,rocket0", "riscv";
+		reg = <0x2>;
+	}
+	L18: cpu@3 {
+		device_type = "cpu";
+		compatible = "sifive,rocket0", "riscv";
+		reg = <0x3>;
+	}
+	L21: cpu@4 {
+		device_type = "cpu";
+		compatible = "sifive,rocket0", "riscv";
+		reg = <0x4>;
+	}
+};
 ===============================================================================
 [1] ARM Linux kernel documentation
     Documentation/devicetree/bindings/arm/cpus.txt
 [2] Devicetree NUMA binding description
     Documentation/devicetree/bindings/numa.txt
+[3] RISC-V Linux kernel documentation
+    Documentation/devicetree/bindings/riscv/cpus.txt
+[4] https://www.devicetree.org/specifications/
-- 
2.7.4

Re: [RFT PATCH v1 2/4] dt-binding: cpu-topology: Move cpu-map to a common binding.

[Sudeep Holla]

On Thu, Nov 29, 2018 at 03:28:18PM -0800, Atish Patra wrote:
> cpu-map binding can be used to described cpu topology for both
> RISC-V & ARM. It makes more sense to move the binding to document
> to a common place.
>
> The relevant discussion can be found here.
> https://lkml.org/lkml/2018/11/6/19
>

Looks good to me apart from a minor query below in the example.

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  .../{arm/topology.txt => cpu/cpu-topology.txt}     | 81 ++++++++++++++++++----
>  1 file changed, 67 insertions(+), 14 deletions(-)
>  rename Documentation/devicetree/bindings/{arm/topology.txt => cpu/cpu-topology.txt} (86%)
>
> diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/cpu/cpu-topology.txt
> similarity index 86%
> rename from Documentation/devicetree/bindings/arm/topology.txt
> rename to Documentation/devicetree/bindings/cpu/cpu-topology.txt
> index 66848355..1de6fbce 100644
> --- a/Documentation/devicetree/bindings/arm/topology.txt
> +++ b/Documentation/devicetree/bindings/cpu/cpu-topology.txt

[...]

> +Example 3: HiFive Unleashed (RISC-V 64 bit, 4 core system)
> +
> +cpus {
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +	compatible = "sifive,fu540g", "sifive,fu500";
> +	model = "sifive,hifive-unleashed-a00";
> +
> +	...
> +
> +	cpu-map {
> +		cluster0 {
> +			core0 {
> +				cpu = <&L12>;
> +		 	};
> +			core1 {
> +				cpu = <&L15>;
> +			};
> +			core2 {
> +				cpu0 = <&L18>;
> +			};
> +			core3 {
> +				cpu0 = <&L21>;
> +			};
> +		};
> + 	};
> +
> +	L12: cpu@1 {
> +		device_type = "cpu";
> +		compatible = "sifive,rocket0", "riscv";
> +		reg = <0x1>;
> +	}
> +
> +	L15: cpu@2 {
> +		device_type = "cpu";
> +		compatible = "sifive,rocket0", "riscv";
> +		reg = <0x2>;
> +	}
> +	L18: cpu@3 {
> +		device_type = "cpu";
> +		compatible = "sifive,rocket0", "riscv";
> +		reg = <0x3>;
> +	}
> +	L21: cpu@4 {
> +		device_type = "cpu";
> +		compatible = "sifive,rocket0", "riscv";
> +		reg = <0x4>;
> +	}
> +};

The labels for the CPUs drew my attention. Is it intentionally random
(or even specific) or just chosen to show anything can be used as labels ?
The reason I ask is people tend to copy from existing DT or examples
like here and so want to make sure if it can be kept as generic as
possible in the example. Just my opinion and I am fine if you want to
keep it as is, thought of checking the intentions here.

--
Regards,
Sudeep

Re: [RFT PATCH v1 2/4] dt-binding: cpu-topology: Move cpu-map to a common binding.

[Atish Patra]

On 12/3/18 8:55 AM, Sudeep Holla wrote:
> On Thu, Nov 29, 2018 at 03:28:18PM -0800, Atish Patra wrote:
>> cpu-map binding can be used to described cpu topology for both
>> RISC-V & ARM. It makes more sense to move the binding to document
>> to a common place.
>>
>> The relevant discussion can be found here.
>> https://lkml.org/lkml/2018/11/6/19
>>
> 
> Looks good to me apart from a minor query below in the example.
> 
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> 
>> Signed-off-by: Atish Patra <atish.patra@wdc.com>
>> ---
>>   .../{arm/topology.txt => cpu/cpu-topology.txt}     | 81 ++++++++++++++++++----
>>   1 file changed, 67 insertions(+), 14 deletions(-)
>>   rename Documentation/devicetree/bindings/{arm/topology.txt => cpu/cpu-topology.txt} (86%)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/cpu/cpu-topology.txt
>> similarity index 86%
>> rename from Documentation/devicetree/bindings/arm/topology.txt
>> rename to Documentation/devicetree/bindings/cpu/cpu-topology.txt
>> index 66848355..1de6fbce 100644
>> --- a/Documentation/devicetree/bindings/arm/topology.txt
>> +++ b/Documentation/devicetree/bindings/cpu/cpu-topology.txt
> 
> [...]
> 
>> +Example 3: HiFive Unleashed (RISC-V 64 bit, 4 core system)
>> +
>> +cpus {
>> +	#address-cells = <2>;
>> +	#size-cells = <2>;
>> +	compatible = "sifive,fu540g", "sifive,fu500";
>> +	model = "sifive,hifive-unleashed-a00";
>> +
>> +	...
>> +
>> +	cpu-map {
>> +		cluster0 {
>> +			core0 {
>> +				cpu = <&L12>;
>> +		 	};
>> +			core1 {
>> +				cpu = <&L15>;
>> +			};
>> +			core2 {
>> +				cpu0 = <&L18>;
>> +			};
>> +			core3 {
>> +				cpu0 = <&L21>;
>> +			};
>> +		};
>> + 	};
>> +
>> +	L12: cpu@1 {
>> +		device_type = "cpu";
>> +		compatible = "sifive,rocket0", "riscv";
>> +		reg = <0x1>;
>> +	}
>> +
>> +	L15: cpu@2 {
>> +		device_type = "cpu";
>> +		compatible = "sifive,rocket0", "riscv";
>> +		reg = <0x2>;
>> +	}
>> +	L18: cpu@3 {
>> +		device_type = "cpu";
>> +		compatible = "sifive,rocket0", "riscv";
>> +		reg = <0x3>;
>> +	}
>> +	L21: cpu@4 {
>> +		device_type = "cpu";
>> +		compatible = "sifive,rocket0", "riscv";
>> +		reg = <0x4>;
>> +	}
>> +};
> 
> The labels for the CPUs drew my attention. Is it intentionally random
> (or even specific) or just chosen to show anything can be used as labels ?

SiFive generates the device tree from RTL directly. So I am not sure if 
they assign random numbers or a particular algorithm chooses the label. 
I tried to put the exact ones that is available publicly.

https://github.com/riscv/riscv-device-tree-doc/blob/master/examples/sifive-hifive_unleashed-microsemi.dts 


Regards,
Atish
> The reason I ask is people tend to copy from existing DT or examples
> like here and so want to make sure if it can be kept as generic as
> possible in the example. Just my opinion and I am fine if you want to
> keep it as is, thought of checking the intentions here.
> 



> --
> Regards,
> Sudeep
> 

Re: [RFT PATCH v1 2/4] dt-binding: cpu-topology: Move cpu-map to a common binding.

[Sudeep Holla]

On Mon, Dec 03, 2018 at 09:23:42AM -0800, Atish Patra wrote:
> On 12/3/18 8:55 AM, Sudeep Holla wrote:
> > On Thu, Nov 29, 2018 at 03:28:18PM -0800, Atish Patra wrote:
> > > cpu-map binding can be used to described cpu topology for both
> > > RISC-V & ARM. It makes more sense to move the binding to document
> > > to a common place.
> > > 
> > > The relevant discussion can be found here.
> > > https://lkml.org/lkml/2018/11/6/19
> > > 
> > 
> > Looks good to me apart from a minor query below in the example.
> > 
> > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> > 
> > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > ---
> > >   .../{arm/topology.txt => cpu/cpu-topology.txt}     | 81 ++++++++++++++++++----
> > >   1 file changed, 67 insertions(+), 14 deletions(-)
> > >   rename Documentation/devicetree/bindings/{arm/topology.txt => cpu/cpu-topology.txt} (86%)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/cpu/cpu-topology.txt
> > > similarity index 86%
> > > rename from Documentation/devicetree/bindings/arm/topology.txt
> > > rename to Documentation/devicetree/bindings/cpu/cpu-topology.txt
> > > index 66848355..1de6fbce 100644
> > > --- a/Documentation/devicetree/bindings/arm/topology.txt
> > > +++ b/Documentation/devicetree/bindings/cpu/cpu-topology.txt
> > 
> > [...]
> > 
> > > +Example 3: HiFive Unleashed (RISC-V 64 bit, 4 core system)
> > > +
> > > +cpus {
> > > +	#address-cells = <2>;
> > > +	#size-cells = <2>;
> > > +	compatible = "sifive,fu540g", "sifive,fu500";
> > > +	model = "sifive,hifive-unleashed-a00";
> > > +
> > > +	...
> > > +
> > > +	cpu-map {
> > > +		cluster0 {
> > > +			core0 {
> > > +				cpu = <&L12>;
> > > +		 	};
> > > +			core1 {
> > > +				cpu = <&L15>;
> > > +			};
> > > +			core2 {
> > > +				cpu0 = <&L18>;
> > > +			};
> > > +			core3 {
> > > +				cpu0 = <&L21>;
> > > +			};
> > > +		};
> > > + 	};
> > > +
> > > +	L12: cpu@1 {
> > > +		device_type = "cpu";
> > > +		compatible = "sifive,rocket0", "riscv";
> > > +		reg = <0x1>;
> > > +	}
> > > +
> > > +	L15: cpu@2 {
> > > +		device_type = "cpu";
> > > +		compatible = "sifive,rocket0", "riscv";
> > > +		reg = <0x2>;
> > > +	}
> > > +	L18: cpu@3 {
> > > +		device_type = "cpu";
> > > +		compatible = "sifive,rocket0", "riscv";
> > > +		reg = <0x3>;
> > > +	}
> > > +	L21: cpu@4 {
> > > +		device_type = "cpu";
> > > +		compatible = "sifive,rocket0", "riscv";
> > > +		reg = <0x4>;
> > > +	}
> > > +};
> > 
> > The labels for the CPUs drew my attention. Is it intentionally random
> > (or even specific) or just chosen to show anything can be used as labels ?
> 
> SiFive generates the device tree from RTL directly. So I am not sure if they
> assign random numbers or a particular algorithm chooses the label. I tried
> to put the exact ones that is available publicly.
> 
> https://github.com/riscv/riscv-device-tree-doc/blob/master/examples/sifive-hifive_unleashed-microsemi.dts

Cool, love that. So you don't have the problem I was trying to explain.
But I still see the possibility of some other RISC-V vendor copy-pasting
from here ;). Anyways it's left to you.

--
Regards,
Sudeep

Re: [RFT PATCH v1 2/4] dt-binding: cpu-topology: Move cpu-map to a common binding.

[Atish Patra]

On 12/3/18 9:33 AM, Sudeep Holla wrote:
> On Mon, Dec 03, 2018 at 09:23:42AM -0800, Atish Patra wrote:
>> On 12/3/18 8:55 AM, Sudeep Holla wrote:
>>> On Thu, Nov 29, 2018 at 03:28:18PM -0800, Atish Patra wrote:
>>>> cpu-map binding can be used to described cpu topology for both
>>>> RISC-V & ARM. It makes more sense to move the binding to document
>>>> to a common place.
>>>>
>>>> The relevant discussion can be found here.
>>>> https://lkml.org/lkml/2018/11/6/19
>>>>
>>>
>>> Looks good to me apart from a minor query below in the example.
>>>
>>> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
>>>
>>>> Signed-off-by: Atish Patra <atish.patra@wdc.com>
>>>> ---
>>>>    .../{arm/topology.txt => cpu/cpu-topology.txt}     | 81 ++++++++++++++++++----
>>>>    1 file changed, 67 insertions(+), 14 deletions(-)
>>>>    rename Documentation/devicetree/bindings/{arm/topology.txt => cpu/cpu-topology.txt} (86%)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/cpu/cpu-topology.txt
>>>> similarity index 86%
>>>> rename from Documentation/devicetree/bindings/arm/topology.txt
>>>> rename to Documentation/devicetree/bindings/cpu/cpu-topology.txt
>>>> index 66848355..1de6fbce 100644
>>>> --- a/Documentation/devicetree/bindings/arm/topology.txt
>>>> +++ b/Documentation/devicetree/bindings/cpu/cpu-topology.txt
>>>
>>> [...]
>>>
>>>> +Example 3: HiFive Unleashed (RISC-V 64 bit, 4 core system)
>>>> +
>>>> +cpus {
>>>> +	#address-cells = <2>;
>>>> +	#size-cells = <2>;
>>>> +	compatible = "sifive,fu540g", "sifive,fu500";
>>>> +	model = "sifive,hifive-unleashed-a00";
>>>> +
>>>> +	...
>>>> +
>>>> +	cpu-map {
>>>> +		cluster0 {
>>>> +			core0 {
>>>> +				cpu = <&L12>;
>>>> +		 	};
>>>> +			core1 {
>>>> +				cpu = <&L15>;
>>>> +			};
>>>> +			core2 {
>>>> +				cpu0 = <&L18>;
>>>> +			};
>>>> +			core3 {
>>>> +				cpu0 = <&L21>;
>>>> +			};
>>>> +		};
>>>> + 	};
>>>> +
>>>> +	L12: cpu@1 {
>>>> +		device_type = "cpu";
>>>> +		compatible = "sifive,rocket0", "riscv";
>>>> +		reg = <0x1>;
>>>> +	}
>>>> +
>>>> +	L15: cpu@2 {
>>>> +		device_type = "cpu";
>>>> +		compatible = "sifive,rocket0", "riscv";
>>>> +		reg = <0x2>;
>>>> +	}
>>>> +	L18: cpu@3 {
>>>> +		device_type = "cpu";
>>>> +		compatible = "sifive,rocket0", "riscv";
>>>> +		reg = <0x3>;
>>>> +	}
>>>> +	L21: cpu@4 {
>>>> +		device_type = "cpu";
>>>> +		compatible = "sifive,rocket0", "riscv";
>>>> +		reg = <0x4>;
>>>> +	}
>>>> +};
>>>
>>> The labels for the CPUs drew my attention. Is it intentionally random
>>> (or even specific) or just chosen to show anything can be used as labels ?
>>
>> SiFive generates the device tree from RTL directly. So I am not sure if they
>> assign random numbers or a particular algorithm chooses the label. I tried
>> to put the exact ones that is available publicly.
>>
>> https://github.com/riscv/riscv-device-tree-doc/blob/master/examples/sifive-hifive_unleashed-microsemi.dts
> 
> Cool, love that. So you don't have the problem I was trying to explain.
> But I still see the possibility of some other RISC-V vendor copy-pasting
> from here ;). Anyways it's left to you.
> 

I am fine with either way. I hoped other vendors won't blindly copy as 
this example was specific to HiFive Unleashed board. But I get your 
point. As this DT entry is a generic architecture entry, we should have 
generic examples instead of platform specific examples.

I will change it to a generic one.


Regards,
Atish
> --
> Regards,
> Sudeep
> 

Re: [RFT PATCH v1 2/4] dt-binding: cpu-topology: Move cpu-map to a common binding.

[Rob Herring]

On Mon, Dec 03, 2018 at 09:23:42AM -0800, Atish Patra wrote:
> On 12/3/18 8:55 AM, Sudeep Holla wrote:
> > On Thu, Nov 29, 2018 at 03:28:18PM -0800, Atish Patra wrote:
> > > cpu-map binding can be used to described cpu topology for both
> > > RISC-V & ARM. It makes more sense to move the binding to document
> > > to a common place.
> > > 
> > > The relevant discussion can be found here.
> > > https://lkml.org/lkml/2018/11/6/19
> > > 
> > 
> > Looks good to me apart from a minor query below in the example.
> > 
> > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> > 
> > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > ---
> > >   .../{arm/topology.txt => cpu/cpu-topology.txt}     | 81 ++++++++++++++++++----
> > >   1 file changed, 67 insertions(+), 14 deletions(-)
> > >   rename Documentation/devicetree/bindings/{arm/topology.txt => cpu/cpu-topology.txt} (86%)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/cpu/cpu-topology.txt
> > > similarity index 86%
> > > rename from Documentation/devicetree/bindings/arm/topology.txt
> > > rename to Documentation/devicetree/bindings/cpu/cpu-topology.txt
> > > index 66848355..1de6fbce 100644
> > > --- a/Documentation/devicetree/bindings/arm/topology.txt
> > > +++ b/Documentation/devicetree/bindings/cpu/cpu-topology.txt
> > 
> > [...]
> > 
> > > +Example 3: HiFive Unleashed (RISC-V 64 bit, 4 core system)
> > > +
> > > +cpus {
> > > +	#address-cells = <2>;
> > > +	#size-cells = <2>;
> > > +	compatible = "sifive,fu540g", "sifive,fu500";
> > > +	model = "sifive,hifive-unleashed-a00";
> > > +
> > > +	...
> > > +
> > > +	cpu-map {
> > > +		cluster0 {
> > > +			core0 {
> > > +				cpu = <&L12>;
> > > +		 	};
> > > +			core1 {
> > > +				cpu = <&L15>;
> > > +			};
> > > +			core2 {
> > > +				cpu0 = <&L18>;
> > > +			};
> > > +			core3 {
> > > +				cpu0 = <&L21>;
> > > +			};
> > > +		};
> > > + 	};
> > > +
> > > +	L12: cpu@1 {
> > > +		device_type = "cpu";
> > > +		compatible = "sifive,rocket0", "riscv";
> > > +		reg = <0x1>;
> > > +	}
> > > +
> > > +	L15: cpu@2 {
> > > +		device_type = "cpu";
> > > +		compatible = "sifive,rocket0", "riscv";
> > > +		reg = <0x2>;
> > > +	}
> > > +	L18: cpu@3 {
> > > +		device_type = "cpu";
> > > +		compatible = "sifive,rocket0", "riscv";
> > > +		reg = <0x3>;
> > > +	}
> > > +	L21: cpu@4 {
> > > +		device_type = "cpu";
> > > +		compatible = "sifive,rocket0", "riscv";
> > > +		reg = <0x4>;
> > > +	}
> > > +};
> > 
> > The labels for the CPUs drew my attention. Is it intentionally random
> > (or even specific) or just chosen to show anything can be used as labels ?
> 
> SiFive generates the device tree from RTL directly. So I am not sure if they
> assign random numbers or a particular algorithm chooses the label. I tried
> to put the exact ones that is available publicly.
> 
> https://github.com/riscv/riscv-device-tree-doc/blob/master/examples/sifive-hifive_unleashed-microsemi.dts

Oh, that's really terrible. I wouldn't care as this was just source 
level stuff, but labels are part of the ABI with overlays.

Rob

Re: [RFT PATCH v1 2/4] dt-binding: cpu-topology: Move cpu-map to a common binding.

[Rob Herring]

On Thu, Nov 29, 2018 at 03:28:18PM -0800, Atish Patra wrote:
> cpu-map binding can be used to described cpu topology for both
> RISC-V & ARM. It makes more sense to move the binding to document
> to a common place.
> 
> The relevant discussion can be found here.
> https://lkml.org/lkml/2018/11/6/19
> 
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  .../{arm/topology.txt => cpu/cpu-topology.txt}     | 81 ++++++++++++++++++----
>  1 file changed, 67 insertions(+), 14 deletions(-)
>  rename Documentation/devicetree/bindings/{arm/topology.txt => cpu/cpu-topology.txt} (86%)
> 
> diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/cpu/cpu-topology.txt
> similarity index 86%
> rename from Documentation/devicetree/bindings/arm/topology.txt
> rename to Documentation/devicetree/bindings/cpu/cpu-topology.txt
> index 66848355..1de6fbce 100644
> --- a/Documentation/devicetree/bindings/arm/topology.txt
> +++ b/Documentation/devicetree/bindings/cpu/cpu-topology.txt
> @@ -1,12 +1,12 @@
>  ===========================================
> -ARM topology binding description
> +CPU topology binding description
>  ===========================================
>  
>  ===========================================
>  1 - Introduction
>  ===========================================
>  
> -In an ARM system, the hierarchy of CPUs is defined through three entities that
> +In a SMP system, the hierarchy of CPUs is defined through three entities that
>  are used to describe the layout of physical CPUs in the system:
>  
>  - socket
> @@ -14,9 +14,6 @@ are used to describe the layout of physical CPUs in the system:
>  - core
>  - thread
>  
> -The cpu nodes (bindings defined in [1]) represent the devices that
> -correspond to physical CPUs and are to be mapped to the hierarchy levels.
> -
>  The bottom hierarchy level sits at core or thread level depending on whether
>  symmetric multi-threading (SMT) is supported or not.
>  
> @@ -25,33 +22,37 @@ threads existing in the system and map to the hierarchy level "thread" above.
>  In systems where SMT is not supported "cpu" nodes represent all cores present
>  in the system and map to the hierarchy level "core" above.
>  
> -ARM topology bindings allow one to associate cpu nodes with hierarchical groups
> +CPU topology bindings allow one to associate cpu nodes with hierarchical groups
>  corresponding to the system hierarchy; syntactically they are defined as device
>  tree nodes.
>  
> -The remainder of this document provides the topology bindings for ARM, based
> -on the Devicetree Specification, available from:
> +Currently, only ARM/RISC-V intend to use this cpu topology binding but it may be
> +used for any other architecture as well.
>  
> -https://www.devicetree.org/specifications/
> +The remainder of this document provides the topology bindings for ARM/RISC-V, based

You already said who are current users, why restrict it to ARM and 
RISC-V here?

> +on the Devicetree Specification, available at [4].
> +
> +The cpu nodes (bindings defined in [1] for ARM or [2] for RISC-V) represent the devices that
> +correspond to physical CPUs and are to be mapped to the hierarchy levels.

The cpu topology isn't dependent on anything beyond what the DT spec 
says for cpu nodes so I think this can be simplified to just refer to 
the spec.

Plus, shouldn't [2] (numa) be [3] here.

>  If not stated otherwise, whenever a reference to a cpu node phandle is made its
>  value must point to a cpu node compliant with the cpu node bindings as
> -documented in [1].
> +documented in [1] or [3] for respective ISA.
>  A topology description containing phandles to cpu nodes that are not compliant
> -with bindings standardized in [1] is therefore considered invalid.
> +with bindings standardized in [1] or [3] is therefore considered invalid.
>  
>  ===========================================
>  2 - cpu-map node
>  ===========================================
>  
> -The ARM CPU topology is defined within the cpu-map node, which is a direct
> +The ARM/RISC-V CPU topology is defined within the cpu-map node, which is a direct
>  child of the cpus node and provides a container where the actual topology
>  nodes are listed.
>  
>  - cpu-map node
>  
> -	Usage: Optional - On ARM SMP systems provide CPUs topology to the OS.
> -			  ARM uniprocessor systems do not require a topology
> +	Usage: Optional - On SMP systems provide CPUs topology to the OS.
> +			  Uniprocessor systems do not require a topology
>  			  description and therefore should not define a
>  			  cpu-map node.
>  
> @@ -494,8 +495,60 @@ cpus {
>  	};
>  };
>  
> +Example 3: HiFive Unleashed (RISC-V 64 bit, 4 core system)
> +
> +cpus {
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +	compatible = "sifive,fu540g", "sifive,fu500";
> +	model = "sifive,hifive-unleashed-a00";

This is wrong. Looks like the root node, but called 'cpus'.

> +
> +	...
> +
> +	cpu-map {
> +		cluster0 {
> +			core0 {
> +				cpu = <&L12>;
> +		 	};

Mixed space and tabs.

> +			core1 {
> +				cpu = <&L15>;
> +			};
> +			core2 {
> +				cpu0 = <&L18>;
> +			};
> +			core3 {
> +				cpu0 = <&L21>;
> +			};
> +		};
> + 	};

Mixed space and tab.

> +
> +	L12: cpu@1 {
> +		device_type = "cpu";
> +		compatible = "sifive,rocket0", "riscv";
> +		reg = <0x1>;
> +	}
> +
> +	L15: cpu@2 {
> +		device_type = "cpu";
> +		compatible = "sifive,rocket0", "riscv";
> +		reg = <0x2>;
> +	}
> +	L18: cpu@3 {
> +		device_type = "cpu";
> +		compatible = "sifive,rocket0", "riscv";
> +		reg = <0x3>;
> +	}
> +	L21: cpu@4 {
> +		device_type = "cpu";
> +		compatible = "sifive,rocket0", "riscv";
> +		reg = <0x4>;
> +	}
> +};
>  ===============================================================================
>  [1] ARM Linux kernel documentation
>      Documentation/devicetree/bindings/arm/cpus.txt
>  [2] Devicetree NUMA binding description
>      Documentation/devicetree/bindings/numa.txt
> +[3] RISC-V Linux kernel documentation
> +    Documentation/devicetree/bindings/riscv/cpus.txt
> +[4] https://www.devicetree.org/specifications/
> -- 
> 2.7.4
> 

Re: [RFT PATCH v1 2/4] dt-binding: cpu-topology: Move cpu-map to a common binding.

[Atish Patra]

On 12/11/18 6:31 PM, Rob Herring wrote:
> On Thu, Nov 29, 2018 at 03:28:18PM -0800, Atish Patra wrote:
>> cpu-map binding can be used to described cpu topology for both
>> RISC-V & ARM. It makes more sense to move the binding to document
>> to a common place.
>>
>> The relevant discussion can be found here.
>> https://lkml.org/lkml/2018/11/6/19
>>
>> Signed-off-by: Atish Patra <atish.patra@wdc.com>
>> ---
>>   .../{arm/topology.txt => cpu/cpu-topology.txt}     | 81 ++++++++++++++++++----
>>   1 file changed, 67 insertions(+), 14 deletions(-)
>>   rename Documentation/devicetree/bindings/{arm/topology.txt => cpu/cpu-topology.txt} (86%)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/cpu/cpu-topology.txt
>> similarity index 86%
>> rename from Documentation/devicetree/bindings/arm/topology.txt
>> rename to Documentation/devicetree/bindings/cpu/cpu-topology.txt
>> index 66848355..1de6fbce 100644
>> --- a/Documentation/devicetree/bindings/arm/topology.txt
>> +++ b/Documentation/devicetree/bindings/cpu/cpu-topology.txt
>> @@ -1,12 +1,12 @@
>>   ===========================================
>> -ARM topology binding description
>> +CPU topology binding description
>>   ===========================================
>>   
>>   ===========================================
>>   1 - Introduction
>>   ===========================================
>>   
>> -In an ARM system, the hierarchy of CPUs is defined through three entities that
>> +In a SMP system, the hierarchy of CPUs is defined through three entities that
>>   are used to describe the layout of physical CPUs in the system:
>>   
>>   - socket
>> @@ -14,9 +14,6 @@ are used to describe the layout of physical CPUs in the system:
>>   - core
>>   - thread
>>   
>> -The cpu nodes (bindings defined in [1]) represent the devices that
>> -correspond to physical CPUs and are to be mapped to the hierarchy levels.
>> -
>>   The bottom hierarchy level sits at core or thread level depending on whether
>>   symmetric multi-threading (SMT) is supported or not.
>>   
>> @@ -25,33 +22,37 @@ threads existing in the system and map to the hierarchy level "thread" above.
>>   In systems where SMT is not supported "cpu" nodes represent all cores present
>>   in the system and map to the hierarchy level "core" above.
>>   
>> -ARM topology bindings allow one to associate cpu nodes with hierarchical groups
>> +CPU topology bindings allow one to associate cpu nodes with hierarchical groups
>>   corresponding to the system hierarchy; syntactically they are defined as device
>>   tree nodes.
>>   
>> -The remainder of this document provides the topology bindings for ARM, based
>> -on the Devicetree Specification, available from:
>> +Currently, only ARM/RISC-V intend to use this cpu topology binding but it may be
>> +used for any other architecture as well.
>>   
>> -https://www.devicetree.org/specifications/
>> +The remainder of this document provides the topology bindings for ARM/RISC-V, based
> 
> You already said who are current users, why restrict it to ARM and
> RISC-V here?
> 
I will remove that. The examples are only for ARM/RISC-V specific.


>> +on the Devicetree Specification, available at [4].
>> +
>> +The cpu nodes (bindings defined in [1] for ARM or [2] for RISC-V) represent the devices that
>> +correspond to physical CPUs and are to be mapped to the hierarchy levels.
> 
> The cpu topology isn't dependent on anything beyond what the DT spec
> says for cpu nodes so I think this can be simplified to just refer to
> the spec.
> 

ok sure.

> Plus, shouldn't [2] (numa) be [3] here.
> 

My bad.

>>   If not stated otherwise, whenever a reference to a cpu node phandle is made its
>>   value must point to a cpu node compliant with the cpu node bindings as
>> -documented in [1].
>> +documented in [1] or [3] for respective ISA.
>>   A topology description containing phandles to cpu nodes that are not compliant
>> -with bindings standardized in [1] is therefore considered invalid.
>> +with bindings standardized in [1] or [3] is therefore considered invalid.
>>   
>>   ===========================================
>>   2 - cpu-map node
>>   ===========================================
>>   
>> -The ARM CPU topology is defined within the cpu-map node, which is a direct
>> +The ARM/RISC-V CPU topology is defined within the cpu-map node, which is a direct
>>   child of the cpus node and provides a container where the actual topology
>>   nodes are listed.
>>   
>>   - cpu-map node
>>   
>> -	Usage: Optional - On ARM SMP systems provide CPUs topology to the OS.
>> -			  ARM uniprocessor systems do not require a topology
>> +	Usage: Optional - On SMP systems provide CPUs topology to the OS.
>> +			  Uniprocessor systems do not require a topology
>>   			  description and therefore should not define a
>>   			  cpu-map node.
>>   
>> @@ -494,8 +495,60 @@ cpus {
>>   	};
>>   };
>>   
>> +Example 3: HiFive Unleashed (RISC-V 64 bit, 4 core system)
>> +
>> +cpus {
>> +	#address-cells = <2>;
>> +	#size-cells = <2>;
>> +	compatible = "sifive,fu540g", "sifive,fu500";
>> +	model = "sifive,hifive-unleashed-a00";
> 
> This is wrong. Looks like the root node, but called 'cpus'.
> 
Yeah it got mixed up. I will fix it in v2.

>> +
>> +	...
>> +
>> +	cpu-map {
>> +		cluster0 {
>> +			core0 {
>> +				cpu = <&L12>;
>> +		 	};
> 
> Mixed space and tabs.
> 
>> +			core1 {
>> +				cpu = <&L15>;
>> +			};
>> +			core2 {
>> +				cpu0 = <&L18>;
>> +			};
>> +			core3 {
>> +				cpu0 = <&L21>;
>> +			};
>> +		};
>> + 	};
> 
> Mixed space and tab.
> 

Sorry. I will fix this.

Thanks for the review.

Regards,
Atish

>> +
>> +	L12: cpu@1 {
>> +		device_type = "cpu";
>> +		compatible = "sifive,rocket0", "riscv";
>> +		reg = <0x1>;
>> +	}
>> +
>> +	L15: cpu@2 {
>> +		device_type = "cpu";
>> +		compatible = "sifive,rocket0", "riscv";
>> +		reg = <0x2>;
>> +	}
>> +	L18: cpu@3 {
>> +		device_type = "cpu";
>> +		compatible = "sifive,rocket0", "riscv";
>> +		reg = <0x3>;
>> +	}
>> +	L21: cpu@4 {
>> +		device_type = "cpu";
>> +		compatible = "sifive,rocket0", "riscv";
>> +		reg = <0x4>;
>> +	}
>> +};
>>   ===============================================================================
>>   [1] ARM Linux kernel documentation
>>       Documentation/devicetree/bindings/arm/cpus.txt
>>   [2] Devicetree NUMA binding description
>>       Documentation/devicetree/bindings/numa.txt
>> +[3] RISC-V Linux kernel documentation
>> +    Documentation/devicetree/bindings/riscv/cpus.txt
>> +[4] https://www.devicetree.org/specifications/
>> -- 
>> 2.7.4
>>
> 

[RFT PATCH v1 3/4] cpu-topology: Move cpu topology code to common code.

[Atish Patra]

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>
---
 arch/arm64/include/asm/topology.h |  22 ---
 arch/arm64/kernel/topology.c      | 303 +-------------------------------------
 drivers/base/arch_topology.c      | 294 ++++++++++++++++++++++++++++++++++++
 include/linux/arch_topology.h     |  26 ++++
 include/linux/topology.h          |   1 +
 5 files changed, 325 insertions(+), 321 deletions(-)

diff --git a/arch/arm64/include/asm/topology.h b/arch/arm64/include/asm/topology.h
index 0524f243..b1b61e0f 100644
--- a/arch/arm64/include/asm/topology.h
+++ b/arch/arm64/include/asm/topology.h
@@ -4,29 +4,7 @@
 
 #include <linux/cpumask.h>
 
-struct cpu_topology {
-	int thread_id;
-	int core_id;
-	int package_id;
-	int llc_id;
-	cpumask_t thread_sibling;
-	cpumask_t core_sibling;
-	cpumask_t llc_sibling;
-};
-
-extern struct cpu_topology cpu_topology[NR_CPUS];
-
-#define topology_physical_package_id(cpu)	(cpu_topology[cpu].package_id)
-#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)
-#define topology_llc_cpumask(cpu)	(&cpu_topology[cpu].llc_sibling)
-
-void init_cpu_topology(void);
 void store_cpu_topology(unsigned int cpuid);
-void remove_cpu_topology(unsigned int cpuid);
-const struct cpumask *cpu_coregroup_mask(int cpu);
-
 #ifdef CONFIG_NUMA
 
 struct pci_bus;
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 0825c4a8..6b95c91e 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -14,250 +14,13 @@
 #include <linux/acpi.h>
 #include <linux/arch_topology.h>
 #include <linux/cacheinfo.h>
-#include <linux/cpu.h>
-#include <linux/cpumask.h>
 #include <linux/init.h>
 #include <linux/percpu.h>
-#include <linux/node.h>
-#include <linux/nodemask.h>
-#include <linux/of.h>
-#include <linux/sched.h>
-#include <linux/sched/topology.h>
-#include <linux/slab.h>
-#include <linux/smp.h>
-#include <linux/string.h>
 
 #include <asm/cpu.h>
 #include <asm/cputype.h>
 #include <asm/topology.h>
 
-static int __init get_cpu_for_node(struct device_node *node)
-{
-	struct device_node *cpu_node;
-	int cpu;
-
-	cpu_node = of_parse_phandle(node, "cpu", 0);
-	if (!cpu_node)
-		return -1;
-
-	cpu = of_cpu_node_to_id(cpu_node);
-	if (cpu >= 0)
-		topology_parse_cpu_capacity(cpu_node, cpu);
-	else
-		pr_crit("Unable to find CPU node for %pOF\n", cpu_node);
-
-	of_node_put(cpu_node);
-	return cpu;
-}
-
-static int __init parse_core(struct device_node *core, int package_id,
-			     int core_id)
-{
-	char name[10];
-	bool leaf = true;
-	int i = 0;
-	int cpu;
-	struct device_node *t;
-
-	do {
-		snprintf(name, sizeof(name), "thread%d", i);
-		t = of_get_child_by_name(core, name);
-		if (t) {
-			leaf = false;
-			cpu = get_cpu_for_node(t);
-			if (cpu >= 0) {
-				cpu_topology[cpu].package_id = package_id;
-				cpu_topology[cpu].core_id = core_id;
-				cpu_topology[cpu].thread_id = i;
-			} else {
-				pr_err("%pOF: Can't get CPU for thread\n",
-				       t);
-				of_node_put(t);
-				return -EINVAL;
-			}
-			of_node_put(t);
-		}
-		i++;
-	} while (t);
-
-	cpu = get_cpu_for_node(core);
-	if (cpu >= 0) {
-		if (!leaf) {
-			pr_err("%pOF: Core has both threads and CPU\n",
-			       core);
-			return -EINVAL;
-		}
-
-		cpu_topology[cpu].package_id = package_id;
-		cpu_topology[cpu].core_id = core_id;
-	} else if (leaf) {
-		pr_err("%pOF: Can't get CPU for leaf core\n", core);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
-static int __init parse_cluster(struct device_node *cluster, int depth)
-{
-	char name[10];
-	bool leaf = true;
-	bool has_cores = false;
-	struct device_node *c;
-	static int package_id __initdata;
-	int core_id = 0;
-	int i, ret;
-
-	/*
-	 * First check for child clusters; we currently ignore any
-	 * information about the nesting of clusters and present the
-	 * scheduler with a flat list of them.
-	 */
-	i = 0;
-	do {
-		snprintf(name, sizeof(name), "cluster%d", i);
-		c = of_get_child_by_name(cluster, name);
-		if (c) {
-			leaf = false;
-			ret = parse_cluster(c, depth + 1);
-			of_node_put(c);
-			if (ret != 0)
-				return ret;
-		}
-		i++;
-	} while (c);
-
-	/* Now check for cores */
-	i = 0;
-	do {
-		snprintf(name, sizeof(name), "core%d", i);
-		c = of_get_child_by_name(cluster, name);
-		if (c) {
-			has_cores = true;
-
-			if (depth == 0) {
-				pr_err("%pOF: cpu-map children should be clusters\n",
-				       c);
-				of_node_put(c);
-				return -EINVAL;
-			}
-
-			if (leaf) {
-				ret = parse_core(c, package_id, core_id++);
-			} else {
-				pr_err("%pOF: Non-leaf cluster with core %s\n",
-				       cluster, name);
-				ret = -EINVAL;
-			}
-
-			of_node_put(c);
-			if (ret != 0)
-				return ret;
-		}
-		i++;
-	} while (c);
-
-	if (leaf && !has_cores)
-		pr_warn("%pOF: empty cluster\n", cluster);
-
-	if (leaf)
-		package_id++;
-
-	return 0;
-}
-
-static int __init parse_dt_topology(void)
-{
-	struct device_node *cn, *map;
-	int ret = 0;
-	int cpu;
-
-	cn = of_find_node_by_path("/cpus");
-	if (!cn) {
-		pr_err("No CPU information found in DT\n");
-		return 0;
-	}
-
-	/*
-	 * When topology is provided cpu-map is essentially a root
-	 * cluster with restricted subnodes.
-	 */
-	map = of_get_child_by_name(cn, "cpu-map");
-	if (!map)
-		goto out;
-
-	ret = parse_cluster(map, 0);
-	if (ret != 0)
-		goto out_map;
-
-	topology_normalize_cpu_scale();
-
-	/*
-	 * Check that all cores are in the topology; the SMP code will
-	 * only mark cores described in the DT as possible.
-	 */
-	for_each_possible_cpu(cpu)
-		if (cpu_topology[cpu].package_id == -1)
-			ret = -EINVAL;
-
-out_map:
-	of_node_put(map);
-out:
-	of_node_put(cn);
-	return ret;
-}
-
-/*
- * cpu topology table
- */
-struct cpu_topology cpu_topology[NR_CPUS];
-EXPORT_SYMBOL_GPL(cpu_topology);
-
-const struct cpumask *cpu_coregroup_mask(int cpu)
-{
-	const cpumask_t *core_mask = cpumask_of_node(cpu_to_node(cpu));
-
-	/* Find the smaller of NUMA, core or LLC siblings */
-	if (cpumask_subset(&cpu_topology[cpu].core_sibling, core_mask)) {
-		/* not numa in package, lets use the package siblings */
-		core_mask = &cpu_topology[cpu].core_sibling;
-	}
-	if (cpu_topology[cpu].llc_id != -1) {
-		if (cpumask_subset(&cpu_topology[cpu].llc_sibling, core_mask))
-			core_mask = &cpu_topology[cpu].llc_sibling;
-	}
-
-	return core_mask;
-}
-
-static void update_siblings_masks(unsigned int cpuid)
-{
-	struct cpu_topology *cpu_topo, *cpuid_topo = &cpu_topology[cpuid];
-	int cpu;
-
-	/* update core and thread sibling masks */
-	for_each_online_cpu(cpu) {
-		cpu_topo = &cpu_topology[cpu];
-
-		if (cpuid_topo->llc_id == cpu_topo->llc_id) {
-			cpumask_set_cpu(cpu, &cpuid_topo->llc_sibling);
-			cpumask_set_cpu(cpuid, &cpu_topo->llc_sibling);
-		}
-
-		if (cpuid_topo->package_id != cpu_topo->package_id)
-			continue;
-
-		cpumask_set_cpu(cpuid, &cpu_topo->core_sibling);
-		cpumask_set_cpu(cpu, &cpuid_topo->core_sibling);
-
-		if (cpuid_topo->core_id != cpu_topo->core_id)
-			continue;
-
-		cpumask_set_cpu(cpuid, &cpu_topo->thread_sibling);
-		cpumask_set_cpu(cpu, &cpuid_topo->thread_sibling);
-	}
-}
-
 void store_cpu_topology(unsigned int cpuid)
 {
 	struct cpu_topology *cpuid_topo = &cpu_topology[cpuid];
@@ -296,59 +59,19 @@ void store_cpu_topology(unsigned int cpuid)
 	update_siblings_masks(cpuid);
 }
 
-static void clear_cpu_topology(int cpu)
-{
-	struct cpu_topology *cpu_topo = &cpu_topology[cpu];
-
-	cpumask_clear(&cpu_topo->llc_sibling);
-	cpumask_set_cpu(cpu, &cpu_topo->llc_sibling);
-
-	cpumask_clear(&cpu_topo->core_sibling);
-	cpumask_set_cpu(cpu, &cpu_topo->core_sibling);
-	cpumask_clear(&cpu_topo->thread_sibling);
-	cpumask_set_cpu(cpu, &cpu_topo->thread_sibling);
-}
-
-static void __init reset_cpu_topology(void)
-{
-	unsigned int cpu;
-
-	for_each_possible_cpu(cpu) {
-		struct cpu_topology *cpu_topo = &cpu_topology[cpu];
-
-		cpu_topo->thread_id = -1;
-		cpu_topo->core_id = 0;
-		cpu_topo->package_id = -1;
-		cpu_topo->llc_id = -1;
-
-		clear_cpu_topology(cpu);
-	}
-}
-
-void remove_cpu_topology(unsigned int cpu)
-{
-	int sibling;
-
-	for_each_cpu(sibling, topology_core_cpumask(cpu))
-		cpumask_clear_cpu(cpu, topology_core_cpumask(sibling));
-	for_each_cpu(sibling, topology_sibling_cpumask(cpu))
-		cpumask_clear_cpu(cpu, topology_sibling_cpumask(sibling));
-	for_each_cpu(sibling, topology_llc_cpumask(cpu))
-		cpumask_clear_cpu(cpu, topology_llc_cpumask(sibling));
-
-	clear_cpu_topology(cpu);
-}
-
 #ifdef CONFIG_ACPI
 /*
  * Propagate the topology information of the processor_topology_node tree to the
  * cpu_topology array.
  */
-static int __init parse_acpi_topology(void)
+int __init parse_acpi_topology(void)
 {
 	bool is_threaded;
 	int cpu, topology_id;
 
+	if (acpi_disabled)
+		return 0;
+
 	is_threaded = read_cpuid_mpidr() & MPIDR_MT_BITMASK;
 
 	for_each_possible_cpu(cpu) {
@@ -384,24 +107,6 @@ static int __init parse_acpi_topology(void)
 
 	return 0;
 }
-
-#else
-static inline int __init parse_acpi_topology(void)
-{
-	return -EINVAL;
-}
 #endif
 
-void __init init_cpu_topology(void)
-{
-	reset_cpu_topology();
 
-	/*
-	 * Discard anything that was parsed if we hit an error so we
-	 * don't use partial information.
-	 */
-	if (!acpi_disabled && parse_acpi_topology())
-		reset_cpu_topology();
-	else if (of_have_populated_dt() && parse_dt_topology())
-		reset_cpu_topology();
-}
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index edfcf8d9..abddcb3d 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -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,292 @@ static void parsing_done_workfn(struct work_struct *work)
 #else
 core_initcall(free_raw_capacity);
 #endif
+
+static int __init get_cpu_for_node(struct device_node *node)
+{
+	struct device_node *cpu_node;
+	int cpu;
+
+	cpu_node = of_parse_phandle(node, "cpu", 0);
+	if (!cpu_node)
+		return -1;
+
+	cpu = of_cpu_node_to_id(cpu_node);
+	if (cpu >= 0)
+		topology_parse_cpu_capacity(cpu_node, cpu);
+	else
+		pr_crit("Unable to find CPU node for %pOF\n", cpu_node);
+
+	of_node_put(cpu_node);
+	return cpu;
+}
+
+static int __init parse_core(struct device_node *core, int package_id,
+			     int core_id)
+{
+	char name[10];
+	bool leaf = true;
+	int i = 0;
+	int cpu;
+	struct device_node *t;
+
+	do {
+		snprintf(name, sizeof(name), "thread%d", i);
+		t = of_get_child_by_name(core, name);
+		if (t) {
+			leaf = false;
+			cpu = get_cpu_for_node(t);
+			if (cpu >= 0) {
+				cpu_topology[cpu].package_id = package_id;
+				cpu_topology[cpu].core_id = core_id;
+				cpu_topology[cpu].thread_id = i;
+			} else {
+				pr_err("%pOF: Can't get CPU for thread\n",
+				       t);
+				of_node_put(t);
+				return -EINVAL;
+			}
+			of_node_put(t);
+		}
+		i++;
+	} while (t);
+
+	cpu = get_cpu_for_node(core);
+	if (cpu >= 0) {
+		if (!leaf) {
+			pr_err("%pOF: Core has both threads and CPU\n",
+			       core);
+			return -EINVAL;
+		}
+
+		cpu_topology[cpu].package_id = package_id;
+		cpu_topology[cpu].core_id = core_id;
+	} else if (leaf) {
+		pr_err("%pOF: Can't get CPU for leaf core\n", core);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int __init parse_cluster(struct device_node *cluster, int depth)
+{
+	char name[10];
+	bool leaf = true;
+	bool has_cores = false;
+	int core_id = 0;
+	int i, ret;
+	struct device_node *c;
+	static int package_id __initdata;
+
+	/*
+	 * First check for child clusters; we currently ignore any
+	 * information about the nesting of clusters and present the
+	 * scheduler with a flat list of them.
+	 */
+	i = 0;
+	do {
+		snprintf(name, sizeof(name), "cluster%d", i);
+		c = of_get_child_by_name(cluster, name);
+		if (c) {
+			leaf = false;
+			ret = parse_cluster(c, depth + 1);
+			of_node_put(c);
+			if (ret != 0)
+				return ret;
+		}
+		i++;
+	} while (c);
+
+	/* Now check for cores */
+	i = 0;
+	do {
+		snprintf(name, sizeof(name), "core%d", i);
+		c = of_get_child_by_name(cluster, name);
+		if (c) {
+			has_cores = true;
+
+			if (depth == 0) {
+				pr_err("%pOF: cpu-map children should be clusters\n",
+				       c);
+				of_node_put(c);
+				return -EINVAL;
+			}
+
+			if (leaf) {
+				ret = parse_core(c, package_id, core_id++);
+			} else {
+				pr_err("%pOF: Non-leaf cluster with core %s\n",
+				       cluster, name);
+				ret = -EINVAL;
+			}
+
+			of_node_put(c);
+			if (ret != 0)
+				return ret;
+		}
+		i++;
+	} while (c);
+
+	if (leaf && !has_cores)
+		pr_warn("%pOF: empty cluster\n", cluster);
+
+	if (leaf)
+		package_id++;
+
+	return 0;
+}
+
+static int __init parse_dt_topology(void)
+{
+	struct device_node *cn, *map;
+	int ret = 0;
+	int cpu;
+
+	cn = of_find_node_by_path("/cpus");
+	if (!cn) {
+		pr_err("No CPU information found in DT\n");
+		return 0;
+	}
+
+	/*
+	 * When topology is provided cpu-map is essentially a root
+	 * cluster with restricted subnodes.
+	 */
+	map = of_get_child_by_name(cn, "cpu-map");
+	if (!map)
+		goto out;
+
+	ret = parse_cluster(map, 0);
+	if (ret != 0)
+		goto out_map;
+
+	topology_normalize_cpu_scale();
+
+	/*
+	 * Check that all cores are in the topology; the SMP code will
+	 * only mark cores described in the DT as possible.
+	 */
+	for_each_possible_cpu(cpu)
+		if (cpu_topology[cpu].package_id == -1)
+			ret = -EINVAL;
+
+out_map:
+	of_node_put(map);
+out:
+	of_node_put(cn);
+	return ret;
+}
+
+/*
+ * cpu topology table
+ */
+struct cpu_topology cpu_topology[NR_CPUS];
+EXPORT_SYMBOL_GPL(cpu_topology);
+
+const struct cpumask *cpu_coregroup_mask(int cpu)
+{
+	const cpumask_t *core_mask = cpumask_of_node(cpu_to_node(cpu));
+
+	/* Find the smaller of NUMA, core or LLC siblings */
+	if (cpumask_subset(&cpu_topology[cpu].core_sibling, core_mask)) {
+		/* not numa in package, lets use the package siblings */
+		core_mask = &cpu_topology[cpu].core_sibling;
+	}
+	if (cpu_topology[cpu].llc_id != -1) {
+		if (cpumask_subset(&cpu_topology[cpu].llc_sibling, core_mask))
+			core_mask = &cpu_topology[cpu].llc_sibling;
+	}
+
+	return core_mask;
+}
+
+void update_siblings_masks(unsigned int cpuid)
+{
+	struct cpu_topology *cpu_topo, *cpuid_topo = &cpu_topology[cpuid];
+	int cpu;
+
+	/* update core and thread sibling masks */
+	for_each_online_cpu(cpu) {
+		cpu_topo = &cpu_topology[cpu];
+
+		if (cpuid_topo->llc_id == cpu_topo->llc_id) {
+			cpumask_set_cpu(cpu, &cpuid_topo->llc_sibling);
+			cpumask_set_cpu(cpuid, &cpu_topo->llc_sibling);
+		}
+
+		if (cpuid_topo->package_id != cpu_topo->package_id)
+			continue;
+
+		cpumask_set_cpu(cpuid, &cpu_topo->core_sibling);
+		cpumask_set_cpu(cpu, &cpuid_topo->core_sibling);
+
+		if (cpuid_topo->core_id != cpu_topo->core_id)
+			continue;
+
+		cpumask_set_cpu(cpuid, &cpu_topo->thread_sibling);
+		cpumask_set_cpu(cpu, &cpuid_topo->thread_sibling);
+	}
+}
+
+static void clear_cpu_topology(int cpu)
+{
+	struct cpu_topology *cpu_topo = &cpu_topology[cpu];
+
+	cpumask_clear(&cpu_topo->llc_sibling);
+	cpumask_set_cpu(cpu, &cpu_topo->llc_sibling);
+
+	cpumask_clear(&cpu_topo->core_sibling);
+	cpumask_set_cpu(cpu, &cpu_topo->core_sibling);
+	cpumask_clear(&cpu_topo->thread_sibling);
+	cpumask_set_cpu(cpu, &cpu_topo->thread_sibling);
+}
+
+static void __init reset_cpu_topology(void)
+{
+	unsigned int cpu;
+
+	for_each_possible_cpu(cpu) {
+		struct cpu_topology *cpu_topo = &cpu_topology[cpu];
+
+		cpu_topo->thread_id = -1;
+		cpu_topo->core_id = 0;
+		cpu_topo->package_id = -1;
+		cpu_topo->llc_id = -1;
+
+		clear_cpu_topology(cpu);
+	}
+}
+
+void remove_cpu_topology(unsigned int cpu)
+{
+	int sibling;
+
+	for_each_cpu(sibling, topology_core_cpumask(cpu))
+		cpumask_clear_cpu(cpu, topology_core_cpumask(sibling));
+	for_each_cpu(sibling, topology_sibling_cpumask(cpu))
+		cpumask_clear_cpu(cpu, topology_sibling_cpumask(sibling));
+	for_each_cpu(sibling, topology_llc_cpumask(cpu))
+		cpumask_clear_cpu(cpu, topology_llc_cpumask(sibling));
+
+	clear_cpu_topology(cpu);
+}
+
+__weak int __init parse_acpi_topology(void)
+{
+	return 0;
+}
+
+void __init init_cpu_topology(void)
+{
+	reset_cpu_topology();
+
+	/*
+	 * Discard anything that was parsed if we hit an error so we
+	 * don't use partial information.
+	 */
+	if (parse_acpi_topology())
+		reset_cpu_topology();
+	else if (of_have_populated_dt() && parse_dt_topology())
+		reset_cpu_topology();
+}
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
index d9bdc1a7..167e9dd6 100644
--- a/include/linux/arch_topology.h
+++ b/include/linux/arch_topology.h
@@ -33,4 +33,30 @@ unsigned long topology_get_freq_scale(int cpu)
 	return per_cpu(freq_scale, cpu);
 }
 
+struct cpu_topology {
+	int thread_id;
+	int core_id;
+	int package_id;
+	int llc_id;
+	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];
+
+#define topology_physical_package_id(cpu)	(cpu_topology[cpu].package_id)
+#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)
+#define topology_llc_cpumask(cpu)	(&cpu_topology[cpu].llc_sibling)
+
+#endif
+
+void init_cpu_topology(void);
+void update_siblings_masks(unsigned int cpu);
+void remove_cpu_topology(unsigned int cpuid);
+const struct cpumask *cpu_coregroup_mask(int cpu);
+
 #endif /* _LINUX_ARCH_TOPOLOGY_H_ */
diff --git a/include/linux/topology.h b/include/linux/topology.h
index cb0775e1..81501fac 100644
--- a/include/linux/topology.h
+++ b/include/linux/topology.h
@@ -32,6 +32,7 @@
 #include <linux/mmzone.h>
 #include <linux/smp.h>
 #include <linux/percpu.h>
+#include <linux/arch_topology.h>
 #include <asm/topology.h>
 
 #ifndef nr_cpus_node
-- 
2.7.4

Re: [RFT PATCH v1 3/4] cpu-topology: Move cpu topology code to common code.

[Will Deacon]

On Thu, Nov 29, 2018 at 03:28:19PM -0800, 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>
> ---
>  arch/arm64/include/asm/topology.h |  22 ---
>  arch/arm64/kernel/topology.c      | 303 +-------------------------------------
>  drivers/base/arch_topology.c      | 294 ++++++++++++++++++++++++++++++++++++

Whilst I'm happy seeing this code moved out, I'd like to have an entry in
MAINTAINERS so that somebody can review changes from the arm64 side without
having to poll a load of different lists. I assume patches would still go
via Rafael and Greg, but if we could have Sudeep added as Reviewer for this
file, then I'd really appreciate it.

Cheers,

Will

Re: [RFT PATCH v1 3/4] cpu-topology: Move cpu topology code to common code.

[Sudeep Holla]

(Fixed Juri Lelli's email)

On Mon, Dec 03, 2018 at 04:58:34PM +0000, Will Deacon wrote:
> On Thu, Nov 29, 2018 at 03:28:19PM -0800, 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>
> > ---
> >  arch/arm64/include/asm/topology.h |  22 ---
> >  arch/arm64/kernel/topology.c      | 303 +-------------------------------------
> >  drivers/base/arch_topology.c      | 294 ++++++++++++++++++++++++++++++++++++
>
> Whilst I'm happy seeing this code moved out, I'd like to have an entry in
> MAINTAINERS so that somebody can review changes from the arm64 side without
> having to poll a load of different lists. I assume patches would still go
> via Rafael and Greg, but if we could have Sudeep added as Reviewer for this
> file, then I'd really appreciate it.
>

Thanks Will for pointing this. I agree with you and like to be added as
reviewer for this.

Since arch_topology.c was mostly added for ARM/ARM64 and by Juri Lelli 
(who is no longer with ARM) and I did review the initial version, I would
like to assume maintenance for this file if both Juri and Greg are OK
with it so that I can keep an eye on this as we expect more changes to
this both for ARM and RISC-V. I am fine if anyone who has contributed
more to this file wants to maintain.

--
Regards,
Sudeep

Re: [RFT PATCH v1 3/4] cpu-topology: Move cpu topology code to common code.

[Juri Lelli]

Hi,

On 03/12/18 17:12, Sudeep Holla wrote:
> (Fixed Juri Lelli's email)
> 
> On Mon, Dec 03, 2018 at 04:58:34PM +0000, Will Deacon wrote:
> > On Thu, Nov 29, 2018 at 03:28:19PM -0800, 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>
> > > ---
> > >  arch/arm64/include/asm/topology.h |  22 ---
> > >  arch/arm64/kernel/topology.c      | 303 +-------------------------------------
> > >  drivers/base/arch_topology.c      | 294 ++++++++++++++++++++++++++++++++++++
> >
> > Whilst I'm happy seeing this code moved out, I'd like to have an entry in
> > MAINTAINERS so that somebody can review changes from the arm64 side without
> > having to poll a load of different lists. I assume patches would still go
> > via Rafael and Greg, but if we could have Sudeep added as Reviewer for this
> > file, then I'd really appreciate it.
> >
> 
> Thanks Will for pointing this. I agree with you and like to be added as
> reviewer for this.
> 
> Since arch_topology.c was mostly added for ARM/ARM64 and by Juri Lelli 
> (who is no longer with ARM) and I did review the initial version, I would
> like to assume maintenance for this file if both Juri and Greg are OK
> with it so that I can keep an eye on this as we expect more changes to
> this both for ARM and RISC-V.

Sure, this is OK with me. Thanks for keeping an eye on this.

Best,

- Juri

Re: [RFT PATCH v1 3/4] cpu-topology: Move cpu topology code to common code.

[Sudeep Holla]

On Thu, Nov 29, 2018 at 03:28:19PM -0800, 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>
> ---
>  arch/arm64/include/asm/topology.h |  22 ---
>  arch/arm64/kernel/topology.c      | 303 +-------------------------------------
>  drivers/base/arch_topology.c      | 294 ++++++++++++++++++++++++++++++++++++
>  include/linux/arch_topology.h     |  26 ++++
>  include/linux/topology.h          |   1 +
>  5 files changed, 325 insertions(+), 321 deletions(-)
>
From a quick look and diffstat, it looks like a simple move. However
I would like to throw this at 0-day bot to test various build configurations
so that it's not breaking anything. I can push to a branch in my git
@kernel.org but not sure on the build coverage. Anyways will update
with any results on build and testing on my side ASAP.

--
Regards,
Sudeep

Re: [RFT PATCH v1 3/4] cpu-topology: Move cpu topology code to common code.

[Atish Patra]

On 12/3/18 9:16 AM, Sudeep Holla wrote:
> On Thu, Nov 29, 2018 at 03:28:19PM -0800, 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>
>> ---
>>   arch/arm64/include/asm/topology.h |  22 ---
>>   arch/arm64/kernel/topology.c      | 303 +-------------------------------------
>>   drivers/base/arch_topology.c      | 294 ++++++++++++++++++++++++++++++++++++
>>   include/linux/arch_topology.h     |  26 ++++
>>   include/linux/topology.h          |   1 +
>>   5 files changed, 325 insertions(+), 321 deletions(-)
>>
>  From a quick look and diffstat, it looks like a simple move. However
> I would like to throw this at 0-day bot to test various build configurations
> so that it's not breaking anything. I can push to a branch in my git
> @kernel.org but not sure on the build coverage. Anyways will update
> with any results on build and testing on my side ASAP.
> 
Thanks. It definitely need to rigorous build testing for different 
configurations.

What is the best practice to trigger expansive build tests?
linux-kernel@vger.kernel.org is in CC.

Regards,
Atish
> --
> Regards,
> Sudeep
> 

[RFT PATCH v1 4/4] RISC-V: Parse cpu topology during boot.

[Atish Patra]

Currently, there are no topology defined for RISC-V.
Parse the cpu-map node from device tree and setup the
cpu topology.

CPU topology after applying the patch.
$cat /sys/devices/system/cpu/cpu2/topology/core_siblings_list
0-3
$cat /sys/devices/system/cpu/cpu3/topology/core_siblings_list
0-3
$cat /sys/devices/system/cpu/cpu3/topology/physical_package_id
0
$cat /sys/devices/system/cpu/cpu3/topology/core_id
3

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/Kconfig          | 1 +
 arch/riscv/kernel/smpboot.c | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 55da93f4..b0b1fe1a 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -42,6 +42,7 @@ config RISCV
 	select THREAD_INFO_IN_TASK
 	select RISCV_TIMER
 	select GENERIC_IRQ_MULTI_HANDLER
+	select GENERIC_ARCH_TOPOLOGY if SMP
 	select ARCH_HAS_PTE_SPECIAL
 
 config MMU
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 18cda0e8..5f435af7 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -16,6 +16,7 @@
  * GNU General Public License for more details.
  */
 
+#include <linux/arch_topology.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
@@ -42,6 +43,7 @@ void *__cpu_up_task_pointer[NR_CPUS];
 
 void __init smp_prepare_boot_cpu(void)
 {
+	init_cpu_topology();
 }
 
 void __init smp_prepare_cpus(unsigned int max_cpus)
@@ -115,6 +117,7 @@ asmlinkage void __init smp_callin(void)
 
 	trap_init();
 	notify_cpu_starting(smp_processor_id());
+	update_siblings_masks(smp_processor_id());
 	set_cpu_online(smp_processor_id(), 1);
 	/*
 	 * Remote TLB flushes are ignored while the CPU is offline, so emit
-- 
2.7.4

Re: [RFT PATCH v1 4/4] RISC-V: Parse cpu topology during boot.

[Sudeep Holla]

On Thu, Nov 29, 2018 at 03:28:20PM -0800, Atish Patra wrote:
> Currently, there are no topology defined for RISC-V.
> Parse the cpu-map node from device tree and setup the
> cpu topology.
>
> CPU topology after applying the patch.
> $cat /sys/devices/system/cpu/cpu2/topology/core_siblings_list
> 0-3
> $cat /sys/devices/system/cpu/cpu3/topology/core_siblings_list
> 0-3
> $cat /sys/devices/system/cpu/cpu3/topology/physical_package_id
> 0
> $cat /sys/devices/system/cpu/cpu3/topology/core_id
> 3
>

That looks simple. Good reuse and thanks for unifying.

FWIW,

Acked-by:Sudeep Holla <sudeep.holla@arm.com>

--
Regards,
Sudeep

Re: [RFT PATCH v1 0/4] Unify CPU topology across ARM64 & RISC-V

[Jeffrey Hugo]

On 11/29/2018 4:28 PM, Atish Patra wrote:
> The cpu-map DT entry in ARM64 can describe the CPU topology in
> much better way compared to other existing approaches. RISC-V can
> easily adopt this binding to represent it's 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 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.
> 
> The patches have been tested for RISC-V and compile tested for
> ARM64 & 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/riscv-qemu/tree/cpu_topo
> 
> Apologies for the previous patch series with incorrect title and
> was sent only to kernel mailing list due to a bug in my config.
> Please ignore that.
> 
> Atish Patra (3):
> dt-binding: cpu-topology: Move cpu-map to a common binding.
> cpu-topology: Move cpu topology code to common code.
> RISC-V: Parse cpu topology during boot.
> 
> Sudeep Holla (1):
> Documentation: DT: arm: add support for sockets defining package
> boundaries
> 
> .../{arm/topology.txt => cpu/cpu-topology.txt}     | 133 +++++++--
> arch/arm64/include/asm/topology.h                  |  22 --
> arch/arm64/kernel/topology.c                       | 303 +--------------------
> arch/riscv/Kconfig                                 |   1 +
> arch/riscv/kernel/smpboot.c                        |   3 +
> drivers/base/arch_topology.c                       | 294 ++++++++++++++++++++
> include/linux/arch_topology.h                      |  26 ++
> include/linux/topology.h                           |   1 +
> 8 files changed, 435 insertions(+), 348 deletions(-)
> rename Documentation/devicetree/bindings/{arm/topology.txt => cpu/cpu-topology.txt} (66%)
> 
> --
> 2.7.4
> 

Seems to test fine on QDF2400.

Tested-by: Jeffrey Hugo <jhugo@codeaurora.org>

I did see that git am complained about patch #2 -

patch:103: space before tab in indent.
                         };
patch:114: space before tab in indent.
         };
warning: 2 lines add whitespace errors.


-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [RFT PATCH v1 0/4] Unify CPU topology across ARM64 & RISC-V

[Atish Patra]

On 12/5/18 9:53 AM, Jeffrey Hugo wrote:
> On 11/29/2018 4:28 PM, Atish Patra wrote:
>> The cpu-map DT entry in ARM64 can describe the CPU topology in
>> much better way compared to other existing approaches. RISC-V can
>> easily adopt this binding to represent it's 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 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.
>>
>> The patches have been tested for RISC-V and compile tested for
>> ARM64 & 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/riscv-qemu/tree/cpu_topo
>>
>> Apologies for the previous patch series with incorrect title and
>> was sent only to kernel mailing list due to a bug in my config.
>> Please ignore that.
>>
>> Atish Patra (3):
>> dt-binding: cpu-topology: Move cpu-map to a common binding.
>> cpu-topology: Move cpu topology code to common code.
>> RISC-V: Parse cpu topology during boot.
>>
>> Sudeep Holla (1):
>> Documentation: DT: arm: add support for sockets defining package
>> boundaries
>>
>> .../{arm/topology.txt => cpu/cpu-topology.txt}     | 133 +++++++--
>> arch/arm64/include/asm/topology.h                  |  22 --
>> arch/arm64/kernel/topology.c                       | 303 +--------------------
>> arch/riscv/Kconfig                                 |   1 +
>> arch/riscv/kernel/smpboot.c                        |   3 +
>> drivers/base/arch_topology.c                       | 294 ++++++++++++++++++++
>> include/linux/arch_topology.h                      |  26 ++
>> include/linux/topology.h                           |   1 +
>> 8 files changed, 435 insertions(+), 348 deletions(-)
>> rename Documentation/devicetree/bindings/{arm/topology.txt => cpu/cpu-topology.txt} (66%)
>>
>> --
>> 2.7.4
>>
> 
> Seems to test fine on QDF2400.
> 
> Tested-by: Jeffrey Hugo <jhugo@codeaurora.org>
>


Thanks for verifying the patches.

> I did see that git am complained about patch #2 -
> 
> patch:103: space before tab in indent.
>                           };
> patch:114: space before tab in indent.
>           };
> warning: 2 lines add whitespace errors.
> 
> 

Thanks for pointing it out. For some reason, cherry-pick did not 
complain about these. I will fix them.

Regards,
Atish

Re: [RFT PATCH v1 0/4] Unify CPU topology across ARM64 & RISC-V

[Morten Rasmussen]

Hi,

On Thu, Nov 29, 2018 at 03:28:16PM -0800, Atish Patra wrote:
> The cpu-map DT entry in ARM64 can describe the CPU topology in
> much better way compared to other existing approaches. RISC-V can
> easily adopt this binding to represent it's 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 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.
> 
> The patches have been tested for RISC-V and compile tested for
> ARM64 & x86.

The cpu-map bindings are used for arch/arm too, and so is
arch_topology.c. In fact, it was introduced to allow code-sharing
between arm and arm64. Applying patch three breaks arm.

Moving the DT parsing to arch_topology.c we have to unify all three
architectures. Be aware that arm and arm64 have some differences in how
they detect cpu capacities. I think we might have to look at the split
of code between arch/* and arch_topology.c again :-/

Morten

Re: [RFT PATCH v1 0/4] Unify CPU topology across ARM64 & RISC-V

[Sudeep Holla]

On Fri, Dec 07, 2018 at 01:45:21PM +0000, Morten Rasmussen wrote:
> Hi,
>
> On Thu, Nov 29, 2018 at 03:28:16PM -0800, Atish Patra wrote:
> > The cpu-map DT entry in ARM64 can describe the CPU topology in
> > much better way compared to other existing approaches. RISC-V can
> > easily adopt this binding to represent it's 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 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.
> >
> > The patches have been tested for RISC-V and compile tested for
> > ARM64 & x86.
>
> The cpu-map bindings are used for arch/arm too, and so is
> arch_topology.c. In fact, it was introduced to allow code-sharing
> between arm and arm64. Applying patch three breaks arm.
>

Ah right. Though I remember the whole point of moving these to arch_topology
was to share between ARM and ARM64, I completely forgot to check the
impact of this for ARM platforms.

> Moving the DT parsing to arch_topology.c we have to unify all three
> architectures. Be aware that arm and arm64 have some differences in how
> they detect cpu capacities. I think we might have to look at the split
> of code between arch/* and arch_topology.c again :-/
>

Thanks for pointing this out. I completely agree and apologies for
forgetting about arm32.

Regards,
Sudeep

Re: [RFT PATCH v1 0/4] Unify CPU topology across ARM64 & RISC-V

[Atish Patra]

On 12/7/18 5:45 AM, Morten Rasmussen wrote:
> Hi,
> 
> On Thu, Nov 29, 2018 at 03:28:16PM -0800, Atish Patra wrote:
>> The cpu-map DT entry in ARM64 can describe the CPU topology in
>> much better way compared to other existing approaches. RISC-V can
>> easily adopt this binding to represent it's 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 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.
>>
>> The patches have been tested for RISC-V and compile tested for
>> ARM64 & x86.
> 
> The cpu-map bindings are used for arch/arm too, and so is
> arch_topology.c. In fact, it was introduced to allow code-sharing
> between arm and arm64. Applying patch three breaks arm.
> 
> Moving the DT parsing to arch_topology.c we have to unify all three
> architectures. Be aware that arm and arm64 have some differences in how
> they detect cpu capacities. I think we might have to look at the split
> of code between arch/* and arch_topology.c again :-/
> 
> Morten
> 
Thank you for bringing this up. I will send a new version and make sure 
that it works on arm32 as well.


Regards,
Atish