Re: [PATCH v5] Documentation, dt, numa: Add note to empty NUMA node

[Rob Herring]

On Thu, Jul 1, 2021 at 6:02 PM Gavin Shan <gshan@xxxxxxxxxx> 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@xxxxxxxxxx>
> >> ---
> >> 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