Virtualization - Apr 2009 - [PATCH RFC 0/8] virtio: add guest MSI-X support

Add optional MSI-X support: use a vector per virtqueue with
fallback to a common vector and finally to regular interrupt.
Teach all drivers to use it.

I added 2 new virtio operations: request_vqs/free_vqs because MSI
needs to know the total number of vectors upfront.

Signed-off-by: Michael S. Tsirkin <mst at redhat.com>

---

Here's a draft set of patches for MSI-X support in the guest.  It still
needs to be tested properly, and performance impact measured, but I
thought I'd share it here in the hope of getting some very early
feedback/flames.

Michael S. Tsirkin (8):
  virtio: add request_vqs/free_vqs virtio operations
  virtio_blk: add request_vqs/free_vqs calls
  virtio-rng: add request_vqs/free_vqs calls
  virtio_console: add request_vqs/free_vqs calls
  virtio_net: add request_vqs/free_vqs calls
  virtio_balloon: add request_vqs/free_vqs calls
  virtio_pci: split up vp_interrupt
  virtio_pci: optional MSI-X support

 drivers/block/virtio_blk.c          |    9 ++-
 drivers/char/hw_random/virtio-rng.c |   10 ++-
 drivers/char/virtio_console.c       |    8 ++-
 drivers/net/virtio_net.c            |   16 +++-
 drivers/virtio/virtio_balloon.c     |    9 ++-
 drivers/virtio/virtio_pci.c         |  192 ++++++++++++++++++++++++++++++-----
 include/linux/virtio_config.h       |   35 +++++++
 7 files changed, 247 insertions(+), 32 deletions(-)

Am Monday 27 April 2009 14:31:36 schrieb Michael S.
Tsirkin:
> Add optional MSI-X support: use a vector per virtqueue with
> fallback to a common vector and finally to regular interrupt.
> Teach all drivers to use it.
> 
> I added 2 new virtio operations: request_vqs/free_vqs because MSI
> needs to know the total number of vectors upfront.
> 
> Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
I dont know, if that is feasible for MSI, but the transport(virtio_pci) should
already know the number of virtqueues, which should match the number of
vectors, no?

In fact, the transport has to have a way of getting the number of virtqeues
because find_vq returns ENOENT on invalid index numbers.

Christian

On Mon, Apr 27, 2009 at 04:00:30PM +0200, Christian Borntraeger
wrote:
> Am Monday 27 April 2009 14:31:36 schrieb Michael S. Tsirkin:
> > Add optional MSI-X support: use a vector per virtqueue with
> > fallback to a common vector and finally to regular interrupt.
> > Teach all drivers to use it.
> > 
> > I added 2 new virtio operations: request_vqs/free_vqs because MSI
> > needs to know the total number of vectors upfront.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> 
> I dont know, if that is feasible for MSI, but the transport(virtio_pci)
should
> already know the number of virtqueues, which should match the number of
> vectors, no?
I think no, the transport can find out the max number of vectors the
device supports (pci_msix_table_size), but not how many virtqueues are
needed by the driver.
As  number of virtqueues <= number of vectors,
we could pre-allocate all vectors that host supports, but this seems
a bit drastic as an MSI-X device could support up to 2K vectors.
> In fact, the transport has to have a way of getting the number of virtqeues
> because find_vq returns ENOENT on invalid index numbers.
> 
> Christian
So again, I think this is an upper bound supported by host. Right?

-- 
MST

Michael S. Tsirkin wrote:
> Add optional MSI-X support: use a vector per virtqueue with
> fallback to a common vector and finally to regular interrupt.
> Teach all drivers to use it.
>
> I added 2 new virtio operations: request_vqs/free_vqs because MSI
> needs to know the total number of vectors upfront.
>
> Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
>   
Do you have userspace patches for testing?

Regards,

Anthony Liguori

Christian Borntraeger wrote:
> Am Monday 27 April 2009 14:31:36 schrieb Michael S. Tsirkin:
>   
>> Add optional MSI-X support: use a vector per virtqueue with
>> fallback to a common vector and finally to regular interrupt.
>> Teach all drivers to use it.
>>
>> I added 2 new virtio operations: request_vqs/free_vqs because MSI
>> needs to know the total number of vectors upfront.
>>
>> Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
>>     
>
> I dont know, if that is feasible for MSI, but the transport(virtio_pci)
should
> already know the number of virtqueues, which should match the number of
> vectors, no?
>
> In fact, the transport has to have a way of getting the number of virtqeues
> because find_vq returns ENOENT on invalid index numbers.
>   
One thing I thought was to decouple the virtqueue count from the MSI 
entry count, and let the guest choose how many MSI entries it wants to 
use.  This is useful if it wants several queues to share an interrupt, 
perhaps it wants both rx and tx rings on one cpu to share a single vector.

So the device could expose a large, constant number of MSI entries, and 
in addition expose a table mapping ring number to msi entry number.  The 
guest could point all rings to one entry, or have each ring use a 
private entry, or anything in between.

That saves us the new API (at the expense of a lot more code, but with 
added flexibility).

-- 
error compiling committee.c: too many arguments to function

Am Monday 27 April 2009 16:32:56 schrieb Michael S. Tsirkin:
> > I dont know, if that is feasible for MSI, but the
transport(virtio_pci) should
> > already know the number of virtqueues, which should match the number
of
> > vectors, no?
> 
> I think no, the transport can find out the max number of vectors the
> device supports (pci_msix_table_size), but not how many virtqueues are
> needed by the driver.
I was not talking about the number of vectors, but the number of real exisiting
virtqueues.
I just read virtio_pci again and yes,it would be a bit hard to get the number
of available virtqueues, because we need to loop over all possible vqs:
(something like)

for (i=0; i < SOMEMAX; i++) {
        iowrite16(i, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL);
        if (!ioread16(vp_dev->ioaddr + VIRTIO_PCI_QUEUE_NUM))
		break;
}
return i;

which is obviously not very nice.
On the other hand, at the moment we have up to 3 virtqueues per device
(network), and we have a current maximum of 16 virtqeues in qemu
(VIRTIO_PCI_QUEUE_MAX in virtio.c), which would reduce the number.
> As  number of virtqueues <= number of vectors,
> we could pre-allocate all vectors that host supports, but this seems
> a bit drastic as an MSI-X device could support up to 2K vectors.
> 
> > In fact, the transport has to have a way of getting the number of
virtqeues
> > because find_vq returns ENOENT on invalid index numbers.
> > 
> > Christian
> 
> So again, I think this is an upper bound supported by host. Right?
Not the upper bound, but the real available virtqueues. (With current qemu
3 for virtio-net, 2 for virtio-console etc.)

Since I use a different transport (drivers/s390/kvm/kvm_virtio.c), my
motiviation is to keep the virtio interface as generic as possible. I dont
really like the new interface, but I cannot give you silver bullet technical
reasons - its more a gut feeling. The interface would work with lguest and s390.

Anyway. Avis suggestion to decouple MSI count and virtqueue count looks
like a promising approach. 

Christian

On Mon, Apr 27, 2009 at 09:39:06AM -0500, Anthony Liguori
wrote:
> Michael S. Tsirkin wrote:
>> Add optional MSI-X support: use a vector per virtqueue with
>> fallback to a common vector and finally to regular interrupt.
>> Teach all drivers to use it.
>>
>> I added 2 new virtio operations: request_vqs/free_vqs because MSI
>> needs to know the total number of vectors upfront.
>>
>> Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
>>   
>
> Do you have userspace patches for testing?
>
> Regards,
>
> Anthony Liguori
Not ready yet, sorry.

-- 
MST