Table driven hypervisor detection and addition of VirtualBox.

[stevek]

Summary

Instead of individual conditional statements to look for each hypervisor
type, use a table to make it easier to add more in the future, if needed.

Add VirtualBox detection to the table ("VBoxVBoxVBox" is the hypervisor
vendor string to look for.) Also add VM_GUEST_VBOX to the VM_GUEST
enumeration to indicate VirtualBox.

Save the CPUID base for the hypervisor entry that we detected. Driver code
may need to know about it in order to obtain additional CPUID features.

Test Plan

Booted with qemu/KVM and VirtualBox and saw the hypervisors were correctly
detected. Would like additional testers for bhyve, VMware, Hyper-V, and Xen.

[bryanv]

sys/x86/x86/identcpu.c

1309

It looks like this block of comments got copied above so I think this block can be removed.

[stevek]

Removed duplicate comment and replaced it with a more appropriate one
that explains if CPUID2_HV is set, we are running in a hypervisor environment.

[bryanv]

sys/x86/x86/identcpu.c

1267

You copied this correctly from the if/else below but it appears the existing bhyve cpuid string is incorrect.

root@bhyvedev1:~ # uname -a
FreeBSD bhyvedev1 12.0-CURRENT FreeBSD 12.0-CURRENT #0 r335317: Mon Jun 18 16:21:17 UTC 2018 [e-mail]:/usr/obj/usr/src/amd64.amd64/sys/GENERIC amd64
root@bhyvedev1:~ # sysctl kern.vm_guest
kern.vm_guest: generic

It should be "bhyve bhyve " (note the trailing space) https://github.com/freebsd/freebsd/blob/master/sys/amd64/vmm/x86.c#L57.
Adding the space to this diff fixed bhyve detection.

bhyve detection was added in D11090 by @araujo . It is probably easiest to just fix it in this patch, assuming @stevek have plans to MFC this to 11.

[araujo]

sys/x86/x86/identcpu.c

1267

@bryanv Do you mean instead to use "bhyve bhyve" we use "bhyvebhyve"?

[araujo]

sys/x86/x86/identcpu.c

1267

I think would be great to fix that in this patch too and then MFC it to 11 if possible.

[stevek]

Fixed bhyve detection string, as pointed out by bryanv

[jhb]

There is a subtle change here you aren't documenting in the commit message which is that you are now iterating over the list of possible leafs to find a "compatible" hypervisor in case we don't recognize the "real" hypervisor. I think that's actually a fairly major change. If we don't find a hypervisor we recognize we should probably expose the "initial" string in hv_vendor? (I suggested a way to do that above).

sys/x86/x86/identcpu.c

1303

Perhaps add a comment here in the function or as part of the first block comment above the for to note that we are searching for the first hypervisor we recognize.

1331

This should be 'regs[0] >= leaf' I think.

1346

I feel like we should always expose this string, not just known strings. Perhaps we should alter the if to:

if (vm_guest != VM_GUEST_VM || leaf == 0x40000000)

This would always store the first one in hv_vendor[] on the first pass and then only overwrite it if we later find a matching hypervisor.

[stevek]

sys/x86/x86/identcpu.c

1303

Sure, makes sense, I'll update that.

1331

Yes, I think you are correct here. I'll change that.

1346

Okay, I can see that.

[stevek]

Addressed review comments - added additional comments and save the first
hypervisor we found so we have some information even if we cannot find an
exact match.

[jhb]

This change is fine as-is. It (now) occurs to me that what we may really want eventually is to change code that is currently using 'hv_base' and 'vm_guest' directly to check for certain features to instead do a search if a desired VMM is found. That is, suppose I want to check for KVM clock, maybe what I really want is something like this:

int base, high;

if (find_hypervisor_leaf(VM_GUEST_KVM, &base, &high)) {
    /* Look for KVM feature using CPUID nodes at [base, high]. */
}

This would mean that we could always kind KVM clock no matter what the "first" leaf advertised was.