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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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
29 ↗(On Diff #61181)

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

sys/dev/vmd/vmd.c
63–64 ↗(On Diff #61181)

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

65 ↗(On Diff #61181)

BUS_PROBE_GENERIC-1 might be better here...

71 ↗(On Diff #61181)

style nit

134 ↗(On Diff #61181)

seems like a kassert sort of situation

159 ↗(On Diff #61181)

default: panic() ?

rpokala added inline comments.
sys/dev/vmd/vmd.c
64 ↗(On Diff #61181)

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 ↗(On Diff #61181)

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 ↗(On Diff #61181)

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 ↗(On Diff #61181)

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

577 ↗(On Diff #61181)

You have a bunch of duplicates here...?

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

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

share/man/man4/vmd.4
29 ↗(On Diff #61181)

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

sys/dev/vmd/vmd.c
63–64 ↗(On Diff #61181)

Do you have a good example for me to copy?

134 ↗(On Diff #61181)

Should I add

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

infront of the return.

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

Thanks for spotting the dup. from old work.

ambrisko updated this revision to Diff 61767.Sep 6 2019, 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.Sep 6 2019, 11:56 PM

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

ambrisko updated this revision to Diff 61797.Sep 7 2019, 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.Sep 15 2019, 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
52 ↗(On Diff #61797)

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

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

Add new line after sentence.

ambrisko marked an inline comment as done.Sep 17 2019, 10:02 PM
ambrisko added inline comments.
share/man/man4/vmd.4
52 ↗(On Diff #61797)

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

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

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.