On Wed, 26 Sep 2018 11:42:04 +0200
David Hildenbrand <da...@redhat.com> wrote:
> We will factor out get_memory_region() from pc-dimm to memory device code
> soon. Once that is done, get_region_size() can be implemented
> generically and essentially be replaced by
> memory_device_get_region_size (and work only on get_memory_region()).
>
> We have some users of get_memory_region() (spapr and pc-dimm code) that are
> only interested in the size. So let's rework them to use
> memory_device_get_region_size() first, then we can factor out
> get_memory_region() and eventually remove get_region_size() without
> touching the same code multiple times.
>
> Signed-off-by: David Hildenbrand <da...@redhat.com>
Reviewed-by: Igor Mammedov <imamm...@redhat.com>
> ---
> hw/mem/memory-device.c | 13 ++++++++++---
> hw/mem/pc-dimm.c | 10 ++++------
> hw/ppc/spapr.c | 21 +++++++--------------
> include/hw/mem/memory-device.h | 2 ++
> 4 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
> index e935681438..882364b388 100644
> --- a/hw/mem/memory-device.c
> +++ b/hw/mem/memory-device.c
> @@ -57,10 +57,9 @@ 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);
> - const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(obj);
>
> if (dev->realized) {
> - *size += mdc->get_region_size(md, &error_abort);
> + *size += memory_device_get_region_size(md, &error_abort);
> }
> }
>
> @@ -169,7 +168,7 @@ uint64_t memory_device_get_free_addr(MachineState *ms,
> const uint64_t *hint,
> uint64_t md_size, md_addr;
>
> md_addr = mdc->get_addr(md);
> - md_size = mdc->get_region_size(md, &error_abort);
> + md_size = memory_device_get_region_size(md, &error_abort);
>
> if (ranges_overlap(md_addr, md_size, new_addr, size)) {
> if (hint) {
> @@ -268,6 +267,14 @@ void memory_device_unplug_region(MachineState *ms,
> MemoryRegion *mr)
> memory_region_del_subregion(&ms->device_memory->mr, mr);
> }
>
> +uint64_t memory_device_get_region_size(const MemoryDeviceState *md,
> + Error **errp)
> +{
> + MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(md);
> +
> + return mdc->get_region_size(md, errp);
> +}
> +
> static const TypeInfo memory_device_info = {
> .name = TYPE_MEMORY_DEVICE,
> .parent = TYPE_INTERFACE,
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index c948d57c63..76155c3f5a 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -161,16 +161,14 @@ static Property pc_dimm_properties[] = {
> static void pc_dimm_get_size(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> + Error *local_err = NULL;
> uint64_t value;
> - MemoryRegion *mr;
> - PCDIMMDevice *dimm = PC_DIMM(obj);
> - PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(obj);
>
> - mr = ddc->get_memory_region(dimm, errp);
> - if (!mr) {
> + value = memory_device_get_region_size(MEMORY_DEVICE(obj), &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> return;
> }
> - value = memory_region_size(mr);
>
> visit_type_uint64(v, name, &value, errp);
> }
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c078347b66..c08130facb 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3128,12 +3128,10 @@ static void spapr_memory_plug(HotplugHandler
> *hotplug_dev, DeviceState *dev,
> Error *local_err = NULL;
> sPAPRMachineState *ms = SPAPR_MACHINE(hotplug_dev);
> PCDIMMDevice *dimm = PC_DIMM(dev);
> - PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> - MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
> uint64_t size, addr;
> uint32_t node;
>
> - size = memory_region_size(mr);
> + size = memory_device_get_region_size(MEMORY_DEVICE(dev), &error_abort);
>
> pc_dimm_plug(dimm, MACHINE(ms), &local_err);
> if (local_err) {
> @@ -3169,9 +3167,7 @@ static void spapr_memory_pre_plug(HotplugHandler
> *hotplug_dev, DeviceState *dev,
> const sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(hotplug_dev);
> sPAPRMachineState *spapr = SPAPR_MACHINE(hotplug_dev);
> PCDIMMDevice *dimm = PC_DIMM(dev);
> - PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> Error *local_err = NULL;
> - MemoryRegion *mr;
> uint64_t size;
> Object *memdev;
> hwaddr pagesize;
> @@ -3181,11 +3177,11 @@ static void spapr_memory_pre_plug(HotplugHandler
> *hotplug_dev, DeviceState *dev,
> return;
> }
>
> - mr = ddc->get_memory_region(dimm, errp);
> - if (!mr) {
> + size = memory_device_get_region_size(MEMORY_DEVICE(dimm), &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> return;
> }
> - size = memory_region_size(mr);
>
> if (size % SPAPR_MEMORY_BLOCK_SIZE) {
> error_setg(errp, "Hotplugged memory size must be a multiple of "
> @@ -3257,9 +3253,8 @@ static sPAPRDIMMState
> *spapr_recover_pending_dimm_state(sPAPRMachineState *ms,
> PCDIMMDevice *dimm)
> {
> sPAPRDRConnector *drc;
> - PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> - MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
> - uint64_t size = memory_region_size(mr);
> + uint64_t size = memory_device_get_region_size(MEMORY_DEVICE(dimm),
> + &error_abort);
> uint32_t nr_lmbs = size / SPAPR_MEMORY_BLOCK_SIZE;
> uint32_t avail_lmbs = 0;
> uint64_t addr_start, addr;
> @@ -3325,14 +3320,12 @@ static void
> spapr_memory_unplug_request(HotplugHandler *hotplug_dev,
> sPAPRMachineState *spapr = SPAPR_MACHINE(hotplug_dev);
> Error *local_err = NULL;
> PCDIMMDevice *dimm = PC_DIMM(dev);
> - PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> - MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort);
> uint32_t nr_lmbs;
> uint64_t size, addr_start, addr;
> int i;
> sPAPRDRConnector *drc;
>
> - size = memory_region_size(mr);
> + size = memory_device_get_region_size(MEMORY_DEVICE(dimm), &error_abort);
> nr_lmbs = size / SPAPR_MEMORY_BLOCK_SIZE;
>
> addr_start = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP,
> diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h
> index 1d15cfc357..2f04d67d68 100644
> --- a/include/hw/mem/memory-device.h
> +++ b/include/hw/mem/memory-device.h
> @@ -63,5 +63,7 @@ uint64_t memory_device_get_free_addr(MachineState *ms,
> const uint64_t *hint,
> void memory_device_plug_region(MachineState *ms, MemoryRegion *mr,
> uint64_t addr);
> void memory_device_unplug_region(MachineState *ms, MemoryRegion *mr);
> +uint64_t memory_device_get_region_size(const MemoryDeviceState *md,
> + Error **errp);
>
> #endif
