git: c23e2b282bcb - stable/12 - nvme: Enable interrupts after qpair fully constructed

[Warner Losh]

The branch stable/12 has been updated by imp:

URL: https://cgit.FreeBSD.org/src/commit/?id=c23e2b282bcb89bb4bf46b380ba8db11b914b436

commit c23e2b282bcb89bb4bf46b380ba8db11b914b436
Author:     Warner Losh <imp at FreeBSD.org>
AuthorDate: 2021-07-15 22:17:23 +0000
Commit:     Warner Losh <imp at FreeBSD.org>
CommitDate: 2021-07-21 16:16:33 +0000

    nvme: Enable interrupts after qpair fully constructed
    
    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
    
    (cherry picked from commit fc9a0840231770bc7e7dcfe4616babdc6d4389a6)
---
 sys/dev/nvme/nvme_qpair.c | 50 +++++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/sys/dev/nvme/nvme_qpair.c b/sys/dev/nvme/nvme_qpair.c
index 0fe80c645f1c..9be4fc67b923 100644
--- a/sys/dev/nvme/nvme_qpair.c
+++ b/sys/dev/nvme/nvme_qpair.c
@@ -661,31 +661,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. */
@@ -806,6 +781,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: