| 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
| ||
|
Message-ID: <20080630190132.GA6492@polina.dev.rtsoft.ru>
Date: Mon, 30 Jun 2008 23:01:32 +0400
From: Anton Vorontsov <avorontsov@...mvista.com>
To: Benjamin Herrenschmidt <benh@...nel.crashing.org>
Cc: Ingo Molnar <mingo@...e.hu>,
Bartlomiej Zolnierkiewicz <bzolnier@...il.com>,
Alan Cox <alan@...rguk.ukuu.org.uk>,
Sergei Shtylyov <sshtylyov@...mvista.com>,
linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
Steven Rostedt <rostedt@...dmis.org>,
Daniel Walker <dwalker@...sta.com>
Subject: [RT] MPIC edge sensitive issues with hardirq preemption (was: Re:
[PATCH v2 -rt] ide: workaround buggy hardware issues with
preemptable hardirqs)
On Mon, Jun 30, 2008 at 09:26:14AM +1000, Benjamin Herrenschmidt wrote:
> On Sat, 2008-06-28 at 04:54 +0400, Anton Vorontsov wrote:
> > IDE interrupt handler relies on the fact that, if necessary, hardirqs
> > will re-trigger on ISR exit. The assumption is valid for level sensitive
> > interrupts.
> >
> > But some hardware (namely ULi M5228 in the ULi M1575 "Super South Brige")
> > behaves in a strange way: it asserts interrupts as edge sensitive. And
> > because preemptable IRQ handler disables PIC's interrupt, PIC will likely
> > miss it.
>
> Don't we replay edge IRQs that happen while soft-disabled ? Could be a
> bug in your PIC code not to do so...
Thanks for the idea. With hardirq preempton, fasteoi path does not replay
edge interrupts indeed (at least for MPIC).
Here how I tested this: I have external interrupt connected to the button
on this board, thus I registered irq handler which is doing exactly this
(irq is edge sensitive):
printk("handled\n"); mdelay(2000);
Without hardirq preemption: pressing button twice prints two messages.
With hardirq preemption: pressing button twice prints just one message.
This happens because:
- irq has come;
- fasteoi handler mask()s it, and wakes up the thread;
- thread calls irq handler;
- another IRQ is coming, but MPIC's IRQ is still masked and MPIC does not
see it.
- thread unmasks the IRQ, second IRQ got lost.
But how this could be a bug in the PIC code? IMO this is a bug in the
kernel/irq code, since it assumes that fasteoi PIC will retrigger masked
edge sources... This isn't true for at least MPIC. To make this work for
all fasteoi PICs, we should mask edge sensitive interrupts very very
carefully.
Also...
I found that I completely messed with MPIC's sense types: my brain was
jammed with the idea that MPIC irq type 0 is low level sensitive, but the
true thing is that it is rising edge sensitive. (Ah, I know where I got
confused, type 0 is active-low for ISA PICs).
So in all my previous emails I was wrong when I was saying "mpic is
programmed to low level sensitive". It was programmed for rising edge
sensitive. An all my further reasonings were flawed because of this.
Re-programming MPIC to high level sensitive also makes IDE work. But
this doesn't mean that IRQ code is correct.
So, in the end, this isn't IDE issue... Much thanks to everyone for
ideas and help... instead of one bug, it seems I've found two. :-)
None in the IDE code.
Following patch fixes MPIC edge sensitive interrupts processing with
hardirqs preemption.
We're still using handle_fasteoi_irq, but passing control to
thread_edge_irq if interrupt is edge sensitive. Otherwise usual fasteoi
path handles the IRQ.
Plys, we're masking edge interrupt _and_ marking it as pending and masked
only if we're already handling one (exactly as we do with !preemptable
hardirqs). And the trick is that thread_edge_irq() will unmask it before
executing handle_IRQ_event.
So here is the patch, how does it look?
(quite weird that I have to pass irq trigger flags to desc->status,
but really, there is no other way. it seems safe though, because
all irqactions should conform only to one trigger type).
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 9aa6b24..6c9f4ae 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -134,6 +134,10 @@ int set_irq_type(unsigned int irq, unsigned int type)
if (desc->chip->set_type) {
spin_lock_irqsave(&desc->lock, flags);
ret = desc->chip->set_type(irq, type);
+ if (!ret) {
+ desc->status &= ~IRQ_TYPE_SENSE_MASK;
+ desc->status |= type & IRQ_TYPE_SENSE_MASK;
+ }
spin_unlock_irqrestore(&desc->lock, flags);
}
return ret;
@@ -430,7 +434,7 @@ handle_fasteoi_irq(unsigned int irq, struct irq_desc *desc)
action = desc->action;
if (unlikely(!action || (desc->status & (IRQ_INPROGRESS |
IRQ_DISABLED)))) {
- desc->status |= IRQ_PENDING;
+ desc->status |= IRQ_PENDING | IRQ_MASKED;
if (desc->chip->mask)
desc->chip->mask(irq);
goto out;
@@ -439,9 +443,10 @@ handle_fasteoi_irq(unsigned int irq, struct irq_desc *desc)
desc->status |= IRQ_INPROGRESS;
/*
* In the threaded case we fall back to a mask+eoi sequence:
+ * Though, we don't mask edge interrupts, thread may lose it then.
*/
if (redirect_hardirq(desc)) {
- if (desc->chip->mask)
+ if (!(desc->status & IRQ_TYPE_EDGE_BOTH) && desc->chip->mask)
desc->chip->mask(irq);
goto out;
}
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index c30aa1e..ffeb87f 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -385,10 +385,11 @@ int setup_irq(unsigned int irq, struct irqaction *new)
/* Setup the type (level, edge polarity) if configured: */
if (new->flags & IRQF_TRIGGER_MASK) {
- if (desc->chip && desc->chip->set_type)
+ if (desc->chip && desc->chip->set_type) {
desc->chip->set_type(irq,
new->flags & IRQF_TRIGGER_MASK);
- else
+ desc->status |= new->flags & IRQF_TRIGGER_MASK;
+ } else
/*
* IRQF_TRIGGER_* but the PIC does not support
* multiple flow-types?
@@ -772,9 +773,12 @@ static void do_hardirq(struct irq_desc *desc)
thread_simple_irq(desc);
else if (desc->handle_irq == handle_level_irq)
thread_level_irq(desc);
- else if (desc->handle_irq == handle_fasteoi_irq)
- thread_fasteoi_irq(desc);
- else if (desc->handle_irq == handle_edge_irq)
+ else if (desc->handle_irq == handle_fasteoi_irq) {
+ if (desc->status & IRQ_TYPE_EDGE_BOTH)
+ thread_edge_irq(desc);
+ else
+ thread_fasteoi_irq(desc);
+ } else if (desc->handle_irq == handle_edge_irq)
thread_edge_irq(desc);
else
thread_do_irq(desc);
--
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/
Powered by blists - more mailing lists