Re: [Qemu-ppc] [PATCH v3 22/22] pc: support for virtio-pmem

[David Hildenbrand]

On 21/09/2018 18:00, Dr. David Alan Gilbert wrote:
> * David Hildenbrand (address@hidden) wrote:
>> Compile virtio-pmem for x86 and properly hotplug virtio-pmem devices
>> (memory-device part) from our pc machine hotplug handler.
>>
>> Signed-off-by: David Hildenbrand <address@hidden>
> 
> I see a few other places in the pc subdir that have checks for
> TYPE_NVDIMM; can you just explain why they are untouched:
>   i386/pc.c: pc_memory_unplug_request

I think I had an answer why that isn't needed, but for unplug we might
still be missing something.

In general: unplug requests are always directed at the proxy device (via
qdev_unplug()) (e.g. virtio-pmem-pci, see below). virtio-pmem is a child
device of virtio-pmem-pci. Once virtio-pmem-pci is finally unrealized,
the virtio-mem device will be unrealized.

Now, there are no hotplug handlers getting called as far as I can see
for that child device. there would have to be an proper call to a
hotplug handler somewhere when setting the device to unrealized. (and
that's the place where a unplug requests does not make really sense, but
only a straight unplug - because we are half way through removing the
hierarchy of devices - one of the reason why also failures of unrealize
are usually ignored).

I'll look into the details. Thanks for pointing that out, this stuff
always messes with my head.

>   acpi/ich9.c: ich9_pm_device_plug_cb
>   acpi/piix4.c:piix4_device_plug_cb

virtio-pmem is not an acpi device, that's why these don't apply.

E.g. if virtio-pmem-pci is plugged, it will most probably trigger these
hotplug handlers for the proxy virtio pci device. The proxy device
creates and realized the virtio-mem (memory-device) - without any
hotplug handlers involved. That#s the point where we can jump in and
realize the device via the machine instead.

> 
> (Not that I understand the detail of those paths, but I just
> followed where the existing nvdimm code goes)
> 
> Dave


-- 

Thanks,

David / dhildenb