Messages in this thread | | | Date | Thu, 17 Jul 2008 17:58:26 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: PCI: MSI interrupts masked using prohibited method |
| |
On Thu, 17 Jul 2008, Matthew Wilcox wrote: > > The device generates an MSI interrupt. > Some magic happens in assembly, and the processor ends up in do_IRQ() > which calls generic_handle_irq(). Because it's an MSI IRQ, > handle_edge_irq() is called instead of __do_IRQ().
__do_IRQ() is the old all-in-one handler which is not called on platforms which have GENERIC_HARDIRQS set. You can safely ignore what __do_IRQ() does.
> There are two opportunities for calling the ->mask() here, one is: > > if (unlikely((desc->status & (IRQ_INPROGRESS | IRQ_DISABLED)) || > !desc->action)) { > desc->status |= (IRQ_PENDING | IRQ_MASKED); > mask_ack_irq(desc, irq); > > The other is: > > if (unlikely(!action)) { > desc->chip->mask(irq); > > This is all in generic code which knows nothing about any device-specific > method of masking interrupts from the chip.
Right and it is not supposed to know anything about the hardware details at all. The per irq setting can provide NOOP functions for all the mask/mask_ack/unmask things when thats the right way for the particular irq line.
> > Regardless, doesn't __do_IRQ() handle this already anyway?
That's irrelevant. All the interrupts are handled via irq_desc[irq].handle_irq() when GENERIC_HARDIRQS is set.
> This is a good thought, let's follow it through. What if we simply make > ->mask a no-op for devices which don't support mask bits?
Yep. You can also use fasteoi_handler, which just calls ->eoi() after the handler.
> MSIs are 'edge triggered' interrupts. They're sent once and then > forgotten by the hardware (as opposed to level triggered which will > continue to be triggered until deasserted). > > The first time through (assuming ->action is non-NULL ...), we won't > try to mask the irq. The second time through, IRQ_INPROGRESS will be set, > so we try to mask_ack_irq(). > > How about we simply don't ack the irq at this point? That should prevent > it being triggered again, right? > > Working on a patch to do this now ...
You want to use the fasteoi_handler.
Thanks, tglx
| |