From owner-dev-commits-src-all@freebsd.org Fri Oct 1 17:10:24 2021 Return-Path: Delivered-To: dev-commits-src-all@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 B12EF6B2AC4; Fri, 1 Oct 2021 17:10:24 +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 4HLc7N4Zwmz4lMQ; Fri, 1 Oct 2021 17:10:24 +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 7E8112333D; Fri, 1 Oct 2021 17:10:24 +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 191HAODo026552; Fri, 1 Oct 2021 17:10:24 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 191HAO5w026551; Fri, 1 Oct 2021 17:10:24 GMT (envelope-from git) Date: Fri, 1 Oct 2021 17:10:24 GMT Message-Id: <202110011710.191HAO5w026551@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Warner Losh Subject: git: 4b3da659bf62 - main - nvme: Only reset once on attach. 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/main X-Git-Reftype: branch X-Git-Commit: 4b3da659bf62b0f5306b5acee9add41b84361498 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 01 Oct 2021 17:10:24 -0000 The branch main has been updated by imp: URL: https://cgit.FreeBSD.org/src/commit/?id=4b3da659bf62b0f5306b5acee9add41b84361498 commit 4b3da659bf62b0f5306b5acee9add41b84361498 Author: Warner Losh AuthorDate: 2021-10-01 17:09:34 +0000 Commit: Warner Losh CommitDate: 2021-10-01 17:09:34 +0000 nvme: Only reset once on attach. The FreeBSD nvme driver has reset the nvme controller twice on attach to address a theoretical issue assuring the hardware is in a known state. However, exierence has shown the second reset is unnecessary and increases the time to boot. Eliminate the second reset. Should there be a situation when you need a second reset (for buggy or at least somewhat out of the mainstream hardware), the hardware option NVME_2X_RESET will restore the old behavior. Document this in nvme(4). If there's any trouble at all with this, I'll add a sysctl tunable to control it. Sponsored by: Netflix Reviewed by: cperciva, mav Differential Revision: https://reviews.freebsd.org/D32241 --- share/man/man4/nvme.4 | 8 ++++++++ sys/conf/options | 1 + sys/dev/nvme/nvme_ctrlr.c | 27 +++++++++++++++++---------- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/share/man/man4/nvme.4 b/share/man/man4/nvme.4 index db31b6f26b3e..eb9f9b222f51 100644 --- a/share/man/man4/nvme.4 +++ b/share/man/man4/nvme.4 @@ -172,6 +172,14 @@ value in hw.nvme.verbose_cmd_dump=1 .Ed .Pp +Prior versions of the driver reset the card twice on boot. +This proved to be unnecessary and inefficient, so the driver now resets drive +controller only once. +The old behavior may be restored in the kernel config file with +.Bd -literal -offset indent +.Cd options NVME_2X_RESET +.Ed +.Pp .Sh SYSCTL VARIABLES The following controller-level sysctls are currently implemented: .Bl -tag -width indent diff --git a/sys/conf/options b/sys/conf/options index 42703b621752..2d99dc8c67db 100644 --- a/sys/conf/options +++ b/sys/conf/options @@ -1006,6 +1006,7 @@ EKCD opt_ekcd.h # NVME options NVME_USE_NVD opt_nvme.h +NVME_2X_RESET opt_nvme.h # amdsbwd options AMDSBWD_DEBUG opt_amdsbwd.h diff --git a/sys/dev/nvme/nvme_ctrlr.c b/sys/dev/nvme/nvme_ctrlr.c index d0be0da39902..ca89e99d2934 100644 --- a/sys/dev/nvme/nvme_ctrlr.c +++ b/sys/dev/nvme/nvme_ctrlr.c @@ -30,6 +30,7 @@ __FBSDID("$FreeBSD$"); #include "opt_cam.h" +#include "opt_nvme.h" #include #include @@ -1135,12 +1136,6 @@ nvme_ctrlr_start_config_hook(void *arg) TSENTER(); - /* - * Reset controller twice to ensure we do a transition from cc.en==1 to - * cc.en==0. This is because we don't really know what status the - * controller was left in when boot handed off to OS. Linux doesn't do - * this, however. If we adopt that policy, see also nvme_ctrlr_resume(). - */ if (nvme_ctrlr_hw_reset(ctrlr) != 0) { fail: nvme_ctrlr_fail(ctrlr); @@ -1148,8 +1143,17 @@ fail: return; } +#ifdef NVME_2X_RESET + /* + * Reset controller twice to ensure we do a transition from cc.en==1 to + * cc.en==0. This is because we don't really know what status the + * controller was left in when boot handed off to OS. Linux doesn't do + * this, however, and when the controller is in state cc.en == 0, no + * I/O can happen. + */ if (nvme_ctrlr_hw_reset(ctrlr) != 0) goto fail; +#endif nvme_qpair_reset(&ctrlr->adminq); nvme_admin_qpair_enable(&ctrlr->adminq); @@ -1672,14 +1676,17 @@ nvme_ctrlr_resume(struct nvme_controller *ctrlr) if (ctrlr->is_failed) return (0); - /* - * Have to reset the hardware twice, just like we do on attach. See - * nmve_attach() for why. - */ if (nvme_ctrlr_hw_reset(ctrlr) != 0) goto fail; +#ifdef NVME_2X_RESET + /* + * Prior to FreeBSD 13.1, FreeBSD's nvme driver reset the hardware twice + * to get it into a known good state. However, the hardware's state is + * good and we don't need to do this for proper functioning. + */ if (nvme_ctrlr_hw_reset(ctrlr) != 0) goto fail; +#endif /* * Now that we've reset the hardware, we can restart the controller. Any