Page MenuHomeFreeBSD

bhyve: Simplify setting vCPU capabilities.
ClosedPublic

Authored by jhb on Dec 9 2022, 12:56 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Oct 19, 11:32 AM
Unknown Object (File)
Sep 21 2024, 7:33 PM
Unknown Object (File)
Sep 21 2024, 3:37 PM
Unknown Object (File)
Sep 21 2024, 8:05 AM
Unknown Object (File)
Sep 18 2024, 5:26 AM
Unknown Object (File)
Sep 18 2024, 5:26 AM
Unknown Object (File)
Sep 18 2024, 5:26 AM
Unknown Object (File)
Sep 18 2024, 5:14 AM
Subscribers

Details

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.

Diff Detail

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

Event Timeline

jhb requested review of this revision.Dec 9 2022, 12:56 AM
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.

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.

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

Ping, any more thoughts on this one?

This revision is now accepted and ready to land.Dec 21 2022, 9:26 AM
markj added inline comments.
usr.sbin/bhyve/bhyverun.c
1082–1085

I guess this call can legitimately fail on AMD?

usr.sbin/bhyve/bhyverun.c
1082–1085

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

This revision was automatically updated to reflect the committed changes.