Re: [PATCH v2 2/3] hw/nvme: namespace parameter for EUI-64

2021-06-14 Thread Keith Busch
On Sat, Jun 12, 2021 at 01:46:30AM +0200, Heinrich Schuchardt wrote:
> +``eui64``
> +  Set the EUI-64 of the namespace. This will be reported as a "IEEE Extended
> +  Unique Identifier" descriptor in the Namespace Identification Descriptor 
> List.

This needs to match Identify Namespace's EUI64 field, too. The
Descriptor List is really just a copy of what is reported in that
structure.



Re: [PATCH v2 2/3] hw/nvme: namespace parameter for EUI-64

2021-06-14 Thread Klaus Jensen

On Jun 14 11:48, Keith Busch wrote:

On Sat, Jun 12, 2021 at 01:46:30AM +0200, Heinrich Schuchardt wrote:

+``eui64``
+  Set the EUI-64 of the namespace. This will be reported as a "IEEE Extended
+  Unique Identifier" descriptor in the Namespace Identification Descriptor 
List.


This needs to match Identify Namespace's EUI64 field, too. The
Descriptor List is really just a copy of what is reported in that
structure.


It's missing in the documentation here, but the patch does include a 
hunk that sets id_ns->eui64.


signature.asc
Description: PGP signature


[PATCH v2 2/3] hw/nvme: namespace parameter for EUI-64

2021-06-11 Thread Heinrich Schuchardt
The EUI-64 field is the only identifier for NVMe namespaces in UEFI device
paths. Add a new namespace property "eui64", that provides the user the
option to specify the EUI-64.

Signed-off-by: Heinrich Schuchardt 
Acked-by: Klaus Jensen 
---
v2:
fix typo %s/EUI64/EUI-64/
---
 docs/system/nvme.rst |  4 +++
 hw/nvme/ctrl.c   | 58 ++--
 hw/nvme/ns.c |  2 ++
 hw/nvme/nvme.h   |  1 +
 4 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/docs/system/nvme.rst b/docs/system/nvme.rst
index f7f63d6bf6..b5f8288d7c 100644
--- a/docs/system/nvme.rst
+++ b/docs/system/nvme.rst
@@ -81,6 +81,10 @@ There are a number of parameters available:
   Set the UUID of the namespace. This will be reported as a "Namespace UUID"
   descriptor in the Namespace Identification Descriptor List.

+``eui64``
+  Set the EUI-64 of the namespace. This will be reported as a "IEEE Extended
+  Unique Identifier" descriptor in the Namespace Identification Descriptor 
List.
+
 ``bus``
   If there are more ``nvme`` devices defined, this parameter may be used to
   attach the namespace to a specific ``nvme`` device (identified by an ``id``
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 8dd9cb2ccb..f37c4fd635 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -4436,19 +4436,19 @@ static uint16_t nvme_identify_ns_descr_list(NvmeCtrl 
*n, NvmeRequest *req)
 NvmeIdentify *c = (NvmeIdentify *)>cmd;
 uint32_t nsid = le32_to_cpu(c->nsid);
 uint8_t list[NVME_IDENTIFY_DATA_SIZE] = {};
-
-struct data {
-struct {
-NvmeIdNsDescr hdr;
-uint8_t v[NVME_NIDL_UUID];
-} uuid;
-struct {
-NvmeIdNsDescr hdr;
-uint8_t v;
-} csi;
-};
-
-struct data *ns_descrs = (struct data *)list;
+uint8_t *pos = list;
+struct {
+NvmeIdNsDescr hdr;
+uint8_t v[NVME_NIDL_UUID];
+} QEMU_PACKED uuid;
+struct {
+NvmeIdNsDescr hdr;
+uint64_t v;
+} QEMU_PACKED eui64;
+struct {
+NvmeIdNsDescr hdr;
+uint8_t v;
+} QEMU_PACKED csi;

 trace_pci_nvme_identify_ns_descr_list(nsid);

@@ -4462,17 +4462,29 @@ static uint16_t nvme_identify_ns_descr_list(NvmeCtrl 
*n, NvmeRequest *req)
 }

 /*
- * Because the NGUID and EUI64 fields are 0 in the Identify Namespace data
- * structure, a Namespace UUID (nidt = 3h) must be reported in the
- * Namespace Identification Descriptor. Add the namespace UUID here.
+ * If the EUI-64 field is 0 and the NGUID field is 0, the namespace must
+ * provide a valid Namespace UUID in the Namespace Identification 
Descriptor
+ * data structure. QEMU does not yet support setting NGUID.
  */
-ns_descrs->uuid.hdr.nidt = NVME_NIDT_UUID;
-ns_descrs->uuid.hdr.nidl = NVME_NIDL_UUID;
-memcpy(_descrs->uuid.v, ns->params.uuid.data, NVME_NIDL_UUID);
-
-ns_descrs->csi.hdr.nidt = NVME_NIDT_CSI;
-ns_descrs->csi.hdr.nidl = NVME_NIDL_CSI;
-ns_descrs->csi.v = ns->csi;
+uuid.hdr.nidt = NVME_NIDT_UUID;
+uuid.hdr.nidl = NVME_NIDL_UUID;
+memcpy(uuid.v, ns->params.uuid.data, NVME_NIDL_UUID);
+memcpy(pos, , sizeof(uuid));
+pos += sizeof(uuid);
+
+if (ns->params.eui64) {
+eui64.hdr.nidt = NVME_NIDT_EUI64;
+eui64.hdr.nidl = NVME_NIDL_EUI64;
+eui64.v = cpu_to_be64(ns->params.eui64);
+memcpy(pos, , sizeof(eui64));
+pos += sizeof(eui64);
+}
+
+csi.hdr.nidt = NVME_NIDT_CSI;
+csi.hdr.nidl = NVME_NIDL_CSI;
+csi.v = ns->csi;
+memcpy(pos, , sizeof(csi));
+pos += sizeof(csi);

 return nvme_c2h(n, list, sizeof(list), req);
 }
diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index 3fec9c6273..45e457de6a 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -77,6 +77,7 @@ static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
 id_ns->mssrl = cpu_to_le16(ns->params.mssrl);
 id_ns->mcl = cpu_to_le32(ns->params.mcl);
 id_ns->msrc = ns->params.msrc;
+id_ns->eui64 = cpu_to_be64(ns->params.eui64);

 ds = 31 - clz32(ns->blkconf.logical_block_size);
 ms = ns->params.ms;
@@ -511,6 +512,7 @@ static Property nvme_ns_props[] = {
 DEFINE_PROP_BOOL("shared", NvmeNamespace, params.shared, false),
 DEFINE_PROP_UINT32("nsid", NvmeNamespace, params.nsid, 0),
 DEFINE_PROP_UUID("uuid", NvmeNamespace, params.uuid),
+DEFINE_PROP_UINT64("eui64", NvmeNamespace, params.eui64, 0),
 DEFINE_PROP_UINT16("ms", NvmeNamespace, params.ms, 0),
 DEFINE_PROP_UINT8("mset", NvmeNamespace, params.mset, 0),
 DEFINE_PROP_UINT8("pi", NvmeNamespace, params.pi, 0),
diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 93a7e0e538..ac90e13d7b 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -83,6 +83,7 @@ typedef struct NvmeNamespaceParams {
 bool shared;
 uint32_t nsid;
 QemuUUID uuid;
+uint64_t eui64;

 uint16_t ms;
 uint8_t  mset;
--
2.30.2