[Qemu-devel] [RFC v1 0/2] Enforce gaps between DIMMs

[Qemu-devel] [RFC v1 0/2] Enforce gaps between DIMMs

[Bharata B Rao]

The suggested way to work around the virtio bug reported here

http://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg00522.html

is to introduce gaps between DIMMs. Igor's patchset changes the pc-dimm
auto-address assignment to introduce gaps and ues the same from pc memhp.
This patchset does the same for sPAPR PowerPC.

Before introducing the gap, ensure that memory hotplug region has enough
room for alignment adjustment. We accommodate a max alignment of 256MB for
each slot since sPAPR memory hotplug enforces an alignment requirement of
256MB on RAM size, maxmem and NUMA node mem sizes.

This applies on David's spapr-next branch + Igor's patchset applied.

Changes in v1
-------------
- Create DRC objects spanning the alignment-adjusted hotplug memory
  region instead of just for (maxmem - ramsize).
- Pass an alignment of 256M to pc_dimm_memory_plug() so that we always
  get 256M aligned DIMM addresses.

v0: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg00749.html

Bharata B Rao (2):
  spapr: Accommadate alignment gaps in hotplug memory region
  spapr: Force gaps between DIMM's GPA

 hw/ppc/spapr.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

-- 
2.1.0

[Qemu-devel] [RFC v1 1/2] spapr: Accommadate alignment gaps in hotplug memory region

[Bharata B Rao]

Accommodate enough space in hotplug memory region so that it will
be possible to align memory in each slot to 256M. When PowerPC memory
hotplug enables inter-dimm gaps, then we could end up having
unused/unassigned 256M memory chuck between DIMMs in the hotplug memory
region. Hence create DRC objects spanning the entire alignment-adjusted
hotplug memory region instead of just the acutal
hotpluggable size (machine->maxram_size - machine->ram_size).

In addition, pass 256M alignment to pc_dimm_memory_plug() so that
the DIMM address gets aligned to 256M.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index fc5e7d6..4a901f0 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -721,7 +721,7 @@ static int spapr_populate_drconf_memory(sPAPRMachineState *spapr, void *fdt)
     int ret, i, offset;
     uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
     uint32_t prop_lmb_size[] = {0, cpu_to_be32(lmb_size)};
-    uint32_t nr_lmbs = (machine->maxram_size - machine->ram_size)/lmb_size;
+    uint32_t nr_lmbs = memory_region_size(&spapr->hotplug_memory.mr)/lmb_size;
     uint32_t *int_buf, *cur_index, buf_len;
     int nr_nodes = nb_numa_nodes ? nb_numa_nodes : 1;
 
@@ -1614,9 +1614,8 @@ static void spapr_drc_reset(void *opaque)
 
 static void spapr_create_lmb_dr_connectors(sPAPRMachineState *spapr)
 {
-    MachineState *machine = MACHINE(spapr);
     uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
-    uint32_t nr_lmbs = (machine->maxram_size - machine->ram_size)/lmb_size;
+    uint32_t nr_lmbs = memory_region_size(&spapr->hotplug_memory.mr)/lmb_size;
     int i;
 
     for (i = 0; i < nr_lmbs; i++) {
@@ -1792,6 +1791,20 @@ static void ppc_spapr_init(MachineState *machine)
 
         spapr->hotplug_memory.base = ROUND_UP(machine->ram_size,
                                               SPAPR_HOTPLUG_MEM_ALIGN);
+
+        /*
+         * Ensure that there is enough space in the hotplug memory
+         * region to support a maximum alignment of 256M per slot.
+         * Though PowerPC has 16M and 16G huge pages, PowerKVM supports
+         * only 16M and hence 256M alignment works until we start
+         * supporting 1G hugepage.
+         *
+         * Instead of max hugepage size alignment, we go with 256M
+         * alignment because we require the DIMM address to be 256M
+         * aligned as we maintain DRC objects for every 256M memory
+         * chunk in the hotplug memory region.
+         */
+        hotplug_mem_size +=  SPAPR_MEMORY_BLOCK_SIZE * machine->ram_slots;
         memory_region_init(&spapr->hotplug_memory.mr, OBJECT(spapr),
                            "hotplug-memory", hotplug_mem_size);
         memory_region_add_subregion(sysmem, spapr->hotplug_memory.base,
@@ -2114,7 +2127,7 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
     PCDIMMDevice *dimm = PC_DIMM(dev);
     PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
     MemoryRegion *mr = ddc->get_memory_region(dimm);
-    uint64_t align = memory_region_get_alignment(mr);
+    uint64_t align = SPAPR_MEMORY_BLOCK_SIZE;
     uint64_t size = memory_region_size(mr);
     uint64_t addr;
 
-- 
2.1.0

Re: [Qemu-devel] [RFC v1 1/2] spapr: Accommadate alignment gaps in hotplug memory region

[Igor Mammedov]

On Fri,  9 Oct 2015 12:19:59 +0530
Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:

> Accommodate enough space in hotplug memory region so that it will
> be possible to align memory in each slot to 256M. When PowerPC memory
> hotplug enables inter-dimm gaps, then we could end up having
> unused/unassigned 256M memory chuck between DIMMs in the hotplug
s/chuck/chunks/

> memory region. Hence create DRC objects spanning the entire
> alignment-adjusted hotplug memory region instead of just the acutal
> hotpluggable size (machine->maxram_size - machine->ram_size).
> 
> In addition, pass 256M alignment to pc_dimm_memory_plug() so that
> the DIMM address gets aligned to 256M.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
beside fixup above,
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/ppc/spapr.c | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index fc5e7d6..4a901f0 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -721,7 +721,7 @@ static int
> spapr_populate_drconf_memory(sPAPRMachineState *spapr, void *fdt) int
> ret, i, offset; uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
>      uint32_t prop_lmb_size[] = {0, cpu_to_be32(lmb_size)};
> -    uint32_t nr_lmbs = (machine->maxram_size -
> machine->ram_size)/lmb_size;
> +    uint32_t nr_lmbs =
> memory_region_size(&spapr->hotplug_memory.mr)/lmb_size; uint32_t
> *int_buf, *cur_index, buf_len; int nr_nodes = nb_numa_nodes ?
> nb_numa_nodes : 1; 
> @@ -1614,9 +1614,8 @@ static void spapr_drc_reset(void *opaque)
>  
>  static void spapr_create_lmb_dr_connectors(sPAPRMachineState *spapr)
>  {
> -    MachineState *machine = MACHINE(spapr);
>      uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
> -    uint32_t nr_lmbs = (machine->maxram_size -
> machine->ram_size)/lmb_size;
> +    uint32_t nr_lmbs =
> memory_region_size(&spapr->hotplug_memory.mr)/lmb_size; int i;
>  
>      for (i = 0; i < nr_lmbs; i++) {
> @@ -1792,6 +1791,20 @@ static void ppc_spapr_init(MachineState
> *machine) 
>          spapr->hotplug_memory.base = ROUND_UP(machine->ram_size,
>                                                SPAPR_HOTPLUG_MEM_ALIGN);
> +
> +        /*
> +         * Ensure that there is enough space in the hotplug memory
> +         * region to support a maximum alignment of 256M per slot.
> +         * Though PowerPC has 16M and 16G huge pages, PowerKVM
> supports
> +         * only 16M and hence 256M alignment works until we start
> +         * supporting 1G hugepage.
> +         *
> +         * Instead of max hugepage size alignment, we go with 256M
> +         * alignment because we require the DIMM address to be 256M
> +         * aligned as we maintain DRC objects for every 256M memory
> +         * chunk in the hotplug memory region.
> +         */
> +        hotplug_mem_size +=  SPAPR_MEMORY_BLOCK_SIZE *
> machine->ram_slots; memory_region_init(&spapr->hotplug_memory.mr,
> OBJECT(spapr), "hotplug-memory", hotplug_mem_size);
>          memory_region_add_subregion(sysmem,
> spapr->hotplug_memory.base, @@ -2114,7 +2127,7 @@ static void
> spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> PCDIMMDevice *dimm = PC_DIMM(dev); PCDIMMDeviceClass *ddc =
> PC_DIMM_GET_CLASS(dimm); MemoryRegion *mr =
> ddc->get_memory_region(dimm);
> -    uint64_t align = memory_region_get_alignment(mr);
> +    uint64_t align = SPAPR_MEMORY_BLOCK_SIZE;
>      uint64_t size = memory_region_size(mr);
>      uint64_t addr;
>  

[Qemu-devel] [RFC v1 2/2] spapr: Force gaps between DIMM's GPA

[Bharata B Rao]

Mapping DIMMs non contiguously allows to workaround virtio bug reported
earlier:
http://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg00522.html
In this case guest kernel doesn't allocate buffers that can cross DIMM
boundary keeping each buffer local to a DIMM.

Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 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 4a901f0..a66833f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2137,7 +2137,7 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
         goto out;
     }
 
-    pc_dimm_memory_plug(dev, &ms->hotplug_memory, mr, align, false, &local_err);
+    pc_dimm_memory_plug(dev, &ms->hotplug_memory, mr, align, true, &local_err);
     if (local_err) {
         goto out;
     }
-- 
2.1.0

Re: [Qemu-devel] [RFC v1 2/2] spapr: Force gaps between DIMM's GPA

[Igor Mammedov]

On Fri,  9 Oct 2015 12:20:00 +0530
Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:

> Mapping DIMMs non contiguously allows to workaround virtio bug
> reported earlier:
> http://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg00522.html
> In this case guest kernel doesn't allocate buffers that can cross DIMM
> boundary keeping each buffer local to a DIMM.
> 
> Suggested-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  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 4a901f0..a66833f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2137,7 +2137,7 @@ static void spapr_memory_plug(HotplugHandler
> *hotplug_dev, DeviceState *dev, goto out;
>      }
>  
> -    pc_dimm_memory_plug(dev, &ms->hotplug_memory, mr, align, false,
> &local_err);
> +    pc_dimm_memory_plug(dev, &ms->hotplug_memory, mr, align, true,
> &local_err); if (local_err) {
>          goto out;
>      }