bhyve reviewers group
Thu, Aug 29
Aug 16 2019
After discussion with @markj and @jhb, I drafted up a change which involves using an sxlock to guard against racing vioapic writes while allowing the main vioapic mutex to be dropped during the sensitive portion of the TMR update process. I don't have test hardware I can dedicate to running FreeBSD bhyve, so its performance is speculative.
Aug 13 2019
Thanks for pointing that out @markj. This wasn't a problem in SmartOS bhyve since all of the mutexes there are adaptive, rather than spinlocks. (Due to interface differences, we couldn't guarantee a lack of voluntary context switching inside the critical_enter/critical_exit section, and spinlocks came with their own host of deadlock problems.) Considering the constraints in play on the FreeBSD side, I'm not sure this possible without serious restructuring. The vioapic lock would need to be sleeping, so the EOI processing would require a full exit from the VMRUN critical section. Other consumers (hpet, etc) would probably require similar treatment.
Aug 7 2019
Aug 1 2019
- integrate updated vCPU pause mechanism from @jhb that uses the VM debug components.
- separate BSP vCPU initialization from other vCPUs (do not initialize all vCPUs before starting the VM when not restoring; do not automatically grant UNRESTRICTED_GUEST capabilities to BSP)
- fix various coding errors, such as removing unused macros, using proper buffer size for snprintf and coding style
Jul 30 2019
Jul 24 2019
Jul 20 2019
Jul 17 2019
Jul 10 2019
- kick the guest threads out of vmrun when starting the pausing vCPUs.
- remove host registers from intel/vmx.c snapshot process (AMD registers have not been removed yet).
Jul 3 2019
Jul 2 2019
Jun 27 2019
Even though I wasn't on the call, I agree with Mihai that a project branch isn't a good approach. It's no better than having it in a git branch, and arguably worse as svn merge not as nice as a git rebase for a patch like this.
Remove some irrelevant comments and unused functions
Jun 26 2019
I have applied the feedback @jhb gave, except for the #ifdef guards. Will work on it after rebasing with a newer master.
- rework vCPU pausing and resuming mechanism
- rewrite VM suspend procedure - do not resume VM and devices before exit
- add missing break clause in vmm_dev switch-case
- various minor coding style fixes pointed by @jhb
- remove unnecessary critical section nesting level check
Jun 24 2019
Merged downstream in SmartOS as ec6f18e9
Jun 20 2019
Jun 18 2019
I agree with @jhb that this absolutely should be placed behind #ifdef guards for now.
So I think having this be under an optional #ifdef for now is probably best, especially since it doesn't yet work with capsicum. I can add a new WITH_BHYVE_SNAPSHOT src.conf option. I can help with adding the #ifdef or you can do that. You would do something like this in bhyve's Makefile:
Rebased to reflect upstream changes
Jun 10 2019
Merged downstream in SmartOS as aa2898c4.
Jun 5 2019
Remove debug printfs from pci_ahci.c
Remove vLAPIC fields from snapshot procedure, if they are not required after a restore / can be computed from other fields.
Save vLAPIC's LAPIC page as part of vLAPIC snapshot procedure, instead of a separate data structure.
Rename variables / functions / macros used to snapshot guest to host memory mappings to make them more intuitive.
Jun 4 2019
This is a ping. The pr https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=229852 has had another success report. This really should get in tree and in release, what is holding it up?
Jun 3 2019
Jun 1 2019
Eliminate the ILLEGAL_VECTOR qualification in vlapic_set_error per @jhb's feedback and rebase.
May 24 2019
thanks everyone with your help here :)
@jhb, reopening a commited phab review messes with things, as now when you make a second commit the default top patch in this review well be the one you commit, and one has to go digging in the review history to find the prior patch that was in the first commit. It is fine to reopen a review for a revert, but I think one should start a new review for a new change, which is what this is.
May 23 2019
José, would you be able to test the updated version that permits read as well as write and verify it still works for your test case? It would be sufficient to just update your local patch to enable reading, no need to rebase on top of what I've committed so far.
- Support read and write.
FreeBSD at least expects to be able to read from this MSR as well as write it, though FreeBSD is careful to only do this on bare metal.
I've chosen to name the constant MSR_LS_CFG since that seems more consistent with how FreeBSD has chosen constants for AMD MSRs. I've also put MSR_LS_CFG next to MSR_IC_CFG in an earlier section in xmsr.c.
Er, I see you added a defined constant. We should use that in identcpu :-). Anyway, LGTM in principle.