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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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
149–158 ↗(On Diff #57533)

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

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

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

1455–1457 ↗(On Diff #57533)

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
1455–1457 ↗(On Diff #57533)

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.