Page MenuHomeFreeBSD

Add VMD support to FreeBSD
ClosedPublic

Authored by ambrisko on Aug 23 2019, 5:48 PM.
Tags
None
Referenced Files
Unknown Object (File)
Apr 7 2024, 2:43 PM
Unknown Object (File)
Apr 1 2024, 1:14 PM
Unknown Object (File)
Mar 7 2024, 8:12 PM
Unknown Object (File)
Mar 5 2024, 10:19 AM
Unknown Object (File)
Mar 5 2024, 10:19 AM
Unknown Object (File)
Mar 5 2024, 10:19 AM
Unknown Object (File)
Mar 5 2024, 10:19 AM
Unknown Object (File)
Feb 11 2024, 8:30 PM
Subscribers

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 Not Applicable
Unit
Tests Not Applicable

Event Timeline

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...?

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.

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

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

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.