[Xen-devel] Re: Using handle_fasteoi_irq for pirqs

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

 >>> On 03.09.10 at 20:46, Jeremy Fitzhardinge <jeremy@xxxxxxxx> wrote:
> Using .startup/.shutdown for enable/disable seems very heavyweight.  Do
> we really want to be rebinding the pirq each time?  Isn't unmask/masking
> the event channel sufficient?

Depends - the original (2.6.18) implementation only makes enable_pirq()
a conditional startup (and really startup_pirq() is conditional too), while
disable_pirq() does nothing at all. While forward porting, considering
the contexts in which ->disable() gets called (namely note_interrupt())
and after initially having had no ->enable()/->disable() methods at all
(for default_enable() calling ->unmask() anyway, and default_disable()
being a no-op as much as disable_pirq() was), I got to the conclusion
that it might be better to do a full shutdown/startup, since it isn't
known whether a disable is permanent or just temporary.

Now part of the question whether this is actually a good choice is
why default_disable() doesn't mask the affected IRQ - likely because
IRQ_DISABLED is checked for and handled accordingly in all non-
trivial flow handlers.

The other aspect is that with the (original) switch to use
handle_level_irq() we noticed at some point that the disabling of
bad IRQs (where e.g. storms are noticed) didn't work anymore,
due to that logic sitting in ->end(), which doesn't usually get
called at all (nor does any other method except ->unmask() for
the level case). Right now I don't really remember whether making
->disable() an alias of shutdown wasn't just a (failed iirc) attempt
at getting Xen to know of the need to shut down such a bad IRQ.
After the switch to fasteoi this logic should now be working again
independent of what ->disable() does, so I will have to consider
to un-alias disable_pirq() and shutdown_pirq() again.

> At the moment my xen_evtchn_do_upcall() is masking and clearing the
> event channel before calling into generic_handle_irq_desc(), which will
> call handle_fasteoi_irq fairly directly.  That runs straight through and
> the priq_chip's eoi just does an EOI on the pirq if Xen says it needs one.
> 
> But apparently this isn't enough.  Is there anything else I should be doing?

I can't see anything, and our kernel also doesn't.

> (I just implemented PHYSDEVOP_pirq_eoi_gmfn method of getting the pirq
> eoi flags, but I haven't tested it yet.  I'm also not really sure what
> the advantage of it is.)

This is for you to avoid the EOI hypercall if it would be a no-op in
Xen anyway.

Jan


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