hybrid: interrupts to userspace, and a crashed driver that stays crashed

docs/plan/hybrid.md mechanism 3. A process asks for a device's interrupts with
PCIIOC_IRQ and then reads them, which is how Linux's uio does it and for the
same reason: a descriptor composes with poll, and a driver that stops reading
falls behind rather than losing events. read() returns a count rather than a
flag, so a driver that was busy sees it missed three and acts once.

Proven against an e1000e by asserting a cause through its ICS register and
reading the interrupt back out in userspace:

    interrupts routed here (line 10)
    interrupt delivered, count = 1
    ICR = 0x80000004

0x80000004 is the cause the test set plus INT_ASSERTED -- the device's own
register, read by userspace as its acknowledgement.

**The masking is the design, not a detail.** A PCI line is level-triggered and
stays asserted until the cause is cleared at the device, which is a different
register on every chip and so is something only the driver can do. The kernel
masks the line on arrival and userspace's write() unmasks it. Signalling EOI on
a still-asserted line re-enters the handler immediately and for ever; this
kernel did exactly that by accident at 72,000 interrupts a second, and what it
looked like from outside was a clock running seven times slow (§13).

Which decides what happens when a driver dies mid-interrupt: the line is left
**masked**. The instinct is to unmask -- do not leave the device dead -- and it
is wrong, because unmasking an asserted line with no handler is a crashed
driver taking the machine down with it. Masked is safe and not permanent: the
next PCIIOC_IRQ arms it. Both halves verified by crashing a driver on purpose
and then starting another one.

A line already carrying kernel handlers is refused outright. Masking is
per-line, so handing out a shared one would silence whichever kernel driver is
also there, with nothing logged. MSI is the real answer and pci_msi_enable() is
already in the tree waiting for a caller.

Also fixed, found by running the crash twice: DMA buffers were never returned
when a process exited. The arena is 4 MB and contiguous, so a driver that
restarts a few times exhausted it -- "out of DMA memory, 3584 free" -- which is
a poor property for an interface whose whole promise is that a driver can crash
and come back.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQ7Qx8E1q1M1bbCq8QLHS2

cmd/pcitest/main.c

@@ -13,6 +13,10 @@
 //
 //   pcitest            list the bus
 //   pcitest <index>    claim that device, map BAR0, read it, allocate DMA
+//   pcitest <index> x  the same, then assert an interrupt and exit WITHOUT
+//                      acking it -- which is what a driver that crashed looks
+//                      like from the kernel's side, and the case that used to
+//                      leave a level-triggered line asserted with no handler
 
 #include "types.h"
 #include "user.h"
@@ -89,7 +93,7 @@ main(int argc, char *argv[])
     }
 
     if (argc == 1) {
-        puts("  #  bdf       vendor:device  class        driver\n");
+        puts("  #  bdf       vendor:device  irq  class        driver\n");
         for (int i = 0; i < count; i++) {
             struct pciuser_info in;
             in.index = (unsigned int)i;
@@ -98,7 +102,8 @@ main(int argc, char *argv[])
             puts("  "); putn(i); puts("  ");
             puthex(in.bus, 2); puts(":"); puthex(in.slot, 2);
             puts("."); puthex(in.func, 1); puts("   ");
-            puthex(in.vendor, 4); puts(":"); puthex(in.device, 4); puts("     ");
+            puthex(in.vendor, 4); puts(":"); puthex(in.device, 4); puts("   ");
+            if (in.irq == 0xFF) puts(" -  "); else { putn(in.irq); puts("   "); }
             puts(class_name(in.class));
             puts("      ");
             puts(in.driver[0] ? in.driver : "-");
@@ -191,6 +196,52 @@ main(int argc, char *argv[])
     else
         puts("  DMA buffer did not read back correctly\n");
 
-    puts("\nmechanisms 2 and 4 work: a driver could live out here.\n");
+    // Mechanism 3. Ask for the device's interrupts, then make it interrupt.
+    //
+    // The e1000 family has an Interrupt Cause Set register at 0x00C8: writing
+    // a cause bit asserts the line exactly as the hardware would, which is the
+    // only way to test interrupt delivery without traffic arriving from
+    // somewhere. Reading ICR at 0x00C0 clears it, which is the ack the kernel
+    // cannot do for us.
+    if (in.vendor != 0x8086 || in.class != 0x02) {
+        puts("\nmechanisms 2 and 4 work: a driver could live out here.\n");
+        puts("(interrupts untested -- that part knows only Intel NICs)\n");
+        exit();
+    }
+
+    if (ioctl(fd, PCIIOC_IRQ, &idx) < 0) {
+        puts("  interrupts refused -- the line is shared with a kernel driver\n");
+        puts("\nmechanisms 2 and 4 work: a driver could live out here.\n");
+        exit();
+    }
+    puts("  interrupts routed here (line "); putn(in.irq); puts(")\n");
+
+    volatile unsigned int *IMS = &regs[0x00D0 / 4];   // interrupt mask set
+    volatile unsigned int *ICS = &regs[0x00C8 / 4];   // interrupt cause set
+    volatile unsigned int *ICR = &regs[0x00C0 / 4];   // cause read, clears
+
+    *IMS = 0x00000004;        // LSC: link status change, a cause with no traffic
+    (void)*ICR;               // clear anything already pending
+    *ICS = 0x00000004;        // assert it
+
+    // Crash simulation: assert the cause and leave, without reading or
+    // acking. The line is asserted and this process is about to stop existing.
+    if (argc > 2 && argv[2][0] == 'x') {
+        puts("  interrupt asserted; exiting WITHOUT acking (simulated crash)\n");
+        exit();
+    }
+
+    unsigned long long n = 0;
+    if (read(fd, &n, sizeof n) != (int)sizeof n) {
+        puts("  read() did not return an interrupt\n");
+        exit();
+    }
+    puts("  interrupt delivered, count = "); putn((int)n); puts("\n");
+
+    unsigned int cause = *ICR;          // ack at the device
+    puts("  ICR = 0x"); puthex(cause, 8); puts("\n");
+    write(fd, &n, sizeof n);            // re-arm
+
+    puts("\nall three mechanisms work: a driver could live out here.\n");
     exit();
 }

docs/plan/hybrid.md

@@ -68,7 +68,41 @@ link-up set -- which is how you tell a working mapping from one that reads
 refused, and everything a process held is released when it exits, because a
 driver that crashed must not keep its hardware away from its replacement.
 
-**Interrupts (mechanism 3) and the control channel (mechanism 1) are next.**
+**Mechanism 3 exists too.** A process asks for a device's interrupts and then
+reads them, uio-style:
+
+```
+  interrupts routed here (line 10)
+  interrupt delivered, count = 1
+  ICR = 0x80000004
+```
+
+`0x80000004` is the e1000e's cause register with the bit the test asserted and
+INT_ASSERTED set, read back by userspace as its acknowledgement.
+
+The line is **masked by the kernel on arrival and unmasked by userspace's
+write()**, and that is the entire design rather than a detail. A PCI interrupt
+line is level-triggered: it stays asserted until the cause is cleared *at the
+device*, which is a different register on every chip and therefore something
+only the driver can do. Signalling EOI on a line that is still asserted
+re-enters the handler immediately and for ever -- 72,000 times a second, when
+this kernel last did it by accident (history/kernel.md §13).
+
+So a driver that dies mid-interrupt leaves the line **masked, not unmasked**.
+The instinct is the other way round -- do not leave the device dead -- and it
+is wrong: unmasking an asserted line with no handler attached is a crashed
+driver taking the machine down with it. Verified by killing one on purpose:
+
+```
+  interrupt asserted; exiting WITHOUT acking (simulated crash)
+  timer: 3431 ticks taken, 3436 expected
+```
+
+Five ticks lost in seven seconds, which is the ordinary background. The next
+driver to claim the device re-arms the line, so masked is safe without being
+permanent -- also verified.
+
+**The control channel (mechanism 1) is what remains.**
 
 ### One thing the interface is deliberately not
 

include/irq.h

@@ -35,6 +35,7 @@ typedef void (*irq_handler_t)(void *arg);
 int  irq_register(int irq, irq_handler_t fn, void *arg, const char *name,
                   int level_low);
 void irq_unregister(int irq, irq_handler_t fn, void *arg);
+int  irq_handler_count(int irq);
 
 // Called from trap() with the IOAPIC pin number, not the vector.
 void irq_dispatch(int irq);

include/pciuser.h

@@ -19,8 +19,7 @@
 //   - to be given it, exclusively                                CLAIM
 //   - to reach its registers                                     mmap(BAR)
 //   - memory it can hand to the device                           DMA
-//   - to be told when the device wants attention                 (interrupts,
+//   - to be told when the device wants attention                 IRQ + read()
-//                                                                 not yet)
 //
 // There is no IOMMU. A process holding a DMA buffer can point the device at
 // any physical address, so this isolates a driver against its own *bugs* and
@@ -81,6 +80,37 @@ struct pciuser_dma {
 #define PCIIOC_CLAIM    0x9002   // in: int index -- take it from the kernel
 #define PCIIOC_RELEASE  0x9003   // in: int index
 #define PCIIOC_DMA      0x9004   // struct pciuser_dma
+#define PCIIOC_IRQ      0x9005   // in: int index -- deliver this device's
+                                 //     interrupts to this process
+
+// Interrupts arrive by *reading* this device, which is how Linux's uio does it
+// and for the same reason: a descriptor composes with poll, and a driver that
+// stops reading falls behind rather than losing events.
+//
+//     ioctl(fd, PCIIOC_IRQ, &index);
+//     for (;;) {
+//         unsigned long long n;
+//         read(fd, &n, sizeof n);      // blocks until the device interrupts
+//         ... service the device, and ACK IT AT THE DEVICE ...
+//         write(fd, &n, sizeof n);     // re-arm
+//     }
+//
+// **The write is not optional.** A PCI interrupt line is level-triggered: it
+// stays asserted until the driver clears the cause *at the device*, and the
+// kernel cannot do that because clearing it is device-specific -- it is a
+// different register on every chip. So the kernel masks the line when the
+// interrupt arrives and leaves it masked until the write says the device has
+// been dealt with.
+//
+// Without that, EOI is signalled on a line that is still asserted and the
+// interrupt re-fires immediately, for ever. That is not hypothetical: it
+// happened in this kernel, at 72,000 interrupts a second, and what it looked
+// like from userspace was a clock running seven times slow.
+// See docs/history/kernel.md section 13.
+//
+// One device per process. A driver drives one thing; if you need two, run two.
+
+
 
 // Registers are reached with mmap() on this device, where the *offset is the
 // physical address of the BAR*:

kernel/dev/irq.c

@@ -300,3 +300,17 @@ irq_info(char *buf, int cap)
   }
   return off;
 }
+
+// How many handlers are attached to a line.
+//
+// Asked by kernel/dev/pciuser.c before it gives a line to a userspace driver:
+// that driver masks the line while it works, and a masked line is masked for
+// everyone on it. Handing out a shared line would silence whichever kernel
+// driver is also there, with no message and no obvious cause.
+int
+irq_handler_count(int irq)
+{
+  if (irq < 0 || irq >= NIRQ)
+    return 0;
+  return line[irq].n;
+}

kernel/dev/pciuser.c

@@ -27,6 +27,7 @@
 #include "pci.h"
 #include "errno.h"
 #include "uaccess.h"
+#include "irq.h"
 #include "pciuser.h"
 int mappages(pml4e_t *pgdir, void *va, uint64 size, uint64 pa, int perm);
 
@@ -37,6 +38,57 @@ static struct proc *owner[NPCI];
 
 static struct spinlock pciuser_lock;
 
+// Interrupt delivery, one device per process.
+//
+// The count is what read() returns and what the reader compares against: a
+// driver that was slow sees the count jump rather than missing an edge, which
+// is the property that makes a count better than a flag.
+struct irqdev {
+  struct proc *proc;        // who reads, 0 when unused
+  int    index;             // pci_get() index
+  int    irq;               // the line
+  uint64 count;             // interrupts taken
+  uint64 seen;              // what the reader has consumed
+  int    masked;
+};
+static struct irqdev irqdev;
+
+// DMA buffers handed out, so they can be taken back.
+//
+// The arena is small and contiguous -- it has to be, a device cannot follow a
+// free list -- so a driver that exits without returning its buffers exhausts
+// it in a handful of restarts. Which is precisely the workload this interface
+// invites: the whole promise is that a driver can crash and be restarted, and
+// a promise that leaks a few kilobytes every time it is kept is not one.
+#define PCIUSER_MAXDMA 16
+struct dmablk {
+  struct proc *proc;
+  void  *kva;
+  uint64 size;
+};
+static struct dmablk dmablk[PCIUSER_MAXDMA];
+
+// The handler. Runs in interrupt context, so it does the two things that
+// cannot wait and nothing else.
+//
+// Masking is the important one. The line is level-triggered and still asserted
+// -- only the device's own driver can clear that, and its driver is a process
+// that is not running yet. Signalling EOI on an asserted line re-enters here
+// immediately and for ever; see docs/history/kernel.md section 13 for what
+// that looks like from outside. So the line goes quiet until userspace says
+// it has dealt with the device.
+static void
+pciuser_irq_handler(void *arg)
+{
+  (void)arg;
+  irqdev.count++;
+  if (!irqdev.masked) {
+    ioapicmask(irqdev.irq);
+    irqdev.masked = 1;
+  }
+  wakeup(&irqdev);
+}
+
 void
 pciuser_init(void)
 {
@@ -89,6 +141,37 @@ pciuser_release_all(struct proc *p)
   for (int i = 0; i < NPCI; i++)
     if (owner[i] == p)
       owner[i] = 0;
+
+  // And its interrupt line -- unregistered, and left MASKED.
+  //
+  // The instinct is to unmask on the way out, so the device is not left dead.
+  // That is exactly wrong, and for the reason running through this whole file:
+  // the driver died, so nobody cleared the cause at the device, so the line is
+  // very likely still asserted. Unmasking it with no handler attached means
+  // every EOI re-enters an empty dispatch, for ever -- a crashed driver taking
+  // the machine down with it, which is the precise opposite of what running
+  // drivers in processes is for.
+  //
+  // Masked is safe and it is not permanent: PCIIOC_IRQ arms the line when the
+  // next driver claims the device.
+  // Its DMA buffers. The process's page tables are torn down by exit() either
+  // way; what needs saying here is that the *arena* gets its pages back.
+  for (int i = 0; i < PCIUSER_MAXDMA; i++) {
+    if (dmablk[i].proc != p)
+      continue;
+    if (dmablk[i].kva)
+      dma_free(dmablk[i].kva, dmablk[i].size);
+    dmablk[i].proc = 0;
+    dmablk[i].kva  = 0;
+    dmablk[i].size = 0;
+  }
+
+  if (irqdev.proc == p) {
+    irq_unregister(irqdev.irq, pciuser_irq_handler, 0);
+    ioapicmask(irqdev.irq);
+    irqdev.masked = 1;
+    irqdev.proc = 0;
+  }
   release(&pciuser_lock);
 }
 
@@ -221,10 +304,23 @@ do_dma(uint64 argp)
   if (size == 0 || size > 4 * 1024 * 1024)
     return -EINVAL;
 
-  void *kva = dma_alloc(size, PGSIZE);
+  // A slot to remember it by, found before allocating: handing out memory that
-  if (!kva)
+  // cannot be recorded is how the leak this fixes got written in the first
+  // place.
+  struct dmablk *blk = 0;
+  acquire(&pciuser_lock);
+  for (int i = 0; i < PCIUSER_MAXDMA; i++)
+    if (!dmablk[i].proc) { blk = &dmablk[i]; blk->proc = myproc(); break; }
+  release(&pciuser_lock);
+  if (!blk)
     return -ENOMEM;
 
+  void *kva = dma_alloc(size, PGSIZE);
+  if (!kva) {
+    blk->proc = 0;
+    return -ENOMEM;
+  }
+
   // Its own mapping in the caller, taken from the same bump allocator mmap
   // uses so the two cannot collide.
   struct proc *p = myproc();
@@ -236,14 +332,65 @@ do_dma(uint64 argp)
   uint64 phys = dma_phys(kva);
   if (mappages(p->pgdir, (void *)va, size, phys, PTE_W | PTE_U) < 0) {
     dma_free(kva, size);
+    blk->proc = 0;
     return -ENOMEM;
   }
+  blk->kva  = kva;
+  blk->size = size;
 
   req->uaddr = va;
   req->paddr = phys;
   return 0;
 }
 
+// Hand a device's interrupts to the calling process.
+static int
+do_irq(uint64 argp)
+{
+  int idx;
+  if (copy_from_user(&idx, argp, sizeof(idx)) < 0)
+    return -EFAULT;
+  if (idx < 0 || idx >= pci_count() || idx >= NPCI)
+    return -ENODEV;
+  if (owner[idx] != myproc())
+    return -EACCES;               // claim it first
+
+  const struct pci_dev *d = pci_get(idx);
+  if (!d || d->irq == 0xFF)
+    return -ENODEV;               // the firmware routed it nowhere
+
+  // Refuse a line the kernel is already listening on.
+  //
+  // This handler masks the line, and a masked line is masked for *everyone* on
+  // it. Doing that to a shared line silences whichever kernel driver is also
+  // there, and the symptom -- a device that simply stops, with nothing logged
+  // -- is about as hard to find as bugs get. MSI is the real answer, since it
+  // is per-device and cannot be shared; pci_msi_enable() is already in the
+  // tree waiting for a caller.
+  if (irq_handler_count(d->irq) > 0)
+    return -EBUSY;
+
+  if (irqdev.proc && irqdev.proc != myproc())
+    return -EBUSY;                // one device per process, and one process
+
+  irqdev.proc   = myproc();
+  irqdev.index  = idx;
+  irqdev.irq    = d->irq;
+  irqdev.count  = 0;
+  irqdev.seen   = 0;
+  irqdev.masked = 0;
+
+  // Level-triggered and active low, like every PCI INTx line.
+  irq_register(d->irq, pciuser_irq_handler, 0, "userspace", 1);
+
+  // Arm it. The line may have been left masked by whatever held this device
+  // before -- see pciuser_release_all -- and a driver that has to write()
+  // before its first read() to get going is an interface nobody will guess.
+  ioapicroute(d->irq, 0, 1);
+  irqdev.masked = 0;
+  return 0;
+}
+
 int
 pciuser_ioctl(uint64 request, uint64 argp)
 {
@@ -257,23 +404,65 @@ pciuser_ioctl(uint64 request, uint64 argp)
   case PCIIOC_CLAIM:   return do_claim(argp);
   case PCIIOC_RELEASE: return do_release(argp);
   case PCIIOC_DMA:     return do_dma(argp);
+  case PCIIOC_IRQ:     return do_irq(argp);
   default:             return -ENOTTY;
   }
 }
 
-// Reads and writes are not how this device is used -- registers come through
+// Wait for the device to interrupt.
-// mmap and everything else through ioctl -- but a device in devsw needs both,
+//
-// and returning 0 is a better answer than a null pointer dereference.
+// Returns the total count rather than a flag, so a driver that was busy can
+// see it missed three and act once -- which is what you want, because the
+// alternative is either losing them or queueing them, and a device does not
+// care how many times you noticed it.
 int
 pciuser_read(struct inode *ip, uint64 off, char *dst, int n)
 {
-  (void)ip; (void)off; (void)dst; (void)n;
+  (void)off;
-  return 0;
+
+  if (irqdev.proc != myproc())
+    return -1;
+  if (n < (int)sizeof(uint64))
+    return -1;
+
+  iunlock(ip);
+  acquire(&pciuser_lock);
+  while (irqdev.count == irqdev.seen) {
+    if (myproc()->killed) {
+      release(&pciuser_lock);
+      ilock(ip);
+      return -1;
+    }
+    sleep(&irqdev, &pciuser_lock);
+  }
+  uint64 c = irqdev.count;
+  irqdev.seen = c;
+  release(&pciuser_lock);
+  ilock(ip);
+
+  memmove(dst, &c, sizeof(c));
+  return (int)sizeof(c);
 }
 
+// "I have dealt with the device" -- unmask the line.
+//
+// The driver must have cleared the interrupt *at the device* before this, or
+// the line is still asserted and unmasking simply re-enters the handler. That
+// is the contract, it cannot be checked from here, and getting it wrong is the
+// 72,000-interrupts-a-second failure in docs/history/kernel.md section 13.
 int
 pciuser_write(struct inode *ip, uint64 off, char *src, int n)
 {
   (void)ip; (void)off; (void)src;
+
+  if (irqdev.proc != myproc())
+    return -1;
+
+  acquire(&pciuser_lock);
+  if (irqdev.masked) {
+    ioapicroute(irqdev.irq, 0, 1);     // level, active low: a PCI INTx line
+    irqdev.masked = 0;
+  }
+  release(&pciuser_lock);
   return n;
 }