New pci device hints wiring.

[imp]

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.

[imp]

sys/dev/pci/pci.c

4251–4252 ↗

(On Diff #36785)

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.

[jhb]

share/man/man4/pci.4

541 ↗

(On Diff #36785)

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

sys/dev/acpica/acpi.c

1022 ↗

(On Diff #36785)

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 ↗

(On Diff #36785)

This can't ever fail either.

[rpokala]

share/man/man4/pci.4

534 ↗

(On Diff #36785)

s/there's/there is/

536 ↗

(On Diff #36785)

s/there's/there is/

sys/dev/pci/pci.c

4247 ↗

(On Diff #36785)

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 ↗

(On Diff #36785)

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

4256 ↗

(On Diff #36785)

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;
		}

[imp]

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

[imp]

missed one .Xr

[imp]

Updated with all these suggestions.

sys/dev/acpica/acpi.c

1022 ↗

(On Diff #36785)

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

sys/dev/pci/pci.c

4247 ↗

(On Diff #36785)

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

4249 ↗

(On Diff #36785)

Correct.

4256 ↗

(On Diff #36785)

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.