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
_______________________________________________
freebsd-virtualization@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
To unsubscribe, send any mail to 
"freebsd-virtualization-unsubscr...@freebsd.org"

Reply via email to