[v3,21/22] virtio-pmem: hotplug support functions

virtio-pmem devices will have to be hotplugged using the machine hotplug
handler just like other memory devices. Therefore, all machines that
want to support virtio-pmem will have to modify their machine hotplug
handler.

virtio-pmem devices are realized when their parent proxy device
(virtio-pmem-pci) is realized. Therefore, they are attached to a bus
without a hotplug handler. This makes things a lot easier, because
without a hotplug handler, we can directly get control over the device
in the machine hotplug handler (otherwise we would have to overwrite
control and pass control to the bus hotplug handler from the machine
hotplug handler).

As we have to implement support for each machine we want to support,
add a safety net ("pre_plugged") that catches if the pre_plug handler
was not called - if trying to realize it with a machine that does not
support it.

Otherwise creating and realizing virtio-pmem-pci along with virtio-pmem
would work, however the memory-device part would not properly get
hotplugged.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/virtio/virtio-pmem.c         | 34 +++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio-pmem.h | 11 +++++++++++
 2 files changed, 45 insertions(+)

On Thu, Sep 20, 2018 at 12:32:42PM +0200, David Hildenbrand wrote:
> virtio-pmem devices will have to be hotplugged using the machine hotplug
> handler just like other memory devices. Therefore, all machines that
> want to support virtio-pmem will have to modify their machine hotplug
> handler.
> 
> virtio-pmem devices are realized when their parent proxy device
> (virtio-pmem-pci) is realized. Therefore, they are attached to a bus
> without a hotplug handler. This makes things a lot easier, because
> without a hotplug handler, we can directly get control over the device
> in the machine hotplug handler (otherwise we would have to overwrite
> control and pass control to the bus hotplug handler from the machine
> hotplug handler).
> 
> As we have to implement support for each machine we want to support,
> add a safety net ("pre_plugged") that catches if the pre_plug handler
> was not called - if trying to realize it with a machine that does not
> support it.
> 
> Otherwise creating and realizing virtio-pmem-pci along with virtio-pmem
> would work, however the memory-device part would not properly get
> hotplugged.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>

I'm thinking through how we'd make this work for pseries guests.

Am I right in thinking that the stuff here is purely for getting the
wiring correct on the qemu side.  On the guest side, the hotplug event
for the proxy device should be sufficient to trigger all the necessary
stuff.  Is that right?

If not, pseries presents some problems, because we'd need to invent a
way of notifying hotplug of a completely new type of device which
would be pretty awkward.

> ---
>  hw/virtio/virtio-pmem.c         | 34 +++++++++++++++++++++++++++++++++
>  include/hw/virtio/virtio-pmem.h | 11 +++++++++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/hw/virtio/virtio-pmem.c b/hw/virtio/virtio-pmem.c
> index 0f8b509f0f..0342e4482b 100644
> --- a/hw/virtio/virtio-pmem.c
> +++ b/hw/virtio/virtio-pmem.c
> @@ -112,6 +112,12 @@ static void virtio_pmem_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>  
> +    /* pre_plug handler wasn't executed (e.g. from machine hotplug handler) */
> +    if (!pmem->pre_plugged) {
> +        error_setg(errp, "virtio-pmem is not compatible with the machine");
> +        return;
> +    }
> +
>      host_memory_backend_set_mapped(pmem->memdev, true);
>      virtio_init(vdev, TYPE_VIRTIO_PMEM, VIRTIO_ID_PMEM,
>                                            sizeof(struct virtio_pmem_config));
> @@ -205,6 +211,34 @@ static void virtio_pmem_class_init(ObjectClass *klass, void *data)
>      mdc->get_device_id = virtio_pmem_md_get_device_id;
>  }
>  
> +void virtio_pmem_pre_plug(VirtIOPMEM *pmem, MachineState *ms, Error **errp)
> +{
> +    /*
> +     * The proxy device (e.g. virtio-pmem-pci) has an hotplug handler and
> +     * will attach the virtio-pmem device to its bus (parent_bus). This
> +     * device will realize the virtio-mem device from its realize function,
> +     * therefore when it is hotplugged itself. The proxy device bus
> +     * therefore has no hotplug handler and we don't have to forward any
> +     * calls.
> +     */
> +    if (!DEVICE(pmem)->parent_bus ||
> +        DEVICE(pmem)->parent_bus->hotplug_handler) {
> +        error_setg(errp, "virtio-pmem is not compatible with the proxy.");
> +    }
> +    memory_device_pre_plug(MEMORY_DEVICE(pmem), ms, NULL, errp);
> +    pmem->pre_plugged = true;
> +}
> +
> +void virtio_pmem_plug(VirtIOPMEM *pmem, MachineState *ms, Error **errp)
> +{
> +    memory_device_plug(MEMORY_DEVICE(pmem), ms);
> +}
> +
> +void virtio_pmem_unplug(VirtIOPMEM *pmem, MachineState *ms, Error **errp)
> +{
> +    memory_device_unplug(MEMORY_DEVICE(pmem), ms);
> +}
> +
>  static TypeInfo virtio_pmem_info = {
>      .name          = TYPE_VIRTIO_PMEM,
>      .parent        = TYPE_VIRTIO_DEVICE,
> diff --git a/include/hw/virtio/virtio-pmem.h b/include/hw/virtio/virtio-pmem.h
> index 11e549e0f1..640051f3b6 100644
> --- a/include/hw/virtio/virtio-pmem.h
> +++ b/include/hw/virtio/virtio-pmem.h
> @@ -31,10 +31,21 @@ typedef struct VirtIOPMEM {
>      VirtQueue *rq_vq;
>      uint64_t start;
>      HostMemoryBackend *memdev;
> +
> +    /*
> +     * Safety net to make sure we can catch trying to be realized on a
> +     * machine that is not prepared to properly hotplug virtio-pmem from
> +     * its machine hotplug handler.
> +     */
> +    bool pre_plugged;
>  } VirtIOPMEM;
>  
>  struct virtio_pmem_config {
>      uint64_t start;
>      uint64_t size;
>  };
> +
> +void virtio_pmem_pre_plug(VirtIOPMEM *pmem, MachineState *ms, Error **errp);
> +void virtio_pmem_plug(VirtIOPMEM *pmem, MachineState *ms, Error **errp);
> +void virtio_pmem_unplug(VirtIOPMEM *pmem, MachineState *ms, Error **errp);
>  #endif

On 24/09/2018 08:44, David Gibson wrote:
> On Thu, Sep 20, 2018 at 12:32:42PM +0200, David Hildenbrand wrote:
>> virtio-pmem devices will have to be hotplugged using the machine hotplug
>> handler just like other memory devices. Therefore, all machines that
>> want to support virtio-pmem will have to modify their machine hotplug
>> handler.
>>
>> virtio-pmem devices are realized when their parent proxy device
>> (virtio-pmem-pci) is realized. Therefore, they are attached to a bus
>> without a hotplug handler. This makes things a lot easier, because
>> without a hotplug handler, we can directly get control over the device
>> in the machine hotplug handler (otherwise we would have to overwrite
>> control and pass control to the bus hotplug handler from the machine
>> hotplug handler).
>>
>> As we have to implement support for each machine we want to support,
>> add a safety net ("pre_plugged") that catches if the pre_plug handler
>> was not called - if trying to realize it with a machine that does not
>> support it.
>>
>> Otherwise creating and realizing virtio-pmem-pci along with virtio-pmem
>> would work, however the memory-device part would not properly get
>> hotplugged.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
> 
> I'm thinking through how we'd make this work for pseries guests.
> 
> Am I right in thinking that the stuff here is purely for getting the
> wiring correct on the qemu side.  On the guest side, the hotplug event
> for the proxy device should be sufficient to trigger all the necessary
> stuff.  Is that right?

Yes, this is just wiring for to get the address assigned and the memory
region properly registered in guest physical address space. The "real"
hotplug ("notification") happens via the proxy device. Just like
detection and hotplug of other virtio devices (e.g. virtio-balloon).

> 
> If not, pseries presents some problems, because we'd need to invent a
> way of notifying hotplug of a completely new type of device which
> would be pretty awkward.

Fortunately this is all handled by virtio (the proxy device) already.

Thanks!

On Mon, Sep 24, 2018 at 09:57:35AM +0200, David Hildenbrand wrote:
> On 24/09/2018 08:44, David Gibson wrote:
> > On Thu, Sep 20, 2018 at 12:32:42PM +0200, David Hildenbrand wrote:
> >> virtio-pmem devices will have to be hotplugged using the machine hotplug
> >> handler just like other memory devices. Therefore, all machines that
> >> want to support virtio-pmem will have to modify their machine hotplug
> >> handler.
> >>
> >> virtio-pmem devices are realized when their parent proxy device
> >> (virtio-pmem-pci) is realized. Therefore, they are attached to a bus
> >> without a hotplug handler. This makes things a lot easier, because
> >> without a hotplug handler, we can directly get control over the device
> >> in the machine hotplug handler (otherwise we would have to overwrite
> >> control and pass control to the bus hotplug handler from the machine
> >> hotplug handler).
> >>
> >> As we have to implement support for each machine we want to support,
> >> add a safety net ("pre_plugged") that catches if the pre_plug handler
> >> was not called - if trying to realize it with a machine that does not
> >> support it.
> >>
> >> Otherwise creating and realizing virtio-pmem-pci along with virtio-pmem
> >> would work, however the memory-device part would not properly get
> >> hotplugged.
> >>
> >> Signed-off-by: David Hildenbrand <david@redhat.com>
> > 
> > I'm thinking through how we'd make this work for pseries guests.
> > 
> > Am I right in thinking that the stuff here is purely for getting the
> > wiring correct on the qemu side.  On the guest side, the hotplug event
> > for the proxy device should be sufficient to trigger all the necessary
> > stuff.  Is that right?
> 
> Yes, this is just wiring for to get the address assigned and the memory
> region properly registered in guest physical address space. The "real"
> hotplug ("notification") happens via the proxy device. Just like
> detection and hotplug of other virtio devices (e.g. virtio-balloon).

Ok, that should be easy then.