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

2023-11-02 Thread Eugenio Perez Martin 
On Thu, Nov 2, 2023 at 9:49 AM Si-Wei Liu  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 
> > ---
> >   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(>shared->listener, dev->vdev->dma_as);
> > +if (!v->shared->listener_registered) {
> > +memory_listener_register(>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, );
> > +if (unlikely(r < 0)) {
> > +return r;
> > +}
> > +
> > +memory_listener_register(>listener, dma_as);
> > +shared->listener_registered = true;
> > +return 0;
> > +}
>

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

2023-11-02 Thread Si-Wei Liu 




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 
---
  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(>shared->listener, dev->vdev->dma_as);
+if (!v->shared->listener_registered) {
+memory_listener_register(>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.


-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, );
+if (unlikely(r < 0)) {
+return r;
+}
+
+memory_listener_register(>listener, dma_as);
+shared->listener_registered = true;
+return 0;
+}

[RFC PATCH 15/18] vdpa: add vhost_vdpa_load_setup

2023-10-19 Thread Eugenio Pérez 
Callers can use this function to setup the incoming migration.

Signed-off-by: Eugenio Pérez 
---
 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(>shared->listener, dev->vdev->dma_as);
+if (!v->shared->listener_registered) {
+memory_listener_register(>shared->listener, dev->vdev->dma_as);
+}
 
 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, );
+if (unlikely(r < 0)) {
+return r;
+}
+
+memory_listener_register(>listener, dma_as);
+shared->listener_registered = true;
+return 0;
+}
-- 
2.39.3