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

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

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.

You wouldn't have a nice chart of the right type of events to
do for different types of interrupts, would you?

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