Page MenuHomeFreeBSD

bhyve: Split vmexit handling into a separate file
ClosedPublic

Authored by markj on Jun 14 2023, 9:52 PM.
Tags
None
Referenced Files
F83928267: D40556.id128240.diff
Thu, May 16, 11:43 PM
F83928259: D40556.id123258.diff
Thu, May 16, 11:42 PM
Unknown Object (File)
Wed, May 15, 12:56 AM
Unknown Object (File)
Wed, May 15, 12:50 AM
Unknown Object (File)
Wed, May 15, 12:41 AM
Unknown Object (File)
Mar 6 2024, 3:28 AM
Unknown Object (File)
Feb 8 2024, 12:00 PM
Unknown Object (File)
Feb 7 2024, 12:30 PM

Details

Summary

Put it in amd64, since most of it is MD and won't be used on arm64. Add
a bit of glue to bhyverun.h to make CPU startup and shutdown work
without having to export more global variables. AP startup will be
reworked further in a future revision.

This makes bhyverun.c much more machine-independent.

No functional change intended.

Diff Detail

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

Event Timeline

markj requested review of this revision.Jun 14 2023, 9:52 PM

Like the commit in general.

usr.sbin/bhyve/bhyverun.c
485

It might be useful to move the logic from vmexit_suspend up into fbsdrun_deletecpu with the new signature, etc. as a commit prior to this one (so people can bisect it just in case) and then this commit is a bit simpler as it's just moving the vmexit handler. Also, looking at the new code, I almost wonder if you don't want something like:

pthread_mutex_lock(&resetcpu_mtx);
if (!CPU_ISSET(...)) {
}

CPU_CLR(vcpu, &cpumask);

if (vcpu != BSP) {
    pthread_cond_signal(...);
    pthread_mutex_unlock(...);
    pthread_exit(...);
}

while (!CPU_EMPTY(&cpumask))
    pthread_cond_wait(...);
pthread_mutex_unlock(...);

Could even keep the lock as a file-level global and use it instead of atomics in fbsdrun_addcpu if you wanted.

1028–1029

We just set these always now it seems? Wonder if it might be worth it to do that as a standalone commit first just so there's less noise in this commit?

This revision is now accepted and ready to land.Jun 14 2023, 11:47 PM

LGTM but I prefer to split this commit as jhb@ already mentioned.

usr.sbin/bhyve/bhyverun.h
54

Nit: This is vmexit related. So, IMHO it should be defined in vmexit.h.

markj added inline comments.
usr.sbin/bhyve/bhyverun.c
485
1028–1029

Yes, sorry, I neglected to mention this. https://reviews.freebsd.org/D40572

markj marked 2 inline comments as done.

Rebase, handle review feedback.

This revision now requires review to proceed.Jun 15 2023, 10:13 PM
This revision is now accepted and ready to land.Jun 16 2023, 5:37 AM