From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37347) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avfgr-0006cO-BB for qemu-devel@nongnu.org; Thu, 28 Apr 2016 02:44:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1avfgn-0002hz-B0 for qemu-devel@nongnu.org; Thu, 28 Apr 2016 02:44:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56878) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avfgn-0002ha-76 for qemu-devel@nongnu.org; Thu, 28 Apr 2016 02:44:41 -0400 Date: Thu, 28 Apr 2016 14:44:34 +0800 From: Peter Xu Message-ID: <20160428064434.GC20143@pxdev.xzpeter.org> References: <1461055122-32378-1-git-send-email-peterx@redhat.com> <571DA823.1030003@web.de> <20160425071806.GF3261@pxdev.xzpeter.org> <571DC61C.9020006@web.de> <20160426073426.GD28545@pxdev.xzpeter.org> <20160426141859.GB19789@potion> <20160427072923.GG28545@pxdev.xzpeter.org> <20160427143109.GA6496@potion> <20160428060617.GB20143@pxdev.xzpeter.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160428060617.GB20143@pxdev.xzpeter.org> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Radim =?utf-8?B?S3LEjW3DocWZ?= Cc: Jan Kiszka , 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, alex.williamson@redhat.com, wexu@redhat.com On Thu, Apr 28, 2016 at 02:06:17PM +0800, Peter Xu wrote: > On Wed, Apr 27, 2016 at 04:31:13PM +0200, Radim Kr=C4=8Dm=C3=A1=C5=99 w= rote: > > >> > + */ > > >> > +static inline void > > >> > +ioapic_fix_edge_remote_irr(uint64_t *entry) > > >> > +{ > > >> > + if (*entry & IOAPIC_LVT_TRIGGER_MODE) { > > >> > + /* Level triggered interrupts, make sure remote IRR is = zero */ > > >> > + *entry &=3D ~((uint64_t)IOAPIC_LVT_REMOTE_IRR); > > >>=20 > > >> (You can just unconditionally zero it, edge doesn't care.) > > >=20 > > > Ah! I made a mistake. I suppose what I really want is: > > >=20 > > > + if (!(*entry & IOAPIC_LVT_TRIGGER_MODE)) { > > > + /* Edge-triggered interrupts, make sure remote IRR is zero= */ > > > + *entry &=3D ~((uint64_t)IOAPIC_LVT_REMOTE_IRR); > > > + } > > >=20 > > > Though both should help do the trick, I should be using this new > > > one in v5. > >=20 > > (You'd need to look at the old value for this to work.) >=20 > Yes, you are right. The problem is that, we actually has RW > permission for remote IRR bit in emulated IOAPIC. If so, I'd rather > take the original version, and unconditionally zero it, as you have > adviced (also, will fix up the comments to get them aligned). After a second thought, a better idea (though may need several more lines of codes) is to make sure the RO bits in IOAPIC entry are read-only (I mean, "real" read-only) before the above hack. I suppose this further matches with real hardware behavior. Let me send v5 directly to see the codes. Thanks, -- peterx