From ab132285915ec97232417b8a2c9cd73eefa3d0eb Mon Sep 17 00:00:00 2001 From: Victor Sun Date: Wed, 25 Dec 2019 14:26:54 +0800 Subject: [PATCH] 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 Reviewed-by: Jason Chen CJ Acked-by: Eddie Dong --- hypervisor/arch/x86/guest/vcpu.c | 53 +++++++++++++++--------- hypervisor/arch/x86/guest/vlapic.c | 8 ++-- hypervisor/common/hypercall.c | 7 +--- hypervisor/include/arch/x86/guest/vcpu.h | 7 ++-- 4 files changed, 43 insertions(+), 32 deletions(-) diff --git a/hypervisor/arch/x86/guest/vcpu.c b/hypervisor/arch/x86/guest/vcpu.c index 071e201d1..29e3a48f4 100644 --- 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 index f487a74ff..e1fca5e82 100644 --- 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 index 94ec0da4c..2db10328b 100644 --- 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 index dc2cb99a0..2be7c8423 100644 --- 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.