Date: Thu, 25 Jan 2018 15:49:51 +0000 From: "chuck (Chuck Tuffli)" <phabric-noreply@FreeBSD.org> To: freebsd-virtualization@freebsd.org Subject: [Differential] D13995: NVMe controller emulator for bhyve. Message-ID: <9f7bce24a61a85bd15cbf3813ce69932@localhost.localdomain> In-Reply-To: <differential-rev-PHID-DREV-vgewguea2dhplsjkeozl-req@FreeBSD.org> References: <differential-rev-PHID-DREV-vgewguea2dhplsjkeozl-req@FreeBSD.org>
index | next in thread | previous in thread | raw e-mail
chuck added a comment. 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. INLINE COMMENTS > pci_nvme.c:371 > + TAILQ_INIT(&qinfo->iobhd); > + return 0; > +} style(9): Values in return statements should be enclosed in parentheses. > pci_nvme.c:404 > + * Attempt to open the backing image. Use the PCI > + * slot/func for the identifier string. > + */ Indentation problem here and for several lines > pci_nvme.c:415 > + pci_set_cfgdata16(pi, PCIR_VENDOR, 0x8086); > + pci_set_cfgdata8(pi, PCIR_CLASS, PCIC_STORAGE); > + 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 > pci_nvme.c:566 > + > + return; > +} style(9) (I think) nit; No return needed for void functions. REVISION DETAIL https://reviews.freebsd.org/D13995 EMAIL PREFERENCES https://reviews.freebsd.org/settings/panel/emailpreferences/ To: sux2mfgj_gmail.com, grehan, trasz, imp Cc: chuck, seanc, rgrimes, cem, freebsd-virtualization-listhelp
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?9f7bce24a61a85bd15cbf3813ce69932>