[Qemu-devel] [PATCH v3 0/6] spapr: rework memory nodes

[Qemu-devel] [PATCH v3 0/6] spapr: rework memory nodes

[Alexey Kardashevskiy]

c4177479 "spapr: make sure RMA is in first mode of first memory node"
introduced regression which prevents from running guests with memoryless
NUMA node#0 which may happen on real POWER8 boxes and which would make
sense to debug in QEMU.

This patchset aim is to fix that and also fix various code problems in
memory nodes generation.

These 2 patches could be merged (the resulting patch looks rather ugly):
spapr: Use DT memory node rendering helper for other nodes
spapr: Move DT memory node rendering to a helper


Alex, there are "numa: enable sparse node numbering ..." patches from Nish,
which set can go first so the other could rebase on top of it? Thanks!



Changes:
v3:
* fixed bug with ram_size bigger than the only NUMA node
* fixed bug with 64bit addresses in memory node creation loop

v2:
* minor cosmetic change in spapr_node0_size()
* spapr_populate_memory() fixed to work in a no-numa config
* patch changing max numa nodes is removed

Please comment. Thanks!




Alexey Kardashevskiy (6):
  spapr: Move DT memory node rendering to a helper
  spapr: Use DT memory node rendering helper for other nodes
  spapr: Refactor spapr_populate_memory() to allow memoryless nodes
  spapr: Split memory nodes to power-of-two blocks
  spapr: Add a helper for node0_size calculation
  spapr: Fix ibm,associativity for memory nodes

 hw/ppc/spapr.c | 111 ++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 63 insertions(+), 48 deletions(-)

-- 
2.0.0

[Qemu-devel] [PATCH v3 1/6] spapr: Move DT memory node rendering to a helper

[Alexey Kardashevskiy]

This moves recurring bits of code related to memory@xxx nodes
creation to a helper.

This makes use of the new helper for node@0.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr.c | 48 ++++++++++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 506d4fc..a5ffcba 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -605,6 +605,31 @@ int spapr_h_cas_compose_response(target_ulong addr, target_ulong size)
     return 0;
 }
 
+static void spapr_populate_memory_node(void *fdt, int nodeid, hwaddr start,
+                                       hwaddr size)
+{
+    uint32_t associativity[] = {
+        cpu_to_be32(0x4), /* length */
+        cpu_to_be32(0x0), cpu_to_be32(0x0),
+        cpu_to_be32(nodeid), cpu_to_be32(nodeid)
+    };
+    char mem_name[32];
+    uint64_t mem_reg_property[2];
+    int off;
+
+    mem_reg_property[0] = cpu_to_be64(start);
+    mem_reg_property[1] = cpu_to_be64(size);
+
+    sprintf(mem_name, "memory@" TARGET_FMT_lx, start);
+    off = fdt_add_subnode(fdt, 0, mem_name);
+    _FDT(off);
+    _FDT((fdt_setprop_string(fdt, off, "device_type", "memory")));
+    _FDT((fdt_setprop(fdt, off, "reg", mem_reg_property,
+                      sizeof(mem_reg_property))));
+    _FDT((fdt_setprop(fdt, off, "ibm,associativity", associativity,
+                      sizeof(associativity))));
+}
+
 static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
 {
     uint32_t associativity[] = {cpu_to_be32(0x4), cpu_to_be32(0x0),
@@ -623,29 +648,12 @@ static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
     }
 
     /* RMA */
-    mem_reg_property[0] = 0;
-    mem_reg_property[1] = cpu_to_be64(spapr->rma_size);
-    off = fdt_add_subnode(fdt, 0, "memory@0");
-    _FDT(off);
-    _FDT((fdt_setprop_string(fdt, off, "device_type", "memory")));
-    _FDT((fdt_setprop(fdt, off, "reg", mem_reg_property,
-                      sizeof(mem_reg_property))));
-    _FDT((fdt_setprop(fdt, off, "ibm,associativity", associativity,
-                      sizeof(associativity))));
+    spapr_populate_memory_node(fdt, 0, 0, spapr->rma_size);
 
     /* RAM: Node 0 */
     if (node0_size > spapr->rma_size) {
-        mem_reg_property[0] = cpu_to_be64(spapr->rma_size);
-        mem_reg_property[1] = cpu_to_be64(node0_size - spapr->rma_size);
-
-        sprintf(mem_name, "memory@" TARGET_FMT_lx, spapr->rma_size);
-        off = fdt_add_subnode(fdt, 0, mem_name);
-        _FDT(off);
-        _FDT((fdt_setprop_string(fdt, off, "device_type", "memory")));
-        _FDT((fdt_setprop(fdt, off, "reg", mem_reg_property,
-                          sizeof(mem_reg_property))));
-        _FDT((fdt_setprop(fdt, off, "ibm,associativity", associativity,
-                          sizeof(associativity))));
+        spapr_populate_memory_node(fdt, 0, spapr->rma_size,
+                                   node0_size - spapr->rma_size);
     }
 
     /* RAM: Node 1 and beyond */
-- 
2.0.0

[Qemu-devel] [PATCH v3 2/6] spapr: Use DT memory node rendering helper for other nodes

[Alexey Kardashevskiy]

This finishes refactoring by using the spapr_populate_memory_node helper
for all nodes and removing leftovers from spapr_populate_memory().

This is not a part of the previous patch because the patches look
nicer apart.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr.c | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a5ffcba..832bfcf 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -632,13 +632,8 @@ static void spapr_populate_memory_node(void *fdt, int nodeid, hwaddr start,
 
 static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
 {
-    uint32_t associativity[] = {cpu_to_be32(0x4), cpu_to_be32(0x0),
-                                cpu_to_be32(0x0), cpu_to_be32(0x0),
-                                cpu_to_be32(0x0)};
-    char mem_name[32];
     hwaddr node0_size, mem_start, node_size;
-    uint64_t mem_reg_property[2];
-    int i, off;
+    int i;
 
     /* memory node(s) */
     if (nb_numa_nodes > 1 && numa_info[0].node_mem < ram_size) {
@@ -659,7 +654,6 @@ static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
     /* RAM: Node 1 and beyond */
     mem_start = node0_size;
     for (i = 1; i < nb_numa_nodes; i++) {
-        mem_reg_property[0] = cpu_to_be64(mem_start);
         if (mem_start >= ram_size) {
             node_size = 0;
         } else {
@@ -668,16 +662,7 @@ static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
                 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);
-        _FDT(off);
-        _FDT((fdt_setprop_string(fdt, off, "device_type", "memory")));
-        _FDT((fdt_setprop(fdt, off, "reg", mem_reg_property,
-                          sizeof(mem_reg_property))));
-        _FDT((fdt_setprop(fdt, off, "ibm,associativity", associativity,
-                          sizeof(associativity))));
+        spapr_populate_memory_node(fdt, i, mem_start, node_size);
         mem_start += node_size;
     }
 
-- 
2.0.0

[Qemu-devel] [PATCH v3 3/6] spapr: Refactor spapr_populate_memory() to allow memoryless nodes

[Alexey Kardashevskiy]

Current QEMU does not support memoryless NUMA nodes, however
actual hardware may have them so it makes sense to have a way
to emulate them in QEMU. This prepares SPAPR for that.

This moves 2 calls of spapr_populate_memory_node() into
the existing loop over numa nodes so first several nodes may
have no memory and this still will work.

If there is no numa configuration, the code assumes there is just
a single node at 0 and it has all the guest memory.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* rebased on top of new NodeInfo code

v2:
* fixed spapr_populate_memory() to work in no-numa config
---
 hw/ppc/spapr.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 832bfcf..ec6d541 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -632,36 +632,36 @@ static void spapr_populate_memory_node(void *fdt, int nodeid, hwaddr start,
 
 static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
 {
-    hwaddr node0_size, mem_start, node_size;
-    int i;
+    hwaddr mem_start, node_size;
+    int i, nb_nodes = nb_numa_nodes;
+    NodeInfo *nodes = numa_info;
+    NodeInfo ramnode;
 
-    /* memory node(s) */
-    if (nb_numa_nodes > 1 && numa_info[0].node_mem < ram_size) {
-        node0_size = numa_info[0].node_mem;
-    } else {
-        node0_size = ram_size;
+    /* No NUMA nodes, assume there is just one node with whole RAM */
+    if (!nb_numa_nodes) {
+        nb_nodes = 1;
+        ramnode.node_mem = ram_size;
+        nodes = &ramnode;
     }
 
-    /* RMA */
-    spapr_populate_memory_node(fdt, 0, 0, spapr->rma_size);
-
-    /* RAM: Node 0 */
-    if (node0_size > spapr->rma_size) {
-        spapr_populate_memory_node(fdt, 0, spapr->rma_size,
-                                   node0_size - spapr->rma_size);
-    }
-
-    /* RAM: Node 1 and beyond */
-    mem_start = node0_size;
-    for (i = 1; i < nb_numa_nodes; i++) {
+    for (i = 0, mem_start = 0; i < nb_nodes; ++i) {
+        if (!nodes[i].node_mem) {
+            continue;
+        }
         if (mem_start >= ram_size) {
             node_size = 0;
         } else {
-            node_size = numa_info[i].node_mem;
+            node_size = nodes[i].node_mem;
             if (node_size > ram_size - mem_start) {
                 node_size = ram_size - mem_start;
             }
         }
+        if (!mem_start) {
+            /* ppc_spapr_init() checks for rma_size <= node0_size already */
+            spapr_populate_memory_node(fdt, i, 0, spapr->rma_size);
+            mem_start += spapr->rma_size;
+            node_size -= spapr->rma_size;
+        }
         spapr_populate_memory_node(fdt, i, mem_start, node_size);
         mem_start += node_size;
     }
-- 
2.0.0

[Qemu-devel] [PATCH v3 4/6] spapr: Split memory nodes to power-of-two blocks

[Alexey Kardashevskiy]

Linux kernel expects nodes to have power-of-two size and
does WARN_ON if this is not the case:
[    0.041456] WARNING: at drivers/base/memory.c:115
which is:
===
	/* Validate blk_sz is a power of 2 and not less than section size */
	if ((block_sz & (block_sz - 1)) || (block_sz < MIN_MEMORY_BLOCK_SIZE)) {
        	WARN_ON(1);
	        block_sz = MIN_MEMORY_BLOCK_SIZE;
	}
===

This splits memory nodes into set of smaller blocks with
a size which is a power of two. This makes sure the start
address of every node is aligned to the node size.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* s/ffs/ffsl/ as addresses are 64bit long

v2:
* tiny code cleanup in "sizetmp = MIN(sizetmp, 1 << (ffs(mem_start) - 1))"
* updated commit log with a piece of kernel code doing WARN_ON
---
 hw/ppc/spapr.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ec6d541..680d7f9 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -662,8 +662,18 @@ static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
             mem_start += spapr->rma_size;
             node_size -= spapr->rma_size;
         }
-        spapr_populate_memory_node(fdt, i, mem_start, node_size);
-        mem_start += node_size;
+        for ( ; node_size; ) {
+            hwaddr sizetmp = pow2floor(node_size);
+
+            /* mem_start != 0 here */
+            if (ffsl(mem_start) < ffsl(sizetmp)) {
+                sizetmp = 1ULL << (ffsl(mem_start) - 1);
+            }
+
+            spapr_populate_memory_node(fdt, i, mem_start, sizetmp);
+            node_size -= sizetmp;
+            mem_start += sizetmp;
+        }
     }
 
     return 0;
-- 
2.0.0

[Qemu-devel] [PATCH v3 5/6] spapr: Add a helper for node0_size calculation

[Alexey Kardashevskiy]

In multiple places there is a node0_size variable calculation
which assumes that NUMA node #0 and memory node #0 are the same
things which they are not. Since we are going to change it and
do not want to change it in multiple places, let's make a helper.

This adds a spapr_node0_size() helper and makes use of it.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* fixed bug when QEMU is started with RAM size
bigger that the only NUMA node like this:
 -m 8192 -smp 4 -numa node,nodeid=0,cpus=0-3,mem=4096

v2:
* removed duplicated "return ram_size" from spapr_node0_size()
---
 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 680d7f9..c71ce1f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -282,6 +282,19 @@ static size_t create_page_sizes_prop(CPUPPCState *env, uint32_t *prop,
     return (p - prop) * sizeof(uint32_t);
 }
 
+static hwaddr spapr_node0_size(void)
+{
+    if (nb_numa_nodes) {
+        int i;
+        for (i = 0; i < nb_numa_nodes; ++i) {
+            if (numa_info[i].node_mem) {
+                return MIN(pow2floor(numa_info[i].node_mem), ram_size);
+            }
+        }
+    }
+    return ram_size;
+}
+
 #define _FDT(exp) \
     do { \
         int ret = (exp);                                           \
@@ -803,9 +816,8 @@ static void spapr_reset_htab(sPAPREnvironment *spapr)
 
     /* Update the RMA size if necessary */
     if (spapr->vrma_adjust) {
-        hwaddr node0_size = (nb_numa_nodes > 1) ?
-            numa_info[0].node_mem : ram_size;
-        spapr->rma_size = kvmppc_rma_size(node0_size, spapr->htab_shift);
+        spapr->rma_size = kvmppc_rma_size(spapr_node0_size(),
+                                          spapr->htab_shift);
     }
 }
 
@@ -1236,7 +1248,7 @@ static void ppc_spapr_init(MachineState *machine)
     MemoryRegion *sysmem = get_system_memory();
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     hwaddr rma_alloc_size;
-    hwaddr node0_size = (nb_numa_nodes > 1) ? numa_info[0].node_mem : ram_size;
+    hwaddr node0_size = spapr_node0_size();
     uint32_t initrd_base = 0;
     long kernel_size = 0, initrd_size = 0;
     long load_limit, rtas_limit, fw_size;
-- 
2.0.0

[Qemu-devel] [PATCH v3 6/6] spapr: Fix ibm, associativity for memory nodes

[Alexey Kardashevskiy]

We want the associtivity lists of memory and CPU nodes to match but
memory nodes have incorrect domain#3 which is zero for CPU so they won't
match.

This clears domain#3 in the list to match CPUs associtivity lists.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index c71ce1f..166b2fc 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -624,7 +624,7 @@ static void spapr_populate_memory_node(void *fdt, int nodeid, hwaddr start,
     uint32_t associativity[] = {
         cpu_to_be32(0x4), /* length */
         cpu_to_be32(0x0), cpu_to_be32(0x0),
-        cpu_to_be32(nodeid), cpu_to_be32(nodeid)
+        cpu_to_be32(0x0), cpu_to_be32(nodeid)
     };
     char mem_name[32];
     uint64_t mem_reg_property[2];
-- 
2.0.0

Re: [Qemu-devel] [PATCH v3 0/6] spapr: rework memory nodes

[Alexey Kardashevskiy]

On 07/03/2014 01:10 PM, Alexey Kardashevskiy wrote:
> c4177479 "spapr: make sure RMA is in first mode of first memory node"
> introduced regression which prevents from running guests with memoryless
> NUMA node#0 which may happen on real POWER8 boxes and which would make
> sense to debug in QEMU.
> 
> This patchset aim is to fix that and also fix various code problems in
> memory nodes generation.
> 
> These 2 patches could be merged (the resulting patch looks rather ugly):
> spapr: Use DT memory node rendering helper for other nodes
> spapr: Move DT memory node rendering to a helper
> 
> 
> Alex, there are "numa: enable sparse node numbering ..." patches from Nish,
> which set can go first so the other could rebase on top of it? Thanks!


Ping. This is for 2.2 indeed.


> 
> 
> 
> Changes:
> v3:
> * fixed bug with ram_size bigger than the only NUMA node
> * fixed bug with 64bit addresses in memory node creation loop
> 
> v2:
> * minor cosmetic change in spapr_node0_size()
> * spapr_populate_memory() fixed to work in a no-numa config
> * patch changing max numa nodes is removed
> 
> Please comment. Thanks!
> 
> 
> 
> 
> Alexey Kardashevskiy (6):
>   spapr: Move DT memory node rendering to a helper
>   spapr: Use DT memory node rendering helper for other nodes
>   spapr: Refactor spapr_populate_memory() to allow memoryless nodes
>   spapr: Split memory nodes to power-of-two blocks
>   spapr: Add a helper for node0_size calculation
>   spapr: Fix ibm,associativity for memory nodes
> 
>  hw/ppc/spapr.c | 111 ++++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 63 insertions(+), 48 deletions(-)
> 


-- 
Alexey

Re: [Qemu-devel] [PATCH v3 0/6] spapr: rework memory nodes

[Alexander Graf]

On 21.07.14 05:15, Alexey Kardashevskiy wrote:
> On 07/03/2014 01:10 PM, Alexey Kardashevskiy wrote:
>> c4177479 "spapr: make sure RMA is in first mode of first memory node"
>> introduced regression which prevents from running guests with memoryless
>> NUMA node#0 which may happen on real POWER8 boxes and which would make
>> sense to debug in QEMU.
>>
>> This patchset aim is to fix that and also fix various code problems in
>> memory nodes generation.
>>
>> These 2 patches could be merged (the resulting patch looks rather ugly):
>> spapr: Use DT memory node rendering helper for other nodes
>> spapr: Move DT memory node rendering to a helper
>>
>>
>> Alex, there are "numa: enable sparse node numbering ..." patches from Nish,
>> which set can go first so the other could rebase on top of it? Thanks!
>
> Ping. This is for 2.2 indeed.

Thanks, applied all to ppc-next-2.2.


Alex