git: 4b3da659bf62 - main - nvme: Only reset once on attach.

[Warner Losh]

The branch main has been updated by imp:

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

commit 4b3da659bf62b0f5306b5acee9add41b84361498
Author:     Warner Losh <imp at FreeBSD.org>
AuthorDate: 2021-10-01 17:09:34 +0000
Commit:     Warner Losh <imp at FreeBSD.org>
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 <sys/param.h>
 #include <sys/systm.h>
@@ -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