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 <alex.william...@redhat.com>
> > > ---
> > >
> > > 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);
> > > }
> > >
> > >
> >
>
>
