Re: [Qemu-devel] [PATCH v3 10/22] memory-device: add device class function set_addr()

[David Gibson]

On Thu, Sep 20, 2018 at 12:32:31PM +0200, David Hildenbrand wrote:
1;5202;0c> To be able to factor out address asignment of memory devices, we will
> 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 from the callback.
> 
> Signed-off-by: David Hildenbrand <address@hidden>

Reviewed-by: David Gibson <address@hidden>

> ---
>  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 95c3c4bd76..3474b810ef 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -235,6 +235,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)
>  {
> @@ -285,6 +291,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 64df232919..b2fd26c262 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.
>   * @get_memory_region: The memory region of the @md of the @md that's
> @@ -48,6 +49,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,

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature