Page MenuHomeFreeBSD

x86/cpu: improve hypervisor detection
ClosedPublic

Authored by royger on Jan 19 2024, 9:32 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, May 8, 8:24 PM
Unknown Object (File)
Sun, Apr 28, 11:11 AM
Unknown Object (File)
Wed, Apr 24, 3:42 AM
Unknown Object (File)
Apr 6 2024, 9:30 AM
Unknown Object (File)
Feb 22 2024, 12:38 PM
Unknown Object (File)
Jan 28 2024, 4:56 PM
Unknown Object (File)
Jan 27 2024, 12:20 PM
Subscribers

Details

Summary

Some hypervisors can expose multiple signatures, for example Xen will expose
both the Xen and the HyperV signatures if Viridian extensions are enabled for
the guest. Presence of multiple signatures is currently not handled by
FreeBSD, that will exit once a known signature is found in cpuid output.

Exposing the HyperV signature on hypervisors different than HyperV is not
uncommon, this is done so that such hypervisor can expose a (subset) of the
Viridian extensions to Windows guests for performance reasons.

Fix the specific case of HyperV by not exiting from the scan if the HperV
signature is found, and prefer a second signature if one is found.

Note that long term we might wish to convert vm_guest into a bitmap, so that it
can signal detection of multiple hypervisor interfaces.

This allows to boot a FreeBSD guest using the Xen PV interfaces when the
Viridian extensions are enabled in Xen.

Fixes: b0165dc4539f ('x86/xen: fix HVM guest hypercall page setup')
PR: 276421
Sponsored by: Cloud Software Group

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/x86/x86/identcpu.c
1423

Do you need to modify the enclosing if statement as well? I think it assumes that vm_guest will only be assigned once.

sys/x86/x86/identcpu.c
1423

bah, indeed, or else it will update the cached leaf even if no hypervisor was found. Will update the diff.

I think what we want instead is for vm_guest to be the "main" hypervisor for the first leaf, but have a way that drivers can ask "is hypervisor X present" when checking for X-specific extensions or PV devices, etc. That is, if you are running under Xen but with support for some Hyper-V PV devices, I think vm_guest should still be XEN, but we want the PV device drivers to be calling some bool hypervisor_present(enum) instead, so that instead of doing if (vm_guest == VM_GUEST_HV) the drivers call if (hypervisor_present(VM_GUEST_HV)).

In D43508#992957, @jhb wrote:

I think what we want instead is for vm_guest to be the "main" hypervisor for the first leaf, but have a way that drivers can ask "is hypervisor X present" when checking for X-specific extensions or PV devices, etc. That is, if you are running under Xen but with support for some Hyper-V PV devices, I think vm_guest should still be XEN, but we want the PV device drivers to be calling some bool hypervisor_present(enum) instead, so that instead of doing if (vm_guest == VM_GUEST_HV) the drivers call if (hypervisor_present(VM_GUEST_HV)).

Sadly when Xen exposes Viridian support, the HyperV signature is in the first leaf, and the Xen signature is in the followup one (+0x100). I assume this is done because (older?) Windows versions would only check the first leaf for HyperV signature. In such case we would still want vm_guest == XEN, but the logic can't be driven by what's on the first CPUID leaf.

Oof, ok. I guess for now this approach is fine of treating "HV" as a secondary signature (i.e. one other hypervisors will emulate). I do think we do probably want to eventually transition to more of a "hypervisor_present()" sort of thing I described which can either be a bitmap or something else and remove direct checks of vm_guest. One instance of s/HperV/HyperV/ in the commit log btw. (I really wish code review tools permitted per-line comments on commit logs, not just the diff.)

This revision is now accepted and ready to land.Jan 23 2024, 4:13 PM
In D43508#993354, @jhb wrote:

Oof, ok. I guess for now this approach is fine of treating "HV" as a secondary signature (i.e. one other hypervisors will emulate). I do think we do probably want to eventually transition to more of a "hypervisor_present()" sort of thing I described which can either be a bitmap or something else and remove direct checks of vm_guest. One instance of s/HperV/HyperV/ in the commit log btw. (I really wish code review tools permitted per-line comments on commit logs, not just the diff.)

Yeah, email is better for this TBH. Will see about updating the diff. Thanks.

This revision now requires review to proceed.Feb 6 2024, 8:37 AM
This revision is now accepted and ready to land.Feb 6 2024, 3:53 PM
This revision was automatically updated to reflect the committed changes.