hw/block/nvme: use locally assigned QEMU IEEE OUI

[PATCH v2] hw/block/nvme: use locally assigned QEMU IEEE OUI

Posted by Klaus Jensen 3 years, 2 months ago

From: Gollu Appalanaidu <anaidu.gollu@samsung.com>

Commit 6eb7a071292a ("hw/block/nvme: change controller pci id") changed
the controller to use a Red Hat assigned PCI Device and Vendor ID, but
did not change the IEEE OUI away from the Intel IEEE OUI.

Fix that and use the locally assigned QEMU IEEE OUI instead if the
`use-intel-id` parameter is not explicitly set. Also reverse the Intel
IEEE OUI bytes.

Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---

v2: drop telemetry and add a check on the use_intel_id parameter.

 hw/block/nvme.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index c2f0c88fbf39..870e9d8e1c17 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -4685,9 +4685,17 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
     id->cntlid = cpu_to_le16(n->cntlid);
 
     id->rab = 6;
-    id->ieee[0] = 0x00;
-    id->ieee[1] = 0x02;
-    id->ieee[2] = 0xb3;
+
+    if (n->params.use_intel_id) {
+        id->ieee[0] = 0xb3;
+        id->ieee[1] = 0x02;
+        id->ieee[2] = 0x00;
+    } else {
+        id->ieee[0] = 0x00;
+        id->ieee[1] = 0x54;
+        id->ieee[2] = 0x52;
+    }
+
     id->mdts = n->params.mdts;
     id->ver = cpu_to_le32(NVME_SPEC_VER);
     id->oacs = cpu_to_le16(0);
-- 
2.30.0

Re: [PATCH v2] hw/block/nvme: use locally assigned QEMU IEEE OUI

Posted by Philippe Mathieu-Daudé 3 years, 2 months ago

On 2/9/21 11:45 AM, Klaus Jensen wrote:
> From: Gollu Appalanaidu <anaidu.gollu@samsung.com>
> 
> Commit 6eb7a071292a ("hw/block/nvme: change controller pci id") changed
> the controller to use a Red Hat assigned PCI Device and Vendor ID, but
> did not change the IEEE OUI away from the Intel IEEE OUI.
> 
> Fix that and use the locally assigned QEMU IEEE OUI instead if the
> `use-intel-id` parameter is not explicitly set. Also reverse the Intel
> IEEE OUI bytes.
> 
> Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
> 
> v2: drop telemetry and add a check on the use_intel_id parameter.
> 
>  hw/block/nvme.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index c2f0c88fbf39..870e9d8e1c17 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -4685,9 +4685,17 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
>      id->cntlid = cpu_to_le16(n->cntlid);
>  
>      id->rab = 6;
> -    id->ieee[0] = 0x00;
> -    id->ieee[1] = 0x02;
> -    id->ieee[2] = 0xb3;
> +
> +    if (n->params.use_intel_id) {
> +        id->ieee[0] = 0xb3;
> +        id->ieee[1] = 0x02;
> +        id->ieee[2] = 0x00;
> +    } else {
> +        id->ieee[0] = 0x00;
> +        id->ieee[1] = 0x54;
> +        id->ieee[2] = 0x52;
> +    }

Correct.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Ideally we should have definitions and use them here and in
qemu_macaddr_default_if_unset() instead of this magic values.

> +
>      id->mdts = n->params.mdts;
>      id->ver = cpu_to_le32(NVME_SPEC_VER);
>      id->oacs = cpu_to_le16(0);
>

Re: [PATCH v2] hw/block/nvme: use locally assigned QEMU IEEE OUI

Posted by Klaus Jensen 3 years, 2 months ago

On Feb  9 12:10, Philippe Mathieu-Daudé wrote:
> On 2/9/21 11:45 AM, Klaus Jensen wrote:
> > From: Gollu Appalanaidu <anaidu.gollu@samsung.com>
> > 
> > Commit 6eb7a071292a ("hw/block/nvme: change controller pci id") changed
> > the controller to use a Red Hat assigned PCI Device and Vendor ID, but
> > did not change the IEEE OUI away from the Intel IEEE OUI.
> > 
> > Fix that and use the locally assigned QEMU IEEE OUI instead if the
> > `use-intel-id` parameter is not explicitly set. Also reverse the Intel
> > IEEE OUI bytes.
> > 
> > Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> > ---
> > 
> > v2: drop telemetry and add a check on the use_intel_id parameter.
> > 
> >  hw/block/nvme.c | 14 +++++++++++---
> >  1 file changed, 11 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > index c2f0c88fbf39..870e9d8e1c17 100644
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -4685,9 +4685,17 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
> >      id->cntlid = cpu_to_le16(n->cntlid);
> >  
> >      id->rab = 6;
> > -    id->ieee[0] = 0x00;
> > -    id->ieee[1] = 0x02;
> > -    id->ieee[2] = 0xb3;
> > +
> > +    if (n->params.use_intel_id) {
> > +        id->ieee[0] = 0xb3;
> > +        id->ieee[1] = 0x02;
> > +        id->ieee[2] = 0x00;
> > +    } else {
> > +        id->ieee[0] = 0x00;
> > +        id->ieee[1] = 0x54;
> > +        id->ieee[2] = 0x52;
> > +    }
> 
> Correct.
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Ideally we should have definitions and use them here and in
> qemu_macaddr_default_if_unset() instead of this magic values.
> 

For MAC-addresses we seem to inject some more bytes.

And thanks! Applied to nvme-next!