Re: [PATCH 15/16] nvme: factor out cmb setup

[Philippe Mathieu-Daudé]

On 4/15/20 12:24 PM, Klaus Jensen wrote:

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

Signed-off-by: Klaus Jensen <k.jen...@samsung.com>
---
  hw/block/nvme.c | 50 +++++++++++++++++++++++++++----------------------
  1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 16d01af53a07..7387cf409f96 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -44,6 +44,8 @@
  #include "trace.h"
  #include "nvme.h"

+#define NVME_CMB_BIR 2

+
  #define NVME_GUEST_ERR(trace, fmt, ...) \
      do { \
          (trace_##trace)(__VA_ARGS__); \
@@ -1382,6 +1384,28 @@ static int nvme_init_namespace(NvmeCtrl *n, 
NvmeNamespace *ns, Error **errp)
      return 0;
  }

+static void nvme_init_cmb(NvmeCtrl *n, PCIDevice *pci_dev)

+{
+    NVME_CMBLOC_SET_BIR(n->bar.cmbloc, NVME_CMB_BIR);
+    NVME_CMBLOC_SET_OFST(n->bar.cmbloc, 0);
+
+    NVME_CMBSZ_SET_SQS(n->bar.cmbsz, 1);
+    NVME_CMBSZ_SET_CQS(n->bar.cmbsz, 0);
+    NVME_CMBSZ_SET_LISTS(n->bar.cmbsz, 0);
+    NVME_CMBSZ_SET_RDS(n->bar.cmbsz, 1);
+    NVME_CMBSZ_SET_WDS(n->bar.cmbsz, 1);
+    NVME_CMBSZ_SET_SZU(n->bar.cmbsz, 2);

We lost the /* MBs */ comment, otherwise:
Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>

+    NVME_CMBSZ_SET_SZ(n->bar.cmbsz, n->params.cmb_size_mb);
+
+    n->cmbuf = g_malloc0(NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
+    memory_region_init_io(&n->ctrl_mem, OBJECT(n), &nvme_cmb_ops, n,
+                          "nvme-cmb", NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
+    pci_register_bar(pci_dev, NVME_CMBLOC_BIR(n->bar.cmbloc),
+                     PCI_BASE_ADDRESS_SPACE_MEMORY |
+                     PCI_BASE_ADDRESS_MEM_TYPE_64 |
+                     PCI_BASE_ADDRESS_MEM_PREFETCH, &n->ctrl_mem);
+}
+
  static void nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev)
  {
      uint8_t *pci_conf = pci_dev->config;
@@ -1398,6 +1422,10 @@ static void nvme_init_pci(NvmeCtrl *n, PCIDevice 
*pci_dev)
      pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
                       PCI_BASE_ADDRESS_MEM_TYPE_64, &n->iomem);
      msix_init_exclusive_bar(pci_dev, n->params.max_ioqpairs + 1, 4, NULL);
+
+    if (n->params.cmb_size_mb) {
+        nvme_init_cmb(n, pci_dev);
+    }
  }

static void nvme_realize(PCIDevice *pci_dev, Error **errp)

@@ -1454,28 +1482,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error 
**errp)
      n->bar.vs = 0x00010200;
      n->bar.intmc = n->bar.intms = 0;

- if (n->params.cmb_size_mb) {

-
-        NVME_CMBLOC_SET_BIR(n->bar.cmbloc, 2);
-        NVME_CMBLOC_SET_OFST(n->bar.cmbloc, 0);
-
-        NVME_CMBSZ_SET_SQS(n->bar.cmbsz, 1);
-        NVME_CMBSZ_SET_CQS(n->bar.cmbsz, 0);
-        NVME_CMBSZ_SET_LISTS(n->bar.cmbsz, 0);
-        NVME_CMBSZ_SET_RDS(n->bar.cmbsz, 1);
-        NVME_CMBSZ_SET_WDS(n->bar.cmbsz, 1);
-        NVME_CMBSZ_SET_SZU(n->bar.cmbsz, 2); /* MBs */
-        NVME_CMBSZ_SET_SZ(n->bar.cmbsz, n->params.cmb_size_mb);
-
-        n->cmbuf = g_malloc0(NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
-        memory_region_init_io(&n->ctrl_mem, OBJECT(n), &nvme_cmb_ops, n,
-                              "nvme-cmb", NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
-        pci_register_bar(pci_dev, NVME_CMBLOC_BIR(n->bar.cmbloc),
-            PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64 |
-            PCI_BASE_ADDRESS_MEM_PREFETCH, &n->ctrl_mem);
-
-    }
-
      for (i = 0; i < n->num_namespaces; i++) {
          if (nvme_init_namespace(n, &n->namespaces[i], errp)) {
              return;