Date: Sun, 22 Dec 2013 20:29:59 +0000 (UTC) From: Neel Natu <neel@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r259737 - in head: sys/amd64/include sys/amd64/vmm usr.sbin/bhyve Message-ID: <201312222029.rBMKTxup014899@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: neel Date: Sun Dec 22 20:29:59 2013 New Revision: 259737 URL: http://svnweb.freebsd.org/changeset/base/259737 Log: 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 Modified: head/sys/amd64/include/vmm.h head/sys/amd64/vmm/vmm.c head/sys/amd64/vmm/vmm_dev.c head/usr.sbin/bhyve/bhyverun.c Modified: head/sys/amd64/include/vmm.h ============================================================================== --- head/sys/amd64/include/vmm.h Sun Dec 22 19:47:22 2013 (r259736) +++ head/sys/amd64/include/vmm.h Sun Dec 22 20:29:59 2013 (r259737) @@ -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 Modified: head/sys/amd64/vmm/vmm.c ============================================================================== --- head/sys/amd64/vmm/vmm.c Sun Dec 22 19:47:22 2013 (r259736) +++ head/sys/amd64/vmm/vmm.c Sun Dec 22 20:29:59 2013 (r259737) @@ -804,13 +804,27 @@ 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 * FROZEN -> RUNNING -> FROZEN @@ -830,12 +844,14 @@ vcpu_set_state_locked(struct vcpu *vcpu, 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 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); } @@ -852,7 +868,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); } @@ -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 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); Modified: head/sys/amd64/vmm/vmm_dev.c ============================================================================== --- head/sys/amd64/vmm/vmm_dev.c Sun Dec 22 19:47:22 2013 (r259736) +++ head/sys/amd64/vmm/vmm_dev.c Sun Dec 22 20:29:59 2013 (r259737) @@ -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; } @@ -382,10 +382,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: head/usr.sbin/bhyve/bhyverun.c ============================================================================== --- head/usr.sbin/bhyve/bhyverun.c Sun Dec 22 19:47:22 2013 (r259736) +++ head/usr.sbin/bhyve/bhyverun.c Sun Dec 22 20:29:59 2013 (r259737) @@ -485,19 +485,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;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201312222029.rBMKTxup014899>