[RFC PATCH] Documentation: DT: arm: Add topology property to define package boundaries

[RFC PATCH] Documentation: DT: arm: Add topology property to define package boundaries

[Lorenzo Pieralisi]

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.

As opposed to the ACPI topology description ([1], PPTT table), this
hierarchical representation of clusters does not allow to describe what
topology level actually represents the physical package boundary, which
is a key piece of information to be used by an operating system to
optimize resource allocation and scheduling.

Define an optional, backward compatible boolean property for cluster
nodes that, by reusing the ACPI nomenclature, add to the ARM DT
topological description a binding to define what cluster level
represents a physical package boundary.

[1] http://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
Cc: Jeremy Linton <jeremy.linton-5wv7dgnIgG8@public.gmane.org>
Cc: Morten Rasmussen <morten.rasmussen-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
---
 Documentation/devicetree/bindings/arm/topology.txt | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/arm/topology.txt
index de9eb0486630..8e78d76b0671 100644
--- a/Documentation/devicetree/bindings/arm/topology.txt
+++ b/Documentation/devicetree/bindings/arm/topology.txt
@@ -109,6 +109,15 @@ Bindings for cluster/cpu/thread nodes are defined as follows:
 	The cluster node name must be "clusterN" as described in 2.1 above.
 	A cluster node can not be a leaf node.
 
+	Properties for cluster nodes:
+
+	- physical-package
+		Usage: optional
+		Value type: <empty>
+		Definition: if present the cluster node represents the
+			    boundary of a physical package, whether socketed
+			    or surface mounted.
+
 	A cluster node's child nodes must be:
 
 	- one or more cluster nodes; or
-- 
2.15.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] Documentation: DT: arm: Add topology property to define package boundaries

[Sudeep Holla]

On 22/01/18 17:15, Lorenzo Pieralisi wrote:
> 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.
> 
> As opposed to the ACPI topology description ([1], PPTT table), this
> hierarchical representation of clusters does not allow to describe what
> topology level actually represents the physical package boundary, which
> is a key piece of information to be used by an operating system to
> optimize resource allocation and scheduling.
> 
> Define an optional, backward compatible boolean property for cluster
> nodes that, by reusing the ACPI nomenclature, add to the ARM DT
> topological description a binding to define what cluster level
> represents a physical package boundary.
> 
> [1] http://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
> Cc: Jeremy Linton <jeremy.linton-5wv7dgnIgG8@public.gmane.org>
> Cc: Morten Rasmussen <morten.rasmussen-5wv7dgnIgG8@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/arm/topology.txt | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/arm/topology.txt
> index de9eb0486630..8e78d76b0671 100644
> --- a/Documentation/devicetree/bindings/arm/topology.txt
> +++ b/Documentation/devicetree/bindings/arm/topology.txt
> @@ -109,6 +109,15 @@ Bindings for cluster/cpu/thread nodes are defined as follows:
>  	The cluster node name must be "clusterN" as described in 2.1 above.
>  	A cluster node can not be a leaf node.
>  
> +	Properties for cluster nodes:
> +
> +	- physical-package
> +		Usage: optional
> +		Value type: <empty>

IIUC, value type must be specified as boolean in this case.

> +		Definition: if present the cluster node represents the
> +			    boundary of a physical package, whether socketed
> +			    or surface mounted.
> +
>  	A cluster node's child nodes must be:
>  
>  	- one or more cluster nodes; or
> 

Otherwise looks good to me.

-- 
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] Documentation: DT: arm: Add topology property to define package boundaries

[Jeremy Linton]

Hi,

On 01/22/2018 11:15 AM, Lorenzo Pieralisi wrote:
> 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.
> 
> As opposed to the ACPI topology description ([1], PPTT table), this
> hierarchical representation of clusters does not allow to describe what
> topology level actually represents the physical package boundary, which
> is a key piece of information to be used by an operating system to
> optimize resource allocation and scheduling.
> 
> Define an optional, backward compatible boolean property for cluster
> nodes that, by reusing the ACPI nomenclature, add to the ARM DT
> topological description a binding to define what cluster level
> represents a physical package boundary.
> 
> [1] http://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
> Cc: Jeremy Linton <jeremy.linton-5wv7dgnIgG8@public.gmane.org>
> Cc: Morten Rasmussen <morten.rasmussen-5wv7dgnIgG8@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> ---
>   Documentation/devicetree/bindings/arm/topology.txt | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/arm/topology.txt
> index de9eb0486630..8e78d76b0671 100644
> --- a/Documentation/devicetree/bindings/arm/topology.txt
> +++ b/Documentation/devicetree/bindings/arm/topology.txt
> @@ -109,6 +109,15 @@ Bindings for cluster/cpu/thread nodes are defined as follows:
>   	The cluster node name must be "clusterN" as described in 2.1 above.
>   	A cluster node can not be a leaf node.
>   
> +	Properties for cluster nodes:
> +
> +	- physical-package
> +		Usage: optional
> +		Value type: <empty>
> +		Definition: if present the cluster node represents the
> +			    boundary of a physical package, whether socketed
> +			    or surface mounted.
> +
>   	A cluster node's child nodes must be:
>   
>   	- one or more cluster nodes; or
> 

Looks good so far, but I would worry about it being optional. That 
seemed like a mistake on the ACPI side.

Assuming it is optional, it might be worth a sentence explaining what 
the fallback behavior is if the property is missing.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] Documentation: DT: arm: Add topology property to define package boundaries

[Frank Rowand]

On 01/22/18 09:15, Lorenzo Pieralisi wrote:
> 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.
> 
> As opposed to the ACPI topology description ([1], PPTT table), this
> hierarchical representation of clusters does not allow to describe what
> topology level actually represents the physical package boundary, which
> is a key piece of information to be used by an operating system to
> optimize resource allocation and scheduling.
> 
> Define an optional, backward compatible boolean property for cluster
> nodes that, by reusing the ACPI nomenclature, add to the ARM DT
> topological description a binding to define what cluster level
> represents a physical package boundary.
> 
> [1] http://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> Cc: Morten Rasmussen <morten.rasmussen@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---
>  Documentation/devicetree/bindings/arm/topology.txt | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/arm/topology.txt
> index de9eb0486630..8e78d76b0671 100644
> --- a/Documentation/devicetree/bindings/arm/topology.txt
> +++ b/Documentation/devicetree/bindings/arm/topology.txt
> @@ -109,6 +109,15 @@ Bindings for cluster/cpu/thread nodes are defined as follows:
>  	The cluster node name must be "clusterN" as described in 2.1 above.
>  	A cluster node can not be a leaf node.
>  
> +	Properties for cluster nodes:
> +
> +	- physical-package
> +		Usage: optional
> +		Value type: <empty>
> +		Definition: if present the cluster node represents the
> +			    boundary of a physical package, whether socketed
> +			    or surface mounted.

I don't know how to interpret this.  Is the node with this property inside
or outside the boundary?  If I had to guess, I would guess inside.  A few
extra words to clarify this please.


> +
>  	A cluster node's child nodes must be:
>  
>  	- one or more cluster nodes; or
> 

Re: [RFC PATCH] Documentation: DT: arm: Add topology property to define package boundaries

[Sudeep Holla]

On 22/01/18 23:25, Jeremy Linton wrote:
> Hi,
> 
> On 01/22/2018 11:15 AM, Lorenzo Pieralisi wrote:
>> 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.
>>
>> As opposed to the ACPI topology description ([1], PPTT table), this
>> hierarchical representation of clusters does not allow to describe what
>> topology level actually represents the physical package boundary, which
>> is a key piece of information to be used by an operating system to
>> optimize resource allocation and scheduling.
>>
>> Define an optional, backward compatible boolean property for cluster
>> nodes that, by reusing the ACPI nomenclature, add to the ARM DT
>> topological description a binding to define what cluster level
>> represents a physical package boundary.
>>
>> [1] http://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf
>>
>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
>> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Cc: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
>> Cc: Jeremy Linton <jeremy.linton-5wv7dgnIgG8@public.gmane.org>
>> Cc: Morten Rasmussen <morten.rasmussen-5wv7dgnIgG8@public.gmane.org>
>> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>> ---
>>   Documentation/devicetree/bindings/arm/topology.txt | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/topology.txt
>> b/Documentation/devicetree/bindings/arm/topology.txt
>> index de9eb0486630..8e78d76b0671 100644
>> --- a/Documentation/devicetree/bindings/arm/topology.txt
>> +++ b/Documentation/devicetree/bindings/arm/topology.txt
>> @@ -109,6 +109,15 @@ Bindings for cluster/cpu/thread nodes are defined
>> as follows:
>>       The cluster node name must be "clusterN" as described in 2.1 above.
>>       A cluster node can not be a leaf node.
>>   +    Properties for cluster nodes:
>> +
>> +    - physical-package
>> +        Usage: optional
>> +        Value type: <empty>
>> +        Definition: if present the cluster node represents the
>> +                boundary of a physical package, whether socketed
>> +                or surface mounted.
>> +
>>       A cluster node's child nodes must be:
>>         - one or more cluster nodes; or
>>
> 
> Looks good so far, but I would worry about it being optional. 

We have no choice, it has to be optional for compatibility reasons as
stated in the commit message.

> That seemed like a mistake on the ACPI side.

Not necessarily. It's firmware choice as long as it knows how to deal
with the default(absence in case of DT)

> 
> Assuming it is optional, it might be worth a sentence explaining what
> the fallback behavior is if the property is missing.

That would be helpful.

-- 
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] Documentation: DT: arm: Add topology property to define package boundaries

[Lorenzo Pieralisi]

On Mon, Jan 22, 2018 at 08:45:26PM -0800, Frank Rowand wrote:
> On 01/22/18 09:15, Lorenzo Pieralisi wrote:
> > 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.
> > 
> > As opposed to the ACPI topology description ([1], PPTT table), this
> > hierarchical representation of clusters does not allow to describe what
> > topology level actually represents the physical package boundary, which
> > is a key piece of information to be used by an operating system to
> > optimize resource allocation and scheduling.
> > 
> > Define an optional, backward compatible boolean property for cluster
> > nodes that, by reusing the ACPI nomenclature, add to the ARM DT
> > topological description a binding to define what cluster level
> > represents a physical package boundary.
> > 
> > [1] http://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > Cc: Jeremy Linton <jeremy.linton@arm.com>
> > Cc: Morten Rasmussen <morten.rasmussen@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > ---
> >  Documentation/devicetree/bindings/arm/topology.txt | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/arm/topology.txt
> > index de9eb0486630..8e78d76b0671 100644
> > --- a/Documentation/devicetree/bindings/arm/topology.txt
> > +++ b/Documentation/devicetree/bindings/arm/topology.txt
> > @@ -109,6 +109,15 @@ Bindings for cluster/cpu/thread nodes are defined as follows:
> >  	The cluster node name must be "clusterN" as described in 2.1 above.
> >  	A cluster node can not be a leaf node.
> >  
> > +	Properties for cluster nodes:
> > +
> > +	- physical-package
> > +		Usage: optional
> > +		Value type: <empty>
> > +		Definition: if present the cluster node represents the
> > +			    boundary of a physical package, whether socketed
> > +			    or surface mounted.
> 
> I don't know how to interpret this.  Is the node with this property inside
> or outside the boundary?  If I had to guess, I would guess inside.  A few
> extra words to clarify this please.

The node is neither inside nor outside, it _is_ the boundary. Every node
defines a topology level - the property is there to define which one
corresponds to a package, please let me know if it makes things clearer.

Thanks,
Lorenzo

> > +
> >  	A cluster node's child nodes must be:
> >  
> >  	- one or more cluster nodes; or
> > 
> 

Re: [RFC PATCH] Documentation: DT: arm: Add topology property to define package boundaries

[Lorenzo Pieralisi]

On Mon, Jan 22, 2018 at 05:29:20PM +0000, Sudeep Holla wrote:
> 
> 
> On 22/01/18 17:15, Lorenzo Pieralisi wrote:
> > 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.
> > 
> > As opposed to the ACPI topology description ([1], PPTT table), this
> > hierarchical representation of clusters does not allow to describe what
> > topology level actually represents the physical package boundary, which
> > is a key piece of information to be used by an operating system to
> > optimize resource allocation and scheduling.
> > 
> > Define an optional, backward compatible boolean property for cluster
> > nodes that, by reusing the ACPI nomenclature, add to the ARM DT
> > topological description a binding to define what cluster level
> > represents a physical package boundary.
> > 
> > [1] http://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> > Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Cc: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
> > Cc: Jeremy Linton <jeremy.linton-5wv7dgnIgG8@public.gmane.org>
> > Cc: Morten Rasmussen <morten.rasmussen-5wv7dgnIgG8@public.gmane.org>
> > Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> > ---
> >  Documentation/devicetree/bindings/arm/topology.txt | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/arm/topology.txt
> > index de9eb0486630..8e78d76b0671 100644
> > --- a/Documentation/devicetree/bindings/arm/topology.txt
> > +++ b/Documentation/devicetree/bindings/arm/topology.txt
> > @@ -109,6 +109,15 @@ Bindings for cluster/cpu/thread nodes are defined as follows:
> >  	The cluster node name must be "clusterN" as described in 2.1 above.
> >  	A cluster node can not be a leaf node.
> >  
> > +	Properties for cluster nodes:
> > +
> > +	- physical-package
> > +		Usage: optional
> > +		Value type: <empty>
> 
> IIUC, value type must be specified as boolean in this case.

That's what I thought too but according to DT specs v0.2, table 2.3 the
property value type is <empty>.

I do not mind changing it - whatever DT maintainers think is more
appropriate.

Thanks,
Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] Documentation: DT: arm: Add topology property to define package boundaries

[Frank Rowand]

On 02/08/18 02:57, Lorenzo Pieralisi wrote:
> On Mon, Jan 22, 2018 at 08:45:26PM -0800, Frank Rowand wrote:
>> On 01/22/18 09:15, Lorenzo Pieralisi wrote:
>>> 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.
>>>
>>> As opposed to the ACPI topology description ([1], PPTT table), this
>>> hierarchical representation of clusters does not allow to describe what
>>> topology level actually represents the physical package boundary, which
>>> is a key piece of information to be used by an operating system to
>>> optimize resource allocation and scheduling.
>>>
>>> Define an optional, backward compatible boolean property for cluster
>>> nodes that, by reusing the ACPI nomenclature, add to the ARM DT
>>> topological description a binding to define what cluster level
>>> represents a physical package boundary.
>>>
>>> [1] http://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf
>>>
>>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
>>> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>> Cc: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
>>> Cc: Jeremy Linton <jeremy.linton-5wv7dgnIgG8@public.gmane.org>
>>> Cc: Morten Rasmussen <morten.rasmussen-5wv7dgnIgG8@public.gmane.org>
>>> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>>> ---
>>>  Documentation/devicetree/bindings/arm/topology.txt | 9 +++++++++
>>>  1 file changed, 9 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/arm/topology.txt
>>> index de9eb0486630..8e78d76b0671 100644
>>> --- a/Documentation/devicetree/bindings/arm/topology.txt
>>> +++ b/Documentation/devicetree/bindings/arm/topology.txt
>>> @@ -109,6 +109,15 @@ Bindings for cluster/cpu/thread nodes are defined as follows:
>>>  	The cluster node name must be "clusterN" as described in 2.1 above.
>>>  	A cluster node can not be a leaf node.
>>>  
>>> +	Properties for cluster nodes:
>>> +
>>> +	- physical-package
>>> +		Usage: optional
>>> +		Value type: <empty>
>>> +		Definition: if present the cluster node represents the
>>> +			    boundary of a physical package, whether socketed
>>> +			    or surface mounted.
>>
>> I don't know how to interpret this.  Is the node with this property inside
>> or outside the boundary?  If I had to guess, I would guess inside.  A few
>> extra words to clarify this please.
> 
> The node is neither inside nor outside, it _is_ the boundary. Every node
> defines a topology level - the property is there to define which one
> corresponds to a package, please let me know if it makes things clearer.

Not at all clear.

Using Example 1, from section "4 - Example dts" of topology.txt:


       cpu-map {
                cluster0 {
                        cluster0 {
                                core0 {
                                        thread0 {
                                                cpu = <&CPU0>;
                                        };
                                        thread1 {
                                                cpu = <&CPU1>;
                                        };
                                };

                                core1 {
                                        thread0 {
                                                cpu = <&CPU2>;
                                        };
                                        thread1 {
                                                cpu = <&CPU3>;
                                        };
                                };
                        };

                        cluster1 {
                                core0 {
                                        thread0 {
                                                cpu = <&CPU4>;
                                        };
                                        thread1 {
                                                cpu = <&CPU5>;
                                        };
                                };

                                core1 {
                                        thread0 {
                                                cpu = <&CPU6>;
                                        };
                                        thread1 {
                                                cpu = <&CPU7>;
                                        };
                                };
                        };
                };

Pretend that cpu-map/cluster0/cluster0 is a physical package that
contains two cores, and cpu-map/cluster0/cluster1 is another
physical package that contains two cores.  My guess as to how
to use the property "physical-package" would be to place it
in nodes cpu-map/cluster0/cluster0 and cpu-map/cluster0/cluster1.
In that case, those two nodes are on the "inside" of two different
packages.

The alternate way to use the property "physical-package" would be
to place it in node cpu-map/cluster0.  In this case, the node is
"outside" of the packages.

Again, I suspect that the intended use is the first of my two
examples, but the proposed binding wording does not make that
clear to me.  My use of "inside" and "outside" may not be the
proper words or concept, but the binding somehow needs to
say which of my two above example locations is the correct place
to use the "physical-package" property.

-Frank

> 
> Thanks,
> Lorenzo
> 
>>> +
>>>  	A cluster node's child nodes must be:
>>>  
>>>  	- one or more cluster nodes; or
>>>
>>
> .
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] Documentation: DT: arm: Add topology property to define package boundaries

[Lorenzo Pieralisi]

On Thu, Feb 08, 2018 at 02:22:00PM -0800, Frank Rowand wrote:
> On 02/08/18 02:57, Lorenzo Pieralisi wrote:
> > On Mon, Jan 22, 2018 at 08:45:26PM -0800, Frank Rowand wrote:
> >> On 01/22/18 09:15, Lorenzo Pieralisi wrote:
> >>> 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.
> >>>
> >>> As opposed to the ACPI topology description ([1], PPTT table), this
> >>> hierarchical representation of clusters does not allow to describe what
> >>> topology level actually represents the physical package boundary, which
> >>> is a key piece of information to be used by an operating system to
> >>> optimize resource allocation and scheduling.
> >>>
> >>> Define an optional, backward compatible boolean property for cluster
> >>> nodes that, by reusing the ACPI nomenclature, add to the ARM DT
> >>> topological description a binding to define what cluster level
> >>> represents a physical package boundary.
> >>>
> >>> [1] http://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf
> >>>
> >>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> >>> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >>> Cc: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
> >>> Cc: Jeremy Linton <jeremy.linton-5wv7dgnIgG8@public.gmane.org>
> >>> Cc: Morten Rasmussen <morten.rasmussen-5wv7dgnIgG8@public.gmane.org>
> >>> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> >>> ---
> >>>  Documentation/devicetree/bindings/arm/topology.txt | 9 +++++++++
> >>>  1 file changed, 9 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/arm/topology.txt
> >>> index de9eb0486630..8e78d76b0671 100644
> >>> --- a/Documentation/devicetree/bindings/arm/topology.txt
> >>> +++ b/Documentation/devicetree/bindings/arm/topology.txt
> >>> @@ -109,6 +109,15 @@ Bindings for cluster/cpu/thread nodes are defined as follows:
> >>>  	The cluster node name must be "clusterN" as described in 2.1 above.
> >>>  	A cluster node can not be a leaf node.
> >>>  
> >>> +	Properties for cluster nodes:
> >>> +
> >>> +	- physical-package
> >>> +		Usage: optional
> >>> +		Value type: <empty>
> >>> +		Definition: if present the cluster node represents the
> >>> +			    boundary of a physical package, whether socketed
> >>> +			    or surface mounted.
> >>
> >> I don't know how to interpret this.  Is the node with this property inside
> >> or outside the boundary?  If I had to guess, I would guess inside.  A few
> >> extra words to clarify this please.
> > 
> > The node is neither inside nor outside, it _is_ the boundary. Every node
> > defines a topology level - the property is there to define which one
> > corresponds to a package, please let me know if it makes things clearer.
> 
> Not at all clear.
> 
> Using Example 1, from section "4 - Example dts" of topology.txt:
> 
> 
>        cpu-map {
>                 cluster0 {
>                         cluster0 {
>                                 core0 {
>                                         thread0 {
>                                                 cpu = <&CPU0>;
>                                         };
>                                         thread1 {
>                                                 cpu = <&CPU1>;
>                                         };
>                                 };
> 
>                                 core1 {
>                                         thread0 {
>                                                 cpu = <&CPU2>;
>                                         };
>                                         thread1 {
>                                                 cpu = <&CPU3>;
>                                         };
>                                 };
>                         };
> 
>                         cluster1 {
>                                 core0 {
>                                         thread0 {
>                                                 cpu = <&CPU4>;
>                                         };
>                                         thread1 {
>                                                 cpu = <&CPU5>;
>                                         };
>                                 };
> 
>                                 core1 {
>                                         thread0 {
>                                                 cpu = <&CPU6>;
>                                         };
>                                         thread1 {
>                                                 cpu = <&CPU7>;
>                                         };
>                                 };
>                         };
>                 };
> 
> Pretend that cpu-map/cluster0/cluster0 is a physical package that
> contains two cores, and cpu-map/cluster0/cluster1 is another
> physical package that contains two cores.  My guess as to how
> to use the property "physical-package" would be to place it
> in nodes cpu-map/cluster0/cluster0 and cpu-map/cluster0/cluster1.
> In that case, those two nodes are on the "inside" of two different
> packages.
> 
> The alternate way to use the property "physical-package" would be
> to place it in node cpu-map/cluster0.  In this case, the node is
> "outside" of the packages.
> 
> Again, I suspect that the intended use is the first of my two
> examples, but the proposed binding wording does not make that
> clear to me.  My use of "inside" and "outside" may not be the
> proper words or concept, but the binding somehow needs to
> say which of my two above example locations is the correct place
> to use the "physical-package" property.

Ok, I see, I will update the description accordingly to make it
clearer.

Thank you,
Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html