bhyve: Support other schemes for naming pass-through devices.

[jhb]

Summary

Permit naming pass through devices using the syntax accepted by
pciconf (pci[<domain>:]<bus>:<slot>:<func>) as well as by device name
(e.g. "ppt0").

While here, fix an error in the manpage that had the bus and slot
arguments for the original /-delimited scheme swapped.

Sponsored by: The FreeBSD Foundation

[imp]

Modulo a few nits noted, this looks good to my eye.

usr.sbin/bhyve/pci_passthru.c

121–122

Unrelated change, but it's good and I'm not sure it's worth a separate commit

151–152

unrelated, but correct, etc

171–172

unrelated, but correct, etc

822–827

I'd be tempted to either indent this, or put it before the if. Having it in the #if is a bit weird.

[bcr]

OK from manpages.

[jhb]

usr.sbin/bhyve/pci_passthru.c

822–827

It's only used in the else, but yeah, I could move it up.

[markj]

usr.sbin/bhyve/pci_passthru.c

670

Why add whitespace before the function number?

681

756

With this decrement, the first iteration of the following loop will check the second-last character, not the last character. Is that intentional?

757

Can this be UB? With NDEBUG defined the compiler has no guarantee that cp is a valid pointer.

[jhb]

usr.sbin/bhyve/pci_passthru.c

151–152

I've moved these out to a separate commit.

670

A stray tab. :)

756

Hmm, this is just copied from pciconf (which is also code I wrote). However, I do think it's true we do want to check that the last character is a valid digit. I think a string like "ppt0a" still ends up getting caught when strtol() below returns with *cp not equal to '\0'. Probably I did the cp-- originally to avoid checking the trailing nul. However, it's probably safer to drop the decrement, and I think it will work fine to reject values like an empty string (though the caller will never pass an empty string).

757

I don't think it can be given the constraint that name must be a valid C string? The one edge case currently that doesn't work is if name is an empty string and dropping the cp-- should handle that.

[jhb]

• Various review fixes.

[markj]

usr.sbin/bhyve/bhyve_config.5

580

Isn't it the driver that attaches to the device, not the other way around?

usr.sbin/bhyve/pci_passthru.c

757

In theory name could be NULL, assertion notwithstanding. I'm not sure I'd trust the compiler to not do something silly, but as you say, pciconf is doing the same thing already.

[jhb]

usr.sbin/bhyve/bhyve_config.5

580

Yeah, I guess it would be "must be attached to by the ppt(4) device driver" but that's kind of a mouthful. In a way, you can view the "attach" relationship as bi-directional? Once the driver claims a device, the device is then attached to that driver? Could perhaps say "device .. must be owned/managed by ppt(4)", but "attached" seems to be the most accurate jargon in FreeBSD terms.

[jhb]

• Move the macro out of the else.

[markj]

usr.sbin/bhyve/bhyve_config.5

580

I might write The ppt(4) driver must be attached to the PCI device being passed through., but that also sounds a bit awkward. It's clear enough anyway.

[jhb]

usr.sbin/bhyve/bhyve_config.5

580

Oh, I will go with that actually.