From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: irq_guest_eoi_timer interaction with MSI Date: Fri, 14 Nov 2008 09:28:56 +0000 Message-ID: <491D52E8.76E4.0078.0@novell.com> References: <491C672A.76E4.0078.0@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: Keir Fraser Cc: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org >>> Keir Fraser 13.11.08 17:50 >>> >On 13/11/08 16:43, "Jan Beulich" wrote: > >> Up to now, MSI didn't require an EOI, and devices that support masking = (in >> particular all MSI-X ones) wouldn't generally require an EOI even with = the >> patch send earlier. What you propose would make them all require an EOI >> all of the sudden, despite them needing hypervisor assistance only when >> the interrupt got masked. >>=20 >>> Also I'll add we currently do a hypercall for every level-triggered = IO-APIC >>> IRQ, which was really all we supported until recently. Seemed to work = well >>> enough performance-wise in that case. > >So we'd add a pirq-indexed bitmap to mitigate that. Whether we use >PHYSDEVOP_irq_eoi or EVTCHNOP_unmask, we need a new shared-memory bitmap, >right? Might as well use irq_eoi and index by pirq, I'd say. Hmm, I'm still not convinced: With what you propose, it's unclear to me = who would when clear the bit in that bitmap for the 'temporarily masked' case. Anyway, unless you get to implement your version earlier (and thus convince me that things will work out correctly), I'll try to get = implemented what I would think should be appropriate here once I find time to do so. The other concern I have (as a consequence of the NR_IRQS related discussion) is that adding a NR_IRQS (or NR_PIRQS) indexed bitmap to shared_info seems problematic wrt forward compatibility: You hinted at making the value build-time configurable, and even if it remained a manifest constant, any value (you suggested 1024) would only call for being too small at some future point in time (as will unavoidably be the case for the number of event channels). Jan