This shouldn't be required anymore.

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

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

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).

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.

Continuing lines should be indented by four spaces.

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

Spurious newline.

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.

Extra newline here.

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

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.

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

This change should be dropped.

Why did this change?

Still here.

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

No, it's not. Seems I've overlooked an unfortunate difference in how vcpu_init() works on FreeBSD vs. on illumos. I've fixed. it.

This is part of the change being ported (llumos #14485). When moving the code to a separate file they took the opportunity to fix this typo, which is common practice in the illumos project.

Removing this small change from this port of #14485 seems to needlessly introduces a difference to the code in illumos.

I have no idea. Probably by mistake. illumos doesn't have this particular CPUID leaf yet, thus it must be something I introduced accidentally.