On Thu, 2012-12-20 at 18:36 +0200, Michael S. Tsirkin wrote: > On Thu, Dec 20, 2012 at 09:06:41AM -0700, Alex Williamson wrote: > > Guests typically enable MSI-X with all of the vectors in the MSI-X > > vector table masked. Only when the vector is enabled does the vector > > get unmasked, resulting in a vector_use callback. These two points, > > enable and unmask, correspond to pci_enable_msix() and request_irq() > > for Linux guests. Some drivers rely on VF/PF or PF/fw communication > > channels that expect the physical state of the device to match the > > guest visible state of the device. They don't appreciate lazily > > enabling MSI-X on the physical device. > > > > To solve this, enable MSI-X with a single vector when the MSI-X > > capability is enabled and immediate disable the vector. This leaves > > the physical device in exactly the same state between host and guest. > > Furthermore, the brief gap where we enable vector 0, it fires into > > userspace, not KVM, so the guest doesn't get spurious interrupts. > > Ideally we could call VFIO_DEVICE_SET_IRQS with the right parameters > > to enable MSI-X with zero vectors, but this will currently return an > > error as the Linux MSI-X interfaces do not allow it. > > > > Cc: qemu-sta...@nongnu.org > > Signed-off-by: Alex Williamson <alex.william...@redhat.com> > > Do you need an interface for this? Can you do low-level pci config > access instead? I imagine you would then enable MSIX and mask all > vectors at the same time. > > No?
I really don't like the idea of enabling MSI-X directly through config space. We're just asking for ownership conflicts doing that. In fact, vfio prevents MSI/X from being enabled through config space since those are controlled through ioctl. It also prevents access to the MSI-X vector table since userspace has no business reading or modifying it. Thanks, Alex > > --- > > hw/vfio_pci.c | 31 +++++++++++++++++++++++++++---- > > 1 file changed, 27 insertions(+), 4 deletions(-) > > > > VFIO makes this a bit cleaner, so I think this is both the stable and > > final fix here. > > > > diff --git a/hw/vfio_pci.c b/hw/vfio_pci.c > > index 7c27834..5178ccc 100644 > > --- a/hw/vfio_pci.c > > +++ b/hw/vfio_pci.c > > @@ -561,8 +561,9 @@ static int vfio_enable_vectors(VFIODevice *vdev, bool > > msix) > > return ret; > > } > > > > -static int vfio_msix_vector_use(PCIDevice *pdev, > > - unsigned int nr, MSIMessage msg) > > +static int vfio_msix_vector_do_use(PCIDevice *pdev, unsigned int nr, > > + MSIMessage msg, bool try_kvm, > > + IOHandler *handler) > > { > > VFIODevice *vdev = DO_UPCAST(VFIODevice, pdev, pdev); > > VFIOMSIVector *vector; > > @@ -586,7 +587,7 @@ static int vfio_msix_vector_use(PCIDevice *pdev, > > * Attempt to enable route through KVM irqchip, > > * default to userspace handling if unavailable. > > */ > > - vector->virq = kvm_irqchip_add_msi_route(kvm_state, msg); > > + vector->virq = try_kvm ? kvm_irqchip_add_msi_route(kvm_state, msg) : > > -1; > > if (vector->virq < 0 || > > kvm_irqchip_add_irqfd_notifier(kvm_state, &vector->interrupt, > > vector->virq) < 0) { > > @@ -595,7 +596,7 @@ static int vfio_msix_vector_use(PCIDevice *pdev, > > vector->virq = -1; > > } > > qemu_set_fd_handler(event_notifier_get_fd(&vector->interrupt), > > - vfio_msi_interrupt, NULL, vector); > > + handler, NULL, vector); > > } > > > > /* > > @@ -638,6 +639,12 @@ static int vfio_msix_vector_use(PCIDevice *pdev, > > return 0; > > } > > > > +static int vfio_msix_vector_use(PCIDevice *pdev, > > + unsigned int nr, MSIMessage msg) > > +{ > > + return vfio_msix_vector_do_use(pdev, nr, msg, true, > > vfio_msi_interrupt); > > +} > > + > > static void vfio_msix_vector_release(PCIDevice *pdev, unsigned int nr) > > { > > VFIODevice *vdev = DO_UPCAST(VFIODevice, pdev, pdev); > > @@ -696,6 +703,22 @@ static void vfio_enable_msix(VFIODevice *vdev) > > > > vdev->interrupt = VFIO_INT_MSIX; > > > > + /* > > + * Some communication channels between VF & PF or PF & fw rely on the > > + * physical state of the device and expect that enabling MSI-X from the > > + * guest enables the same on the host. When our guest is Linux, the > > + * guest driver call to pci_enable_msix() sets the enabling bit in the > > + * MSI-X capability, but leaves the vector table masked. We therefore > > + * can't rely on a vector_use callback (from request_irq() in the > > guest) > > + * to switch the physical device into MSI-X mode because that may come > > a > > + * long time after pci_enable_msix(). This code enables vector 0 with > > + * triggering to userspace, then immediately release the vector, > > leaving > > + * the physical device with no vectors enabled, but MSI-X enabled, just > > + * like the guest view. > > + */ > > + vfio_msix_vector_do_use(&vdev->pdev, 0, (MSIMessage) { 0, 0 }, false, > > NULL); > > + vfio_msix_vector_release(&vdev->pdev, 0); > > + > > if (msix_set_vector_notifiers(&vdev->pdev, vfio_msix_vector_use, > > vfio_msix_vector_release)) { > > error_report("vfio: msix_set_vector_notifiers failed\n");