From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: Re: Losing PS/2 Interrupts Date: Tue, 24 May 2011 16:40:38 +0100 Message-ID: <4DDBED9602000078000433EA@vpn.id2.novell.com> References: <20110520175044.GA30367@dumpdata.com> <5D477258-8216-48BD-8A93-186E044118B9@gmail.com> <4DDA366E0200007800042C71@vpn.id2.novell.com> <1D3BFCDD-9D53-48BA-9ECD-D009AD535C2B@gmail.com> <04C6DFB0-08C8-4A8B-968F-FFE712BCABA1@gmail.com> <4DDB916F0200007800043155@vpn.id2.novell.com> <4DDBBF98020000780004328A@vpn.id2.novell.com> <20110524125816.GA10017@dumpdata.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <20110524125816.GA10017@dumpdata.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Konrad Rzeszutek Wilk Cc: Simon Graham , "xen-devel@lists.xensource.com" , Thomas Goetz , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org >>> On 24.05.11 at 14:58, Konrad Rzeszutek Wilk = wrote: > On Tue, May 24, 2011 at 01:24:24PM +0100, Jan Beulich wrote: >> >>> On 24.05.11 at 13:04, Stefano Stabellini >> wrote: >> > On Tue, 24 May 2011, Jan Beulich wrote: >> >> > Relevant code snippets included below: >> >> >=20 >> >> > if (pirq_needs_eoi(irq)) { >> >> > printk(KERN_ERR "%s: irq %d handle_fasteoi_irq\n",= =20 >> >> > __FUNCTION__, irq); >> >> > set_irq_chip_and_handler_name(irq, &xen_pirq_chip, >> >> > handle_fasteoi_irq, name); >> >> > } else { >> >> > printk(KERN_ERR "%s: irq %d handle_edge_irq\n",=20 >> >> > __FUNCTION__, irq); >> >> > set_irq_chip_and_handler_name(irq, &xen_pirq_chip, >> >> > handle_edge_irq, name); >> >> > } >> >>=20 >> >> Now this, imo, is a very good reason to not use handle_edge_irq() >> >> at all, and instead use the prior control flow (masking and clearing >> >> the event channel up front in do_upcall()) with only fasteoi = (leaving >> >> aside per-CPU ones). >> >>=20 >> >=20 >> > Actually I think it is a good reason to fix pirq_needs_eoi that = shouldn't >> > return unconditionally yes if dom0 doesn't support pirq_eoi_map. >> > The comment in Xen says: >> >=20 >> > /* >> > * Even edge-triggered or message-based IRQs can need masking = from >> > * time to time. If teh guest is not dynamically checking for = this >> > * via the new pirq_eoi_map mechanism, it must conservatively = always >> > * execute the EOI hypercall. In practice, this only really makes = a >> > * difference for maskable MSI sources, and if those are = supported >> > * then dom0 is probably modern anyway. >> > */ >> >=20 >> > Considering that I would rather avoid supporting pirq_eoi_map and we = are >> > talking about edge triggered interrupts, do you think it would be = safe >> > for me to send a patch to xen to change this behaviour? >> > Shouldn't we set XENIRQSTAT_needs_eoi only for level triggered >> > interrupts (and maybe maskable MSI sources)? >>=20 >> Only if you can prove that the very first part of that comment is >> incorrect (in including "edge-triggered" and ignoring whether MSI >> sources are maskable). And your Linux side code would then still >> be incorrect for maskable MSIs (you'd continue to handle them >> as fasteoi with no up front clearing/masking while that is necessary >> as Thomas' report made clear). >=20 > I believe we handle MSI's as 'handle_edge_chip' (xen_bind_pirq_msi_to_irq= ) > irregardless of the above hypercall. So how do you issue the possibly necessary EOI call then? > You wouldn't have a nice chart of the right type of events to > do for different types of interrupts, would you? No, sorry - all I usually look at are the three more or less different implementations. Jan >>=20 >> What's so wrong with pirq_eoi_map that you're trying to avoid it >> by all means? >=20 > My recollection is that we had a hard time trying to work it in with the > tglr'x rewrite of the IRQ code and not enough understanding of this = (at=20 > least > on my side). Any help here would be appreciated.