vmm/amd64: Port bhyve CPUID control from illumos

[rosenfeld_grumpf.hope-2000.org]

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.

[corvink]

sys/amd64/include/vmm.h

717

735

sys/amd64/vmm/vmm_cpuid.c

32

This shouldn't be required anymore.

573

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

552–569

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

[emaste]

sys/amd64/vmm/vmm_cpuid.c

2

573

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]

sys/amd64/vmm/vmm_dev_machdep.c

552–569

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]

Remove illumos#16223 from this change.

[markj]

sys/amd64/vmm/intel/vmx.c

2659

Continuing lines should be indented by four spaces.

sys/amd64/vmm/vmm_cpuid.c

35

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

43

Spurious newline.

396

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

64

Extra newline here.

515

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

[rosenfeld_grumpf.hope-2000.org]

Review from Mark

[rosenfeld_grumpf.hope-2000.org]

sys/amd64/vmm/vmm_cpuid.c

396

Thanks for spotting this. This does indeed happen for a technically valid but bogus cpuid configuration that defines no standard leaves and at least one extended leaf, and enables the intel fallback mode.

I think it makes sense to return cpuid_empty_entry in this particular case.

[rosenfeld_grumpf.hope-2000.org]

Adapt to the recent bhyve header refactoring.

[rosenfeld_grumpf.hope-2000.org]

Rebase.

[rosenfeld_grumpf.hope-2000.org]

Rebase.

[markj]

Mostly seems ok to me, but I'm not sure about whether we want to reset cpuid state each time the VM is reinited.

sys/amd64/vmm/vmm_cpuid.c

295

This function gets called whenever the VM is reinitialized, not just at first creation time. Is that intentional?

541

This change should be dropped.

935

Why did this change?

sys/amd64/vmm/vmm_dev_machdep.c

64

Still here.

513

573

591

No need for a null check here, you don't bother with it above.