| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <6aa8dd66-7910-4227-709a-8d0bbc2939ee@redhat.com>
Date: Mon, 27 Sep 2021 11:16:30 +1000
From: Gavin Shan <gshan@...hat.com>
To: Rob Herring <robh@...nel.org>
Cc: Ard Biesheuvel <ardb@...nel.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
linux-efi <linux-efi@...r.kernel.org>,
"open list:KERNEL VIRTUAL MACHINE FOR ARM64 (KVM/arm64)"
<kvmarm@...ts.cs.columbia.edu>,
Randy Dunlap <rdunlap@...radead.org>,
Andrew Jones <drjones@...hat.com>,
Will Deacon <will@...nel.org>, Marc Zyngier <maz@...nel.org>,
Catalin Marinas <catalin.marinas@....com>, shan.gavin@...il.com
Subject: Re: [PATCH] Documentation, dt, numa: Add note to empty NUMA node
Hi Rob,
> On 9/24/21 1:17 AM, Rob Herring wrote:
> On Thu, Sep 23, 2021 at 1:32 AM Gavin Shan <gshan@...hat.com> wrote:
>> On 9/22/21 9:05 PM, Ard Biesheuvel wrote:
>>> On Tue, 21 Sept 2021 at 21:45, Rob Herring <robh@...nel.org> wrote:
>>>> On Sun, Sep 5, 2021 at 11:16 PM Gavin Shan <gshan@...hat.com> 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. Currently, QEMU fails
>>>>> to boot when multiple empty memory nodes are specified. It's
>>>>> caused by device-tree population failure and duplicated memory
>>>>> node names.
>>>
>>> Those memory regions are known in advance, right? So wouldn't it be
>>> better to use something like 'status = "disabled"' here?
>>>
>>
>> Yes, these memory regions are known in advance. For the empty nodes,
>> their 'len' property is zero and it's equal to disabled state.
>>
>>>>
>>>> I still don't like the fake addresses. I can't really give suggestions
>>>> on alternative ways to fix this with you just presenting a solution.
>>>>
>>>
>>> Agreed. Please try to explain what the problem is, and why this is the
>>> best way to solve it. Please include other solutions that were
>>> considered and rejected if any exist.
>>>
>>>> What is the failure you see? Can we relax the kernel's expectations?
>>>> What about UEFI boot as the memory nodes aren't used (or maybe they
>>>> are for NUMA?) How does this work with ACPI?
>>>>
>>>
>>> The EFI memory map only needs to describe the memory that was present
>>> at boot. More memory can be represented as ACPI objects, including
>>> coldplugged memory that is already present at boot. None of this
>>> involves the memory nodes in DT.
>>>
>>
>> I'm using the following command line to start a virtual machine (VM).
>> There are 4 NUMA nodes specified, but the last two are empty. In QEMU,
>> the device-tree nodes are populated to represent these 4 NUMA nodes.
>> Unfortunately, QEMU fails to start because of the conflicting names
>> for the empty node are found, as the following error message indicates.
>>
>> /home/gavin/sandbox/qemu.main/build/qemu-system-aarch64 \
>> -accel kvm -machine virt,gic-version=host \
>> -cpu host -smp 4,sockets=2,cores=2,threads=1 \
>> -m 1024M,slots=16,maxmem=64G \
>> -object memory-backend-ram,id=mem0,size=512M \
>> -object memory-backend-ram,id=mem1,size=512M \
>> -numa node,nodeid=0,cpus=0-1,memdev=mem0 \
>> -numa node,nodeid=1,cpus=2-3,memdev=mem1 \
>> -numa node,nodeid=2 \
>> -numa node,nodeid=3 \
>> :
>> -device virtio-balloon-pci,id=balloon0,free-page-reporting=yes
>> :
>> :
>> qemu-system-aarch64: FDT: Failed to create subnode /memory@...00000: FDT_ERR_EXISTS
>>
>> According to device-tree specification, the memory device-tree node's
>> name is following the format 'memory@...e-address'. For the empty
>> NUMA nodes, their base addresses aren't determined. The device-tree
>> specification doesn't indicate what 'base-address' should be assigned
>> for the empty nodes. So I proposed this patch because I think the
>> linux device-tree binding documentation is best place to get this
>> documented.
>
> Why even create the node?
>
> What does IBM pSeries do here. AIUI, those platforms create/remove
> nodes for hotplug. That's the reason CONFIG_OF_DYNAMIC existed
> originally. Unfortunately, that's the extent of my knowledge on that.
>
It has been long time that I didn't read pSeries related code. I
spent some time on that and you're correct. On pSeries, the device-tree
node is added dynamically. However, the IBM private nodes or properties,
whose names start with "ibm", are used for the memory hotplug. So
ARM64 can't follow without adaption.
I agree on your suggestion, which will be reflected in v2:
(a) the memory device-tree nodes are added and removed on memory hot
add and removal.
(b) the supported NUMA nodes, including the empty ones, are identified
through "numa-distance-map" compatible device-tree node. It's
exactly same to what you suggested below.
In this way, we won't have the issue of the conflicting memory node
names, introduced by the empty NUMA nodes.
>> ACPI is different story. The NUMA nodes are represented by SRAT
>> (System Resource Affinity Table). In the above example, there are
>> 4 SRATs. We needn't assign names to the tables and we don't have
>> the conflicting names as we do in device-tree case.
>>
>> By the way, QEMU currently prevents to expose SRATs for empty NUMA
>> nodes. I need submit QEMU patch to break the limitation in future.
>> With the limitation, the hot-added memory is always put into the
>> last NUMA node and it's not exactly customer wants.
>>
>>>>> As device-tree specification indicates, the 'unit-address' of
>>>>> these empty memory nodes, part of their names, are the equivalents
>>>>> to 'base-address'. Unfortunately, I finds difficulty to get where
>>>>> the assignment of 'base-address' is properly documented for these
>>>>> empty memory nodes. So lets add a section for empty memory nodes
>>>>> to cover this in NUMA binding document. The 'unit-address',
>>>>> equivalent to 'base-address' in the 'reg' property of these empty
>>>>> memory nodes is specified to be the summation of highest memory
>>>>> address plus the NUMA node ID.
>>>>>
>>>>> Signed-off-by: Gavin Shan <gshan@...hat.com>
>>>>> Acked-by: Randy Dunlap <rdunlap@...radead.org>
>>>>> ---
>>>>> Documentation/devicetree/bindings/numa.txt | 60 +++++++++++++++++++++-
>>>>> 1 file changed, 59 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/numa.txt b/Documentation/devicetree/bindings/numa.txt
>>>>> index 21b35053ca5a..82f047bc8dd6 100644
>>>>> --- a/Documentation/devicetree/bindings/numa.txt
>>>>> +++ b/Documentation/devicetree/bindings/numa.txt
>>>>> @@ -103,7 +103,65 @@ 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. However, the 'base-address' is a
>>>>> +dummy and invalid address, which is the summation of highest memory address
>>>>> +plus the NUMA node ID. 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@...00000 {
>>>>> + device_type = "memory";
>>>>> + reg = <0x0 0x80000000 0x0 0x80000000>;
>>>>> + numa-node-id = <1>;
>>>>> + };
>>>>> +
>>>>> + /* Empty memory node */
>>>>> + memory@...000002 {
>>>>> + device_type = "memory";
>>>>> + reg = <0x1 0x2 0x0 0x0>;
>>>>> + numa-node-id = <2>;
>>>>> + };
>>>>> +
>>>>> + /* Empty memory node */
>>>>> + memory@...000003 {
>>>>> + device_type = "memory";
>>>>> + reg = <0x1 0x3 0x0 0x0>;
>>>>> + numa-node-id = <3>;
>>>>> + };
>>>>
>>>> Do you really need the memory nodes here or just some way to define
>>>> numa node id's 2 and 3 as valid?
>>>>
>>
>> It's the way to define NUMA node IDs are valid. Besides, the 'reg'
>> property provides 'base-address', which is part of the device-tree
>> node's name, as described in this patch.
>
> The distance-matrix already lists all possible NUMA node IDs. That
> should be enough information for the kernel. If not, fix the kernel.
>
Thanks, Rob. An extra patch will address the issue in v2.
Thanks,
Gavin
Powered by blists - more mailing lists