Hi David,
On 9/26/18 11:42 AM, David Hildenbrand wrote:
> To be able to factor out address asignment of memory devices, we will
s/asignment/assignment
> have to read (get_addr()) and write (set_addr()) the address.
>
> We can't use properties for this purpose, as properties are device
> specific. E.g. while the address property for a DIMM is called "addr", it
> might be called differently (e.g. "memaddr") for other devices.
>
> Especially virtio based memory devices cannot use "addr" as that is already
> reserved and used for the address on the bus (for the proxy device).
>
> Also, it might be possible to have memory devices without address
> properties (e.g. internal DIMM-like thingies).
>
> In contrast to get_addr(), we expect that set_addr() can fail.
>
> Keep it simple for now for pc-dimm and simply set the static property, that
> will fail once realized.
>
> Reviewed-by: David Gibson <da...@gibson.dropbear.id.au>
> Reviewed-by: Igor Mammedov <imamm...@redhat.com>
> Signed-off-by: David Hildenbrand <da...@redhat.com>
Reviewed-by: Eric Auger <eric.au...@redhat.com>
Thanks
Eric
> ---
> hw/mem/pc-dimm.c | 7 +++++++
> include/hw/mem/memory-device.h | 2 ++
> 2 files changed, 9 insertions(+)
>
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 4bd8c496bb..5873172175 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -236,6 +236,12 @@ static uint64_t pc_dimm_md_get_addr(const
> MemoryDeviceState *md)
> return dimm->addr;
> }
>
> +static void pc_dimm_md_set_addr(MemoryDeviceState *md, uint64_t addr,
> + Error **errp)
> +{
> + object_property_set_uint(OBJECT(md), addr, PC_DIMM_ADDR_PROP, errp);
> +}
> +
> static MemoryRegion *pc_dimm_md_get_memory_region(MemoryDeviceState *md,
> Error **errp)
> {
> @@ -286,6 +292,7 @@ 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;
> + mdc->set_addr = pc_dimm_md_set_addr;
> /* for a dimm plugged_size == region_size */
> mdc->get_plugged_size = memory_device_get_region_size;
> mdc->get_memory_region = pc_dimm_md_get_memory_region;
> diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h
> index 642797655f..6395942b27 100644
> --- a/include/hw/mem/memory-device.h
> +++ b/include/hw/mem/memory-device.h
> @@ -34,6 +34,7 @@ typedef struct MemoryDeviceState {
> * @get_addr: The address of the @md in guest physical memory. "0" means that
> * no address has been specified by the user and that no address has been
> * assigned yet.
> + * @set_addr: Set the address of the @md in guest physical memory.
> * @get_plugged_size: The amount of memory provided by this @md currently
> * usable ("plugged") by the guest. This is helpful for devices that
> * dynamically manage the amount of memory accessible by the guest via
> @@ -51,6 +52,7 @@ typedef struct MemoryDeviceClass {
>
> /* public */
> uint64_t (*get_addr)(const MemoryDeviceState *md);
> + void (*set_addr)(MemoryDeviceState *md, uint64_t addr, Error **errp);
> uint64_t (*get_plugged_size)(const MemoryDeviceState *md, Error **errp);
> MemoryRegion *(*get_memory_region)(MemoryDeviceState *md, Error **errp);
> void (*fill_device_info)(const MemoryDeviceState *md,
>
