Re: [PATCH 1/3] hw/nvme/ctrl: add controller list cns 0x13

[Klaus Jensen <its@irrelevant.dk>]

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

On May 17 15:37, Gollu Appalanaidu wrote:
>Add the controller identifiers list available in NVM Subsystem
>that may or may not be attached to namespaces.
>
>Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
>---
> hw/nvme/ctrl.c       | 25 +++++++++++++++----------
> hw/nvme/trace-events |  2 +-
> include/block/nvme.h |  1 +
> 3 files changed, 17 insertions(+), 11 deletions(-)
>
>diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
>index 2e7498a73e..d08a3350e2 100644
>--- a/hw/nvme/ctrl.c
>+++ b/hw/nvme/ctrl.c
>@@ -4251,7 +4251,8 @@ static uint16_t nvme_identify_ns(NvmeCtrl *n, NvmeRequest *req, bool active)
>     return NVME_INVALID_CMD_SET | NVME_DNR;
> }
>
>-static uint16_t nvme_identify_ns_attached_list(NvmeCtrl *n, NvmeRequest *req)
>+static uint16_t nvme_identify_ctrl_list(NvmeCtrl *n, NvmeRequest *req,
>+                                        bool attached)
> {
>     NvmeIdentify *c = (NvmeIdentify *)&req->cmd;
>     uint16_t min_id = le16_to_cpu(c->ctrlid);
>@@ -4261,15 +4262,17 @@ static uint16_t nvme_identify_ns_attached_list(NvmeCtrl *n, NvmeRequest *req)
>     NvmeCtrl *ctrl;
>     int cntlid, nr_ids = 0;
>
>-    trace_pci_nvme_identify_ns_attached_list(min_id);
>+    trace_pci_nvme_identify_ctrl_list(c->cns, min_id);
>
>-    if (c->nsid == NVME_NSID_BROADCAST) {
>-        return NVME_INVALID_FIELD | NVME_DNR;
>-    }
>+    if (attached) {
>+        if (c->nsid == NVME_NSID_BROADCAST) {
>+            return NVME_INVALID_FIELD | NVME_DNR;
>+        }
>
>-    ns = nvme_subsys_ns(n->subsys, c->nsid);
>-    if (!ns) {
>-        return NVME_INVALID_FIELD | NVME_DNR;
>+        ns = nvme_subsys_ns(n->subsys, c->nsid);
>+        if (!ns) {
>+            return NVME_INVALID_FIELD | NVME_DNR;
>+        }
>     }
>
>     for (cntlid = min_id; cntlid < ARRAY_SIZE(n->subsys->ctrls); cntlid++) {
>@@ -4278,7 +4281,7 @@ static uint16_t nvme_identify_ns_attached_list(NvmeCtrl *n, NvmeRequest *req)
>             continue;
>         }
>
>-        if (!nvme_ns(ctrl, c->nsid)) {
>+        if (attached && !nvme_ns(ctrl, c->nsid)) {
>             continue;
>         }
>
>@@ -4493,7 +4496,9 @@ static uint16_t nvme_identify(NvmeCtrl *n, NvmeRequest *req)
>     case NVME_ID_CNS_NS_PRESENT:
>         return nvme_identify_ns(n, req, false);
>     case NVME_ID_CNS_NS_ATTACHED_CTRL_LIST:
>-        return nvme_identify_ns_attached_list(n, req);
>+        return nvme_identify_ctrl_list(n, req, true);
>+    case NVME_ID_CNS_CTRL_LIST:
>+        return nvme_identify_ctrl_list(n, req, false);
>     case NVME_ID_CNS_CS_NS:
>         return nvme_identify_ns_csi(n, req, true);
>     case NVME_ID_CNS_CS_NS_PRESENT:
>diff --git a/hw/nvme/trace-events b/hw/nvme/trace-events
>index ea33d0ccc3..7ba3714671 100644
>--- a/hw/nvme/trace-events
>+++ b/hw/nvme/trace-events
>@@ -55,7 +55,7 @@ pci_nvme_identify(uint16_t cid, uint8_t cns, uint16_t ctrlid, uint8_t csi) "cid
> pci_nvme_identify_ctrl(void) "identify controller"
> pci_nvme_identify_ctrl_csi(uint8_t csi) "identify controller, csi=0x%"PRIx8""
> pci_nvme_identify_ns(uint32_t ns) "nsid %"PRIu32""
>-pci_nvme_identify_ns_attached_list(uint16_t cntid) "cntid=%"PRIu16""
>+pci_nvme_identify_ctrl_list(uint8_t cns, uint16_t cntid) "cns 0x%"PRIx8" cntid=%"PRIu16""

Inconsistency here. Please use "field value", not "field=value" for 
cntid.

> pci_nvme_identify_ns_csi(uint32_t ns, uint8_t csi) "nsid=%"PRIu32", csi=0x%"PRIx8""
> pci_nvme_identify_nslist(uint32_t ns) "nsid %"PRIu32""
> pci_nvme_identify_nslist_csi(uint16_t ns, uint8_t csi) "nsid=%"PRIu16", csi=0x%"PRIx8""
>diff --git a/include/block/nvme.h b/include/block/nvme.h
>index 0ff9ce17a9..188ab460df 100644
>--- a/include/block/nvme.h
>+++ b/include/block/nvme.h
>@@ -980,6 +980,7 @@ enum NvmeIdCns {
>     NVME_ID_CNS_NS_PRESENT_LIST       = 0x10,
>     NVME_ID_CNS_NS_PRESENT            = 0x11,
>     NVME_ID_CNS_NS_ATTACHED_CTRL_LIST = 0x12,
>+    NVME_ID_CNS_CTRL_LIST             = 0x13,
>     NVME_ID_CNS_CS_NS_PRESENT_LIST    = 0x1a,
>     NVME_ID_CNS_CS_NS_PRESENT         = 0x1b,
>     NVME_ID_CNS_IO_COMMAND_SET        = 0x1c,
>-- 
>2.17.1
>

-- 
One of us - No more doubt, silence or taboo about mental illness.

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