Page MenuHomeFreeBSD

vmm/amd64: port illumos#14485: bhyve nees better cpuid control
Needs ReviewPublic

Authored by rosenfeld_grumpf.hope-2000.org on Jul 26 2025, 11:17 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Oct 15, 10:35 AM
Unknown Object (File)
Thu, Oct 9, 1:47 PM
Unknown Object (File)
Thu, Oct 9, 1:47 PM
Unknown Object (File)
Thu, Oct 9, 12:44 PM
Unknown Object (File)
Tue, Oct 7, 8:07 PM
Unknown Object (File)
Thu, Oct 2, 12:56 PM
Unknown Object (File)
Fri, Sep 26, 11:07 PM
Unknown Object (File)
Fri, Sep 26, 10:39 PM

Details

Reviewers
None
Group Reviewers
bhyve
Summary

This is a port of this bhyve improvements coming from illumos:

14485 bhyve needs better cpuid control
https://www.illumos.org/issues/14485

This adds vmm kernel support for overriding the CPUID information for
a guest.

This does not include the bhyve tests added with illumos#14485 since
FreeBSD lacks the bhyve test suite. This does also not contain any
libvmmapi/bhyve userspace code to make this feature actually useful.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 65751
Build 62634: arc lint + arc unit

Event Timeline

corvink added inline comments.
sys/amd64/include/vmm.h
808
826
sys/amd64/vmm/vmm_cpuid.c
28

This shouldn't be required anymore.

572

This could be split into a separate commit to make it easier to review and merge earlier if you like to.

sys/amd64/vmm/vmm_dev_machdep.c
504–521

That's checked by vm_set_cpuid too. I would prefer checking this stuff once.

emaste added inline comments.
sys/amd64/vmm/vmm_cpuid.c
2
572

In general if this is a bug fix or otherwise an improvement that can stand alone I would indeed do it in a separate commit (that may be MFC'd to stable branches independent of or earlier than the full new functionality).

rosenfeld_grumpf.hope-2000.org added inline comments.
sys/amd64/vmm/vmm_dev_machdep.c
504–521

I think it's better to keep both checks. In particular, I think the ioctl() handler should always validate the parameters it gets from userspace. In addition to that, vm_set_cpuid() doing its own basic sanity checks is useful if it's ever called from a different context, which it very well may be if I continue to improve the CPUID support as I have recently suggested.

All that being said, another point to consider here is that this is working and reviewed code that's been shipping in illumos for a few years. There's very little gained from introducing a difference to the origin here that has no actual benefit such as fixing a real bug, but it's a pain the neck for porting changes in either direction.

rosenfeld_grumpf.hope-2000.org retitled this revision from vmm/amd64: port illumos cpuid control to vmm/amd64: port illumos#14485: bhyve nees better cpuid control.Mon, Oct 20, 7:48 AM
rosenfeld_grumpf.hope-2000.org edited the summary of this revision. (Show Details)
markj added inline comments.
sys/amd64/vmm/intel/vmx.c
2659

Continuing lines should be indented by four spaces.

sys/amd64/vmm/vmm_cpuid.c
38

These should be grouped with the existing copyright statement at the beginning of the file.

46

Spurious newline.

399

The caller asserts that last_std != NULL, but why is that true? It seems to me that userspace could construct a cpuid configuration where it's possible to return NULL, resulting in a null pointer dereference.

sys/amd64/vmm/vmm_dev_machdep.c
63

Extra newline here.

467

Rather than having the explicit bzero call, just use the M_ZERO malloc flag.