NVMe controller emulator for bhyve.

[grehan]

Summary

This emulator is implemented according to NVMe specification 1.0, and didn't covered all. However, I tested using newfs and nvmecontrol perf manually; These ware cleared.

[cem]

Awesome!

I can't speak to the NVMe emulation itself (I know almost nothing about the protocol and nothing about Bhyve HW emu interfaces). Maybe Warner can.

sys/dev/nvme/nvme.h

127

indentation looks off here. is it just phabricator?

usr.sbin/bhyve/Makefile

65

don't need trailing slash

usr.sbin/bhyve/pci_nvme.c

19

style nit: static FILE *dbg; (asterisk goes on the right side of the space)

21

style nit: Should be tab indents throughout (can't tell if that's just Phab making it look like 4 spaces or actually 4 spaces).

[rgrimes]

I did a very quick one time scan down the code just to see what was here, these are my comments about it.
None of the functions have block start comments stating what that function does.

usr.sbin/bhyve/pci_nvme.c

1

File is missing a copyright, and a $FreeBSD$, where did this code come from? Who wrote it?

850

style nit, extra blank line

985

Why is this code here and all commented out?
#if 0/#endif would make this more readable.

[grehan]

Leon Dang has also been working on NVMe emulation, and his version works with Linux, Windows and UEFI boot. I'll post that code for review since it is a bit more recent and tested. In the meantime, that version can be seen at www.freebsd.org/~grehan/pci_nvme.c

(There is also a version of UEFI with NVMe support compiled in at www.freebsd.org/~grehan/BHYVE_NVMe.fd.xz )

[grehan]

The updated version of this code is at https://reviews.freebsd.org/D14022

I've moved the reviewer/subscriber list over as well.

[chuck]

Overall, it is exciting to see this work being done. I realize the code is in its early stages and has asserts to help catch "the important" code paths, but it might be good to remove some of the asserts and have the commands set standard NVMe errors where appropriate.

usr.sbin/bhyve/pci_nvme.c

371

style(9): Values in return statements should be enclosed in parentheses.

404

Indentation problem here and for several lines

415

It would be good to set the subclass and programming interface values. I.e.:

PCIR_SUBCLASS to 08h and
PCIR_PROGIF to 02h

Note that both the values above have #defines in pcireg.h

566

style(9) (I think) nit; No return needed for void functions.

[grehan]

Chuck - the review for this is now in D14022, where I believe your comments have already been addressed.

[chuck]

@grehan OK. So is the thought that this review be closed and efforts should focus on D14022?

[grehan]

Yep, that's correct. The code in the new review is based on this code so it's really a continuation of the effort.

[grehan]

Closing this revision - moving to D14022