irq: MSI first, and poll the INTx lines that never arrive

PCI drivers took their interrupt from INTERRUPT_LINE. On real hardware
that is the 8259's number, not the IOAPIC pin the device is wired to
(that is in ACPI _PRT, which needs AML), so nothing ever arrived. xHCI
is entirely interrupt-driven, so the USB keyboard and a USB root would
have hung even on a clean boot. Under QEMU the two numbers agree,
which is why it never showed.

- pci_irq_attach()/pci_irq_detach(): MSI, then MSI-X if the driver
  allows it, then INTx. The handler is registered before MSI is
  enabled, so no edge is lost. xHCI, e1000, r8169 and rtl8139 use it.
- Lines 24-29 are MSI vectors. The LAPIC error vector moves from 19,
  which is a PCI pin on Intel chipsets, to 30. Nothing is placed at or
  above vector 64 (T_SYSCALL).
- A storming line is masked and its handlers are polled instead of
  being left to eat the CPU. A level line that has had a handler for 2 s
  and never fired is polled every tick by a kernel thread.

Tested in QEMU q35 with the root on a USB stick: xHCI and e1000e on
MSI; then MSI disabled and INTx deliberately misrouted, where xHCI got
zero interrupts and the root still mounted through the poller. i440fx
INTx and the rtl8139 hand-over are unchanged.

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

docs/architecture/devices.md

@@ -266,6 +266,57 @@ existing mechanisms for keeping that true mechanically rather than by intention.
 
 ---
 
+## Interrupts
+
+A PCI driver connects its interrupt with one call, `pci_irq_attach()`, and
+does not choose how. In order of preference:
+
+1. **MSI**: the device writes a vector to the local APIC. No pin, no routing,
+   nothing to share.
+2. **MSI-X**, but only if the driver passes `PCI_IRQF_MSIX`. Some parts change
+   how they report interrupt causes in that mode; the 82574 needs its IVAR
+   programmed, which `e1000` does not do, so it does not ask.
+3. **INTx**, on the line named by the device's `INTERRUPT_LINE` register.
+
+MSI comes first because INTx on real hardware is **the wrong pin**.
+`INTERRUPT_LINE` holds the number the firmware gave the 8259. In APIC mode the
+device is wired to an IOAPIC input that only ACPI's `_PRT` describes, and
+reading `_PRT` takes an AML interpreter, which SchOS does not have. Under QEMU
+the two numbers happen to agree, which is why this went unnoticed until the
+drivers were read with a real board in mind.
+
+INTx is therefore backed by the poller in `kernel/dev/irq.c`:
+
+- A level-triggered line that has had a handler for two seconds and has never
+  fired gets its handlers called every tick, from a kernel thread with
+  interrupts off.
+- A line that **storms** (an idle ISA input configured active-low reads as
+  asserted forever) is masked and polled instead of being reported and left
+  running.
+
+Every handler already has to cope with being called when its device is idle,
+because lines are shared, so a spurious call costs only a register read.
+`/proc/interrupts` marks such a line `[polled]`, and an MSI line `msi`.
+
+The vector layout:
+
+| Lines | What they are |
+|---|---|
+| 0–23 | IOAPIC pins |
+| 24–29 | MSI vectors, allocated by `irq_alloc_msi()` |
+| 30 | LAPIC error (it was 19, which is a PCI pin on Intel chipsets) |
+| 31 | spurious |
+
+Nothing goes as high as vector 64, which is `T_SYSCALL`.
+
+Tested under QEMU q35 with the root filesystem on a USB stick, in three
+configurations:
+
+- xHCI and e1000e on MSI.
+- MSI disabled and the INTx line deliberately misrouted. xHCI took no
+  interrupts at all, the poller carried every transfer, and the root still
+  mounted.
+- The old INTx path, unchanged on i440fx.
 
 ---
 

include/irq.h

@@ -20,7 +20,16 @@
 // whether its own device actually raised the interrupt and do nothing if it
 // did not.
 
-#define NIRQ    24      // IOAPIC redirection entries on a PC
+// Lines 0-23 are the IOAPIC's pins. 24-29 are MSI vectors, which are not pins
+// at all -- a device writes the vector number to the local APIC directly --
+// and are dispatched through the same table so that a handler cannot tell
+// which kind it is on. 30 and 31 are the LAPIC's error and spurious vectors.
+// Everything stops below vector 64, which is T_SYSCALL: a line there would
+// let a user-mode `int $0x40` dispatch an interrupt and EOI one that is not.
+#define NIRQ_IOAPIC   24
+#define IRQ_MSI_FIRST 24
+#define IRQ_MSI_LAST  29
+#define NIRQ          31
 #define NCHAIN   4      // handlers that may share one line
 
 typedef void (*irq_handler_t)(void *arg);
@@ -41,4 +50,11 @@ int  irq_handler_count(int irq);
 void irq_dispatch(int irq);
 
 void irq_init(void);
+
+// An MSI vector for pci_irq_attach(). -1 when all six are in use.
+int  irq_alloc_msi(void);
+void irq_free_msi(int irq);
+
+// The thread that polls PCI lines which never interrupt. After sinit().
+void irq_poll_start(void);
 int  irq_info(char *buf, int cap);   // /proc/interrupts

include/pci.h

@@ -74,8 +74,22 @@ struct pci_dev {
     // The driver itself, so it can be asked to release the device. The name
     // alone cannot be called.
     const struct pci_driver *drv;
+    // How pci_irq_attach() connected the interrupt: PCI_IRQ_INTX, _MSI or
+    // _MSIX, and on which line. See kernel/dev/pci.c.
+    uint8  irq_kind;
+    int    irq_line;
 };
 
+#define PCI_IRQ_NONE  0
+#define PCI_IRQ_INTX  1
+#define PCI_IRQ_MSI   2
+#define PCI_IRQ_MSIX  3
+
+// pci_irq_attach() flags. MSI is always tried first; MSI-X only when the
+// driver says it can take it, because some parts change how they report
+// interrupt causes once MSI-X is on (the 82574's IVAR, for one).
+#define PCI_IRQF_MSIX 0x1
+
 // One row of a driver's match table. Terminate the table with a row whose
 // vendor is 0.
 struct pci_match {
@@ -90,6 +104,16 @@ struct pci_match {
 #include "device.h"
 
 enum dev_class pci_class_of(const struct pci_dev *);
+
+// Connect a device's interrupt to a handler: an MSI vector if the device has
+// one, MSI-X if allowed and it has that instead, and its INTx line otherwise.
+// Returns the line, or -1 if there is no way to interrupt at all -- which is
+// not fatal; a driver with a poll path should say so and carry on.
+#include "irq.h"
+int  pci_irq_attach(struct pci_dev *, irq_handler_t, void *arg,
+                    const char *name, int flags);
+void pci_irq_detach(struct pci_dev *, irq_handler_t, void *arg);
+const char *pci_irq_kind(const struct pci_dev *);
 void           pci_describe(const struct pci_dev *, char *out, int cap);
 
 struct pci_driver {

include/traps.h

@@ -30,7 +30,9 @@
 #define IRQ_COM1         4
 #define IRQ_MOUSE       12
 #define IRQ_IDE         14
-#define IRQ_ERROR       19
+// 30, not xv6's 19. Vector T_IRQ0+19 is IOAPIC pin 19, which real chipsets
+// use for PCI; the LAPIC error interrupt would have shared it.
+#define IRQ_ERROR       30
 #define IRQ_SPURIOUS    31
 
 // System call interrupt number for legacy int 0x40 or 0x80 if used

kernel/core/main.c

@@ -362,6 +362,9 @@ kmain(uint32 fb_addr)
   // until mpmain() below enters the scheduler.
   netinit();
   if (boot_verbose) cprintf("verbose boot: netinit done\n");
+  // For the same reason: a kernel thread. See irq_poll_thread() for the
+  // lines it is there for, which on real hardware is most PCI INTx.
+  irq_poll_start();
   // The debugger's own thread, for the same reason netinit's comes after
   // sinit: kthread_create needs a usable process table. From here on Ctrl-G
   // on the console answers, with no filesystem mounted and nothing in

kernel/dev/irq.c

@@ -20,9 +20,15 @@ static struct {
   int  n;               // handlers installed; only ever grows during boot
   int  enabled;
   int  level_low;
+  int  msi;             // an MSI vector, not an IOAPIC pin: nothing to route
+  int  polled;          // handlers run from irq_poll_thread as well
+  uint64 delivered;     // real interrupts, not polls
+  uint64 since;         // ticks when the first handler was attached
   uint64 unhandled;
 } line[NIRQ];
 
+static int msi_used[NIRQ];
+
 // Guards registration against itself. The *dispatch* path deliberately does
 // not take it: an interrupt handler that blocked on a lock held by whoever
 // was registering would deadlock the CPU it fired on. Instead registration
@@ -44,10 +50,10 @@ irq_init(void)
 }
 
 // IRQ_ERROR and IRQ_SPURIOUS are not IOAPIC pins -- they are local APIC
-// vectors that xv6 numbered as though they were, so T_IRQ0+19 and T_IRQ0+31
+// vectors that xv6 numbered as though they were. The error vector used to be
-// are already spoken for. A PCI device routed to pin 19 would deliver on the
+// 19, which *is* a pin: one of the four an Intel chipset wires its PCI
-// same vector as the LAPIC error interrupt. Refuse it here rather than let it
+// interrupts to, so a device there would have been refused. It is 30 now,
-// be debugged later.
+// above every pin; see include/traps.h.
 static int
 reserved(int irq)
 {
@@ -91,10 +97,13 @@ irq_register(int irq, irq_handler_t fn, void *arg, const char *name,
   a->count = 0;
   __sync_synchronize();         // publish the entry before it is reachable
   line[irq].n++;
+  if(line[irq].n == 1)
+    line[irq].since = ticks;
 
   if(!line[irq].enabled){
     line[irq].level_low = level_low;
-    ioapicroute(irq, 0, level_low);
+    if(!line[irq].msi)
+      ioapicroute(irq, 0, level_low);
     line[irq].enabled = 1;
   }
 
@@ -117,7 +126,11 @@ irq_unregister(int irq, irq_handler_t fn, void *arg)
       // last handler. Compacting the array moves entries that a dispatcher on
       // another CPU may be walking, so the line has to be quiet for the
       // duration, and then brought back only if anything is left on it.
-      ioapicmask(irq);
+      //
+      // An MSI vector has no pin to mask. Its caller, pci_irq_detach(), has
+      // already switched the device's MSI off, which is the same thing.
+      if(!line[irq].msi)
+        ioapicmask(irq);
       line[irq].enabled = 0;
 
       for(j = i; j + 1 < line[irq].n; j++)
@@ -126,8 +139,12 @@ irq_unregister(int irq, irq_handler_t fn, void *arg)
       __sync_synchronize();
 
       if(line[irq].n > 0){
-        ioapicroute(irq, 0, line[irq].level_low);
+        if(!line[irq].msi && !line[irq].polled)
+          ioapicroute(irq, 0, line[irq].level_low);
         line[irq].enabled = 1;
+      } else {
+        line[irq].polled = 0;
+        line[irq].delivered = 0;
       }
       break;
     }
@@ -189,6 +206,17 @@ storm_check(int irq)
     cprintf("irq %d:   handler '%s' is attached\n", irq, line[irq].act[i].name);
   if(line[irq].n == 0)
     cprintf("irq %d:   no handler is attached at all\n", irq);
+
+  // And stop it. Reporting a storm and carrying on is a machine that spends
+  // every cycle in this handler -- which is what a PCI line configured on the
+  // wrong pin does on real hardware, where an idle ISA input read active-low
+  // is asserted for ever. The handlers move to the poller, so a device that
+  // was really on this line still works, ten milliseconds at a time.
+  if(irq < NIRQ_IOAPIC && !line[irq].msi){
+    ioapicmask(irq);
+    line[irq].polled = 1;
+    cprintf("irq %d: masked; its handlers are polled from now on\n", irq);
+  }
 }
 
 void
@@ -206,6 +234,8 @@ irq_dispatch(int irq)
     return;
   }
 
+  line[irq].delivered++;
+
   // Every handler on a shared line is offered the interrupt; each one is
   // responsible for recognising that its device is idle and returning.
   for(i = 0; i < n; i++){
@@ -267,7 +297,10 @@ irq_info(char *buf, int cap)
       off = emit(buf, cap, off, "(unhandled)");
     if(line[irq].unhandled)
       off = emit(buf, cap, off, " [spurious]");
-    off = emit(buf, cap, off, line[irq].level_low ? " level\n" : " edge\n");
+    if(line[irq].polled)
+      off = emit(buf, cap, off, " [polled]");
+    off = emit(buf, cap, off, line[irq].msi ? " msi\n" :
+                              line[irq].level_low ? " level\n" : " edge\n");
   }
 
   // How many timer interrupts were lost.
@@ -314,3 +347,106 @@ irq_handler_count(int irq)
     return 0;
   return line[irq].n;
 }
+
+// ---------------------------------------------------------------------------
+// MSI vectors
+// ---------------------------------------------------------------------------
+
+// A line that is an MSI vector rather than a pin. They are handed out from
+// IRQ_MSI_FIRST..IRQ_MSI_LAST, which are vectors the IOAPIC never uses; see
+// pci_irq_attach() in kernel/dev/pci.c, which is the only caller.
+int
+irq_alloc_msi(void)
+{
+  acquire(&irqlock);
+  for(int irq = IRQ_MSI_FIRST; irq <= IRQ_MSI_LAST; irq++){
+    if(!msi_used[irq] && !reserved(irq)){
+      msi_used[irq] = 1;
+      line[irq].msi = 1;
+      release(&irqlock);
+      return irq;
+    }
+  }
+  release(&irqlock);
+  return -1;
+}
+
+void
+irq_free_msi(int irq)
+{
+  if(irq < IRQ_MSI_FIRST || irq > IRQ_MSI_LAST)
+    return;
+  acquire(&irqlock);
+  if(line[irq].n == 0){
+    msi_used[irq] = 0;
+    line[irq].msi = 0;
+  }
+  release(&irqlock);
+}
+
+// ---------------------------------------------------------------------------
+// Polling the lines that do not interrupt
+// ---------------------------------------------------------------------------
+//
+// A PCI driver that cannot have MSI learns its line from the device's
+// INTERRUPT_LINE register, and on real hardware that is the wrong answer.
+// The register holds the number firmware gave the *8259* -- 10, 11 -- and in
+// APIC mode the device is wired to an IOAPIC input the firmware only
+// describes in ACPI's _PRT, which takes an AML interpreter to read. So the
+// pin gets unmasked, the device's real pin stays masked, and nothing arrives.
+// Under QEMU the two numbers happen to agree, which is why it never showed.
+//
+// Until there is an AML interpreter, a level-triggered line that has had a
+// handler for two seconds and has not once fired is presumed to be that,
+// and its handlers are called from here every tick. Every handler is already
+// written to be called when its device has nothing to say -- a shared line
+// needs that -- so being called spuriously costs a register read. A storm
+// does the same from the other side; see storm_check().
+//
+// The moment a real interrupt arrives the line stops being polled, unless it
+// was put here for storming.
+#define POLL_AFTER (2 * HZ)
+
+static void
+irq_poll_thread(void)
+{
+  static uchar said[NIRQ];
+
+  for(;;){
+    acquire(&tickslock);
+    uint start = ticks;
+    while(ticks == start)
+      sleep(&ticks, &tickslock);
+    uint now = ticks;
+    release(&tickslock);
+
+    for(int irq = 0; irq < NIRQ_IOAPIC; irq++){
+      if(line[irq].n == 0 || line[irq].msi || reserved(irq))
+        continue;
+      int poll = line[irq].polled ||
+                 (line[irq].level_low && line[irq].delivered == 0 &&
+                  now - line[irq].since > POLL_AFTER);
+      if(!poll)
+        continue;
+      if(!said[irq] && !line[irq].polled){
+        said[irq] = 1;
+        kinfo("irq %d: nothing has arrived on it; polling its handlers\n",
+              irq);
+      }
+      // As an interrupt would: on this CPU, with interrupts off. n is read
+      // once, as irq_dispatch() reads it.
+      pushcli();
+      int n = line[irq].n;
+      for(int i = 0; i < n; i++)
+        line[irq].act[i].fn(line[irq].act[i].arg);
+      popcli();
+    }
+  }
+}
+
+void
+irq_poll_start(void)
+{
+  if(kthread_create(irq_poll_thread, "irqpoll") < 0)
+    cprintf("irq: cannot start the poller; misrouted PCI lines stay silent\n");
+}

kernel/dev/pci.c

@@ -24,6 +24,10 @@
 #include "sbuf.h"
 #include "dbg.h"
 #include "device.h"
+#include "traps.h"
+#include "irq.h"
+#include "memlayout.h"
+#include "proc.h"
 
 static struct pci_dev devs[NPCI];
 static int ndevs;
@@ -178,6 +182,135 @@ pci_msi_enable(struct pci_dev *d, int vector, int cpu)
   return 0;
 }
 
+// The same for MSI-X, entry 0 only.
+//
+// MSI-X keeps its address/data pairs in a table inside one of the device's
+// own BARs rather than in config space, which is the whole difference. Some
+// controllers -- several AMD xHCIs among them -- offer MSI-X and no MSI, and
+// without this they had only INTx, which on real hardware is the line that
+// does not arrive.
+static int
+pci_msix_enable(struct pci_dev *d, int vector, int apicid)
+{
+  uint8 cap = pci_find_cap(d, PCI_CAP_MSIX);
+  if(cap == 0)
+    return -1;
+
+  uint32 tbl = pci_read32(d->bus, d->slot, d->func, cap + 4);
+  int bir = tbl & 7;
+  uint64 off = tbl & ~7u;
+  if(bir > 5 || d->bar[bir].base == 0 || d->bar[bir].is_io)
+    return -1;
+
+  volatile uint32 *t = ioremap(d->bar[bir].base + off, 16);
+  if(t == 0)
+    return -1;
+
+  // Enable with the whole function masked, program entry 0, unmask it, then
+  // drop the function mask: at no point is an entry live with half an
+  // address in it. The other entries come out of reset masked and stay so.
+  uint32 dw = pci_read32(d->bus, d->slot, d->func, cap);
+  pci_write32(d->bus, d->slot, d->func, cap, dw | (3u << 30));   // enable + fmask
+  t[0] = 0xFEE00000u | ((uint32)apicid << 12);
+  t[1] = 0;
+  t[2] = (uint32)vector;
+  t[3] = 0;                                                      // unmask
+  dw = pci_read32(d->bus, d->slot, d->func, cap);
+  pci_write32(d->bus, d->slot, d->func, cap, (dw | (1u << 31)) & ~(1u << 30));
+
+  pci_write32(d->bus, d->slot, d->func, PCI_COMMAND,
+              pci_read32(d->bus, d->slot, d->func, PCI_COMMAND) | (1 << 10));
+  return 0;
+}
+
+// Turn off whichever of MSI or MSI-X is on, and give the device back its INTx
+// -- the next driver to claim it may be one that only knows INTx.
+static void
+pci_msi_off(struct pci_dev *d)
+{
+  uint8 cap;
+  uint32 dw;
+
+  if((cap = pci_find_cap(d, PCI_CAP_MSI)) != 0){
+    dw = pci_read32(d->bus, d->slot, d->func, cap);
+    pci_write32(d->bus, d->slot, d->func, cap, dw & ~(1u << 16));
+  }
+  if((cap = pci_find_cap(d, PCI_CAP_MSIX)) != 0){
+    dw = pci_read32(d->bus, d->slot, d->func, cap);
+    pci_write32(d->bus, d->slot, d->func, cap, dw & ~(1u << 31));
+  }
+  pci_write32(d->bus, d->slot, d->func, PCI_COMMAND,
+              pci_read32(d->bus, d->slot, d->func, PCI_COMMAND) & ~(1u << 10));
+}
+
+int
+pci_irq_attach(struct pci_dev *d, irq_handler_t fn, void *arg,
+               const char *name, int flags)
+{
+  // The processor the interrupt goes to, by APIC ID: the boot processor's,
+  // which is the one IOAPIC lines are routed to as well.
+  int dest = cpus[0].apicid;
+  int irq = irq_alloc_msi();
+
+  if(irq >= 0){
+    // Handler first, then the device. An MSI is an edge: one that arrives
+    // before anything is listening is gone, and an xHCI whose first event
+    // interrupt is lost will not raise another until it is acknowledged.
+    if(irq_register(irq, fn, arg, name, 0) == 0){
+      int kind = PCI_IRQ_NONE;
+      if(pci_msi_enable(d, T_IRQ0 + irq, dest) == 0)
+        kind = PCI_IRQ_MSI;
+      else if((flags & PCI_IRQF_MSIX) &&
+              pci_msix_enable(d, T_IRQ0 + irq, dest) == 0)
+        kind = PCI_IRQ_MSIX;
+      if(kind != PCI_IRQ_NONE){
+        d->irq_kind = kind;
+        d->irq_line = irq;
+        return irq;
+      }
+      irq_unregister(irq, fn, arg);
+    }
+    irq_free_msi(irq);
+  }
+
+  if(d->irq == 0xFF || d->irq >= NIRQ_IOAPIC)
+    return -1;
+  if(irq_register(d->irq, fn, arg, name, 1) < 0)
+    return -1;
+  d->irq_kind = PCI_IRQ_INTX;
+  d->irq_line = d->irq;
+  return d->irq;
+}
+
+void
+pci_irq_detach(struct pci_dev *d, irq_handler_t fn, void *arg)
+{
+  switch(d->irq_kind){
+  case PCI_IRQ_MSI:
+  case PCI_IRQ_MSIX:
+    // The device first, so nothing is written to a vector being taken away.
+    pci_msi_off(d);
+    irq_unregister(d->irq_line, fn, arg);
+    irq_free_msi(d->irq_line);
+    break;
+  case PCI_IRQ_INTX:
+    irq_unregister(d->irq_line, fn, arg);
+    break;
+  }
+  d->irq_kind = PCI_IRQ_NONE;
+}
+
+const char *
+pci_irq_kind(const struct pci_dev *d)
+{
+  switch(d->irq_kind){
+  case PCI_IRQ_MSI:  return "msi";
+  case PCI_IRQ_MSIX: return "msi-x";
+  case PCI_IRQ_INTX: return "intx";
+  default:           return "none";
+  }
+}
+
 // ---------------------------------------------------------------------------
 // Base address registers
 // ---------------------------------------------------------------------------

kernel/drivers/bus/xhci/xhci.c

@@ -1169,7 +1169,7 @@ xhci_dump(void)
             rd32(x->rt, XHCI_IR0 + IR_IMAN), rd32(x->rt, XHCI_IR0 + IR_IMOD),
             rd32(x->rt, XHCI_IR0 + IR_ERSTSZ),
             rd64(x->rt, XHCI_IR0 + IR_ERDP), x->ev.deq, x->ev.cycle);
-    cprintf("  pci irq %d\n", x->pci->irq);
+    cprintf("  pci irq %d (%s)\n", x->pci->irq_line, pci_irq_kind(x->pci));
 
     for(p = 1; p <= x->maxports; p++){
       uint32 sc = rd32(x->op, XHCI_PORTSC(p));
@@ -1328,17 +1328,26 @@ xhci_probe(struct pci_dev *d)
   if(x->ndevices == 0)
     cprintf("xhci: no devices attached\n");
 
-  // Probing is over: from here a bulk transfer has a process behind it and
-  // can sleep rather than spin.
-  x->can_sleep = 1;
 
   // Now, and only now, turn interrupts on. The order is: claim the line
   // (which unmasks it and sets level/active-low), clear anything the
   // controller already considers pending so that it drives INTx low, and then
   // let it interrupt. The next event is a genuine low-to-high transition,
   // which is the only thing an interrupt controller can act on.
-  if(d->irq != 0xFF){
+  //
-    irq_register(d->irq, xhci_irq, x, "xhci", 1);   // PCI: level, active low
+  // MSI or MSI-X when the controller has either, which every xHCI does. On
+  // real hardware that is not a refinement: the INTx line the config space
+  // names is the 8259's number, the IOAPIC pin the controller is really on
+  // is a different one, and an xHCI waiting on INTx waited for ever -- the
+  // USB keyboard dead and a USB root never mounted.
+  //
+  // Probing is over: from here a bulk transfer has a process behind it and
+  // can sleep rather than spin -- but only if something will wake it. With
+  // no interrupt at all, transfers go on spinning as they did during probe.
+  if(pci_irq_attach(d, xhci_irq, x, "xhci", PCI_IRQF_MSIX) < 0){
+    cprintf("xhci: no interrupt; transfers will be polled\n");
+  } else {
+    x->can_sleep = 1;
 
     wr32(x->op, XHCI_USBSTS, STS_EINT);
     wr32(x->rt, XHCI_IR0 + IR_IMAN, IMAN_IP | IMAN_IE);

kernel/drivers/net/e1000/e1000.c

@@ -359,19 +359,20 @@ e1000_probe(struct pci_dev *d)
   wr(e, E1000_RCTL, E1000_RCTL_EN | E1000_RCTL_BAM | E1000_RCTL_SECRC |
                     E1000_RCTL_SZ_2048 | E1000_RCTL_LBM_NONE);
 
-  if (d->irq != 0xFF)
+  // MSI where the part has it -- the 82574 and the rest of the PCIe
-    // level_low = 1: this is a PCI interrupt, which is level-triggered and
+  // generation do -- and INTx otherwise. MSI-X is not offered: the 82574 in
-    // active low. Registering it as edge-triggered makes it fire once and then
+  // MSI-X mode routes its causes through IVAR, which this driver does not
-    // wedge, because the device holds the line asserted and no further edge
+  // program, and its interrupts would go nowhere. If neither works the
-    // ever arrives. See the comment on irq_register in include/irq.h.
+  // network thread still collects frames on its tick.
-    irq_register(d->irq, e1000_intr, e, "e1000", 1);
+  pci_irq_attach(d, e1000_intr, e, "e1000", 0);
   wr(e, E1000_IMS, E1000_ICR_RXT0 | E1000_ICR_RXDMT0 | E1000_ICR_LSC);
 
   e->ready = 1;
 
-  kinfo("e1000: %x:%x:%x:%x:%x:%x, link %s, irq %d\n",
+  kinfo("e1000: %x:%x:%x:%x:%x:%x, link %s, irq %d (%s)\n",
           e->mac[0], e->mac[1], e->mac[2], e->mac[3], e->mac[4], e->mac[5],
-          (rd(e, E1000_STATUS) & E1000_STATUS_LU) ? "up" : "down", d->irq);
+          (rd(e, E1000_STATUS) & E1000_STATUS_LU) ? "up" : "down",
+          d->irq_line, pci_irq_kind(d));
   return 0;
 }
 
@@ -458,8 +459,7 @@ e1000_release(struct pci_dev *d)
   wr(e, E1000_TCTL, 0);                    // stop transmitting
   rd(e, E1000_STATUS);                     // post the writes
 
-  if (d->irq != 0xFF)
+  pci_irq_detach(d, e1000_intr, e);
-    irq_unregister(d->irq, e1000_intr, e);
 
   // And take the interface out of the stack.
   //

kernel/drivers/net/r8169/r8169.c

@@ -468,9 +468,9 @@ r8169_probe(struct pci_dev *d)
   wr16(r, R_IMR, ISR_ROK | ISR_RER | ISR_TOK | ISR_TER |
                  ISR_RDU | ISR_LINKCHG);
 
-  if (d->irq != 0xFF && d->irq < 24)
+  // MSI first: every 8111 has it, and on the boards these are soldered to it
-    irq_register(d->irq, r8169_intr, r, "r8169", 1);
+  // is the only interrupt that is sure to arrive. See pci_irq_attach().
-  else
+  if (pci_irq_attach(d, r8169_intr, r, "r8169", 0) < 0)
     cprintf("r8169: no usable interrupt line; the link will be polled\n");
 
   r->ready = 1;
@@ -487,10 +487,10 @@ r8169_probe(struct pci_dev *d)
     uint32 xid = (rd32(r, R_TCR) >> 20) & 0xFCF;
     int    bmsr = mii_read(r, 1);
 
-    cprintf("r8169: xid %x, %x:%x:%x:%x:%x:%x, irq %d, %s (bmsr %x)\n",
+    cprintf("r8169: xid %x, %x:%x:%x:%x:%x:%x, irq %d (%s), %s (bmsr %x)\n",
             xid,
             r->mac[0], r->mac[1], r->mac[2], r->mac[3], r->mac[4], r->mac[5],
-            d->irq,
+            d->irq_line, pci_irq_kind(d),
             (rd8(r, R_PHYSTATUS) & PHYSTATUS_LINK) ? "link up" : "link down",
             bmsr);
   }

kernel/drivers/net/rtl8139/rtl8139.c

@@ -360,8 +360,7 @@ rtl8139_probe(struct pci_dev *d)
   wr16(r, R_IMR, ISR_ROK | ISR_RER | ISR_TOK | ISR_TER |
                  ISR_RXOVW | ISR_FOVW | ISR_LINKCHG);
 
-  if (d->irq != 0xFF && d->irq < 24)
+  pci_irq_attach(d, rtl8139_intr, r, "rtl8139", 0);   // no MSI on this part
-    irq_register(d->irq, rtl8139_intr, r, "rtl8139", 1);
 
   r->ready = 1;
   kinfo("rtl8139: %x:%x:%x:%x:%x:%x, %s, irq %d\n",
@@ -405,8 +404,7 @@ rtl8139_release(struct pci_dev *d)
   wr8(r, R_CR, 0);                // receiver and transmitter off
   release(&r->lock);
 
-  if (d->irq != 0xFF && d->irq < 24)
+  pci_irq_detach(d, rtl8139_intr, r);
-    irq_unregister(d->irq, rtl8139_intr, r);
 
   // Reset, and wait for it, so the chip has truly stopped before its buffers
   // are reused. The reset bit clears itself when the chip is done.