Page MenuHomeFreeBSD

Set a specific value in vm_guest for older detection methods
ClosedPublic

Authored by stevek on May 18 2019, 6:14 PM.

Details

Summary

The older detection methods (smbios.bios.vendor and smbios.system.product)
are able to determine some virtual machines, but the vm_guest variable was
still only being set to VM_GUEST_VM.

Since we do know what some of them specifically are, we can set vm_guest
appropriately.

Also, if we see the CPUID has the HV flag, but we were unable to find a
definitive vendor in the Hypervisor CPUID Information Leaf, fall back to
the older detection methods, as they may be able to determine a specific
HV type.

Add VM_GUEST_PARALLELS value to VM_GUEST for Parallels.

Test Plan

Tested on Parallels and VirtualBox.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 24321
Build 23144: arc lint + arc unit

Event Timeline

stevek created this revision.May 18 2019, 6:14 PM
cem added a comment.May 18 2019, 8:44 PM

Thanks for doing this! It mostly looks great to me, just some minor const and bool style quibbles below.

sys/kern/subr_param.c
148–158

This isn't new in this commit (so feel free to leave as-is), but for this kind of list I like to use C99 array initializers to avoid mismatches as the list gets longer or is edited.

i.e., [VM_GUEST_PARALLELS] = "parallels", and similar.

Feel free to leave out of this change, it's unrelated.

sys/x86/x86/identcpu.c
1309

Rather than tagging the vm_bname pointer as const, I'd just tag the entire array as const. (Of course, the string value it points to should remain const.)

That'd look something like:

static const struct {
    const char *     vm_bname;
    ...
} vm_bnames[] = {
...

Same idea for vm_pnames

1422

style(9) nit: compare non-bool values against a zero value for conditional expressions

1463–1466

Maybe we want to return early if vm_guest isn't the generic VM_GUEST_VM?

Maybe not a real problem, but I'm imagining detecting a more specific VM type via vm_bname and then overriding it with a less-specific generic VM type via vm_pname. Feel free to leave as-is.

stevek added inline comments.May 19 2019, 3:20 PM
sys/x86/x86/identcpu.c
1463–1466

Yes, probably each block should check if the vm_guest we find. If it is not VM_GUEST_VM, return early, as we found a specific hypervisor. Otherwise, let it continue, as there could be another check that could find a more-specific one.

I'm not sure if there will be any case like that, but it would seem reasonable to do that.

stevek updated this revision to Diff 57564.May 19 2019, 4:10 PM

Take care of review comments.
Also change the conditional in print_hypervisor_info() to explicitly check
for NUL character instead of treating a character as a boolean.

cem accepted this revision.May 19 2019, 6:32 PM

Looks great, thanks for doing this!

This revision is now accepted and ready to land.May 19 2019, 6:32 PM
This revision was automatically updated to reflect the committed changes.