ArmVirtPkg: Fix boot fail on numa system.

[edk2-devel] [PATCH] ArmVirtPkg: Fix boot fail on numa system.

Posted by Mark-PK Tsai via groups.io 1 year, 10 months ago

If "numa-node-id" is specified in a memory node,
take node 0 as system memory instead of taking
the first memory node.

Cc: YJ Chiang <yj.chiang@medaitek.com>
Signed-off-by: Mark-PK Tsai <mark-pk.tsai@mediatek.com>
---
 ArmVirtPkg/PrePi/FdtParser.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/ArmVirtPkg/PrePi/FdtParser.c b/ArmVirtPkg/PrePi/FdtParser.c
index 5a91f7e62d..5c7de3bc31 100644
--- a/ArmVirtPkg/PrePi/FdtParser.c
+++ b/ArmVirtPkg/PrePi/FdtParser.c
@@ -19,19 +19,43 @@ FindMemnode (
   INT32        SizeCells;
   INT32        Length;
   CONST INT32  *Prop;
+  INT32        NumaId;
+  INT32        Node, Prev;
+  CONST CHAR8  *Type;
 
   if (fdt_check_header (DeviceTreeBlob) != 0) {
     return FALSE;
   }
 
   //
-  // Look for a node called "memory" at the lowest level of the tree
+  // Look for the lowest memory node.
+  // On Numa system, use node 0 as system memory.
   //
-  MemoryNode = fdt_path_offset (DeviceTreeBlob, "/memory");
-  if (MemoryNode <= 0) {
-    return FALSE;
+  MemoryNode = -1;
+  NumaId = -1;
+
+  for (Prev = 0; ; Prev = Node) {
+    Node = fdt_next_node (DeviceTreeBlob, Prev, NULL);
+    if (Node < 0)
+      break;
+
+    Type = fdt_getprop (DeviceTreeBlob, Node, "device_type", &Length);
+    if (Type && (AsciiStrnCmp (Type, "memory", Length) == 0)) {
+      Prop = fdt_getprop (DeviceTreeBlob, Node, "numa-node-id", &Length);
+      if (Prop && Length == 4) {
+        NumaId = fdt32_to_cpu (*Prop);
+      }
+
+      if (!Prop || (Prop && NumaId == 0)) {
+        MemoryNode = Node;
+        break;
+      }
+    }
   }
 
+  if (MemoryNode < 0)
+    return FALSE;
+
   //
   // Retrieve the #address-cells and #size-cells properties
   // from the root node, or use the default if not provided.
-- 
2.32.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91068): https://edk2.groups.io/g/devel/message/91068
Mute This Topic: https://groups.io/mt/92188181/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] ArmVirtPkg: Fix boot fail on numa system.

Posted by Ard Biesheuvel 1 year, 7 months ago

On Tue, 5 Jul 2022 at 08:55, Mark-PK Tsai <mark-pk.tsai@mediatek.com> wrote:
>
> If "numa-node-id" is specified in a memory node,
> take node 0 as system memory instead of taking
> the first memory node.
>
> Cc: YJ Chiang <yj.chiang@medaitek.com>

Typo in email address

> Signed-off-by: Mark-PK Tsai <mark-pk.tsai@mediatek.com>


> ---
>  ArmVirtPkg/PrePi/FdtParser.c | 32 ++++++++++++++++++++++++++++----
>  1 file changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/ArmVirtPkg/PrePi/FdtParser.c b/ArmVirtPkg/PrePi/FdtParser.c
> index 5a91f7e62d..5c7de3bc31 100644
> --- a/ArmVirtPkg/PrePi/FdtParser.c
> +++ b/ArmVirtPkg/PrePi/FdtParser.c
> @@ -19,19 +19,43 @@ FindMemnode (
>    INT32        SizeCells;
>    INT32        Length;
>    CONST INT32  *Prop;
> +  INT32        NumaId;
> +  INT32        Node, Prev;
> +  CONST CHAR8  *Type;
>
>    if (fdt_check_header (DeviceTreeBlob) != 0) {
>      return FALSE;
>    }
>
>    //
> -  // Look for a node called "memory" at the lowest level of the tree
> +  // Look for the lowest memory node.
> +  // On Numa system, use node 0 as system memory.

There could be other reasons there are multiple memory nodes, right?
So wouldn't it be better if we cross reference the memory node with
some other information, for instance, where the firmware image itself
was loaded in memory?

>    //
> -  MemoryNode = fdt_path_offset (DeviceTreeBlob, "/memory");
> -  if (MemoryNode <= 0) {
> -    return FALSE;
> +  MemoryNode = -1;
> +  NumaId = -1;
> +
> +  for (Prev = 0; ; Prev = Node) {
> +    Node = fdt_next_node (DeviceTreeBlob, Prev, NULL);
> +    if (Node < 0)
> +      break;
> +
> +    Type = fdt_getprop (DeviceTreeBlob, Node, "device_type", &Length);
> +    if (Type && (AsciiStrnCmp (Type, "memory", Length) == 0)) {
> +      Prop = fdt_getprop (DeviceTreeBlob, Node, "numa-node-id", &Length);
> +      if (Prop && Length == 4) {
> +        NumaId = fdt32_to_cpu (*Prop);
> +      }
> +
> +      if (!Prop || (Prop && NumaId == 0)) {
> +        MemoryNode = Node;
> +        break;
> +      }
> +    }
>    }
>
> +  if (MemoryNode < 0)
> +    return FALSE;
> +
>    //
>    // Retrieve the #address-cells and #size-cells properties
>    // from the root node, or use the default if not provided.
> --
> 2.32.0
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#93356): https://edk2.groups.io/g/devel/message/93356
Mute This Topic: https://groups.io/mt/92188181/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-