Re: [PATCH 14/17] hw/block/nvme: support identify namespace descriptor list

2020-07-02 Thread Dmitry Fomichev
Looks good,

Reviewed-by: Dmitry Fomichev 

On Mon, 2020-06-29 at 20:26 +0200, Klaus Jensen wrote:
> From: Klaus Jensen 
> 
> Since we are not providing the NGUID or EUI64 fields, we must support
> the Namespace UUID. We do not have any way of storing a persistent
> unique identifier, so conjure up a UUID that is just the namespace id.
> 
> Signed-off-by: Klaus Jensen 
> ---
>  hw/block/nvme.c   | 41 +
>  hw/block/trace-events |  1 +
>  2 files changed, 42 insertions(+)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 2279d8395aaa..8a816b558eeb 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -972,6 +972,45 @@ static uint16_t nvme_identify_nslist(NvmeCtrl *n, 
> NvmeIdentify *c)
>  return ret;
>  }
>  
> +static uint16_t nvme_identify_ns_descr_list(NvmeCtrl *n, NvmeIdentify *c)
> +{
> +uint32_t nsid = le32_to_cpu(c->nsid);
> +uint64_t prp1 = le64_to_cpu(c->prp1);
> +uint64_t prp2 = le64_to_cpu(c->prp2);
> +
> +uint8_t list[NVME_IDENTIFY_DATA_SIZE];
> +
> +struct data {
> +struct {
> +NvmeIdNsDescr hdr;
> +uint8_t v[16];
> +} uuid;
> +};
> +
> +struct data *ns_descrs = (struct data *)list;
> +
> +trace_pci_nvme_identify_ns_descr_list(nsid);
> +
> +if (unlikely(nsid == 0 || nsid > n->num_namespaces)) {
> +trace_pci_nvme_err_invalid_ns(nsid, n->num_namespaces);
> +return NVME_INVALID_NSID | NVME_DNR;
> +}
> +
> +memset(list, 0x0, sizeof(list));
> +
> +/*
> + * Because the NGUID and EUI64 fields are 0 in the Identify Namespace 
> data
> + * structure, a Namespace UUID (nidt = 0x3) must be reported in the
> + * Namespace Identification Descriptor. Add a very basic Namespace UUID
> + * here.
> + */
> +ns_descrs->uuid.hdr.nidt = NVME_NIDT_UUID;
> +ns_descrs->uuid.hdr.nidl = NVME_NIDT_UUID_LEN;
> +stl_be_p(_descrs->uuid.v, nsid);
> +
> +return nvme_dma_read_prp(n, list, NVME_IDENTIFY_DATA_SIZE, prp1, prp2);
> +}
> +
>  static uint16_t nvme_identify(NvmeCtrl *n, NvmeCmd *cmd)
>  {
>  NvmeIdentify *c = (NvmeIdentify *)cmd;
> @@ -983,6 +1022,8 @@ static uint16_t nvme_identify(NvmeCtrl *n, NvmeCmd *cmd)
>  return nvme_identify_ctrl(n, c);
>  case NVME_ID_CNS_NS_ACTIVE_LIST:
>  return nvme_identify_nslist(n, c);
> +case NVME_ID_CNS_NS_DESCR_LIST:
> +return nvme_identify_ns_descr_list(n, c);
>  default:
>  trace_pci_nvme_err_invalid_identify_cns(le32_to_cpu(c->cns));
>  return NVME_INVALID_FIELD | NVME_DNR;
> diff --git a/hw/block/trace-events b/hw/block/trace-events
> index 4a4ef34071df..7b7303cab1dd 100644
> --- a/hw/block/trace-events
> +++ b/hw/block/trace-events
> @@ -45,6 +45,7 @@ pci_nvme_del_cq(uint16_t cqid) "deleted completion queue, 
> cqid=%"PRIu16""
>  pci_nvme_identify_ctrl(void) "identify controller"
>  pci_nvme_identify_ns(uint32_t ns) "nsid %"PRIu32""
>  pci_nvme_identify_nslist(uint32_t ns) "nsid %"PRIu32""
> +pci_nvme_identify_ns_descr_list(uint32_t ns) "nsid %"PRIu32""
>  pci_nvme_get_log(uint16_t cid, uint8_t lid, uint8_t lsp, uint8_t rae, 
> uint32_t len, uint64_t off) "cid %"PRIu16" lid 0x%"PRIx8" lsp 0x%"PRIx8" rae 
> 0x%"PRIx8" len %"PRIu32" off %"PRIu64""
>  pci_nvme_getfeat(uint16_t cid, uint8_t fid, uint8_t sel, uint32_t cdw11) 
> "cid %"PRIu16" fid 0x%"PRIx8" sel 0x%"PRIx8" cdw11 0x%"PRIx32""
>  pci_nvme_setfeat(uint16_t cid, uint8_t fid, uint8_t save, uint32_t cdw11) 
> "cid %"PRIu16" fid 0x%"PRIx8" save 0x%"PRIx8" cdw11 0x%"PRIx32""


[PATCH 14/17] hw/block/nvme: support identify namespace descriptor list

2020-06-29 Thread Klaus Jensen
From: Klaus Jensen 

Since we are not providing the NGUID or EUI64 fields, we must support
the Namespace UUID. We do not have any way of storing a persistent
unique identifier, so conjure up a UUID that is just the namespace id.

Signed-off-by: Klaus Jensen 
---
 hw/block/nvme.c   | 41 +
 hw/block/trace-events |  1 +
 2 files changed, 42 insertions(+)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 2279d8395aaa..8a816b558eeb 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -972,6 +972,45 @@ static uint16_t nvme_identify_nslist(NvmeCtrl *n, 
NvmeIdentify *c)
 return ret;
 }
 
+static uint16_t nvme_identify_ns_descr_list(NvmeCtrl *n, NvmeIdentify *c)
+{
+uint32_t nsid = le32_to_cpu(c->nsid);
+uint64_t prp1 = le64_to_cpu(c->prp1);
+uint64_t prp2 = le64_to_cpu(c->prp2);
+
+uint8_t list[NVME_IDENTIFY_DATA_SIZE];
+
+struct data {
+struct {
+NvmeIdNsDescr hdr;
+uint8_t v[16];
+} uuid;
+};
+
+struct data *ns_descrs = (struct data *)list;
+
+trace_pci_nvme_identify_ns_descr_list(nsid);
+
+if (unlikely(nsid == 0 || nsid > n->num_namespaces)) {
+trace_pci_nvme_err_invalid_ns(nsid, n->num_namespaces);
+return NVME_INVALID_NSID | NVME_DNR;
+}
+
+memset(list, 0x0, sizeof(list));
+
+/*
+ * Because the NGUID and EUI64 fields are 0 in the Identify Namespace data
+ * structure, a Namespace UUID (nidt = 0x3) must be reported in the
+ * Namespace Identification Descriptor. Add a very basic Namespace UUID
+ * here.
+ */
+ns_descrs->uuid.hdr.nidt = NVME_NIDT_UUID;
+ns_descrs->uuid.hdr.nidl = NVME_NIDT_UUID_LEN;
+stl_be_p(_descrs->uuid.v, nsid);
+
+return nvme_dma_read_prp(n, list, NVME_IDENTIFY_DATA_SIZE, prp1, prp2);
+}
+
 static uint16_t nvme_identify(NvmeCtrl *n, NvmeCmd *cmd)
 {
 NvmeIdentify *c = (NvmeIdentify *)cmd;
@@ -983,6 +1022,8 @@ static uint16_t nvme_identify(NvmeCtrl *n, NvmeCmd *cmd)
 return nvme_identify_ctrl(n, c);
 case NVME_ID_CNS_NS_ACTIVE_LIST:
 return nvme_identify_nslist(n, c);
+case NVME_ID_CNS_NS_DESCR_LIST:
+return nvme_identify_ns_descr_list(n, c);
 default:
 trace_pci_nvme_err_invalid_identify_cns(le32_to_cpu(c->cns));
 return NVME_INVALID_FIELD | NVME_DNR;
diff --git a/hw/block/trace-events b/hw/block/trace-events
index 4a4ef34071df..7b7303cab1dd 100644
--- a/hw/block/trace-events
+++ b/hw/block/trace-events
@@ -45,6 +45,7 @@ pci_nvme_del_cq(uint16_t cqid) "deleted completion queue, 
cqid=%"PRIu16""
 pci_nvme_identify_ctrl(void) "identify controller"
 pci_nvme_identify_ns(uint32_t ns) "nsid %"PRIu32""
 pci_nvme_identify_nslist(uint32_t ns) "nsid %"PRIu32""
+pci_nvme_identify_ns_descr_list(uint32_t ns) "nsid %"PRIu32""
 pci_nvme_get_log(uint16_t cid, uint8_t lid, uint8_t lsp, uint8_t rae, uint32_t 
len, uint64_t off) "cid %"PRIu16" lid 0x%"PRIx8" lsp 0x%"PRIx8" rae 0x%"PRIx8" 
len %"PRIu32" off %"PRIu64""
 pci_nvme_getfeat(uint16_t cid, uint8_t fid, uint8_t sel, uint32_t cdw11) "cid 
%"PRIu16" fid 0x%"PRIx8" sel 0x%"PRIx8" cdw11 0x%"PRIx32""
 pci_nvme_setfeat(uint16_t cid, uint8_t fid, uint8_t save, uint32_t cdw11) "cid 
%"PRIu16" fid 0x%"PRIx8" save 0x%"PRIx8" cdw11 0x%"PRIx32""
-- 
2.27.0