Re: [Xen-devel] Re: [PATCH] xen: do not clear and mask evtchns in __xen_evtchn_do_upcall

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

On Tue, 5 Apr 2011, Ian Campbell wrote:
> Since this patch revolves around the interaction with the irq core I
> think it would be worth running it by tglx as well as xen-devel for the
> next iteration.
> 

Yes, I'll do that.


> Does something about the switch of handler type fix the issue with
> drivers which call disable_irq() in their interrupt handler and leave it
> disabled until a later time? (e.g. the userspace evtchn driver). I think
> I know why this is the case but it would be useful to mention in the
> commit message.

Yep, I added a note in the commit message:

"it fixes a problem occurring when a driver calls disable_irq() in its
handler: the old code was unconditionally unmasking the evtchn even if
the irq is disabled when irq_eoi was called"


> On Mon, 2011-04-04 at 17:46 +0100, Stefano Stabellini wrote:
> > 
> >     - it uses these handlers in the same way Linux would use them: it let
> >     Linux mask\unmask and ack the irq when Linux want to mask\unmask and ack
> >     the irq;
> 
> This code is in Linux, perhaps you mean "the same way native code would
> use them" or "with the semantics which the generic code defines" or
> something along those lines.

good point


> > +       pirq_query_unmask(irq);
> > +       /* we try to follow the same convention as Linux on native:
> 
> That's basically a coincidence, isn't it?
> 
> What we are actually trying to do is use the handler with the
> appropriate semantics for the type of interrupt, e.g. event channels are
> naturally edge triggered but some event channels can be bound to pirqs
> with configurations that require an EOI and therefore have a level of
> levelness about them.

I completely rewrote the paragraph, adding many more details, including
the Xen side of the interface.

> 
> > +        * handle_edge_irq for edge irqs and handle_fasteoi_irq for level
> > +        * irqs
> 
> should probably mention the relationship between pirq_needs_eoi=>level
> triggering.

done.


> Is there any relationship between pirq_needs_eoi and the trigger value
> passed to PHYSDEVOP_setup_gsi at setup time?

PHYSDEVOP_setup_gsi tells Xen the trigger value of the physical
interrupt, depending on this value Xen will handle the interrupt
differently. Besides Xen used to set the need_eoi flag of the
corresponding irq depending on the real trigger value of the underlying
physical interrupt, but it doesn't anymore.