[PATCH 0/2] hw/nvme: add monitor commands for inspecting state

[PATCH 0/2] hw/nvme: add monitor commands for inspecting state

[Mateusz Nowicki]

Add two HMP commands for inspecting emulated NVMe controllers from
the QEMU monitor without attaching gdb to the QEMU process:

  - 'info nvme'        - per-controller summary (PCI, identify
                         fields, CC/CSTS/AQA, queue counts)
  - 'info nvme-queues' - per-queue listing of admin and I/O SQ/CQ
                         (size, head/tail, PRP1, doorbell offset,
                         phase tag)

Useful for verifying queue setup, doorbell rings, AERs held in the
admin SQ and similar driver/controller interaction details from a
running QEMU monitor.

Mateusz Nowicki (2):
  hw/nvme: add 'info nvme' HMP command
  hw/nvme: add 'info nvme-queues' HMP command

 hmp-commands-info.hx |  28 +++++++++
 hw/nvme/meson.build  |   2 +-
 hw/nvme/monitor.c    | 139 +++++++++++++++++++++++++++++++++++++++++++
 qapi/machine.json    |  34 +++++++++++
 4 files changed, 202 insertions(+), 1 deletion(-)
 create mode 100644 hw/nvme/monitor.c

-- 
2.53.0

[PATCH 1/2] hw/nvme: add 'info nvme' HMP command

[Mateusz Nowicki]

Add an 'info nvme' HMP command for inspecting emulated NVMe
controllers from the QEMU monitor.

For each NVMe controller in the QOM tree the command prints:

  - PCI BDF, vendor/device ID and BAR0 base address;
  - Identify Controller fields visible to the host driver: SN, MN, FR
    and CNTLID;
  - the CC, CSTS and AQA registers (raw 32-bit values);
  - the number of admin and active I/O submission/completion queues.

Useful when debugging the Linux NVMe driver against the QEMU emulation
without attaching gdb to the QEMU process.

Example:

  (qemu) info nvme
  /machine/peripheral-anon/device[0]
    PCI:    BDF 00:04.0  VID=8086 DID=5845  BAR0=0x00000000feb50000
    ID:     SN=NVME0001  MN=QEMU NVMe Ctrl  FR=...  CNTLID=0x0000
    CC:     0x00460001
    CSTS:   0x00000001
    AQA:    0x001f001f
    Queues: 1 admin + 4 IO SQ / 4 IO CQ

Signed-off-by: Mateusz Nowicki <mateusz.nowicki@posteo.net>
---
 hmp-commands-info.hx | 13 ++++++++
 hw/nvme/meson.build  |  2 +-
 hw/nvme/monitor.c    | 76 ++++++++++++++++++++++++++++++++++++++++++++
 qapi/machine.json    | 17 ++++++++++
 4 files changed, 107 insertions(+), 1 deletion(-)
 create mode 100644 hw/nvme/monitor.c

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 82134eb6c2..b984691c3c 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -340,6 +340,19 @@ SRST
     Show guest USB devices.
 ERST
 
+    {
+        .name       = "nvme",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show emulated NVMe controllers",
+        .cmd_info_hrt = qmp_x_query_nvme,
+    },
+
+SRST
+  ``info nvme``
+    Show emulated NVMe controllers.
+ERST
+
     {
         .name       = "usbhost",
         .args_type  = "",
diff --git a/hw/nvme/meson.build b/hw/nvme/meson.build
index 7d5caa53c2..3017f058f9 100644
--- a/hw/nvme/meson.build
+++ b/hw/nvme/meson.build
@@ -1 +1 @@
-system_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('ctrl.c', 'dif.c', 'ns.c', 'subsys.c', 'nguid.c'))
\ No newline at end of file
+system_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('ctrl.c', 'dif.c', 'ns.c', 'subsys.c', 'nguid.c', 'monitor.c'))
\ No newline at end of file
diff --git a/hw/nvme/monitor.c b/hw/nvme/monitor.c
new file mode 100644
index 0000000000..95a6754437
--- /dev/null
+++ b/hw/nvme/monitor.c
@@ -0,0 +1,76 @@
+/*
+ * QEMU NVMe Controller monitor commands
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-machine.h"
+#include "qapi/type-helpers.h"
+#include "hw/pci/pci.h"
+
+#include "nvme.h"
+
+static int collect_one(Object *obj, void *opaque)
+{
+    GString *buf = opaque;
+    NvmeCtrl *n;
+    PCIDevice *pci;
+    pcibus_t bar0;
+    unsigned io_sq = 0, io_cq = 0;
+
+    if (!object_dynamic_cast(obj, TYPE_NVME)) {
+        return 0;
+    }
+    n = NVME(obj);
+    pci = PCI_DEVICE(n);
+    bar0 = pci_get_bar_addr(pci, 0);
+
+    for (unsigned i = 1; i <= n->params.max_ioqpairs; i++) {
+        if (n->sq && n->sq[i]) {
+            io_sq++;
+        }
+        if (n->cq && n->cq[i]) {
+            io_cq++;
+        }
+    }
+
+    g_string_append_printf(buf, "%s\n", object_get_canonical_path(obj));
+    g_string_append_printf(buf,
+        "  PCI:    BDF %02x:%02x.%x  VID=%04x DID=%04x  ",
+        pci_dev_bus_num(pci), PCI_SLOT(pci->devfn), PCI_FUNC(pci->devfn),
+        pci_get_word(pci->config + PCI_VENDOR_ID),
+        pci_get_word(pci->config + PCI_DEVICE_ID));
+    if (bar0 == PCI_BAR_UNMAPPED) {
+        g_string_append(buf, "BAR0=unmapped\n");
+    } else {
+        g_string_append_printf(buf, "BAR0=0x%016" PRIx64 "\n",
+                               (uint64_t)bar0);
+    }
+    g_string_append_printf(buf,
+        "  ID:     SN=%.20s  MN=%.40s  FR=%.8s  CNTLID=0x%04x\n",
+        n->id_ctrl.sn, n->id_ctrl.mn, n->id_ctrl.fr, n->cntlid);
+    g_string_append_printf(buf, "  CC:     0x%08x\n",
+                           ldl_le_p(&n->bar.cc));
+    g_string_append_printf(buf, "  CSTS:   0x%08x\n",
+                           ldl_le_p(&n->bar.csts));
+    g_string_append_printf(buf, "  AQA:    0x%08x\n",
+                           ldl_le_p(&n->bar.aqa));
+    g_string_append_printf(buf,
+        "  Queues: 1 admin + %u IO SQ / %u IO CQ\n", io_sq, io_cq);
+    return 0;
+}
+
+HumanReadableText *qmp_x_query_nvme(Error **errp)
+{
+    g_autoptr(GString) buf = g_string_new("");
+
+    object_child_foreach_recursive(object_get_root(), collect_one, buf);
+
+    if (buf->len == 0) {
+        g_string_append(buf, "no NVMe controllers\n");
+    }
+
+    return human_readable_text_from_str(buf);
+}
diff --git a/qapi/machine.json b/qapi/machine.json
index 685e4e29b8..d4a589e768 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1772,6 +1772,23 @@
   'returns': 'HumanReadableText',
   'features': [ 'unstable' ] }
 
+##
+# @x-query-nvme:
+#
+# Query state of emulated NVMe controllers.
+#
+# Features:
+#
+# @unstable: This command is meant for debugging.
+#
+# Returns: NVMe controller state as human-readable text
+#
+# Since: 11.1
+##
+{ 'command': 'x-query-nvme',
+  'returns': 'HumanReadableText',
+  'features': [ 'unstable' ] }
+
 ##
 # @SmbiosEntryPointType:
 #
-- 
2.53.0

[PATCH 2/2] hw/nvme: add 'info nvme-queues' HMP command

[Mateusz Nowicki]

Add an 'info nvme-queues' HMP command that lists, for every emulated
NVMe controller, the admin SQ/CQ and every active I/O SQ/CQ.

For each queue the command prints:

  - the ring size, head and tail indices;
  - the associated CQ id (for SQ) or interrupt vector (for CQ);
  - the queue's PRP1 (DMA base address of the ring);
  - the BAR0-relative doorbell offset (SQyTDBL / CQyHDBL);
  - the current CQ phase tag.

The doorbell offsets are computed from CAP.DSTRD so they remain
correct if the emulation ever exposes a non-zero stride.

Useful when debugging the Linux NVMe driver against the QEMU
emulation - queue setup, doorbell rings, AERs held in the admin SQ
etc. - without attaching gdb to the QEMU process.

Example, with the long PRP1 ring address and BAR0-relative doorbell
offset replaced by '...'; phase tag column omitted for brevity:

  (qemu) info nvme-queues
  /machine/peripheral-anon/device[0]
  SQ 0  size=32    head=9     tail=9     cqid=0    prp1=...  SQTDBL=...
  CQ 0  size=32    head=8     tail=8     iv=0      prp1=...  CQHDBL=...
  SQ 1  size=1024  head=2     tail=2     cqid=1    prp1=...  SQTDBL=...
  CQ 1  size=1024  head=2     tail=2     iv=1      prp1=...  CQHDBL=...

Signed-off-by: Mateusz Nowicki <mateusz.nowicki@posteo.net>
---
 hmp-commands-info.hx | 15 +++++++++++
 hw/nvme/monitor.c    | 63 ++++++++++++++++++++++++++++++++++++++++++++
 qapi/machine.json    | 17 ++++++++++++
 3 files changed, 95 insertions(+)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index b984691c3c..874c1e1970 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -353,6 +353,21 @@ SRST
     Show emulated NVMe controllers.
 ERST
 
+    {
+        .name       = "nvme-queues",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show active NVMe queues and their doorbells",
+        .cmd_info_hrt = qmp_x_query_nvme_queues,
+    },
+
+SRST
+  ``info nvme-queues``
+    Show all active NVMe submission and completion queues, including
+    head/tail, DMA address of the ring and BAR0-relative doorbell
+    offset.
+ERST
+
     {
         .name       = "usbhost",
         .args_type  = "",
diff --git a/hw/nvme/monitor.c b/hw/nvme/monitor.c
index 95a6754437..c8161943c0 100644
--- a/hw/nvme/monitor.c
+++ b/hw/nvme/monitor.c
@@ -74,3 +74,66 @@ HumanReadableText *qmp_x_query_nvme(Error **errp)
 
     return human_readable_text_from_str(buf);
 }
+
+static void append_sq(GString *buf, NvmeSQueue *sq, unsigned stride)
+{
+    hwaddr db_off = 0x1000 + 2 * sq->sqid * stride;
+
+    g_string_append_printf(buf,
+        "  SQ %u  size=%-5u head=%-5u tail=%-5u cqid=%-3u  "
+        "prp1=0x%016" PRIx64 "  SQTDBL=BAR0+0x%03" HWADDR_PRIx "\n",
+        sq->sqid, sq->size, sq->head, sq->tail, sq->cqid,
+        sq->dma_addr, db_off);
+}
+
+static void append_cq(GString *buf, NvmeCQueue *cq, unsigned stride)
+{
+    hwaddr db_off = 0x1000 + (2 * cq->cqid + 1) * stride;
+
+    g_string_append_printf(buf,
+        "  CQ %u  size=%-5u head=%-5u tail=%-5u iv=%-5u  "
+        "prp1=0x%016" PRIx64 "  CQHDBL=BAR0+0x%03" HWADDR_PRIx
+        "  phaseTag=%u\n",
+        cq->cqid, cq->size, cq->head, cq->tail, cq->vector,
+        cq->dma_addr, db_off, cq->phase);
+}
+
+static int collect_queues(Object *obj, void *opaque)
+{
+    GString *buf = opaque;
+    NvmeCtrl *n;
+    unsigned stride;
+
+    if (!object_dynamic_cast(obj, TYPE_NVME)) {
+        return 0;
+    }
+    n = NVME(obj);
+    stride = 4u << NVME_CAP_DSTRD(ldq_le_p(&n->bar.cap));
+
+    g_string_append_printf(buf, "%s\n", object_get_canonical_path(obj));
+    append_sq(buf, &n->admin_sq, stride);
+    append_cq(buf, &n->admin_cq, stride);
+
+    for (unsigned i = 1; i <= n->params.max_ioqpairs; i++) {
+        if (n->sq && n->sq[i]) {
+            append_sq(buf, n->sq[i], stride);
+        }
+        if (n->cq && n->cq[i]) {
+            append_cq(buf, n->cq[i], stride);
+        }
+    }
+    return 0;
+}
+
+HumanReadableText *qmp_x_query_nvme_queues(Error **errp)
+{
+    g_autoptr(GString) buf = g_string_new("");
+
+    object_child_foreach_recursive(object_get_root(), collect_queues, buf);
+
+    if (buf->len == 0) {
+        g_string_append(buf, "no NVMe controllers\n");
+    }
+
+    return human_readable_text_from_str(buf);
+}
diff --git a/qapi/machine.json b/qapi/machine.json
index d4a589e768..c21b128e0a 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1789,6 +1789,23 @@
   'returns': 'HumanReadableText',
   'features': [ 'unstable' ] }
 
+##
+# @x-query-nvme-queues:
+#
+# Query state of all active SQ/CQ for emulated NVMe controllers.
+#
+# Features:
+#
+# @unstable: This command is meant for debugging.
+#
+# Returns: per-queue state as human-readable text
+#
+# Since: 11.1
+##
+{ 'command': 'x-query-nvme-queues',
+  'returns': 'HumanReadableText',
+  'features': [ 'unstable' ] }
+
 ##
 # @SmbiosEntryPointType:
 #
-- 
2.53.0

Re: [PATCH 1/2] hw/nvme: add 'info nvme' HMP command

[Dr. David Alan Gilbert]

* Mateusz Nowicki (mateusz.nowicki@posteo.net) wrote:
> Add an 'info nvme' HMP command for inspecting emulated NVMe
> controllers from the QEMU monitor.
> 
> For each NVMe controller in the QOM tree the command prints:
> 
>   - PCI BDF, vendor/device ID and BAR0 base address;
>   - Identify Controller fields visible to the host driver: SN, MN, FR
>     and CNTLID;
>   - the CC, CSTS and AQA registers (raw 32-bit values);
>   - the number of admin and active I/O submission/completion queues.
> 
> Useful when debugging the Linux NVMe driver against the QEMU emulation
> without attaching gdb to the QEMU process.
> 
> Example:
> 
>   (qemu) info nvme
>   /machine/peripheral-anon/device[0]
>     PCI:    BDF 00:04.0  VID=8086 DID=5845  BAR0=0x00000000feb50000
>     ID:     SN=NVME0001  MN=QEMU NVMe Ctrl  FR=...  CNTLID=0x0000
>     CC:     0x00460001
>     CSTS:   0x00000001
>     AQA:    0x001f001f
>     Queues: 1 admin + 4 IO SQ / 4 IO CQ

I won't claim to know what those NVMe magic numbers mean, but I'll
trust you they're useful.

> Signed-off-by: Mateusz Nowicki <mateusz.nowicki@posteo.net>
> ---
>  hmp-commands-info.hx | 13 ++++++++
>  hw/nvme/meson.build  |  2 +-
>  hw/nvme/monitor.c    | 76 ++++++++++++++++++++++++++++++++++++++++++++
>  qapi/machine.json    | 17 ++++++++++

So that lot looks OK from an HMP point of view, I wonder if
qapi/machine.json is the right place for it.  Markus?

Acked-by: Dr. David Alan Gilbert <dave@treblig.org>

>  4 files changed, 107 insertions(+), 1 deletion(-)
>  create mode 100644 hw/nvme/monitor.c
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 82134eb6c2..b984691c3c 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -340,6 +340,19 @@ SRST
>      Show guest USB devices.
>  ERST
>  
> +    {
> +        .name       = "nvme",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show emulated NVMe controllers",
> +        .cmd_info_hrt = qmp_x_query_nvme,
> +    },
> +
> +SRST
> +  ``info nvme``
> +    Show emulated NVMe controllers.
> +ERST
> +
>      {
>          .name       = "usbhost",
>          .args_type  = "",
> diff --git a/hw/nvme/meson.build b/hw/nvme/meson.build
> index 7d5caa53c2..3017f058f9 100644
> --- a/hw/nvme/meson.build
> +++ b/hw/nvme/meson.build
> @@ -1 +1 @@
> -system_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('ctrl.c', 'dif.c', 'ns.c', 'subsys.c', 'nguid.c'))
> \ No newline at end of file
> +system_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('ctrl.c', 'dif.c', 'ns.c', 'subsys.c', 'nguid.c', 'monitor.c'))
> \ No newline at end of file
> diff --git a/hw/nvme/monitor.c b/hw/nvme/monitor.c
> new file mode 100644
> index 0000000000..95a6754437
> --- /dev/null
> +++ b/hw/nvme/monitor.c
> @@ -0,0 +1,76 @@
> +/*
> + * QEMU NVMe Controller monitor commands
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-commands-machine.h"
> +#include "qapi/type-helpers.h"
> +#include "hw/pci/pci.h"
> +
> +#include "nvme.h"
> +
> +static int collect_one(Object *obj, void *opaque)
> +{
> +    GString *buf = opaque;
> +    NvmeCtrl *n;
> +    PCIDevice *pci;
> +    pcibus_t bar0;
> +    unsigned io_sq = 0, io_cq = 0;
> +
> +    if (!object_dynamic_cast(obj, TYPE_NVME)) {
> +        return 0;
> +    }
> +    n = NVME(obj);
> +    pci = PCI_DEVICE(n);
> +    bar0 = pci_get_bar_addr(pci, 0);
> +
> +    for (unsigned i = 1; i <= n->params.max_ioqpairs; i++) {
> +        if (n->sq && n->sq[i]) {
> +            io_sq++;
> +        }
> +        if (n->cq && n->cq[i]) {
> +            io_cq++;
> +        }
> +    }
> +
> +    g_string_append_printf(buf, "%s\n", object_get_canonical_path(obj));
> +    g_string_append_printf(buf,
> +        "  PCI:    BDF %02x:%02x.%x  VID=%04x DID=%04x  ",
> +        pci_dev_bus_num(pci), PCI_SLOT(pci->devfn), PCI_FUNC(pci->devfn),
> +        pci_get_word(pci->config + PCI_VENDOR_ID),
> +        pci_get_word(pci->config + PCI_DEVICE_ID));
> +    if (bar0 == PCI_BAR_UNMAPPED) {
> +        g_string_append(buf, "BAR0=unmapped\n");
> +    } else {
> +        g_string_append_printf(buf, "BAR0=0x%016" PRIx64 "\n",
> +                               (uint64_t)bar0);
> +    }
> +    g_string_append_printf(buf,
> +        "  ID:     SN=%.20s  MN=%.40s  FR=%.8s  CNTLID=0x%04x\n",
> +        n->id_ctrl.sn, n->id_ctrl.mn, n->id_ctrl.fr, n->cntlid);
> +    g_string_append_printf(buf, "  CC:     0x%08x\n",
> +                           ldl_le_p(&n->bar.cc));
> +    g_string_append_printf(buf, "  CSTS:   0x%08x\n",
> +                           ldl_le_p(&n->bar.csts));
> +    g_string_append_printf(buf, "  AQA:    0x%08x\n",
> +                           ldl_le_p(&n->bar.aqa));
> +    g_string_append_printf(buf,
> +        "  Queues: 1 admin + %u IO SQ / %u IO CQ\n", io_sq, io_cq);
> +    return 0;
> +}
> +
> +HumanReadableText *qmp_x_query_nvme(Error **errp)
> +{
> +    g_autoptr(GString) buf = g_string_new("");
> +
> +    object_child_foreach_recursive(object_get_root(), collect_one, buf);
> +
> +    if (buf->len == 0) {
> +        g_string_append(buf, "no NVMe controllers\n");
> +    }
> +
> +    return human_readable_text_from_str(buf);
> +}
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 685e4e29b8..d4a589e768 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1772,6 +1772,23 @@
>    'returns': 'HumanReadableText',
>    'features': [ 'unstable' ] }
>  
> +##
> +# @x-query-nvme:
> +#
> +# Query state of emulated NVMe controllers.
> +#
> +# Features:
> +#
> +# @unstable: This command is meant for debugging.
> +#
> +# Returns: NVMe controller state as human-readable text
> +#
> +# Since: 11.1
> +##
> +{ 'command': 'x-query-nvme',
> +  'returns': 'HumanReadableText',
> +  'features': [ 'unstable' ] }
> +
>  ##
>  # @SmbiosEntryPointType:
>  #
> -- 
> 2.53.0
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Re: [PATCH 2/2] hw/nvme: add 'info nvme-queues' HMP command

[Dr. David Alan Gilbert]

* Mateusz Nowicki (mateusz.nowicki@posteo.net) wrote:
> Add an 'info nvme-queues' HMP command that lists, for every emulated
> NVMe controller, the admin SQ/CQ and every active I/O SQ/CQ.
> 
> For each queue the command prints:
> 
>   - the ring size, head and tail indices;
>   - the associated CQ id (for SQ) or interrupt vector (for CQ);
>   - the queue's PRP1 (DMA base address of the ring);
>   - the BAR0-relative doorbell offset (SQyTDBL / CQyHDBL);
>   - the current CQ phase tag.
> 
> The doorbell offsets are computed from CAP.DSTRD so they remain
> correct if the emulation ever exposes a non-zero stride.
> 
> Useful when debugging the Linux NVMe driver against the QEMU
> emulation - queue setup, doorbell rings, AERs held in the admin SQ
> etc. - without attaching gdb to the QEMU process.
> 
> Example, with the long PRP1 ring address and BAR0-relative doorbell
> offset replaced by '...'; phase tag column omitted for brevity:
> 
>   (qemu) info nvme-queues
>   /machine/peripheral-anon/device[0]
>   SQ 0  size=32    head=9     tail=9     cqid=0    prp1=...  SQTDBL=...
>   CQ 0  size=32    head=8     tail=8     iv=0      prp1=...  CQHDBL=...
>   SQ 1  size=1024  head=2     tail=2     cqid=1    prp1=...  SQTDBL=...
>   CQ 1  size=1024  head=2     tail=2     iv=1      prp1=...  CQHDBL=...
> 
> Signed-off-by: Mateusz Nowicki <mateusz.nowicki@posteo.net>

as the previous patch; looks OK from HMP but check where the json should live.

Acked-by: Dr. David Alan Gilbert <dave@treblig.org>

> ---
>  hmp-commands-info.hx | 15 +++++++++++
>  hw/nvme/monitor.c    | 63 ++++++++++++++++++++++++++++++++++++++++++++
>  qapi/machine.json    | 17 ++++++++++++
>  3 files changed, 95 insertions(+)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index b984691c3c..874c1e1970 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -353,6 +353,21 @@ SRST
>      Show emulated NVMe controllers.
>  ERST
>  
> +    {
> +        .name       = "nvme-queues",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show active NVMe queues and their doorbells",
> +        .cmd_info_hrt = qmp_x_query_nvme_queues,
> +    },
> +
> +SRST
> +  ``info nvme-queues``
> +    Show all active NVMe submission and completion queues, including
> +    head/tail, DMA address of the ring and BAR0-relative doorbell
> +    offset.
> +ERST
> +
>      {
>          .name       = "usbhost",
>          .args_type  = "",
> diff --git a/hw/nvme/monitor.c b/hw/nvme/monitor.c
> index 95a6754437..c8161943c0 100644
> --- a/hw/nvme/monitor.c
> +++ b/hw/nvme/monitor.c
> @@ -74,3 +74,66 @@ HumanReadableText *qmp_x_query_nvme(Error **errp)
>  
>      return human_readable_text_from_str(buf);
>  }
> +
> +static void append_sq(GString *buf, NvmeSQueue *sq, unsigned stride)
> +{
> +    hwaddr db_off = 0x1000 + 2 * sq->sqid * stride;
> +
> +    g_string_append_printf(buf,
> +        "  SQ %u  size=%-5u head=%-5u tail=%-5u cqid=%-3u  "
> +        "prp1=0x%016" PRIx64 "  SQTDBL=BAR0+0x%03" HWADDR_PRIx "\n",
> +        sq->sqid, sq->size, sq->head, sq->tail, sq->cqid,
> +        sq->dma_addr, db_off);
> +}
> +
> +static void append_cq(GString *buf, NvmeCQueue *cq, unsigned stride)
> +{
> +    hwaddr db_off = 0x1000 + (2 * cq->cqid + 1) * stride;
> +
> +    g_string_append_printf(buf,
> +        "  CQ %u  size=%-5u head=%-5u tail=%-5u iv=%-5u  "
> +        "prp1=0x%016" PRIx64 "  CQHDBL=BAR0+0x%03" HWADDR_PRIx
> +        "  phaseTag=%u\n",
> +        cq->cqid, cq->size, cq->head, cq->tail, cq->vector,
> +        cq->dma_addr, db_off, cq->phase);
> +}
> +
> +static int collect_queues(Object *obj, void *opaque)
> +{
> +    GString *buf = opaque;
> +    NvmeCtrl *n;
> +    unsigned stride;
> +
> +    if (!object_dynamic_cast(obj, TYPE_NVME)) {
> +        return 0;
> +    }
> +    n = NVME(obj);
> +    stride = 4u << NVME_CAP_DSTRD(ldq_le_p(&n->bar.cap));
> +
> +    g_string_append_printf(buf, "%s\n", object_get_canonical_path(obj));
> +    append_sq(buf, &n->admin_sq, stride);
> +    append_cq(buf, &n->admin_cq, stride);
> +
> +    for (unsigned i = 1; i <= n->params.max_ioqpairs; i++) {
> +        if (n->sq && n->sq[i]) {
> +            append_sq(buf, n->sq[i], stride);
> +        }
> +        if (n->cq && n->cq[i]) {
> +            append_cq(buf, n->cq[i], stride);
> +        }
> +    }
> +    return 0;
> +}
> +
> +HumanReadableText *qmp_x_query_nvme_queues(Error **errp)
> +{
> +    g_autoptr(GString) buf = g_string_new("");
> +
> +    object_child_foreach_recursive(object_get_root(), collect_queues, buf);
> +
> +    if (buf->len == 0) {
> +        g_string_append(buf, "no NVMe controllers\n");
> +    }
> +
> +    return human_readable_text_from_str(buf);
> +}
> diff --git a/qapi/machine.json b/qapi/machine.json
> index d4a589e768..c21b128e0a 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1789,6 +1789,23 @@
>    'returns': 'HumanReadableText',
>    'features': [ 'unstable' ] }
>  
> +##
> +# @x-query-nvme-queues:
> +#
> +# Query state of all active SQ/CQ for emulated NVMe controllers.
> +#
> +# Features:
> +#
> +# @unstable: This command is meant for debugging.
> +#
> +# Returns: per-queue state as human-readable text
> +#
> +# Since: 11.1
> +##
> +{ 'command': 'x-query-nvme-queues',
> +  'returns': 'HumanReadableText',
> +  'features': [ 'unstable' ] }
> +
>  ##
>  # @SmbiosEntryPointType:
>  #
> -- 
> 2.53.0
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Re: [PATCH 1/2] hw/nvme: add 'info nvme' HMP command

[Markus Armbruster]

"Dr. David Alan Gilbert" <dave@treblig.org> writes:

> * Mateusz Nowicki (mateusz.nowicki@posteo.net) wrote:
>> Add an 'info nvme' HMP command for inspecting emulated NVMe
>> controllers from the QEMU monitor.
>> 
>> For each NVMe controller in the QOM tree the command prints:
>> 
>>   - PCI BDF, vendor/device ID and BAR0 base address;
>>   - Identify Controller fields visible to the host driver: SN, MN, FR
>>     and CNTLID;
>>   - the CC, CSTS and AQA registers (raw 32-bit values);
>>   - the number of admin and active I/O submission/completion queues.
>> 
>> Useful when debugging the Linux NVMe driver against the QEMU emulation
>> without attaching gdb to the QEMU process.
>> 
>> Example:
>> 
>>   (qemu) info nvme
>>   /machine/peripheral-anon/device[0]
>>     PCI:    BDF 00:04.0  VID=8086 DID=5845  BAR0=0x00000000feb50000
>>     ID:     SN=NVME0001  MN=QEMU NVMe Ctrl  FR=...  CNTLID=0x0000
>>     CC:     0x00460001
>>     CSTS:   0x00000001
>>     AQA:    0x001f001f
>>     Queues: 1 admin + 4 IO SQ / 4 IO CQ
>
> I won't claim to know what those NVMe magic numbers mean, but I'll
> trust you they're useful.
>
>> Signed-off-by: Mateusz Nowicki <mateusz.nowicki@posteo.net>
>> ---
>>  hmp-commands-info.hx | 13 ++++++++
>>  hw/nvme/meson.build  |  2 +-
>>  hw/nvme/monitor.c    | 76 ++++++++++++++++++++++++++++++++++++++++++++
>>  qapi/machine.json    | 17 ++++++++++
>
> So that lot looks OK from an HMP point of view, I wonder if
> qapi/machine.json is the right place for it.  Markus?

qapi/machine.json has become a bit of a dumping ground, I'm afraid.

Precedence for putting queries for the machine's device frontends of
certain kinds there: x-query-irq, x-query-interrupt-controllers, and
x-query-usb.

All the other NVMe QAPI stuff is in qapi/block-core.json, but that's the
block backend, not frontend.  Not a good home for x-query-nvme.

Let's take the patch as is.

One nitpick below.

> Acked-by: Dr. David Alan Gilbert <dave@treblig.org>
>
>>  4 files changed, 107 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/nvme/monitor.c
>> 
>> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
>> index 82134eb6c2..b984691c3c 100644
>> --- a/hmp-commands-info.hx
>> +++ b/hmp-commands-info.hx
>> @@ -340,6 +340,19 @@ SRST
>>      Show guest USB devices.
>>  ERST
>>  
>> +    {
>> +        .name       = "nvme",
>> +        .args_type  = "",
>> +        .params     = "",
>> +        .help       = "show emulated NVMe controllers",
>> +        .cmd_info_hrt = qmp_x_query_nvme,
>> +    },
>> +
>> +SRST
>> +  ``info nvme``
>> +    Show emulated NVMe controllers.
>> +ERST
>> +
>>      {
>>          .name       = "usbhost",
>>          .args_type  = "",
>> diff --git a/hw/nvme/meson.build b/hw/nvme/meson.build
>> index 7d5caa53c2..3017f058f9 100644
>> --- a/hw/nvme/meson.build
>> +++ b/hw/nvme/meson.build
>> @@ -1 +1 @@
>> -system_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('ctrl.c', 'dif.c', 'ns.c', 'subsys.c', 'nguid.c'))
>> \ No newline at end of file
>> +system_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('ctrl.c', 'dif.c', 'ns.c', 'subsys.c', 'nguid.c', 'monitor.c'))
>> \ No newline at end of file

Please use the opportunity to add the newline.

[...]

QAPI schema
Acked-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH 2/2] hw/nvme: add 'info nvme-queues' HMP command

[Markus Armbruster]

"Dr. David Alan Gilbert" <dave@treblig.org> writes:

> * Mateusz Nowicki (mateusz.nowicki@posteo.net) wrote:
>> Add an 'info nvme-queues' HMP command that lists, for every emulated
>> NVMe controller, the admin SQ/CQ and every active I/O SQ/CQ.
>> 
>> For each queue the command prints:
>> 
>>   - the ring size, head and tail indices;
>>   - the associated CQ id (for SQ) or interrupt vector (for CQ);
>>   - the queue's PRP1 (DMA base address of the ring);
>>   - the BAR0-relative doorbell offset (SQyTDBL / CQyHDBL);
>>   - the current CQ phase tag.
>> 
>> The doorbell offsets are computed from CAP.DSTRD so they remain
>> correct if the emulation ever exposes a non-zero stride.
>> 
>> Useful when debugging the Linux NVMe driver against the QEMU
>> emulation - queue setup, doorbell rings, AERs held in the admin SQ
>> etc. - without attaching gdb to the QEMU process.
>> 
>> Example, with the long PRP1 ring address and BAR0-relative doorbell
>> offset replaced by '...'; phase tag column omitted for brevity:
>> 
>>   (qemu) info nvme-queues
>>   /machine/peripheral-anon/device[0]
>>   SQ 0  size=32    head=9     tail=9     cqid=0    prp1=...  SQTDBL=...
>>   CQ 0  size=32    head=8     tail=8     iv=0      prp1=...  CQHDBL=...
>>   SQ 1  size=1024  head=2     tail=2     cqid=1    prp1=...  SQTDBL=...
>>   CQ 1  size=1024  head=2     tail=2     iv=1      prp1=...  CQHDBL=...
>> 
>> Signed-off-by: Mateusz Nowicki <mateusz.nowicki@posteo.net>
>
> as the previous patch; looks OK from HMP but check where the json should live.
>
> Acked-by: Dr. David Alan Gilbert <dave@treblig.org>

Same answer.

QAPI schema
Acked-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH 1/2] hw/nvme: add 'info nvme' HMP command

[Mateusz Nowicki]

Thanks for the reviews. v2 sent, with the meson.build trailing newline
fixed and your Acked-by tags picked up:

  https://lore.kernel.org/qemu-devel/cover.1778694320.git.mateusz.nowicki@posteo.net/

Mateusz

Re: [PATCH 1/2] hw/nvme: add 'info nvme' HMP command

[Klaus Jensen]

[-- Attachment #1: Type: text/plain, Size: 5927 bytes --]

On May 10 10:57, Mateusz Nowicki wrote:
> Add an 'info nvme' HMP command for inspecting emulated NVMe
> controllers from the QEMU monitor.
> 
> For each NVMe controller in the QOM tree the command prints:
> 
>   - PCI BDF, vendor/device ID and BAR0 base address;
>   - Identify Controller fields visible to the host driver: SN, MN, FR
>     and CNTLID;
>   - the CC, CSTS and AQA registers (raw 32-bit values);
>   - the number of admin and active I/O submission/completion queues.
> 
> Useful when debugging the Linux NVMe driver against the QEMU emulation
> without attaching gdb to the QEMU process.
> 
> Example:
> 
>   (qemu) info nvme
>   /machine/peripheral-anon/device[0]
>     PCI:    BDF 00:04.0  VID=8086 DID=5845  BAR0=0x00000000feb50000
>     ID:     SN=NVME0001  MN=QEMU NVMe Ctrl  FR=...  CNTLID=0x0000
>     CC:     0x00460001
>     CSTS:   0x00000001
>     AQA:    0x001f001f
>     Queues: 1 admin + 4 IO SQ / 4 IO CQ
> 
> Signed-off-by: Mateusz Nowicki <mateusz.nowicki@posteo.net>
> ---
>  hmp-commands-info.hx | 13 ++++++++
>  hw/nvme/meson.build  |  2 +-
>  hw/nvme/monitor.c    | 76 ++++++++++++++++++++++++++++++++++++++++++++
>  qapi/machine.json    | 17 ++++++++++
>  4 files changed, 107 insertions(+), 1 deletion(-)
>  create mode 100644 hw/nvme/monitor.c
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 82134eb6c2..b984691c3c 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -340,6 +340,19 @@ SRST
>      Show guest USB devices.
>  ERST
>  
> +    {
> +        .name       = "nvme",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show emulated NVMe controllers",
> +        .cmd_info_hrt = qmp_x_query_nvme,
> +    },
> +
> +SRST
> +  ``info nvme``
> +    Show emulated NVMe controllers.
> +ERST
> +
>      {
>          .name       = "usbhost",
>          .args_type  = "",
> diff --git a/hw/nvme/meson.build b/hw/nvme/meson.build
> index 7d5caa53c2..3017f058f9 100644
> --- a/hw/nvme/meson.build
> +++ b/hw/nvme/meson.build
> @@ -1 +1 @@
> -system_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('ctrl.c', 'dif.c', 'ns.c', 'subsys.c', 'nguid.c'))
> \ No newline at end of file
> +system_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('ctrl.c', 'dif.c', 'ns.c', 'subsys.c', 'nguid.c', 'monitor.c'))
> \ No newline at end of file
> diff --git a/hw/nvme/monitor.c b/hw/nvme/monitor.c
> new file mode 100644
> index 0000000000..95a6754437
> --- /dev/null
> +++ b/hw/nvme/monitor.c
> @@ -0,0 +1,76 @@
> +/*
> + * QEMU NVMe Controller monitor commands
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-commands-machine.h"
> +#include "qapi/type-helpers.h"
> +#include "hw/pci/pci.h"
> +
> +#include "nvme.h"
> +
> +static int collect_one(Object *obj, void *opaque)
> +{
> +    GString *buf = opaque;
> +    NvmeCtrl *n;
> +    PCIDevice *pci;
> +    pcibus_t bar0;
> +    unsigned io_sq = 0, io_cq = 0;
> +
> +    if (!object_dynamic_cast(obj, TYPE_NVME)) {
> +        return 0;
> +    }
> +    n = NVME(obj);
> +    pci = PCI_DEVICE(n);
> +    bar0 = pci_get_bar_addr(pci, 0);
> +
> +    for (unsigned i = 1; i <= n->params.max_ioqpairs; i++) {

I think this needs to run over n->conf_ioqpairs to also work for SR-IOV
VFs? Same goes for patch 2 I think.

> +        if (n->sq && n->sq[i]) {
> +            io_sq++;
> +        }
> +        if (n->cq && n->cq[i]) {
> +            io_cq++;
> +        }
> +    }
> +
> +    g_string_append_printf(buf, "%s\n", object_get_canonical_path(obj));
> +    g_string_append_printf(buf,
> +        "  PCI:    BDF %02x:%02x.%x  VID=%04x DID=%04x  ",
> +        pci_dev_bus_num(pci), PCI_SLOT(pci->devfn), PCI_FUNC(pci->devfn),
> +        pci_get_word(pci->config + PCI_VENDOR_ID),
> +        pci_get_word(pci->config + PCI_DEVICE_ID));
> +    if (bar0 == PCI_BAR_UNMAPPED) {
> +        g_string_append(buf, "BAR0=unmapped\n");
> +    } else {
> +        g_string_append_printf(buf, "BAR0=0x%016" PRIx64 "\n",
> +                               (uint64_t)bar0);
> +    }
> +    g_string_append_printf(buf,
> +        "  ID:     SN=%.20s  MN=%.40s  FR=%.8s  CNTLID=0x%04x\n",
> +        n->id_ctrl.sn, n->id_ctrl.mn, n->id_ctrl.fr, n->cntlid);
> +    g_string_append_printf(buf, "  CC:     0x%08x\n",
> +                           ldl_le_p(&n->bar.cc));
> +    g_string_append_printf(buf, "  CSTS:   0x%08x\n",
> +                           ldl_le_p(&n->bar.csts));
> +    g_string_append_printf(buf, "  AQA:    0x%08x\n",
> +                           ldl_le_p(&n->bar.aqa));
> +    g_string_append_printf(buf,
> +        "  Queues: 1 admin + %u IO SQ / %u IO CQ\n", io_sq, io_cq);
> +    return 0;
> +}
> +
> +HumanReadableText *qmp_x_query_nvme(Error **errp)
> +{
> +    g_autoptr(GString) buf = g_string_new("");
> +
> +    object_child_foreach_recursive(object_get_root(), collect_one, buf);
> +
> +    if (buf->len == 0) {
> +        g_string_append(buf, "no NVMe controllers\n");
> +    }
> +
> +    return human_readable_text_from_str(buf);
> +}
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 685e4e29b8..d4a589e768 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1772,6 +1772,23 @@
>    'returns': 'HumanReadableText',
>    'features': [ 'unstable' ] }
>  
> +##
> +# @x-query-nvme:
> +#
> +# Query state of emulated NVMe controllers.
> +#
> +# Features:
> +#
> +# @unstable: This command is meant for debugging.
> +#
> +# Returns: NVMe controller state as human-readable text
> +#
> +# Since: 11.1
> +##
> +{ 'command': 'x-query-nvme',
> +  'returns': 'HumanReadableText',
> +  'features': [ 'unstable' ] }
> +
>  ##
>  # @SmbiosEntryPointType:
>  #
> -- 
> 2.53.0
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/2] hw/nvme: add 'info nvme' HMP command

[mateusz.nowicki]

> I think this needs to run over n->conf_ioqpairs to also work for SR-IOV
> VFs? Same goes for patch 2 I think.
> 

Good catch, agreed.  Sending v3 with the fix applied to both patches.
Thanks,
Mateusz