qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/2] Revert "pc: memhp: force gaps between DIMM's GPA"
@ 2015-10-28 16:57 Michael S. Tsirkin
  2015-10-28 16:57 ` [Qemu-devel] [PATCH 2/2] Revert "memhp: extend address auto assignment to support gaps" Michael S. Tsirkin
  2015-10-29 10:31 ` [Qemu-devel] [PATCH 1/2] Revert "pc: memhp: force gaps between DIMM's GPA" Igor Mammedov
  0 siblings, 2 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2015-10-28 16:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Igor Mammedov, Richard Henderson, Eduardo Habkost, Paolo Bonzini

This reverts commit aa8580cddf011e8cedcf87f7a0fdea7549fc4704.

As described in
http://article.gmane.org/gmane.comp.emulators.qemu/371432
that commit causes linux guests to crash on memory hot-unplug.

The original problem it's trying to solve has now
been addressed within virtio.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/i386/pc.h | 1 -
 hw/i386/pc.c         | 6 ++----
 hw/i386/pc_piix.c    | 1 -
 hw/i386/pc_q35.c     | 1 -
 4 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index c5961d7..93c6dab 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -60,7 +60,6 @@ struct PCMachineClass {
 
     /*< public >*/
     bool broken_reserved_end;
-    bool inter_dimm_gap;
     HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
                                            DeviceState *dev);
 };
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 208f553..b1800fc 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1614,7 +1614,6 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
     HotplugHandlerClass *hhc;
     Error *local_err = NULL;
     PCMachineState *pcms = PC_MACHINE(hotplug_dev);
-    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
     PCDIMMDevice *dimm = PC_DIMM(dev);
     PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
     MemoryRegion *mr = ddc->get_memory_region(dimm);
@@ -1630,8 +1629,8 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
         goto out;
     }
 
-    pc_dimm_memory_plug(dev, &pcms->hotplug_memory, mr, align,
-                        pcmc->inter_dimm_gap, &local_err);
+    pc_dimm_memory_plug(dev, &pcms->hotplug_memory, mr, align, false,
+                        &local_err);
     if (local_err) {
         goto out;
     }
@@ -1951,7 +1950,6 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     PCMachineClass *pcmc = PC_MACHINE_CLASS(oc);
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
 
-    pcmc->inter_dimm_gap = true;
     pcmc->get_hotplug_handler = mc->get_hotplug_handler;
     mc->get_hotplug_handler = pc_get_hotpug_handler;
     mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 0eacde1..153a445 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -482,7 +482,6 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m)
     m->alias = NULL;
     m->is_default = 0;
     pcmc->broken_reserved_end = true;
-    pcmc->inter_dimm_gap = false;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
 }
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 3744abd..2f8f396 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -385,7 +385,6 @@ static void pc_q35_2_4_machine_options(MachineClass *m)
     pc_q35_2_5_machine_options(m);
     m->alias = NULL;
     pcmc->broken_reserved_end = true;
-    pcmc->inter_dimm_gap = false;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
 }
 
-- 
MST

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [Qemu-devel] [PATCH 2/2] Revert "memhp: extend address auto assignment to support gaps"
  2015-10-28 16:57 [Qemu-devel] [PATCH 1/2] Revert "pc: memhp: force gaps between DIMM's GPA" Michael S. Tsirkin
@ 2015-10-28 16:57 ` Michael S. Tsirkin
  2015-10-29 10:32   ` Igor Mammedov
  2015-10-29 10:31 ` [Qemu-devel] [PATCH 1/2] Revert "pc: memhp: force gaps between DIMM's GPA" Igor Mammedov
  1 sibling, 1 reply; 4+ messages in thread
From: Michael S. Tsirkin @ 2015-10-28 16:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Eduardo Habkost, Alexander Graf, qemu-ppc, Paolo Bonzini,
	Igor Mammedov, David Gibson, Richard Henderson

This reverts commit df0acded19ec4b826aa095cfc19d341bd66fafd3.

There's no point to it now that the only user has been reverted.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/mem/pc-dimm.h |  7 +++----
 hw/i386/pc.c             |  3 +--
 hw/mem/pc-dimm.c         | 15 ++++++---------
 hw/ppc/spapr.c           |  2 +-
 4 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index c1ee7b0..d83bf30 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -83,16 +83,15 @@ typedef struct MemoryHotplugState {
 
 uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
                                uint64_t address_space_size,
-                               uint64_t *hint, uint64_t align, bool gap,
-                               uint64_t size, Error **errp);
+                               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);
 
 int qmp_pc_dimm_device_list(Object *obj, void *opaque);
 uint64_t pc_existing_dimms_capacity(Error **errp);
 void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
-                         MemoryRegion *mr, uint64_t align, bool gap,
-                         Error **errp);
+                         MemoryRegion *mr, uint64_t align, Error **errp);
 void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState *hpms,
                            MemoryRegion *mr);
 #endif
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b1800fc..4bc5640 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1629,8 +1629,7 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
         goto out;
     }
 
-    pc_dimm_memory_plug(dev, &pcms->hotplug_memory, mr, align, false,
-                        &local_err);
+    pc_dimm_memory_plug(dev, &pcms->hotplug_memory, mr, align, &local_err);
     if (local_err) {
         goto out;
     }
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 2bae994..80f424b 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -33,8 +33,7 @@ typedef struct pc_dimms_capacity {
 } pc_dimms_capacity;
 
 void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
-                         MemoryRegion *mr, uint64_t align, bool gap,
-                         Error **errp)
+                         MemoryRegion *mr, uint64_t align, Error **errp)
 {
     int slot;
     MachineState *machine = MACHINE(qdev_get_machine());
@@ -50,7 +49,7 @@ void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
 
     addr = pc_dimm_get_free_addr(hpms->base,
                                  memory_region_size(&hpms->mr),
-                                 !addr ? NULL : &addr, align, gap,
+                                 !addr ? NULL : &addr, align,
                                  memory_region_size(mr), &local_err);
     if (local_err) {
         goto out;
@@ -295,8 +294,8 @@ 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 align, bool gap,
-                               uint64_t size, Error **errp)
+                               uint64_t *hint, uint64_t align, uint64_t size,
+                               Error **errp)
 {
     GSList *list = NULL, *item;
     uint64_t new_addr, ret = 0;
@@ -341,15 +340,13 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
             goto out;
         }
 
-        if (ranges_overlap(dimm->addr, dimm_size, new_addr,
-                           size + (gap ? 1 : 0))) {
+        if (ranges_overlap(dimm->addr, dimm_size, new_addr, size)) {
             if (hint) {
                 DeviceState *d = DEVICE(dimm);
                 error_setg(errp, "address range conflicts with '%s'", d->id);
                 goto out;
             }
-            new_addr = QEMU_ALIGN_UP(dimm->addr + dimm_size + (gap ? 1 : 0),
-                                     align);
+            new_addr = QEMU_ALIGN_UP(dimm->addr + dimm_size, align);
         }
     }
     ret = new_addr;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d1b0e53..a9b5f2a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2096,7 +2096,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, &local_err);
     if (local_err) {
         goto out;
     }
-- 
MST

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Revert "pc: memhp: force gaps between DIMM's GPA"
  2015-10-28 16:57 [Qemu-devel] [PATCH 1/2] Revert "pc: memhp: force gaps between DIMM's GPA" Michael S. Tsirkin
  2015-10-28 16:57 ` [Qemu-devel] [PATCH 2/2] Revert "memhp: extend address auto assignment to support gaps" Michael S. Tsirkin
@ 2015-10-29 10:31 ` Igor Mammedov
  1 sibling, 0 replies; 4+ messages in thread
From: Igor Mammedov @ 2015-10-29 10:31 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Paolo Bonzini, qemu-devel, Eduardo Habkost, Richard Henderson

On Wed, 28 Oct 2015 18:57:00 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> This reverts commit aa8580cddf011e8cedcf87f7a0fdea7549fc4704.
> 
> As described in
> http://article.gmane.org/gmane.comp.emulators.qemu/371432
> that commit causes linux guests to crash on memory hot-unplug.
> 
> The original problem it's trying to solve has now
> been addressed within virtio.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  include/hw/i386/pc.h | 1 -
>  hw/i386/pc.c         | 6 ++----
>  hw/i386/pc_piix.c    | 1 -
>  hw/i386/pc_q35.c     | 1 -
>  4 files changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index c5961d7..93c6dab 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -60,7 +60,6 @@ struct PCMachineClass {
>  
>      /*< public >*/
>      bool broken_reserved_end;
> -    bool inter_dimm_gap;
>      HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
>                                             DeviceState *dev);
>  };
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 208f553..b1800fc 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1614,7 +1614,6 @@ static void pc_dimm_plug(HotplugHandler
> *hotplug_dev, HotplugHandlerClass *hhc;
>      Error *local_err = NULL;
>      PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> -    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>      PCDIMMDevice *dimm = PC_DIMM(dev);
>      PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
>      MemoryRegion *mr = ddc->get_memory_region(dimm);
> @@ -1630,8 +1629,8 @@ static void pc_dimm_plug(HotplugHandler
> *hotplug_dev, goto out;
>      }
>  
> -    pc_dimm_memory_plug(dev, &pcms->hotplug_memory, mr, align,
> -                        pcmc->inter_dimm_gap, &local_err);
> +    pc_dimm_memory_plug(dev, &pcms->hotplug_memory, mr, align, false,
> +                        &local_err);
>      if (local_err) {
>          goto out;
>      }
> @@ -1951,7 +1950,6 @@ static void pc_machine_class_init(ObjectClass
> *oc, void *data) PCMachineClass *pcmc = PC_MACHINE_CLASS(oc);
>      HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>  
> -    pcmc->inter_dimm_gap = true;
>      pcmc->get_hotplug_handler = mc->get_hotplug_handler;
>      mc->get_hotplug_handler = pc_get_hotpug_handler;
>      mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 0eacde1..153a445 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -482,7 +482,6 @@ static void
> pc_i440fx_2_4_machine_options(MachineClass *m) m->alias = NULL;
>      m->is_default = 0;
>      pcmc->broken_reserved_end = true;
> -    pcmc->inter_dimm_gap = false;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
>  }
>  
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 3744abd..2f8f396 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -385,7 +385,6 @@ static void
> pc_q35_2_4_machine_options(MachineClass *m)
> pc_q35_2_5_machine_options(m); m->alias = NULL;
>      pcmc->broken_reserved_end = true;
> -    pcmc->inter_dimm_gap = false;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
>  }
>  

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] Revert "memhp: extend address auto assignment to support gaps"
  2015-10-28 16:57 ` [Qemu-devel] [PATCH 2/2] Revert "memhp: extend address auto assignment to support gaps" Michael S. Tsirkin
@ 2015-10-29 10:32   ` Igor Mammedov
  0 siblings, 0 replies; 4+ messages in thread
From: Igor Mammedov @ 2015-10-29 10:32 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Eduardo Habkost, Alexander Graf, qemu-devel, qemu-ppc,
	Paolo Bonzini, Richard Henderson, David Gibson

On Wed, 28 Oct 2015 18:57:03 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> This reverts commit df0acded19ec4b826aa095cfc19d341bd66fafd3.
> 
> There's no point to it now that the only user has been reverted.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  include/hw/mem/pc-dimm.h |  7 +++----
>  hw/i386/pc.c             |  3 +--
>  hw/mem/pc-dimm.c         | 15 ++++++---------
>  hw/ppc/spapr.c           |  2 +-
>  4 files changed, 11 insertions(+), 16 deletions(-)
> 
> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> index c1ee7b0..d83bf30 100644
> --- a/include/hw/mem/pc-dimm.h
> +++ b/include/hw/mem/pc-dimm.h
> @@ -83,16 +83,15 @@ typedef struct MemoryHotplugState {
>  
>  uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
>                                 uint64_t address_space_size,
> -                               uint64_t *hint, uint64_t align, bool
> gap,
> -                               uint64_t size, Error **errp);
> +                               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); 
>  int qmp_pc_dimm_device_list(Object *obj, void *opaque);
>  uint64_t pc_existing_dimms_capacity(Error **errp);
>  void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
> -                         MemoryRegion *mr, uint64_t align, bool gap,
> -                         Error **errp);
> +                         MemoryRegion *mr, uint64_t align, Error
> **errp); void pc_dimm_memory_unplug(DeviceState *dev,
> MemoryHotplugState *hpms, MemoryRegion *mr);
>  #endif
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index b1800fc..4bc5640 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1629,8 +1629,7 @@ static void pc_dimm_plug(HotplugHandler
> *hotplug_dev, goto out;
>      }
>  
> -    pc_dimm_memory_plug(dev, &pcms->hotplug_memory, mr, align, false,
> -                        &local_err);
> +    pc_dimm_memory_plug(dev, &pcms->hotplug_memory, mr, align,
> &local_err); if (local_err) {
>          goto out;
>      }
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 2bae994..80f424b 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -33,8 +33,7 @@ typedef struct pc_dimms_capacity {
>  } pc_dimms_capacity;
>  
>  void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
> -                         MemoryRegion *mr, uint64_t align, bool gap,
> -                         Error **errp)
> +                         MemoryRegion *mr, uint64_t align, Error
> **errp) {
>      int slot;
>      MachineState *machine = MACHINE(qdev_get_machine());
> @@ -50,7 +49,7 @@ void pc_dimm_memory_plug(DeviceState *dev,
> MemoryHotplugState *hpms, 
>      addr = pc_dimm_get_free_addr(hpms->base,
>                                   memory_region_size(&hpms->mr),
> -                                 !addr ? NULL : &addr, align, gap,
> +                                 !addr ? NULL : &addr, align,
>                                   memory_region_size(mr), &local_err);
>      if (local_err) {
>          goto out;
> @@ -295,8 +294,8 @@ 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 align, bool
> gap,
> -                               uint64_t size, Error **errp)
> +                               uint64_t *hint, uint64_t align,
> uint64_t size,
> +                               Error **errp)
>  {
>      GSList *list = NULL, *item;
>      uint64_t new_addr, ret = 0;
> @@ -341,15 +340,13 @@ uint64_t pc_dimm_get_free_addr(uint64_t
> address_space_start, goto out;
>          }
>  
> -        if (ranges_overlap(dimm->addr, dimm_size, new_addr,
> -                           size + (gap ? 1 : 0))) {
> +        if (ranges_overlap(dimm->addr, dimm_size, new_addr, size)) {
>              if (hint) {
>                  DeviceState *d = DEVICE(dimm);
>                  error_setg(errp, "address range conflicts with
> '%s'", d->id); goto out;
>              }
> -            new_addr = QEMU_ALIGN_UP(dimm->addr + dimm_size + (gap ?
> 1 : 0),
> -                                     align);
> +            new_addr = QEMU_ALIGN_UP(dimm->addr + dimm_size, align);
>          }
>      }
>      ret = new_addr;
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d1b0e53..a9b5f2a 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2096,7 +2096,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,
> &local_err); if (local_err) {
>          goto out;
>      }

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-10-29 10:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-28 16:57 [Qemu-devel] [PATCH 1/2] Revert "pc: memhp: force gaps between DIMM's GPA" Michael S. Tsirkin
2015-10-28 16:57 ` [Qemu-devel] [PATCH 2/2] Revert "memhp: extend address auto assignment to support gaps" Michael S. Tsirkin
2015-10-29 10:32   ` Igor Mammedov
2015-10-29 10:31 ` [Qemu-devel] [PATCH 1/2] Revert "pc: memhp: force gaps between DIMM's GPA" Igor Mammedov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).