Patchwork [PATCHv2,05/12] virtio: add APIs for queue fields

login
register
mail settings
Submitter Michael S. Tsirkin
Date Feb. 25, 2010, 6:27 p.m.
Message ID <19fc42bc126bcf69ef1fac10e6e8073e387148dd.1267122331.git.mst@redhat.com>
Download mbox | patch
Permalink /patch/46261/
State New
Headers show

Comments

Michael S. Tsirkin - Feb. 25, 2010, 6:27 p.m.
vhost needs physical addresses for ring and other queue fields,
so add APIs for these.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/virtio.h |   10 +++++++++-
 2 files changed, 59 insertions(+), 1 deletions(-)
Blue Swirl - Feb. 25, 2010, 6:49 p.m.
On 2/25/10, Michael S. Tsirkin <mst@redhat.com> wrote:
> vhost needs physical addresses for ring and other queue fields,
>  so add APIs for these.
>
>  Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>  ---
>   hw/virtio.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   hw/virtio.h |   10 +++++++++-
>   2 files changed, 59 insertions(+), 1 deletions(-)
>
>  diff --git a/hw/virtio.c b/hw/virtio.c
>  index 1f5e7be..b017d7b 100644
>  --- a/hw/virtio.c
>  +++ b/hw/virtio.c
>  @@ -74,6 +74,11 @@ struct VirtQueue
>      uint16_t vector;
>      void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq);
>      VirtIODevice *vdev;
>  +<<<<<<< HEAD
>  +=======
>  +    EventNotifier guest_notifier;
>  +    EventNotifier host_notifier;
>  +>>>>>>> 8afa4fd... virtio: add APIs for queue fields

Bug.

>   };
>
>   /* virt queue functions */
>  @@ -593,6 +598,12 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
>      return &vdev->vq[i];
>   }
>
>  +void virtio_irq(VirtQueue *vq)
>  +{
>  +    vq->vdev->isr |= 0x01;
>  +    virtio_notify_vector(vq->vdev, vq->vector);
>  +}
>  +
>   void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
>   {
>      /* Always notify when queue is empty (when feature acknowledge) */
>  @@ -736,3 +747,42 @@ void virtio_bind_device(VirtIODevice *vdev, const VirtIOBindings *binding,
>      vdev->binding = binding;
>      vdev->binding_opaque = opaque;
>   }
>  +
>  +target_phys_addr_t virtio_queue_get_desc(VirtIODevice *vdev, int n)
>  +{
>  +       return vdev->vq[n].vring.desc;
>  +}
>  +
>  +target_phys_addr_t virtio_queue_get_avail(VirtIODevice *vdev, int n)
>  +{
>  +       return vdev->vq[n].vring.avail;
>  +}
>  +
>  +target_phys_addr_t virtio_queue_get_used(VirtIODevice *vdev, int n)
>  +{
>  +       return vdev->vq[n].vring.used;
>  +}
>  +
>  +uint16_t virtio_queue_last_avail_idx(VirtIODevice *vdev, int n)
>  +{
>  +       return vdev->vq[n].last_avail_idx;
>  +}
>  +
>  +void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx)
>  +{
>  +       vdev->vq[n].last_avail_idx = idx;
>  +}
>  +
>  +VirtQueue *virtio_queue(VirtIODevice *vdev, int n)
>  +{
>  +       return vdev->vq + n;
>  +}
>  +
>  +EventNotifier *virtio_queue_guest_notifier(VirtQueue *vq)
>  +{
>  +       return &vq->guest_notifier;
>  +}
>  +EventNotifier *virtio_queue_host_notifier(VirtQueue *vq)
>  +{
>  +       return &vq->host_notifier;
>  +}
>  diff --git a/hw/virtio.h b/hw/virtio.h
>  index af87889..2ebf2dd 100644
>  --- a/hw/virtio.h
>  +++ b/hw/virtio.h
>  @@ -184,5 +184,13 @@ void virtio_net_exit(VirtIODevice *vdev);
>         DEFINE_PROP_BIT("indirect_desc", _state, _field, \
>                         VIRTIO_RING_F_INDIRECT_DESC, true)
>
>  -
>  +target_phys_addr_t virtio_queue_get_desc(VirtIODevice *vdev, int n);
>  +target_phys_addr_t virtio_queue_get_avail(VirtIODevice *vdev, int n);
>  +target_phys_addr_t virtio_queue_get_used(VirtIODevice *vdev, int n);
>  +uint16_t virtio_queue_last_avail_idx(VirtIODevice *vdev, int n);
>  +void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx);
>  +VirtQueue *virtio_queue(VirtIODevice *vdev, int n);
>  +EventNotifier *virtio_queue_guest_notifier(VirtQueue *vq);
>  +EventNotifier *virtio_queue_host_notifier(VirtQueue *vq);
>  +void virtio_irq(VirtQueue *vq);
>   #endif
>
> --
>  1.7.0.18.g0d53a5
>
>
>
>
Anthony Liguori - Feb. 25, 2010, 7:25 p.m.
On 02/25/2010 12:27 PM, Michael S. Tsirkin wrote:
> vhost needs physical addresses for ring and other queue fields,
> so add APIs for these.
>
> Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
> ---
>   hw/virtio.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   hw/virtio.h |   10 +++++++++-
>   2 files changed, 59 insertions(+), 1 deletions(-)
>
> diff --git a/hw/virtio.c b/hw/virtio.c
> index 1f5e7be..b017d7b 100644
> --- a/hw/virtio.c
> +++ b/hw/virtio.c
> @@ -74,6 +74,11 @@ struct VirtQueue
>       uint16_t vector;
>       void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq);
>       VirtIODevice *vdev;
> +<<<<<<<  HEAD
> +=======
> +    EventNotifier guest_notifier;
> +    EventNotifier host_notifier;
> +>>>>>>>  8afa4fd... virtio: add APIs for queue fields
>    

That's clearly not right :-)

>   };
>
>   /* virt queue functions */
> @@ -593,6 +598,12 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
>       return&vdev->vq[i];
>   }
>
> +void virtio_irq(VirtQueue *vq)
> +{
> +    vq->vdev->isr |= 0x01;
> +    virtio_notify_vector(vq->vdev, vq->vector);
> +}
> +
>   void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
>   {
>       /* Always notify when queue is empty (when feature acknowledge) */
> @@ -736,3 +747,42 @@ void virtio_bind_device(VirtIODevice *vdev, const VirtIOBindings *binding,
>       vdev->binding = binding;
>       vdev->binding_opaque = opaque;
>   }
> +
> +target_phys_addr_t virtio_queue_get_desc(VirtIODevice *vdev, int n)
> +{
> +	return vdev->vq[n].vring.desc;
> +}
> +
> +target_phys_addr_t virtio_queue_get_avail(VirtIODevice *vdev, int n)
> +{
> +	return vdev->vq[n].vring.avail;
> +}
> +
> +target_phys_addr_t virtio_queue_get_used(VirtIODevice *vdev, int n)
> +{
> +	return vdev->vq[n].vring.used;
> +}
> +
> +uint16_t virtio_queue_last_avail_idx(VirtIODevice *vdev, int n)
> +{
> +	return vdev->vq[n].last_avail_idx;
> +}
> +
> +void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx)
> +{
> +	vdev->vq[n].last_avail_idx = idx;
> +}
>    

Is it really necessary to return last_avail?  Can't vhost maintain it's 
own last_avail?

I'm not a huge fan of returning physical addresses for each queue 
element.  I think it makes more sense to just return the start of the 
ring queue.  The ABI defines the queue to have a very specific layout 
afterall.

Regards,

Anthony Liguori

> +VirtQueue *virtio_queue(VirtIODevice *vdev, int n)
> +{
> +	return vdev->vq + n;
> +}
> +
> +EventNotifier *virtio_queue_guest_notifier(VirtQueue *vq)
> +{
> +	return&vq->guest_notifier;
> +}
> +EventNotifier *virtio_queue_host_notifier(VirtQueue *vq)
> +{
> +	return&vq->host_notifier;
> +}
> diff --git a/hw/virtio.h b/hw/virtio.h
> index af87889..2ebf2dd 100644
> --- a/hw/virtio.h
> +++ b/hw/virtio.h
> @@ -184,5 +184,13 @@ void virtio_net_exit(VirtIODevice *vdev);
>   	DEFINE_PROP_BIT("indirect_desc", _state, _field, \
>   			VIRTIO_RING_F_INDIRECT_DESC, true)
>
> -
> +target_phys_addr_t virtio_queue_get_desc(VirtIODevice *vdev, int n);
> +target_phys_addr_t virtio_queue_get_avail(VirtIODevice *vdev, int n);
> +target_phys_addr_t virtio_queue_get_used(VirtIODevice *vdev, int n);
> +uint16_t virtio_queue_last_avail_idx(VirtIODevice *vdev, int n);
> +void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx);
> +VirtQueue *virtio_queue(VirtIODevice *vdev, int n);
> +EventNotifier *virtio_queue_guest_notifier(VirtQueue *vq);
> +EventNotifier *virtio_queue_host_notifier(VirtQueue *vq);
> +void

> virtio_irq(VirtQueue *vq);
>   #endif
>
Gleb Natapov - Feb. 26, 2010, 8:46 a.m.
On Thu, Feb 25, 2010 at 01:25:46PM -0600, Anthony Liguori wrote:
> On 02/25/2010 12:27 PM, Michael S. Tsirkin wrote:
> >vhost needs physical addresses for ring and other queue fields,
> >so add APIs for these.
> >
> >Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
> >---
> >  hw/virtio.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  hw/virtio.h |   10 +++++++++-
> >  2 files changed, 59 insertions(+), 1 deletions(-)
> >
> >diff --git a/hw/virtio.c b/hw/virtio.c
> >index 1f5e7be..b017d7b 100644
> >--- a/hw/virtio.c
> >+++ b/hw/virtio.c
> >@@ -74,6 +74,11 @@ struct VirtQueue
> >      uint16_t vector;
> >      void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq);
> >      VirtIODevice *vdev;
> >+<<<<<<<  HEAD
> >+=======
> >+    EventNotifier guest_notifier;
> >+    EventNotifier host_notifier;
> >+>>>>>>>  8afa4fd... virtio: add APIs for queue fields
> 
> That's clearly not right :-)
> 
Merge conflict resolution is left as exercise for the reader.

--
			Gleb.
Michael S. Tsirkin - Feb. 26, 2010, 2:53 p.m.
> Bug.

Ouch.

Patch

diff --git a/hw/virtio.c b/hw/virtio.c
index 1f5e7be..b017d7b 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -74,6 +74,11 @@  struct VirtQueue
     uint16_t vector;
     void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq);
     VirtIODevice *vdev;
+<<<<<<< HEAD
+=======
+    EventNotifier guest_notifier;
+    EventNotifier host_notifier;
+>>>>>>> 8afa4fd... virtio: add APIs for queue fields
 };
 
 /* virt queue functions */
@@ -593,6 +598,12 @@  VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
     return &vdev->vq[i];
 }
 
+void virtio_irq(VirtQueue *vq)
+{
+    vq->vdev->isr |= 0x01;
+    virtio_notify_vector(vq->vdev, vq->vector);
+}
+
 void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
 {
     /* Always notify when queue is empty (when feature acknowledge) */
@@ -736,3 +747,42 @@  void virtio_bind_device(VirtIODevice *vdev, const VirtIOBindings *binding,
     vdev->binding = binding;
     vdev->binding_opaque = opaque;
 }
+
+target_phys_addr_t virtio_queue_get_desc(VirtIODevice *vdev, int n)
+{
+	return vdev->vq[n].vring.desc;
+}
+
+target_phys_addr_t virtio_queue_get_avail(VirtIODevice *vdev, int n)
+{
+	return vdev->vq[n].vring.avail;
+}
+
+target_phys_addr_t virtio_queue_get_used(VirtIODevice *vdev, int n)
+{
+	return vdev->vq[n].vring.used;
+}
+
+uint16_t virtio_queue_last_avail_idx(VirtIODevice *vdev, int n)
+{
+	return vdev->vq[n].last_avail_idx;
+}
+
+void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx)
+{
+	vdev->vq[n].last_avail_idx = idx;
+}
+
+VirtQueue *virtio_queue(VirtIODevice *vdev, int n)
+{
+	return vdev->vq + n;
+}
+
+EventNotifier *virtio_queue_guest_notifier(VirtQueue *vq)
+{
+	return &vq->guest_notifier;
+}
+EventNotifier *virtio_queue_host_notifier(VirtQueue *vq)
+{
+	return &vq->host_notifier;
+}
diff --git a/hw/virtio.h b/hw/virtio.h
index af87889..2ebf2dd 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -184,5 +184,13 @@  void virtio_net_exit(VirtIODevice *vdev);
 	DEFINE_PROP_BIT("indirect_desc", _state, _field, \
 			VIRTIO_RING_F_INDIRECT_DESC, true)
 
-
+target_phys_addr_t virtio_queue_get_desc(VirtIODevice *vdev, int n);
+target_phys_addr_t virtio_queue_get_avail(VirtIODevice *vdev, int n);
+target_phys_addr_t virtio_queue_get_used(VirtIODevice *vdev, int n);
+uint16_t virtio_queue_last_avail_idx(VirtIODevice *vdev, int n);
+void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx);
+VirtQueue *virtio_queue(VirtIODevice *vdev, int n);
+EventNotifier *virtio_queue_guest_notifier(VirtQueue *vq);
+EventNotifier *virtio_queue_host_notifier(VirtQueue *vq);
+void virtio_irq(VirtQueue *vq);
 #endif