bhyve: Simplify setting vCPU capabilities.

[jhb]

Summary

• Enable VM_CAP_IPI_EXIT in fbsdrun_set_capabilities along with other capabilities enabled on all vCPUs.

• Don't call fbsdrun_set_capabilities a second time on the BSP in spinup_vcpu.

• To preserve previous behavior, don't enable unrestricted guest mode on the BSP (this unbreaks single-vCPU guests on Nehalem systems, though supporting such setups is of dubious value).

• Don't set any capabilities in spinup_ap(). These are now all redundant with earlier settings from spinup_vcpu().

• While here, axe a stale comment from fbsdrun_addcpu(). This function is now always called from the main thread for all vCPUs.

[corvink]

usr.sbin/bhyve/bhyverun.c

1165

Are there any disadvantages of settings this capability on the BSP?

1446

Why don't we remove this set_capabilities? This could be done by spinup_vcpu too.

[markj]

usr.sbin/bhyve/bhyverun.c

1165

From the commit description:

To preserve previous behavior, don't enable unrestricted guest mode on the BSP (this unbreaks single-vCPU guests on Nehalem systems, though supporting such setups is of dubious value).

Though, I'm not sure what exactly the previous behaviour is here. Maybe it'd be worth adding a comment.

[jhb]

usr.sbin/bhyve/bhyverun.c

1165

The first generation of Intel CPUs supported by bhyve (Nehalem) do not support unrestricted guest. For these systems you can only use single-CPU 64-bit VMs. If we required unrestricted guest on the BSP we would break those setups.

Now, we might not really care as much about those setups now, but dropping support for them should probably be an intentional change.

A bit more digging:

• libvmmapi:vm_setup_freebsd_registers_i386() is what enables UG for the BSP and it fails if the attempt to enable UG fails rather than just being an assert().
• If UEFI is used, we try to enable UG on the BSP but check and report an error if it doesn't work.
• When determining the maximum number of vCPUs, we check if UG is supported and if not cap the max vCPUs at 1.

That is, prior to the change to use spinup_vcpu unconditionally, we were very careful to not assume UG was required for the BSP. The change to use spinup_vcpu for the BSP is what broke this.

1446

When run on the BSP, this code has side effects in that it sets globals, etc. We also want to still do the error checking it does where it will exit gracefully before starting any vCPU threads.

(A related thought I've had but decided is not a race worth fixing in practice is that we perhaps want to actually start the BSP as suspended and explicitly unsuspend it after all of the vCPUs have been started.)

[jhb]

Ping, any more thoughts on this one?

[markj]

usr.sbin/bhyve/bhyverun.c

1082

I guess this call can legitimately fail on AMD?

[jhb]

usr.sbin/bhyve/bhyverun.c

1082

Yes, AMD doesn't handle this capability. It looks like you can also disable use of INVPCID via a tunable.