[Differential] D13995: NVMe controller emulator for bhyve.

[chuck (Chuck Tuffli)]

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-list