HV: ensure valid vcpu state transition

The vcpu state machine transition should follow below rule: old vcpu state new vcpu state ============== ============== VCPU_OFFLINE --- create_vcpu --> VCPU_INIT VCPU_INIT --- launch_vcpu --> VCPU_RUNNING VCPU_RUNNING --- pause_vcpu --> VCPU_PAUSED VCPU_PAUSED --- resume_vcpu --> VCPU_RUNNING VCPU_RUNNING/PAUSED --- pause_vcpu --> VCPU_ZOMBIE VCPU_INIT --- pause_vcpu --> VCPU_ZOMBIE VCPU_ZOMBIE --- reset_vcpu --> VCPU_INIT VCPU_ZOMBIE --- offline_vcpu--> VCPU_OFFLINE Tracked-On: #4267 Signed-off-by: Victor Sun <victor.sun@intel.com> Reviewed-by: Jason Chen CJ <jason.cj.chen@intel.com> Acked-by: Eddie Dong <eddie.dong@intel.com>
projectacrn · Dec 27, 2019 · ab13228 · ab13228
1 parent a5158e2
commit ab13228
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 32 deletions.
diff --git a/hypervisor/arch/x86/guest/vcpu.c b/hypervisor/arch/x86/guest/vcpu.c
@@ -633,9 +633,11 @@ int32_t run_vcpu(struct acrn_vcpu *vcpu)
  */
 void offline_vcpu(struct acrn_vcpu *vcpu)
 {
-	vlapic_free(vcpu);
-	per_cpu(ever_run_vcpu, pcpuid_from_vcpu(vcpu)) = NULL;
-	vcpu->state = VCPU_OFFLINE;
+	if (vcpu->state == VCPU_ZOMBIE) {
+		vlapic_free(vcpu);
+		per_cpu(ever_run_vcpu, pcpuid_from_vcpu(vcpu)) = NULL;
+		vcpu->state = VCPU_OFFLINE;
+	}
 }
 
 void kick_vcpu(const struct acrn_vcpu *vcpu)
@@ -677,10 +679,10 @@ static uint64_t build_stack_frame(struct acrn_vcpu *vcpu)
 void reset_vcpu(struct acrn_vcpu *vcpu, enum reset_mode mode)
 {
 	pr_dbg("vcpu%hu reset", vcpu->vcpu_id);
-	ASSERT(vcpu->state != VCPU_RUNNING,
-			"reset vcpu when it's running");
+	ASSERT(vcpu->state == VCPU_ZOMBIE,
+			"reset vcpu only when it's in zombie");
 
-	if (vcpu->state != VCPU_INIT) {
+	if (vcpu->state == VCPU_ZOMBIE) {
 		vcpu_reset_internal(vcpu, mode);
 		vcpu->state = VCPU_INIT;
 	}
@@ -693,27 +695,36 @@ void pause_vcpu(struct acrn_vcpu *vcpu, enum vcpu_state new_state)
 	pr_dbg("vcpu%hu paused, new state: %d",
 		vcpu->vcpu_id, new_state);
 
-	vcpu->prev_state = vcpu->state;
-	vcpu->state = new_state;
+	if (((vcpu->state == VCPU_RUNNING) || (vcpu->state == VCPU_PAUSED) || (vcpu->state == VCPU_INIT))
+			&& ((new_state == VCPU_PAUSED) || (new_state == VCPU_ZOMBIE))) {
+		vcpu->prev_state = vcpu->state;
+		vcpu->state = new_state;
 
-	if (vcpu->prev_state == VCPU_RUNNING) {
-		sleep_thread(&vcpu->thread_obj);
-	}
-	if (pcpu_id != get_pcpu_id()) {
-		while (vcpu->running) {
-			asm_pause();
+		if (vcpu->prev_state == VCPU_RUNNING) {
+			sleep_thread(&vcpu->thread_obj);
+		}
+		if (pcpu_id != get_pcpu_id()) {
+			while (vcpu->running) {
+				asm_pause();
+			}
 		}
 	}
 }
 
-void resume_vcpu(struct acrn_vcpu *vcpu)
+int32_t resume_vcpu(struct acrn_vcpu *vcpu)
 {
+	int32_t ret = -1;
+
 	pr_dbg("vcpu%hu resumed", vcpu->vcpu_id);
 
-	vcpu->state = vcpu->prev_state;
-	if (vcpu->state == VCPU_RUNNING) {
-		wake_thread(&vcpu->thread_obj);
+	if (vcpu->state == VCPU_PAUSED) {
+		vcpu->state = vcpu->prev_state;
+		if (vcpu->state == VCPU_RUNNING) {
+			wake_thread(&vcpu->thread_obj);
+		}
+		ret = 0;
 	}
+	return ret;
 }
 
 void save_xsave_area(struct ext_context *ectx)
@@ -780,10 +791,12 @@ void launch_vcpu(struct acrn_vcpu *vcpu)
 {
 	uint16_t pcpu_id = pcpuid_from_vcpu(vcpu);
 
-	vcpu->state = VCPU_RUNNING;
 	pr_dbg("vcpu%hu scheduled on pcpu%hu", vcpu->vcpu_id, pcpu_id);
 
-	wake_thread(&vcpu->thread_obj);
+	if (vcpu->state == VCPU_INIT) {
+		vcpu->state = VCPU_RUNNING;
+		wake_thread(&vcpu->thread_obj);
+	}
 }
 
 /* help function for vcpu create */

diff --git a/hypervisor/arch/x86/guest/vlapic.c b/hypervisor/arch/x86/guest/vlapic.c
@@ -1161,9 +1161,11 @@ vlapic_process_init_sipi(struct acrn_vcpu* target_vcpu, uint32_t mode, uint32_t
 				"Sending INIT to %hu",
 				target_vcpu->vcpu_id);
 
-			/* put target vcpu to INIT state and wait for SIPI */
-			pause_vcpu(target_vcpu, VCPU_PAUSED);
-			reset_vcpu(target_vcpu, INIT_RESET);
+			if (target_vcpu->state != VCPU_INIT) {
+				/* put target vcpu to INIT state and wait for SIPI */
+				pause_vcpu(target_vcpu, VCPU_ZOMBIE);
+				reset_vcpu(target_vcpu, INIT_RESET);
+			}
 			/* new cpu model only need one SIPI to kick AP run,
 			 * the second SIPI will be ignored as it move out of
 			 * wait-for-SIPI state.

diff --git a/hypervisor/common/hypercall.c b/hypervisor/common/hypercall.c
@@ -552,11 +552,8 @@ int32_t hcall_notify_ioreq_finish(uint16_t vmid, uint16_t vcpu_id)
 				__func__, vcpu_id, target_vm->vm_id);
 		} else {
 			vcpu = vcpu_from_vid(target_vm, vcpu_id);
-			if (vcpu->state == VCPU_PAUSED) {
-				if (!vcpu->vm->sw.is_completion_polling) {
-					resume_vcpu(vcpu);
-				}
-				ret = 0;
+			if (!vcpu->vm->sw.is_completion_polling) {
+				ret = resume_vcpu(vcpu);
 			}
 		}
 	}

diff --git a/hypervisor/include/arch/x86/guest/vcpu.h b/hypervisor/include/arch/x86/guest/vcpu.h
@@ -131,12 +131,11 @@
 		if (vcpu->state != VCPU_OFFLINE)
 
 enum vcpu_state {
+	VCPU_OFFLINE = 0U,
 	VCPU_INIT,
 	VCPU_RUNNING,
 	VCPU_PAUSED,
 	VCPU_ZOMBIE,
-	VCPU_OFFLINE,
-	VCPU_UNKNOWN_STATE,
 };
 
 enum vm_cpu_mode {
@@ -626,9 +625,9 @@ void pause_vcpu(struct acrn_vcpu *vcpu, enum vcpu_state new_state);
  *
  * @param[inout] vcpu pointer to vcpu data structure
  *
- * @return None
+ * @return 0 on success, -1 on failure.
  */
-void resume_vcpu(struct acrn_vcpu *vcpu);
+int32_t resume_vcpu(struct acrn_vcpu *vcpu);
 
 /**
  * @brief set the vcpu to running state, then it will be scheculed.