14024 bhyve vm_suspend should be more flexible

Reviewed by: Dan Cross <cross@oxidecomputer.com>
Reviewed by: Luqman Aden <luqman@oxide.computer>
Reviewed by: Joshua M. Clulow <josh@sysmgr.org>
Approved by: Dan McDonald <danmcd@joyent.com>

7 files changed, 103 insertions, 27 deletions

diff --git a/usr/src/cmd/bhyve/bhyverun.c b/usr/src/cmd/bhyve/bhyverun.c
index b127c7cadc..267d4329ed 100644
--- a/usr/src/cmd/bhyve/bhyverun.c
+++ b/usr/src/cmd/bhyve/bhyverun.c
@@ -1307,7 +1307,11 @@ do_open(const char *vmname)
 #endif
  
 	if (reinit) {
+#ifndef __FreeBSD__
+		error = vm_reinit(ctx, 0);
+#else
 		error = vm_reinit(ctx);
+#endif
 		if (error) {
 			perror("vm_reinit");
 			exit(4);
diff --git a/usr/src/lib/libvmmapi/common/vmmapi.c b/usr/src/lib/libvmmapi/common/vmmapi.c
index ec27949a43..c074d999c6 100644
--- a/usr/src/lib/libvmmapi/common/vmmapi.c
+++ b/usr/src/lib/libvmmapi/common/vmmapi.c
@@ -822,12 +822,24 @@ vm_suspend(struct vmctx *ctx, enum vm_suspend_how how)
 	return (ioctl(ctx->fd, VM_SUSPEND, &vmsuspend));
 }
 
+#ifndef __FreeBSD__
+int
+vm_reinit(struct vmctx *ctx, uint64_t flags)
+{
+	struct vm_reinit reinit = {
+		.flags = flags
+	};
+
+	return (ioctl(ctx->fd, VM_REINIT, &reinit));
+}
+#else
 int
 vm_reinit(struct vmctx *ctx)
 {
 
 	return (ioctl(ctx->fd, VM_REINIT, 0));
 }
+#endif
 
 int
 vm_inject_exception(struct vmctx *ctx, int vcpu, int vector, int errcode_valid,
diff --git a/usr/src/lib/libvmmapi/common/vmmapi.h b/usr/src/lib/libvmmapi/common/vmmapi.h
index e239b70a56..6949ea4e5a 100644
--- a/usr/src/lib/libvmmapi/common/vmmapi.h
+++ b/usr/src/lib/libvmmapi/common/vmmapi.h
@@ -175,7 +175,11 @@ int	vm_get_register_set(struct vmctx *ctx, int vcpu, unsigned int count,
 int	vm_run(struct vmctx *ctx, int vcpu, const struct vm_entry *vm_entry,
     struct vm_exit *vm_exit);
 int	vm_suspend(struct vmctx *ctx, enum vm_suspend_how how);
+#ifndef __FreeBSD__
+int	vm_reinit(struct vmctx *ctx, uint64_t);
+#else
 int	vm_reinit(struct vmctx *ctx);
+#endif
 int	vm_apicid2vcpu(struct vmctx *ctx, int apicid);
 int	vm_inject_exception(struct vmctx *ctx, int vcpu, int vector,
     int errcode_valid, uint32_t errcode, int restart_instruction);
diff --git a/usr/src/uts/i86pc/io/vmm/sys/vmm_kernel.h b/usr/src/uts/i86pc/io/vmm/sys/vmm_kernel.h
index 4191aaee5c..3a50dafd6d 100644
--- a/usr/src/uts/i86pc/io/vmm/sys/vmm_kernel.h
+++ b/usr/src/uts/i86pc/io/vmm/sys/vmm_kernel.h
@@ -117,7 +117,7 @@ extern struct vmm_ops vmm_ops_amd;
 
 int vm_create(const char *name, uint64_t flags, struct vm **retvm);
 void vm_destroy(struct vm *vm);
-int vm_reinit(struct vm *vm);
+int vm_reinit(struct vm *vm, uint64_t);
 const char *vm_name(struct vm *vm);
 uint16_t vm_get_maxcpus(struct vm *vm);
 void vm_get_topology(struct vm *vm, uint16_t *sockets, uint16_t *cores,
diff --git a/usr/src/uts/i86pc/io/vmm/vmm.c b/usr/src/uts/i86pc/io/vmm/vmm.c
index 80c9ec6bd7..f95e415e40 100644
--- a/usr/src/uts/i86pc/io/vmm/vmm.c
+++ b/usr/src/uts/i86pc/io/vmm/vmm.c
@@ -634,22 +634,42 @@ vm_destroy(struct vm *vm)
 }
 
 int
-vm_reinit(struct vm *vm)
+vm_reinit(struct vm *vm, uint64_t flags)
 {
-	int error;
+	/* A virtual machine can be reset only if all vcpus are suspended. */
+	if (CPU_CMP(&vm->suspended_cpus, &vm->active_cpus) != 0) {
+		if ((flags & VM_REINIT_F_FORCE_SUSPEND) == 0) {
+			return (EBUSY);
+		}
 
-	/*
-	 * A virtual machine can be reset only if all vcpus are suspended.
-	 */
-	if (CPU_CMP(&vm->suspended_cpus, &vm->active_cpus) == 0) {
-		vm_cleanup(vm, false);
-		vm_init(vm, false);
-		error = 0;
-	} else {
-		error = EBUSY;
+		/*
+		 * Force the VM (and all its vCPUs) into a suspended state.
+		 * This should be quick and easy, since the vm_reinit() call is
+		 * made while holding the VM write lock, which requires holding
+		 * all of the vCPUs in the VCPU_FROZEN state.
+		 */
+		(void) atomic_cmpset_int((uint_t *)&vm->suspend, 0,
+		    VM_SUSPEND_RESET);
+		for (uint_t i = 0; i < vm->maxcpus; i++) {
+			struct vcpu *vcpu = &vm->vcpu[i];
+
+			if (CPU_ISSET(i, &vm->suspended_cpus) ||
+			    !CPU_ISSET(i, &vm->active_cpus)) {
+				continue;
+			}
+
+			vcpu_lock(vcpu);
+			VERIFY3U(vcpu->state, ==, VCPU_FROZEN);
+			CPU_SET_ATOMIC(i, &vm->suspended_cpus);
+			vcpu_unlock(vcpu);
+		}
+
+		VERIFY0(CPU_CMP(&vm->suspended_cpus, &vm->active_cpus));
 	}
 
-	return (error);
+	vm_cleanup(vm, false);
+	vm_init(vm, false);
+	return (0);
 }
 
 const char *
@@ -1953,27 +1973,37 @@ vm_handle_wrmsr(struct vm *vm, int vcpuid, struct vm_exit *vme)
 int
 vm_suspend(struct vm *vm, enum vm_suspend_how how)
 {
-	int i;
-
 	if (how <= VM_SUSPEND_NONE || how >= VM_SUSPEND_LAST)
 		return (EINVAL);
 
 	if (atomic_cmpset_int((uint_t *)&vm->suspend, 0, how) == 0) {
-		VM_CTR2(vm, "virtual machine already suspended %d/%d",
-		    vm->suspend, how);
 		return (EALREADY);
 	}
 
-	VM_CTR1(vm, "virtual machine successfully suspended %d", how);
-
 	/*
 	 * Notify all active vcpus that they are now suspended.
 	 */
-	for (i = 0; i < vm->maxcpus; i++) {
-		if (CPU_ISSET(i, &vm->active_cpus))
-			vcpu_notify_event(vm, i);
-	}
+	for (uint_t i = 0; i < vm->maxcpus; i++) {
+		struct vcpu *vcpu = &vm->vcpu[i];
 
+		vcpu_lock(vcpu);
+		if (vcpu->state == VCPU_IDLE || vcpu->state == VCPU_FROZEN) {
+			/*
+			 * Any vCPUs not actively running or in HLT can be
+			 * marked as suspended immediately.
+			 */
+			if (CPU_ISSET(i, &vm->active_cpus)) {
+				CPU_SET_ATOMIC(i, &vm->suspended_cpus);
+			}
+		} else {
+			/*
+			 * Those which are running or in HLT will pick up the
+			 * suspended state after notification.
+			 */
+			vcpu_notify_event_locked(vcpu, VCPU_NOTIFY_EXIT);
+		}
+		vcpu_unlock(vcpu);
+	}
 	return (0);
 }
 
@@ -2199,8 +2229,6 @@ vm_run(struct vm *vm, int vcpuid, const struct vm_entry *entry)
 		return (EINVAL);
 	if (!CPU_ISSET(vcpuid, &vm->active_cpus))
 		return (EINVAL);
-	if (CPU_ISSET(vcpuid, &vm->suspended_cpus))
-		return (EINVAL);
 
 	pmap = vmspace_pmap(vm->vmspace);
 	vcpu = &vm->vcpu[vcpuid];
@@ -3103,8 +3131,22 @@ vm_activate_cpu(struct vm *vm, int vcpuid)
 	if (CPU_ISSET(vcpuid, &vm->active_cpus))
 		return (EBUSY);
 
+	if (vm->suspend != 0) {
+		return (EBUSY);
+	}
+
 	VCPU_CTR0(vm, vcpuid, "activated");
 	CPU_SET_ATOMIC(vcpuid, &vm->active_cpus);
+
+	/*
+	 * It is possible that this vCPU was undergoing activation at the same
+	 * time that the VM was being suspended.  If that happens to be the
+	 * case, it should reflect the suspended state immediately.
+	 */
+	if (atomic_load_acq_int((uint_t *)&vm->suspend) != 0) {
+		CPU_SET_ATOMIC(vcpuid, &vm->suspended_cpus);
+	}
+
 	return (0);
 }
 
diff --git a/usr/src/uts/i86pc/io/vmm/vmm_sol_dev.c b/usr/src/uts/i86pc/io/vmm/vmm_sol_dev.c
index 8e8f68789a..92d1494e04 100644
--- a/usr/src/uts/i86pc/io/vmm/vmm_sol_dev.c
+++ b/usr/src/uts/i86pc/io/vmm/vmm_sol_dev.c
@@ -466,6 +466,7 @@ vmmdev_do_ioctl(vmm_softc_t *sc, int cmd, intptr_t arg, int md,
 		break;
 
 	case VM_IOAPIC_PINCOUNT:
+	case VM_SUSPEND:
 	default:
 		break;
 	}
@@ -515,7 +516,13 @@ vmmdev_do_ioctl(vmm_softc_t *sc, int cmd, intptr_t arg, int md,
 		error = vm_suspend(sc->vmm_vm, vmsuspend.how);
 		break;
 	}
-	case VM_REINIT:
+	case VM_REINIT: {
+		struct vm_reinit reinit;
+
+		if (ddi_copyin(datap, &reinit, sizeof (reinit), md)) {
+			error = EFAULT;
+			break;
+		}
 		if ((error = vmm_drv_block_hook(sc, B_TRUE)) != 0) {
 			/*
 			 * The VM instance should be free of driver-attached
@@ -523,9 +530,10 @@ vmmdev_do_ioctl(vmm_softc_t *sc, int cmd, intptr_t arg, int md,
 			 */
 			break;
 		}
-		error = vm_reinit(sc->vmm_vm);
+		error = vm_reinit(sc->vmm_vm, reinit.flags);
 		(void) vmm_drv_block_hook(sc, B_FALSE);
 		break;
+	}
 	case VM_STAT_DESC: {
 		struct vm_stat_desc statdesc;
 
diff --git a/usr/src/uts/i86pc/sys/vmm_dev.h b/usr/src/uts/i86pc/sys/vmm_dev.h
index f371ad1266..5884c1a554 100644
--- a/usr/src/uts/i86pc/sys/vmm_dev.h
+++ b/usr/src/uts/i86pc/sys/vmm_dev.h
@@ -212,6 +212,12 @@ struct vm_suspend {
 	enum vm_suspend_how how;
 };
 
+#define	VM_REINIT_F_FORCE_SUSPEND	(1 << 0)
+
+struct vm_reinit {
+	uint64_t	flags;
+};
+
 struct vm_gla2gpa {
 	int		vcpuid;		/* inputs */
 	int		prot;		/* PROT_READ or PROT_WRITE */