Download raw body.
nvme(4): fix panic on hotplug
Hotplugging a USB4 NVMe enclosure ASMedia ASM2464PD bridge Kingston
NV2 disk on a ThinkPad X1 Nano Gen 1 panics every time:
uvm_fault(0xffffffff82b72e90, 0x28, 0, 1) -> e
nvme_q_complete+0x2a: movq 0x28(%rsi),%rax
nvme_q_complete(ffff80000235d000,0) at nvme_q_complete+0x2a
nvme_intr(ffff80000235d000) at nvme_intr+0x2b
...
nvme_identify(ffff80000235d000,c) at nvme_identify+0x318
nvme_attach(ffff80000235d000) at nvme_attach+0x2f5
nvme_pci_attach(...) at nvme_pci_attach+0x1cb
The interrupt is established before nvme_attach(), so an interrupt
during nvme_identify() reaches nvme_intr() while sc_q is still NULL.
At boot attach runs while cold, which hides it.
jmatthew@ posted the same check in nvme_intr() in February, together
with clearing sc_q in nvme_shutdown() [1]. This diff includes his
change, also checks sc_admin_q, and clears the queue pointers freed in
the error paths of nvme_attach() and nvme_resume().
Tested on -current with only this diff, enclosure hotplugged and
connected at boot: attach, suspend/resume and reboot all work.
Hotplug with the diff:
ppb3 at pci2 dev 0 function 0 vendor "ASMedia", unknown product 0x2463 rev 0x00
pci4 at ppb3 bus 33
ppb4 at pci4 dev 0 function 0 vendor "ASMedia", unknown product 0x2463 rev 0x00: msi
pci5 at ppb4 bus 34
nvme1 at pci5 dev 0 function 0 vendor "Kingston", unknown product 0x5023 rev 0x03: msix, NVMe 1.4
nvme1: KINGSTON SNV2S1000G, firmware SCK00105
scsibus4 at nvme1: 2 targets, initiator 0
sd2 at scsibus4 targ 1 lun 0: <NVMe, KINGSTON SNV2S10, SCK0>
sd2: 953869MB, 512 bytes/sector, 1953525168 sectors
[1] https://marc.info/?l=openbsd-tech&m=177078335611415
ok?
diff /home/ijanc/tmp/src-master
path + /home/ijanc/tmp/src-master
commit - b53fbb088c0ddbd80eb70c94d8ac96a2c20e38f1
blob - c965d6dcc45e5c3c9f1fb6a1a1b786798013f7e0
file + sys/dev/ic/nvme.c
--- sys/dev/ic/nvme.c
+++ sys/dev/ic/nvme.c
@@ -444,12 +444,14 @@ nvme_attach(struct nvme_softc *sc)
free_q:
nvme_q_free(sc, sc->sc_q);
+ sc->sc_q = NULL;
disable:
nvme_disable(sc);
free_ccbs:
nvme_ccbs_free(sc, nccbs);
free_admin_q:
nvme_q_free(sc, sc->sc_admin_q);
+ sc->sc_admin_q = NULL;
return (1);
}
@@ -489,6 +491,7 @@ nvme_resume(struct nvme_softc *sc)
free_q:
nvme_q_free(sc, sc->sc_q);
+ sc->sc_q = NULL;
disable:
nvme_disable(sc);
@@ -557,6 +560,7 @@ nvme_shutdown(struct nvme_softc *sc)
printf("%s: unable to delete q, disabling\n", DEVNAME(sc));
goto disable;
}
+ sc->sc_q = NULL;
cc = nvme_read4(sc, NVME_CC);
CLR(cc, NVME_CC_SHN_MASK);
@@ -1583,9 +1587,9 @@ nvme_intr(void *xsc)
struct nvme_softc *sc = xsc;
int rv = 0;
- if (nvme_q_complete(sc, sc->sc_q))
+ if (sc->sc_q != NULL && nvme_q_complete(sc, sc->sc_q))
rv = 1;
- if (nvme_q_complete(sc, sc->sc_admin_q))
+ if (sc->sc_admin_q != NULL && nvme_q_complete(sc, sc->sc_admin_q))
rv = 1;
return (rv);
nvme(4): fix panic on hotplug