* [PATCH v2 0/3] hw/nvme: namespace parameter for EUI-64
@ 2021-06-11 23:46 Heinrich Schuchardt
2021-06-11 23:46 ` [PATCH v2 1/3] hw: virt: consider hw_compat_6_0 Heinrich Schuchardt
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Heinrich Schuchardt @ 2021-06-11 23:46 UTC (permalink / raw)
To: Keith Busch, Klaus Jensen, Peter Maydell
Cc: Eduardo Habkost, qemu-block, Heinrich Schuchardt, Cornelia Huck,
qemu-devel, qemu-arm
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.
v2:
include patch for hw_compat_6_0
add a patch to implement default values for the EUI-64
Heinrich Schuchardt (3):
hw: virt: consider hw_compat_6_0
hw/nvme: namespace parameter for EUI-64
hw/nvme: default for namespace EUI-64
docs/system/nvme.rst | 6 +++++
hw/arm/virt.c | 2 ++
hw/core/machine.c | 1 +
hw/nvme/ctrl.c | 58 ++++++++++++++++++++++++++------------------
hw/nvme/ns.c | 11 +++++++++
hw/nvme/nvme.h | 3 +++
6 files changed, 58 insertions(+), 23 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/3] hw: virt: consider hw_compat_6_0
2021-06-11 23:46 [PATCH v2 0/3] hw/nvme: namespace parameter for EUI-64 Heinrich Schuchardt
@ 2021-06-11 23:46 ` Heinrich Schuchardt
2021-06-14 16:51 ` Heinrich Schuchardt
2021-06-11 23:46 ` [PATCH v2 2/3] hw/nvme: namespace parameter for EUI-64 Heinrich Schuchardt
2021-06-11 23:46 ` [PATCH v2 3/3] hw/nvme: default for namespace EUI-64 Heinrich Schuchardt
2 siblings, 1 reply; 10+ messages in thread
From: Heinrich Schuchardt @ 2021-06-11 23:46 UTC (permalink / raw)
To: Keith Busch, Klaus Jensen, Peter Maydell
Cc: Eduardo Habkost, qemu-block, Heinrich Schuchardt, Cornelia Huck,
qemu-devel, qemu-arm
virt-6.0 must consider hw_compat_6_0.
Fixes: da7e13c00b59 ("hw: add compat machines for 6.1")
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
v2:
add missing Fixes: tag
---
hw/arm/virt.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 840758666d..8bc3b408fe 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2764,6 +2764,8 @@ DEFINE_VIRT_MACHINE_AS_LATEST(6, 1)
static void virt_machine_6_0_options(MachineClass *mc)
{
+ virt_machine_6_1_options(mc);
+ compat_props_add(mc->compat_props, hw_compat_6_0, hw_compat_6_0_len);
}
DEFINE_VIRT_MACHINE(6, 0)
--
2.30.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] hw/nvme: namespace parameter for EUI-64
2021-06-11 23:46 [PATCH v2 0/3] hw/nvme: namespace parameter for EUI-64 Heinrich Schuchardt
2021-06-11 23:46 ` [PATCH v2 1/3] hw: virt: consider hw_compat_6_0 Heinrich Schuchardt
@ 2021-06-11 23:46 ` Heinrich Schuchardt
2021-06-14 18:48 ` Keith Busch
2021-06-11 23:46 ` [PATCH v2 3/3] hw/nvme: default for namespace EUI-64 Heinrich Schuchardt
2 siblings, 1 reply; 10+ messages in thread
From: Heinrich Schuchardt @ 2021-06-11 23:46 UTC (permalink / raw)
To: Keith Busch, Klaus Jensen, Peter Maydell
Cc: Eduardo Habkost, qemu-block, Heinrich Schuchardt, Cornelia Huck,
qemu-devel, qemu-arm, Klaus Jensen
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 <xypron.glpk@gmx.de>
Acked-by: Klaus Jensen <k.jensen@samsung.com>
---
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 *)&req->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(&ns_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, &uuid, 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, &eui64, sizeof(eui64));
+ pos += sizeof(eui64);
+ }
+
+ csi.hdr.nidt = NVME_NIDT_CSI;
+ csi.hdr.nidl = NVME_NIDL_CSI;
+ csi.v = ns->csi;
+ memcpy(pos, &csi, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] hw/nvme: default for namespace EUI-64
2021-06-11 23:46 [PATCH v2 0/3] hw/nvme: namespace parameter for EUI-64 Heinrich Schuchardt
2021-06-11 23:46 ` [PATCH v2 1/3] hw: virt: consider hw_compat_6_0 Heinrich Schuchardt
2021-06-11 23:46 ` [PATCH v2 2/3] hw/nvme: namespace parameter for EUI-64 Heinrich Schuchardt
@ 2021-06-11 23:46 ` Heinrich Schuchardt
2021-06-14 18:31 ` Klaus Jensen
2 siblings, 1 reply; 10+ messages in thread
From: Heinrich Schuchardt @ 2021-06-11 23:46 UTC (permalink / raw)
To: Keith Busch, Klaus Jensen, Peter Maydell
Cc: Eduardo Habkost, qemu-block, Heinrich Schuchardt, Cornelia Huck,
qemu-devel, qemu-arm
On machines with version > 6.0 replace a missing EUI-64 by a generated
value.
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2:
new patch
---
docs/system/nvme.rst | 2 ++
hw/core/machine.c | 1 +
hw/nvme/ns.c | 9 +++++++++
hw/nvme/nvme.h | 2 ++
4 files changed, 14 insertions(+)
diff --git a/docs/system/nvme.rst b/docs/system/nvme.rst
index b5f8288d7c..33a15c7dbc 100644
--- a/docs/system/nvme.rst
+++ b/docs/system/nvme.rst
@@ -84,6 +84,8 @@ There are a number of parameters available:
``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.
+ Since machine type 6.1 a non-zero default value is used if the parameter
+ is not provided. For earlier machine types the field defaults to 0.
``bus``
If there are more ``nvme`` devices defined, this parameter may be used to
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 55b9bc7817..d0e9348888 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -39,6 +39,7 @@
GlobalProperty hw_compat_6_0[] = {
{ "gpex-pcihost", "allow-unmapped-accesses", "false" },
{ "i8042", "extended-state", "false"},
+ { "nvme-ns", "eui64-default", "off"},
};
const size_t hw_compat_6_0_len = G_N_ELEMENTS(hw_compat_6_0);
diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index 45e457de6a..4275c3db63 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -56,6 +56,7 @@ void nvme_ns_init_format(NvmeNamespace *ns)
static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
{
+ static uint64_t ns_count;
NvmeIdNs *id_ns = &ns->id_ns;
uint8_t ds;
uint16_t ms;
@@ -73,6 +74,12 @@ static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
id_ns->nmic |= NVME_NMIC_NS_SHARED;
}
+ /* Substitute a missing EUI-64 by an autogenerated one */
+ ++ns_count;
+ if (!ns->params.eui64 && ns->params.eui64_default) {
+ ns->params.eui64 = ns_count + NVME_EUI64_DEFAULT;
+ }
+
/* simple copy */
id_ns->mssrl = cpu_to_le16(ns->params.mssrl);
id_ns->mcl = cpu_to_le32(ns->params.mcl);
@@ -533,6 +540,8 @@ static Property nvme_ns_props[] = {
params.max_open_zones, 0),
DEFINE_PROP_UINT32("zoned.descr_ext_size", NvmeNamespace,
params.zd_extension_size, 0),
+ DEFINE_PROP_BOOL("eui64-default", NvmeNamespace, params.eui64_default,
+ true),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index ac90e13d7b..3fb869731d 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -26,6 +26,7 @@
#define NVME_MAX_CONTROLLERS 32
#define NVME_MAX_NAMESPACES 256
+#define NVME_EUI64_DEFAULT 0x27fed9272381cbd0UL
typedef struct NvmeCtrl NvmeCtrl;
typedef struct NvmeNamespace NvmeNamespace;
@@ -84,6 +85,7 @@ typedef struct NvmeNamespaceParams {
uint32_t nsid;
QemuUUID uuid;
uint64_t eui64;
+ bool eui64_default;
uint16_t ms;
uint8_t mset;
--
2.30.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] hw: virt: consider hw_compat_6_0
2021-06-11 23:46 ` [PATCH v2 1/3] hw: virt: consider hw_compat_6_0 Heinrich Schuchardt
@ 2021-06-14 16:51 ` Heinrich Schuchardt
0 siblings, 0 replies; 10+ messages in thread
From: Heinrich Schuchardt @ 2021-06-14 16:51 UTC (permalink / raw)
To: Keith Busch, Klaus Jensen, Peter Maydell
Cc: Eduardo Habkost, qemu-block, Cornelia Huck, qemu-devel, qemu-arm
On 6/12/21 1:46 AM, Heinrich Schuchardt wrote:
> virt-6.0 must consider hw_compat_6_0.
>
> Fixes: da7e13c00b59 ("hw: add compat machines for 6.1")
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
This patch was applied to target-arm.next by Peter.
> ---
> v2:
> add missing Fixes: tag
> ---
> hw/arm/virt.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 840758666d..8bc3b408fe 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2764,6 +2764,8 @@ DEFINE_VIRT_MACHINE_AS_LATEST(6, 1)
>
> static void virt_machine_6_0_options(MachineClass *mc)
> {
> + virt_machine_6_1_options(mc);
> + compat_props_add(mc->compat_props, hw_compat_6_0, hw_compat_6_0_len);
> }
> DEFINE_VIRT_MACHINE(6, 0)
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] hw/nvme: default for namespace EUI-64
2021-06-11 23:46 ` [PATCH v2 3/3] hw/nvme: default for namespace EUI-64 Heinrich Schuchardt
@ 2021-06-14 18:31 ` Klaus Jensen
2021-06-14 19:23 ` Heinrich Schuchardt
0 siblings, 1 reply; 10+ messages in thread
From: Klaus Jensen @ 2021-06-14 18:31 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Peter Maydell, Eduardo Habkost, qemu-block, Cornelia Huck,
qemu-devel, qemu-arm, Keith Busch
[-- Attachment #1: Type: text/plain, Size: 3456 bytes --]
On Jun 12 01:46, Heinrich Schuchardt wrote:
>On machines with version > 6.0 replace a missing EUI-64 by a generated
>value.
>
>Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>---
>v2:
> new patch
>---
> docs/system/nvme.rst | 2 ++
> hw/core/machine.c | 1 +
> hw/nvme/ns.c | 9 +++++++++
> hw/nvme/nvme.h | 2 ++
> 4 files changed, 14 insertions(+)
>
>diff --git a/docs/system/nvme.rst b/docs/system/nvme.rst
>index b5f8288d7c..33a15c7dbc 100644
>--- a/docs/system/nvme.rst
>+++ b/docs/system/nvme.rst
>@@ -84,6 +84,8 @@ There are a number of parameters available:
> ``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.
>+ Since machine type 6.1 a non-zero default value is used if the parameter
>+ is not provided. For earlier machine types the field defaults to 0.
>
> ``bus``
> If there are more ``nvme`` devices defined, this parameter may be used to
>diff --git a/hw/core/machine.c b/hw/core/machine.c
>index 55b9bc7817..d0e9348888 100644
>--- a/hw/core/machine.c
>+++ b/hw/core/machine.c
>@@ -39,6 +39,7 @@
> GlobalProperty hw_compat_6_0[] = {
> { "gpex-pcihost", "allow-unmapped-accesses", "false" },
> { "i8042", "extended-state", "false"},
>+ { "nvme-ns", "eui64-default", "off"},
> };
> const size_t hw_compat_6_0_len = G_N_ELEMENTS(hw_compat_6_0);
>
>diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
>index 45e457de6a..4275c3db63 100644
>--- a/hw/nvme/ns.c
>+++ b/hw/nvme/ns.c
>@@ -56,6 +56,7 @@ void nvme_ns_init_format(NvmeNamespace *ns)
>
> static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
> {
>+ static uint64_t ns_count;
> NvmeIdNs *id_ns = &ns->id_ns;
> uint8_t ds;
> uint16_t ms;
>@@ -73,6 +74,12 @@ static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
> id_ns->nmic |= NVME_NMIC_NS_SHARED;
> }
>
>+ /* Substitute a missing EUI-64 by an autogenerated one */
>+ ++ns_count;
>+ if (!ns->params.eui64 && ns->params.eui64_default) {
>+ ns->params.eui64 = ns_count + NVME_EUI64_DEFAULT;
>+ }
>+
> /* simple copy */
> id_ns->mssrl = cpu_to_le16(ns->params.mssrl);
> id_ns->mcl = cpu_to_le32(ns->params.mcl);
>@@ -533,6 +540,8 @@ static Property nvme_ns_props[] = {
> params.max_open_zones, 0),
> DEFINE_PROP_UINT32("zoned.descr_ext_size", NvmeNamespace,
> params.zd_extension_size, 0),
>+ DEFINE_PROP_BOOL("eui64-default", NvmeNamespace, params.eui64_default,
>+ true),
> DEFINE_PROP_END_OF_LIST(),
> };
>
>diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
>index ac90e13d7b..3fb869731d 100644
>--- a/hw/nvme/nvme.h
>+++ b/hw/nvme/nvme.h
>@@ -26,6 +26,7 @@
>
> #define NVME_MAX_CONTROLLERS 32
> #define NVME_MAX_NAMESPACES 256
>+#define NVME_EUI64_DEFAULT 0x27fed9272381cbd0UL
Is there any significance to this value? Any particular reason it is not
using the QEMU OUI (52:54:00)?
>
> typedef struct NvmeCtrl NvmeCtrl;
> typedef struct NvmeNamespace NvmeNamespace;
>@@ -84,6 +85,7 @@ typedef struct NvmeNamespaceParams {
> uint32_t nsid;
> QemuUUID uuid;
> uint64_t eui64;
>+ bool eui64_default;
>
> uint16_t ms;
> uint8_t mset;
>--
>2.30.2
>
>
--
One of us - No more doubt, silence or taboo about mental illness.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] hw/nvme: namespace parameter for EUI-64
2021-06-11 23:46 ` [PATCH v2 2/3] hw/nvme: namespace parameter for EUI-64 Heinrich Schuchardt
@ 2021-06-14 18:48 ` Keith Busch
2021-06-14 18:55 ` Klaus Jensen
0 siblings, 1 reply; 10+ messages in thread
From: Keith Busch @ 2021-06-14 18:48 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Peter Maydell, Eduardo Habkost, qemu-block, Klaus Jensen,
Cornelia Huck, qemu-devel, qemu-arm, Klaus Jensen
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] hw/nvme: namespace parameter for EUI-64
2021-06-14 18:48 ` Keith Busch
@ 2021-06-14 18:55 ` Klaus Jensen
0 siblings, 0 replies; 10+ messages in thread
From: Klaus Jensen @ 2021-06-14 18:55 UTC (permalink / raw)
To: Keith Busch
Cc: Peter Maydell, Eduardo Habkost, qemu-block, Heinrich Schuchardt,
Cornelia Huck, qemu-devel, qemu-arm, Klaus Jensen
[-- Attachment #1: Type: text/plain, Size: 543 bytes --]
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.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] hw/nvme: default for namespace EUI-64
2021-06-14 18:31 ` Klaus Jensen
@ 2021-06-14 19:23 ` Heinrich Schuchardt
2021-06-14 19:27 ` Heinrich Schuchardt
0 siblings, 1 reply; 10+ messages in thread
From: Heinrich Schuchardt @ 2021-06-14 19:23 UTC (permalink / raw)
To: Klaus Jensen
Cc: Peter Maydell, Eduardo Habkost, qemu-block, Cornelia Huck,
qemu-devel, qemu-arm, Keith Busch
On 6/14/21 8:31 PM, Klaus Jensen wrote:
> On Jun 12 01:46, Heinrich Schuchardt wrote:
>> On machines with version > 6.0 replace a missing EUI-64 by a generated
>> value.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>> v2:
>> new patch
>> ---
>> docs/system/nvme.rst | 2 ++
>> hw/core/machine.c | 1 +
>> hw/nvme/ns.c | 9 +++++++++
>> hw/nvme/nvme.h | 2 ++
>> 4 files changed, 14 insertions(+)
>>
>> diff --git a/docs/system/nvme.rst b/docs/system/nvme.rst
>> index b5f8288d7c..33a15c7dbc 100644
>> --- a/docs/system/nvme.rst
>> +++ b/docs/system/nvme.rst
>> @@ -84,6 +84,8 @@ There are a number of parameters available:
>> ``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.
>> + Since machine type 6.1 a non-zero default value is used if the
>> parameter
>> + is not provided. For earlier machine types the field defaults to 0.
>>
>> ``bus``
>> If there are more ``nvme`` devices defined, this parameter may be
>> used to
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index 55b9bc7817..d0e9348888 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -39,6 +39,7 @@
>> GlobalProperty hw_compat_6_0[] = {
>> { "gpex-pcihost", "allow-unmapped-accesses", "false" },
>> { "i8042", "extended-state", "false"},
>> + { "nvme-ns", "eui64-default", "off"},
>> };
>> const size_t hw_compat_6_0_len = G_N_ELEMENTS(hw_compat_6_0);
>>
>> diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
>> index 45e457de6a..4275c3db63 100644
>> --- a/hw/nvme/ns.c
>> +++ b/hw/nvme/ns.c
>> @@ -56,6 +56,7 @@ void nvme_ns_init_format(NvmeNamespace *ns)
>>
>> static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
>> {
>> + static uint64_t ns_count;
>> NvmeIdNs *id_ns = &ns->id_ns;
>> uint8_t ds;
>> uint16_t ms;
>> @@ -73,6 +74,12 @@ static int nvme_ns_init(NvmeNamespace *ns, Error
>> **errp)
>> id_ns->nmic |= NVME_NMIC_NS_SHARED;
>> }
>>
>> + /* Substitute a missing EUI-64 by an autogenerated one */
>> + ++ns_count;
>> + if (!ns->params.eui64 && ns->params.eui64_default) {
>> + ns->params.eui64 = ns_count + NVME_EUI64_DEFAULT;
>> + }
>> +
>> /* simple copy */
>> id_ns->mssrl = cpu_to_le16(ns->params.mssrl);
>> id_ns->mcl = cpu_to_le32(ns->params.mcl);
>> @@ -533,6 +540,8 @@ static Property nvme_ns_props[] = {
>> params.max_open_zones, 0),
>> DEFINE_PROP_UINT32("zoned.descr_ext_size", NvmeNamespace,
>> params.zd_extension_size, 0),
>> + DEFINE_PROP_BOOL("eui64-default", NvmeNamespace,
>> params.eui64_default,
>> + true),
>> DEFINE_PROP_END_OF_LIST(),
>> };
>>
>> diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
>> index ac90e13d7b..3fb869731d 100644
>> --- a/hw/nvme/nvme.h
>> +++ b/hw/nvme/nvme.h
>> @@ -26,6 +26,7 @@
>>
>> #define NVME_MAX_CONTROLLERS 32
>> #define NVME_MAX_NAMESPACES 256
>> +#define NVME_EUI64_DEFAULT 0x27fed9272381cbd0UL
>
> Is there any significance to this value? Any particular reason it is not
> using the QEMU OUI (52:54:00)?
52:54:00 does not exist in http://standards-oui.ieee.org/oui/oui.txt
So this seems not to be an OUI. We should not use a number that IEEE may
sell to some other entity.
Until somebody buys a 36bit OUI in the name of QEMU for 780 USD we have
to use a locally assigned number.
Best regards
Heinrich
>
>>
>> typedef struct NvmeCtrl NvmeCtrl;
>> typedef struct NvmeNamespace NvmeNamespace;
>> @@ -84,6 +85,7 @@ typedef struct NvmeNamespaceParams {
>> uint32_t nsid;
>> QemuUUID uuid;
>> uint64_t eui64;
>> + bool eui64_default;
>>
>> uint16_t ms;
>> uint8_t mset;
>> --
>> 2.30.2
>>
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] hw/nvme: default for namespace EUI-64
2021-06-14 19:23 ` Heinrich Schuchardt
@ 2021-06-14 19:27 ` Heinrich Schuchardt
0 siblings, 0 replies; 10+ messages in thread
From: Heinrich Schuchardt @ 2021-06-14 19:27 UTC (permalink / raw)
To: Klaus Jensen
Cc: Peter Maydell, Eduardo Habkost, qemu-block, Cornelia Huck,
qemu-devel, qemu-arm, Keith Busch
On 6/14/21 9:23 PM, Heinrich Schuchardt wrote:
> On 6/14/21 8:31 PM, Klaus Jensen wrote:
>> On Jun 12 01:46, Heinrich Schuchardt wrote:
>>> On machines with version > 6.0 replace a missing EUI-64 by a generated
>>> value.
>>>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> ---
>>> v2:
>>> new patch
>>> ---
>>> docs/system/nvme.rst | 2 ++
>>> hw/core/machine.c | 1 +
>>> hw/nvme/ns.c | 9 +++++++++
>>> hw/nvme/nvme.h | 2 ++
>>> 4 files changed, 14 insertions(+)
>>>
>>> diff --git a/docs/system/nvme.rst b/docs/system/nvme.rst
>>> index b5f8288d7c..33a15c7dbc 100644
>>> --- a/docs/system/nvme.rst
>>> +++ b/docs/system/nvme.rst
>>> @@ -84,6 +84,8 @@ There are a number of parameters available:
>>> ``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.
>>> + Since machine type 6.1 a non-zero default value is used if the
>>> parameter
>>> + is not provided. For earlier machine types the field defaults to 0.
>>>
>>> ``bus``
>>> If there are more ``nvme`` devices defined, this parameter may be
>>> used to
>>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>>> index 55b9bc7817..d0e9348888 100644
>>> --- a/hw/core/machine.c
>>> +++ b/hw/core/machine.c
>>> @@ -39,6 +39,7 @@
>>> GlobalProperty hw_compat_6_0[] = {
>>> { "gpex-pcihost", "allow-unmapped-accesses", "false" },
>>> { "i8042", "extended-state", "false"},
>>> + { "nvme-ns", "eui64-default", "off"},
>>> };
>>> const size_t hw_compat_6_0_len = G_N_ELEMENTS(hw_compat_6_0);
>>>
>>> diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
>>> index 45e457de6a..4275c3db63 100644
>>> --- a/hw/nvme/ns.c
>>> +++ b/hw/nvme/ns.c
>>> @@ -56,6 +56,7 @@ void nvme_ns_init_format(NvmeNamespace *ns)
>>>
>>> static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
>>> {
>>> + static uint64_t ns_count;
>>> NvmeIdNs *id_ns = &ns->id_ns;
>>> uint8_t ds;
>>> uint16_t ms;
>>> @@ -73,6 +74,12 @@ static int nvme_ns_init(NvmeNamespace *ns, Error
>>> **errp)
>>> id_ns->nmic |= NVME_NMIC_NS_SHARED;
>>> }
>>>
>>> + /* Substitute a missing EUI-64 by an autogenerated one */
>>> + ++ns_count;
>>> + if (!ns->params.eui64 && ns->params.eui64_default) {
>>> + ns->params.eui64 = ns_count + NVME_EUI64_DEFAULT;
>>> + }
>>> +
>>> /* simple copy */
>>> id_ns->mssrl = cpu_to_le16(ns->params.mssrl);
>>> id_ns->mcl = cpu_to_le32(ns->params.mcl);
>>> @@ -533,6 +540,8 @@ static Property nvme_ns_props[] = {
>>> params.max_open_zones, 0),
>>> DEFINE_PROP_UINT32("zoned.descr_ext_size", NvmeNamespace,
>>> params.zd_extension_size, 0),
>>> + DEFINE_PROP_BOOL("eui64-default", NvmeNamespace,
>>> params.eui64_default,
>>> + true),
>>> DEFINE_PROP_END_OF_LIST(),
>>> };
>>>
>>> diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
>>> index ac90e13d7b..3fb869731d 100644
>>> --- a/hw/nvme/nvme.h
>>> +++ b/hw/nvme/nvme.h
>>> @@ -26,6 +26,7 @@
>>>
>>> #define NVME_MAX_CONTROLLERS 32
>>> #define NVME_MAX_NAMESPACES 256
>>> +#define NVME_EUI64_DEFAULT 0x27fed9272381cbd0UL
>>
>> Is there any significance to this value? Any particular reason it is not
>> using the QEMU OUI (52:54:00)?
>
> 52:54:00 does not exist in http://standards-oui.ieee.org/oui/oui.txt
> So this seems not to be an OUI. We should not use a number that IEEE may
> sell to some other entity.
>
> Until somebody buys a 36bit OUI in the name of QEMU for 780 USD we have
> to use a locally assigned number.
I got that wrong IEEE_COMPANY_LOCALLY_ASSIGNED 0x525400 is locally
assigned and therefore not in the OUI list.
I will resubmit the patch.
Best regards
Heinrich
>
> Best regards
>
> Heinrich
>
>>
>>>
>>> typedef struct NvmeCtrl NvmeCtrl;
>>> typedef struct NvmeNamespace NvmeNamespace;
>>> @@ -84,6 +85,7 @@ typedef struct NvmeNamespaceParams {
>>> uint32_t nsid;
>>> QemuUUID uuid;
>>> uint64_t eui64;
>>> + bool eui64_default;
>>>
>>> uint16_t ms;
>>> uint8_t mset;
>>> --
>>> 2.30.2
>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-06-14 19:28 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-11 23:46 [PATCH v2 0/3] hw/nvme: namespace parameter for EUI-64 Heinrich Schuchardt
2021-06-11 23:46 ` [PATCH v2 1/3] hw: virt: consider hw_compat_6_0 Heinrich Schuchardt
2021-06-14 16:51 ` Heinrich Schuchardt
2021-06-11 23:46 ` [PATCH v2 2/3] hw/nvme: namespace parameter for EUI-64 Heinrich Schuchardt
2021-06-14 18:48 ` Keith Busch
2021-06-14 18:55 ` Klaus Jensen
2021-06-11 23:46 ` [PATCH v2 3/3] hw/nvme: default for namespace EUI-64 Heinrich Schuchardt
2021-06-14 18:31 ` Klaus Jensen
2021-06-14 19:23 ` Heinrich Schuchardt
2021-06-14 19:27 ` Heinrich Schuchardt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.