Re: [Qemu-devel] [PATCH V3 09/14] virtio: introduce vector to virtqueues mapping

[Jason Wang <jasowang@redhat.com>]

On Fri, Mar 6, 2015 at 8:55 PM, Cornelia Huck 
<cornelia.huck@de.ibm.com> wrote:
> On Thu,  5 Mar 2015 13:48:46 +0800
> Jason Wang <jasowang@redhat.com> wrote:
> 
>>  Currently we will try to traverse all virtqueues to find a subset 
>> that
>>  using a specific vector. This is sub optimal when we will support
>>  hundreds or even thousands of virtqueues. So this patch introduces a
>>  method which could be used by transport to get all virtqueues that
>>  using a same vector. This is done through QLISTs and the number of
>>  QLISTs was queried through a transport specific method. When guest
>>  setting vectors, the virtqueue will be linked and helpers for 
>> traverse
>>  the list was also introduced.
>>  
>>  The first user will be virtio pci which will use this to speed up
>>  MSI-X masking and unmasking handling.
> 
> Will there be any users beyond virtio-pci, though?

Currently not. But if the vector could be shared in another transport, 
this is still useful.
>  For virtio-ccw, at
> least, "vectors" are an identity mapping of the queue index. I'm not
> sure if introducing (memory) overhead for everyone is worth it.

I see. Another reason I make it generic is because I believe we try 
hard to not expose the internals of VirtQueue to transport specific 
code. 
> 
> 
>>  
>>  Cc: Anthony Liguori <aliguori@amazon.com>
>>  Cc: Michael S. Tsirkin <mst@redhat.com>
>>  Signed-off-by: Jason Wang <jasowang@redhat.com>
>>  ---
>>   hw/virtio/virtio-pci.c         |  8 ++++++++
>>   hw/virtio/virtio.c             | 32 
>> ++++++++++++++++++++++++++++++--
>>   include/hw/virtio/virtio-bus.h |  1 +
>>   include/hw/virtio/virtio.h     |  3 +++
>>   4 files changed, 42 insertions(+), 2 deletions(-)
>>  
> 
>>   void virtio_queue_set_vector(VirtIODevice *vdev, int n, uint16_t 
>> vector)
>>   {
>>  -    if (n < virtio_get_queue_max(vdev))
>>  +    VirtQueue *vq = &vdev->vq[n];
>>  +
>>  +    if (n < virtio_get_queue_max(vdev)) {
>>  +        if (vdev->vq[n].vector != VIRTIO_NO_VECTOR) {
>>  +            QLIST_REMOVE(vq, node);
>>  +        }
>>           vdev->vq[n].vector = vector;
>>  +        if (vector != VIRTIO_NO_VECTOR) {
>>  +            QLIST_INSERT_HEAD(&vdev->vector_queues[vector], vq, 
>> node);
>>  +        }
>>  +    }
>>   }
> 
> Is there any way to remove an entry?

Yes, e.g setting the vector to VIRTIO_NO_VECTOR (e.g during reset or 
driver exit).
>  E.g., if the guest unassociates
> virtqueues. (I just noticed I probably need to use VIRTIO_NO_VECTOR
> instead of 0 for that case in ccw.)

Yes, I see 0 is used in virtio_ccw_set_vqs() when addr is zero. If 
there's no special consideration here, need to use VIRITO_NO_VECTOR.
> 
> 
> Or maybe I'm completely misunderstanding what vectors are doing on
> pci :)