Page MenuHomeFreeBSD

Add VMD support to FreeBSD
ClosedPublic

Authored by ambrisko on Aug 23 2019, 5:48 PM.

Details

Summary

This driver attached to the Intel VMD drive and connect a new PCI domain starting
at the max, and then working donwn. Then existing FreeBSD drivers will attach.
Interrupt routing from the VMD MSI-X to the NVME disk is not well known, so any
interrupt is sent to all children that register.

VROC used Intel meta data so GRAID works with it. However, GRAID supports RAID
0,1,10 for read and write. I have some early code to support writes with RAID 5.
Note that RAID 5 can have life issues with SSD since it can cause write
amplification updating the party etc.

Test Plan

Configure VROC and VMD in the BIOS, have vmd and graid enabled.
Tested in static kernel and as a module.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 26324
Build 24803: arc lint + arc unit

Event Timeline

this generally looks good. a couple of nits.

share/man/man4/vmd.4
29

silly nit: today's date and only one comma :)

sys/dev/vmd/vmd.c
63–64

using the new PCI_PNP_INFO macros would let this be autoloaded by devmatch

65

BUS_PROBE_GENERIC-1 might be better here...

71

style nit

134

seems like a kassert sort of situation

159

default: panic() ?

rpokala added inline comments.
sys/dev/vmd/vmd.c
64

For maintainability, I suggest using a lookup-table a la struct bge_type in sys/dev/bge/if_bge.c; that would allow for trivially adding additional devids. vmd_probe() would then just walk the table looking for a match.

In addition, the table could also be passed to MODULE_PNP_INFO() for automatic loading of the kernel module.

65

Please don't use a magic number; either use one of the existing BUS_PROBE_* defines, or add a new one and use it here.

139

Since this is a wrapper around bus_space_write_X(), and that takes the value to write as the last argument, shouldn't vmd_write_config() also take the value to write as the last argument?

172

Similarly, this should also have it's last two args reversed.

577

You have a bunch of duplicates here...?

Thanks for the feed back. If's kind of a strange driver.

share/man/man4/vmd.4
29

I'll fix the comma. The date is a moving target ...

sys/dev/vmd/vmd.c
63–64

Do you have a good example for me to copy?

134

Should I add

KASSERT(1, ("Invalid width requested"));

infront of the return.

Thanks for spotting the dup. from old work.

Add VMD support to FreeBSD - updated

Address review comments and fix a bunch of other things;

  • when not using a task queue for the interrupt handler

use locks.

  • clean up vmd_bus.c to use standard child management

functions. This also fixes a panic on detach when
I was not using the softc from the parent.

Need to add bus_generic_attach, to probe children when not
loaded by devd.

Accidentally removed the part that chnaged the resources for the
bus. Adding it back in the attachment.

Minor nit for the man page. Run textproc/igor over it to tell you common errors. Invoking "mandoc -Tlint" will also check for common errors.

share/man/man4/vmd.4
52

You need to make a line break after a sentence stop.

Add new line after sentence.

ambrisko added inline comments.
share/man/man4/vmd.4
52

Ran igor and mandoc -Tlint. It flags the wrong date.

OK from manpages. You can bump the document date when you do the commit. Thanks!

This revision was not accepted when it landed; it landed in state Needs Review.Oct 10 2019, 3:12 AM
This revision was automatically updated to reflect the committed changes.
ambrisko marked an inline comment as done.