Page MenuHomeFreeBSD

New pci device hints wiring.
ClosedPublic

Authored by imp on Dec 20 2017, 1:17 AM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 23 2023, 3:14 AM
Unknown Object (File)
Nov 29 2023, 2:42 PM
Unknown Object (File)
Nov 22 2023, 12:30 PM
Unknown Object (File)
Oct 19 2023, 11:29 PM
Unknown Object (File)
Sep 11 2023, 12:22 PM
Unknown Object (File)
Aug 7 2023, 4:29 PM
Unknown Object (File)
Aug 5 2023, 1:45 AM
Unknown Object (File)
Aug 5 2023, 1:45 AM
Subscribers
None

Details

Summary

Return domain, bus, slot, and function for the transport settings in
PATH_INQ requests for nvme.

Be pedantic and check the return value of resource_string_value. Given
that we've just found it, this is likely overkill, but if we can't
find the entry, better to fail safe.

Add device location wiring to the pci bus. This allows one to specify,
for example, that if there's an igb card in bus 12, slot 0, function
0, it should be assigned igb5. If there isn't, or there's one in a
different slot, normal numbering rules apply (hinted units are
skipped). Adding 'hint.igb.5.at="pci12:0:0"' or
'hint.igb.5.at="pci0:12:0:0"' to /boot/device.hints will accomplish
this. The quotes are important, and these are the only two forms that
are supported to avoid embedding a parser in the kernel.

Test Plan

Note: this mixes three commits together: the wiring, a tiny fix to acpi, and a patch to have the nvme sim pass data up the CAM stack about the device's location. These will be separate commits.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 13663
Build 13883: arc lint + arc unit

Event Timeline

imp added reviewers: peter, jhb, scottl.
imp added inline comments.
sys/dev/pci/pci.c
4251–4252

The simple string match is intentional. That way we don't have to parse and guess intent, nor make sure that the parser is hardened against attack.

share/man/man4/pci.4
541

Maybe use '.Xr igb 4', etc. for device drivers in this text?

sys/dev/acpica/acpi.c
1022

This shouldn't fail due to the resource_find_dev(), that's why it dosen't bother checking. Could perhaps put '(void) ' in front of it if you want.

sys/dev/pci/pci.c
4249

This can't ever fail either.

share/man/man4/pci.4
534

s/there's/there is/

536

s/there's/there is/

sys/dev/pci/pci.c
4247

If I'm reading subr_hints.c correctly, resource_find_dev() will return 0 if there is a hint.<name>.*.at, and populate unit in the process. A non-zero return means there are no more "at" hints for this driver, so the break skips over the rest of the loop. Is that correct?

4249

And then resource_string_value() populates at with the pointer to the hint.<name>.<unit>.at=... string in question?

4256

I think

		if (strcmp(at, me1) != 0 &&
		    strcmp(at, me2) != 0)
			continue;
		*unitp = unit;
		break;

would be much clearer like this:

		if (strcmp(at, me1) == 0 ||
		    strcmp(at, me2) == 0) {
			*unitp = unit;
			break;
		}

Simplify the code a bit, make changes from review (except
one). Prepare for landing.

imp marked 8 inline comments as done.Dec 20 2017, 3:48 PM

Updated with all these suggestions.

sys/dev/acpica/acpi.c
1022

OK. Also simplified the loop construct a bit instead :)

sys/dev/pci/pci.c
4247

Correct. However, I've simplified the loop so that we have a direct while loop.

4249

Correct.

4256

I disagree. We're doing a elimination loop. We may need to add more things that eliminate in the future and your change would interfere with that.

This revision was not accepted when it landed; it landed in state Needs Review.Dec 20 2017, 7:14 PM
This revision was automatically updated to reflect the committed changes.
imp marked 4 inline comments as done.