From 553d59644b991e2d621c336a1c2de86a8b88b42d Mon Sep 17 00:00:00 2001 From: Tao Yuhong Date: Thu, 17 Jun 2021 09:21:34 -0400 Subject: [PATCH] HV: Fix decode_instruction() trigger #UD for emulating UC-lock When ACRN uses decode_instruction to emulate split-lock/uc-lock instruction, It is actually a try-decode to see if it is XCHG. If the instruction is XCHG instruction, ACRN must emulate it (inject #PF if it is triggered) with peer VCPUs paused, and advance the guest IP. If the instruction is a LOCK prefixed instruction with accessing the UC memory, ACRN Halted the peer VCPUs, and advance the IP to skip the LOCK prefix, and then let the VCPU Executes one instruction by enabling IRQ Windows vm-exit. For other cases, ACRN injects the exception back to VCPU without emulating it. So change the API to decode_instruction(vcpu, bool full_decode), when full_decode is true, the API does same thing as before. When full_decode is false, the different is if decode_instruction() meet unknown instruction, will keep return = -1 and do not inject #UD. We can use this to distinguish that an #UD has been skipped, and need inject #AC/#GP back. Tracked-On: #6299 Signed-off-by: Tao Yuhong --- hypervisor/arch/x86/guest/instr_emul.c | 17 +++++++++++++---- hypervisor/arch/x86/guest/splitlock.c | 7 +++++-- hypervisor/arch/x86/guest/vlapic.c | 2 +- hypervisor/arch/x86/guest/vmx_io.c | 2 +- .../include/arch/x86/asm/guest/instr_emul.h | 2 +- 5 files changed, 21 insertions(+), 9 deletions(-) diff --git a/hypervisor/arch/x86/guest/instr_emul.c b/hypervisor/arch/x86/guest/instr_emul.c index 342d728d3..6d3ee6f59 100644 --- a/hypervisor/arch/x86/guest/instr_emul.c +++ b/hypervisor/arch/x86/guest/instr_emul.c @@ -2340,7 +2340,14 @@ static int32_t instr_check_gva(struct acrn_vcpu *vcpu, enum vm_cpu_mode cpu_mode return ret; } -int32_t decode_instruction(struct acrn_vcpu *vcpu) + /* @retval >=0 on success + * @retval -EINVAL on any failure if (full_decode == true). + * @retval -EINVAL on any failure except unknown instruction if (full_decode == false). + * @retval -1 for unknown instruction if (full_decode == false). + * + * For unknown instruction, when full_decode is false, will keep retval = -1, and do not inject #UD + */ +int32_t decode_instruction(struct acrn_vcpu *vcpu, bool full_decode) { struct instr_emul_ctxt *emul_ctxt; uint32_t csar; @@ -2361,9 +2368,11 @@ int32_t decode_instruction(struct acrn_vcpu *vcpu) retval = local_decode_instruction(cpu_mode, seg_desc_def32(csar), &emul_ctxt->vie); if (retval != 0) { - pr_err("decode instruction failed @ 0x%016lx:", vcpu_get_rip(vcpu)); - vcpu_inject_ud(vcpu); - retval = -EFAULT; + if (full_decode) { + pr_err("decode instruction failed @ 0x%016lx:", vcpu_get_rip(vcpu)); + vcpu_inject_ud(vcpu); + retval = -EFAULT; + } } else { /* * We do operand check in instruction decode phase and diff --git a/hypervisor/arch/x86/guest/splitlock.c b/hypervisor/arch/x86/guest/splitlock.c index 09bcace47..69eed801d 100644 --- a/hypervisor/arch/x86/guest/splitlock.c +++ b/hypervisor/arch/x86/guest/splitlock.c @@ -111,7 +111,7 @@ int32_t emulate_splitlock(struct acrn_vcpu *vcpu, uint32_t exception_vector, boo /* Skip the #AC, we have emulated it. */ *queue_exception = false; } else { - status = decode_instruction(vcpu); + status = decode_instruction(vcpu, false); if (status >= 0) { /* * If this is the xchg, then emulate it, otherwise, @@ -149,10 +149,13 @@ int32_t emulate_splitlock(struct acrn_vcpu *vcpu, uint32_t exception_vector, boo } else { if (status == -EFAULT) { pr_info("page fault happen during decode_instruction"); - status = 0; /* For this case, Inject #PF, not to queue #AC */ *queue_exception = false; } + + /* if decode_instruction(full_decode = false) return -1, that means this is an unknown instruction, + * and has skipped #UD injection. Just keep queue_exception = true to inject #AC back */ + status = 0; } } } diff --git a/hypervisor/arch/x86/guest/vlapic.c b/hypervisor/arch/x86/guest/vlapic.c index b3afd47ba..0e94e2ca1 100644 --- a/hypervisor/arch/x86/guest/vlapic.c +++ b/hypervisor/arch/x86/guest/vlapic.c @@ -2404,7 +2404,7 @@ int32_t apic_access_vmexit_handler(struct acrn_vcpu *vcpu) * 3 = linear access (read or write) during event delivery */ if (((access_type == TYPE_LINEAR_APIC_INST_READ) || (access_type == TYPE_LINEAR_APIC_INST_WRITE)) && - (decode_instruction(vcpu) >= 0)) { + (decode_instruction(vcpu, true) >= 0)) { vlapic = vcpu_vlapic(vcpu); offset = (uint32_t)apic_access_offset(qual); mmio = &vcpu->req.reqs.mmio_request; diff --git a/hypervisor/arch/x86/guest/vmx_io.c b/hypervisor/arch/x86/guest/vmx_io.c index 7fa2c959a..63a311d57 100644 --- a/hypervisor/arch/x86/guest/vmx_io.c +++ b/hypervisor/arch/x86/guest/vmx_io.c @@ -150,7 +150,7 @@ int32_t ept_violation_vmexit_handler(struct acrn_vcpu *vcpu) */ mmio_req->address = gpa; - ret = decode_instruction(vcpu); + ret = decode_instruction(vcpu, true); if (ret > 0) { mmio_req->size = (uint64_t)ret; /* diff --git a/hypervisor/include/arch/x86/asm/guest/instr_emul.h b/hypervisor/include/arch/x86/asm/guest/instr_emul.h index 6969c0412..2db70b42f 100644 --- a/hypervisor/include/arch/x86/asm/guest/instr_emul.h +++ b/hypervisor/include/arch/x86/asm/guest/instr_emul.h @@ -92,7 +92,7 @@ struct instr_emul_ctxt { }; int32_t emulate_instruction(struct acrn_vcpu *vcpu); -int32_t decode_instruction(struct acrn_vcpu *vcpu); +int32_t decode_instruction(struct acrn_vcpu *vcpu, bool full_decode); bool is_current_opcode_xchg(struct acrn_vcpu *vcpu); #endif