>>> On 24.05.11 at 14:58, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> wrote:
> On Tue, May 24, 2011 at 01:24:24PM +0100, Jan Beulich wrote:
>> >>> On 24.05.11 at 13:04, Stefano Stabellini
>> >>> <stefano.stabellini@xxxxxxxxxxxxx>
>> wrote:
>> > On Tue, 24 May 2011, Jan Beulich wrote:
>> >> > Relevant code snippets included below:
>> >> >
>> >> > if (pirq_needs_eoi(irq)) {
>> >> > printk(KERN_ERR "%s: irq %d handle_fasteoi_irq\n",
>> >> > __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",
>> >> > __FUNCTION__, irq);
>> >> > set_irq_chip_and_handler_name(irq, &xen_pirq_chip,
>> >> > handle_edge_irq, name);
>> >> > }
>> >>
>> >> 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).
>> >>
>> >
>> > 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:
>> >
>> > /*
>> > * 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.
>> > */
>> >
>> > 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)?
>>
>> 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).
>
> 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
>>
>> What's so wrong with pirq_eoi_map that you're trying to avoid it
>> by all means?
>
> 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
> least
> on my side). Any help here would be appreciated.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|