| Message ID | 1413990815-40479-1-git-send-email-blaschka@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Wed, 2014-10-22 at 17:13 +0200, Frank Blaschka wrote: > From: Frank Blaschka <frank.blaschka@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.blaschka@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 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 an SR-IOV VF from an INTx perspective to the user. That may be the more consistent option so that the VFIO IRQ INFO aligns with config space. Thanks, Alex > + vdev->intx.available = true; > + ret = 0; > + } else if (irq_info.count == 0) { > + vdev->intx.available = false; > + } else { > + vdev->intx.available = true; > + } > + > error: > if (ret) { > QLIST_REMOVE(vdev, next);
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.blaschka@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.blaschka@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? > an SR-IOV VF from an INTx perspective to the user. That may be the more > consistent option so that the VFIO IRQ INFO aligns with config space. > Thanks, > > Alex > > > + vdev->intx.available = true; > > + ret = 0; > > + } else if (irq_info.count == 0) { > > + vdev->intx.available = false; > > + } else { > > + vdev->intx.available = true; > > + } > > + > > error: > > if (ret) { > > QLIST_REMOVE(vdev, next); > > > >
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 + */ + vdev->intx.available = true; + ret = 0; + } else if (irq_info.count == 0) { + vdev->intx.available = false; + } else { + vdev->intx.available = true; + } + error: if (ret) { QLIST_REMOVE(vdev, next);