Re: [Xen-devel] Re: Losing PS/2 Interrupts

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

>>> 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).

What's so wrong with pirq_eoi_map that you're trying to avoid it
by all means?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel