Re: [Qemu-devel] [RESEND PATCH] pci-assign: Enable MSIX on device to match guest

[Marcelo Tosatti]

On Mon, Jan 07, 2013 at 06:01:19PM +0200, Michael S. Tsirkin wrote:
> 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.

Go for it.

> 
> >  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;
> >          }
> >  
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to address@hidden
> More majordomo info at  http://vger.kernel.org/majordomo-info.html