Message ID | 20180926094219.20322-13-david@redhat.com |
---|---|
State | New |
Headers | show |
Series | memory-device: complete refactoring + virtio-pmem | expand |
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 <david@gibson.dropbear.id.au> > Reviewed-by: Igor Mammedov <imammedo@redhat.com> > Signed-off-by: David Hildenbrand <david@redhat.com> Reviewed-by: Eric Auger <eric.auger@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, >
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,