From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52821) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XhDec-0002j3-Hb for qemu-devel@nongnu.org; Thu, 23 Oct 2014 04:22:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XhDeT-0002Dw-El for qemu-devel@nongnu.org; Thu, 23 Oct 2014 04:21:54 -0400 Received: from e06smtp15.uk.ibm.com ([195.75.94.111]:44956) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XhDeT-0002Dr-5M for qemu-devel@nongnu.org; Thu, 23 Oct 2014 04:21:45 -0400 Received: from /spool/local by e06smtp15.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 23 Oct 2014 09:21:43 +0100 Received: from b06cxnps4074.portsmouth.uk.ibm.com (d06relay11.portsmouth.uk.ibm.com [9.149.109.196]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id 7EFFF1B08023 for ; Thu, 23 Oct 2014 09:21:42 +0100 (BST) Received: from d06av03.portsmouth.uk.ibm.com (d06av03.portsmouth.uk.ibm.com [9.149.37.213]) by b06cxnps4074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id s9N8Lfsb11665876 for ; Thu, 23 Oct 2014 08:21:41 GMT Received: from d06av03.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av03.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s9N8LfTW015801 for ; Thu, 23 Oct 2014 02:21:41 -0600 Date: Thu, 23 Oct 2014 10:21:40 +0200 From: Frank Blaschka Message-ID: <20141023082140.GA40097@tuxmaker.boeblingen.de.ibm.com> References: <1413990815-40479-1-git-send-email-blaschka@linux.vnet.ibm.com> <1413998231.4202.235.camel@ul30vt.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1413998231.4202.235.camel@ul30vt.home> Subject: Re: [Qemu-devel] [PATCH] vfio: check if host device supports INTx List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: pbonzini@redhat.com, Frank Blaschka , agraf@suse.de, qemu-devel@nongnu.org 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 > > > > 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 > > --- > > 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); > > > >