On Thu, Oct 23, 2014 at 08:26:51AM -0600, Alex Williamson wrote: > On Thu, 2014-10-23 at 10:21 +0200, Frank Blaschka wrote: > > On Wed, Oct 22, 2014 at 11:17:11AM -0600, Alex Williamson wrote: > > > On Wed, 2014-10-22 at 17:13 +0200, Frank Blaschka wrote: > > > > From: Frank Blaschka <frank.blasc...@de.ibm.com> > > > > > > > > Let the kernel announce if INTx is available. Yes, there are platforms > > > > (e.g. s390) which do not support INTx. > > > > > > > > Signed-off-by: Frank Blaschka <frank.blasc...@de.ibm.com> > > > > --- > > > > hw/misc/vfio.c | 18 +++++++++++++++++- > > > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c > > > > index d66f3d2..3e9600b 100644 > > > > --- a/hw/misc/vfio.c > > > > +++ b/hw/misc/vfio.c > > > > @@ -109,6 +109,7 @@ typedef struct VFIOVGA { > > > > } VFIOVGA; > > > > > > > > typedef struct VFIOINTx { > > > > + bool available; /* intx available */ > > > > bool pending; /* interrupt pending */ > > > > bool kvm_accel; /* set when QEMU bypass through KVM enabled */ > > > > uint8_t pin; /* which pin to pull for qemu_set_irq */ > > > > @@ -554,7 +555,7 @@ static int vfio_enable_intx(VFIODevice *vdev) > > > > struct vfio_irq_set *irq_set; > > > > int32_t *pfd; > > > > > > > > - if (!pin) { > > > > + if (!pin || !vdev->intx.available) { > > > > return 0; > > > > } > > > > > > > > @@ -4032,6 +4033,21 @@ static int vfio_get_device(VFIOGroup *group, > > > > const char *name, VFIODevice *vdev) > > > > vdev->host.function); > > > > } > > > > > > > > + irq_info.index = VFIO_PCI_INTX_IRQ_INDEX; > > > > + ret = ioctl(vdev->fd, VFIO_DEVICE_GET_IRQ_INFO, &irq_info); > > > > + if (ret) { > > > > + /* > > > > + * This can fail for an old kernel or legacy PCI dev > > > > + * we assume intx is available > > > > + */ > > > > > > Is this true? It's unfortunately from an ABI stability standpoint that > > > we weren't calling this, but the ioctl should have always been there and > > > worked. I'd rather error out here than add a fallback if this is just > > > > ok > > > > > paranoia. Note that SR-IOV VFs will also report count=0 but their IRQ > > > pin register will read 0. We do also have the option to virtualize the > > > IRQ pin register for s390 devices which would make them look the same as > > > > sounds interesting, can you elaborate a little bit more on this? > > At what point can we hook in and modify the pci device config space? > > Untested, but I think it would be something like this: >
This is great, works perfect and there is no need to do any qemu vfio changes anymore. You can forget about this patch. Thx, Frank > --- a/drivers/vfio/pci/vfio_pci_config.c > +++ b/drivers/vfio/pci/vfio_pci_config.c > @@ -609,6 +609,10 @@ static int __init init_pci_cap_basic_perm(struct > perm_bits > > /* Sometimes used by sw, just virtualize */ > p_setb(perm, PCI_INTERRUPT_LINE, (u8)ALL_VIRT, (u8)ALL_WRITE); > + > + /* Virtualize interrupt pin to allow hiding INTx */ > + p_setb(perm, PCI_INTERRUPT_PIN, (u8)ALL_VIRT, (u8)NO_WRITE); > + > return 0; > } > > @@ -1445,6 +1449,9 @@ int vfio_config_init(struct vfio_pci_device *vdev) > *(__le16 *)&vconfig[PCI_DEVICE_ID] = > cpu_to_le16(pdev->device); > } > > + if (!IS_ENABLED(CONFIG_VFIO_PCI_INTX)) > + vconfig[PCI_INTERRUPT_PIN] = 0; > + > ret = vfio_cap_init(vdev); > if (ret) > goto out; > >