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?
> ---
> 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");
