linux-kernel - Re: [patch 3/3] genirq: mark io_apic level interrupts to avoid resend

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: <1187079032.12828.161.camel@chaos>
Date:	Tue, 14 Aug 2007 10:10:32 +0200
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Jarek Poplawski <jarkao2@...pl>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...e.hu>,
	Stable Team <stable@...nel.org>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Jean-Baptiste Vignaud <vignaud@...dmail.fr>,
	"marcin.slusarz" <marcin.slusarz@...il.com>
Subject: Re: [patch 3/3] genirq: mark io_apic level interrupts to avoid
	resend

On Tue, 2007-08-14 at 09:12 +0200, Jarek Poplawski wrote:
> > No, the point is that the resend is suppressed for all interrupts which
> > are marked with IRQ_LEVEL:
> > 
> >         /*
> >          * We do not resend level type interrupts. Level type
> >          * interrupts are resent by hardware when they are still
> >          * active.
> >          */
> >         if ((status & (IRQ_LEVEL | IRQ_PENDING | IRQ_REPLAY)) == IRQ_PENDING) {
> > 	....
> > 
> > This is not witchcraft, this is how the hardware works.
> 
> Sorry! It's probably something with my English: I like this flag very
> much! But I simply can't find where this flag is set for any irq using
> handle_level_irq, or, otherwise, can't understand why it's not set
> (because in this case I don't think not setting IRQ_PENDING by the
> handler should be enough).

handle_level_irq() does not set the PENDING bit on delayed disable.

> > 
> It's different because e.g. for x86_64 fasteoi level type irqs were
> masked during disable_irq, so there was very small probability any
> irq were skipped, plus the state of io_apic was different from this
> point (regarding this irq). Now it's for sure many interrupts could
> be 'missing'.

No. Let me explain:

Before delayed disable:

	irq_disable(); /* Mask in hardware */
	....
	-> Interrupt line is asserted. No interrupt due to hardware mask
	....
	irq_enable();  /* Unmask */
	
	-> When interrupt line is still active, then the interrupt is
	   invoked. Otherwise nothing happens

Delayed disable (with level fix):

	irq_disable(); /* Do not mask in hardware */
	....
	-> Interrupt line is asserted.
	---> interrupt handler is invoked: interrupt is masked in
	     hardware
	....
	irq_enable();  /* Unmask */
	
	-> When interrupt line is still active, then the interrupt is
	   invoked. Otherwise nothing happens

Can we agree, that this is the same ?

> BTW, of course, my knowledge of this is very limited, but I wonder
> about these level type irqs used e.g. by apics. 'Normal' chips hold
> some data until it's read by a driver, so there is something more
> needed than an ack by io_apic. But isn't there any possibility
> some level type irqs possible for IPIs or local interrupts (82489DX?)
> could be missing here? Are we sure there is no hardware using level
> type irqs in a similar way (drop after acking)?

Level type interrupts _are_ active as long as the hardware pin of the
interrupt line is driven by a device to the active level. 

When the hardware pin is kept at the active level by a device, the ACK
of the interrupt controller does not change the interrupt line of the
device. The interrupt comes back again immediately.

Edge type interrupts are different:

The interrupt is only triggered on the transition of the hardware pin
from inactive to active level. So the above scheme looks like:

Before delayed disable:

	irq_disable(); /* Mask in hardware */
	....
	-> Interrupt line is asserted. No interrupt due to hardware mask
	....
	irq_enable();  /* Unmask */
	
	-> The now unmasked interrupt becomes active and the interrupt
	   handler is invoked.

Delayed disable:

	irq_disable(); /* Do not mask in hardware */
	....
	-> Interrupt line is asserted.
	---> interrupt handler is invoked: interrupt is masked in
	     hardware, acknowledged and marked PENDING
	....
	irq_enable();  /* Unmask */
	
	-> On unmask the hardware does not invoke the interrupt, because
	   we acknowledged it above. Here we need to use the
	   hard-/soft-ware resend mechanism, otherwise the interrupt
	   would be lost

> > > there is no reason to endanger even a small number of users/admins
> > > for stresses like this, done to Marcin or Jean-Baptiste, when it's
> > > possible to do this safer without much changes.
> > 
> > Safer in what way ? 
> 
> Because, if there were a visible config option or kernel parameter
> e.g.  with a comment like "legacy level irq handling - obsolete",
> some people would be happy when they find it's useful for them, and
> you would know about the problem much sooner, as well.

Well, there is nothing legacy. level type interrupts do not need the
resend mechanism at all. This misfeature was introduced with the delayed
disable and went unnoticed until now.

	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/