From owner-dev-commits-src-branches@freebsd.org Wed Jul 21 16:19:01 2021 Return-Path: Delivered-To: dev-commits-src-branches@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 7D966666D22; Wed, 21 Jul 2021 16:19:01 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4GVLPH6zkQz3Mnj; Wed, 21 Jul 2021 16:18:59 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id BE9102123E; Wed, 21 Jul 2021 16:18:59 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 16LGIxEV081588; Wed, 21 Jul 2021 16:18:59 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 16LGIxIe081587; Wed, 21 Jul 2021 16:18:59 GMT (envelope-from git) Date: Wed, 21 Jul 2021 16:18:59 GMT Message-Id: <202107211618.16LGIxIe081587@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Warner Losh Subject: git: c23e2b282bcb - stable/12 - nvme: Enable interrupts after qpair fully constructed MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: imp X-Git-Repository: src X-Git-Refname: refs/heads/stable/12 X-Git-Reftype: branch X-Git-Commit: c23e2b282bcb89bb4bf46b380ba8db11b914b436 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-branches@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commits to the stable branches of the FreeBSD src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Jul 2021 16:19:03 -0000 The branch stable/12 has been updated by imp: URL: https://cgit.FreeBSD.org/src/commit/?id=c23e2b282bcb89bb4bf46b380ba8db11b914b436 commit c23e2b282bcb89bb4bf46b380ba8db11b914b436 Author: Warner Losh AuthorDate: 2021-07-15 22:17:23 +0000 Commit: Warner Losh 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: