diff options
author | Grant Grundler <grundler@parisc-linux.org> | 2005-11-17 16:26:20 -0500 |
---|---|---|
committer | Kyle McMartin <kyle@parisc-linux.org> | 2005-11-17 16:26:20 -0500 |
commit | 3f902886a81c6d4e6c399760936b645b5c7a7342 (patch) | |
tree | 9e46c6f799390497784f2048a2294b3967bb857e /arch | |
parent | 9a8b4584065dd241d6c2bf818e349986bd900b8e (diff) |
[PARISC] Disable nesting of interrupts
Disable nesting of interrupts - still has holes
The offending sequence starts out like this:
1) take external interrupt
2) set_eiem() to only allow TIMER_IRQ; local interrupts still disabled
3) read the EIRR to get a "list" of pending interrupts
4) clear EIRR of pending interrupts we intend to handle
5) call __do_IRQ() to handle IRQ.
6) handle_IRQ_event() enables local interrupts (I-Bit)
7) take a timer interrupt
8) read EIRR to get a new list of pending interrupts
9) clear EIRR of pending interrupts we just read
10) handle pending interrupts found in (8)
11) set_eiem(cpu_eiem) and return
[ TROUBLE! all enabled CPU IRQs are unmasked. }
12) handle remaining interrupts pending from (3)
e.g. call __do_IRQ() -> handle_IRQ_event()..etc
[ TROUBLE! call to handle_IRQ_event() can now enable *any* IRQ. }
13) set_eiem(cpu_eiem) and return
The problem is we now get into ugly race conditions with Timer and IPI
interrupts at this point. I'm not exactly sure what happens when
things go wrong (perhaps nest calls to IPI or timer interrupt?).
But I'm certain it's not good.
This sequence will break sooner if (10) would accidentally leave
interrupts enabled.
I'm pretty sure the right answer is now to make cpu_eiem
a per CPU variable since all external interrupts on parisc
are per CPU. This means we will NOT need to send an IPI to
every CPU in the system when enabling or disabling an IRQ
since only one CPU needs to change it's EIEM.
Thanks to James Bottomley for (once again) pointing out the problem.
Signed-off-by: Grant Grundler <grundler@parisc-linux.org>
Signed-off-by: Kyle McMartin <kyle@parisc-linux.org>
Diffstat (limited to 'arch')
-rw-r--r-- | arch/parisc/kernel/irq.c | 13 |
1 files changed, 6 insertions, 7 deletions
diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c index f7ae2bcd49a5..21a9c5ad580b 100644 --- a/arch/parisc/kernel/irq.c +++ b/arch/parisc/kernel/irq.c | |||
@@ -250,10 +250,11 @@ void do_cpu_irq_mask(struct pt_regs *regs) | |||
250 | irq_enter(); | 250 | irq_enter(); |
251 | 251 | ||
252 | /* | 252 | /* |
253 | * Only allow interrupt processing to be interrupted by the | 253 | * Don't allow TIMER or IPI nested interrupts. |
254 | * timer tick | 254 | * Allowing any single interrupt to nest can lead to that CPU |
255 | * handling interrupts with all enabled interrupts unmasked. | ||
255 | */ | 256 | */ |
256 | set_eiem(EIEM_MASK(TIMER_IRQ)); | 257 | set_eiem(0UL); |
257 | 258 | ||
258 | /* 1) only process IRQs that are enabled/unmasked (cpu_eiem) | 259 | /* 1) only process IRQs that are enabled/unmasked (cpu_eiem) |
259 | * 2) We loop here on EIRR contents in order to avoid | 260 | * 2) We loop here on EIRR contents in order to avoid |
@@ -267,9 +268,6 @@ void do_cpu_irq_mask(struct pt_regs *regs) | |||
267 | if (!eirr_val) | 268 | if (!eirr_val) |
268 | break; | 269 | break; |
269 | 270 | ||
270 | if (eirr_val & EIEM_MASK(TIMER_IRQ)) | ||
271 | set_eiem(0); | ||
272 | |||
273 | mtctl(eirr_val, 23); /* reset bits we are going to process */ | 271 | mtctl(eirr_val, 23); /* reset bits we are going to process */ |
274 | 272 | ||
275 | /* Work our way from MSb to LSb...same order we alloc EIRs */ | 273 | /* Work our way from MSb to LSb...same order we alloc EIRs */ |
@@ -283,7 +281,8 @@ void do_cpu_irq_mask(struct pt_regs *regs) | |||
283 | __do_IRQ(irq, regs); | 281 | __do_IRQ(irq, regs); |
284 | } | 282 | } |
285 | } | 283 | } |
286 | set_eiem(cpu_eiem); | 284 | |
285 | set_eiem(cpu_eiem); /* restore original mask */ | ||
287 | irq_exit(); | 286 | irq_exit(); |
288 | } | 287 | } |
289 | 288 | ||