[Qemu-devel] [PATCH 00/11] pc: kvm: memory hotplug fixes

[Qemu-devel] [PATCH 00/11] pc: kvm: memory hotplug fixes

[Igor Mammedov]

Series 
  * fixes [1/11] QEMU crash when non aligned DIMM is used in KVM mode.
  * adds extra checks/enforcement to avoid non aligned DIMM address/sizes
    and prevents guest failures when it tries to deal with such DIMMs
  * prevents QEMU from starting with not baked by KVM amount of
    memory slots
  * fixes hotplug-memory region fragmentation, enabled for 2.2 and newer
  * enforces backend alignment on DIMMs address/size for optimal
    performance, enabled for 2.2 and newer
  

Igor Mammedov (11):
  pc: kvm: check if KVM has free memory slots to avoid abort()
  kvm: provide API to query amount of memory slots supported by KVM
  pc: check if KVM has enough memory slots for DIMM devices
  pc: make pc_dimm_plug() more readble
  pc: limit DIMM address and size to page aligned values
  memory: expose alignment used for allocating RAM as MemoryRegion API
  pc: add pc_init_pci_2_1() and pc_compat_2_1()
  pc: align DIMM's address/size by backend's alignment value
  pc: pc-dimm: use backend alignment during address auto allocation
  pc: explicitly check maxmem limit when adding DIMM
  pc: count in 1Gb hugepage alignment when sizing hotplug-memory
    container

 exec.c                   |  9 +++--
 hw/i386/pc.c             | 98 ++++++++++++++++++++++++++++++++++++++++++++++--
 hw/i386/pc_piix.c        | 16 +++++++-
 hw/i386/pc_q35.c         | 16 +++++++-
 hw/mem/pc-dimm.c         | 19 +++++++++-
 include/exec/exec-all.h  |  2 +-
 include/exec/memory.h    |  2 +
 include/hw/i386/pc.h     |  4 ++
 include/hw/mem/pc-dimm.h |  2 +-
 include/qemu/osdep.h     |  3 +-
 include/sysemu/kvm.h     |  2 +
 kvm-all.c                | 32 +++++++++++++++-
 kvm-stub.c               | 10 +++++
 memory.c                 |  5 +++
 target-s390x/kvm.c       |  2 +-
 util/oslib-posix.c       |  5 ++-
 util/oslib-win32.c       |  2 +-
 17 files changed, 212 insertions(+), 17 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 01/11] pc: kvm: check if KVM has free memory slots to avoid abort()

[Igor Mammedov]

When more memory devices han available KVM memory slots
are used, QEMU crashes with:

kvm_alloc_slot: no free slot available
Aborted (core dumped)

Fix this by checking that KVM has a free slot before
attempting to map memory in guest address space.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c         |  5 +++++
 include/sysemu/kvm.h |  1 +
 kvm-all.c            | 18 +++++++++++++++++-
 kvm-stub.c           |  5 +++++
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 61aba9f..f6dfd9b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1607,6 +1607,11 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
         goto out;
     }
 
+    if (kvm_enabled() && !kvm_has_free_slot(machine)) {
+        error_setg(&local_err, "hypervisor has no free memory slots left");
+        goto out;
+    }
+
     memory_region_add_subregion(&pcms->hotplug_memory,
                                 addr - pcms->hotplug_memory_base, mr);
     vmstate_register_ram(mr, dev);
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index b0cd657..22e42ef 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -163,6 +163,7 @@ extern KVMState *kvm_state;
 
 /* external API */
 
+bool kvm_has_free_slot(MachineState *ms);
 int kvm_has_sync_mmu(void);
 int kvm_has_vcpu_events(void);
 int kvm_has_robust_singlestep(void);
diff --git a/kvm-all.c b/kvm-all.c
index 44a5e72..c24e74b 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -132,7 +132,7 @@ static const KVMCapabilityInfo kvm_required_capabilites[] = {
     KVM_CAP_LAST_INFO
 };
 
-static KVMSlot *kvm_alloc_slot(KVMState *s)
+static KVMSlot *kvm_get_free_slot(KVMState *s)
 {
     int i;
 
@@ -142,6 +142,22 @@ static KVMSlot *kvm_alloc_slot(KVMState *s)
         }
     }
 
+    return NULL;
+}
+
+bool kvm_has_free_slot(MachineState *ms)
+{
+    return kvm_get_free_slot(KVM_STATE(ms->accelerator));
+}
+
+static KVMSlot *kvm_alloc_slot(KVMState *s)
+{
+    KVMSlot *slot = kvm_get_free_slot(s);
+
+    if (slot) {
+        return slot;
+    }
+
     fprintf(stderr, "%s: no free slot available\n", __func__);
     abort();
 }
diff --git a/kvm-stub.c b/kvm-stub.c
index 43fc0dd..7ba90c5 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -147,4 +147,9 @@ int kvm_irqchip_remove_irqfd_notifier(KVMState *s, EventNotifier *n, int virq)
 {
     return -ENOSYS;
 }
+
+bool kvm_has_free_slot(MachineState *ms)
+{
+    return false;
+}
 #endif
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 01/11] pc: kvm: check if KVM has free memory slots to avoid abort()

[Paolo Bonzini]

On 31/10/2014 17:38, Igor Mammedov wrote:
> When more memory devices han available KVM memory slots
> are used, QEMU crashes with:
> 
> kvm_alloc_slot: no free slot available
> Aborted (core dumped)
> 
> Fix this by checking that KVM has a free slot before
> attempting to map memory in guest address space.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/i386/pc.c         |  5 +++++
>  include/sysemu/kvm.h |  1 +
>  kvm-all.c            | 18 +++++++++++++++++-
>  kvm-stub.c           |  5 +++++
>  4 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 61aba9f..f6dfd9b 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1607,6 +1607,11 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
>          goto out;
>      }
>  
> +    if (kvm_enabled() && !kvm_has_free_slot(machine)) {
> +        error_setg(&local_err, "hypervisor has no free memory slots left");
> +        goto out;
> +    }
> +
>      memory_region_add_subregion(&pcms->hotplug_memory,
>                                  addr - pcms->hotplug_memory_base, mr);
>      vmstate_register_ram(mr, dev);
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index b0cd657..22e42ef 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -163,6 +163,7 @@ extern KVMState *kvm_state;
>  
>  /* external API */
>  
> +bool kvm_has_free_slot(MachineState *ms);
>  int kvm_has_sync_mmu(void);
>  int kvm_has_vcpu_events(void);
>  int kvm_has_robust_singlestep(void);
> diff --git a/kvm-all.c b/kvm-all.c
> index 44a5e72..c24e74b 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -132,7 +132,7 @@ static const KVMCapabilityInfo kvm_required_capabilites[] = {
>      KVM_CAP_LAST_INFO
>  };
>  
> -static KVMSlot *kvm_alloc_slot(KVMState *s)
> +static KVMSlot *kvm_get_free_slot(KVMState *s)
>  {
>      int i;
>  
> @@ -142,6 +142,22 @@ static KVMSlot *kvm_alloc_slot(KVMState *s)
>          }
>      }
>  
> +    return NULL;
> +}
> +
> +bool kvm_has_free_slot(MachineState *ms)
> +{
> +    return kvm_get_free_slot(KVM_STATE(ms->accelerator));
> +}
> +
> +static KVMSlot *kvm_alloc_slot(KVMState *s)
> +{
> +    KVMSlot *slot = kvm_get_free_slot(s);
> +
> +    if (slot) {
> +        return slot;
> +    }
> +
>      fprintf(stderr, "%s: no free slot available\n", __func__);
>      abort();
>  }
> diff --git a/kvm-stub.c b/kvm-stub.c
> index 43fc0dd..7ba90c5 100644
> --- a/kvm-stub.c
> +++ b/kvm-stub.c
> @@ -147,4 +147,9 @@ int kvm_irqchip_remove_irqfd_notifier(KVMState *s, EventNotifier *n, int virq)
>  {
>      return -ENOSYS;
>  }
> +
> +bool kvm_has_free_slot(MachineState *ms)
> +{
> +    return false;
> +}
>  #endif
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH 01/11] pc: kvm: check if KVM has free memory slots to avoid abort()

[Marcel Apfelbaum]

On Fri, 2014-10-31 at 16:38 +0000, Igor Mammedov wrote:
> When more memory devices han available KVM memory slots
/s/han/than

> are used, QEMU crashes with:

Better to say (from Laura Novich <lnovich@redhat.com>):

When more memory devices are used than available
KVM memory slots, QEMU crashes with:

Thanks,
Marcel


> kvm_alloc_slot: no free slot available
> Aborted (core dumped)
> 
> Fix this by checking that KVM has a free slot before
> attempting to map memory in guest address space.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/i386/pc.c         |  5 +++++
>  include/sysemu/kvm.h |  1 +
>  kvm-all.c            | 18 +++++++++++++++++-
>  kvm-stub.c           |  5 +++++
>  4 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 61aba9f..f6dfd9b 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1607,6 +1607,11 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
>          goto out;
>      }
>  
> +    if (kvm_enabled() && !kvm_has_free_slot(machine)) {
> +        error_setg(&local_err, "hypervisor has no free memory slots left");
> +        goto out;
> +    }
> +
>      memory_region_add_subregion(&pcms->hotplug_memory,
>                                  addr - pcms->hotplug_memory_base, mr);
>      vmstate_register_ram(mr, dev);
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index b0cd657..22e42ef 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -163,6 +163,7 @@ extern KVMState *kvm_state;
>  
>  /* external API */
>  
> +bool kvm_has_free_slot(MachineState *ms);
>  int kvm_has_sync_mmu(void);
>  int kvm_has_vcpu_events(void);
>  int kvm_has_robust_singlestep(void);
> diff --git a/kvm-all.c b/kvm-all.c
> index 44a5e72..c24e74b 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -132,7 +132,7 @@ static const KVMCapabilityInfo kvm_required_capabilites[] = {
>      KVM_CAP_LAST_INFO
>  };
>  
> -static KVMSlot *kvm_alloc_slot(KVMState *s)
> +static KVMSlot *kvm_get_free_slot(KVMState *s)
>  {
>      int i;
>  
> @@ -142,6 +142,22 @@ static KVMSlot *kvm_alloc_slot(KVMState *s)
>          }
>      }
>  
> +    return NULL;
> +}
> +
> +bool kvm_has_free_slot(MachineState *ms)
> +{
> +    return kvm_get_free_slot(KVM_STATE(ms->accelerator));
> +}
> +
> +static KVMSlot *kvm_alloc_slot(KVMState *s)
> +{
> +    KVMSlot *slot = kvm_get_free_slot(s);
> +
> +    if (slot) {
> +        return slot;
> +    }
> +
>      fprintf(stderr, "%s: no free slot available\n", __func__);
>      abort();
>  }
> diff --git a/kvm-stub.c b/kvm-stub.c
> index 43fc0dd..7ba90c5 100644
> --- a/kvm-stub.c
> +++ b/kvm-stub.c
> @@ -147,4 +147,9 @@ int kvm_irqchip_remove_irqfd_notifier(KVMState *s, EventNotifier *n, int virq)
>  {
>      return -ENOSYS;
>  }
> +
> +bool kvm_has_free_slot(MachineState *ms)
> +{
> +    return false;
> +}
>  #endif

[Qemu-devel] [PATCH v2 01/11] pc: kvm: check if KVM has free memory slots to avoid abort()

[Igor Mammedov]

When more memory devices are used than available
KVM memory slots, QEMU crashes with:

kvm_alloc_slot: no free slot available
Aborted (core dumped)

Fix this by checking that KVM has a free slot before
attempting to map memory in guest address space.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
---
v2:
 reworded commit message fixing spelling error in it along the way
---
 hw/i386/pc.c         |  5 +++++
 include/sysemu/kvm.h |  1 +
 kvm-all.c            | 18 +++++++++++++++++-
 kvm-stub.c           |  5 +++++
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 1205db8..ce7b752 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1598,6 +1598,11 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
         goto out;
     }
 
+    if (kvm_enabled() && !kvm_has_free_slot(machine)) {
+        error_setg(&local_err, "hypervisor has no free memory slots left");
+        goto out;
+    }
+
     memory_region_add_subregion(&pcms->hotplug_memory,
                                 addr - pcms->hotplug_memory_base, mr);
     vmstate_register_ram(mr, dev);
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index b0cd657..22e42ef 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -163,6 +163,7 @@ extern KVMState *kvm_state;
 
 /* external API */
 
+bool kvm_has_free_slot(MachineState *ms);
 int kvm_has_sync_mmu(void);
 int kvm_has_vcpu_events(void);
 int kvm_has_robust_singlestep(void);
diff --git a/kvm-all.c b/kvm-all.c
index 44a5e72..c24e74b 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -132,7 +132,7 @@ static const KVMCapabilityInfo kvm_required_capabilites[] = {
     KVM_CAP_LAST_INFO
 };
 
-static KVMSlot *kvm_alloc_slot(KVMState *s)
+static KVMSlot *kvm_get_free_slot(KVMState *s)
 {
     int i;
 
@@ -142,6 +142,22 @@ static KVMSlot *kvm_alloc_slot(KVMState *s)
         }
     }
 
+    return NULL;
+}
+
+bool kvm_has_free_slot(MachineState *ms)
+{
+    return kvm_get_free_slot(KVM_STATE(ms->accelerator));
+}
+
+static KVMSlot *kvm_alloc_slot(KVMState *s)
+{
+    KVMSlot *slot = kvm_get_free_slot(s);
+
+    if (slot) {
+        return slot;
+    }
+
     fprintf(stderr, "%s: no free slot available\n", __func__);
     abort();
 }
diff --git a/kvm-stub.c b/kvm-stub.c
index 43fc0dd..7ba90c5 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -147,4 +147,9 @@ int kvm_irqchip_remove_irqfd_notifier(KVMState *s, EventNotifier *n, int virq)
 {
     return -ENOSYS;
 }
+
+bool kvm_has_free_slot(MachineState *ms)
+{
+    return false;
+}
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH 02/11] kvm: provide API to query amount of memory slots supported by KVM

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/sysemu/kvm.h |  1 +
 kvm-all.c            | 14 ++++++++++++++
 kvm-stub.c           |  5 +++++
 3 files changed, 20 insertions(+)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 22e42ef..a49c1cd 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -163,6 +163,7 @@ extern KVMState *kvm_state;
 
 /* external API */
 
+int kvm_free_slot_count(MachineState *ms);
 bool kvm_has_free_slot(MachineState *ms);
 int kvm_has_sync_mmu(void);
 int kvm_has_vcpu_events(void);
diff --git a/kvm-all.c b/kvm-all.c
index c24e74b..bb61f0d 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -132,6 +132,20 @@ static const KVMCapabilityInfo kvm_required_capabilites[] = {
     KVM_CAP_LAST_INFO
 };
 
+int kvm_free_slot_count(MachineState *ms)
+{
+    int i, count;
+    KVMState *s = KVM_STATE(ms->accelerator);
+
+    for (i = 0, count = 0; i < s->nr_slots; i++) {
+        if (s->slots[i].memory_size == 0) {
+            ++count;
+        }
+    }
+
+    return count;
+}
+
 static KVMSlot *kvm_get_free_slot(KVMState *s)
 {
     int i;
diff --git a/kvm-stub.c b/kvm-stub.c
index 7ba90c5..e49d732 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -152,4 +152,9 @@ bool kvm_has_free_slot(MachineState *ms)
 {
     return false;
 }
+
+int kvm_free_slot_count(MachineState *ms)
+{
+    return INT_MAX;
+}
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH 03/11] pc: check if KVM has enough memory slots for DIMM devices

[Igor Mammedov]

check amount of available KVM memory slots after all
devices were initialized and exit with error if
there isn't enough free memory slots for DIMMs.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f6dfd9b..41d91fb 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1125,6 +1125,36 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
     return guest_info;
 }
 
+static int pc_dimm_count(Object *obj, void *opaque)
+{
+    int *count = opaque;
+
+    if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
+        (*count)++;
+    }
+
+    object_child_foreach(obj, pc_dimm_count, opaque);
+    return 0;
+}
+
+static void pc_kvm_slot_check(Notifier *notifier, void *data)
+{
+    MachineState *ms = MACHINE(qdev_get_machine());
+    int free_slots = kvm_free_slot_count(ms);
+    int used_ram_slots = 0;
+
+    pc_dimm_count(OBJECT(ms), &used_ram_slots);
+    if ((ms->ram_slots - used_ram_slots) > free_slots) {
+        error_report("KVM doesn't support more than %d memory slots",
+                     kvm_free_slot_count(ms));
+        exit(EXIT_FAILURE);
+    }
+}
+
+static Notifier kvm_slot_check_on_machine_done = {
+                        .notify = pc_kvm_slot_check
+                     };
+
 /* setup pci memory address space mapping into system address space */
 void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
                             MemoryRegion *pci_address_space)
@@ -1269,6 +1299,8 @@ FWCfgState *pc_memory_init(MachineState *machine,
                            "hotplug-memory", hotplug_mem_size);
         memory_region_add_subregion(system_memory, pcms->hotplug_memory_base,
                                     &pcms->hotplug_memory);
+
+        qemu_add_machine_init_done_notifier(&kvm_slot_check_on_machine_done);
     }
 
     /* Initialize PC system firmware */
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 03/11] pc: check if KVM has enough memory slots for DIMM devices

[Paolo Bonzini]

On 31/10/2014 17:38, Igor Mammedov wrote:
> check amount of available KVM memory slots after all
> devices were initialized and exit with error if
> there isn't enough free memory slots for DIMMs.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/i386/pc.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index f6dfd9b..41d91fb 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1125,6 +1125,36 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
>      return guest_info;
>  }
>  
> +static int pc_dimm_count(Object *obj, void *opaque)
> +{
> +    int *count = opaque;
> +
> +    if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
> +        (*count)++;
> +    }
> +
> +    object_child_foreach(obj, pc_dimm_count, opaque);
> +    return 0;
> +}
> +
> +static void pc_kvm_slot_check(Notifier *notifier, void *data)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    int free_slots = kvm_free_slot_count(ms);
> +    int used_ram_slots = 0;
> +
> +    pc_dimm_count(OBJECT(ms), &used_ram_slots);
> +    if ((ms->ram_slots - used_ram_slots) > free_slots) {
> +        error_report("KVM doesn't support more than %d memory slots",
> +                     kvm_free_slot_count(ms));
> +        exit(EXIT_FAILURE);
> +    }
> +}
> +
> +static Notifier kvm_slot_check_on_machine_done = {
> +                        .notify = pc_kvm_slot_check
> +                     };
> +
>  /* setup pci memory address space mapping into system address space */
>  void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
>                              MemoryRegion *pci_address_space)
> @@ -1269,6 +1299,8 @@ FWCfgState *pc_memory_init(MachineState *machine,
>                             "hotplug-memory", hotplug_mem_size);
>          memory_region_add_subregion(system_memory, pcms->hotplug_memory_base,
>                                      &pcms->hotplug_memory);
> +
> +        qemu_add_machine_init_done_notifier(&kvm_slot_check_on_machine_done);
>      }
>  
>      /* Initialize PC system firmware */
> 

This will always be off by 4 or so due to system RAM and ROM slots.  I
think patch 1 is enough.

Paolo

Re: [Qemu-devel] [PATCH 03/11] pc: check if KVM has enough memory slots for DIMM devices

[Igor Mammedov]

On Mon, 03 Nov 2014 12:51:50 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 31/10/2014 17:38, Igor Mammedov wrote:
> > check amount of available KVM memory slots after all
> > devices were initialized and exit with error if
> > there isn't enough free memory slots for DIMMs.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  hw/i386/pc.c | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index f6dfd9b..41d91fb 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -1125,6 +1125,36 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t
> > below_4g_mem_size, return guest_info;
> >  }
> >  
> > +static int pc_dimm_count(Object *obj, void *opaque)
> > +{
> > +    int *count = opaque;
> > +
> > +    if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
> > +        (*count)++;
> > +    }
> > +
> > +    object_child_foreach(obj, pc_dimm_count, opaque);
> > +    return 0;
> > +}
> > +
> > +static void pc_kvm_slot_check(Notifier *notifier, void *data)
> > +{
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> > +    int free_slots = kvm_free_slot_count(ms);
> > +    int used_ram_slots = 0;
> > +
> > +    pc_dimm_count(OBJECT(ms), &used_ram_slots);
> > +    if ((ms->ram_slots - used_ram_slots) > free_slots) {
> > +        error_report("KVM doesn't support more than %d memory
> > slots",
> > +                     kvm_free_slot_count(ms));
> > +        exit(EXIT_FAILURE);
> > +    }
> > +}
> > +
> > +static Notifier kvm_slot_check_on_machine_done = {
> > +                        .notify = pc_kvm_slot_check
> > +                     };
> > +
> >  /* setup pci memory address space mapping into system address
> > space */ void pc_pci_as_mapping_init(Object *owner, MemoryRegion
> > *system_memory, MemoryRegion *pci_address_space)
> > @@ -1269,6 +1299,8 @@ FWCfgState *pc_memory_init(MachineState
> > *machine, "hotplug-memory", hotplug_mem_size);
> >          memory_region_add_subregion(system_memory,
> > pcms->hotplug_memory_base, &pcms->hotplug_memory);
> > +
> > +
> > qemu_add_machine_init_done_notifier(&kvm_slot_check_on_machine_done); }
> >  
> >      /* Initialize PC system firmware */
> > 
> 
> This will always be off by 4 or so due to system RAM and ROM slots.  I
> think patch 1 is enough.
The goal of this patch is to prevent starting guest with not supported
amount of lots requested on CLI with -m slots=too_much, patch 1 won't
prevent user from it.

As for "always be off by 4 or so", unfortunately consumed slots are not
constant and depend on CLI options.
If guest is started 1Gb then 5 slots are consumed, with 4Gb 5 slots are
consumed. Devices might also consume slots, for example vga does. I
would bet that some another device won't consume it. Hence machine done
notifier is suggested.

I wouldn't try to assume/make default consumed slots as constant
sinceit's  quite fragile and easy to break. Runtime check looks like
more stable approach in general.



> 
> Paolo

Re: [Qemu-devel] [PATCH 03/11] pc: check if KVM has enough memory slots for DIMM devices

[Paolo Bonzini]

On 03/11/2014 18:00, Igor Mammedov wrote:
>> > This will always be off by 4 or so due to system RAM and ROM slots.  I
>> > think patch 1 is enough.
> The goal of this patch is to prevent starting guest with not supported
> amount of lots requested on CLI with -m slots=too_much, patch 1 won't
> prevent user from it.

Understood.  But I think -m slots=too_much is not an important problem.
 What is problematic is filling those slots, which patch 1 will help with.

Paolo

Re: [Qemu-devel] [PATCH 03/11] pc: check if KVM has enough memory slots for DIMM devices

[Igor Mammedov]

On Mon, 03 Nov 2014 18:32:51 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> 
> 
> On 03/11/2014 18:00, Igor Mammedov wrote:
> >> > This will always be off by 4 or so due to system RAM and ROM
> >> > slots.  I think patch 1 is enough.
> > The goal of this patch is to prevent starting guest with not
> > supported amount of lots requested on CLI with -m slots=too_much,
> > patch 1 won't prevent user from it.
> 
> Understood.  But I think -m slots=too_much is not an important
> problem. What is problematic is filling those slots, which patch 1
> will help with.

From user's pov it's still error, i.e. he asked on CLI for one amount
of hotpluggable slots and couldn't use all of them at runtime.

I think in addition to this patch we need QMP accessible parameter for
libvirt to tell limit, so that upper layers could tell user how many
slot it's possible to use. something like this:
 /machine/available_memory_slots


> 
> Paolo

Re: [Qemu-devel] [PATCH 03/11] pc: check if KVM has enough memory slots for DIMM devices

[Paolo Bonzini]

On 03/11/2014 20:11, Igor Mammedov wrote:
>> > Understood.  But I think -m slots=too_much is not an important
>> > problem. What is problematic is filling those slots, which patch 1
>> > will help with.
> From user's pov it's still error, i.e. he asked on CLI for one amount
> of hotpluggable slots and couldn't use all of them at runtime.

But he cannot even with your patch, if he was off by just a couple
items.  So I don't think the improvement is worth the extra complication
of the code and the small layering violation.

Paolo

Re: [Qemu-devel] [PATCH 03/11] pc: check if KVM has enough memory slots for DIMM devices

[Igor Mammedov]

On Tue, 04 Nov 2014 09:45:15 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 03/11/2014 20:11, Igor Mammedov wrote:
> >> > Understood.  But I think -m slots=too_much is not an important
> >> > problem. What is problematic is filling those slots, which patch 1
> >> > will help with.
> > From user's pov it's still error, i.e. he asked on CLI for one amount
> > of hotpluggable slots and couldn't use all of them at runtime.
> 
> But he cannot even with your patch, if he was off by just a couple
> items.  So I don't think the improvement is worth the extra complication
> of the code and the small layering violation.
With this patch user can launch QEMU with exact max amount of possible
slots with exception that he won't hotplug other than pc-dimm devices
that consume KVM's memory slots (not sure if such exists (hotpluggable)).
but, sure we can drop this patch until someone complains.

How about adding QMP interface to query amount of possible slots,
so libvirt could get a hint about max possible value?

Re: [Qemu-devel] [PATCH 03/11] pc: check if KVM has enough memory slots for DIMM devices

[Paolo Bonzini]

On 04/11/2014 16:39, Igor Mammedov wrote:
>> > But he cannot even with your patch, if he was off by just a couple
>> > items.  So I don't think the improvement is worth the extra complication
>> > of the code and the small layering violation.
> With this patch user can launch QEMU with exact max amount of possible
> slots with exception that he won't hotplug other than pc-dimm devices
> that consume KVM's memory slots (not sure if such exists (hotpluggable)).
> but, sure we can drop this patch until someone complains.

Note that option ROM BARs (which are usually mapped only by firmware,
but also when accessed via sysfs) and device assignment both consume
memory slots.  Slots occupied by pflash can also come and go, depending
on whether the flash is visible as ROM or in programming mode.

Paolo

Re: [Qemu-devel] [PATCH 03/11] pc: check if KVM has enough memory slots for DIMM devices

[Igor Mammedov]

On Tue, 04 Nov 2014 16:42:23 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> 
> 
> On 04/11/2014 16:39, Igor Mammedov wrote:
> >> > But he cannot even with your patch, if he was off by just a couple
> >> > items.  So I don't think the improvement is worth the extra complication
> >> > of the code and the small layering violation.
> > With this patch user can launch QEMU with exact max amount of possible
> > slots with exception that he won't hotplug other than pc-dimm devices
> > that consume KVM's memory slots (not sure if such exists (hotpluggable)).
> > but, sure we can drop this patch until someone complains.
> 
> Note that option ROM BARs (which are usually mapped only by firmware,
> but also when accessed via sysfs) and device assignment both consume
> memory slots.  Slots occupied by pflash can also come and go, depending
> on whether the flash is visible as ROM or in programming mode.
Then there is a bigger problem, once KVM slots are saturated QEMU will crash
if one of above actions from guest happens, and we can't even error out on them.

> 
> Paolo
> 

Re: [Qemu-devel] [PATCH 03/11] pc: check if KVM has enough memory slots for DIMM devices

[Paolo Bonzini]

On 04/11/2014 17:14, Igor Mammedov wrote:
>> > Note that option ROM BARs (which are usually mapped only by firmware,
>> > but also when accessed via sysfs) and device assignment both consume
>> > memory slots.  Slots occupied by pflash can also come and go, depending
>> > on whether the flash is visible as ROM or in programming mode.
> Then there is a bigger problem, once KVM slots are saturated QEMU will crash
> if one of above actions from guest happens, and we can't even error out on them.

Yes.  I wish I had a better idea than checking for free memslots
manually in all of them.

Paolo

[Qemu-devel] [PATCH 04/11] pc: make pc_dimm_plug() more readble

[Igor Mammedov]

split addr initialization from declaration so that
later when new local vars are added property getter
wouldn't drift off of error check.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 41d91fb..31f11f3 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1597,8 +1597,9 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
     PCDIMMDevice *dimm = PC_DIMM(dev);
     PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
     MemoryRegion *mr = ddc->get_memory_region(dimm);
-    uint64_t addr = object_property_get_int(OBJECT(dimm), PC_DIMM_ADDR_PROP,
-                                            &local_err);
+    uint64_t addr;
+
+    addr = object_property_get_int(OBJECT(dimm), PC_DIMM_ADDR_PROP, &local_err);
     if (local_err) {
         goto out;
     }
-- 
1.8.3.1

[Qemu-devel] [PATCH 05/11] pc: limit DIMM address and size to page aligned values

[Igor Mammedov]

When running in KVM mode, kvm_set_phys_mem() will silently
fail if registered MemoryRegion address/size is not page
aligned. Causing memory hotplug failure in guest.

Mapping non aligned MemoryRegion in TCG mode 'works', but
sane guest OS still expects page aligned memory module
and fails to initialize it if it's not aligned.

So do not allow non aligned (i.e. valid) address/size
values for DIMM to avoid either KVM failure or guest
issues caused by it.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c             |  3 ++-
 hw/mem/pc-dimm.c         | 14 +++++++++++++-
 include/hw/mem/pc-dimm.h |  2 +-
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 31f11f3..1460074 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1597,6 +1597,7 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
     PCDIMMDevice *dimm = PC_DIMM(dev);
     PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
     MemoryRegion *mr = ddc->get_memory_region(dimm);
+    uint64_t align = TARGET_PAGE_SIZE;
     uint64_t addr;
 
     addr = object_property_get_int(OBJECT(dimm), PC_DIMM_ADDR_PROP, &local_err);
@@ -1606,7 +1607,7 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
 
     addr = pc_dimm_get_free_addr(pcms->hotplug_memory_base,
                                  memory_region_size(&pcms->hotplug_memory),
-                                 !addr ? NULL : &addr,
+                                 !addr ? NULL : &addr, align,
                                  memory_region_size(mr), &local_err);
     if (local_err) {
         goto out;
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index a800ea7..4944f0f 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -139,7 +139,7 @@ static int pc_dimm_built_list(Object *obj, void *opaque)
 
 uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
                                uint64_t address_space_size,
-                               uint64_t *hint, uint64_t size,
+                               uint64_t *hint, uint64_t align, uint64_t size,
                                Error **errp)
 {
     GSList *list = NULL, *item;
@@ -152,6 +152,18 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
         goto out;
     }
 
+    if (hint && QEMU_ALIGN_UP(*hint, align) != *hint) {
+        error_setg(errp, "address must be aligned to 0x%" PRIx64 " bytes",
+                   align);
+        goto out;
+    }
+
+    if (QEMU_ALIGN_UP(size, align) != size) {
+        error_setg(errp, "backend memory size must be multiple of 0x%"
+                   PRIx64, align);
+        goto out;
+    }
+
     assert(address_space_end > address_space_start);
     object_child_foreach(qdev_get_machine(), pc_dimm_built_list, &list);
 
diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index 761eeef..e1dcbbc 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -72,7 +72,7 @@ typedef struct PCDIMMDeviceClass {
 
 uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
                                uint64_t address_space_size,
-                               uint64_t *hint, uint64_t size,
+                               uint64_t *hint, uint64_t align, uint64_t size,
                                Error **errp);
 
 int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp);
-- 
1.8.3.1

[Qemu-devel] [PATCH 06/11] memory: expose alignment used for allocating RAM as MemoryRegion API

[Igor Mammedov]

introduce memory_region_get_alignment() that returns
underlying memory block alignment or 0 if it's not
relevant/implemented for backend.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 exec.c                  | 9 ++++++---
 include/exec/exec-all.h | 2 +-
 include/exec/memory.h   | 2 ++
 include/qemu/osdep.h    | 3 ++-
 memory.c                | 5 +++++
 target-s390x/kvm.c      | 2 +-
 util/oslib-posix.c      | 5 ++++-
 util/oslib-win32.c      | 2 +-
 8 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/exec.c b/exec.c
index 759055d..ad5cf12 100644
--- a/exec.c
+++ b/exec.c
@@ -909,14 +909,15 @@ static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
                              uint16_t section);
 static subpage_t *subpage_init(AddressSpace *as, hwaddr base);
 
-static void *(*phys_mem_alloc)(size_t size) = qemu_anon_ram_alloc;
+static void *(*phys_mem_alloc)(size_t size, uint64_t *align) =
+                               qemu_anon_ram_alloc;
 
 /*
  * Set a custom physical guest memory alloator.
  * Accelerators with unusual needs may need this.  Hopefully, we can
  * get rid of it eventually.
  */
-void phys_mem_set_alloc(void *(*alloc)(size_t))
+void phys_mem_set_alloc(void *(*alloc)(size_t, uint64_t *align))
 {
     phys_mem_alloc = alloc;
 }
@@ -1098,6 +1099,7 @@ static void *file_ram_alloc(RAMBlock *block,
         error_propagate(errp, local_err);
         goto error;
     }
+    block->mr->align = hpagesize;
 
     if (memory < hpagesize) {
         error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to "
@@ -1309,7 +1311,8 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp)
         if (xen_enabled()) {
             xen_ram_alloc(new_block->offset, new_block->length, new_block->mr);
         } else {
-            new_block->host = phys_mem_alloc(new_block->length);
+            new_block->host = phys_mem_alloc(new_block->length,
+                                             &new_block->mr->align);
             if (!new_block->host) {
                 error_setg_errno(errp, errno,
                                  "cannot set up guest memory '%s'",
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 421a142..0844885 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -333,7 +333,7 @@ extern uintptr_t tci_tb_ptr;
 
 #if !defined(CONFIG_USER_ONLY)
 
-void phys_mem_set_alloc(void *(*alloc)(size_t));
+void phys_mem_set_alloc(void *(*alloc)(size_t, uint64_t *align));
 
 struct MemoryRegion *iotlb_to_region(AddressSpace *as, hwaddr index);
 bool io_mem_read(struct MemoryRegion *mr, hwaddr addr,
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 072aad2..4caaa9b 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -146,6 +146,7 @@ struct MemoryRegion {
     hwaddr addr;
     void (*destructor)(MemoryRegion *mr);
     ram_addr_t ram_addr;
+    uint64_t align;
     bool subpage;
     bool terminates;
     bool romd_mode;
@@ -819,6 +820,7 @@ void memory_region_add_subregion_overlap(MemoryRegion *mr,
  */
 ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr);
 
+uint64_t memory_region_get_alignment(const MemoryRegion *mr);
 /**
  * memory_region_del_subregion: Remove a subregion.
  *
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 1565404..f0c77fd 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -5,6 +5,7 @@
 #include <stdarg.h>
 #include <stddef.h>
 #include <stdbool.h>
+#include <stdint.h>
 #include <sys/types.h>
 #ifdef __OpenBSD__
 #include <sys/signal.h>
@@ -97,7 +98,7 @@ typedef signed int              int_fast16_t;
 int qemu_daemon(int nochdir, int noclose);
 void *qemu_try_memalign(size_t alignment, size_t size);
 void *qemu_memalign(size_t alignment, size_t size);
-void *qemu_anon_ram_alloc(size_t size);
+void *qemu_anon_ram_alloc(size_t size, uint64_t *align);
 void qemu_vfree(void *ptr);
 void qemu_anon_ram_free(void *ptr, size_t size);
 
diff --git a/memory.c b/memory.c
index 30f77b2..bf50a2c 100644
--- a/memory.c
+++ b/memory.c
@@ -1739,6 +1739,11 @@ ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr)
     return mr->ram_addr;
 }
 
+uint64_t memory_region_get_alignment(const MemoryRegion *mr)
+{
+    return mr->align;
+}
+
 static int cmp_flatrange_addr(const void *addr_, const void *fr_)
 {
     const AddrRange *addr = addr_;
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 5b10a25..f49e5bc 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -404,7 +404,7 @@ int kvm_arch_get_registers(CPUState *cs)
  * to grow. We also have to use MAP parameters that avoid
  * read-only mapping of guest pages.
  */
-static void *legacy_s390_alloc(size_t size)
+static void *legacy_s390_alloc(size_t size, , uint64_t *align)
 {
     void *mem;
 
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 016a047..96f3b9c 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -124,7 +124,7 @@ void *qemu_memalign(size_t alignment, size_t size)
 }
 
 /* alloc shared memory pages */
-void *qemu_anon_ram_alloc(size_t size)
+void *qemu_anon_ram_alloc(size_t size, uint64_t *alignment)
 {
     size_t align = QEMU_VMALLOC_ALIGN;
     size_t total = size + align - getpagesize();
@@ -136,6 +136,9 @@ void *qemu_anon_ram_alloc(size_t size)
         return NULL;
     }
 
+    if (alignment) {
+        *alignment = align;
+    }
     ptr += offset;
     total -= offset;
 
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index a3eab4a..87cfbe0 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -67,7 +67,7 @@ void *qemu_memalign(size_t alignment, size_t size)
     return qemu_oom_check(qemu_try_memalign(alignment, size));
 }
 
-void *qemu_anon_ram_alloc(size_t size)
+void *qemu_anon_ram_alloc(size_t size, uint64_t *align)
 {
     void *ptr;
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 07/11] pc: add pc_init_pci_2_1() and pc_compat_2_1()

[Igor Mammedov]

it will be used later to disable incompatible
machine changes in 2.2.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc_piix.c | 13 ++++++++++++-
 hw/i386/pc_q35.c  | 13 ++++++++++++-
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 91a20cb..162b77e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -302,8 +302,13 @@ static void pc_init_pci(MachineState *machine)
     pc_init1(machine, 1, 1);
 }
 
+static void pc_compat_2_1(MachineState *machine)
+{
+}
+
 static void pc_compat_2_0(MachineState *machine)
 {
+    pc_compat_2_1(machine);
     /* This value depends on the actual DSDT and SSDT compiled into
      * the source QEMU; unfortunately it depends on the binary and
      * not on the machine type, so we cannot make pc-i440fx-1.7 work on
@@ -368,6 +373,12 @@ static void pc_compat_1_2(MachineState *machine)
     x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_PV_EOI);
 }
 
+static void pc_init_pci_2_1(MachineState *machine)
+{
+    pc_compat_2_1(machine);
+    pc_init_pci(machine);
+}
+
 static void pc_init_pci_2_0(MachineState *machine)
 {
     pc_compat_2_0(machine);
@@ -471,7 +482,7 @@ static QEMUMachine pc_i440fx_machine_v2_2 = {
 static QEMUMachine pc_i440fx_machine_v2_1 = {
     PC_I440FX_2_1_MACHINE_OPTIONS,
     .name = "pc-i440fx-2.1",
-    .init = pc_init_pci,
+    .init = pc_init_pci_2_1,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_2_1,
         { /* end of list */ }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index e225c6d..0490980 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -280,8 +280,13 @@ static void pc_q35_init(MachineState *machine)
     }
 }
 
+static void pc_compat_2_1(MachineState *machine)
+{
+}
+
 static void pc_compat_2_0(MachineState *machine)
 {
+    pc_compat_2_1(machine);
     smbios_legacy_mode = true;
     has_reserved_memory = false;
     pc_set_legacy_acpi_data_size();
@@ -315,6 +320,12 @@ static void pc_compat_1_4(MachineState *machine)
     x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, CPUID_EXT_PCLMULQDQ);
 }
 
+static void pc_q35_init_2_1(MachineState *machine)
+{
+    pc_compat_2_1(machine);
+    pc_q35_init(machine);
+}
+
 static void pc_q35_init_2_0(MachineState *machine)
 {
     pc_compat_2_0(machine);
@@ -367,7 +378,7 @@ static QEMUMachine pc_q35_machine_v2_2 = {
 static QEMUMachine pc_q35_machine_v2_1 = {
     PC_Q35_2_1_MACHINE_OPTIONS,
     .name = "pc-q35-2.1",
-    .init = pc_q35_init,
+    .init = pc_q35_init_2_1,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_2_1,
         { /* end of list */ }
-- 
1.8.3.1

[Qemu-devel] [PATCH 08/11] pc: align DIMM's address/size by backend's alignment value

[Igor Mammedov]

Performance wise it's better to align GVA by the backend's
page size.

Also do not allow to create DIMM device with suboptimal
size (i.e. not aligned to backends page size) to aviod
memory loss.

Do above only for 2.2 and newer machine types to avoid
breaking working configs with 2.1 machine type.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c         | 15 +++++++++++++++
 hw/i386/pc_piix.c    |  3 +++
 hw/i386/pc_q35.c     |  3 +++
 include/hw/i386/pc.h |  4 ++++
 4 files changed, 25 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 1460074..ed50344 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1605,6 +1605,10 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
         goto out;
     }
 
+    if (memory_region_get_alignment(mr) && pcms->enforce_aligned_dimm) {
+        align = memory_region_get_alignment(mr);
+    }
+
     addr = pc_dimm_get_free_addr(pcms->hotplug_memory_base,
                                  memory_region_size(&pcms->hotplug_memory),
                                  !addr ? NULL : &addr, align,
@@ -1727,6 +1731,13 @@ static void pc_machine_set_max_ram_below_4g(Object *obj, Visitor *v,
     pcms->max_ram_below_4g = value;
 }
 
+static bool pc_machine_get_aligned_dimm(Object *obj, Error **errp)
+{
+    PCMachineState *pcms = PC_MACHINE(obj);
+
+    return pcms->enforce_aligned_dimm;
+}
+
 static void pc_machine_initfn(Object *obj)
 {
     PCMachineState *pcms = PC_MACHINE(obj);
@@ -1739,6 +1750,10 @@ static void pc_machine_initfn(Object *obj)
                         pc_machine_get_max_ram_below_4g,
                         pc_machine_set_max_ram_below_4g,
                         NULL, NULL, NULL);
+    pcms->enforce_aligned_dimm = true;
+    object_property_add_bool(obj, PC_MACHINE_ENFORCE_ALINED_DIMM,
+                             pc_machine_get_aligned_dimm,
+                             NULL, NULL);
 }
 
 static void pc_machine_class_init(ObjectClass *oc, void *data)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 162b77e..4adcd9b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -304,6 +304,9 @@ static void pc_init_pci(MachineState *machine)
 
 static void pc_compat_2_1(MachineState *machine)
 {
+    PCMachineState *pcms = PC_MACHINE(machine);
+
+    pcms->enforce_aligned_dimm = false;
 }
 
 static void pc_compat_2_0(MachineState *machine)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 0490980..cf4953e 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -282,6 +282,9 @@ static void pc_q35_init(MachineState *machine)
 
 static void pc_compat_2_1(MachineState *machine)
 {
+    PCMachineState *pcms = PC_MACHINE(machine);
+
+    pcms->enforce_aligned_dimm = false;
 }
 
 static void pc_compat_2_0(MachineState *machine)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index c4ee520..a4e2a39 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -23,6 +23,8 @@
  * address space begins.
  * @hotplug_memory: hotplug memory addess space container
  * @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
  */
 struct PCMachineState {
     /*< private >*/
@@ -35,11 +37,13 @@ struct PCMachineState {
     HotplugHandler *acpi_dev;
 
     uint64_t max_ram_below_4g;
+    bool enforce_aligned_dimm;
 };
 
 #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device"
 #define PC_MACHINE_MEMHP_REGION_SIZE "hotplug-memory-region-size"
 #define PC_MACHINE_MAX_RAM_BELOW_4G "max-ram-below-4g"
+#define PC_MACHINE_ENFORCE_ALINED_DIMM "enforce-aligned-dimm"
 
 /**
  * PCMachineClass:
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 08/11] pc: align DIMM's address/size by backend's alignment value

[Andrey Korolyov]

On Fri, Oct 31, 2014 at 7:38 PM, Igor Mammedov <imammedo@redhat.com> wrote:
> Performance wise it's better to align GVA by the backend's
> page size.
>
> Also do not allow to create DIMM device with suboptimal
> size (i.e. not aligned to backends page size) to aviod
> memory loss.
>
> Do above only for 2.2 and newer machine types to avoid
> breaking working configs with 2.1 machine type.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/i386/pc.c         | 15 +++++++++++++++
>  hw/i386/pc_piix.c    |  3 +++
>  hw/i386/pc_q35.c     |  3 +++
>  include/hw/i386/pc.h |  4 ++++
>  4 files changed, 25 insertions(+)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 1460074..ed50344 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1605,6 +1605,10 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
>          goto out;
>      }
>
> +    if (memory_region_get_alignment(mr) && pcms->enforce_aligned_dimm) {
> +        align = memory_region_get_alignment(mr);
> +    }
> +
>      addr = pc_dimm_get_free_addr(pcms->hotplug_memory_base,
>                                   memory_region_size(&pcms->hotplug_memory),
>                                   !addr ? NULL : &addr, align,
> @@ -1727,6 +1731,13 @@ static void pc_machine_set_max_ram_below_4g(Object *obj, Visitor *v,
>      pcms->max_ram_below_4g = value;
>  }
>
> +static bool pc_machine_get_aligned_dimm(Object *obj, Error **errp)
> +{
> +    PCMachineState *pcms = PC_MACHINE(obj);
> +
> +    return pcms->enforce_aligned_dimm;
> +}
> +
>  static void pc_machine_initfn(Object *obj)
>  {
>      PCMachineState *pcms = PC_MACHINE(obj);
> @@ -1739,6 +1750,10 @@ static void pc_machine_initfn(Object *obj)
>                          pc_machine_get_max_ram_below_4g,
>                          pc_machine_set_max_ram_below_4g,
>                          NULL, NULL, NULL);
> +    pcms->enforce_aligned_dimm = true;
> +    object_property_add_bool(obj, PC_MACHINE_ENFORCE_ALINED_DIMM,
> +                             pc_machine_get_aligned_dimm,
> +                             NULL, NULL);
>  }
>
>  static void pc_machine_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 162b77e..4adcd9b 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -304,6 +304,9 @@ static void pc_init_pci(MachineState *machine)
>
>  static void pc_compat_2_1(MachineState *machine)
>  {
> +    PCMachineState *pcms = PC_MACHINE(machine);
> +
> +    pcms->enforce_aligned_dimm = false;
>  }
>
>  static void pc_compat_2_0(MachineState *machine)
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 0490980..cf4953e 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -282,6 +282,9 @@ static void pc_q35_init(MachineState *machine)
>
>  static void pc_compat_2_1(MachineState *machine)
>  {
> +    PCMachineState *pcms = PC_MACHINE(machine);
> +
> +    pcms->enforce_aligned_dimm = false;
>  }
>
>  static void pc_compat_2_0(MachineState *machine)
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index c4ee520..a4e2a39 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -23,6 +23,8 @@
>   * address space begins.
>   * @hotplug_memory: hotplug memory addess space container
>   * @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
>   */
>  struct PCMachineState {
>      /*< private >*/
> @@ -35,11 +37,13 @@ struct PCMachineState {
>      HotplugHandler *acpi_dev;
>
>      uint64_t max_ram_below_4g;
> +    bool enforce_aligned_dimm;
>  };
>
>  #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device"
>  #define PC_MACHINE_MEMHP_REGION_SIZE "hotplug-memory-region-size"
>  #define PC_MACHINE_MAX_RAM_BELOW_4G "max-ram-below-4g"
> +#define PC_MACHINE_ENFORCE_ALINED_DIMM "enforce-aligned-dimm"
>
>  /**
>   * PCMachineClass:
> --
> 1.8.3.1
>
>


Spell: ALINED.

Re: [Qemu-devel] [PATCH 08/11] pc: align DIMM's address/size by backend's alignment value

[Michael S. Tsirkin]

On Mon, Nov 03, 2014 at 09:48:20PM +0400, Andrey Korolyov wrote:
> On Fri, Oct 31, 2014 at 7:38 PM, Igor Mammedov <imammedo@redhat.com> wrote:
> > Performance wise it's better to align GVA by the backend's
> > page size.
> >
> > Also do not allow to create DIMM device with suboptimal
> > size (i.e. not aligned to backends page size) to aviod
> > memory loss.
> >
> > Do above only for 2.2 and newer machine types to avoid
> > breaking working configs with 2.1 machine type.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  hw/i386/pc.c         | 15 +++++++++++++++
> >  hw/i386/pc_piix.c    |  3 +++
> >  hw/i386/pc_q35.c     |  3 +++
> >  include/hw/i386/pc.h |  4 ++++
> >  4 files changed, 25 insertions(+)
> >
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 1460074..ed50344 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -1605,6 +1605,10 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
> >          goto out;
> >      }
> >
> > +    if (memory_region_get_alignment(mr) && pcms->enforce_aligned_dimm) {
> > +        align = memory_region_get_alignment(mr);
> > +    }
> > +
> >      addr = pc_dimm_get_free_addr(pcms->hotplug_memory_base,
> >                                   memory_region_size(&pcms->hotplug_memory),
> >                                   !addr ? NULL : &addr, align,
> > @@ -1727,6 +1731,13 @@ static void pc_machine_set_max_ram_below_4g(Object *obj, Visitor *v,
> >      pcms->max_ram_below_4g = value;
> >  }
> >
> > +static bool pc_machine_get_aligned_dimm(Object *obj, Error **errp)
> > +{
> > +    PCMachineState *pcms = PC_MACHINE(obj);
> > +
> > +    return pcms->enforce_aligned_dimm;
> > +}
> > +
> >  static void pc_machine_initfn(Object *obj)
> >  {
> >      PCMachineState *pcms = PC_MACHINE(obj);
> > @@ -1739,6 +1750,10 @@ static void pc_machine_initfn(Object *obj)
> >                          pc_machine_get_max_ram_below_4g,
> >                          pc_machine_set_max_ram_below_4g,
> >                          NULL, NULL, NULL);
> > +    pcms->enforce_aligned_dimm = true;
> > +    object_property_add_bool(obj, PC_MACHINE_ENFORCE_ALINED_DIMM,
> > +                             pc_machine_get_aligned_dimm,
> > +                             NULL, NULL);
> >  }
> >
> >  static void pc_machine_class_init(ObjectClass *oc, void *data)
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index 162b77e..4adcd9b 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -304,6 +304,9 @@ static void pc_init_pci(MachineState *machine)
> >
> >  static void pc_compat_2_1(MachineState *machine)
> >  {
> > +    PCMachineState *pcms = PC_MACHINE(machine);
> > +
> > +    pcms->enforce_aligned_dimm = false;
> >  }
> >
> >  static void pc_compat_2_0(MachineState *machine)
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 0490980..cf4953e 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -282,6 +282,9 @@ static void pc_q35_init(MachineState *machine)
> >
> >  static void pc_compat_2_1(MachineState *machine)
> >  {
> > +    PCMachineState *pcms = PC_MACHINE(machine);
> > +
> > +    pcms->enforce_aligned_dimm = false;
> >  }
> >
> >  static void pc_compat_2_0(MachineState *machine)
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index c4ee520..a4e2a39 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -23,6 +23,8 @@
> >   * address space begins.
> >   * @hotplug_memory: hotplug memory addess space container
> >   * @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
> >   */
> >  struct PCMachineState {
> >      /*< private >*/
> > @@ -35,11 +37,13 @@ struct PCMachineState {
> >      HotplugHandler *acpi_dev;
> >
> >      uint64_t max_ram_below_4g;
> > +    bool enforce_aligned_dimm;
> >  };
> >
> >  #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device"
> >  #define PC_MACHINE_MEMHP_REGION_SIZE "hotplug-memory-region-size"
> >  #define PC_MACHINE_MAX_RAM_BELOW_4G "max-ram-below-4g"
> > +#define PC_MACHINE_ENFORCE_ALINED_DIMM "enforce-aligned-dimm"
> >
> >  /**
> >   * PCMachineClass:
> > --
> > 1.8.3.1
> >
> >
> 
> 
> Spell: ALINED.


Igor as I've merged this tentatively, pls send fixup on top, with
fixup! prefix.

Thanks!

[Qemu-devel] [PATCH 8/11] fixup! pc: align DIMM's address/size by backend's alignment value

[Igor Mammedov]

Performance wise it's better to align GVA by the backend's
page size.

Also do not allow to create DIMM device with suboptimal
size (i.e. not aligned to backends page size) to aviod
memory loss.

Do above only for 2.2 and newer machine types to avoid
breaking working configs with 2.1 machine type.

Signef-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c         | 2 +-
 include/hw/i386/pc.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 9d2f800..503a9a3 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1789,7 +1789,7 @@ static void pc_machine_initfn(Object *obj)
                         pc_machine_set_max_ram_below_4g,
                         NULL, NULL, NULL);
     pcms->enforce_aligned_dimm = true;
-    object_property_add_bool(obj, PC_MACHINE_ENFORCE_ALINED_DIMM,
+    object_property_add_bool(obj, PC_MACHINE_ENFORCE_ALIGNED_DIMM,
                              pc_machine_get_aligned_dimm,
                              NULL, NULL);
 }
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index a4e2a39..4f9b931 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -43,7 +43,7 @@ struct PCMachineState {
 #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device"
 #define PC_MACHINE_MEMHP_REGION_SIZE "hotplug-memory-region-size"
 #define PC_MACHINE_MAX_RAM_BELOW_4G "max-ram-below-4g"
-#define PC_MACHINE_ENFORCE_ALINED_DIMM "enforce-aligned-dimm"
+#define PC_MACHINE_ENFORCE_ALIGNED_DIMM "enforce-aligned-dimm"
 
 /**
  * PCMachineClass:
-- 
1.8.3.1

[Qemu-devel] [PATCH 09/11] pc: pc-dimm: use backend alignment during address auto allocation

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/mem/pc-dimm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 4944f0f..d431834 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -146,6 +146,9 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
     uint64_t new_addr, ret = 0;
     uint64_t address_space_end = address_space_start + address_space_size;
 
+    g_assert(QEMU_ALIGN_UP(address_space_start, align) == address_space_start);
+    g_assert(QEMU_ALIGN_UP(address_space_size, align) == address_space_size);
+
     if (!address_space_size) {
         error_setg(errp, "memory hotplug is not enabled, "
                          "please add maxmem option");
@@ -189,7 +192,7 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
                 error_setg(errp, "address range conflicts with '%s'", d->id);
                 goto out;
             }
-            new_addr = dimm->addr + dimm_size;
+            new_addr = QEMU_ALIGN_UP(dimm->addr + dimm_size, align);
         }
     }
     ret = new_addr;
-- 
1.8.3.1

[Qemu-devel] [PATCH 10/11] pc: explicitly check maxmem limit when adding DIMM

[Igor Mammedov]

Currently maxmem limit is not checked and depends on
hotplug region container not being able to fit more RAM
than maxmem. Do check explicitly so that it would
be possible to change hotplug container size later
to deal with fragmentation.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index ed50344..0d9681e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1586,6 +1586,25 @@ void qemu_register_pc_machine(QEMUMachine *m)
     g_free(name);
 }
 
+static int pc_existing_dimms_capacity(Object *obj, void *opaque)
+{
+    Error *local_err = NULL;
+    uint64_t *size = opaque;
+
+    if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
+        (*size) += object_property_get_int(obj, PC_DIMM_SIZE_PROP, &local_err);
+
+        if (local_err) {
+            qerror_report_err(local_err);
+            error_free(local_err);
+            return 1;
+        }
+    }
+
+    object_child_foreach(obj, pc_dimm_count, opaque);
+    return 0;
+}
+
 static void pc_dimm_plug(HotplugHandler *hotplug_dev,
                          DeviceState *dev, Error **errp)
 {
@@ -1597,6 +1616,7 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
     PCDIMMDevice *dimm = PC_DIMM(dev);
     PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
     MemoryRegion *mr = ddc->get_memory_region(dimm);
+    uint64_t existing_dimms_capacity = 0;
     uint64_t align = TARGET_PAGE_SIZE;
     uint64_t addr;
 
@@ -1617,6 +1637,19 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
         goto out;
     }
 
+    if (pc_existing_dimms_capacity(OBJECT(machine), &existing_dimms_capacity)) {
+        error_setg(&local_err, "failed to get total size of existing DIMMs");
+        goto out;
+    }
+
+    if (existing_dimms_capacity + memory_region_size(mr) >
+        machine->maxram_size - machine->ram_size) {
+        error_setg(&local_err, "not enough space, currently 0x%" PRIx64
+                   " in use of total 0x%" PRIx64,
+                   existing_dimms_capacity, machine->maxram_size);
+        goto out;
+    }
+
     object_property_set_int(OBJECT(dev), addr, PC_DIMM_ADDR_PROP, &local_err);
     if (local_err) {
         goto out;
-- 
1.8.3.1

[Qemu-devel] [PATCH 10/11] fixup! pc: explicitly check maxmem limit when adding DIMM

[Igor Mammedov]

Fix build failure that would be caused by missing pc_dimm_count()
that was introduced in dropped
  [03/11] pc: check if KVM has enough memory slots for DIMM devices

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 1d413a7..e656658 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1550,6 +1550,18 @@ void qemu_register_pc_machine(QEMUMachine *m)
     g_free(name);
 }
 
+static int pc_dimm_count(Object *obj, void *opaque)
+{
+    int *count = opaque;
+
+    if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
+        (*count)++;
+    }
+
+    object_child_foreach(obj, pc_dimm_count, opaque);
+    return 0;
+}
+
 static int pc_existing_dimms_capacity(Object *obj, void *opaque)
 {
     Error *local_err = NULL;
-- 
1.8.3.1

[Qemu-devel] [PATCH 11/11] pc: count in 1Gb hugepage alignment when sizing hotplug-memory container

[Igor Mammedov]

if DIMMs with mized size/alignment are interleaved
in creation order it could lead hotplug-memory
container fragmentation and following inability
to use all RAM upto maxmem.
For example:
    -m 4G,slots=3,maxmem=7G
    -object memory-backend-file,id=mem-1,size=256M,mem-path=/pagesize-2MB
    -device pc-dimm,id=mem1,memdev=mem-1
    -object memory-backend-file,id=mem-2,size=1G,mem-path=/pagesize-1GB
    -device pc-dimm,id=mem2,memdev=mem-2
    -object memory-backend-file,id=mem-3,size=256M,mem-path=/pagesize-2MB
    -device pc-dimm,id=mem3,memdev=mem-3

creates fragment hotplug-memory container and doesn't
allow to use 1GB hugepage backend to cosume remainig 1Gb.

To ease managment factor in max 1Gb alignment for each
memory slot when sizing hotplug-memory region so that
regadless of fragmentaion it would be possible to add
max aligned DIMM.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0d9681e..9d2f800 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1288,6 +1288,11 @@ FWCfgState *pc_memory_init(MachineState *machine,
         pcms->hotplug_memory_base =
             ROUND_UP(0x100000000ULL + above_4g_mem_size, 1ULL << 30);
 
+        if (pcms->enforce_aligned_dimm) {
+            /* size hotplug region assuming 1G page max alignment per slot */
+            hotplug_mem_size += (1ULL << 30) * machine->ram_slots;
+        }
+
         if ((pcms->hotplug_memory_base + hotplug_mem_size) <
             hotplug_mem_size) {
             error_report("unsupported amount of maximum memory: " RAM_ADDR_FMT,
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 11/11] pc: count in 1Gb hugepage alignment when sizing hotplug-memory container

[Igor Mammedov]

if DIMMs with different size/alignment are interleaved
in creation order, it could lead to hotplug-memory
container fragmentation and following inability to use
all RAM upto maxmem.
For example:
    -m 4G,slots=3,maxmem=7G
    -object memory-backend-file,id=mem-1,size=256M,mem-path=/pagesize-2MB
    -device pc-dimm,id=mem1,memdev=mem-1
    -object memory-backend-file,id=mem-2,size=1G,mem-path=/pagesize-1GB
    -device pc-dimm,id=mem2,memdev=mem-2
    -object memory-backend-file,id=mem-3,size=256M,mem-path=/pagesize-2MB
    -device pc-dimm,id=mem3,memdev=mem-3

fragments hotplug-memory container and doesn't allow
to use 1GB hugepage backend to consume remainig 1Gb.

To ease managment factor count in max 1Gb alignment for
each memory slot when sizing hotplug-memory region so
that regadless of fragmentaion it would be possible to
add max aligned DIMM.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
 spelling fixes in commit message
---
 hw/i386/pc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 4c62ba7..6dccb16 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1277,6 +1277,11 @@ FWCfgState *pc_memory_init(MachineState *machine,
         pcms->hotplug_memory_base =
             ROUND_UP(0x100000000ULL + above_4g_mem_size, 1ULL << 30);
 
+        if (pcms->enforce_aligned_dimm) {
+            /* size hotplug region assuming 1G page max alignment per slot */
+            hotplug_mem_size += (1ULL << 30) * machine->ram_slots;
+        }
+
         if ((pcms->hotplug_memory_base + hotplug_mem_size) <
             hotplug_mem_size) {
             error_report("unsupported amount of maximum memory: " RAM_ADDR_FMT,
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 00/11] pc: kvm: memory hotplug fixes

[Michael S. Tsirkin]

On Fri, Oct 31, 2014 at 04:38:31PM +0000, Igor Mammedov wrote:
> Series 
>   * fixes [1/11] QEMU crash when non aligned DIMM is used in KVM mode.
>   * adds extra checks/enforcement to avoid non aligned DIMM address/sizes
>     and prevents guest failures when it tries to deal with such DIMMs
>   * prevents QEMU from starting with not baked by KVM amount of
>     memory slots
>   * fixes hotplug-memory region fragmentation, enabled for 2.2 and newer
>   * enforces backend alignment on DIMMs address/size for optimal
>     performance, enabled for 2.2 and newer

OK bugfixes so we can merge this after freeze date.
Paolo, could you please ack kvm changes?


> 
> Igor Mammedov (11):
>   pc: kvm: check if KVM has free memory slots to avoid abort()
>   kvm: provide API to query amount of memory slots supported by KVM
>   pc: check if KVM has enough memory slots for DIMM devices
>   pc: make pc_dimm_plug() more readble
>   pc: limit DIMM address and size to page aligned values
>   memory: expose alignment used for allocating RAM as MemoryRegion API
>   pc: add pc_init_pci_2_1() and pc_compat_2_1()
>   pc: align DIMM's address/size by backend's alignment value
>   pc: pc-dimm: use backend alignment during address auto allocation
>   pc: explicitly check maxmem limit when adding DIMM
>   pc: count in 1Gb hugepage alignment when sizing hotplug-memory
>     container
> 
>  exec.c                   |  9 +++--
>  hw/i386/pc.c             | 98 ++++++++++++++++++++++++++++++++++++++++++++++--
>  hw/i386/pc_piix.c        | 16 +++++++-
>  hw/i386/pc_q35.c         | 16 +++++++-
>  hw/mem/pc-dimm.c         | 19 +++++++++-
>  include/exec/exec-all.h  |  2 +-
>  include/exec/memory.h    |  2 +
>  include/hw/i386/pc.h     |  4 ++
>  include/hw/mem/pc-dimm.h |  2 +-
>  include/qemu/osdep.h     |  3 +-
>  include/sysemu/kvm.h     |  2 +
>  kvm-all.c                | 32 +++++++++++++++-
>  kvm-stub.c               | 10 +++++
>  memory.c                 |  5 +++
>  target-s390x/kvm.c       |  2 +-
>  util/oslib-posix.c       |  5 ++-
>  util/oslib-win32.c       |  2 +-
>  17 files changed, 212 insertions(+), 17 deletions(-)
> 
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH 00/11] pc: kvm: memory hotplug fixes

[Paolo Bonzini]

On 31/10/2014 17:38, Igor Mammedov wrote:
> Series 
>   * fixes [1/11] QEMU crash when non aligned DIMM is used in KVM mode.
>   * adds extra checks/enforcement to avoid non aligned DIMM address/sizes
>     and prevents guest failures when it tries to deal with such DIMMs
>   * prevents QEMU from starting with not baked by KVM amount of
>     memory slots
>   * fixes hotplug-memory region fragmentation, enabled for 2.2 and newer
>   * enforces backend alignment on DIMMs address/size for optimal
>     performance, enabled for 2.2 and newer
>   
> 
> Igor Mammedov (11):
>   pc: kvm: check if KVM has free memory slots to avoid abort()
>   kvm: provide API to query amount of memory slots supported by KVM
>   pc: check if KVM has enough memory slots for DIMM devices
>   pc: make pc_dimm_plug() more readble
>   pc: limit DIMM address and size to page aligned values
>   memory: expose alignment used for allocating RAM as MemoryRegion API
>   pc: add pc_init_pci_2_1() and pc_compat_2_1()
>   pc: align DIMM's address/size by backend's alignment value
>   pc: pc-dimm: use backend alignment during address auto allocation
>   pc: explicitly check maxmem limit when adding DIMM
>   pc: count in 1Gb hugepage alignment when sizing hotplug-memory
>     container
> 
>  exec.c                   |  9 +++--
>  hw/i386/pc.c             | 98 ++++++++++++++++++++++++++++++++++++++++++++++--
>  hw/i386/pc_piix.c        | 16 +++++++-
>  hw/i386/pc_q35.c         | 16 +++++++-
>  hw/mem/pc-dimm.c         | 19 +++++++++-
>  include/exec/exec-all.h  |  2 +-
>  include/exec/memory.h    |  2 +
>  include/hw/i386/pc.h     |  4 ++
>  include/hw/mem/pc-dimm.h |  2 +-
>  include/qemu/osdep.h     |  3 +-
>  include/sysemu/kvm.h     |  2 +
>  kvm-all.c                | 32 +++++++++++++++-
>  kvm-stub.c               | 10 +++++
>  memory.c                 |  5 +++
>  target-s390x/kvm.c       |  2 +-
>  util/oslib-posix.c       |  5 ++-
>  util/oslib-win32.c       |  2 +-
>  17 files changed, 212 insertions(+), 17 deletions(-)
> 

I think avoiding aborts is enough.  For now, mst, please drop patches 2
and 3.

Paolo