OpenBSD/386 interrupt masking problems?

["J. J. Farrell" <jjf@abaqos.com>]

I was browsing through the kernel code and came upon what appears
to be a problem in 386 interrupt masking. I've checked on a running
system with ddb, and this seems to confirm my analysis.

The routine intr_calculatemasks() in arch/i386/isa/isa_machdep.c
determines which IRQs use which IPLs, but only does this for IPLs
0 through 4. 0 and 7 are later set to fixed values, leaving 5
(IPL_AUDIO) and 6 (IPL_CLOCK) at 0. The mask for IPL_AUDIO gets
the new mask for IPL_IMP ORed into it, but never gets any bits
removed when handlers are unregistered. The mask for IPL_CLOCK
is left at 0.

This has the following consequences:

- splaudio() may or may not block interrupts at IPL_AUDIO
  depending on how the other IPL_AUDIO sources are distributed
  over the IRQs. Similarly, the execution of an interrupt routine
  at IPL_AUDIO may or may not block other interrupts at the same
  level.

- splclock() has no effect on the CPL - in particular, it doesn't
  block clock interrupts

- the statistics clock and normal clock routines can interrupt
  one-another. I haven't thought it through, but I wonder if this
  is perhaps intentional.


The fix for this is trivial:

--- isa_machdep.c.1.38	Wed Oct  3 16:16:45 2001
+++ isa_machdep.c.1.38.jjf1	Wed Oct  3 16:41:58 2001
@@ -311,5 +311,5 @@
 
 	/* Then figure out which IRQs use each level. */
-	for (level = 0; level < 5; level++) {
+	for (level = IPL_NONE+1; level < IPL_HIGH; level++) {
 		register int irqs = 0;
 		for (irq = 0; irq < ICU_LEN; irq++)

(IPL_NONE and IPL_HIGH omitted from the loop since they are
handled explicitly further down.)

If it is intentional that the two clock interrupts should be able
to interrupt one-another, this functionality can be reintroduced
by also applying the following:

--- isa_machdep.c.1.38	Wed Oct  3 16:16:45 2001
+++ isa_machdep.c.1.38.jjf2	Wed Oct  3 16:49:42 2001
@@ -343,5 +343,6 @@
 		register int irqs = 1 << irq;
 		for (q = intrhand[irq]; q; q = q->ih_next)
-			irqs |= imask[q->ih_level];
+			if (q->ih_level != IPL_CLOCK)
+				irqs |= imask[q->ih_level];
 		intrmask[irq] = irqs | SIR_ALLMASK;
 	}

This allows the routines to interrupt one-another, but makes
splclock() effective.


Having said all that, it's quite possible that I'm misinterpreting
something - this is the first time I've looked at a BSD kernel
and I've spent most of my life avoiding knowing about the PC
"architecture". (I'm an old 68k/SPARC SysV kernel hacker, but
please don't hold it against me.) My apologies if I'm wasting
people's time with this, but I'd be grateful if you could let me
know where I'm wrong.

Regards,
		jjf