[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-stable] [RESEND PATCH] pci-assign: Enable MSIX on device to ma
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-stable] [RESEND PATCH] pci-assign: Enable MSIX on device to match guest |
Date: |
Mon, 7 Jan 2013 18:01:19 +0200 |
On Sun, Jan 06, 2013 at 09:30:31PM -0700, Alex Williamson wrote:
> When a guest enables MSIX on a device we evaluate the MSIX vector
> table, typically find no unmasked vectors and don't switch the device
> to MSIX mode. This generally works fine and the device will be
> switched once the guest enables and therefore unmasks a vector.
> Unfortunately some drivers enable MSIX, then use interfaces to send
> commands between VF & PF or PF & firmware that act based on the host
> state of the device. These therefore may break when MSIX is managed
> lazily. This change re-enables the previous test used to enable MSIX
> (see qemu-kvm a6b402c9), which basically guesses whether a vector
> will be used based on the data field of the vector table.
>
> Cc: address@hidden
> Signed-off-by: Alex Williamson <address@hidden>
> Acked-by: Michael S. Tsirkin <address@hidden>
> ---
>
> Michael has now ack'd this patch as the correct initial first step,
> so I'm resending with that included. I'm actually not sure what the
> expected upstream path is for this file now that it's part of qemu.
> There's no entry for hw/kvm/* in MAINTAINERS nor anything specifically
> for this file. Is kvm still upstream for this, through the uq branch
> or is it qemu for anything not specifically part of a kvm interface?
> Anthony, Gleb, Marcelo, Michael, feel free to add this to your tree,
> any path is fine by me. Thanks,
>
> Alex
I can merge this if there are no other takers.
> hw/kvm/pci-assign.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c
> index 8ee9428..896cfe8 100644
> --- a/hw/kvm/pci-assign.c
> +++ b/hw/kvm/pci-assign.c
> @@ -1031,6 +1031,19 @@ static bool assigned_dev_msix_masked(MSIXTableEntry
> *entry)
> return (entry->ctrl & cpu_to_le32(0x1)) != 0;
> }
>
> +/*
> + * When MSI-X is first enabled the vector table typically has all the
> + * vectors masked, so we can't use that as the obvious test to figure out
> + * how many vectors to initially enable. Instead we look at the data field
> + * because this is what worked for pci-assign for a long time. This makes
> + * sure the physical MSI-X state tracks the guest's view, which is important
> + * for some VF/PF and PF/fw communication channels.
> + */
> +static bool assigned_dev_msix_skipped(MSIXTableEntry *entry)
> +{
> + return !entry->data;
> +}
> +
> static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
> {
> AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
> @@ -1041,7 +1054,7 @@ static int assigned_dev_update_msix_mmio(PCIDevice
> *pci_dev)
>
> /* Get the usable entry number for allocating */
> for (i = 0; i < adev->msix_max; i++, entry++) {
> - if (assigned_dev_msix_masked(entry)) {
> + if (assigned_dev_msix_skipped(entry)) {
> continue;
> }
> entries_nr++;
> @@ -1070,7 +1083,7 @@ static int assigned_dev_update_msix_mmio(PCIDevice
> *pci_dev)
> for (i = 0; i < adev->msix_max; i++, entry++) {
> adev->msi_virq[i] = -1;
>
> - if (assigned_dev_msix_masked(entry)) {
> + if (assigned_dev_msix_skipped(entry)) {
> continue;
> }
>