Re: [Qemu-ppc] [Qemu-devel] [qemu-s390x] [PATCH v4 00/14] MemoryDevice: use multi stage hotplug handlers

[David Hildenbrand]

On 01.06.2018 14:13, Igor Mammedov wrote:
> On Fri, 25 May 2018 14:43:39 +0200
> David Hildenbrand <address@hidden> wrote:
> 
>> On 17.05.2018 10:15, David Hildenbrand wrote:
>>> We can have devices that need certain other resources that are e.g.
>>> system resources managed by the machine. We need a clean way to assign
>>> these resources (without violating layers as brought up by Igor).
>>>
>>> One example is virtio-mem/virtio-pmem. Both device types need to be
>>> assigned some region in guest physical address space. This device memory
>>> belongs to the machine and is managed by it. However, virito devices are
>>> hotplugged using the hotplug handler their proxy device implements. So we
>>> could trigger e.g. a PCI hotplug handler for virtio-pci or a CSS/CCW
>>> hotplug handler for virtio-ccw. But definetly not the machine.
>>>
>>> Now, we can route other devices through the machine hotplug handler, to
>>> properly assign/unassign resources - like a portion in guest physical
>>> address space.
> 
> To sum up review:
> Some comments apply to several patches even though I commented only once.
> 
> I'd suggest to restructure and split series into several:
>   * unplug cleanups 08/14 & co
>   * generic _preplug refactoring so we won't have to go back to that question 
> again

Looking into the details, I don't think this is possible and should be
done for address asignment. It can be done for the node. Here is the
reason why:

In pre_plug(), we acccess a device before it has been realized. This is
okay, as long as we are using "direct properties" like the "node", that
won't be touched during realize.

However, for address detection we need access to the memory region. This
is a "derived property", as it is based on the "memdev" property.

While we can easily verify the validity of "memdev" in the pc_dimm
pre_plug handler (instead of doing that in pc_dimm_realize()), we cannot
access the memory region itself before the device has been realized
using get_memory_region() in all cases.

While this works for PC_DIMM (pc_dimm_get_memory_region() is only based
on dimm->hostmem, which we can verify as stated), we cannot do the same
thing for NVDIMM, because nvdimm_get_memory_region() relies on "realize"
already being called and setting up "nvdimm->nvdimm_mr".

In short: we should call device functions only after realize(),
therefore address assignment is not possible during pre_plug.

-- 

Thanks,

David / dhildenb