diff mbox series

[4/8] hw/block/nvme: fix controller reset/shutdown logic

Message ID 20201218132905.967326-5-its@irrelevant.dk
State New
Headers show
Series hw/block/nvme: misc cmb/pmr patches | expand

Commit Message

Klaus Jensen Dec. 18, 2020, 1:29 p.m. UTC 
From: Klaus Jensen <k.jensen@samsung.com>

Differentiate between Controller Level Reset (i.e. CC.EN transitions
from 1 to 0) and Controller Shutdown (i.e. host sets CC.SHN to 01b or
10b). Prior to this patch the controller would perform the equivalent of
a Controller Level Reset when the host set CC.SHN to 01b or 10b. This
erroneously entails clearing CC.

For a Shutdown, it is the host that *should* delete any I/O queues.
Setting CC.EN to 0 is not part of the Shutdown procedure and it is
specifically recommended that the host does NOT do this. The host should
also stop issuing new I/O commands and allow any outstanding ones to
complete before setting CC.SHN. The only guarantee the controller must
give is that it is safe to power off the controller when CSTS.SHST
indicates that shutdown processing is complete. Do so by issuing flushes
for the attached namespaces and the PMR (if in use) and set CSTS.SHST to
10b. Specifically do not care about draining since the host *should* not
have left any outstanding commands around. If it did, behavior is
undefined.

On the other hand, a Controller Level Reset is not about being safe for
shutdown - so only drain here, do not explicitly flush.

Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/block/nvme.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)
diff mbox series

Patch

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 69f2fe4bd4ff..96c9f9be3dc4 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -2262,7 +2262,26 @@  static void nvme_process_sq(void *opaque)
     }
 }
 
-static void nvme_clear_ctrl(NvmeCtrl *n)
+static void nvme_ctrl_shutdown(NvmeCtrl *n)
+{
+    NvmeNamespace *ns;
+    int i;
+
+    if (n->pmrdev) {
+        memory_region_msync(&n->pmrdev->mr, 0, n->pmrdev->size);
+    }
+
+    for (i = 1; i <= n->num_namespaces; i++) {
+        ns = nvme_ns(n, i);
+        if (!ns) {
+            continue;
+        }
+
+        nvme_ns_flush(ns);
+    }
+}
+
+static void nvme_ctrl_reset(NvmeCtrl *n)
 {
     NvmeNamespace *ns;
     int i;
@@ -2297,15 +2316,6 @@  static void nvme_clear_ctrl(NvmeCtrl *n)
     n->outstanding_aers = 0;
     n->qs_created = false;
 
-    for (i = 1; i <= n->num_namespaces; i++) {
-        ns = nvme_ns(n, i);
-        if (!ns) {
-            continue;
-        }
-
-        nvme_ns_flush(ns);
-    }
-
     n->bar.cc = 0;
 }
 
@@ -2473,12 +2483,12 @@  static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
             }
         } else if (!NVME_CC_EN(data) && NVME_CC_EN(n->bar.cc)) {
             trace_pci_nvme_mmio_stopped();
-            nvme_clear_ctrl(n);
+            nvme_ctrl_reset(n);
             n->bar.csts &= ~NVME_CSTS_READY;
         }
         if (NVME_CC_SHN(data) && !(NVME_CC_SHN(n->bar.cc))) {
             trace_pci_nvme_mmio_shutdown_set();
-            nvme_clear_ctrl(n);
+            nvme_ctrl_shutdown(n);
             n->bar.cc = data;
             n->bar.csts |= NVME_CSTS_SHST_COMPLETE;
         } else if (!NVME_CC_SHN(data) && NVME_CC_SHN(n->bar.cc)) {
@@ -3110,7 +3120,8 @@  static void nvme_exit(PCIDevice *pci_dev)
 {
     NvmeCtrl *n = NVME(pci_dev);
 
-    nvme_clear_ctrl(n);
+    nvme_ctrl_reset(n);
+
     g_free(n->cq);
     g_free(n->sq);
     g_free(n->aer_reqs);