diff mbox

[v5,3/5] ARM: Add numa-node-id for /memory node

Message ID 1461219834-10416-4-git-send-email-zhaoshenglong@huawei.com
State New
Headers show

Commit Message

Shannon Zhao April 21, 2016, 6:23 a.m. UTC
From: Shannon Zhao <shannon.zhao@linaro.org>


When specifying NUMA for ARM machine, generate /memory node according to
NUMA topology.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

---
 hw/arm/boot.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

-- 
2.0.4

Comments

Andrew Jones April 22, 2016, 12:48 p.m. UTC | #1
On Thu, Apr 21, 2016 at 02:23:52PM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>

> 

> When specifying NUMA for ARM machine, generate /memory node according to

> NUMA topology.

> 

> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

> ---

>  hw/arm/boot.c | 31 ++++++++++++++++++++++++++++++-

>  1 file changed, 30 insertions(+), 1 deletion(-)

> 

> diff --git a/hw/arm/boot.c b/hw/arm/boot.c

> index 5975fbf..3770235 100644

> --- a/hw/arm/boot.c

> +++ b/hw/arm/boot.c

> @@ -14,6 +14,7 @@

>  #include "hw/arm/linux-boot-if.h"

>  #include "sysemu/kvm.h"

>  #include "sysemu/sysemu.h"

> +#include "sysemu/numa.h"

>  #include "hw/boards.h"

>  #include "hw/loader.h"

>  #include "elf.h"

> @@ -405,6 +406,9 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo,

>      void *fdt = NULL;

>      int size, rc;

>      uint32_t acells, scells;

> +    char *nodename;

> +    unsigned int i;

> +    hwaddr mem_base, mem_len;

>  

>      if (binfo->dtb_filename) {

>          char *filename;

> @@ -456,14 +460,39 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo,

>          goto fail;

>      }

>  

> +    mem_len = (nb_numa_nodes > 0) ? numa_info[0].node_mem : binfo->ram_size;

>      rc = qemu_fdt_setprop_sized_cells(fdt, "/memory", "reg",


So node0's memory node will still be called '/memory' instead of
'/memory@addr' like the other nodes? Shouldn't we change it too?

>                                        acells, binfo->loader_start,

> -                                      scells, binfo->ram_size);

> +                                      scells, mem_len);

>      if (rc < 0) {

>          fprintf(stderr, "couldn't set /memory/reg\n");

>          goto fail;

>      }

>  

> +    if (nb_numa_nodes > 0) {

> +        /* Set the numa-node-id for the first /memory node. */

> +        qemu_fdt_setprop_cell(fdt, "/memory", "numa-node-id", 0);

> +        /* create /memory node and set properties for other memory numa nodes */

> +        mem_base = binfo->loader_start + mem_len;

> +        for (i = 1; i < nb_numa_nodes; i++) {

> +            mem_len = numa_info[i].node_mem;

> +            nodename = g_strdup_printf("/memory@%" PRIx64, mem_base);

> +            qemu_fdt_add_subnode(fdt, nodename);

> +            qemu_fdt_setprop_string(fdt, nodename, "device_type", "memory");

> +            rc = qemu_fdt_setprop_sized_cells(fdt, nodename, "reg",

> +                                              acells, mem_base,

> +                                              scells, mem_len);

> +            if (rc < 0) {

> +                fprintf(stderr, "couldn't set /memory/reg\n");


I'd extend this error message with either the full memory node name,
i.e. '@addr', or something like "couldn't set... for node %d\n"

> +                goto fail;

> +            }

> +

> +            qemu_fdt_setprop_cell(fdt, nodename, "numa-node-id", i);

> +            mem_base += mem_len;

> +            g_free(nodename);

> +        }

> +    }

> +

>      if (binfo->kernel_cmdline && *binfo->kernel_cmdline) {

>          rc = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",

>                                       binfo->kernel_cmdline);

> -- 

> 2.0.4


Thanks,
drew
Andrew Jones April 23, 2016, 7:45 a.m. UTC | #2
On Sat, Apr 23, 2016 at 09:16:11AM +0800, Shannon Zhao wrote:
> 

> 

> On 2016/4/22 20:48, Andrew Jones wrote:

> > On Thu, Apr 21, 2016 at 02:23:52PM +0800, Shannon Zhao wrote:

> >> From: Shannon Zhao <shannon.zhao@linaro.org>

> >>

> >> When specifying NUMA for ARM machine, generate /memory node according to

> >> NUMA topology.

> >>

> >> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

> >> ---

> >>  hw/arm/boot.c | 31 ++++++++++++++++++++++++++++++-

> >>  1 file changed, 30 insertions(+), 1 deletion(-)

> >>

> >> diff --git a/hw/arm/boot.c b/hw/arm/boot.c

> >> index 5975fbf..3770235 100644

> >> --- a/hw/arm/boot.c

> >> +++ b/hw/arm/boot.c

> >> @@ -14,6 +14,7 @@

> >>  #include "hw/arm/linux-boot-if.h"

> >>  #include "sysemu/kvm.h"

> >>  #include "sysemu/sysemu.h"

> >> +#include "sysemu/numa.h"

> >>  #include "hw/boards.h"

> >>  #include "hw/loader.h"

> >>  #include "elf.h"

> >> @@ -405,6 +406,9 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo,

> >>      void *fdt = NULL;

> >>      int size, rc;

> >>      uint32_t acells, scells;

> >> +    char *nodename;

> >> +    unsigned int i;

> >> +    hwaddr mem_base, mem_len;

> >>  

> >>      if (binfo->dtb_filename) {

> >>          char *filename;

> >> @@ -456,14 +460,39 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo,

> >>          goto fail;

> >>      }

> >>  

> >> +    mem_len = (nb_numa_nodes > 0) ? numa_info[0].node_mem : binfo->ram_size;

> >>      rc = qemu_fdt_setprop_sized_cells(fdt, "/memory", "reg",

> > 

> > So node0's memory node will still be called '/memory' instead of

> > '/memory@addr' like the other nodes? Shouldn't we change it too?

> > 

> Previously I deleted the /memory node creation codes in virt.c and

> create here, but that will cause other boards booting fail since

> load_dtb() is a common function. So to avoid more changes to other

> files, I just use current way. So is there any way to change the node

> name after it's created in qemu?


I'm not sure if that's possible, but we could maybe use qemu_fdt_nop_node
to turn /memory into a NOP node, and then add a new one?

drew
diff mbox

Patch

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 5975fbf..3770235 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -14,6 +14,7 @@ 
 #include "hw/arm/linux-boot-if.h"
 #include "sysemu/kvm.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/numa.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
 #include "elf.h"
@@ -405,6 +406,9 @@  static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
     void *fdt = NULL;
     int size, rc;
     uint32_t acells, scells;
+    char *nodename;
+    unsigned int i;
+    hwaddr mem_base, mem_len;
 
     if (binfo->dtb_filename) {
         char *filename;
@@ -456,14 +460,39 @@  static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
         goto fail;
     }
 
+    mem_len = (nb_numa_nodes > 0) ? numa_info[0].node_mem : binfo->ram_size;
     rc = qemu_fdt_setprop_sized_cells(fdt, "/memory", "reg",
                                       acells, binfo->loader_start,
-                                      scells, binfo->ram_size);
+                                      scells, mem_len);
     if (rc < 0) {
         fprintf(stderr, "couldn't set /memory/reg\n");
         goto fail;
     }
 
+    if (nb_numa_nodes > 0) {
+        /* Set the numa-node-id for the first /memory node. */
+        qemu_fdt_setprop_cell(fdt, "/memory", "numa-node-id", 0);
+        /* create /memory node and set properties for other memory numa nodes */
+        mem_base = binfo->loader_start + mem_len;
+        for (i = 1; i < nb_numa_nodes; i++) {
+            mem_len = numa_info[i].node_mem;
+            nodename = g_strdup_printf("/memory@%" PRIx64, mem_base);
+            qemu_fdt_add_subnode(fdt, nodename);
+            qemu_fdt_setprop_string(fdt, nodename, "device_type", "memory");
+            rc = qemu_fdt_setprop_sized_cells(fdt, nodename, "reg",
+                                              acells, mem_base,
+                                              scells, mem_len);
+            if (rc < 0) {
+                fprintf(stderr, "couldn't set /memory/reg\n");
+                goto fail;
+            }
+
+            qemu_fdt_setprop_cell(fdt, nodename, "numa-node-id", i);
+            mem_base += mem_len;
+            g_free(nodename);
+        }
+    }
+
     if (binfo->kernel_cmdline && *binfo->kernel_cmdline) {
         rc = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
                                      binfo->kernel_cmdline);