From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48046) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bGpMK-0005Nm-Io for qemu-devel@nongnu.org; Sat, 25 Jun 2016 11:19:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bGpME-0006SE-Hb for qemu-devel@nongnu.org; Sat, 25 Jun 2016 11:18:59 -0400 Received: from mout.web.de ([212.227.15.3]:61671) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bGpME-0006Rv-70 for qemu-devel@nongnu.org; Sat, 25 Jun 2016 11:18:54 -0400 References: <1466495274-5011-1-git-send-email-peterx@redhat.com> <1466495274-5011-17-git-send-email-peterx@redhat.com> <576E3BEA.5010400@web.de> <20160625131854.GA16629@pxdev.xzpeter.org> From: Jan Kiszka Message-ID: <576EA0D0.3070602@web.de> Date: Sat, 25 Jun 2016 17:18:40 +0200 MIME-Version: 1.0 In-Reply-To: <20160625131854.GA16629@pxdev.xzpeter.org> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="EKaOmnvt3v6ixV6fI2SixerJPawkXex4p" Subject: Re: [Qemu-devel] [PATCH v10 16/26] intel_iommu: add support for split irqchip List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Xu Cc: qemu-devel@nongnu.org, imammedo@redhat.com, rth@twiddle.net, ehabkost@redhat.com, jasowang@redhat.com, marcel@redhat.com, mst@redhat.com, pbonzini@redhat.com, rkrcmar@redhat.com, alex.williamson@redhat.com, wexu@redhat.com, davidkiarie4@gmail.com, Valentine Sinitsyn This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --EKaOmnvt3v6ixV6fI2SixerJPawkXex4p From: Jan Kiszka To: Peter Xu Cc: qemu-devel@nongnu.org, imammedo@redhat.com, rth@twiddle.net, ehabkost@redhat.com, jasowang@redhat.com, marcel@redhat.com, mst@redhat.com, pbonzini@redhat.com, rkrcmar@redhat.com, alex.williamson@redhat.com, wexu@redhat.com, davidkiarie4@gmail.com, Valentine Sinitsyn Message-ID: <576EA0D0.3070602@web.de> Subject: Re: [PATCH v10 16/26] intel_iommu: add support for split irqchip References: <1466495274-5011-1-git-send-email-peterx@redhat.com> <1466495274-5011-17-git-send-email-peterx@redhat.com> <576E3BEA.5010400@web.de> <20160625131854.GA16629@pxdev.xzpeter.org> In-Reply-To: <20160625131854.GA16629@pxdev.xzpeter.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 2016-06-25 15:18, Peter Xu wrote: > On Sat, Jun 25, 2016 at 10:08:10AM +0200, Jan Kiszka wrote: >=20 > [...] >=20 >> For successful remappings, this is fine - it just caches the result in= >> an interrupt route. But what will happen with invalid interrupts? >> >> My current understanding is that, because the translation happens on >> activation of that interrupt source, not on actual signalling, the IOM= MU >> will report an error too early and none when the interrupt is actually= >> sent. That will lead to unwanted results, in the worst case >> false-positiv IR error reports to the guest, no? >> >> I think we need to do this: >> - silently remap broken sources to an error sink >> - hook up the error sink with the actual IOMMU model (Intel or AMD) >> - when that source actually fires, let the sink report an IR >> translation error to the guest >> >> Am I right? >=20 > Right. I totally missed this one. :( >=20 > Currently when split irqchip is specified, IOAPIC interrupts are > cached in kernel with type KVM_IRQ_ROUTING_MSI (which is the same as > irqfds). When guest specify a fault interrupt entry, it is possible > that we silently fail the update, and all further interrupts are still > the old and correct one. >=20 > I agree with your solution on this. First of all we update the > interrupt even if it's faulty, but we should mark it out. After that, > we should fire QEMU from kernel side when the fault interrupt is > triggered, so that QEMU IOMMU can still generate corresponding fault > report interrupt to guest (though for Intel IOMMU IR, we still haven't > handled any fault report yet, but we should be prepared for it). >=20 > So it seems that finally we cannot avoid touching KVM this time. >=20 > I have a thought on how to implement the "sink" you have mentioned: >=20 > First of all, in KVM, we provide a new KVM_IRQ_ROUTING_* type, maybe > called: >=20 > KVM_IRQ_ROUTING_EVENTFD Not really, because all sources are either using eventfds, which you can also terminate in user space (already done for vhost and vfio in certain scenarios - IIRC) or originate there anyway (IOAPIC). >=20 > When KVM got this kind of interrupt, KVM does not trigger any real > interrupt to guest. Instead, it just do eventfd_signal() to a > pre-defined fd (maybe also with some data along with the notification, > so that we can put the error inside?), which is set during > KVM_SET_GSI_ROUTING ioctl(). >=20 > After that, QEMU register all fault interrupts using this new > KVM_IRQ_ROUTING_EVENTFD type (rather than original > KVM_IRQ_ROUTING_MSI), assign a specific handler to handle the events > from these interrupts, and trigger IOMMU fault report path in that > handler. >=20 > (Here I used KVM_IRQ_ROUTING_EVENTFD rather than something like > KVM_IRQ_ROUTING_FAULT_MSI to make the API a more general one, in case > we can leverage it in other cases in the future) >=20 > Do you think the above workable? >=20 > No matter which solution we will have, I would still suggest we add > this as an "enhancement" after this series, since: >=20 > - there are works that depend on this series, so I would appreciate if > this series can be merged first, so that other people can have a > good basement (Radim's x2apic, David's AMD IOMMU). Though this is > based on the assumption that the basic design of this series is > workable... I understand, and it is probably safe... >=20 > - this problem will only exist for guest driver developers and should > not happen for generic users (right?), so only a small subset of > users might be affected. =2E..provided there is only little risk that some guest programs some half-backed or stale message that would be rejected prematurely. But that risk is most likely low. Jan --EKaOmnvt3v6ixV6fI2SixerJPawkXex4p Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEARECAAYFAlduoNAACgkQitSsb3rl5xTmzQCglMLKLiCXikK6zROyT6jI3HZ6 UVwAoLes7epRZjzWFBy0m7nBa2ziixkA =s1ny -----END PGP SIGNATURE----- --EKaOmnvt3v6ixV6fI2SixerJPawkXex4p--