qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Klaus Jensen <its@irrelevant.dk>
To: Peter Maydell <peter.maydell@linaro.org>, qemu-devel@nongnu.org
Cc: "Stefan Hajnoczi" <stefanha@redhat.com>,
	"Igor Mammedov" <imammedo@redhat.com>,
	"Ani Sinha" <ani@anisinha.ca>, "Hanna Reitz" <hreitz@redhat.com>,
	"Kevin Wolf" <kwolf@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Klaus Jensen" <its@irrelevant.dk>,
	qemu-block@nongnu.org, "Keith Busch" <kbusch@kernel.org>,
	"Fam Zheng" <fam@euphon.net>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
	"Klaus Jensen" <k.jensen@samsung.com>,
	"Łukasz Gieryk" <lukasz.gieryk@linux.intel.com>,
	"Lukasz Maniak" <lukasz.maniak@linux.intel.com>
Subject: [PULL 13/15] hw/nvme: clean up CC register write logic
Date: Thu, 23 Jun 2022 23:34:40 +0200	[thread overview]
Message-ID: <20220623213442.67789-14-its@irrelevant.dk> (raw)
In-Reply-To: <20220623213442.67789-1-its@irrelevant.dk>

From: Klaus Jensen <k.jensen@samsung.com>

The SRIOV series exposed an issued with how CC register writes are
handled and how CSTS is set in response to that. Specifically, after
applying the SRIOV series, the controller could end up in a state with
CC.EN set to '1' but with CSTS.RDY cleared to '0', causing drivers to
expect CSTS.RDY to transition to '1' but timing out.

Clean this up.

Reviewed-by: Łukasz Gieryk <lukasz.gieryk@linux.intel.com>
Reviewed-by: Lukasz Maniak <lukasz.maniak@linux.intel.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/ctrl.c | 38 ++++++++++++++++----------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 658584d417fe..a558f5cb29c1 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -6190,10 +6190,15 @@ static void nvme_ctrl_reset(NvmeCtrl *n, NvmeResetType rst)
 
     if (pci_is_vf(pci_dev)) {
         sctrl = nvme_sctrl(n);
+
         stl_le_p(&n->bar.csts, sctrl->scs ? 0 : NVME_CSTS_FAILED);
     } else {
         stl_le_p(&n->bar.csts, 0);
     }
+
+    stl_le_p(&n->bar.intms, 0);
+    stl_le_p(&n->bar.intmc, 0);
+    stl_le_p(&n->bar.cc, 0);
 }
 
 static void nvme_ctrl_shutdown(NvmeCtrl *n)
@@ -6405,20 +6410,21 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
         nvme_irq_check(n);
         break;
     case NVME_REG_CC:
+        stl_le_p(&n->bar.cc, data);
+
         trace_pci_nvme_mmio_cfg(data & 0xffffffff);
 
-        /* Windows first sends data, then sends enable bit */
-        if (!NVME_CC_EN(data) && !NVME_CC_EN(cc) &&
-            !NVME_CC_SHN(data) && !NVME_CC_SHN(cc))
-        {
-            cc = data;
+        if (NVME_CC_SHN(data) && !(NVME_CC_SHN(cc))) {
+            trace_pci_nvme_mmio_shutdown_set();
+            nvme_ctrl_shutdown(n);
+            csts &= ~(CSTS_SHST_MASK << CSTS_SHST_SHIFT);
+            csts |= NVME_CSTS_SHST_COMPLETE;
+        } else if (!NVME_CC_SHN(data) && NVME_CC_SHN(cc)) {
+            trace_pci_nvme_mmio_shutdown_cleared();
+            csts &= ~(CSTS_SHST_MASK << CSTS_SHST_SHIFT);
         }
 
         if (NVME_CC_EN(data) && !NVME_CC_EN(cc)) {
-            cc = data;
-
-            /* flush CC since nvme_start_ctrl() needs the value */
-            stl_le_p(&n->bar.cc, cc);
             if (unlikely(nvme_start_ctrl(n))) {
                 trace_pci_nvme_err_startfail();
                 csts = NVME_CSTS_FAILED;
@@ -6429,22 +6435,10 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
         } else if (!NVME_CC_EN(data) && NVME_CC_EN(cc)) {
             trace_pci_nvme_mmio_stopped();
             nvme_ctrl_reset(n, NVME_RESET_CONTROLLER);
-            cc = 0;
-            csts &= ~NVME_CSTS_READY;
-        }
 
-        if (NVME_CC_SHN(data) && !(NVME_CC_SHN(cc))) {
-            trace_pci_nvme_mmio_shutdown_set();
-            nvme_ctrl_shutdown(n);
-            cc = data;
-            csts |= NVME_CSTS_SHST_COMPLETE;
-        } else if (!NVME_CC_SHN(data) && NVME_CC_SHN(cc)) {
-            trace_pci_nvme_mmio_shutdown_cleared();
-            csts &= ~NVME_CSTS_SHST_COMPLETE;
-            cc = data;
+            break;
         }
 
-        stl_le_p(&n->bar.cc, cc);
         stl_le_p(&n->bar.csts, csts);
 
         break;
-- 
2.36.1



  parent reply	other threads:[~2022-06-23 21:57 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-23 21:34 [PULL 00/15] hw/nvme updates Klaus Jensen
2022-06-23 21:34 ` [PULL 01/15] hw/nvme: Add support for SR-IOV Klaus Jensen
2022-06-23 21:34 ` [PULL 02/15] hw/nvme: Add support for Primary Controller Capabilities Klaus Jensen
2022-06-23 21:34 ` [PULL 03/15] hw/nvme: Add support for Secondary Controller List Klaus Jensen
2022-06-23 21:34 ` [PULL 04/15] hw/nvme: Implement the Function Level Reset Klaus Jensen
2022-06-23 21:34 ` [PULL 05/15] hw/nvme: Make max_ioqpairs and msix_qsize configurable in runtime Klaus Jensen
2022-06-23 21:34 ` [PULL 06/15] hw/nvme: Remove reg_size variable and update BAR0 size calculation Klaus Jensen
2022-06-23 21:34 ` [PULL 07/15] hw/nvme: Calculate BAR attributes in a function Klaus Jensen
2022-06-23 21:34 ` [PULL 08/15] hw/nvme: Initialize capability structures for primary/secondary controllers Klaus Jensen
2022-06-23 21:34 ` [PULL 09/15] hw/nvme: Add support for the Virtualization Management command Klaus Jensen
2022-06-23 21:34 ` [PULL 10/15] docs: Add documentation for SR-IOV and Virtualization Enhancements Klaus Jensen
2022-06-23 21:34 ` [PULL 11/15] hw/nvme: Update the initalization place for the AER queue Klaus Jensen
2022-06-23 21:34 ` [PULL 12/15] hw/acpi: Make the PCI hot-plug aware of SR-IOV Klaus Jensen
2022-06-23 21:34 ` Klaus Jensen [this message]
2022-06-23 21:34 ` [PULL 14/15] Revert "hw/block/nvme: add support for sgl bit bucket descriptor" Klaus Jensen
2022-06-23 21:34 ` [PULL 15/15] hw/nvme: clear aen mask on reset Klaus Jensen
2022-06-23 23:28 ` [PULL 00/15] hw/nvme updates Richard Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220623213442.67789-14-its@irrelevant.dk \
    --to=its@irrelevant.dk \
    --cc=ani@anisinha.ca \
    --cc=f4bug@amsat.org \
    --cc=fam@euphon.net \
    --cc=hreitz@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=k.jensen@samsung.com \
    --cc=kbusch@kernel.org \
    --cc=kwolf@redhat.com \
    --cc=lukasz.gieryk@linux.intel.com \
    --cc=lukasz.maniak@linux.intel.com \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=mst@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).