Re: [PATCH v2 1/2] hw/nvme: fix endianess conversion and add controller list

[Klaus Jensen <its@irrelevant.dk>]

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

On Jun  1 20:32, Gollu Appalanaidu wrote:
>Add the controller identifiers list CNS 0x13, available list of ctrls
>in NVM Subsystem that may or may not be attached to namespaces.
>
>In Identify Ctrl List of the CNS 0x12 and 0x13 no endian conversion
>for the nsid field.
>
>Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
>
>-v2:
> Fix the review comments from Klaus and squashed 2nd commit into
> 1st commit
>
>---
> hw/nvme/ctrl.c       | 26 ++++++++++++++++----------
> hw/nvme/trace-events |  2 +-
> include/block/nvme.h |  1 +
> 3 files changed, 18 insertions(+), 11 deletions(-)
>
>diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
>index 2e7498a73e..813a72c655 100644
>--- a/hw/nvme/ctrl.c
>+++ b/hw/nvme/ctrl.c
>@@ -4251,9 +4251,11 @@ 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;
>+    uint32_t nsid = le32_to_cpu(c->nsid);
>     uint16_t min_id = le16_to_cpu(c->ctrlid);
>     uint16_t list[NVME_CONTROLLER_LIST_SIZE] = {};
>     uint16_t *ids = &list[1];
>@@ -4261,15 +4263,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 (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, nsid);
>+        if (!ns) {
>+            return NVME_INVALID_FIELD | NVME_DNR;
>+        }
>     }
>
>     for (cntlid = min_id; cntlid < ARRAY_SIZE(n->subsys->ctrls); cntlid++) {

Assume that `attached` is false and `n->subsys` is NULL.

KABOOOOM :)

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