d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

nvme: Enable interrupts after qpair fully constructed

Browse Source 
To guard against the ill effects of a spurious interrupt during
construction (or one that was bogusly pending), enable interrupts after
the qpair is completely constructed. Otherwise, we can die with null
pointer dereferences in nvme_qpair_process_completions. This has been
observed in at least one pre-release NVMe drive where the MSIX interrupt
fired while the queue was being created, before we'd started the NVMe
controller card.

The alternative of only turning on the interrupts after the rest was
tried, but was insufficient to work around this bug and made the code
more complicated w/o benefit.

Reviewed by:		mav, chuck
Sponsored by:		Netflix
Differential Revision:	https://reviews.freebsd.org/D31182
This commit is contained in:
Warner Losh 2021-07-15 16:17:23 -06:00
parent 998abf5a12
commit fc9a084023
1 changed files with 25 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
sys/dev/nvme/nvme_qpair.c
View File

@ -675,30 +675,6 @@ nvme_qpair_construct(struct nvme_qpair *qpair,
qpair->num_trackers = num_trackers;
qpair->ctrlr = ctrlr;
if (ctrlr->msix_enabled) {
/*
* MSI-X vector resource IDs start at 1, so we add one to
* the queue's vector to get the corresponding rid to use.
*/
qpair->rid = qpair->vector + 1;
qpair->res = bus_alloc_resource_any(ctrlr->dev, SYS_RES_IRQ,
&qpair->rid, RF_ACTIVE);
if (bus_setup_intr(ctrlr->dev, qpair->res,
INTR_TYPE_MISC | INTR_MPSAFE, NULL,
nvme_qpair_msix_handler, qpair, &qpair->tag) != 0) {
nvme_printf(ctrlr, "unable to setup intx handler\n");
goto out;
}
if (qpair->id == 0) {
bus_describe_intr(ctrlr->dev, qpair->res, qpair->tag,
"admin");
} else {
bus_describe_intr(ctrlr->dev, qpair->res, qpair->tag,
"io%d", qpair->id - 1);
}
}
mtx_init(&qpair->lock, "nvme qpair lock", NULL, MTX_DEF);
/* Note: NVMe PRP format is restricted to 4-byte alignment. */
@ -818,6 +794,31 @@ nvme_qpair_construct(struct nvme_qpair *qpair,
qpair->act_tr = malloc_domainset(sizeof(struct nvme_tracker *) *
qpair->num_entries, M_NVME, DOMAINSET_PREF(qpair->domain),
M_ZERO | M_WAITOK);
if (ctrlr->msix_enabled) {
/*
* MSI-X vector resource IDs start at 1, so we add one to
* the queue's vector to get the corresponding rid to use.
*/
qpair->rid = qpair->vector + 1;
qpair->res = bus_alloc_resource_any(ctrlr->dev, SYS_RES_IRQ,
&qpair->rid, RF_ACTIVE);
if (bus_setup_intr(ctrlr->dev, qpair->res,
INTR_TYPE_MISC | INTR_MPSAFE, NULL,
nvme_qpair_msix_handler, qpair, &qpair->tag) != 0) {
nvme_printf(ctrlr, "unable to setup intx handler\n");
goto out;
}
if (qpair->id == 0) {
bus_describe_intr(ctrlr->dev, qpair->res, qpair->tag,
"admin");
} else {
bus_describe_intr(ctrlr->dev, qpair->res, qpair->tag,
"io%d", qpair->id - 1);
}
}
return (0);
out: