git: 5ebc003a2882 - stable/13 - bhyve: abort and return FEATURE_NOT_SAVEABLE while set feature with a save flag for NVMe controller.

From: Corvin Köhne <corvink_at_FreeBSD.org>
Date: Wed, 23 Nov 2022 09:44:04 UTC
The branch stable/13 has been updated by corvink:

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

commit 5ebc003a2882a35054a93411a2c4d60f6e9cec3c
Author:     Wanpeng Qian <wanpengqian@gmail.com>
AuthorDate: 2022-11-14 13:02:44 +0000
Commit:     Corvin Köhne <corvink@FreeBSD.org>
CommitDate: 2022-11-22 06:51:28 +0000

    bhyve: abort and return FEATURE_NOT_SAVEABLE while set feature with a save flag for NVMe controller.
    
    Currently bhyve's NVMe controller cannot save feature values cross
    reboot. It should return a FEATURE_NOT_SAVEABLE error when the command
    specifies a save flag.
    
    Quote from NVMe specification, page 205:
    
    https://nvmexpress.org/wp-content/uploads/NVM-Express-1_4-2019.06.10-Ratified.pdf
    
    If the Feature Identifier specified in the Set Features command is not
    saveable by the controller and the controller receives a Set Features
    command with the Save bit set to one, then the command shall be aborted
    with a status of Feature Identifier Not Saveable.
    
    Reviewed by:            chuck (older version)
    Approved by:            manu (mentor)
    MFC after:              1 week
    Differential Revision:  https://reviews.freebsd.org/D32767
    
    (cherry picked from commit 8ab99dbea16728f3e34137310587a6aeb3f3d317)
---
 sys/dev/nvme/nvme.h       | 16 ++++++++++++++++
 usr.sbin/bhyve/pci_nvme.c | 10 +++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/sys/dev/nvme/nvme.h b/sys/dev/nvme/nvme.h
index 177f18f916c1..2e0fb27c1d99 100644
--- a/sys/dev/nvme/nvme.h
+++ b/sys/dev/nvme/nvme.h
@@ -563,9 +563,25 @@ enum nvme_critical_warning_state {
 #define	NVME_SS_PAGE_SSTAT_GDE_SHIFT			(8)
 #define	NVME_SS_PAGE_SSTAT_GDE_MASK			(0x1)
 
+/* Features */
+/* Get Features */
+#define NVME_FEAT_GET_SEL_SHIFT				(8)
+#define NVME_FEAT_GET_SEL_MASK				(0x7)
+#define NVME_FEAT_GET_FID_SHIFT				(0)
+#define NVME_FEAT_GET_FID_MASK				(0xff)
+
+/* Set Features */
+#define NVME_FEAT_SET_SV_SHIFT				(31)
+#define NVME_FEAT_SET_SV_MASK				(0x1)
+#define NVME_FEAT_SET_FID_SHIFT				(0)
+#define NVME_FEAT_SET_FID_MASK				(0xff)
+
 /* Helper macro to combine *_MASK and *_SHIFT defines */
 #define NVMEB(name)	(name##_MASK << name##_SHIFT)
 
+/* Helper macro to extract value from x */
+#define NVMEV(name, x)  (((x) >> name##_SHIFT) & name##_MASK)
+
 /* CC register SHN field values */
 enum shn_value {
 	NVME_SHN_NORMAL		= 0x1,
diff --git a/usr.sbin/bhyve/pci_nvme.c b/usr.sbin/bhyve/pci_nvme.c
index 91cf919f49fd..9704081c12af 100644
--- a/usr.sbin/bhyve/pci_nvme.c
+++ b/usr.sbin/bhyve/pci_nvme.c
@@ -1820,7 +1820,8 @@ nvme_opc_set_features(struct pci_nvme_softc *sc, struct nvme_command *command,
 {
 	struct nvme_feature_obj *feat;
 	uint32_t nsid = command->nsid;
-	uint8_t fid = command->cdw10 & 0xFF;
+	uint8_t fid = NVMEV(NVME_FEAT_SET_FID, command->cdw10);
+	bool sv = NVMEV(NVME_FEAT_SET_SV, command->cdw10);
 
 	DPRINTF("%s: Feature ID 0x%x (%s)", __func__, fid, nvme_fid_to_name(fid));
 
@@ -1829,6 +1830,13 @@ nvme_opc_set_features(struct pci_nvme_softc *sc, struct nvme_command *command,
 		pci_nvme_status_genc(&compl->status, NVME_SC_INVALID_FIELD);
 		return (1);
 	}
+
+	if (sv) {
+		pci_nvme_status_tc(&compl->status, NVME_SCT_COMMAND_SPECIFIC,
+		    NVME_SC_FEATURE_NOT_SAVEABLE);
+		return (1);
+	}
+
 	feat = &sc->feat[fid];
 
 	if (feat->namespace_specific && (nsid == NVME_GLOBAL_NAMESPACE_TAG)) {