On Thu, Jun 02, 2022 at 03:05:16PM -0400, Dave Voutila wrote:
>
> Dave Voutila <[email protected]> writes:
>
> > tech@ et al.:
> >
> > Looking for testers of the following diff for vmm(4). In my efforts to
> > fix some stability issues, I'm taking baby steps tweaking parts of the
> > code to make my upcoming proposal (adding refcnts) easier to swallow.
> >
> > This change removes the calling of vm_teardown from the code path in
> > vm_run after vmm has exited the vm/vcpu and is on its way back to
> > userland/vmd(8).
> >
> > vm_teardown is currently called in 3 areas to destroy/free a vm:
> >
> > - vm_create: as cleanup in an error path
> > - vm_terminate: on a vm the ioctl is killing
> > - vm_run: the run ioctl handler
> >
> > This diff removes that last bullet. It's not needed as vmd will cleanup
> > the vm on child exit, calling vm_terminate. Any non-vmd user of vmm(4)
> > can stop being lazy and use the VMM_IOC_TERM ioctl.
> >
> > Not included in the snippet is the existing final else block that still
> > toggles the vcpu state:
> >
> > } else {
> > vrp->vrp_exit_reason = VM_EXIT_TERMINATED;
> > vcpu->vc_state = VCPU_STATE_TERMINATED;
> > }
> >
> > If testing, please describe *any* difference in shutdown/reboot of vm
> > guests. (n.b. there's a known issue for Linux guests running very recent
> > Linux kernels not being able to reboot. That needs to be addressed in
> > vmd.)
> >
>
> Bumping as the diff has been out for testing and looking for ok's.
>
> -dv
>
ok mlarkin if this helps your subsequent cleanup
> >
> >
> > Index: sys/arch/amd64/amd64/vmm.c
> > ===================================================================
> > RCS file: /opt/cvs/src/sys/arch/amd64/amd64/vmm.c,v
> > retrieving revision 1.311
> > diff -u -p -r1.311 vmm.c
> > --- sys/arch/amd64/amd64/vmm.c 20 May 2022 22:42:09 -0000 1.311
> > +++ sys/arch/amd64/amd64/vmm.c 23 May 2022 11:57:49 -0000
> > @@ -4495,22 +4495,8 @@ vm_run(struct vm_run_params *vrp)
> > ret = vcpu_run_svm(vcpu, vrp);
> > }
> >
> > - /*
> > - * We can set the VCPU states here without CAS because once
> > - * a VCPU is in state RUNNING or REQTERM, only the VCPU itself
> > - * can switch the state.
> > - */
> > atomic_dec_int(&vm->vm_vcpus_running);
> > - if (vcpu->vc_state == VCPU_STATE_REQTERM) {
> > - vrp->vrp_exit_reason = VM_EXIT_TERMINATED;
> > - vcpu->vc_state = VCPU_STATE_TERMINATED;
> > - if (vm->vm_vcpus_running == 0) {
> > - rw_enter_write(&vmm_softc->vm_lock);
> > - vm_teardown(vm);
> > - rw_exit_write(&vmm_softc->vm_lock);
> > - }
> > - ret = 0;
> > - } else if (ret == 0 || ret == EAGAIN) {
> > + if (ret == 0 || ret == EAGAIN) {
> > /* If we are exiting, populate exit data so vmd can help. */
> > vrp->vrp_exit_reason = (ret == 0) ? VM_EXIT_NONE
> > : vcpu->vc_gueststate.vg_exit_reason;
>
>
> --
> -Dave Voutila
