* [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
* 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
* [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
* 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
* [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 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 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).