Re: [Qemu-devel] [PATCH 0/6] Misc PCI cleanups

[Michael S. Tsirkin]

On Mon, Oct 08, 2012 at 01:27:33PM -0600, Alex Williamson wrote:
> On Mon, 2012-10-08 at 22:15 +0200, Michael S. Tsirkin wrote:
> > On Mon, Oct 08, 2012 at 09:58:32AM -0600, Alex Williamson wrote:
> > > Michael, Jan,
> > > 
> > > Any comments on these?  I'd like to make the PCI changes before I update
> > > vfio-pci to make use of the new resampling irqfd in kvm.  We don't have
> > > anyone officially listed as maintainer of pci-assign since it's been
> > > moved to qemu.  I could include the pci-assign patches in my tree if you
> > > prefer.  Thanks,
> > > 
> > > Alex
> > 
> > Patches themselves look fine, but I'd like to
> > better understand why do we want the INTx fallback.
> > Isn't it easier to add intx routing support?
> 
> vfio-pci can work with or without intx routing support.  Its presence is
> just one requirement to enable kvm accelerated intx support.  Regardless
> of whether it's easy or hard to implement intx routing in a given
> chipset, I currently can't probe for it and make useful decisions about
> whether or not to enable kvm support without potentially hitting an
> assert.  It's arguable how important intx acceleration is for specific
> applications, so while I'd like all chipsets to implement it, I don't
> know that it should be a gating factor to chipset integration.  Thanks,
> 
> Alex

Yes but there's nothing kvm specific in the routing API,
and IIRC it actually works fine without kvm.

As I see it, if some chipset does not expose it, it's a bug, and the
reason for lack of support is because no one cares about supporting
device assignment there.

So this API is not something devices should probe for.
How about just assuming it works?

Otherwise, you are adding code and API that will become dead code
when everyone supports the required API.

> > > On Tue, 2012-10-02 at 13:21 -0600, Alex Williamson wrote:
> > > > A few cleanups that I'll also apply to vfio-pci.  First make intx
> > > > route checking non-fatal.  vfio-pci has a fallback INTx mechanism
> > > > that doesn't rely on this, so we can already run on q35, but not if
> > > > we can't even probe for intx routing w/o blowing up.  Next, both
> > > > vfio-pci and pci-assign test whether INTx routing has changed using
> > > > similar functions.  Make this common.  Finally, expose a way to
> > > > get the MSI message for an MSI vector.  Again, both pci-assign and
> > > > vfio-pci need to do this to program the vector for KVM injection.
> > > > Thanks,
> > > > 
> > > > Alex
> > > > 
> > > > ---
> > > > 
> > > > Alex Williamson (6):
> > > >       pci-assign: Use msi_get_message()
> > > >       msi: Add msi_get_message()
> > > >       pci-assign: Use pci_intx_route_changed()
> > > >       pci: Helper function for testing if an INTx route changed
> > > >       pci-assign: Add support for no-route
> > > >       pci: Add INTx no-route option
> > > > 
> > > > 
> > > >  hw/kvm/pci-assign.c |   14 ++++++++------
> > > >  hw/msi.c            |   45 
> > > > +++++++++++++++++++++++++++++----------------
> > > >  hw/msi.h            |    1 +
> > > >  hw/pci.c            |   13 +++++++++++--
> > > >  hw/pci.h            |    2 ++
> > > >  5 files changed, 51 insertions(+), 24 deletions(-)
> > > 
> > > 
> 
>