Page MenuHomeFreeBSD

bhyve: Add EUI64 to NVMe device
ClosedPublic

Authored by chuck on Apr 14 2019, 1:30 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 27, 3:37 PM
Unknown Object (File)
Sun, Mar 3, 12:22 AM
Unknown Object (File)
Feb 18 2024, 8:30 PM
Unknown Object (File)
Feb 10 2024, 5:39 PM
Unknown Object (File)
Jan 4 2024, 2:23 PM
Unknown Object (File)
Dec 31 2023, 7:34 PM
Unknown Object (File)
Dec 31 2023, 10:30 AM
Unknown Object (File)
Dec 31 2023, 10:30 AM

Details

Summary

Add the EUI64 field (part of the Identify Namespace data) to NVMe devices to support UEFI drivers.

The implementation will accept an IEEE Extended Unique Identifier (EUI-64) from the command line. If one isn't provided, it will create one based on

  • The IEEE OUI reported in the Identify Controller data
  • The PCI bus, device/slot, function values
  • The Namespace ID
Test Plan

Checked that multiple NVMe devices had unique EUI64 values inside the UEFI driver

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 25323

Event Timeline

There's only one issue that I see with this commit, and that's OUI namespace management. It's likely not a huge deal, but I think it's an easy thing to get right. emaste@ or kevans@ have the context / information to get the allocation from the foundation that I think we need. It's a minor tweak to the code to o things that way.

usr.sbin/bhyve/pci_nvme.c
431

I see we set this to the FreeBSD OUI, so that's good. However...

435

We partition the namespace so this is not. We need 8 bits for the bus, and another 8 bits for the slot. In bhyve, func will always be 0. So that suggests we need a 16-bit sub-space allocated and we can use that.

See the recent discussion about providing a stable, generated MAC address for context. I can dig that up if you need more than just this vague hint.

And Slot might also always be 0 if we can't get a full 16-bit range.

chuck edited the test plan for this revision. (Show Details)
chuck added a reviewer: kevans.

Updated patch to define OUI range specifically for NVMe
Fixed typo while I was in ieee_oui.h (reviewed by kevans)
Use NVMe OUI macro when creating the EUI64 value. Feeds the vmname + the PCI BDF into crc16 to make the low 16 bits "unique"

Love the update.
One niggle I pointed out that I'll trust you to fix or ask about if you don't know how.

usr.sbin/bhyve/pci_nvme.c
361–366

where does this come from? insert rant on copyright, attribution, license etc :)

Fixed copyright attribution

Hi Chuck!

This note is for your information only. No reply or action needed.

There are many lookup table driven CRC16 implementations out there. They
all have the same the values. For the purpose of attribution, if you do a
web search by the hex values you will get many hits w/ different authors
and licenses.

This is the earliest I know.

http://cpsr.org/prevsite/cpsr/privacy/crypto/tools/other/crc-16.c/

W. David Schwaderer, 1985

It's a small bit of code.

Regards,
Rick

usr.sbin/bhyve/pci_nvme.c
425

To help this comment be more accurate, I might remove the 'htobe64' when setting the manual EUI64 and instead structure this code as something like:

if (eui64 == 0) {
    ...
}
be64enc(nd->eui64, eui64);
433

Does NVMe require a specific formula for computing the input to the CRC? If not, I think a single call to asprintf() might be simpler:

uint64_t eui64 = OUI_FREEBSD_NVME_LOW;
char *data;

asprintf(&data, "%s%u%u%u", vmname, sc->nsc_pi->pi_bus,
    sc->nsc_pi->pi_slot, sc->nsc_pi->pi_func);
if (data != NULL) {
    eui64 |= crc16(data, strlen(data));
    free(data);
}

eui64 = (eui64 << 16) | (nsid & 0xffff);
442

Missing free(data)

445

be64enc might be a better way to do this:

be64enc(nd->eui64, eui64);

It's a bit easier to read and is also safe on non-x86 architectures with alignment constraints, etc.

Updated based on jhb's feedback

This revision is now accepted and ready to land.Jul 12 2019, 5:47 PM
This revision was automatically updated to reflect the committed changes.