Re: [Qemu-devel] [PATCH v1 00/11] memory-device: complete refactoring

[Igor Mammedov <imammedo@redhat.com>]

On Thu,  5 Jul 2018 13:59:32 +0200
David Hildenbrand <david@redhat.com> wrote:

> This is another part of the original series
>     [PATCH v4 00/14] MemoryDevice: use multi stage hotplug handlers
> And is based on
>     [PATCH v3 0/4] pc-dimm: pre_plug "slot" and "addr" assignment
> 
> This series completes refactoring of pre_plug, plug and unplug logic of
> memory devices. With this as a basis, one can easily have e.g. virtio
> based memory devices (virtio-mem, virtio-pmem, virtio-fs?) with minor
> modifications on e.g. x86 and s390x.
> 
> Unfortunately, the "addr" property is already used for virtio devices, so
> we will have to deal with device specific properties. So
> set_addr() for memory devices is introduced to handle that (we already
> have get_addr()).
> 
> The only way I see to avoid that would be for virtio based devices to
> introduce an indirection:
> 
> E.g. right now for my virtio-mem prototype:
> ... -object memory-backend-ram,id=mem0,size=8G \
>     -device virtio-mem-pci,id=vm0,memdev=mem0,node=0,phys-addr=0x12345
Though it seems inconvenient to have different property name,
it is not dimm device (even though it shares GPA resource)
so it is fine for it to have it's own set of properties.
(might make error reporting a bit ugly)

Also what about usecase of mmio based virtio (ARM)?


> To something like:
> ... -object memory-backend-ram,id=mem0,size=8G \
>     -object virtio-mem-backend,id=vmb0,memdev=mem0,addr=0x12345 \
>     -device virtio-mem-pci,id=vm0,vmem=vmb0 \
it's broken conceptually,
backends should deal only with host resource allocation while
'addr' is device model property and belongs to a frontend.

> Or something like (that might be interesting for virtio-pmem):
> ... -device virtio-mem-pci \ /* a virtio-mem bus */
>     -object memory-backend-ram,id=mem0,size=8G \
>     -device virtio-mem-backend,memdev=mem0,addr=0x12345 \
did you mean  ^^^^ virtio-pmem device?

Is it a separate controller and memory resource design
connected together somehow?


> But both alternatives have their pros and cons.
> 
> Opinions?
> 
> David Hildenbrand (11):
>   memory-device: fix error message when hinted address is too small
>   memory-device: introduce separate config option
>   memory-device: get_region_size()/get_plugged_size() might fail
>   memory-device: convert get_region_size() to get_memory_region()
>   memory-device: document MemoryDeviceClass
>   memory-device: add device class function set_addr()
>   pc-dimm: implement memory device class function set_addr()
>   memory-device: complete factoring out pre_plug handling
>   memory-device: complete factoring out plug handling
>   memory-device: complete factoring out unplug handling
>   memory-device: trace when pre_assigning/assigning/unassigning
>     addresses
> 
>  default-configs/i386-softmmu.mak   |  3 +-
>  default-configs/ppc64-softmmu.mak  |  3 +-
>  default-configs/x86_64-softmmu.mak |  3 +-
>  hw/Makefile.objs                   |  2 +-
>  hw/mem/Makefile.objs               |  4 +-
>  hw/mem/memory-device.c             | 60 +++++++++++++++++++-----
>  hw/mem/pc-dimm.c                   | 75 +++++++++++++++++-------------
>  hw/mem/trace-events                |  5 +-
>  include/hw/mem/memory-device.h     | 30 ++++++++----
>  qapi/misc.json                     |  2 +-
>  10 files changed, 126 insertions(+), 61 deletions(-)
>