Page MenuHomeFreeBSD

Add VMD support to FreeBSD
Needs ReviewPublic

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

Details

Reviewers
jhb
scottl
Group Reviewers
manpages
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
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 26538
Build 24933: arc lint + arc unit

Event Timeline

ambrisko created this revision.Aug 23 2019, 5:48 PM
imp added a comment.Aug 23 2019, 7:42 PM

this generally looks good. a couple of nits.

share/man/man4/vmd.4
30

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

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

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

66

BUS_PROBE_GENERIC-1 might be better here...

72

style nit

135

seems like a kassert sort of situation

160

default: panic() ?

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

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.

66

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.

140

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?

173

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

578

You have a bunch of duplicates here...?

ambrisko marked 3 inline comments as done.Fri, Aug 23, 9:19 PM

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

share/man/man4/vmd.4
30

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

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

Do you have a good example for me to copy?

135

Should I add

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

infront of the return.

ambrisko marked an inline comment as done.Tue, Sep 3, 11:14 PM

Thanks for spotting the dup. from old work.

ambrisko updated this revision to Diff 61767.Fri, Sep 6, 10:04 PM

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.

ambrisko updated this revision to Diff 61772.Fri, Sep 6, 11:56 PM

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

ambrisko updated this revision to Diff 61797.Sat, Sep 7, 11:50 PM

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

bcr added a subscriber: bcr.Sun, Sep 15, 10:31 AM

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
53

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

ambrisko updated this revision to Diff 62236.Tue, Sep 17, 9:59 PM

Add new line after sentence.

ambrisko marked an inline comment as done.Tue, Sep 17, 10:02 PM
ambrisko added inline comments.
share/man/man4/vmd.4
53

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

bcr accepted this revision as: manpages.Wed, Sep 18, 7:55 AM

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