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.

More details available here:
http://lists.freebsd.org/pipermail/freebsd-virtualization/2013-December/001825.html

Reviewed by:	grehan
Reported by:	Markiyan Kushnir (markiyan.kushnir at gmail.com)
MFC after:	3 days
This commit is contained in:
Neel Natu 2013-12-22 20:29:59 +00:00
parent 85838e48bd
commit f80330a820
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=259737
4 changed files with 36 additions and 29 deletions

View File

@ -146,7 +146,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

View File

@ -804,12 +804,26 @@ 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"));
}
/*
* The following state transitions are allowed:
* IDLE -> FROZEN -> IDLE
@ -830,12 +844,14 @@ vcpu_set_state_locked(struct vcpu *vcpu, enum vcpu_state newstate)
break;
}
if (error == 0)
vcpu->state = newstate;
else
error = EBUSY;
if (error)
return (EBUSY);
return (error);
vcpu->state = newstate;
if (newstate == VCPU_IDLE)
wakeup(&vcpu->state);
return (0);
}
static void
@ -843,7 +859,7 @@ vcpu_require_state(struct vm *vm, int vcpuid, enum vcpu_state newstate)
{
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);
}
@ -852,7 +868,7 @@ vcpu_require_state_locked(struct vcpu *vcpu, enum vcpu_state newstate)
{
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);
}
@ -1235,7 +1251,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;
@ -1246,7 +1263,7 @@ vcpu_set_state(struct vm *vm, int vcpuid, enum vcpu_state newstate)
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);

View File

@ -197,7 +197,7 @@ vmmdev_ioctl(struct cdev *cdev, u_long cmd, caddr_t data, int fflag,
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 cmd, caddr_t data, int fflag,
*/
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;
}
@ -382,10 +382,10 @@ vmmdev_ioctl(struct cdev *cdev, u_long cmd, caddr_t data, int fflag,
}
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:

View File

@ -485,19 +485,8 @@ vm_loop(struct vmctx *ctx, int vcpu, uint64_t rip)
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;