[Qemu-devel] [PATCH] pc: memhotplug: rise minimum DIMM addr/size alignment to 128Mb

[Igor Mammedov <imammedo@redhat.com>]

it's attempt to workaround virtio bug reported earlier:
http://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg00522.html
where virtio can't handle buffer that crosses border
between 2 DIMM's (i.e. 2 MemoryRegions).

Testing showed that virtio doesn't hit above bug
with 128Mb DIMM's granularity. Also linux memory
hotplug can handle hotplugged memory starting with
128Mb memory sections so lets rise minimum size limit
to 128Mb and align starting DIMM address on 128Mb.

It's certainly not the fix but it reduces risk of
crashing VM till virtio is fixed.
It also could be improved in guest's virtio side if it
would align buffers on 128Mb border and limit max  buffer
size to the same value.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
Based on PCI tree as it has patches that add
2.5 machine type.
---
 hw/i386/pc.c         |  8 +++++---
 hw/i386/pc_piix.c    | 12 ++++++++++--
 hw/i386/pc_q35.c     | 12 ++++++++++--
 include/hw/i386/pc.h |  5 ++---
 4 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b5107f7..ddb6710 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1645,8 +1645,9 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
     MemoryRegion *mr = ddc->get_memory_region(dimm);
     uint64_t align = TARGET_PAGE_SIZE;
 
-    if (memory_region_get_alignment(mr) && pcms->enforce_aligned_dimm) {
-        align = memory_region_get_alignment(mr);
+    if (pcms->enforce_aligned_dimm) {
+        align = MAX(memory_region_get_alignment(mr),
+                    pcms->enforce_aligned_dimm);
     }
 
     if (!pcms->acpi_dev) {
@@ -1936,7 +1937,8 @@ static void pc_machine_initfn(Object *obj)
                                     "Enable vmport (pc & q35)",
                                     &error_abort);
 
-    pcms->enforce_aligned_dimm = true;
+    /* align DIMM starting address/size by 128Mb */
+    pcms->enforce_aligned_dimm = 1ULL << 27;
     object_property_add_bool(obj, PC_MACHINE_ENFORCE_ALIGNED_DIMM,
                              pc_machine_get_aligned_dimm,
                              NULL, &error_abort);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index caa4edc..7671905 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -301,9 +301,17 @@ static void pc_init1(MachineState *machine,
     }
 }
 
+static void pc_compat_2_4(MachineState *machine)
+{
+    PCMachineState *pcms = PC_MACHINE(machine);
+
+    pcms->enforce_aligned_dimm = TARGET_PAGE_SIZE;
+}
+
 static void pc_compat_2_3(MachineState *machine)
 {
     PCMachineState *pcms = PC_MACHINE(machine);
+    pc_compat_2_4(machine);
     savevm_skip_section_footers();
     if (kvm_enabled()) {
         pcms->smm = ON_OFF_AUTO_OFF;
@@ -326,7 +334,7 @@ static void pc_compat_2_1(MachineState *machine)
     pc_compat_2_2(machine);
     smbios_uuid_encoded = false;
     x86_cpu_compat_kvm_no_autodisable(FEAT_8000_0001_ECX, CPUID_EXT3_SVM);
-    pcms->enforce_aligned_dimm = false;
+    pcms->enforce_aligned_dimm = 0;
 }
 
 static void pc_compat_2_0(MachineState *machine)
@@ -485,7 +493,7 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m)
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
 }
 
-DEFINE_I440FX_MACHINE(v2_4, "pc-i440fx-2.4", NULL,
+DEFINE_I440FX_MACHINE(v2_4, "pc-i440fx-2.4", pc_compat_2_3,
                       pc_i440fx_2_4_machine_options)
 
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 506b6bf..72b479f 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -284,9 +284,17 @@ static void pc_q35_init(MachineState *machine)
     }
 }
 
+static void pc_compat_2_4(MachineState *machine)
+{
+    PCMachineState *pcms = PC_MACHINE(machine);
+
+    pcms->enforce_aligned_dimm = TARGET_PAGE_SIZE;
+}
+
 static void pc_compat_2_3(MachineState *machine)
 {
     PCMachineState *pcms = PC_MACHINE(machine);
+    pc_compat_2_4(machine);
     savevm_skip_section_footers();
     if (kvm_enabled()) {
         pcms->smm = ON_OFF_AUTO_OFF;
@@ -307,7 +315,7 @@ static void pc_compat_2_1(MachineState *machine)
     PCMachineState *pcms = PC_MACHINE(machine);
 
     pc_compat_2_2(machine);
-    pcms->enforce_aligned_dimm = false;
+    pcms->enforce_aligned_dimm = 0;
     smbios_uuid_encoded = false;
     x86_cpu_compat_kvm_no_autodisable(FEAT_8000_0001_ECX, CPUID_EXT3_SVM);
 }
@@ -388,7 +396,7 @@ static void pc_q35_2_4_machine_options(MachineClass *m)
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
 }
 
-DEFINE_Q35_MACHINE(v2_4, "pc-q35-2.4", NULL,
+DEFINE_Q35_MACHINE(v2_4, "pc-q35-2.4", pc_compat_2_4,
                    pc_q35_2_4_machine_options);
 
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 6896328..fdcf0ec 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -23,8 +23,7 @@
 /**
  * PCMachineState:
  * @acpi_dev: link to ACPI PM device that performs ACPI hotplug handling
- * @enforce_aligned_dimm: check that DIMM's address/size is aligned by
- *                        backend's alignment value if provided
+ * @enforce_aligned_dimm: minimal DIMM's address/size alignment
  */
 struct PCMachineState {
     /*< private >*/
@@ -37,9 +36,9 @@ struct PCMachineState {
     ISADevice *rtc;
 
     uint64_t max_ram_below_4g;
+    uint64_t enforce_aligned_dimm;
     OnOffAuto vmport;
     OnOffAuto smm;
-    bool enforce_aligned_dimm;
     ram_addr_t below_4g_mem_size, above_4g_mem_size;
 };
 
-- 
1.8.3.1