diff mbox series

[v2,3/3] hw/nvme: default for namespace EUI-64

Message ID 20210611234631.81314-4-xypron.glpk@gmx.de (mailing list archive)
State Superseded, archived
Headers show
Series hw/nvme: namespace parameter for EUI-64 | expand

Commit Message

Heinrich Schuchardt June 11, 2021, 11:46 p.m. UTC 
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(+)

--
2.30.2

Comments

Klaus Jensen June 14, 2021, 6:31 p.m. UTC | #1 
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
>
>
Heinrich Schuchardt June 14, 2021, 7:23 p.m. UTC | #2 
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
>>
>>
>
Heinrich Schuchardt June 14, 2021, 7:27 p.m. UTC | #3 
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
>>>
>>>
>>
>
diff mbox series

Patch

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;