[PATCH v1 0/4] vhost-vdpa: add support for configure interrupt

[PATCH v1 0/4] vhost-vdpa: add support for configure interrupt

[Cindy Lu]

Add configure interrupt support in vdpa_vhost. qemu will use the irqfd
process to support this feature.

These code are all tested in vp-vdpa (support configure interrupt)
vdpa_sim (not support configure interrupt)


Cindy Lu (4):
  virtio:add support in configure interrupt
  virtio-pci:add support for configure interrupt
  vhost_net:enable configure interrupt when vhost_net start
  vhost-vdpa:add callback function for configure interrupt

 hw/net/vhost_net.c                | 19 ++++++-
 hw/virtio/trace-events            |  2 +
 hw/virtio/vhost-vdpa.c            | 29 +++++++++-
 hw/virtio/virtio-pci.c            | 93 +++++++++++++++++++++++++++++++
 hw/virtio/virtio.c                | 25 +++++++++
 include/hw/virtio/vhost-backend.h |  4 ++
 include/hw/virtio/virtio-bus.h    |  2 +
 include/hw/virtio/virtio.h        |  5 ++
 8 files changed, 177 insertions(+), 2 deletions(-)

-- 
2.21.3

[PATCH v1 1/4] virtio:add support in configure interrupt

[Cindy Lu]

Add configure notifier and virtio_set_config_notifier_fd_handler
in virtio

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 hw/virtio/virtio.c         | 25 +++++++++++++++++++++++++
 include/hw/virtio/virtio.h |  5 +++++
 2 files changed, 30 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index ceb58fda6c..66ed1daf95 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3502,6 +3502,15 @@ static void virtio_queue_guest_notifier_read(EventNotifier *n)
     }
 }
 
+static void virtio_queue_config_read(EventNotifier *n)
+{
+    VirtIODevice *vdev = container_of(n, VirtIODevice, config_notifier);
+
+    if (event_notifier_test_and_clear(n)) {
+
+        virtio_notify_config(vdev);
+    }
+}
 void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
                                                 bool with_irqfd)
 {
@@ -3518,6 +3527,17 @@ void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
     }
 }
 
+void virtio_set_config_notifier_fd_handler(VirtIODevice *vdev, bool assign,
+                                                bool with_irqfd)
+{
+    if (assign && !with_irqfd) {
+        event_notifier_set_handler(&vdev->config_notifier,
+                                   virtio_queue_config_read);
+    } else {
+       event_notifier_set_handler(&vdev->config_notifier, NULL);
+    }
+}
+
 EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq)
 {
     return &vq->guest_notifier;
@@ -3591,6 +3611,11 @@ EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq)
     return &vq->host_notifier;
 }
 
+EventNotifier *virtio_queue_get_config_notifier(VirtIODevice *vdev)
+{
+    return &vdev->config_notifier;
+
+}
 void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled)
 {
     vq->host_notifier_enabled = enabled;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index b7ece7a6a8..38bd28242e 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -108,6 +108,7 @@ struct VirtIODevice
     bool use_guest_notifier_mask;
     AddressSpace *dma_as;
     QLIST_HEAD(, VirtQueue) *vector_queues;
+    EventNotifier config_notifier;
 };
 
 struct VirtioDeviceClass {
@@ -310,11 +311,15 @@ uint16_t virtio_get_queue_index(VirtQueue *vq);
 EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq);
 void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
                                                 bool with_irqfd);
+void virtio_set_config_notifier_fd_handler(VirtIODevice *vdev, bool assign,
+                                                bool with_irqfd);
+
 int virtio_device_start_ioeventfd(VirtIODevice *vdev);
 int virtio_device_grab_ioeventfd(VirtIODevice *vdev);
 void virtio_device_release_ioeventfd(VirtIODevice *vdev);
 bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev);
 EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
+EventNotifier *virtio_queue_get_config_notifier(VirtIODevice *vdev);
 void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled);
 void virtio_queue_host_notifier_read(EventNotifier *n);
 void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx,
-- 
2.21.3

[PATCH v1 2/4] virtio-pci:add support for configure interrupt

[Cindy Lu]

Add support for configure interrupt, use kvm_irqfd_assign and set the
gsi to kernel. When the configure notifier was eventfd_signal by host
kernel, this will finally inject an msix interrupt to guest

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 hw/virtio/virtio-pci.c         | 93 ++++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio-bus.h |  2 +
 2 files changed, 95 insertions(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 36524a5728..f8053e1fab 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -762,6 +762,98 @@ undo:
     return ret;
 }
 
+ static int kvm_virtio_pci_config_irqfd_use(VirtIOPCIProxy *proxy,
+                                 unsigned int vector)
+{
+    VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    EventNotifier *n = virtio_queue_get_config_notifier(vdev);
+    assert(irqfd);
+    return kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, n, NULL, irqfd->virq);
+}
+
+static void kvm_virtio_pci_config_irqfd_release(VirtIOPCIProxy *proxy,
+                                      unsigned int vector)
+{
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    EventNotifier *n = virtio_queue_get_config_notifier(vdev);
+    VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
+    assert(irqfd);
+    kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, n, irqfd->virq);
+    return;
+}
+static int kvm_virtio_pci_config_vector_use(VirtIOPCIProxy *proxy,
+                                        unsigned int vector)
+{
+    VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
+    int ret;
+
+    if (irqfd->users == 0) {
+        ret = kvm_irqchip_add_msi_route(kvm_state, vector, &proxy->pci_dev);
+        if (ret < 0) {
+            return ret;
+        }
+        irqfd->virq = ret;
+    }
+    irqfd->users++;
+
+    return 0;
+}
+static int kvm_virtio_pci_vector_config_use(VirtIOPCIProxy *proxy)
+{
+
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    unsigned int vector;
+    int ret;
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+
+    vector = vdev->config_vector ;
+    ret = kvm_virtio_pci_config_vector_use(proxy, vector);
+    if (ret < 0) {
+        goto undo;
+    }
+    ret = kvm_virtio_pci_config_irqfd_use(proxy,  vector);
+    if (ret < 0) {
+        goto undo;
+    }
+    return 0;
+undo:
+    kvm_virtio_pci_config_irqfd_release(proxy, vector);
+    return ret;
+}
+static void kvm_virtio_pci_vector_config_release(VirtIOPCIProxy *proxy)
+{
+    PCIDevice *dev = &proxy->pci_dev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    unsigned int vector;
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+    vector = vdev->config_vector ;
+    if (vector >= msix_nr_vectors_allocated(dev)) {
+        return;
+    }
+    kvm_virtio_pci_config_irqfd_release(proxy, vector);
+    kvm_virtio_pci_vq_vector_release(proxy, vector);
+}
+
+static int virtio_pci_set_guest_config_notifier(DeviceState *d,  bool assign,
+                                         bool with_irqfd)
+{
+    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    EventNotifier *notifier = virtio_queue_get_config_notifier(vdev);
+    int r = 0;
+    if (assign) {
+        r = event_notifier_init(notifier, 1);
+        virtio_set_config_notifier_fd_handler(vdev, true, with_irqfd);
+        kvm_virtio_pci_vector_config_use(proxy);
+    } else {
+        virtio_set_config_notifier_fd_handler(vdev, false, with_irqfd);
+        kvm_virtio_pci_vector_config_release(proxy);
+        event_notifier_cleanup(notifier);
+    }
+    return r;
+}
+
 static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
 {
     PCIDevice *dev = &proxy->pci_dev;
@@ -2137,6 +2229,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
     k->ioeventfd_assign = virtio_pci_ioeventfd_assign;
     k->get_dma_as = virtio_pci_get_dma_as;
     k->queue_enabled = virtio_pci_queue_enabled;
+    k->set_config_notifiers = virtio_pci_set_guest_config_notifier;
 }
 
 static const TypeInfo virtio_pci_bus_info = {
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index ef8abe49c5..dae81ee414 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -93,6 +93,8 @@ struct VirtioBusClass {
      */
     bool has_variable_vring_alignment;
     AddressSpace *(*get_dma_as)(DeviceState *d);
+    int (*set_config_notifiers)(DeviceState *d, bool assign, bool with_irqfd);
+
 };
 
 struct VirtioBusState {
-- 
2.21.3

[PATCH v1 3/4] vhost_net:enable configure interrupt when vhost_net start

[Cindy Lu]

When peer is vhost vdpa, setup the configure interrupt function
vhost_net_start and release the resource when vhost_net_stop

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 hw/net/vhost_net.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 24d555e764..fddc1f51f5 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -345,6 +345,15 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
         error_report("Error binding guest notifier: %d", -r);
         goto err;
     }
+    if (ncs->peer && ncs->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
+        if (k->set_config_notifiers) {
+            r = k->set_config_notifiers(qbus->parent, true, true);
+            if (r < 0) {
+                error_report("Error binding config notifier: %d", -r);
+                goto err;
+            }
+       }
+    }
 
     for (i = 0; i < total_queues; i++) {
         peer = qemu_get_peer(ncs, i);
@@ -391,7 +400,15 @@ void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs,
     for (i = 0; i < total_queues; i++) {
         vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev);
     }
-
+   if (ncs->peer && ncs->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
+        if (k->set_config_notifiers) {
+            r = k->set_config_notifiers(qbus->parent, false, false);
+            if (r < 0) {
+                error_report("Error unbinding config notifier: %d", -r);
+            }
+           assert(r >= 0);
+        }
+    }
     r = k->set_guest_notifiers(qbus->parent, total_queues * 2, false);
     if (r < 0) {
         fprintf(stderr, "vhost guest notifier cleanup failed: %d\n", r);
-- 
2.21.3

[PATCH v1 4/4] vhost-vdpa:add callback function for configure interrupt

[Cindy Lu]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=gbk, Size: 4534 bytes --]

Add call back function for configure interrupt.
Set the notifier's fd to the kernel driver when vdpa start.
also set -1 when vdpa stop. then the kernel will release
the related聽resource聽

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 hw/virtio/trace-events            |  2 ++
 hw/virtio/vhost-vdpa.c            | 29 ++++++++++++++++++++++++++++-
 include/hw/virtio/vhost-backend.h |  4 ++++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 2060a144a2..6710835b46 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -52,6 +52,8 @@ vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd) "dev: %p index:
 vhost_vdpa_get_features(void *dev, uint64_t features) "dev: %p features: 0x%"PRIx64
 vhost_vdpa_set_owner(void *dev) "dev: %p"
 vhost_vdpa_vq_get_addr(void *dev, void *vq, uint64_t desc_user_addr, uint64_t avail_user_addr, uint64_t used_user_addr) "dev: %p vq: %p desc_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64
+vhost_vdpa_set_config_call(void *dev, int *fd)"dev: %p fd: %p"
+
 
 # virtio.c
 virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 01d2101d09..1647bff8b0 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -467,20 +467,39 @@ static int vhost_vdpa_get_config(struct vhost_dev *dev, uint8_t *config,
     }
     return ret;
  }
+static void vhost_vdpa_config_notify_start(struct vhost_dev *dev,
+                                struct VirtIODevice *vdev, bool start)
+{
+    int fd, r;
+    if (start) {
+        fd = event_notifier_get_fd(&vdev->config_notifier);
+     } else {
+        fd = -1;
+     }
+    r = dev->vhost_ops->vhost_set_config_call(dev, &fd);
+    if (r) {
+        error_report("vhost_vdpa_config_notify_start error!");
+    }
+    return;
 
+}
 static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
 {
     struct vhost_vdpa *v = dev->opaque;
     trace_vhost_vdpa_dev_start(dev, started);
+    VirtIODevice *vdev = dev->vdev;
+
     if (started) {
         uint8_t status = 0;
         memory_listener_register(&v->listener, &address_space_memory);
         vhost_vdpa_set_vring_ready(dev);
         vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
         vhost_vdpa_call(dev, VHOST_VDPA_GET_STATUS, &status);
-
+        /*set the configure interrupt call back*/
+        vhost_vdpa_config_notify_start(dev, vdev, true);
         return !(status & VIRTIO_CONFIG_S_DRIVER_OK);
     } else {
+        vhost_vdpa_config_notify_start(dev, vdev, false);
         vhost_vdpa_reset_device(dev);
         vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
                                    VIRTIO_CONFIG_S_DRIVER);
@@ -546,6 +565,13 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
     return vhost_vdpa_call(dev, VHOST_SET_VRING_CALL, file);
 }
 
+static int vhost_vdpa_set_config_call(struct vhost_dev *dev,
+                                       int *fd)
+{
+    trace_vhost_vdpa_set_config_call(dev, fd);
+    return vhost_vdpa_call(dev, VHOST_VDPA_SET_CONFIG_CALL, fd);
+}
+
 static int vhost_vdpa_get_features(struct vhost_dev *dev,
                                      uint64_t *features)
 {
@@ -611,4 +637,5 @@ const VhostOps vdpa_ops = {
         .vhost_get_device_id = vhost_vdpa_get_device_id,
         .vhost_vq_get_addr = vhost_vdpa_vq_get_addr,
         .vhost_force_iommu = vhost_vdpa_force_iommu,
+        .vhost_set_config_call = vhost_vdpa_set_config_call,
 };
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index 8a6f8e2a7a..1a2fee8994 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -125,6 +125,9 @@ typedef int (*vhost_get_device_id_op)(struct vhost_dev *dev, uint32_t *dev_id);
 
 typedef bool (*vhost_force_iommu_op)(struct vhost_dev *dev);
 
+typedef int (*vhost_set_config_call_op)(struct vhost_dev *dev,
+                                       int *fd);
+
 typedef struct VhostOps {
     VhostBackendType backend_type;
     vhost_backend_init vhost_backend_init;
@@ -170,6 +173,7 @@ typedef struct VhostOps {
     vhost_vq_get_addr_op  vhost_vq_get_addr;
     vhost_get_device_id_op vhost_get_device_id;
     vhost_force_iommu_op vhost_force_iommu;
+    vhost_set_config_call_op vhost_set_config_call;
 } VhostOps;
 
 extern const VhostOps user_ops;
-- 
2.21.3

Re: [PATCH v1 4/4] vhost-vdpa:add callback function for configure interrupt

[Michael S. Tsirkin]

On Wed, Jan 13, 2021 at 11:45:40PM +0800, Cindy Lu wrote:
> Add call back function for configure interrupt.
> Set the notifier's fd to the kernel driver when vdpa start.
> also set -1 when vdpa stop. then the kernel will release
> the related聽resource聽

Something weird happened to the commit log here.

> 
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>  hw/virtio/trace-events            |  2 ++
>  hw/virtio/vhost-vdpa.c            | 29 ++++++++++++++++++++++++++++-
>  include/hw/virtio/vhost-backend.h |  4 ++++
>  3 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 2060a144a2..6710835b46 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -52,6 +52,8 @@ vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd) "dev: %p index:
>  vhost_vdpa_get_features(void *dev, uint64_t features) "dev: %p features: 0x%"PRIx64
>  vhost_vdpa_set_owner(void *dev) "dev: %p"
>  vhost_vdpa_vq_get_addr(void *dev, void *vq, uint64_t desc_user_addr, uint64_t avail_user_addr, uint64_t used_user_addr) "dev: %p vq: %p desc_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64
> +vhost_vdpa_set_config_call(void *dev, int *fd)"dev: %p fd: %p"
> +
>  
>  # virtio.c
>  virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 01d2101d09..1647bff8b0 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -467,20 +467,39 @@ static int vhost_vdpa_get_config(struct vhost_dev *dev, uint8_t *config,
>      }
>      return ret;
>   }
> +static void vhost_vdpa_config_notify_start(struct vhost_dev *dev,
> +                                struct VirtIODevice *vdev, bool start)
> +{
> +    int fd, r;
> +    if (start) {
> +        fd = event_notifier_get_fd(&vdev->config_notifier);
> +     } else {
> +        fd = -1;
> +     }
> +    r = dev->vhost_ops->vhost_set_config_call(dev, &fd);
> +    if (r) {
> +        error_report("vhost_vdpa_config_notify_start error!");
> +    }
> +    return;
>  
> +}
>  static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
>  {
>      struct vhost_vdpa *v = dev->opaque;
>      trace_vhost_vdpa_dev_start(dev, started);
> +    VirtIODevice *vdev = dev->vdev;
> +
>      if (started) {
>          uint8_t status = 0;
>          memory_listener_register(&v->listener, &address_space_memory);
>          vhost_vdpa_set_vring_ready(dev);
>          vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
>          vhost_vdpa_call(dev, VHOST_VDPA_GET_STATUS, &status);
> -
> +        /*set the configure interrupt call back*/
> +        vhost_vdpa_config_notify_start(dev, vdev, true);
>          return !(status & VIRTIO_CONFIG_S_DRIVER_OK);
>      } else {
> +        vhost_vdpa_config_notify_start(dev, vdev, false);
>          vhost_vdpa_reset_device(dev);
>          vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
>                                     VIRTIO_CONFIG_S_DRIVER);
> @@ -546,6 +565,13 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
>      return vhost_vdpa_call(dev, VHOST_SET_VRING_CALL, file);
>  }
>  
> +static int vhost_vdpa_set_config_call(struct vhost_dev *dev,
> +                                       int *fd)
> +{
> +    trace_vhost_vdpa_set_config_call(dev, fd);
> +    return vhost_vdpa_call(dev, VHOST_VDPA_SET_CONFIG_CALL, fd);
> +}
> +
>  static int vhost_vdpa_get_features(struct vhost_dev *dev,
>                                       uint64_t *features)
>  {
> @@ -611,4 +637,5 @@ const VhostOps vdpa_ops = {
>          .vhost_get_device_id = vhost_vdpa_get_device_id,
>          .vhost_vq_get_addr = vhost_vdpa_vq_get_addr,
>          .vhost_force_iommu = vhost_vdpa_force_iommu,
> +        .vhost_set_config_call = vhost_vdpa_set_config_call,
>  };
> diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
> index 8a6f8e2a7a..1a2fee8994 100644
> --- a/include/hw/virtio/vhost-backend.h
> +++ b/include/hw/virtio/vhost-backend.h
> @@ -125,6 +125,9 @@ typedef int (*vhost_get_device_id_op)(struct vhost_dev *dev, uint32_t *dev_id);
>  
>  typedef bool (*vhost_force_iommu_op)(struct vhost_dev *dev);
>  
> +typedef int (*vhost_set_config_call_op)(struct vhost_dev *dev,
> +                                       int *fd);
> +
>  typedef struct VhostOps {
>      VhostBackendType backend_type;
>      vhost_backend_init vhost_backend_init;
> @@ -170,6 +173,7 @@ typedef struct VhostOps {
>      vhost_vq_get_addr_op  vhost_vq_get_addr;
>      vhost_get_device_id_op vhost_get_device_id;
>      vhost_force_iommu_op vhost_force_iommu;
> +    vhost_set_config_call_op vhost_set_config_call;
>  } VhostOps;
>  
>  extern const VhostOps user_ops;
> -- 
> 2.21.3

Re: [PATCH v1 1/4] virtio:add support in configure interrupt

[Jason Wang]

On 2021/1/13 下午11:45, Cindy Lu wrote:
> Add configure notifier and virtio_set_config_notifier_fd_handler
> in virtio
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>   hw/virtio/virtio.c         | 25 +++++++++++++++++++++++++
>   include/hw/virtio/virtio.h |  5 +++++
>   2 files changed, 30 insertions(+)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index ceb58fda6c..66ed1daf95 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3502,6 +3502,15 @@ static void virtio_queue_guest_notifier_read(EventNotifier *n)
>       }
>   }
>   
> +static void virtio_queue_config_read(EventNotifier *n)
> +{


Note that the config interrupt belongs to the device. So it's better not 
name it as "queue" here.


> +    VirtIODevice *vdev = container_of(n, VirtIODevice, config_notifier);
> +
> +    if (event_notifier_test_and_clear(n)) {
> +
> +        virtio_notify_config(vdev);
> +    }
> +}
>   void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
>                                                   bool with_irqfd)
>   {
> @@ -3518,6 +3527,17 @@ void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
>       }
>   }
>   
> +void virtio_set_config_notifier_fd_handler(VirtIODevice *vdev, bool assign,
> +                                                bool with_irqfd)
> +{
> +    if (assign && !with_irqfd) {
> +        event_notifier_set_handler(&vdev->config_notifier,
> +                                   virtio_queue_config_read);
> +    } else {
> +       event_notifier_set_handler(&vdev->config_notifier, NULL);
> +    }
> +}


I wonder whether we can simply generalize 
virtio_queue_set_guest_notifier_fd_handler from

void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
                                                 bool with_irqfd)

to

void virtio_set_guest_notifier_fd_handler(EventNotifier *e, bool assign,
                                                 bool with_irqfd)


Since there's actually no virtqueue specific setup in this function, 
what its callee really want is a simple EventNotifier.

Thanks


> +
>   EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq)
>   {
>       return &vq->guest_notifier;
> @@ -3591,6 +3611,11 @@ EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq)
>       return &vq->host_notifier;
>   }
>   
> +EventNotifier *virtio_queue_get_config_notifier(VirtIODevice *vdev)
> +{
> +    return &vdev->config_notifier;
> +
> +}
>   void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled)
>   {
>       vq->host_notifier_enabled = enabled;
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index b7ece7a6a8..38bd28242e 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -108,6 +108,7 @@ struct VirtIODevice
>       bool use_guest_notifier_mask;
>       AddressSpace *dma_as;
>       QLIST_HEAD(, VirtQueue) *vector_queues;
> +    EventNotifier config_notifier;
>   };
>   
>   struct VirtioDeviceClass {
> @@ -310,11 +311,15 @@ uint16_t virtio_get_queue_index(VirtQueue *vq);
>   EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq);
>   void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
>                                                   bool with_irqfd);
> +void virtio_set_config_notifier_fd_handler(VirtIODevice *vdev, bool assign,
> +                                                bool with_irqfd);
> +
>   int virtio_device_start_ioeventfd(VirtIODevice *vdev);
>   int virtio_device_grab_ioeventfd(VirtIODevice *vdev);
>   void virtio_device_release_ioeventfd(VirtIODevice *vdev);
>   bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev);
>   EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
> +EventNotifier *virtio_queue_get_config_notifier(VirtIODevice *vdev);
>   void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled);
>   void virtio_queue_host_notifier_read(EventNotifier *n);
>   void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx,

Re: [PATCH v1 0/4] vhost-vdpa: add support for configure interrupt

[Jason Wang]

On 2021/1/13 下午11:45, Cindy Lu wrote:
> Add configure interrupt support in vdpa_vhost. qemu will use the irqfd
> process to support this feature.
>
> These code are all tested in vp-vdpa (support configure interrupt)
> vdpa_sim (not support configure interrupt)
>
>
> Cindy Lu (4):
>    virtio:add support in configure interrupt
>    virtio-pci:add support for configure interrupt


If possible, I would like to add the MMIO support for this.

One great advantage of vDPA is that it can avoid to expose a PCI device 
for guest.

Thanks


>    vhost_net:enable configure interrupt when vhost_net start
>    vhost-vdpa:add callback function for configure interrupt
>
>   hw/net/vhost_net.c                | 19 ++++++-
>   hw/virtio/trace-events            |  2 +
>   hw/virtio/vhost-vdpa.c            | 29 +++++++++-
>   hw/virtio/virtio-pci.c            | 93 +++++++++++++++++++++++++++++++
>   hw/virtio/virtio.c                | 25 +++++++++
>   include/hw/virtio/vhost-backend.h |  4 ++
>   include/hw/virtio/virtio-bus.h    |  2 +
>   include/hw/virtio/virtio.h        |  5 ++
>   8 files changed, 177 insertions(+), 2 deletions(-)
>

Re: [PATCH v1 2/4] virtio-pci:add support for configure interrupt

[Jason Wang]

On 2021/1/13 下午11:45, Cindy Lu wrote:
> Add support for configure interrupt, use kvm_irqfd_assign and set the
> gsi to kernel. When the configure notifier was eventfd_signal by host
> kernel, this will finally inject an msix interrupt to guest
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>   hw/virtio/virtio-pci.c         | 93 ++++++++++++++++++++++++++++++++++
>   include/hw/virtio/virtio-bus.h |  2 +
>   2 files changed, 95 insertions(+)
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 36524a5728..f8053e1fab 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -762,6 +762,98 @@ undo:
>       return ret;
>   }
>   
> + static int kvm_virtio_pci_config_irqfd_use(VirtIOPCIProxy *proxy,
> +                                 unsigned int vector)
> +{
> +    VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
> +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> +    EventNotifier *n = virtio_queue_get_config_notifier(vdev);
> +    assert(irqfd);
> +    return kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, n, NULL, irqfd->virq);
> +}
> +
> +static void kvm_virtio_pci_config_irqfd_release(VirtIOPCIProxy *proxy,
> +                                      unsigned int vector)
> +{
> +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> +    EventNotifier *n = virtio_queue_get_config_notifier(vdev);
> +    VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
> +    assert(irqfd);
> +    kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, n, irqfd->virq);
> +    return;
> +}
> +static int kvm_virtio_pci_config_vector_use(VirtIOPCIProxy *proxy,
> +                                        unsigned int vector)
> +{
> +    VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
> +    int ret;
> +
> +    if (irqfd->users == 0) {
> +        ret = kvm_irqchip_add_msi_route(kvm_state, vector, &proxy->pci_dev);
> +        if (ret < 0) {
> +            return ret;
> +        }
> +        irqfd->virq = ret;
> +    }
> +    irqfd->users++;
> +
> +    return 0;
> +}


Any reason that we can't resue kvm_virtio_pci_vq_vector_use()? Note that 
at MSIX level there's no difference if it belongs to vq or config.


> +static int kvm_virtio_pci_vector_config_use(VirtIOPCIProxy *proxy)
> +{
> +
> +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> +    unsigned int vector;
> +    int ret;
> +    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> +
> +    vector = vdev->config_vector ;
> +    ret = kvm_virtio_pci_config_vector_use(proxy, vector);
> +    if (ret < 0) {
> +        goto undo;
> +    }
> +    ret = kvm_virtio_pci_config_irqfd_use(proxy,  vector);
> +    if (ret < 0) {
> +        goto undo;
> +    }
> +    return 0;
> +undo:
> +    kvm_virtio_pci_config_irqfd_release(proxy, vector);
> +    return ret;
> +}
> +static void kvm_virtio_pci_vector_config_release(VirtIOPCIProxy *proxy)
> +{
> +    PCIDevice *dev = &proxy->pci_dev;
> +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> +    unsigned int vector;
> +    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> +    vector = vdev->config_vector ;
> +    if (vector >= msix_nr_vectors_allocated(dev)) {
> +        return;
> +    }
> +    kvm_virtio_pci_config_irqfd_release(proxy, vector);
> +    kvm_virtio_pci_vq_vector_release(proxy, vector);
> +}
> +
> +static int virtio_pci_set_guest_config_notifier(DeviceState *d,  bool assign,
> +                                         bool with_irqfd)
> +{
> +    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
> +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> +    EventNotifier *notifier = virtio_queue_get_config_notifier(vdev);
> +    int r = 0;
> +    if (assign) {
> +        r = event_notifier_init(notifier, 1);
> +        virtio_set_config_notifier_fd_handler(vdev, true, with_irqfd);
> +        kvm_virtio_pci_vector_config_use(proxy);
> +    } else {
> +        virtio_set_config_notifier_fd_handler(vdev, false, with_irqfd);
> +        kvm_virtio_pci_vector_config_release(proxy);
> +        event_notifier_cleanup(notifier);
> +    }
> +    return r;
> +}


Any way to re-use virtio_pci_set_guest_notifier() here?

Thanks


> +
>   static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
>   {
>       PCIDevice *dev = &proxy->pci_dev;
> @@ -2137,6 +2229,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
>       k->ioeventfd_assign = virtio_pci_ioeventfd_assign;
>       k->get_dma_as = virtio_pci_get_dma_as;
>       k->queue_enabled = virtio_pci_queue_enabled;
> +    k->set_config_notifiers = virtio_pci_set_guest_config_notifier;
>   }
>   
>   static const TypeInfo virtio_pci_bus_info = {
> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> index ef8abe49c5..dae81ee414 100644
> --- a/include/hw/virtio/virtio-bus.h
> +++ b/include/hw/virtio/virtio-bus.h
> @@ -93,6 +93,8 @@ struct VirtioBusClass {
>        */
>       bool has_variable_vring_alignment;
>       AddressSpace *(*get_dma_as)(DeviceState *d);
> +    int (*set_config_notifiers)(DeviceState *d, bool assign, bool with_irqfd);
> +
>   };
>   
>   struct VirtioBusState {

Re: [PATCH v1 4/4] vhost-vdpa:add callback function for configure interrupt

[Jason Wang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=gbk; format=flowed, Size: 4977 bytes --]


On 2021/1/13 下午11:45, Cindy Lu wrote:
> Add call back function for configure interrupt.
> Set the notifier's fd to the kernel driver when vdpa start.
> also set -1 when vdpa stop. then the kernel will release
> the related聽resource聽
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>   hw/virtio/trace-events            |  2 ++
>   hw/virtio/vhost-vdpa.c            | 29 ++++++++++++++++++++++++++++-
>   include/hw/virtio/vhost-backend.h |  4 ++++
>   3 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 2060a144a2..6710835b46 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -52,6 +52,8 @@ vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd) "dev: %p index:
>   vhost_vdpa_get_features(void *dev, uint64_t features) "dev: %p features: 0x%"PRIx64
>   vhost_vdpa_set_owner(void *dev) "dev: %p"
>   vhost_vdpa_vq_get_addr(void *dev, void *vq, uint64_t desc_user_addr, uint64_t avail_user_addr, uint64_t used_user_addr) "dev: %p vq: %p desc_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64
> +vhost_vdpa_set_config_call(void *dev, int *fd)"dev: %p fd: %p"
> +
>   
>   # virtio.c
>   virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 01d2101d09..1647bff8b0 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -467,20 +467,39 @@ static int vhost_vdpa_get_config(struct vhost_dev *dev, uint8_t *config,
>       }
>       return ret;
>    }
> +static void vhost_vdpa_config_notify_start(struct vhost_dev *dev,
> +                                struct VirtIODevice *vdev, bool start)
> +{
> +    int fd, r;
> +    if (start) {
> +        fd = event_notifier_get_fd(&vdev->config_notifier);
> +     } else {
> +        fd = -1;
> +     }
> +    r = dev->vhost_ops->vhost_set_config_call(dev, &fd);
> +    if (r) {
> +        error_report("vhost_vdpa_config_notify_start error!");
> +    }


So on early version of kernel without config interrupt support. This 
will fail for sure. We need seek a way to make it work.

Thanks


> +    return;
>   
> +}
>   static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
>   {
>       struct vhost_vdpa *v = dev->opaque;
>       trace_vhost_vdpa_dev_start(dev, started);
> +    VirtIODevice *vdev = dev->vdev;
> +
>       if (started) {
>           uint8_t status = 0;
>           memory_listener_register(&v->listener, &address_space_memory);
>           vhost_vdpa_set_vring_ready(dev);
>           vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
>           vhost_vdpa_call(dev, VHOST_VDPA_GET_STATUS, &status);
> -
> +        /*set the configure interrupt call back*/
> +        vhost_vdpa_config_notify_start(dev, vdev, true);
>           return !(status & VIRTIO_CONFIG_S_DRIVER_OK);
>       } else {
> +        vhost_vdpa_config_notify_start(dev, vdev, false);
>           vhost_vdpa_reset_device(dev);
>           vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
>                                      VIRTIO_CONFIG_S_DRIVER);
> @@ -546,6 +565,13 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
>       return vhost_vdpa_call(dev, VHOST_SET_VRING_CALL, file);
>   }
>   
> +static int vhost_vdpa_set_config_call(struct vhost_dev *dev,
> +                                       int *fd)
> +{
> +    trace_vhost_vdpa_set_config_call(dev, fd);
> +    return vhost_vdpa_call(dev, VHOST_VDPA_SET_CONFIG_CALL, fd);
> +}
> +
>   static int vhost_vdpa_get_features(struct vhost_dev *dev,
>                                        uint64_t *features)
>   {
> @@ -611,4 +637,5 @@ const VhostOps vdpa_ops = {
>           .vhost_get_device_id = vhost_vdpa_get_device_id,
>           .vhost_vq_get_addr = vhost_vdpa_vq_get_addr,
>           .vhost_force_iommu = vhost_vdpa_force_iommu,
> +        .vhost_set_config_call = vhost_vdpa_set_config_call,
>   };
> diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
> index 8a6f8e2a7a..1a2fee8994 100644
> --- a/include/hw/virtio/vhost-backend.h
> +++ b/include/hw/virtio/vhost-backend.h
> @@ -125,6 +125,9 @@ typedef int (*vhost_get_device_id_op)(struct vhost_dev *dev, uint32_t *dev_id);
>   
>   typedef bool (*vhost_force_iommu_op)(struct vhost_dev *dev);
>   
> +typedef int (*vhost_set_config_call_op)(struct vhost_dev *dev,
> +                                       int *fd);
> +
>   typedef struct VhostOps {
>       VhostBackendType backend_type;
>       vhost_backend_init vhost_backend_init;
> @@ -170,6 +173,7 @@ typedef struct VhostOps {
>       vhost_vq_get_addr_op  vhost_vq_get_addr;
>       vhost_get_device_id_op vhost_get_device_id;
>       vhost_force_iommu_op vhost_force_iommu;
> +    vhost_set_config_call_op vhost_set_config_call;
>   } VhostOps;
>   
>   extern const VhostOps user_ops;

Re: [PATCH v1 1/4] virtio:add support in configure interrupt

[Cindy Lu]

On Thu, Jan 14, 2021 at 12:34 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2021/1/13 下午11:45, Cindy Lu wrote:
> > Add configure notifier and virtio_set_config_notifier_fd_handler
> > in virtio
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> >   hw/virtio/virtio.c         | 25 +++++++++++++++++++++++++
> >   include/hw/virtio/virtio.h |  5 +++++
> >   2 files changed, 30 insertions(+)
> >
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index ceb58fda6c..66ed1daf95 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -3502,6 +3502,15 @@ static void virtio_queue_guest_notifier_read(EventNotifier *n)
> >       }
> >   }
> >
> > +static void virtio_queue_config_read(EventNotifier *n)
> > +{
>
>
> Note that the config interrupt belongs to the device. So it's better not
> name it as "queue" here.
>
sure I will fix this
>
> > +    VirtIODevice *vdev = container_of(n, VirtIODevice, config_notifier);
> > +
> > +    if (event_notifier_test_and_clear(n)) {
> > +
> > +        virtio_notify_config(vdev);
> > +    }
> > +}
> >   void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
> >                                                   bool with_irqfd)
> >   {
> > @@ -3518,6 +3527,17 @@ void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
> >       }
> >   }
> >
> > +void virtio_set_config_notifier_fd_handler(VirtIODevice *vdev, bool assign,
> > +                                                bool with_irqfd)
> > +{
> > +    if (assign && !with_irqfd) {
> > +        event_notifier_set_handler(&vdev->config_notifier,
> > +                                   virtio_queue_config_read);
> > +    } else {
> > +       event_notifier_set_handler(&vdev->config_notifier, NULL);
> > +    }
> > +}
>
>
> I wonder whether we can simply generalize
> virtio_queue_set_guest_notifier_fd_handler from
>
> void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
>                                                  bool with_irqfd)
>
> to
>
> void virtio_set_guest_notifier_fd_handler(EventNotifier *e, bool assign,
>                                                  bool with_irqfd)
>
>
> Since there's actually no virtqueue specific setup in this function,
> what its callee really want is a simple EventNotifier.
>
> Thanks
>
Thanks Jason， I will fix ths
>
> > +
> >   EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq)
> >   {
> >       return &vq->guest_notifier;
> > @@ -3591,6 +3611,11 @@ EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq)
> >       return &vq->host_notifier;
> >   }
> >
> > +EventNotifier *virtio_queue_get_config_notifier(VirtIODevice *vdev)
> > +{
> > +    return &vdev->config_notifier;
> > +
> > +}
> >   void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled)
> >   {
> >       vq->host_notifier_enabled = enabled;
> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > index b7ece7a6a8..38bd28242e 100644
> > --- a/include/hw/virtio/virtio.h
> > +++ b/include/hw/virtio/virtio.h
> > @@ -108,6 +108,7 @@ struct VirtIODevice
> >       bool use_guest_notifier_mask;
> >       AddressSpace *dma_as;
> >       QLIST_HEAD(, VirtQueue) *vector_queues;
> > +    EventNotifier config_notifier;
> >   };
> >
> >   struct VirtioDeviceClass {
> > @@ -310,11 +311,15 @@ uint16_t virtio_get_queue_index(VirtQueue *vq);
> >   EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq);
> >   void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
> >                                                   bool with_irqfd);
> > +void virtio_set_config_notifier_fd_handler(VirtIODevice *vdev, bool assign,
> > +                                                bool with_irqfd);
> > +
> >   int virtio_device_start_ioeventfd(VirtIODevice *vdev);
> >   int virtio_device_grab_ioeventfd(VirtIODevice *vdev);
> >   void virtio_device_release_ioeventfd(VirtIODevice *vdev);
> >   bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev);
> >   EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
> > +EventNotifier *virtio_queue_get_config_notifier(VirtIODevice *vdev);
> >   void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled);
> >   void virtio_queue_host_notifier_read(EventNotifier *n);
> >   void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx,
>

Re: [PATCH v1 2/4] virtio-pci:add support for configure interrupt

[Cindy Lu]

On Thu, Jan 14, 2021 at 12:43 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2021/1/13 下午11:45, Cindy Lu wrote:
> > Add support for configure interrupt, use kvm_irqfd_assign and set the
> > gsi to kernel. When the configure notifier was eventfd_signal by host
> > kernel, this will finally inject an msix interrupt to guest
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> >   hw/virtio/virtio-pci.c         | 93 ++++++++++++++++++++++++++++++++++
> >   include/hw/virtio/virtio-bus.h |  2 +
> >   2 files changed, 95 insertions(+)
> >
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index 36524a5728..f8053e1fab 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -762,6 +762,98 @@ undo:
> >       return ret;
> >   }
> >
> > + static int kvm_virtio_pci_config_irqfd_use(VirtIOPCIProxy *proxy,
> > +                                 unsigned int vector)
> > +{
> > +    VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
> > +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> > +    EventNotifier *n = virtio_queue_get_config_notifier(vdev);
> > +    assert(irqfd);
> > +    return kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, n, NULL, irqfd->virq);
> > +}
> > +
> > +static void kvm_virtio_pci_config_irqfd_release(VirtIOPCIProxy *proxy,
> > +                                      unsigned int vector)
> > +{
> > +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> > +    EventNotifier *n = virtio_queue_get_config_notifier(vdev);
> > +    VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
> > +    assert(irqfd);
> > +    kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, n, irqfd->virq);
> > +    return;
> > +}
> > +static int kvm_virtio_pci_config_vector_use(VirtIOPCIProxy *proxy,
> > +                                        unsigned int vector)
> > +{
> > +    VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
> > +    int ret;
> > +
> > +    if (irqfd->users == 0) {
> > +        ret = kvm_irqchip_add_msi_route(kvm_state, vector, &proxy->pci_dev);
> > +        if (ret < 0) {
> > +            return ret;
> > +        }
> > +        irqfd->virq = ret;
> > +    }
> > +    irqfd->users++;
> > +
> > +    return 0;
> > +}
>
>
> Any reason that we can't resue kvm_virtio_pci_vq_vector_use()? Note that
> at MSIX level there's no difference if it belongs to vq or config.
>
>
> > +static int kvm_virtio_pci_vector_config_use(VirtIOPCIProxy *proxy)
> > +{
> > +
> > +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> > +    unsigned int vector;
> > +    int ret;
> > +    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> > +
> > +    vector = vdev->config_vector ;
> > +    ret = kvm_virtio_pci_config_vector_use(proxy, vector);
> > +    if (ret < 0) {
> > +        goto undo;
> > +    }
> > +    ret = kvm_virtio_pci_config_irqfd_use(proxy,  vector);
> > +    if (ret < 0) {
> > +        goto undo;
> > +    }
> > +    return 0;
> > +undo:
> > +    kvm_virtio_pci_config_irqfd_release(proxy, vector);
> > +    return ret;
> > +}
> > +static void kvm_virtio_pci_vector_config_release(VirtIOPCIProxy *proxy)
> > +{
> > +    PCIDevice *dev = &proxy->pci_dev;
> > +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> > +    unsigned int vector;
> > +    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> > +    vector = vdev->config_vector ;
> > +    if (vector >= msix_nr_vectors_allocated(dev)) {
> > +        return;
> > +    }
> > +    kvm_virtio_pci_config_irqfd_release(proxy, vector);
> > +    kvm_virtio_pci_vq_vector_release(proxy, vector);
> > +}
> > +
> > +static int virtio_pci_set_guest_config_notifier(DeviceState *d,  bool assign,
> > +                                         bool with_irqfd)
> > +{
> > +    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
> > +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> > +    EventNotifier *notifier = virtio_queue_get_config_notifier(vdev);
> > +    int r = 0;
> > +    if (assign) {
> > +        r = event_notifier_init(notifier, 1);
> > +        virtio_set_config_notifier_fd_handler(vdev, true, with_irqfd);
> > +        kvm_virtio_pci_vector_config_use(proxy);
> > +    } else {
> > +        virtio_set_config_notifier_fd_handler(vdev, false, with_irqfd);
> > +        kvm_virtio_pci_vector_config_release(proxy);
> > +        event_notifier_cleanup(notifier);
> > +    }
> > +    return r;
> > +}
>
>
> Any way to re-use virtio_pci_set_guest_notifier() here?
>
> Thanks
>
These  two functions you mentioned all have something needed such as
queue no/guest_notifier. so I created a new one.
but it's a good idea to re-organize it,  I will post a new version
>
> > +
> >   static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
> >   {
> >       PCIDevice *dev = &proxy->pci_dev;
> > @@ -2137,6 +2229,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
> >       k->ioeventfd_assign = virtio_pci_ioeventfd_assign;
> >       k->get_dma_as = virtio_pci_get_dma_as;
> >       k->queue_enabled = virtio_pci_queue_enabled;
> > +    k->set_config_notifiers = virtio_pci_set_guest_config_notifier;
> >   }
> >
> >   static const TypeInfo virtio_pci_bus_info = {
> > diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> > index ef8abe49c5..dae81ee414 100644
> > --- a/include/hw/virtio/virtio-bus.h
> > +++ b/include/hw/virtio/virtio-bus.h
> > @@ -93,6 +93,8 @@ struct VirtioBusClass {
> >        */
> >       bool has_variable_vring_alignment;
> >       AddressSpace *(*get_dma_as)(DeviceState *d);
> > +    int (*set_config_notifiers)(DeviceState *d, bool assign, bool with_irqfd);
> > +
> >   };
> >
> >   struct VirtioBusState {
>

Re: [PATCH v1 4/4] vhost-vdpa:add callback function for configure interrupt

[Cindy Lu]

On Thu, Jan 14, 2021 at 12:46 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2021/1/13 下午11:45, Cindy Lu wrote:
> > Add call back function for configure interrupt.
> > Set the notifier's fd to the kernel driver when vdpa start.
> > also set -1 when vdpa stop. then the kernel will release
> > the related聽resource聽
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> >   hw/virtio/trace-events            |  2 ++
> >   hw/virtio/vhost-vdpa.c            | 29 ++++++++++++++++++++++++++++-
> >   include/hw/virtio/vhost-backend.h |  4 ++++
> >   3 files changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> > index 2060a144a2..6710835b46 100644
> > --- a/hw/virtio/trace-events
> > +++ b/hw/virtio/trace-events
> > @@ -52,6 +52,8 @@ vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd) "dev: %p index:
> >   vhost_vdpa_get_features(void *dev, uint64_t features) "dev: %p features: 0x%"PRIx64
> >   vhost_vdpa_set_owner(void *dev) "dev: %p"
> >   vhost_vdpa_vq_get_addr(void *dev, void *vq, uint64_t desc_user_addr, uint64_t avail_user_addr, uint64_t used_user_addr) "dev: %p vq: %p desc_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64
> > +vhost_vdpa_set_config_call(void *dev, int *fd)"dev: %p fd: %p"
> > +
> >
> >   # virtio.c
> >   virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"
> > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > index 01d2101d09..1647bff8b0 100644
> > --- a/hw/virtio/vhost-vdpa.c
> > +++ b/hw/virtio/vhost-vdpa.c
> > @@ -467,20 +467,39 @@ static int vhost_vdpa_get_config(struct vhost_dev *dev, uint8_t *config,
> >       }
> >       return ret;
> >    }
> > +static void vhost_vdpa_config_notify_start(struct vhost_dev *dev,
> > +                                struct VirtIODevice *vdev, bool start)
> > +{
> > +    int fd, r;
> > +    if (start) {
> > +        fd = event_notifier_get_fd(&vdev->config_notifier);
> > +     } else {
> > +        fd = -1;
> > +     }
> > +    r = dev->vhost_ops->vhost_set_config_call(dev, &fd);
> > +    if (r) {
> > +        error_report("vhost_vdpa_config_notify_start error!");
> > +    }
>
>
> So on early version of kernel without config interrupt support. This
> will fail for sure. We need seek a way to make it work.
>
> Thanks
>
Thanks Jason I will test the old version and fix this one
>
> > +    return;
> >
> > +}
> >   static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
> >   {
> >       struct vhost_vdpa *v = dev->opaque;
> >       trace_vhost_vdpa_dev_start(dev, started);
> > +    VirtIODevice *vdev = dev->vdev;
> > +
> >       if (started) {
> >           uint8_t status = 0;
> >           memory_listener_register(&v->listener, &address_space_memory);
> >           vhost_vdpa_set_vring_ready(dev);
> >           vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
> >           vhost_vdpa_call(dev, VHOST_VDPA_GET_STATUS, &status);
> > -
> > +        /*set the configure interrupt call back*/
> > +        vhost_vdpa_config_notify_start(dev, vdev, true);
> >           return !(status & VIRTIO_CONFIG_S_DRIVER_OK);
> >       } else {
> > +        vhost_vdpa_config_notify_start(dev, vdev, false);
> >           vhost_vdpa_reset_device(dev);
> >           vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
> >                                      VIRTIO_CONFIG_S_DRIVER);
> > @@ -546,6 +565,13 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
> >       return vhost_vdpa_call(dev, VHOST_SET_VRING_CALL, file);
> >   }
> >
> > +static int vhost_vdpa_set_config_call(struct vhost_dev *dev,
> > +                                       int *fd)
> > +{
> > +    trace_vhost_vdpa_set_config_call(dev, fd);
> > +    return vhost_vdpa_call(dev, VHOST_VDPA_SET_CONFIG_CALL, fd);
> > +}
> > +
> >   static int vhost_vdpa_get_features(struct vhost_dev *dev,
> >                                        uint64_t *features)
> >   {
> > @@ -611,4 +637,5 @@ const VhostOps vdpa_ops = {
> >           .vhost_get_device_id = vhost_vdpa_get_device_id,
> >           .vhost_vq_get_addr = vhost_vdpa_vq_get_addr,
> >           .vhost_force_iommu = vhost_vdpa_force_iommu,
> > +        .vhost_set_config_call = vhost_vdpa_set_config_call,
> >   };
> > diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
> > index 8a6f8e2a7a..1a2fee8994 100644
> > --- a/include/hw/virtio/vhost-backend.h
> > +++ b/include/hw/virtio/vhost-backend.h
> > @@ -125,6 +125,9 @@ typedef int (*vhost_get_device_id_op)(struct vhost_dev *dev, uint32_t *dev_id);
> >
> >   typedef bool (*vhost_force_iommu_op)(struct vhost_dev *dev);
> >
> > +typedef int (*vhost_set_config_call_op)(struct vhost_dev *dev,
> > +                                       int *fd);
> > +
> >   typedef struct VhostOps {
> >       VhostBackendType backend_type;
> >       vhost_backend_init vhost_backend_init;
> > @@ -170,6 +173,7 @@ typedef struct VhostOps {
> >       vhost_vq_get_addr_op  vhost_vq_get_addr;
> >       vhost_get_device_id_op vhost_get_device_id;
> >       vhost_force_iommu_op vhost_force_iommu;
> > +    vhost_set_config_call_op vhost_set_config_call;
> >   } VhostOps;
> >
> >   extern const VhostOps user_ops;
>

Re: [PATCH v1 0/4] vhost-vdpa: add support for configure interrupt

[Cindy Lu]

On Thu, Jan 14, 2021 at 12:36 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2021/1/13 下午11:45, Cindy Lu wrote:
> > Add configure interrupt support in vdpa_vhost. qemu will use the irqfd
> > process to support this feature.
> >
> > These code are all tested in vp-vdpa (support configure interrupt)
> > vdpa_sim (not support configure interrupt)
> >
> >
> > Cindy Lu (4):
> >    virtio:add support in configure interrupt
> >    virtio-pci:add support for configure interrupt
>
>
> If possible, I would like to add the MMIO support for this.
>
> One great advantage of vDPA is that it can avoid to expose a PCI device
> for guest.
>
> Thanks
>
Sure, Thanks Jason, I will check this part.
>
> >    vhost_net:enable configure interrupt when vhost_net start
> >    vhost-vdpa:add callback function for configure interrupt
> >
> >   hw/net/vhost_net.c                | 19 ++++++-
> >   hw/virtio/trace-events            |  2 +
> >   hw/virtio/vhost-vdpa.c            | 29 +++++++++-
> >   hw/virtio/virtio-pci.c            | 93 +++++++++++++++++++++++++++++++
> >   hw/virtio/virtio.c                | 25 +++++++++
> >   include/hw/virtio/vhost-backend.h |  4 ++
> >   include/hw/virtio/virtio-bus.h    |  2 +
> >   include/hw/virtio/virtio.h        |  5 ++
> >   8 files changed, 177 insertions(+), 2 deletions(-)
> >
>

Re: [PATCH v1 4/4] vhost-vdpa:add callback function for configure interrupt

[Cindy Lu]

On Thu, Jan 14, 2021 at 12:17 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Jan 13, 2021 at 11:45:40PM +0800, Cindy Lu wrote:
> > Add call back function for configure interrupt.
> > Set the notifier's fd to the kernel driver when vdpa start.
> > also set -1 when vdpa stop. then the kernel will release
> > the related聽resource聽
>
> Something weird happened to the commit log here.
>
Thanks Micheal, I will fix this part
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> >  hw/virtio/trace-events            |  2 ++
> >  hw/virtio/vhost-vdpa.c            | 29 ++++++++++++++++++++++++++++-
> >  include/hw/virtio/vhost-backend.h |  4 ++++
> >  3 files changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> > index 2060a144a2..6710835b46 100644
> > --- a/hw/virtio/trace-events
> > +++ b/hw/virtio/trace-events
> > @@ -52,6 +52,8 @@ vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd) "dev: %p index:
> >  vhost_vdpa_get_features(void *dev, uint64_t features) "dev: %p features: 0x%"PRIx64
> >  vhost_vdpa_set_owner(void *dev) "dev: %p"
> >  vhost_vdpa_vq_get_addr(void *dev, void *vq, uint64_t desc_user_addr, uint64_t avail_user_addr, uint64_t used_user_addr) "dev: %p vq: %p desc_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64
> > +vhost_vdpa_set_config_call(void *dev, int *fd)"dev: %p fd: %p"
> > +
> >
> >  # virtio.c
> >  virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"
> > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > index 01d2101d09..1647bff8b0 100644
> > --- a/hw/virtio/vhost-vdpa.c
> > +++ b/hw/virtio/vhost-vdpa.c
> > @@ -467,20 +467,39 @@ static int vhost_vdpa_get_config(struct vhost_dev *dev, uint8_t *config,
> >      }
> >      return ret;
> >   }
> > +static void vhost_vdpa_config_notify_start(struct vhost_dev *dev,
> > +                                struct VirtIODevice *vdev, bool start)
> > +{
> > +    int fd, r;
> > +    if (start) {
> > +        fd = event_notifier_get_fd(&vdev->config_notifier);
> > +     } else {
> > +        fd = -1;
> > +     }
> > +    r = dev->vhost_ops->vhost_set_config_call(dev, &fd);
> > +    if (r) {
> > +        error_report("vhost_vdpa_config_notify_start error!");
> > +    }
> > +    return;
> >
> > +}
> >  static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
> >  {
> >      struct vhost_vdpa *v = dev->opaque;
> >      trace_vhost_vdpa_dev_start(dev, started);
> > +    VirtIODevice *vdev = dev->vdev;
> > +
> >      if (started) {
> >          uint8_t status = 0;
> >          memory_listener_register(&v->listener, &address_space_memory);
> >          vhost_vdpa_set_vring_ready(dev);
> >          vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
> >          vhost_vdpa_call(dev, VHOST_VDPA_GET_STATUS, &status);
> > -
> > +        /*set the configure interrupt call back*/
> > +        vhost_vdpa_config_notify_start(dev, vdev, true);
> >          return !(status & VIRTIO_CONFIG_S_DRIVER_OK);
> >      } else {
> > +        vhost_vdpa_config_notify_start(dev, vdev, false);
> >          vhost_vdpa_reset_device(dev);
> >          vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
> >                                     VIRTIO_CONFIG_S_DRIVER);
> > @@ -546,6 +565,13 @@ static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
> >      return vhost_vdpa_call(dev, VHOST_SET_VRING_CALL, file);
> >  }
> >
> > +static int vhost_vdpa_set_config_call(struct vhost_dev *dev,
> > +                                       int *fd)
> > +{
> > +    trace_vhost_vdpa_set_config_call(dev, fd);
> > +    return vhost_vdpa_call(dev, VHOST_VDPA_SET_CONFIG_CALL, fd);
> > +}
> > +
> >  static int vhost_vdpa_get_features(struct vhost_dev *dev,
> >                                       uint64_t *features)
> >  {
> > @@ -611,4 +637,5 @@ const VhostOps vdpa_ops = {
> >          .vhost_get_device_id = vhost_vdpa_get_device_id,
> >          .vhost_vq_get_addr = vhost_vdpa_vq_get_addr,
> >          .vhost_force_iommu = vhost_vdpa_force_iommu,
> > +        .vhost_set_config_call = vhost_vdpa_set_config_call,
> >  };
> > diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
> > index 8a6f8e2a7a..1a2fee8994 100644
> > --- a/include/hw/virtio/vhost-backend.h
> > +++ b/include/hw/virtio/vhost-backend.h
> > @@ -125,6 +125,9 @@ typedef int (*vhost_get_device_id_op)(struct vhost_dev *dev, uint32_t *dev_id);
> >
> >  typedef bool (*vhost_force_iommu_op)(struct vhost_dev *dev);
> >
> > +typedef int (*vhost_set_config_call_op)(struct vhost_dev *dev,
> > +                                       int *fd);
> > +
> >  typedef struct VhostOps {
> >      VhostBackendType backend_type;
> >      vhost_backend_init vhost_backend_init;
> > @@ -170,6 +173,7 @@ typedef struct VhostOps {
> >      vhost_vq_get_addr_op  vhost_vq_get_addr;
> >      vhost_get_device_id_op vhost_get_device_id;
> >      vhost_force_iommu_op vhost_force_iommu;
> > +    vhost_set_config_call_op vhost_set_config_call;
> >  } VhostOps;
> >
> >  extern const VhostOps user_ops;
> > --
> > 2.21.3
>