Re: [Qemu-devel] [PATCH] virtio-serial: Allow one MSI-X vector per virtqueue

[Amit Shah]

On (Mon) 19 Dec 2011 [14:09:43], Zang Hongyong wrote:
> 于 2011/12/16,星期五 17:39, Amit Shah 写道:
> >On (Fri) 16 Dec 2011 [09:14:26], address@hidden wrote:
> >>From: Hongyong Zang<address@hidden>
> >>
> >>In pci_enable_msix(), the guest's virtio-serial driver tries to set msi-x
> >>with one vector per queue. But it fails and eventually all virtio-serial
> >>ports share one MSI-X vector. Because every virtio-serial port has *two*
> >>virtqueues, virtio-serial needs (port+1)*2 vectors other than (port+1).
> >Ouch, good catch.
> >
> >One comment below:
> >
> >>This patch allows every virtqueue to have its own MSI-X vector.
> >>(When the MSI-X vectors needed are more than MSIX_MAX_ENTRIES defined in
> >>qemu: msix.c, all the queues still share one MSI-X vector as before.)
> >>
> >>Signed-off-by: Hongyong Zang<address@hidden>
> >>---
> >>  hw/virtio-pci.c |    5 ++++-
> >>  1 files changed, 4 insertions(+), 1 deletions(-)
> >>
> >>diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> >>index 77b75bc..2c9c6fb 100644
> >>--- a/hw/virtio-pci.c
> >>+++ b/hw/virtio-pci.c
> >>@@ -718,8 +718,11 @@ static int virtio_serial_init_pci(PCIDevice *pci_dev)
> >>          return -1;
> >>      }
> >>      vdev->nvectors = proxy->nvectors == DEV_NVECTORS_UNSPECIFIED
> >>-                                        ? 
> >>proxy->serial.max_virtserial_ports + 1
> >>+                                        ? 
> >>(proxy->serial.max_virtserial_ports + 1) * 2
> >>                                          : proxy->nvectors;
> >>+    /*msix.c: #define MSIX_MAX_ENTRIES 32*/
> >>+    if (vdev->nvectors>  32)
> >>+        vdev->nvectors = 32;
> >This change isn't needed: if the proxy->nvectors value exceeds the max
> >allowed, virtio_init_pci() will end up using a shared vector instead
> >of separate ones.
> >
> Hi Amit,
> If the nvectors exceeds the max, msix_init() will return -EINVAL in QEMU,
> and the front-end driver in Guest will use regular interrupt instead
> of MSI-X.

In that case, I believe msix_init() should be changed to attempt to
share interrupts instead of drivers doing this by themselves.

                Amit