diff mbox series

[v3,08/22] memory-device: factor out get_memory_region() from pc-dimm

Message ID 20180920103243.28474-9-david@redhat.com
State New
Headers show
Series memory-device: complete refactoring + virtio-pmem | expand

Commit Message

David Hildenbrand Sept. 20, 2018, 10:32 a.m. UTC 
The memory region is necessary for plugging/unplugging a memory device.
The region size (via get_region_size()) is no longer sufficient, as
besides the alignment, also the region itself is required in order to
add it to the device memory region of the machine via
- memory_region_add_subregion
- memory_region_del_subregion

So, to factor out plugging/unplugging of memory devices from pc-dimm
code, we have to factor out access to the memory region first.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/mem/nvdimm.c                |  9 ++++++---
 hw/mem/pc-dimm.c               | 26 ++++++++++++++++----------
 include/hw/mem/memory-device.h |  5 +++++
 include/hw/mem/pc-dimm.h       |  4 ----
 4 files changed, 27 insertions(+), 17 deletions(-)

Comments

David Gibson Sept. 21, 2018, 5:17 a.m. UTC | #1 
On Thu, Sep 20, 2018 at 12:32:29PM +0200, David Hildenbrand wrote:
> The memory region is necessary for plugging/unplugging a memory device.
> The region size (via get_region_size()) is no longer sufficient, as
> besides the alignment, also the region itself is required in order to
> add it to the device memory region of the machine via
> - memory_region_add_subregion
> - memory_region_del_subregion
> 
> So, to factor out plugging/unplugging of memory devices from pc-dimm
> code, we have to factor out access to the memory region first.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/mem/nvdimm.c                |  9 ++++++---
>  hw/mem/pc-dimm.c               | 26 ++++++++++++++++----------
>  include/hw/mem/memory-device.h |  5 +++++
>  include/hw/mem/pc-dimm.h       |  4 ----
>  4 files changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
> index 1c6674c4ed..49324f3fae 100644
> --- a/hw/mem/nvdimm.c
> +++ b/hw/mem/nvdimm.c
> @@ -27,6 +27,7 @@
>  #include "qapi/error.h"
>  #include "qapi/visitor.h"
>  #include "hw/mem/nvdimm.h"
> +#include "hw/mem/memory-device.h"
>  
>  static void nvdimm_get_label_size(Object *obj, Visitor *v, const char *name,
>                                    void *opaque, Error **errp)
> @@ -118,9 +119,10 @@ static void nvdimm_prepare_memory_region(NVDIMMDevice *nvdimm, Error **errp)
>      nvdimm->nvdimm_mr->align = align;
>  }
>  
> -static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm, Error **errp)
> +static MemoryRegion *nvdimm_md_get_memory_region(MemoryDeviceState *md,
> +                                                 Error **errp)
>  {
> -    NVDIMMDevice *nvdimm = NVDIMM(dimm);
> +    NVDIMMDevice *nvdimm = NVDIMM(md);
>      Error *local_err = NULL;
>  
>      if (!nvdimm->nvdimm_mr) {
> @@ -190,11 +192,12 @@ static Property nvdimm_properties[] = {
>  static void nvdimm_class_init(ObjectClass *oc, void *data)
>  {
>      PCDIMMDeviceClass *ddc = PC_DIMM_CLASS(oc);
> +    MemoryDeviceClass *mdc = MEMORY_DEVICE_CLASS(oc);
>      NVDIMMClass *nvc = NVDIMM_CLASS(oc);
>      DeviceClass *dc = DEVICE_CLASS(oc);
>  
>      ddc->realize = nvdimm_realize;
> -    ddc->get_memory_region = nvdimm_get_memory_region;
> +    mdc->get_memory_region = nvdimm_md_get_memory_region;
>      dc->props = nvdimm_properties;
>  
>      nvc->read_label_data = nvdimm_read_label_data;
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index a06da7e08e..49ad8bac2d 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -32,8 +32,7 @@ static int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp);
>  void pc_dimm_pre_plug(DeviceState *dev, MachineState *machine,
>                        const uint64_t *legacy_align, Error **errp)
>  {
> -    PCDIMMDevice *dimm = PC_DIMM(dev);
> -    PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> +    MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(dev);
>      Error *local_err = NULL;
>      MemoryRegion *mr;
>      uint64_t addr, align;
> @@ -49,7 +48,7 @@ void pc_dimm_pre_plug(DeviceState *dev, MachineState *machine,
>      object_property_set_int(OBJECT(dev), slot, PC_DIMM_SLOT_PROP, &error_abort);
>      trace_mhp_pc_dimm_assigned_slot(slot);
>  
> -    mr = ddc->get_memory_region(dimm, &local_err);
> +    mr = mdc->get_memory_region(MEMORY_DEVICE(dev), &local_err);
>      if (local_err) {
>          goto out;
>      }
> @@ -73,9 +72,10 @@ void pc_dimm_plug(DeviceState *dev, MachineState *machine, Error **errp)
>  {
>      PCDIMMDevice *dimm = PC_DIMM(dev);
>      PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> +    MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(dev);
>      MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm,
>                                                                &error_abort);
> -    MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
> +    MemoryRegion *mr = mdc->get_memory_region(MEMORY_DEVICE(dev), &error_abort);
>      uint64_t addr;
>  
>      addr = object_property_get_uint(OBJECT(dev), PC_DIMM_ADDR_PROP,
> @@ -89,9 +89,10 @@ void pc_dimm_unplug(DeviceState *dev, MachineState *machine)
>  {
>      PCDIMMDevice *dimm = PC_DIMM(dev);
>      PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> +    MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(dev);
>      MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm,
>                                                                &error_abort);
> -    MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
> +    MemoryRegion *mr = mdc->get_memory_region(MEMORY_DEVICE(dev), &error_abort);
>  
>      memory_device_unplug_region(machine, mr);
>      vmstate_unregister_ram(vmstate_mr, dev);
> @@ -237,12 +238,11 @@ static uint64_t pc_dimm_md_get_addr(const MemoryDeviceState *md)
>  static uint64_t pc_dimm_md_get_region_size(const MemoryDeviceState *md,
>                                             Error **errp)
>  {
> -    /* dropping const here is fine as we don't touch the memory region */
> -    PCDIMMDevice *dimm = PC_DIMM(md);
> -    const PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(md);
> +    MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(md);
>      MemoryRegion *mr;
>  
> -    mr = ddc->get_memory_region(dimm, errp);
> +    /* dropping const here is fine as we don't touch the memory region */
> +    mr = mdc->get_memory_region((MemoryDeviceState *)md, errp);
>      if (!mr) {
>          return 0;
>      }
> @@ -250,6 +250,12 @@ static uint64_t pc_dimm_md_get_region_size(const MemoryDeviceState *md,
>      return memory_region_size(mr);
>  }
>  
> +static MemoryRegion *pc_dimm_md_get_memory_region(MemoryDeviceState *md,
> +                                                  Error **errp)
> +{
> +    return pc_dimm_get_memory_region(PC_DIMM(md), errp);
> +}
> +
>  static void pc_dimm_md_fill_device_info(const MemoryDeviceState *md,
>                                          MemoryDeviceInfo *info)
>  {
> @@ -291,13 +297,13 @@ static void pc_dimm_class_init(ObjectClass *oc, void *data)
>      dc->props = pc_dimm_properties;
>      dc->desc = "DIMM memory module";
>  
> -    ddc->get_memory_region = pc_dimm_get_memory_region;
>      ddc->get_vmstate_memory_region = pc_dimm_get_memory_region;
>  
>      mdc->get_addr = pc_dimm_md_get_addr;
>      /* for a dimm plugged_size == region_size */
>      mdc->get_plugged_size = pc_dimm_md_get_region_size;
>      mdc->get_region_size = pc_dimm_md_get_region_size;
> +    mdc->get_memory_region = pc_dimm_md_get_memory_region;
>      mdc->fill_device_info = pc_dimm_md_fill_device_info;
>  }
>  
> diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h
> index 12f8ca9eb1..0feed4ec0d 100644
> --- a/include/hw/mem/memory-device.h
> +++ b/include/hw/mem/memory-device.h
> @@ -38,6 +38,10 @@ typedef struct MemoryDeviceState {
>   * usable ("plugged") by the guest.
>   * @get_region_size: The size of the memory region of the @md that's mapped
>   * in guest physical memory at @get_addr.
> + * @get_memory_region: The memory region of the @md of the @md that's
> + * mapped in guest physical memory at @get_addr. If a @md is ever composed
> + * of multiple successive memory regions, a covering memory region is to
> + * be used. Scattered memory regions are not supported for single devices.
>   * @fill_device_info: Translate current @md state into #MemoryDeviceInfo.
>   */
>  typedef struct MemoryDeviceClass {
> @@ -48,6 +52,7 @@ typedef struct MemoryDeviceClass {
>      uint64_t (*get_addr)(const MemoryDeviceState *md);
>      uint64_t (*get_plugged_size)(const MemoryDeviceState *md, Error **errp);
>      uint64_t (*get_region_size)(const MemoryDeviceState *md, Error **errp);
> +    MemoryRegion *(*get_memory_region)(MemoryDeviceState *md, Error **errp);
>      void (*fill_device_info)(const MemoryDeviceState *md,
>                               MemoryDeviceInfo *info);
>  } MemoryDeviceClass;
> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> index b382eb4303..b445687081 100644
> --- a/include/hw/mem/pc-dimm.h
> +++ b/include/hw/mem/pc-dimm.h
> @@ -61,9 +61,6 @@ typedef struct PCDIMMDevice {
>   * PCDIMMDeviceClass:
>   * @realize: called after common dimm is realized so that the dimm based
>   * devices get the chance to do specified operations.
> - * @get_memory_region: returns #MemoryRegion associated with @dimm which
> - * is directly mapped into the physical address space of guest. Will not
> - * fail after the device was realized.
>   * @get_vmstate_memory_region: returns #MemoryRegion which indicates the
>   * memory of @dimm should be kept during live migration. Will not fail
>   * after the device was realized.
> @@ -74,7 +71,6 @@ typedef struct PCDIMMDeviceClass {
>  
>      /* public */
>      void (*realize)(PCDIMMDevice *dimm, Error **errp);
> -    MemoryRegion *(*get_memory_region)(PCDIMMDevice *dimm, Error **errp);
>      MemoryRegion *(*get_vmstate_memory_region)(PCDIMMDevice *dimm,
>                                                 Error **errp);
>  } PCDIMMDeviceClass;
Igor Mammedov Sept. 24, 2018, 1:12 p.m. UTC | #2 
On Thu, 20 Sep 2018 12:32:29 +0200
David Hildenbrand <david@redhat.com> wrote:

> The memory region is necessary for plugging/unplugging a memory device.
> The region size (via get_region_size()) is no longer sufficient, as
> besides the alignment, also the region itself is required in order to
> add it to the device memory region of the machine via
> - memory_region_add_subregion
> - memory_region_del_subregion
> 
> So, to factor out plugging/unplugging of memory devices from pc-dimm
> code, we have to factor out access to the memory region first.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  hw/mem/nvdimm.c                |  9 ++++++---
>  hw/mem/pc-dimm.c               | 26 ++++++++++++++++----------
>  include/hw/mem/memory-device.h |  5 +++++
>  include/hw/mem/pc-dimm.h       |  4 ----
>  4 files changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
> index 1c6674c4ed..49324f3fae 100644
> --- a/hw/mem/nvdimm.c
> +++ b/hw/mem/nvdimm.c
> @@ -27,6 +27,7 @@
>  #include "qapi/error.h"
>  #include "qapi/visitor.h"
>  #include "hw/mem/nvdimm.h"
> +#include "hw/mem/memory-device.h"
>  
>  static void nvdimm_get_label_size(Object *obj, Visitor *v, const char *name,
>                                    void *opaque, Error **errp)
> @@ -118,9 +119,10 @@ static void nvdimm_prepare_memory_region(NVDIMMDevice *nvdimm, Error **errp)
>      nvdimm->nvdimm_mr->align = align;
>  }
>  
> -static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm, Error **errp)
> +static MemoryRegion *nvdimm_md_get_memory_region(MemoryDeviceState *md,
> +                                                 Error **errp)
>  {
> -    NVDIMMDevice *nvdimm = NVDIMM(dimm);
> +    NVDIMMDevice *nvdimm = NVDIMM(md);
>      Error *local_err = NULL;
>  
>      if (!nvdimm->nvdimm_mr) {
> @@ -190,11 +192,12 @@ static Property nvdimm_properties[] = {
>  static void nvdimm_class_init(ObjectClass *oc, void *data)
>  {
>      PCDIMMDeviceClass *ddc = PC_DIMM_CLASS(oc);
> +    MemoryDeviceClass *mdc = MEMORY_DEVICE_CLASS(oc);
>      NVDIMMClass *nvc = NVDIMM_CLASS(oc);
>      DeviceClass *dc = DEVICE_CLASS(oc);
>  
>      ddc->realize = nvdimm_realize;
> -    ddc->get_memory_region = nvdimm_get_memory_region;
> +    mdc->get_memory_region = nvdimm_md_get_memory_region;
>      dc->props = nvdimm_properties;
>  
>      nvc->read_label_data = nvdimm_read_label_data;
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index a06da7e08e..49ad8bac2d 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -32,8 +32,7 @@ static int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp);
>  void pc_dimm_pre_plug(DeviceState *dev, MachineState *machine,
>                        const uint64_t *legacy_align, Error **errp)
>  {
> -    PCDIMMDevice *dimm = PC_DIMM(dev);
> -    PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> +    MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(dev);
>      Error *local_err = NULL;
>      MemoryRegion *mr;
>      uint64_t addr, align;
> @@ -49,7 +48,7 @@ void pc_dimm_pre_plug(DeviceState *dev, MachineState *machine,
>      object_property_set_int(OBJECT(dev), slot, PC_DIMM_SLOT_PROP, &error_abort);
>      trace_mhp_pc_dimm_assigned_slot(slot);
>  
> -    mr = ddc->get_memory_region(dimm, &local_err);
> +    mr = mdc->get_memory_region(MEMORY_DEVICE(dev), &local_err);
>      if (local_err) {
>          goto out;
>      }
> @@ -73,9 +72,10 @@ void pc_dimm_plug(DeviceState *dev, MachineState *machine, Error **errp)
>  {
>      PCDIMMDevice *dimm = PC_DIMM(dev);
>      PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> +    MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(dev);
>      MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm,
>                                                                &error_abort);
> -    MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
> +    MemoryRegion *mr = mdc->get_memory_region(MEMORY_DEVICE(dev), &error_abort);
>      uint64_t addr;
>  
>      addr = object_property_get_uint(OBJECT(dev), PC_DIMM_ADDR_PROP,
> @@ -89,9 +89,10 @@ void pc_dimm_unplug(DeviceState *dev, MachineState *machine)
>  {
>      PCDIMMDevice *dimm = PC_DIMM(dev);
>      PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> +    MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(dev);
>      MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm,
>                                                                &error_abort);
> -    MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
> +    MemoryRegion *mr = mdc->get_memory_region(MEMORY_DEVICE(dev), &error_abort);
>  
>      memory_device_unplug_region(machine, mr);
>      vmstate_unregister_ram(vmstate_mr, dev);
> @@ -237,12 +238,11 @@ static uint64_t pc_dimm_md_get_addr(const MemoryDeviceState *md)
>  static uint64_t pc_dimm_md_get_region_size(const MemoryDeviceState *md,
>                                             Error **errp)
>  {
> -    /* dropping const here is fine as we don't touch the memory region */
> -    PCDIMMDevice *dimm = PC_DIMM(md);
> -    const PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(md);
> +    MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(md);
>      MemoryRegion *mr;
>  
> -    mr = ddc->get_memory_region(dimm, errp);
> +    /* dropping const here is fine as we don't touch the memory region */
> +    mr = mdc->get_memory_region((MemoryDeviceState *)md, errp);
>      if (!mr) {
>          return 0;
>      }
> @@ -250,6 +250,12 @@ static uint64_t pc_dimm_md_get_region_size(const MemoryDeviceState *md,
>      return memory_region_size(mr);
>  }
>  
> +static MemoryRegion *pc_dimm_md_get_memory_region(MemoryDeviceState *md,
> +                                                  Error **errp)
> +{
> +    return pc_dimm_get_memory_region(PC_DIMM(md), errp);
> +}
> +
>  static void pc_dimm_md_fill_device_info(const MemoryDeviceState *md,
>                                          MemoryDeviceInfo *info)
>  {
> @@ -291,13 +297,13 @@ static void pc_dimm_class_init(ObjectClass *oc, void *data)
>      dc->props = pc_dimm_properties;
>      dc->desc = "DIMM memory module";
>  
> -    ddc->get_memory_region = pc_dimm_get_memory_region;
>      ddc->get_vmstate_memory_region = pc_dimm_get_memory_region;
>  
>      mdc->get_addr = pc_dimm_md_get_addr;
>      /* for a dimm plugged_size == region_size */
>      mdc->get_plugged_size = pc_dimm_md_get_region_size;
>      mdc->get_region_size = pc_dimm_md_get_region_size;
> +    mdc->get_memory_region = pc_dimm_md_get_memory_region;
>      mdc->fill_device_info = pc_dimm_md_fill_device_info;
>  }
>  
> diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h
> index 12f8ca9eb1..0feed4ec0d 100644
> --- a/include/hw/mem/memory-device.h
> +++ b/include/hw/mem/memory-device.h
> @@ -38,6 +38,10 @@ typedef struct MemoryDeviceState {
>   * usable ("plugged") by the guest.
>   * @get_region_size: The size of the memory region of the @md that's mapped
>   * in guest physical memory at @get_addr.
> + * @get_memory_region: The memory region of the @md of the @md that's
                                            ^^^^^^^^^^ ^^^^^^^^^^ duplicate typo?

> + * mapped in guest physical memory at @get_addr. If a @md is ever composed
> + * of multiple successive memory regions, a covering memory region is to
> + * be used. Scattered memory regions are not supported for single devices.
>   * @fill_device_info: Translate current @md state into #MemoryDeviceInfo.
>   */
>  typedef struct MemoryDeviceClass {
> @@ -48,6 +52,7 @@ typedef struct MemoryDeviceClass {
>      uint64_t (*get_addr)(const MemoryDeviceState *md);
>      uint64_t (*get_plugged_size)(const MemoryDeviceState *md, Error **errp);
>      uint64_t (*get_region_size)(const MemoryDeviceState *md, Error **errp);
> +    MemoryRegion *(*get_memory_region)(MemoryDeviceState *md, Error **errp);
>      void (*fill_device_info)(const MemoryDeviceState *md,
>                               MemoryDeviceInfo *info);
>  } MemoryDeviceClass;
> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> index b382eb4303..b445687081 100644
> --- a/include/hw/mem/pc-dimm.h
> +++ b/include/hw/mem/pc-dimm.h
> @@ -61,9 +61,6 @@ typedef struct PCDIMMDevice {
>   * PCDIMMDeviceClass:
>   * @realize: called after common dimm is realized so that the dimm based
>   * devices get the chance to do specified operations.
> - * @get_memory_region: returns #MemoryRegion associated with @dimm which
> - * is directly mapped into the physical address space of guest. Will not
> - * fail after the device was realized.
>   * @get_vmstate_memory_region: returns #MemoryRegion which indicates the
>   * memory of @dimm should be kept during live migration. Will not fail
>   * after the device was realized.
> @@ -74,7 +71,6 @@ typedef struct PCDIMMDeviceClass {
>  
>      /* public */
>      void (*realize)(PCDIMMDevice *dimm, Error **errp);
> -    MemoryRegion *(*get_memory_region)(PCDIMMDevice *dimm, Error **errp);
>      MemoryRegion *(*get_vmstate_memory_region)(PCDIMMDevice *dimm,
>                                                 Error **errp);
>  } PCDIMMDeviceClass;
Otherwise patch looks good,
with fixup
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
diff mbox series

Patch

diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
index 1c6674c4ed..49324f3fae 100644
--- a/hw/mem/nvdimm.c
+++ b/hw/mem/nvdimm.c
@@ -27,6 +27,7 @@ 
 #include "qapi/error.h"
 #include "qapi/visitor.h"
 #include "hw/mem/nvdimm.h"
+#include "hw/mem/memory-device.h"
 
 static void nvdimm_get_label_size(Object *obj, Visitor *v, const char *name,
                                   void *opaque, Error **errp)
@@ -118,9 +119,10 @@  static void nvdimm_prepare_memory_region(NVDIMMDevice *nvdimm, Error **errp)
     nvdimm->nvdimm_mr->align = align;
 }
 
-static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm, Error **errp)
+static MemoryRegion *nvdimm_md_get_memory_region(MemoryDeviceState *md,
+                                                 Error **errp)
 {
-    NVDIMMDevice *nvdimm = NVDIMM(dimm);
+    NVDIMMDevice *nvdimm = NVDIMM(md);
     Error *local_err = NULL;
 
     if (!nvdimm->nvdimm_mr) {
@@ -190,11 +192,12 @@  static Property nvdimm_properties[] = {
 static void nvdimm_class_init(ObjectClass *oc, void *data)
 {
     PCDIMMDeviceClass *ddc = PC_DIMM_CLASS(oc);
+    MemoryDeviceClass *mdc = MEMORY_DEVICE_CLASS(oc);
     NVDIMMClass *nvc = NVDIMM_CLASS(oc);
     DeviceClass *dc = DEVICE_CLASS(oc);
 
     ddc->realize = nvdimm_realize;
-    ddc->get_memory_region = nvdimm_get_memory_region;
+    mdc->get_memory_region = nvdimm_md_get_memory_region;
     dc->props = nvdimm_properties;
 
     nvc->read_label_data = nvdimm_read_label_data;
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index a06da7e08e..49ad8bac2d 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -32,8 +32,7 @@  static int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp);
 void pc_dimm_pre_plug(DeviceState *dev, MachineState *machine,
                       const uint64_t *legacy_align, Error **errp)
 {
-    PCDIMMDevice *dimm = PC_DIMM(dev);
-    PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
+    MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(dev);
     Error *local_err = NULL;
     MemoryRegion *mr;
     uint64_t addr, align;
@@ -49,7 +48,7 @@  void pc_dimm_pre_plug(DeviceState *dev, MachineState *machine,
     object_property_set_int(OBJECT(dev), slot, PC_DIMM_SLOT_PROP, &error_abort);
     trace_mhp_pc_dimm_assigned_slot(slot);
 
-    mr = ddc->get_memory_region(dimm, &local_err);
+    mr = mdc->get_memory_region(MEMORY_DEVICE(dev), &local_err);
     if (local_err) {
         goto out;
     }
@@ -73,9 +72,10 @@  void pc_dimm_plug(DeviceState *dev, MachineState *machine, Error **errp)
 {
     PCDIMMDevice *dimm = PC_DIMM(dev);
     PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
+    MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(dev);
     MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm,
                                                               &error_abort);
-    MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
+    MemoryRegion *mr = mdc->get_memory_region(MEMORY_DEVICE(dev), &error_abort);
     uint64_t addr;
 
     addr = object_property_get_uint(OBJECT(dev), PC_DIMM_ADDR_PROP,
@@ -89,9 +89,10 @@  void pc_dimm_unplug(DeviceState *dev, MachineState *machine)
 {
     PCDIMMDevice *dimm = PC_DIMM(dev);
     PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
+    MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(dev);
     MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm,
                                                               &error_abort);
-    MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
+    MemoryRegion *mr = mdc->get_memory_region(MEMORY_DEVICE(dev), &error_abort);
 
     memory_device_unplug_region(machine, mr);
     vmstate_unregister_ram(vmstate_mr, dev);
@@ -237,12 +238,11 @@  static uint64_t pc_dimm_md_get_addr(const MemoryDeviceState *md)
 static uint64_t pc_dimm_md_get_region_size(const MemoryDeviceState *md,
                                            Error **errp)
 {
-    /* dropping const here is fine as we don't touch the memory region */
-    PCDIMMDevice *dimm = PC_DIMM(md);
-    const PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(md);
+    MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(md);
     MemoryRegion *mr;
 
-    mr = ddc->get_memory_region(dimm, errp);
+    /* dropping const here is fine as we don't touch the memory region */
+    mr = mdc->get_memory_region((MemoryDeviceState *)md, errp);
     if (!mr) {
         return 0;
     }
@@ -250,6 +250,12 @@  static uint64_t pc_dimm_md_get_region_size(const MemoryDeviceState *md,
     return memory_region_size(mr);
 }
 
+static MemoryRegion *pc_dimm_md_get_memory_region(MemoryDeviceState *md,
+                                                  Error **errp)
+{
+    return pc_dimm_get_memory_region(PC_DIMM(md), errp);
+}
+
 static void pc_dimm_md_fill_device_info(const MemoryDeviceState *md,
                                         MemoryDeviceInfo *info)
 {
@@ -291,13 +297,13 @@  static void pc_dimm_class_init(ObjectClass *oc, void *data)
     dc->props = pc_dimm_properties;
     dc->desc = "DIMM memory module";
 
-    ddc->get_memory_region = pc_dimm_get_memory_region;
     ddc->get_vmstate_memory_region = pc_dimm_get_memory_region;
 
     mdc->get_addr = pc_dimm_md_get_addr;
     /* for a dimm plugged_size == region_size */
     mdc->get_plugged_size = pc_dimm_md_get_region_size;
     mdc->get_region_size = pc_dimm_md_get_region_size;
+    mdc->get_memory_region = pc_dimm_md_get_memory_region;
     mdc->fill_device_info = pc_dimm_md_fill_device_info;
 }
 
diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h
index 12f8ca9eb1..0feed4ec0d 100644
--- a/include/hw/mem/memory-device.h
+++ b/include/hw/mem/memory-device.h
@@ -38,6 +38,10 @@  typedef struct MemoryDeviceState {
  * usable ("plugged") by the guest.
  * @get_region_size: The size of the memory region of the @md that's mapped
  * in guest physical memory at @get_addr.
+ * @get_memory_region: The memory region of the @md of the @md that's
+ * mapped in guest physical memory at @get_addr. If a @md is ever composed
+ * of multiple successive memory regions, a covering memory region is to
+ * be used. Scattered memory regions are not supported for single devices.
  * @fill_device_info: Translate current @md state into #MemoryDeviceInfo.
  */
 typedef struct MemoryDeviceClass {
@@ -48,6 +52,7 @@  typedef struct MemoryDeviceClass {
     uint64_t (*get_addr)(const MemoryDeviceState *md);
     uint64_t (*get_plugged_size)(const MemoryDeviceState *md, Error **errp);
     uint64_t (*get_region_size)(const MemoryDeviceState *md, Error **errp);
+    MemoryRegion *(*get_memory_region)(MemoryDeviceState *md, Error **errp);
     void (*fill_device_info)(const MemoryDeviceState *md,
                              MemoryDeviceInfo *info);
 } MemoryDeviceClass;
diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index b382eb4303..b445687081 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -61,9 +61,6 @@  typedef struct PCDIMMDevice {
  * PCDIMMDeviceClass:
  * @realize: called after common dimm is realized so that the dimm based
  * devices get the chance to do specified operations.
- * @get_memory_region: returns #MemoryRegion associated with @dimm which
- * is directly mapped into the physical address space of guest. Will not
- * fail after the device was realized.
  * @get_vmstate_memory_region: returns #MemoryRegion which indicates the
  * memory of @dimm should be kept during live migration. Will not fail
  * after the device was realized.
@@ -74,7 +71,6 @@  typedef struct PCDIMMDeviceClass {
 
     /* public */
     void (*realize)(PCDIMMDevice *dimm, Error **errp);
-    MemoryRegion *(*get_memory_region)(PCDIMMDevice *dimm, Error **errp);
     MemoryRegion *(*get_vmstate_memory_region)(PCDIMMDevice *dimm,
                                                Error **errp);
 } PCDIMMDeviceClass;