linux-kernel - Re: PCI: MSI interrupts masked using prohibited method

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.1.10.0807171745270.4316@apollo.tec.linutronix.de>
Date:	Thu, 17 Jul 2008 17:58:26 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Matthew Wilcox <matthew@....cx>
cc:	David Vrabel <david.vrabel@....com>,
	Jesse Barnes <jbarnes@...tuousgeek.org>,
	Kernel development list <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...e.hu>
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/