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

[Klaus Jensen]

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.

signature.asc
Description: PGP signature