On Wed, 29 Aug 2018 17:36:09 +0200
David Hildenbrand <da...@redhat.com> wrote:
> To factor out plugging and unplugging of memory device we need access to
> the memory region. So let's replace get_region_size() by
> get_memory_region().
>
> If any memory device will in the future have multiple memory regions
> that make up the total region size, it can simply use a wrapping memory
> region to embed the other ones.
>
> Signed-off-by: David Hildenbrand <da...@redhat.com>
> ---
> hw/mem/memory-device.c | 10 ++++++----
> hw/mem/pc-dimm.c | 19 ++++++++++++++-----
> include/hw/mem/memory-device.h | 2 +-
> 3 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
> index d87599c280..3bd30d98bb 100644
> --- a/hw/mem/memory-device.c
> +++ b/hw/mem/memory-device.c
> @@ -56,11 +56,13 @@ static int memory_device_used_region_size(Object *obj,
> void *opaque)
>
> if (object_dynamic_cast(obj, TYPE_MEMORY_DEVICE)) {
> const DeviceState *dev = DEVICE(obj);
> - const MemoryDeviceState *md = MEMORY_DEVICE(obj);
> + MemoryDeviceState *md = MEMORY_DEVICE(obj);
it's a getter, why do we loose 'const' here?
> const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(obj);
>
> if (dev->realized) {
> - *size += mdc->get_region_size(md, &error_abort);
> + MemoryRegion *mr = mdc->get_memory_region(md, &error_abort);
> +
> + *size += memory_region_size(mr);
> }
> }
>
> @@ -162,12 +164,12 @@ uint64_t memory_device_get_free_addr(MachineState *ms,
> const uint64_t *hint,
> /* find address range that will fit new memory device */
> object_child_foreach(OBJECT(ms), memory_device_build_list, &list);
> for (item = list; item; item = g_slist_next(item)) {
> - const MemoryDeviceState *md = item->data;
> + MemoryDeviceState *md = item->data;
ditto
> const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(OBJECT(md));
> uint64_t md_size, md_addr;
>
> md_addr = mdc->get_addr(md);
> - md_size = mdc->get_region_size(md, &error_abort);
> + md_size = memory_region_size(mdc->get_memory_region(md,
> &error_abort));
> if (*errp) {
> goto out;
> }
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 4bf1a0acc9..a208b17c64 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -236,8 +236,8 @@ static uint64_t pc_dimm_md_get_addr(const
> MemoryDeviceState *md)
> return dimm->addr;
> }
>
> -static uint64_t pc_dimm_md_get_region_size(const MemoryDeviceState *md,
> - Error **errp)
> +static uint64_t pc_dimm_md_get_plugged_size(const MemoryDeviceState *md,
> + Error **errp)
> {
> /* dropping const here is fine as we don't touch the memory region */
> PCDIMMDevice *dimm = PC_DIMM(md);
> @@ -249,9 +249,19 @@ static uint64_t pc_dimm_md_get_region_size(const
> MemoryDeviceState *md,
> return 0;
> }
>
> + /* for a dimm plugged_size == region_size */
> return memory_region_size(mr);
> }
>
> +static MemoryRegion *pc_dimm_md_get_memory_region(MemoryDeviceState *md,
> + Error **errp)
> +{
> + PCDIMMDevice *dimm = PC_DIMM(md);
> + const PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(md);
> +
> + return ddc->get_memory_region(dimm, errp);
> +}
> +
> static void pc_dimm_md_fill_device_info(const MemoryDeviceState *md,
> MemoryDeviceInfo *info)
> {
> @@ -297,9 +307,8 @@ static void pc_dimm_class_init(ObjectClass *oc, void
> *data)
> 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_plugged_size = pc_dimm_md_get_plugged_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 f02b229837..852fd8f98a 100644
> --- a/include/hw/mem/memory-device.h
> +++ b/include/hw/mem/memory-device.h
> @@ -34,7 +34,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;
