Author: jhb
Date: Sun May 18 04:33:24 2014
New Revision: 266393
URL: http://svnweb.freebsd.org/changeset/base/266393
Log:
MFC 259737, 262646:
Fix a couple of issues with vcpu state:
- Add a parameter to 'vcpu_set_state()' to enforce that the vcpu is in the
IDLE state before the requested state transition. This guarantees that
there is exactly one ioctl() operating on a vcpu at any point in time and
prevents unintended state transitions.
- Fix a race between VMRUN() and vcpu_notify_event() due to 'vcpu->hostcpu'
being updated outside of the vcpu_lock().
Modified:
stable/10/sys/amd64/include/vmm.h
stable/10/sys/amd64/vmm/vmm.c
stable/10/sys/amd64/vmm/vmm_dev.c
stable/10/usr.sbin/bhyve/bhyverun.c
Directory Properties:
stable/10/ (props changed)
Modified: stable/10/sys/amd64/include/vmm.h
==============================================================================
--- stable/10/sys/amd64/include/vmm.h Sun May 18 04:21:12 2014
(r266392)
+++ stable/10/sys/amd64/include/vmm.h Sun May 18 04:33:24 2014
(r266393)
@@ -177,7 +177,8 @@ enum vcpu_state {
VCPU_SLEEPING,
};
-int vcpu_set_state(struct vm *vm, int vcpu, enum vcpu_state state);
+int vcpu_set_state(struct vm *vm, int vcpu, enum vcpu_state state,
+ bool from_idle);
enum vcpu_state vcpu_get_state(struct vm *vm, int vcpu, int *hostcpu);
static int __inline
Modified: stable/10/sys/amd64/vmm/vmm.c
==============================================================================
--- stable/10/sys/amd64/vmm/vmm.c Sun May 18 04:21:12 2014
(r266392)
+++ stable/10/sys/amd64/vmm/vmm.c Sun May 18 04:33:24 2014
(r266393)
@@ -837,13 +837,35 @@ save_guest_fpustate(struct vcpu *vcpu)
static VMM_STAT(VCPU_IDLE_TICKS, "number of ticks vcpu was idle");
static int
-vcpu_set_state_locked(struct vcpu *vcpu, enum vcpu_state newstate)
+vcpu_set_state_locked(struct vcpu *vcpu, enum vcpu_state newstate,
+ bool from_idle)
{
int error;
vcpu_assert_locked(vcpu);
/*
+ * State transitions from the vmmdev_ioctl() must always begin from
+ * the VCPU_IDLE state. This guarantees that there is only a single
+ * ioctl() operating on a vcpu at any point.
+ */
+ if (from_idle) {
+ while (vcpu->state != VCPU_IDLE)
+ msleep_spin(&vcpu->state, &vcpu->mtx, "vmstat", hz);
+ } else {
+ KASSERT(vcpu->state != VCPU_IDLE, ("invalid transition from "
+ "vcpu idle state"));
+ }
+
+ if (vcpu->state == VCPU_RUNNING) {
+ KASSERT(vcpu->hostcpu == curcpu, ("curcpu %d and hostcpu %d "
+ "mismatch for running vcpu", curcpu, vcpu->hostcpu));
+ } else {
+ KASSERT(vcpu->hostcpu == NOCPU, ("Invalid hostcpu %d for a "
+ "vcpu that is not running", vcpu->hostcpu));
+ }
+
+ /*
* The following state transitions are allowed:
* IDLE -> FROZEN -> IDLE
* FROZEN -> RUNNING -> FROZEN
@@ -863,12 +885,19 @@ vcpu_set_state_locked(struct vcpu *vcpu,
break;
}
- if (error == 0)
- vcpu->state = newstate;
+ if (error)
+ return (EBUSY);
+
+ vcpu->state = newstate;
+ if (newstate == VCPU_RUNNING)
+ vcpu->hostcpu = curcpu;
else
- error = EBUSY;
+ vcpu->hostcpu = NOCPU;
- return (error);
+ if (newstate == VCPU_IDLE)
+ wakeup(&vcpu->state);
+
+ return (0);
}
static void
@@ -876,7 +905,7 @@ vcpu_require_state(struct vm *vm, int vc
{
int error;
- if ((error = vcpu_set_state(vm, vcpuid, newstate)) != 0)
+ if ((error = vcpu_set_state(vm, vcpuid, newstate, false)) != 0)
panic("Error %d setting state to %d\n", error, newstate);
}
@@ -885,7 +914,7 @@ vcpu_require_state_locked(struct vcpu *v
{
int error;
- if ((error = vcpu_set_state_locked(vcpu, newstate)) != 0)
+ if ((error = vcpu_set_state_locked(vcpu, newstate, false)) != 0)
panic("Error %d setting state to %d", error, newstate);
}
@@ -1131,9 +1160,7 @@ restart:
restore_guest_fpustate(vcpu);
vcpu_require_state(vm, vcpuid, VCPU_RUNNING);
- vcpu->hostcpu = curcpu;
error = VMRUN(vm->cookie, vcpuid, rip, pmap, &vm->rendezvous_func);
- vcpu->hostcpu = NOCPU;
vcpu_require_state(vm, vcpuid, VCPU_FROZEN);
save_guest_fpustate(vcpu);
@@ -1343,7 +1370,8 @@ vm_iommu_domain(struct vm *vm)
}
int
-vcpu_set_state(struct vm *vm, int vcpuid, enum vcpu_state newstate)
+vcpu_set_state(struct vm *vm, int vcpuid, enum vcpu_state newstate,
+ bool from_idle)
{
int error;
struct vcpu *vcpu;
@@ -1354,7 +1382,7 @@ vcpu_set_state(struct vm *vm, int vcpuid
vcpu = &vm->vcpu[vcpuid];
vcpu_lock(vcpu);
- error = vcpu_set_state_locked(vcpu, newstate);
+ error = vcpu_set_state_locked(vcpu, newstate, from_idle);
vcpu_unlock(vcpu);
return (error);
@@ -1475,19 +1503,28 @@ vcpu_notify_event(struct vm *vm, int vcp
vcpu_lock(vcpu);
hostcpu = vcpu->hostcpu;
- if (hostcpu == NOCPU) {
- if (vcpu->state == VCPU_SLEEPING)
- wakeup_one(vcpu);
- } else {
- if (vcpu->state != VCPU_RUNNING)
- panic("invalid vcpu state %d", vcpu->state);
+ if (vcpu->state == VCPU_RUNNING) {
+ KASSERT(hostcpu != NOCPU, ("vcpu running on invalid hostcpu"));
if (hostcpu != curcpu) {
- if (lapic_intr)
+ if (lapic_intr) {
vlapic_post_intr(vcpu->vlapic, hostcpu,
vmm_ipinum);
- else
+ } else {
ipi_cpu(hostcpu, vmm_ipinum);
+ }
+ } else {
+ /*
+ * If the 'vcpu' is running on 'curcpu' then it must
+ * be sending a notification to itself (e.g. SELF_IPI).
+ * The pending event will be picked up when the vcpu
+ * transitions back to guest context.
+ */
}
+ } else {
+ KASSERT(hostcpu == NOCPU, ("vcpu state %d not consistent "
+ "with hostcpu %d", vcpu->state, hostcpu));
+ if (vcpu->state == VCPU_SLEEPING)
+ wakeup_one(vcpu);
}
vcpu_unlock(vcpu);
}
Modified: stable/10/sys/amd64/vmm/vmm_dev.c
==============================================================================
--- stable/10/sys/amd64/vmm/vmm_dev.c Sun May 18 04:21:12 2014
(r266392)
+++ stable/10/sys/amd64/vmm/vmm_dev.c Sun May 18 04:33:24 2014
(r266393)
@@ -197,7 +197,7 @@ vmmdev_ioctl(struct cdev *cdev, u_long c
goto done;
}
- error = vcpu_set_state(sc->vm, vcpu, VCPU_FROZEN);
+ error = vcpu_set_state(sc->vm, vcpu, VCPU_FROZEN, true);
if (error)
goto done;
@@ -214,14 +214,14 @@ vmmdev_ioctl(struct cdev *cdev, u_long c
*/
error = 0;
for (vcpu = 0; vcpu < VM_MAXCPU; vcpu++) {
- error = vcpu_set_state(sc->vm, vcpu, VCPU_FROZEN);
+ error = vcpu_set_state(sc->vm, vcpu, VCPU_FROZEN, true);
if (error)
break;
}
if (error) {
while (--vcpu >= 0)
- vcpu_set_state(sc->vm, vcpu, VCPU_IDLE);
+ vcpu_set_state(sc->vm, vcpu, VCPU_IDLE, false);
goto done;
}
@@ -387,10 +387,10 @@ vmmdev_ioctl(struct cdev *cdev, u_long c
}
if (state_changed == 1) {
- vcpu_set_state(sc->vm, vcpu, VCPU_IDLE);
+ vcpu_set_state(sc->vm, vcpu, VCPU_IDLE, false);
} else if (state_changed == 2) {
for (vcpu = 0; vcpu < VM_MAXCPU; vcpu++)
- vcpu_set_state(sc->vm, vcpu, VCPU_IDLE);
+ vcpu_set_state(sc->vm, vcpu, VCPU_IDLE, false);
}
done:
Modified: stable/10/usr.sbin/bhyve/bhyverun.c
==============================================================================
--- stable/10/usr.sbin/bhyve/bhyverun.c Sun May 18 04:21:12 2014
(r266392)
+++ stable/10/usr.sbin/bhyve/bhyverun.c Sun May 18 04:33:24 2014
(r266393)
@@ -493,19 +493,8 @@ vm_loop(struct vmctx *ctx, int vcpu, uin
while (1) {
error = vm_run(ctx, vcpu, rip, &vmexit[vcpu]);
- if (error != 0) {
- /*
- * It is possible that 'vmmctl' or some other process
- * has transitioned the vcpu to CANNOT_RUN state right
- * before we tried to transition it to RUNNING.
- *
- * This is expected to be temporary so just retry.
- */
- if (errno == EBUSY)
- continue;
- else
- break;
- }
+ if (error != 0)
+ break;
prevcpu = vcpu;
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
