[PATCH-RFC,03/13] virtio: add iofd/irqfd support

Add binding API to set iofd/irqfd support.
Will be used by vhost.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio.c |   13 ++++++++++---
 hw/virtio.h |    4 ++++
 2 files changed, 14 insertions(+), 3 deletions(-)

On 01/11/2010 11:17 AM, Michael S. Tsirkin wrote:
> Add binding API to set iofd/irqfd support.
> Will be used by vhost.
>
> Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
> ---
>   hw/virtio.c |   13 ++++++++++---
>   hw/virtio.h |    4 ++++
>   2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/hw/virtio.c b/hw/virtio.c
> index 8e3c9ad..b9ec863 100644
> --- a/hw/virtio.c
> +++ b/hw/virtio.c
> @@ -572,6 +572,12 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
>       return&vdev->vq[i];
>   }
>
> +void virtio_irq(VirtIODevice *vdev, VirtQueue *vq)
> +{
> +    vdev->isr |= 0x01;
> +    virtio_notify_vector(vdev, vq->vector);
> +}
> +
>   void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
>   {
>       /* Make sure used ring is updated before we check NO_INTERRUPT. */
> @@ -582,8 +588,7 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
>            (vq->inuse || vring_avail_idx(vq) != vq->last_avail_idx)))
>           return;
>
> -    vdev->isr |= 0x01;
> -    virtio_notify_vector(vdev, vq->vector);
> +    virtio_irq(vdev, vq);
>   }
>
>   void virtio_notify_config(VirtIODevice *vdev)
> @@ -696,8 +701,10 @@ VirtIODevice *virtio_common_init(const char *name, uint16_t device_id,
>       vdev->queue_sel = 0;
>       vdev->config_vector = VIRTIO_NO_VECTOR;
>       vdev->vq = qemu_mallocz(sizeof(VirtQueue) * VIRTIO_PCI_QUEUE_MAX);
> -    for(i = 0; i<  VIRTIO_PCI_QUEUE_MAX; i++)
> +    for(i = 0; i<  VIRTIO_PCI_QUEUE_MAX; i++) {
>           vdev->vq[i].vector = VIRTIO_NO_VECTOR;
> +        vdev->vq[i].vdev = vdev;
> +    }
>
>       vdev->name = name;
>       vdev->config_len = config_size;
> diff --git a/hw/virtio.h b/hw/virtio.h
> index ca840e1..193b3f9 100644
> --- a/hw/virtio.h
> +++ b/hw/virtio.h
> @@ -88,6 +88,8 @@ typedef struct {
>       int (*load_config)(void * opaque, QEMUFile *f);
>       int (*load_queue)(void * opaque, int n, QEMUFile *f);
>       unsigned (*get_features)(void * opaque);
> +    int (*set_irqfd)(void * opaque, int n, int fd, bool assigned);
> +    int (*set_queuefd)(void * opaque, int n, int fd, bool assigned);
>    

I'd like to see us abstract things like irqfd a bit more.  It should be 
a first class object with an method to raise/lower.  In fact, you can 
probably just use qemu_irq.

Regards,

Anthony Liguori

On Tue, Jan 12, 2010 at 04:36:45PM -0600, Anthony Liguori wrote:
> On 01/11/2010 11:17 AM, Michael S. Tsirkin wrote:
>> Add binding API to set iofd/irqfd support.
>> Will be used by vhost.
>>
>> Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
>> ---
>>   hw/virtio.c |   13 ++++++++++---
>>   hw/virtio.h |    4 ++++
>>   2 files changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/virtio.c b/hw/virtio.c
>> index 8e3c9ad..b9ec863 100644
>> --- a/hw/virtio.c
>> +++ b/hw/virtio.c
>> @@ -572,6 +572,12 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
>>       return&vdev->vq[i];
>>   }
>>
>> +void virtio_irq(VirtIODevice *vdev, VirtQueue *vq)
>> +{
>> +    vdev->isr |= 0x01;
>> +    virtio_notify_vector(vdev, vq->vector);
>> +}
>> +
>>   void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
>>   {
>>       /* Make sure used ring is updated before we check NO_INTERRUPT. */
>> @@ -582,8 +588,7 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
>>            (vq->inuse || vring_avail_idx(vq) != vq->last_avail_idx)))
>>           return;
>>
>> -    vdev->isr |= 0x01;
>> -    virtio_notify_vector(vdev, vq->vector);
>> +    virtio_irq(vdev, vq);
>>   }
>>
>>   void virtio_notify_config(VirtIODevice *vdev)
>> @@ -696,8 +701,10 @@ VirtIODevice *virtio_common_init(const char *name, uint16_t device_id,
>>       vdev->queue_sel = 0;
>>       vdev->config_vector = VIRTIO_NO_VECTOR;
>>       vdev->vq = qemu_mallocz(sizeof(VirtQueue) * VIRTIO_PCI_QUEUE_MAX);
>> -    for(i = 0; i<  VIRTIO_PCI_QUEUE_MAX; i++)
>> +    for(i = 0; i<  VIRTIO_PCI_QUEUE_MAX; i++) {
>>           vdev->vq[i].vector = VIRTIO_NO_VECTOR;
>> +        vdev->vq[i].vdev = vdev;
>> +    }
>>
>>       vdev->name = name;
>>       vdev->config_len = config_size;
>> diff --git a/hw/virtio.h b/hw/virtio.h
>> index ca840e1..193b3f9 100644
>> --- a/hw/virtio.h
>> +++ b/hw/virtio.h
>> @@ -88,6 +88,8 @@ typedef struct {
>>       int (*load_config)(void * opaque, QEMUFile *f);
>>       int (*load_queue)(void * opaque, int n, QEMUFile *f);
>>       unsigned (*get_features)(void * opaque);
>> +    int (*set_irqfd)(void * opaque, int n, int fd, bool assigned);
>> +    int (*set_queuefd)(void * opaque, int n, int fd, bool assigned);
>>    
>
> I'd like to see us abstract things like irqfd a bit more.  It should be  
> a first class object with an method to raise/lower.  In fact, you can  
> probably just use qemu_irq.
>
> Regards,
>
> Anthony Liguori

With a single provider, I fail to see the point of extra abstraction
layers, and I have no idea how a good abstraction would look. Let's see
when we have multiple implementations, then we can abstract them as
appropriate.  Also, irqfd does not have methods to raise/lower so
qemu_irq is far from a good fit.