[PATCH v1 1/5] virtio: Introduce notify_queue

Edgar E. Iglesias posted 5 patches 2 weeks, 3 days ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>
[PATCH v1 1/5] virtio: Introduce notify_queue
Posted by Edgar E. Iglesias 2 weeks, 3 days ago
From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
 hw/virtio/virtio.c             | 7 +++++++
 include/hw/virtio/virtio-bus.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 153ee0a0cf..8a53fb5f93 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2700,12 +2700,19 @@ static void virtio_irq(VirtQueue *vq)
 
 void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
 {
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+
     WITH_RCU_READ_LOCK_GUARD() {
         if (!virtio_should_notify(vdev, vq)) {
             return;
         }
     }
 
+    if (k->notify_queue) {
+        k->notify_queue(qbus->parent, virtio_get_queue_index(vq));
+    }
+
     trace_virtio_notify(vdev, vq);
     virtio_irq(vq);
 }
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 7ab8c9dab0..043dbeb4cf 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -39,6 +39,7 @@ DECLARE_OBJ_CHECKERS(VirtioBusState, VirtioBusClass,
 struct VirtioBusClass {
     /* This is what a VirtioBus must implement */
     BusClass parent;
+    void (*notify_queue)(DeviceState *d, uint16_t index);
     void (*notify)(DeviceState *d, uint16_t vector);
     void (*save_config)(DeviceState *d, QEMUFile *f);
     void (*save_queue)(DeviceState *d, int n, QEMUFile *f);
-- 
2.43.0
Re: [PATCH v1 1/5] virtio: Introduce notify_queue
Posted by Alex Bennée 1 week, 2 days ago
"Edgar E. Iglesias" <edgar.iglesias@gmail.com> writes:

> From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>  hw/virtio/virtio.c             | 7 +++++++
>  include/hw/virtio/virtio-bus.h | 1 +
>  2 files changed, 8 insertions(+)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 153ee0a0cf..8a53fb5f93 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2700,12 +2700,19 @@ static void virtio_irq(VirtQueue *vq)
>  
>  void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
>  {
> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +
>      WITH_RCU_READ_LOCK_GUARD() {
>          if (!virtio_should_notify(vdev, vq)) {
>              return;
>          }
>      }
>  
> +    if (k->notify_queue) {
> +        k->notify_queue(qbus->parent, virtio_get_queue_index(vq));
> +    }
> +
>      trace_virtio_notify(vdev, vq);
>      virtio_irq(vq);
>  }
> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> index 7ab8c9dab0..043dbeb4cf 100644
> --- a/include/hw/virtio/virtio-bus.h
> +++ b/include/hw/virtio/virtio-bus.h
> @@ -39,6 +39,7 @@ DECLARE_OBJ_CHECKERS(VirtioBusState, VirtioBusClass,
>  struct VirtioBusClass {
>      /* This is what a VirtioBus must implement */
>      BusClass parent;
> +    void (*notify_queue)(DeviceState *d, uint16_t index);
>      void (*notify)(DeviceState *d, uint16_t vector);
>      void (*save_config)(DeviceState *d, QEMUFile *f);
>      void (*save_queue)(DeviceState *d, int n, QEMUFile *f);

The code looks fine but we could do with a little outline of why we need
this is the commit messages. Why do we have notify and notify_queue? Are
they mutually exclusive? 

Not specific to this patch but we should strive to document the
individual methods in each class to give a clearer idea of what they do.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro