Re: [RFC PATCH 15/18] vdpa: add vhost_vdpa_load_setup

[Eugenio Perez Martin]

On Thu, Nov 2, 2023 at 9:49 AM Si-Wei Liu <si-wei.liu@oracle.com> wrote:
>
>
>
> On 10/19/2023 7:34 AM, Eugenio Pérez wrote:
> > Callers can use this function to setup the incoming migration.
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> >   include/hw/virtio/vhost-vdpa.h |  7 +++++++
> >   hw/virtio/vhost-vdpa.c         | 17 ++++++++++++++++-
> >   2 files changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
> > index 8f54e5edd4..edc08b7a02 100644
> > --- a/include/hw/virtio/vhost-vdpa.h
> > +++ b/include/hw/virtio/vhost-vdpa.h
> > @@ -45,6 +45,12 @@ typedef struct vhost_vdpa_shared {
> >
> >       bool iotlb_batch_begin_sent;
> >
> > +    /*
> > +     * The memory listener has been registered, so DMA maps have been sent 
> > to
> > +     * the device.
> > +     */
> > +    bool listener_registered;
> > +
> >       /* Vdpa must send shadow addresses as IOTLB key for data queues, not 
> > GPA */
> >       bool shadow_data;
> >   } VhostVDPAShared;
> > @@ -73,6 +79,7 @@ int vhost_vdpa_dma_map(VhostVDPAShared *s, uint32_t asid, 
> > hwaddr iova,
> >                          hwaddr size, void *vaddr, bool readonly);
> >   int vhost_vdpa_dma_unmap(VhostVDPAShared *s, uint32_t asid, hwaddr iova,
> >                            hwaddr size);
> > +int vhost_vdpa_load_setup(VhostVDPAShared *s, AddressSpace *dma_as);
> >
> >   typedef struct vdpa_iommu {
> >       VhostVDPAShared *dev_shared;
> > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > index cc252fc2d8..bfbe4673af 100644
> > --- a/hw/virtio/vhost-vdpa.c
> > +++ b/hw/virtio/vhost-vdpa.c
> > @@ -1325,7 +1325,9 @@ static int vhost_vdpa_dev_start(struct vhost_dev 
> > *dev, bool started)
> >                            "IOMMU and try again");
> >               return -1;
> >           }
> > -        memory_listener_register(&v->shared->listener, dev->vdev->dma_as);
> > +        if (!v->shared->listener_registered) {
> > +            memory_listener_register(&v->shared->listener, 
> > dev->vdev->dma_as);
> > +        }
> Set listener_registered to true after registration; in addition, it
> looks like the memory_listener_unregister in vhost_vdpa_reset_status
> doesn't clear the listener_registered flag after unregistration. This
> code path can be called during SVQ switching, if not doing so mapping
> can't be added back after a couple rounds of SVQ switching or live
> migration.
>

That's right, it should be also set to false in the unregister. Thanks
for the catch!

> -Siwei
>
> >
> >           return vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
> >       }
> > @@ -1528,3 +1530,16 @@ const VhostOps vdpa_ops = {
> >           .vhost_set_config_call = vhost_vdpa_set_config_call,
> >           .vhost_reset_status = vhost_vdpa_reset_status,
> >   };
> > +
> > +int vhost_vdpa_load_setup(VhostVDPAShared *shared, AddressSpace *dma_as)
> > +{
> > +    uint8_t s = VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER;
> > +    int r = ioctl(shared->device_fd, VHOST_VDPA_SET_STATUS, &s);
> > +    if (unlikely(r < 0)) {
> > +        return r;
> > +    }
> > +
> > +    memory_listener_register(&shared->listener, dma_as);
> > +    shared->listener_registered = true;
> > +    return 0;
> > +}
>