All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5] Documentation, dt, numa: Add note to empty NUMA node
@ 2021-06-28  9:34 Gavin Shan
  2021-06-28 16:15 ` Randy Dunlap
  2021-07-01 17:25 ` Rob Herring
  0 siblings, 2 replies; 8+ messages in thread
From: Gavin Shan @ 2021-06-28  9:34 UTC (permalink / raw)
  To: devicetree; +Cc: linux-kernel, rdunlap, drjones, robh+dt, shan.gavin

The empty memory nodes, where no memory resides in, are allowed.
For these empty memory nodes, the 'len' of 'reg' property is zero.
The NUMA node IDs are still valid and parsed, but memory may be
added to them through hotplug afterwards. I finds difficulty to
get where it's properly documented.

So lets add a section for empty memory nodes in NUMA binding
document. Also, the 'unit-address', equivalent to 'base-address'
in the 'reg' property of these empty memory nodes is suggested to
be the summation of highest memory address plus the NUMA node ID.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
v5: Separate section for empty memory node
---
 Documentation/devicetree/bindings/numa.txt | 61 +++++++++++++++++++++-
 1 file changed, 60 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/numa.txt b/Documentation/devicetree/bindings/numa.txt
index 21b35053ca5a..230c734af948 100644
--- a/Documentation/devicetree/bindings/numa.txt
+++ b/Documentation/devicetree/bindings/numa.txt
@@ -103,7 +103,66 @@ Example:
 		};
 
 ==============================================================================
-4 - Example dts
+4 - Empty memory nodes
+==============================================================================
+
+Empty memory nodes, which no memory resides in, are allowed. The 'length'
+field of the 'reg' property is zero, but the 'base-address' is a dummy
+address and invalid. The 'base-address' could be the summation of highest
+memory address plus the NUMA node ID. However, the NUMA node IDs and
+distance maps are still valid and memory may be added into them through
+hotplug afterwards.
+
+Example:
+
+	memory@0 {
+		device_type = "memory";
+		reg = <0x0 0x0 0x0 0x80000000>;
+		numa-node-id = <0>;
+	};
+
+	memory@0x80000000 {
+		device_type = "memory";
+		reg = <0x0 0x80000000 0x0 0x80000000>;
+		numa-node-id = <1>;
+	};
+
+	/* Empty memory node */
+	memory@0x100000002 {
+		device_type = "memory";
+		reg = <0x1 0x2 0x0 0x0>;
+		numa-node-id = <2>;
+	};
+
+	/* Empty memory node */
+	memory@0x100000003 {
+		device_type = "memory";
+		reg = <0x1 0x3 0x0 0x0>;
+		numa-node-id = <3>;
+	};
+
+	distance-map {
+		compatible = "numa-distance-map-v1";
+		distance-matrix = <0 0  10>,
+				  <0 1  20>,
+				  <0 2  40>,
+				  <0 3  20>,
+				  <1 0  20>,
+				  <1 1  10>,
+				  <1 2  20>,
+				  <1 3  40>,
+				  <2 0  40>,
+				  <2 1  20>,
+				  <2 2  10>,
+				  <2 3  20>,
+				  <3 0  20>,
+				  <3 1  40>,
+				  <3 2  20>,
+				  <3 3  10>;
+	};
+
+==============================================================================
+5 - Example dts
 ==============================================================================
 
 Dual socket system consists of 2 boards connected through ccn bus and
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v5] Documentation, dt, numa: Add note to empty NUMA node
  2021-06-28  9:34 [PATCH v5] Documentation, dt, numa: Add note to empty NUMA node Gavin Shan
@ 2021-06-28 16:15 ` Randy Dunlap
  2021-06-30  7:26   ` Gavin Shan
  2021-07-01 17:25 ` Rob Herring
  1 sibling, 1 reply; 8+ messages in thread
From: Randy Dunlap @ 2021-06-28 16:15 UTC (permalink / raw)
  To: Gavin Shan, devicetree; +Cc: linux-kernel, drjones, robh+dt, shan.gavin

On 6/28/21 2:34 AM, Gavin Shan wrote:
> The empty memory nodes, where no memory resides in, are allowed.
> For these empty memory nodes, the 'len' of 'reg' property is zero.
> The NUMA node IDs are still valid and parsed, but memory may be
> added to them through hotplug afterwards. I finds difficulty to
> get where it's properly documented.
> 
> So lets add a section for empty memory nodes in NUMA binding
> document. Also, the 'unit-address', equivalent to 'base-address'
> in the 'reg' property of these empty memory nodes is suggested to
> be the summation of highest memory address plus the NUMA node ID.
> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
> v5: Separate section for empty memory node
> ---
>  Documentation/devicetree/bindings/numa.txt | 61 +++++++++++++++++++++-
>  1 file changed, 60 insertions(+), 1 deletion(-)
> 

LGTM. Thanks.

Acked-by: Randy Dunlap <rdunlap@infradead.org>

-- 
~Randy

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5] Documentation, dt, numa: Add note to empty NUMA node
  2021-06-28 16:15 ` Randy Dunlap
@ 2021-06-30  7:26   ` Gavin Shan
  0 siblings, 0 replies; 8+ messages in thread
From: Gavin Shan @ 2021-06-30  7:26 UTC (permalink / raw)
  To: Randy Dunlap, devicetree; +Cc: linux-kernel, drjones, robh+dt, shan.gavin

On 6/29/21 2:15 AM, Randy Dunlap wrote:
> On 6/28/21 2:34 AM, Gavin Shan wrote:
>> The empty memory nodes, where no memory resides in, are allowed.
>> For these empty memory nodes, the 'len' of 'reg' property is zero.
>> The NUMA node IDs are still valid and parsed, but memory may be
>> added to them through hotplug afterwards. I finds difficulty to
>> get where it's properly documented.
>>
>> So lets add a section for empty memory nodes in NUMA binding
>> document. Also, the 'unit-address', equivalent to 'base-address'
>> in the 'reg' property of these empty memory nodes is suggested to
>> be the summation of highest memory address plus the NUMA node ID.
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>> v5: Separate section for empty memory node
>> ---
>>   Documentation/devicetree/bindings/numa.txt | 61 +++++++++++++++++++++-
>>   1 file changed, 60 insertions(+), 1 deletion(-)
>>
> 
> LGTM. Thanks.
> 
> Acked-by: Randy Dunlap <rdunlap@infradead.org>
> 

Thanks, Randy. Rob, could you help to review? Thanks in advance.

Thanks,
Gavin


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5] Documentation, dt, numa: Add note to empty NUMA node
  2021-06-28  9:34 [PATCH v5] Documentation, dt, numa: Add note to empty NUMA node Gavin Shan
  2021-06-28 16:15 ` Randy Dunlap
@ 2021-07-01 17:25 ` Rob Herring
  2021-07-02  0:02   ` Gavin Shan
  1 sibling, 1 reply; 8+ messages in thread
From: Rob Herring @ 2021-07-01 17:25 UTC (permalink / raw)
  To: Gavin Shan; +Cc: devicetree, linux-kernel, rdunlap, drjones, shan.gavin

On Mon, Jun 28, 2021 at 05:34:11PM +0800, Gavin Shan wrote:
> The empty memory nodes, where no memory resides in, are allowed.
> For these empty memory nodes, the 'len' of 'reg' property is zero.
> The NUMA node IDs are still valid and parsed, but memory may be
> added to them through hotplug afterwards. I finds difficulty to
> get where it's properly documented.

This is already in use? If so, what platform(s)?

> So lets add a section for empty memory nodes in NUMA binding
> document. Also, the 'unit-address', equivalent to 'base-address'
> in the 'reg' property of these empty memory nodes is suggested to
> be the summation of highest memory address plus the NUMA node ID.

What purpose does this serve? The kernel won't do anything with it other 
than validate the numa-node-id range.

> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
> v5: Separate section for empty memory node
> ---
>  Documentation/devicetree/bindings/numa.txt | 61 +++++++++++++++++++++-
>  1 file changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/numa.txt b/Documentation/devicetree/bindings/numa.txt
> index 21b35053ca5a..230c734af948 100644
> --- a/Documentation/devicetree/bindings/numa.txt
> +++ b/Documentation/devicetree/bindings/numa.txt
> @@ -103,7 +103,66 @@ Example:
>  		};
>  
>  ==============================================================================
> -4 - Example dts
> +4 - Empty memory nodes
> +==============================================================================
> +
> +Empty memory nodes, which no memory resides in, are allowed. The 'length'
> +field of the 'reg' property is zero, but the 'base-address' is a dummy
> +address and invalid. The 'base-address' could be the summation of highest
> +memory address plus the NUMA node ID. However, the NUMA node IDs and
> +distance maps are still valid and memory may be added into them through
> +hotplug afterwards.
> +
> +Example:
> +
> +	memory@0 {
> +		device_type = "memory";
> +		reg = <0x0 0x0 0x0 0x80000000>;
> +		numa-node-id = <0>;
> +	};
> +
> +	memory@0x80000000 {

unit-address should not have '0x'.

> +		device_type = "memory";
> +		reg = <0x0 0x80000000 0x0 0x80000000>;
> +		numa-node-id = <1>;
> +	};
> +
> +	/* Empty memory node */
> +	memory@0x100000002 {
> +		device_type = "memory";
> +		reg = <0x1 0x2 0x0 0x0>;
> +		numa-node-id = <2>;
> +	};
> +
> +	/* Empty memory node */
> +	memory@0x100000003 {
> +		device_type = "memory";
> +		reg = <0x1 0x3 0x0 0x0>;
> +		numa-node-id = <3>;
> +	};
> +
> +	distance-map {
> +		compatible = "numa-distance-map-v1";
> +		distance-matrix = <0 0  10>,
> +				  <0 1  20>,
> +				  <0 2  40>,
> +				  <0 3  20>,
> +				  <1 0  20>,
> +				  <1 1  10>,
> +				  <1 2  20>,
> +				  <1 3  40>,
> +				  <2 0  40>,
> +				  <2 1  20>,
> +				  <2 2  10>,
> +				  <2 3  20>,
> +				  <3 0  20>,
> +				  <3 1  40>,
> +				  <3 2  20>,
> +				  <3 3  10>;
> +	};
> +
> +==============================================================================
> +5 - Example dts
>  ==============================================================================
>  
>  Dual socket system consists of 2 boards connected through ccn bus and
> -- 
> 2.23.0
> 
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5] Documentation, dt, numa: Add note to empty NUMA node
  2021-07-01 17:25 ` Rob Herring
@ 2021-07-02  0:02   ` Gavin Shan
  2021-07-11  0:59     ` Gavin Shan
  2021-07-12 19:44     ` Rob Herring
  0 siblings, 2 replies; 8+ messages in thread
From: Gavin Shan @ 2021-07-02  0:02 UTC (permalink / raw)
  To: Rob Herring; +Cc: devicetree, linux-kernel, rdunlap, drjones, shan.gavin

Hi Rob,

On 7/2/21 3:25 AM, Rob Herring wrote:
> On Mon, Jun 28, 2021 at 05:34:11PM +0800, Gavin Shan wrote:
>> The empty memory nodes, where no memory resides in, are allowed.
>> For these empty memory nodes, the 'len' of 'reg' property is zero.
>> The NUMA node IDs are still valid and parsed, but memory may be
>> added to them through hotplug afterwards. I finds difficulty to
>> get where it's properly documented.
> 
> This is already in use? If so, what platform(s)?
> 

It's not used yet, but will be used by QEMU once this patch is merged.
In QEMU, ARM64 could have multiple empty memory nodes. The corresponding
NUMA ID and distance map are still valid because memory may be added into
these empty memory nodes in future.

For the QEMU case, the names of empty memory nodes are the biggest concern.
According to device-tree specification, the name follows the format of
'memory@unit-address' and the 'unit-address' is equivalent to 'base-address'.
However, the 'base-address' should be invalid one. In current QEMU implementation,
the valid 'base-address' and 'unit-address' are provided to these empty
memory nodes. Another issue in QEMU is trying to populate two empty memory
nodes, which have same names. This leads to failure of device-tree population
because of the duplicated memory node names, blocking VM from booting.

>> So lets add a section for empty memory nodes in NUMA binding
>> document. Also, the 'unit-address', equivalent to 'base-address'
>> in the 'reg' property of these empty memory nodes is suggested to
>> be the summation of highest memory address plus the NUMA node ID.
> 
> What purpose does this serve? The kernel won't do anything with it other
> than validate the numa-node-id range.
> 

As mentioned above, the point is to have dummy, invalid and non-overlapped
'base-address' and 'unit-address' for these empty memory nodes, to avoid
duplicated memory node names in devcie-tree.

>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>> v5: Separate section for empty memory node
>> ---
>>   Documentation/devicetree/bindings/numa.txt | 61 +++++++++++++++++++++-
>>   1 file changed, 60 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/numa.txt b/Documentation/devicetree/bindings/numa.txt
>> index 21b35053ca5a..230c734af948 100644
>> --- a/Documentation/devicetree/bindings/numa.txt
>> +++ b/Documentation/devicetree/bindings/numa.txt
>> @@ -103,7 +103,66 @@ Example:
>>   		};
>>   
>>   ==============================================================================
>> -4 - Example dts
>> +4 - Empty memory nodes
>> +==============================================================================
>> +
>> +Empty memory nodes, which no memory resides in, are allowed. The 'length'
>> +field of the 'reg' property is zero, but the 'base-address' is a dummy
>> +address and invalid. The 'base-address' could be the summation of highest
>> +memory address plus the NUMA node ID. However, the NUMA node IDs and
>> +distance maps are still valid and memory may be added into them through
>> +hotplug afterwards.
>> +
>> +Example:
>> +
>> +	memory@0 {
>> +		device_type = "memory";
>> +		reg = <0x0 0x0 0x0 0x80000000>;
>> +		numa-node-id = <0>;
>> +	};
>> +
>> +	memory@0x80000000 {
> 
> unit-address should not have '0x'.
> 

Ok. Lets fix it in v6 after it's agreed to add the section into the
NUMA binding document. Actually, the '0x' is copied from the existing
example in same document. After this patch is finalized, I will post
separate patch to fix all wrong formats in same document as well.

>> +		device_type = "memory";
>> +		reg = <0x0 0x80000000 0x0 0x80000000>;
>> +		numa-node-id = <1>;
>> +	};
>> +
>> +	/* Empty memory node */
>> +	memory@0x100000002 {
>> +		device_type = "memory";
>> +		reg = <0x1 0x2 0x0 0x0>;
>> +		numa-node-id = <2>;
>> +	};
>> +
>> +	/* Empty memory node */
>> +	memory@0x100000003 {
>> +		device_type = "memory";
>> +		reg = <0x1 0x3 0x0 0x0>;
>> +		numa-node-id = <3>;
>> +	};
>> +
>> +	distance-map {
>> +		compatible = "numa-distance-map-v1";
>> +		distance-matrix = <0 0  10>,
>> +				  <0 1  20>,
>> +				  <0 2  40>,
>> +				  <0 3  20>,
>> +				  <1 0  20>,
>> +				  <1 1  10>,
>> +				  <1 2  20>,
>> +				  <1 3  40>,
>> +				  <2 0  40>,
>> +				  <2 1  20>,
>> +				  <2 2  10>,
>> +				  <2 3  20>,
>> +				  <3 0  20>,
>> +				  <3 1  40>,
>> +				  <3 2  20>,
>> +				  <3 3  10>;
>> +	};
>> +
>> +==============================================================================
>> +5 - Example dts
>>   ==============================================================================
>>   
>>   Dual socket system consists of 2 boards connected through ccn bus and
>> -- 
>> 2.23.0
>>

Thanks,
Gavin


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5] Documentation, dt, numa: Add note to empty NUMA node
  2021-07-02  0:02   ` Gavin Shan
@ 2021-07-11  0:59     ` Gavin Shan
  2021-07-12 19:44     ` Rob Herring
  1 sibling, 0 replies; 8+ messages in thread
From: Gavin Shan @ 2021-07-11  0:59 UTC (permalink / raw)
  To: Rob Herring; +Cc: devicetree, linux-kernel, rdunlap, drjones, shan.gavin

Hi Rob,

On 7/2/21 10:02 AM, Gavin Shan wrote:
> On 7/2/21 3:25 AM, Rob Herring wrote:
>> On Mon, Jun 28, 2021 at 05:34:11PM +0800, Gavin Shan wrote:
>>> The empty memory nodes, where no memory resides in, are allowed.
>>> For these empty memory nodes, the 'len' of 'reg' property is zero.
>>> The NUMA node IDs are still valid and parsed, but memory may be
>>> added to them through hotplug afterwards. I finds difficulty to
>>> get where it's properly documented.
>>
>> This is already in use? If so, what platform(s)?
>>
> 
> It's not used yet, but will be used by QEMU once this patch is merged.
> In QEMU, ARM64 could have multiple empty memory nodes. The corresponding
> NUMA ID and distance map are still valid because memory may be added into
> these empty memory nodes in future.
> 
> For the QEMU case, the names of empty memory nodes are the biggest concern.
> According to device-tree specification, the name follows the format of
> 'memory@unit-address' and the 'unit-address' is equivalent to 'base-address'.
> However, the 'base-address' should be invalid one. In current QEMU implementation,
> the valid 'base-address' and 'unit-address' are provided to these empty
> memory nodes. Another issue in QEMU is trying to populate two empty memory
> nodes, which have same names. This leads to failure of device-tree population
> because of the duplicated memory node names, blocking VM from booting.
> 
>>> So lets add a section for empty memory nodes in NUMA binding
>>> document. Also, the 'unit-address', equivalent to 'base-address'
>>> in the 'reg' property of these empty memory nodes is suggested to
>>> be the summation of highest memory address plus the NUMA node ID.
>>
>> What purpose does this serve? The kernel won't do anything with it other
>> than validate the numa-node-id range.
>>
> 
> As mentioned above, the point is to have dummy, invalid and non-overlapped
> 'base-address' and 'unit-address' for these empty memory nodes, to avoid
> duplicated memory node names in devcie-tree.
> 
>>>
>>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>>> ---
>>> v5: Separate section for empty memory node
>>> ---
>>>   Documentation/devicetree/bindings/numa.txt | 61 +++++++++++++++++++++-
>>>   1 file changed, 60 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/numa.txt b/Documentation/devicetree/bindings/numa.txt
>>> index 21b35053ca5a..230c734af948 100644
>>> --- a/Documentation/devicetree/bindings/numa.txt
>>> +++ b/Documentation/devicetree/bindings/numa.txt
>>> @@ -103,7 +103,66 @@ Example:
>>>           };
>>>   ==============================================================================
>>> -4 - Example dts
>>> +4 - Empty memory nodes
>>> +==============================================================================
>>> +
>>> +Empty memory nodes, which no memory resides in, are allowed. The 'length'
>>> +field of the 'reg' property is zero, but the 'base-address' is a dummy
>>> +address and invalid. The 'base-address' could be the summation of highest
>>> +memory address plus the NUMA node ID. However, the NUMA node IDs and
>>> +distance maps are still valid and memory may be added into them through
>>> +hotplug afterwards.
>>> +
>>> +Example:
>>> +
>>> +    memory@0 {
>>> +        device_type = "memory";
>>> +        reg = <0x0 0x0 0x0 0x80000000>;
>>> +        numa-node-id = <0>;
>>> +    };
>>> +
>>> +    memory@0x80000000 {
>>
>> unit-address should not have '0x'.
>>
> 
> Ok. Lets fix it in v6 after it's agreed to add the section into the
> NUMA binding document. Actually, the '0x' is copied from the existing
> example in same document. After this patch is finalized, I will post
> separate patch to fix all wrong formats in same document as well.
> 

I posted v6 just now, where '0x' in 'unit-address' is dropped. After
this is merged, I will post followup patch to remove '0x' in 'unit-address'
for existing examples.

Thanks,
Gavin

>>> +        device_type = "memory";
>>> +        reg = <0x0 0x80000000 0x0 0x80000000>;
>>> +        numa-node-id = <1>;
>>> +    };
>>> +
>>> +    /* Empty memory node */
>>> +    memory@0x100000002 {
>>> +        device_type = "memory";
>>> +        reg = <0x1 0x2 0x0 0x0>;
>>> +        numa-node-id = <2>;
>>> +    };
>>> +
>>> +    /* Empty memory node */
>>> +    memory@0x100000003 {
>>> +        device_type = "memory";
>>> +        reg = <0x1 0x3 0x0 0x0>;
>>> +        numa-node-id = <3>;
>>> +    };
>>> +
>>> +    distance-map {
>>> +        compatible = "numa-distance-map-v1";
>>> +        distance-matrix = <0 0  10>,
>>> +                  <0 1  20>,
>>> +                  <0 2  40>,
>>> +                  <0 3  20>,
>>> +                  <1 0  20>,
>>> +                  <1 1  10>,
>>> +                  <1 2  20>,
>>> +                  <1 3  40>,
>>> +                  <2 0  40>,
>>> +                  <2 1  20>,
>>> +                  <2 2  10>,
>>> +                  <2 3  20>,
>>> +                  <3 0  20>,
>>> +                  <3 1  40>,
>>> +                  <3 2  20>,
>>> +                  <3 3  10>;
>>> +    };
>>> +
>>> +==============================================================================
>>> +5 - Example dts
>>>   ==============================================================================
>>>   Dual socket system consists of 2 boards connected through ccn bus and
>>> -- 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5] Documentation, dt, numa: Add note to empty NUMA node
  2021-07-02  0:02   ` Gavin Shan
  2021-07-11  0:59     ` Gavin Shan
@ 2021-07-12 19:44     ` Rob Herring
  2021-07-15  4:54       ` Gavin Shan
  1 sibling, 1 reply; 8+ messages in thread
From: Rob Herring @ 2021-07-12 19:44 UTC (permalink / raw)
  To: Gavin Shan; +Cc: devicetree, linux-kernel, Randy Dunlap, drjones, shan.gavin

On Thu, Jul 1, 2021 at 6:02 PM Gavin Shan <gshan@redhat.com> wrote:
>
> Hi Rob,
>
> On 7/2/21 3:25 AM, Rob Herring wrote:
> > On Mon, Jun 28, 2021 at 05:34:11PM +0800, Gavin Shan wrote:
> >> The empty memory nodes, where no memory resides in, are allowed.
> >> For these empty memory nodes, the 'len' of 'reg' property is zero.
> >> The NUMA node IDs are still valid and parsed, but memory may be
> >> added to them through hotplug afterwards. I finds difficulty to
> >> get where it's properly documented.
> >
> > This is already in use? If so, what platform(s)?
> >
>
> It's not used yet, but will be used by QEMU once this patch is merged.
> In QEMU, ARM64 could have multiple empty memory nodes. The corresponding
> NUMA ID and distance map are still valid because memory may be added into
> these empty memory nodes in future.
>
> For the QEMU case, the names of empty memory nodes are the biggest concern.
> According to device-tree specification, the name follows the format of
> 'memory@unit-address' and the 'unit-address' is equivalent to 'base-address'.
> However, the 'base-address' should be invalid one. In current QEMU implementation,
> the valid 'base-address' and 'unit-address' are provided to these empty
> memory nodes. Another issue in QEMU is trying to populate two empty memory
> nodes, which have same names. This leads to failure of device-tree population
> because of the duplicated memory node names, blocking VM from booting.

We accept patches to the DT spec, so why are you working around it?
However, a fake base doesn't seem like a good solution to me, so
premature for any DT spec change.

In any case, I think this needs a lot more context in terms of what
you are trying to accomplish and a wider audience. Some more Arm
folks, UEFI folks, etc. Maybe the boot-architecture list. Maybe that
all happened already, but I doubt it.

> >> So lets add a section for empty memory nodes in NUMA binding
> >> document. Also, the 'unit-address', equivalent to 'base-address'
> >> in the 'reg' property of these empty memory nodes is suggested to
> >> be the summation of highest memory address plus the NUMA node ID.
> >
> > What purpose does this serve? The kernel won't do anything with it other
> > than validate the numa-node-id range.
> >
>
> As mentioned above, the point is to have dummy, invalid and non-overlapped
> 'base-address' and 'unit-address' for these empty memory nodes, to avoid
> duplicated memory node names in devcie-tree.
>
> >>
> >> Signed-off-by: Gavin Shan <gshan@redhat.com>
> >> ---
> >> v5: Separate section for empty memory node
> >> ---
> >>   Documentation/devicetree/bindings/numa.txt | 61 +++++++++++++++++++++-
> >>   1 file changed, 60 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/numa.txt b/Documentation/devicetree/bindings/numa.txt
> >> index 21b35053ca5a..230c734af948 100644
> >> --- a/Documentation/devicetree/bindings/numa.txt
> >> +++ b/Documentation/devicetree/bindings/numa.txt
> >> @@ -103,7 +103,66 @@ Example:
> >>              };
> >>
> >>   ==============================================================================
> >> -4 - Example dts
> >> +4 - Empty memory nodes
> >> +==============================================================================
> >> +
> >> +Empty memory nodes, which no memory resides in, are allowed. The 'length'
> >> +field of the 'reg' property is zero, but the 'base-address' is a dummy
> >> +address and invalid. The 'base-address' could be the summation of highest
> >> +memory address plus the NUMA node ID. However, the NUMA node IDs and
> >> +distance maps are still valid and memory may be added into them through
> >> +hotplug afterwards.
> >> +
> >> +Example:
> >> +
> >> +    memory@0 {
> >> +            device_type = "memory";
> >> +            reg = <0x0 0x0 0x0 0x80000000>;
> >> +            numa-node-id = <0>;
> >> +    };
> >> +
> >> +    memory@0x80000000 {
> >
> > unit-address should not have '0x'.
> >
>
> Ok. Lets fix it in v6 after it's agreed to add the section into the
> NUMA binding document. Actually, the '0x' is copied from the existing
> example in same document. After this patch is finalized, I will post
> separate patch to fix all wrong formats in same document as well.

Fixes first, features second.

Or just don't copy bad examples.

Rob

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5] Documentation, dt, numa: Add note to empty NUMA node
  2021-07-12 19:44     ` Rob Herring
@ 2021-07-15  4:54       ` Gavin Shan
  0 siblings, 0 replies; 8+ messages in thread
From: Gavin Shan @ 2021-07-15  4:54 UTC (permalink / raw)
  To: Rob Herring; +Cc: devicetree, linux-kernel, Randy Dunlap, drjones, shan.gavin

Hi Rob,

On 7/13/21 5:44 AM, Rob Herring wrote:
> On Thu, Jul 1, 2021 at 6:02 PM Gavin Shan <gshan@redhat.com> wrote:
>> On 7/2/21 3:25 AM, Rob Herring wrote:
>>> On Mon, Jun 28, 2021 at 05:34:11PM +0800, Gavin Shan wrote:
>>>> The empty memory nodes, where no memory resides in, are allowed.
>>>> For these empty memory nodes, the 'len' of 'reg' property is zero.
>>>> The NUMA node IDs are still valid and parsed, but memory may be
>>>> added to them through hotplug afterwards. I finds difficulty to
>>>> get where it's properly documented.
>>>
>>> This is already in use? If so, what platform(s)?
>>>
>>
>> It's not used yet, but will be used by QEMU once this patch is merged.
>> In QEMU, ARM64 could have multiple empty memory nodes. The corresponding
>> NUMA ID and distance map are still valid because memory may be added into
>> these empty memory nodes in future.
>>
>> For the QEMU case, the names of empty memory nodes are the biggest concern.
>> According to device-tree specification, the name follows the format of
>> 'memory@unit-address' and the 'unit-address' is equivalent to 'base-address'.
>> However, the 'base-address' should be invalid one. In current QEMU implementation,
>> the valid 'base-address' and 'unit-address' are provided to these empty
>> memory nodes. Another issue in QEMU is trying to populate two empty memory
>> nodes, which have same names. This leads to failure of device-tree population
>> because of the duplicated memory node names, blocking VM from booting.
> 
> We accept patches to the DT spec, so why are you working around it?
> However, a fake base doesn't seem like a good solution to me, so
> premature for any DT spec change.
> 
> In any case, I think this needs a lot more context in terms of what
> you are trying to accomplish and a wider audience. Some more Arm
> folks, UEFI folks, etc. Maybe the boot-architecture list. Maybe that
> all happened already, but I doubt it.
> 

Thanks for your comments again. How about to have something like below.
I need your help to review it first before I'm going to send the formal
patch to more maillists for further reviews. The commit log will be improved
that time either, to includes more details.


==============================================================================
4 - Empty memory nodes
==============================================================================

Empty memory nodes, which no memory resides in, are allowed on some systems
like virtual machine (VM). The 'length' field of the 'reg' property is zero,
but the 'base-address' is an invalid address, which is the summation of highest
memory address plus the NUMA node ID. However, the NUMA node IDs and distance
maps are still valid and memory may be added into them by hotplug afterwards.

[examples will be included when patch is posted]

Changes since v5:

    * The virtual machine (VM) is mentioned.
    * The 'base-address' is fixed to be the summation of highest memory
      address plus the NUMA node ID.

>>>> So lets add a section for empty memory nodes in NUMA binding
>>>> document. Also, the 'unit-address', equivalent to 'base-address'
>>>> in the 'reg' property of these empty memory nodes is suggested to
>>>> be the summation of highest memory address plus the NUMA node ID.
>>>
>>> What purpose does this serve? The kernel won't do anything with it other
>>> than validate the numa-node-id range.
>>>
>>
>> As mentioned above, the point is to have dummy, invalid and non-overlapped
>> 'base-address' and 'unit-address' for these empty memory nodes, to avoid
>> duplicated memory node names in devcie-tree.
>>
>>>>
>>>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>>>> ---
>>>> v5: Separate section for empty memory node
>>>> ---
>>>>    Documentation/devicetree/bindings/numa.txt | 61 +++++++++++++++++++++-
>>>>    1 file changed, 60 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/numa.txt b/Documentation/devicetree/bindings/numa.txt
>>>> index 21b35053ca5a..230c734af948 100644
>>>> --- a/Documentation/devicetree/bindings/numa.txt
>>>> +++ b/Documentation/devicetree/bindings/numa.txt
>>>> @@ -103,7 +103,66 @@ Example:
>>>>               };
>>>>
>>>>    ==============================================================================
>>>> -4 - Example dts
>>>> +4 - Empty memory nodes
>>>> +==============================================================================
>>>> +
>>>> +Empty memory nodes, which no memory resides in, are allowed. The 'length'
>>>> +field of the 'reg' property is zero, but the 'base-address' is a dummy
>>>> +address and invalid. The 'base-address' could be the summation of highest
>>>> +memory address plus the NUMA node ID. However, the NUMA node IDs and
>>>> +distance maps are still valid and memory may be added into them through
>>>> +hotplug afterwards.
>>>> +
>>>> +Example:
>>>> +
>>>> +    memory@0 {
>>>> +            device_type = "memory";
>>>> +            reg = <0x0 0x0 0x0 0x80000000>;
>>>> +            numa-node-id = <0>;
>>>> +    };
>>>> +
>>>> +    memory@0x80000000 {
>>>
>>> unit-address should not have '0x'.
>>>
>>
>> Ok. Lets fix it in v6 after it's agreed to add the section into the
>> NUMA binding document. Actually, the '0x' is copied from the existing
>> example in same document. After this patch is finalized, I will post
>> separate patch to fix all wrong formats in same document as well.
> 
> Fixes first, features second.
> 
> Or just don't copy bad examples.
> 

Please ignore this part because we don't have the issue in existing
examples. Sorry about the confusion. We just need to remove '0x' prefix
in the examples introduced by this patch itself.

Thanks,
Gavin


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-07-15  4:54 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-28  9:34 [PATCH v5] Documentation, dt, numa: Add note to empty NUMA node Gavin Shan
2021-06-28 16:15 ` Randy Dunlap
2021-06-30  7:26   ` Gavin Shan
2021-07-01 17:25 ` Rob Herring
2021-07-02  0:02   ` Gavin Shan
2021-07-11  0:59     ` Gavin Shan
2021-07-12 19:44     ` Rob Herring
2021-07-15  4:54       ` Gavin Shan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.