[Qemu-devel] [PATCH v5 0/2] spapr: rma and numa nodes fixes

[Qemu-devel] [PATCH v5 0/2] spapr: rma and numa nodes fixes

[Alexey Kardashevskiy]

The first patch got RB from Thomas, and the second one is new but depends
on the first one so I am (re)posting them together. Thanks.


Alexey Kardashevskiy (1):
  spapr: make sure RMA is in first mode of first memory node

Paul Mackerras (1):
  spapr: limit numa memory regions by ram size

 hw/ppc/spapr.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

-- 
1.8.4.rc4

[Qemu-devel] [PATCH v5 1/2] spapr: make sure RMA is in first mode of first memory node

[Alexey Kardashevskiy]

The SPAPR specification says that the RMA starts at the LPAR's logical
address 0 and is the first logical memory block reported in
the LPAR’s device tree.

So SLOF only maps the first block and that block needs to span
the full RMA.

This makes sure that the RMA area is where SLOF expects it.

Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v4:
* fixed a bug with preallocated RMA (thanks to Thomas Huth)

v3:
* removed unnecessary RMA fixup from spapr_populate_memory()

v2:
* changed as recommended by Alex Graf
---
 hw/ppc/spapr.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7e53a5f..036246c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -532,9 +532,6 @@ static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
 
     /* memory node(s) */
     node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
-    if (spapr->rma_size > node0_size) {
-        spapr->rma_size = node0_size;
-    }
 
     /* RMA */
     mem_reg_property[0] = 0;
@@ -1113,6 +1110,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
     MemoryRegion *sysmem = get_system_memory();
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     hwaddr rma_alloc_size;
+    hwaddr node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
     uint32_t initrd_base = 0;
     long kernel_size = 0, initrd_size = 0;
     long load_limit, rtas_limit, fw_size;
@@ -1134,10 +1132,10 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
         exit(1);
     }
 
-    if (rma_alloc_size && (rma_alloc_size < ram_size)) {
+    if (rma_alloc_size && (rma_alloc_size < node0_size)) {
         spapr->rma_size = rma_alloc_size;
     } else {
-        spapr->rma_size = ram_size;
+        spapr->rma_size = node0_size;
 
         /* With KVM, we don't actually know whether KVM supports an
          * unbounded RMA (PR KVM) or is limited by the hash table size
@@ -1154,6 +1152,12 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
         }
     }
 
+    if (spapr->rma_size > node0_size) {
+        fprintf(stderr, "Error: Numa node 0 has to span the RMA (%#08"HWADDR_PRIx")\n",
+                spapr->rma_size);
+        exit(1);
+    }
+
     /* We place the device tree and RTAS just below either the top of the RMA,
      * or just below 2GB, whichever is lowere, so that it can be
      * processed with 32-bit real mode code if necessary */
-- 
1.8.4.rc4

[Qemu-devel] [PATCH v5 2/2] spapr: limit numa memory regions by ram size

[Alexey Kardashevskiy]

From: Paul Mackerras <paulus@samba.org>

This makes sure that all NUMA memory blocks beside within RAM or
have zero length.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---

This is a bugfix for:
-m 500
-smp 8,sockets=2,cores=2,threads=2
-numa node,nodeid=0,cpus=0-3,mem=500
-numa node,nodeid=1,cpus=4-7,mem=500
---
 hw/ppc/spapr.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 036246c..a7f6af8 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -526,12 +526,16 @@ static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
                                 cpu_to_be32(0x0), cpu_to_be32(0x0),
                                 cpu_to_be32(0x0)};
     char mem_name[32];
-    hwaddr node0_size, mem_start;
+    hwaddr node0_size, mem_start, node_size;
     uint64_t mem_reg_property[2];
     int i, off;
 
     /* memory node(s) */
-    node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
+    if (nb_numa_nodes > 1 && node_mem[0] < ram_size) {
+        node0_size = node_mem[0];
+    } else {
+        node0_size = ram_size;
+    }
 
     /* RMA */
     mem_reg_property[0] = 0;
@@ -563,7 +567,15 @@ static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
     mem_start = node0_size;
     for (i = 1; i < nb_numa_nodes; i++) {
         mem_reg_property[0] = cpu_to_be64(mem_start);
-        mem_reg_property[1] = cpu_to_be64(node_mem[i]);
+        if (mem_start >= ram_size) {
+            node_size = 0;
+        } else {
+            node_size = node_mem[i];
+            if (node_size > ram_size - mem_start) {
+                node_size = ram_size - mem_start;
+            }
+        }
+        mem_reg_property[1] = cpu_to_be64(node_size);
         associativity[3] = associativity[4] = cpu_to_be32(i);
         sprintf(mem_name, "memory@" TARGET_FMT_lx, mem_start);
         off = fdt_add_subnode(fdt, 0, mem_name);
@@ -573,7 +585,7 @@ static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
                           sizeof(mem_reg_property))));
         _FDT((fdt_setprop(fdt, off, "ibm,associativity", associativity,
                           sizeof(associativity))));
-        mem_start += node_mem[i];
+        mem_start += node_size;
     }
 
     return 0;
-- 
1.8.4.rc4

Re: [Qemu-devel] [PATCH v5 2/2] spapr: limit numa memory regions by ram size

[Thomas Huth]

On Mon, 18 Nov 2013 15:19:32 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> From: Paul Mackerras <paulus@samba.org>
> 
> This makes sure that all NUMA memory blocks beside within RAM or

s/beside/reside/ ?

> have zero length.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> 
> This is a bugfix for:
> -m 500
> -smp 8,sockets=2,cores=2,threads=2
> -numa node,nodeid=0,cpus=0-3,mem=500
> -numa node,nodeid=1,cpus=4-7,mem=500
> ---
>  hw/ppc/spapr.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 036246c..a7f6af8 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -526,12 +526,16 @@ static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
>                                  cpu_to_be32(0x0), cpu_to_be32(0x0),
>                                  cpu_to_be32(0x0)};
>      char mem_name[32];
> -    hwaddr node0_size, mem_start;
> +    hwaddr node0_size, mem_start, node_size;
>      uint64_t mem_reg_property[2];
>      int i, off;
> 
>      /* memory node(s) */
> -    node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
> +    if (nb_numa_nodes > 1 && node_mem[0] < ram_size) {
> +        node0_size = node_mem[0];
> +    } else {
> +        node0_size = ram_size;
> +    }
> 
>      /* RMA */
>      mem_reg_property[0] = 0;
> @@ -563,7 +567,15 @@ static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
>      mem_start = node0_size;
>      for (i = 1; i < nb_numa_nodes; i++) {
>          mem_reg_property[0] = cpu_to_be64(mem_start);
> -        mem_reg_property[1] = cpu_to_be64(node_mem[i]);
> +        if (mem_start >= ram_size) {
> +            node_size = 0;
> +        } else {
> +            node_size = node_mem[i];
> +            if (node_size > ram_size - mem_start) {
> +                node_size = ram_size - mem_start;
> +            }
> +        }
> +        mem_reg_property[1] = cpu_to_be64(node_size);
>          associativity[3] = associativity[4] = cpu_to_be32(i);
>          sprintf(mem_name, "memory@" TARGET_FMT_lx, mem_start);
>          off = fdt_add_subnode(fdt, 0, mem_name);
> @@ -573,7 +585,7 @@ static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
>                            sizeof(mem_reg_property))));
>          _FDT((fdt_setprop(fdt, off, "ibm,associativity", associativity,
>                            sizeof(associativity))));
> -        mem_start += node_mem[i];
> +        mem_start += node_size;
>      }
> 
>      return 0;

Apart from the typo in the commit message, the patch looks good to me.

Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>