Re: [Qemu-devel] [PATCH] pci: Return PCI_INTX_DISABLED when no bus INTx routing support

[Michael S. Tsirkin]

On Wed, Oct 17, 2012 at 12:54:36PM -0600, Alex Williamson wrote:
> On Wed, 2012-10-17 at 20:40 +0200, Jan Kiszka wrote:
> > On 2012-10-17 20:25, Alex Williamson wrote:
> > > Rather than assert, simply return PCI_INTX_DISABLED when we don't
> > > have a pci_route_irq_fn.  PIIX already returns DISABLED for an
> > > invalid pin, so users already deal with this state.  Users of this
> > > interface should only be acting on an ENABLED or INVERTED return
> > > value (though we really have no support for INVERTED).
> > > 
> > > Signed-off-by: Alex Williamson <address@hidden>
> > > ---
> > > 
> > > A compromise to the gridlock; defuse the assert, but don't add
> > > a new state to the API.  Thanks,
> > 
> > But how do you tell "unsupported" apart from "disabled" in VFIO? If the
> > chipset truly disabled the line, you must not provide it to the guest in
> 
> The downside to not being able to distinguish is that vfio can't tell
> the user anything useful about whether it should work, but needs to be
> implemented in the chipset or if it's just currently disabled.  Maybe an
> error_report below would fix that, but yes, we do lose granularity we
> had with NOROUTE.

Stick error_report in pci_device_route_intx_to_irq you mean? OK why not.

> Functionally, there's no direct kvm interrupt injection when this
> returns DISABLED, so vfio routes the interrupt to qemu where we do
> qemu_set_irq, and it can get dropped if it's really disabled.
> pci-assign will just not program the interrupt to kvm and intx won't
> work :-\  Thanks,
> 
> Alex
> 

All this handling is just dead code anyway, it's there just in case anyway.


> > >  hw/pci.c |    6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/pci.c b/hw/pci.c
> > > index 83d262a..9525220 100644
> > > --- a/hw/pci.c
> > > +++ b/hw/pci.c
> > > @@ -1094,7 +1094,11 @@ PCIINTxRoute 
> > > pci_device_route_intx_to_irq(PCIDevice *dev, int pin)
> > >           pin = bus->map_irq(dev, pin);
> > >           dev = bus->parent_dev;
> > >      } while (dev);
> > > -    assert(bus->route_intx_to_irq);
> > > +
> > > +    if (!bus->route_intx_to_irq) {
> > > +        return (PCIINTxRoute) { PCI_INTX_DISABLED, -1 };
> > > +    }
> > > +
> > >      return bus->route_intx_to_irq(bus->irq_opaque, pin);
> > >  }
> > >  
> > > 
> > 
> 
>