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:37:46 +0100 Message-ID: <4DDBECEA02000078000433E7@vpn.id2.novell.com> References: <3E2050B5-59DC-4E4F-9C8D-8C04A6B465EB@gmail.com> <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> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: 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: Stefano Stabellini Cc: Simon Graham , "xen-devel@lists.xensource.com" , Thomas Goetz , Konrad Rzeszutek Rzeszutek Wilk List-Id: xen-devel@lists.xenproject.org >>> On 24.05.11 at 15:52, Stefano Stabellini wrote: > On Tue, 24 May 2011, Jan Beulich wrote: >> > 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 >> What's so wrong with pirq_eoi_map that you're trying to avoid it >> by all means? > =20 > The main issue is that if pirq_eoi_map is enabled PHYSDEVOP_eoi > automatically unmask the event channel. > There isn't even a way to specify if we want the unmask to be done or > not, it just does it. I can't think of situations where this would be a problem. It certainly never has been in our kernels. > I also think that it is a violation of the interface, see this comment > from xen/include/public/xen.h: >=20 > * Event channels are addressed by a "port index". Each channel is > * associated with two bits of information: > * 1. PENDING -- notifies the domain that there is a pending = notification > * to be processed. This bit is cleared by the guest. > * 2. MASK -- if this bit is clear then a 0->1 transition of = PENDING > * will cause an asynchronous upcall to be scheduled. This bit = is only > --> * updated by the guest. It is read-only within Xen. If a = channel Yeah, that should have been updated when the new feature got introduced. But I'm sure you know how things go wrt documentation (especially when a comment like this sits far away from any code touched during the implementation of something new)... Anyway - if a kernel is using the new feature, it clearly ought to be aware that the bitmap then no longer is read-only to the hypervisor. Jan > * becomes pending while the channel is masked then the 'edge' = is lost > * (i.e., when the channel is unmasked, the guest must manually = handle > * pending notifications as no upcall will be scheduled by Xen).