[Qemu-devel] [PATCH v2] pc: memhp: enforce minimal 128Mb alignment for pc-dimm

[Igor Mammedov <imammedo@redhat.com>]

commit aa8580cd "pc: memhp: force gaps between DIMM's GPA"
regressed memory hot-unplug for linux guests triggering
following BUGON
 =====
 kernel BUG at mm/memory_hotplug.c:703!
 ...
 [<ffffffff81385fa7>] acpi_memory_device_remove+0x79/0xa5
 [<ffffffff81357818>] acpi_bus_trim+0x5a/0x8d
 [<ffffffff81359026>] acpi_device_hotplug+0x1b7/0x418
 ===
    BUG_ON(phys_start_pfn & ~PAGE_SECTION_MASK);
 ===

reson for it is that x86-64 linux guest supports memory
hotplug in chunks of 128Mb and memory section also should
be 128Mb aligned.
However gaps forced between 128Mb DIMMs with backend's
natural alignment of 2Mb make the 2nd and following
DIMMs not being aligned on 128Mb boundary as it was
originally. To fix regression enforce minimal 128Mb
alignment like it was done for PPC.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
PS:
  PAGE_SECTION_MASK is derived from SECTION_SIZE_BITS which
  is arch dependent so this is fix for x86-64 target only.
  If anyone cares obout 32bit guests, it should also be fine
  for x86-32 which has 64Mb memory sections/alignment.
---
 hw/i386/pc.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 3d958ba..0f7cf7c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1610,6 +1610,8 @@ void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name)
     }
 }
 
+#define PC_MIN_DIMM_ALIGNMENT (1ULL << 27) /* 128Mb */
+
 static void pc_dimm_plug(HotplugHandler *hotplug_dev,
                          DeviceState *dev, Error **errp)
 {
@@ -1624,6 +1626,16 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
 
     if (memory_region_get_alignment(mr) && pcms->enforce_aligned_dimm) {
         align = memory_region_get_alignment(mr);
+        /*
+         * Linux x64 guests expect 128Mb aligned DIMM,
+         * but this change causes memory layout change so
+         * for compatibility apply 128Mb alignment only
+         * when forced gaps are enabled since it is the cause
+         * of misalignment.
+         */
+        if (pcmc->inter_dimm_gap && align < PC_MIN_DIMM_ALIGNMENT) {
+            align = PC_MIN_DIMM_ALIGNMENT;
+        }
     }
 
     if (!pcms->acpi_dev) {
-- 
1.8.3.1