As all callers in inject_emulated_exception() use "struct x86_exception" directly, capture it locally instead of using the context. No functional change intended. Suggested-by: Sean Christopherson Signed-off-by: Hou Wenlong --- arch/x86/kvm/x86.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 0c6d899d53dd..ab298bfa7d9f 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -8923,15 +8923,14 @@ static void toggle_interruptibility(struct kvm_vcpu *vcpu, u32 mask) static void inject_emulated_exception(struct kvm_vcpu *vcpu) { - struct x86_emulate_ctxt *ctxt = vcpu->arch.emulate_ctxt; + struct x86_exception *ex = &vcpu->arch.emulate_ctxt->exception; - if (ctxt->exception.vector == PF_VECTOR) - kvm_inject_emulated_page_fault(vcpu, &ctxt->exception); - else if (ctxt->exception.error_code_valid) - kvm_queue_exception_e(vcpu, ctxt->exception.vector, - ctxt->exception.error_code); + if (ex->vector == PF_VECTOR) + kvm_inject_emulated_page_fault(vcpu, ex); + else if (ex->error_code_valid) + kvm_queue_exception_e(vcpu, ex->vector, ex->error_code); else - kvm_queue_exception(vcpu, ctxt->exception.vector); + kvm_queue_exception(vcpu, ex->vector); } static struct x86_emulate_ctxt *alloc_emulate_ctxt(struct kvm_vcpu *vcpu) -- 2.31.1 Record DR6 in emulate_db() and use kvm_queue_exception_p() to set DR6 instead of directly using kvm_set_dr6() in emulation, which keeps the handling of DR6 during #DB injection consistent with other code paths. No functional change intended. Signed-off-by: Hou Wenlong --- arch/x86/kvm/emulate.c | 14 ++++---------- arch/x86/kvm/kvm_emulate.h | 6 +++++- arch/x86/kvm/x86.c | 5 ++++- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index c8e292e9a24d..997cd6e46d90 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -540,8 +540,9 @@ static int emulate_exception(struct x86_emulate_ctxt *ctxt, int vec, return X86EMUL_PROPAGATE_FAULT; } -static int emulate_db(struct x86_emulate_ctxt *ctxt) +static int emulate_db(struct x86_emulate_ctxt *ctxt, unsigned long dr6) { + ctxt->exception.dr6 = dr6; return emulate_exception(ctxt, DB_VECTOR, 0, false); } @@ -3834,15 +3835,8 @@ static int check_dr_read(struct x86_emulate_ctxt *ctxt) if ((cr4 & X86_CR4_DE) && (dr == 4 || dr == 5)) return emulate_ud(ctxt); - if (ctxt->ops->get_dr(ctxt, 7) & DR7_GD) { - ulong dr6; - - dr6 = ctxt->ops->get_dr(ctxt, 6); - dr6 &= ~DR_TRAP_BITS; - dr6 |= DR6_BD | DR6_ACTIVE_LOW; - ctxt->ops->set_dr(ctxt, 6, dr6); - return emulate_db(ctxt); - } + if (ctxt->ops->get_dr(ctxt, 7) & DR7_GD) + return emulate_db(ctxt, DR6_BD); return X86EMUL_CONTINUE; } diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h index fb3dab4b5a53..7fe38b174e18 100644 --- a/arch/x86/kvm/kvm_emulate.h +++ b/arch/x86/kvm/kvm_emulate.h @@ -24,7 +24,11 @@ struct x86_exception { bool error_code_valid; u16 error_code; bool nested_page_fault; - u64 address; /* cr2 or nested page fault gpa */ + union { + u64 address; /* cr2 or nested page fault gpa */ + unsigned long dr6; + u64 payload; + }; u8 async_page_fault; unsigned long exit_qualification; }; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index ab298bfa7d9f..f33ce947633e 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -8925,7 +8925,9 @@ static void inject_emulated_exception(struct kvm_vcpu *vcpu) { struct x86_exception *ex = &vcpu->arch.emulate_ctxt->exception; - if (ex->vector == PF_VECTOR) + if (ex->vector == DB_VECTOR) + kvm_queue_exception_e(vcpu, DB_VECTOR, ex->dr6); + else if (ex->vector == PF_VECTOR) kvm_inject_emulated_page_fault(vcpu, ex); else if (ex->error_code_valid) kvm_queue_exception_e(vcpu, ex->vector, ex->error_code); @@ -8970,6 +8972,7 @@ static void init_emulate_ctxt(struct kvm_vcpu *vcpu) ctxt->interruptibility = 0; ctxt->have_exception = false; ctxt->exception.vector = -1; + ctxt->exception.payload = 0; ctxt->perm_ok = false; init_decode_cache(ctxt); -- 2.31.1 When a DR access instruction is emulated by the x86 instruction emulator, only the guest DR7.GD is checked. Since the instruction emulation path has already performed some guest debug checks, add a guest debug check in the DR access instruction emulation to improve the guest debug logic in the instruction emulator. Suggested-by: Lai Jiangshan Signed-off-by: Hou Wenlong --- arch/x86/kvm/emulate.c | 2 +- arch/x86/kvm/kvm_emulate.h | 1 + arch/x86/kvm/x86.c | 52 +++++++++++++++++++++++++++++++++----- arch/x86/kvm/x86.h | 7 +++++ 4 files changed, 54 insertions(+), 8 deletions(-) diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 997cd6e46d90..f2e2ee412b78 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -3835,7 +3835,7 @@ static int check_dr_read(struct x86_emulate_ctxt *ctxt) if ((cr4 & X86_CR4_DE) && (dr == 4 || dr == 5)) return emulate_ud(ctxt); - if (ctxt->ops->get_dr(ctxt, 7) & DR7_GD) + if (ctxt->ops->get_eff_dr(ctxt, 7) & DR7_GD) return emulate_db(ctxt, DR6_BD); return X86EMUL_CONTINUE; diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h index 7fe38b174e18..006a439a60b3 100644 --- a/arch/x86/kvm/kvm_emulate.h +++ b/arch/x86/kvm/kvm_emulate.h @@ -216,6 +216,7 @@ struct x86_emulate_ops { int (*set_cr)(struct x86_emulate_ctxt *ctxt, int cr, ulong val); int (*cpl)(struct x86_emulate_ctxt *ctxt); ulong (*get_dr)(struct x86_emulate_ctxt *ctxt, int dr); + ulong (*get_eff_dr)(struct x86_emulate_ctxt *ctxt, int dr); int (*set_dr)(struct x86_emulate_ctxt *ctxt, int dr, ulong value); int (*set_msr_with_filter)(struct x86_emulate_ctxt *ctxt, u32 msr_index, u64 data); int (*get_msr_with_filter)(struct x86_emulate_ctxt *ctxt, u32 msr_index, u64 *pdata); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index f33ce947633e..824eb489de43 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -1592,6 +1592,22 @@ unsigned long kvm_get_dr(struct kvm_vcpu *vcpu, int dr) } EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_get_dr); +static unsigned long kvm_get_eff_dr(struct kvm_vcpu *vcpu, int dr) +{ + size_t size = ARRAY_SIZE(vcpu->arch.eff_db); + + switch (dr) { + case 0 ... 3: + return vcpu->arch.eff_db[array_index_nospec(dr, size)]; + case 4: + case 6: + return vcpu->arch.dr6; + case 5: + default: /* 7 */ + return kvm_get_eff_dr7(vcpu); + } +} + int kvm_emulate_rdpmc(struct kvm_vcpu *vcpu) { u32 pmc = kvm_rcx_read(vcpu); @@ -8504,6 +8520,11 @@ static unsigned long emulator_get_dr(struct x86_emulate_ctxt *ctxt, int dr) return kvm_get_dr(emul_to_vcpu(ctxt), dr); } +static unsigned long emulator_get_eff_dr(struct x86_emulate_ctxt *ctxt, int dr) +{ + return kvm_get_eff_dr(emul_to_vcpu(ctxt), dr); +} + static int emulator_set_dr(struct x86_emulate_ctxt *ctxt, int dr, unsigned long value) { @@ -8877,6 +8898,7 @@ static const struct x86_emulate_ops emulate_ops = { .set_cr = emulator_set_cr, .cpl = emulator_get_cpl, .get_dr = emulator_get_dr, + .get_eff_dr = emulator_get_eff_dr, .set_dr = emulator_set_dr, .set_msr_with_filter = emulator_set_msr_with_filter, .get_msr_with_filter = emulator_get_msr_with_filter, @@ -8921,18 +8943,36 @@ static void toggle_interruptibility(struct kvm_vcpu *vcpu, u32 mask) } } -static void inject_emulated_exception(struct kvm_vcpu *vcpu) +static int kvm_inject_emulated_db(struct kvm_vcpu *vcpu, unsigned long dr6) +{ + struct kvm_run *kvm_run = vcpu->run; + + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) { + kvm_run->debug.arch.dr6 = dr6 | DR6_ACTIVE_LOW; + kvm_run->debug.arch.pc = kvm_get_linear_rip(vcpu); + kvm_run->debug.arch.exception = DB_VECTOR; + kvm_run->exit_reason = KVM_EXIT_DEBUG; + return 0; + } + + kvm_queue_exception_p(vcpu, DB_VECTOR, dr6); + return 1; +} + +static int inject_emulated_exception(struct kvm_vcpu *vcpu) { struct x86_exception *ex = &vcpu->arch.emulate_ctxt->exception; if (ex->vector == DB_VECTOR) - kvm_queue_exception_e(vcpu, DB_VECTOR, ex->dr6); - else if (ex->vector == PF_VECTOR) + return kvm_inject_emulated_db(vcpu, ex->dr6); + + if (ex->vector == PF_VECTOR) kvm_inject_emulated_page_fault(vcpu, ex); else if (ex->error_code_valid) kvm_queue_exception_e(vcpu, ex->vector, ex->error_code); else kvm_queue_exception(vcpu, ex->vector); + return 1; } static struct x86_emulate_ctxt *alloc_emulate_ctxt(struct kvm_vcpu *vcpu) @@ -9441,8 +9481,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, */ WARN_ON_ONCE(ctxt->exception.vector == UD_VECTOR || exception_type(ctxt->exception.vector) == EXCPT_TRAP); - inject_emulated_exception(vcpu); - return 1; + return inject_emulated_exception(vcpu); } return handle_emulation_failure(vcpu, emulation_type); } @@ -9537,8 +9576,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, if (ctxt->have_exception) { WARN_ON_ONCE(vcpu->mmio_needed && !vcpu->mmio_is_write); vcpu->mmio_needed = false; - r = 1; - inject_emulated_exception(vcpu); + r = inject_emulated_exception(vcpu); } else if (vcpu->arch.pio.count) { if (!vcpu->arch.pio.in) { /* FIXME: return into emulator if single-stepping. */ diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h index fdab0ad49098..7543ab2ec1f1 100644 --- a/arch/x86/kvm/x86.h +++ b/arch/x86/kvm/x86.h @@ -617,6 +617,13 @@ static inline bool kvm_dr6_valid(u64 data) return !(data >> 32); } +static inline unsigned long kvm_get_eff_dr7(struct kvm_vcpu *vcpu) +{ + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) + return vcpu->arch.guest_debug_dr7; + return vcpu->arch.dr7; +} + /* * Trigger machine check on the host. We assume all the MSRs are already set up * by the CPU and that we still run on the same CPU as the MCE occurred on. -- 2.31.1 When guest debug is enabled, the effective breakpoints are controlled by guest debug rather than by the guest itself. Therefore, only check the code breakpoints of guest debug in emulation if guest debug is enabled, in order to maintain consistency with hardware behavior. Fixes: 4a1e10d5b5d8 ("KVM: x86: handle hardware breakpoints during emulation") Signed-off-by: Hou Wenlong --- arch/x86/kvm/x86.c | 35 ++++++++++------------------------- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 824eb489de43..3000139a19db 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -9272,6 +9272,9 @@ EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_skip_emulated_instruction); static bool kvm_is_code_breakpoint_inhibited(struct kvm_vcpu *vcpu) { + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) + return false; + if (kvm_get_rflags(vcpu) & X86_EFLAGS_RF) return true; @@ -9288,6 +9291,8 @@ static bool kvm_is_code_breakpoint_inhibited(struct kvm_vcpu *vcpu) static bool kvm_vcpu_check_code_breakpoint(struct kvm_vcpu *vcpu, int emulation_type, int *r) { + unsigned long dr7 = kvm_get_eff_dr7(vcpu); + WARN_ON_ONCE(emulation_type & EMULTYPE_NO_DECODE); /* @@ -9308,34 +9313,14 @@ static bool kvm_vcpu_check_code_breakpoint(struct kvm_vcpu *vcpu, EMULTYPE_TRAP_UD | EMULTYPE_VMWARE_GP | EMULTYPE_PF)) return false; - if (unlikely(vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) && - (vcpu->arch.guest_debug_dr7 & DR7_BP_EN_MASK)) { - struct kvm_run *kvm_run = vcpu->run; - unsigned long eip = kvm_get_linear_rip(vcpu); - u32 dr6 = kvm_vcpu_check_hw_bp(eip, 0, - vcpu->arch.guest_debug_dr7, - vcpu->arch.eff_db); - - if (dr6 != 0) { - kvm_run->debug.arch.dr6 = dr6 | DR6_ACTIVE_LOW; - kvm_run->debug.arch.pc = eip; - kvm_run->debug.arch.exception = DB_VECTOR; - kvm_run->exit_reason = KVM_EXIT_DEBUG; - *r = 0; - return true; - } - } - - if (unlikely(vcpu->arch.dr7 & DR7_BP_EN_MASK) && + if (unlikely(dr7 & DR7_BP_EN_MASK) && !kvm_is_code_breakpoint_inhibited(vcpu)) { unsigned long eip = kvm_get_linear_rip(vcpu); - u32 dr6 = kvm_vcpu_check_hw_bp(eip, 0, - vcpu->arch.dr7, - vcpu->arch.db); + u32 dr6 = kvm_vcpu_check_hw_bp(eip, 0, dr7, + vcpu->arch.eff_db); - if (dr6 != 0) { - kvm_queue_exception_p(vcpu, DB_VECTOR, dr6); - *r = 1; + if (dr6) { + *r = kvm_inject_emulated_db(vcpu, dr6); return true; } } -- 2.31.1 Use kvm_inject_emulated_db() in kvm_vcpu_do_singlestep() to consolidate 'KVM_GUESTDBG_SINGLESTEP' check into kvm_inject_emulated_db() during emulation. No functional change intended. Suggested-by: Lai Jiangshan Signed-off-by: Hou Wenlong --- arch/x86/kvm/x86.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 3000139a19db..44c2886589d7 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -8947,7 +8947,7 @@ static int kvm_inject_emulated_db(struct kvm_vcpu *vcpu, unsigned long dr6) { struct kvm_run *kvm_run = vcpu->run; - if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) { + if (vcpu->guest_debug & (KVM_GUESTDBG_USE_HW_BP | KVM_GUESTDBG_SINGLESTEP)) { kvm_run->debug.arch.dr6 = dr6 | DR6_ACTIVE_LOW; kvm_run->debug.arch.pc = kvm_get_linear_rip(vcpu); kvm_run->debug.arch.exception = DB_VECTOR; @@ -9232,17 +9232,7 @@ static int kvm_vcpu_check_hw_bp(unsigned long addr, u32 type, u32 dr7, static int kvm_vcpu_do_singlestep(struct kvm_vcpu *vcpu) { - struct kvm_run *kvm_run = vcpu->run; - - if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) { - kvm_run->debug.arch.dr6 = DR6_BS | DR6_ACTIVE_LOW; - kvm_run->debug.arch.pc = kvm_get_linear_rip(vcpu); - kvm_run->debug.arch.exception = DB_VECTOR; - kvm_run->exit_reason = KVM_EXIT_DEBUG; - return 0; - } - kvm_queue_exception_p(vcpu, DB_VECTOR, DR6_BS); - return 1; + return kvm_inject_emulated_db(vcpu, DR6_BS); } int kvm_skip_emulated_instruction(struct kvm_vcpu *vcpu) -- 2.31.1 The single-step trap #DB should be injected after the instruction completes, as 'ctxt->tf' already records the old value of 'X86_EFLAGS_TF'. Therefore, it's okay to move kvm_set_rflags() up before kvm_vcpu_do_single_step() to align it more closely with hardware behavior. No functional change intended. Suggested-by: Sean Christopherson Signed-off-by: Hou Wenlong --- arch/x86/kvm/x86.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 44c2886589d7..7352c2114bab 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -9593,10 +9593,10 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, if (ctxt->is_branch) kvm_pmu_branch_retired(vcpu); kvm_rip_write(vcpu, ctxt->eip); + __kvm_set_rflags(vcpu, ctxt->eflags); if (r && (ctxt->tf || (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP))) r = kvm_vcpu_do_singlestep(vcpu); kvm_x86_call(update_emulated_instruction)(vcpu); - __kvm_set_rflags(vcpu, ctxt->eflags); } /* -- 2.31.1 The VM-Entry has a consistency check that when the 'STI' or 'MOVSS' blocking is active, the 'PENDING_DBG_EXCEPTIONS.BS' bit must be set if 'RFLAGS.TF' is set; otherwise, a VM-Fail is triggered during VM-entry. However, when 'STI' or 'MOV SS' is emulated (e.g., using the 'force emulation' prefix), the emulator only refreshes interruptibility state but pending debug exception state is not refreshed. Since the force emulation prefix causes a VM-Exit due to #UD interception, which clears the 'PENDING_DBG_EXCEPTIONS' bits, the emulator should refresh the 'PENDING_DBG_EXCEPTIONS.BS' bit when the 'RFLAGS.TF' bit is set to ensure the success of VM-Entry. Signed-off-by: Hou Wenlong --- arch/x86/include/asm/kvm-x86-ops.h | 1 + arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/vmx/main.c | 9 +++++++++ arch/x86/kvm/vmx/vmx.c | 15 ++++++++++----- arch/x86/kvm/vmx/x86_ops.h | 1 + arch/x86/kvm/x86.c | 7 ++++++- 6 files changed, 28 insertions(+), 6 deletions(-) diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h index de709fb5bd76..9a591f95adc4 100644 --- a/arch/x86/include/asm/kvm-x86-ops.h +++ b/arch/x86/include/asm/kvm-x86-ops.h @@ -50,6 +50,7 @@ KVM_X86_OP(get_gdt) KVM_X86_OP(set_gdt) KVM_X86_OP(sync_dirty_debug_regs) KVM_X86_OP(set_dr7) +KVM_X86_OP_OPTIONAL(refresh_pending_dbg_exceptions) KVM_X86_OP(cache_reg) KVM_X86_OP(get_rflags) KVM_X86_OP(set_rflags) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 5a3bfa293e8b..46fe25dee391 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -1763,6 +1763,7 @@ struct kvm_x86_ops { void (*set_gdt)(struct kvm_vcpu *vcpu, struct desc_ptr *dt); void (*sync_dirty_debug_regs)(struct kvm_vcpu *vcpu); void (*set_dr7)(struct kvm_vcpu *vcpu, unsigned long value); + void (*refresh_pending_dbg_exceptions)(struct kvm_vcpu *vcpu); void (*cache_reg)(struct kvm_vcpu *vcpu, enum kvm_reg reg); unsigned long (*get_rflags)(struct kvm_vcpu *vcpu); void (*set_rflags)(struct kvm_vcpu *vcpu, unsigned long rflags); diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c index a46ccd670785..2e20a5d65b91 100644 --- a/arch/x86/kvm/vmx/main.c +++ b/arch/x86/kvm/vmx/main.c @@ -465,6 +465,14 @@ static void vt_set_dr7(struct kvm_vcpu *vcpu, unsigned long val) vmx_set_dr7(vcpu, val); } +static void vt_refresh_pending_dbg_exceptions(struct kvm_vcpu *vcpu) +{ + if (WARN_ON_ONCE(is_td_vcpu(vcpu))) + return; + + vmx_refresh_pending_dbg_exceptions(vcpu); +} + static void vt_sync_dirty_debug_regs(struct kvm_vcpu *vcpu) { /* @@ -915,6 +923,7 @@ struct kvm_x86_ops vt_x86_ops __initdata = { .get_gdt = vt_op(get_gdt), .set_gdt = vt_op(set_gdt), .set_dr7 = vt_op(set_dr7), + .refresh_pending_dbg_exceptions = vt_op(refresh_pending_dbg_exceptions), .sync_dirty_debug_regs = vt_op(sync_dirty_debug_regs), .cache_reg = vt_op(cache_reg), .get_rflags = vt_op(get_rflags), diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 4cbe8c84b636..8ed031293549 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -5431,11 +5431,8 @@ static int handle_exception_nmi(struct kvm_vcpu *vcpu) */ if (is_icebp(intr_info)) WARN_ON(!skip_emulated_instruction(vcpu)); - else if ((vmx_get_rflags(vcpu) & X86_EFLAGS_TF) && - (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & - (GUEST_INTR_STATE_STI | GUEST_INTR_STATE_MOV_SS))) - vmcs_writel(GUEST_PENDING_DBG_EXCEPTIONS, - vmcs_readl(GUEST_PENDING_DBG_EXCEPTIONS) | DR6_BS); + else + vmx_refresh_pending_dbg_exceptions(vcpu); kvm_queue_exception_p(vcpu, DB_VECTOR, dr6); return 1; @@ -5742,6 +5739,14 @@ void vmx_set_dr7(struct kvm_vcpu *vcpu, unsigned long val) vmcs_writel(GUEST_DR7, val); } +void vmx_refresh_pending_dbg_exceptions(struct kvm_vcpu *vcpu) +{ + if ((vmx_get_rflags(vcpu) & X86_EFLAGS_TF) && + vmx_get_interrupt_shadow(vcpu)) + vmcs_writel(GUEST_PENDING_DBG_EXCEPTIONS, + vmcs_readl(GUEST_PENDING_DBG_EXCEPTIONS) | DR6_BS); +} + static int handle_tpr_below_threshold(struct kvm_vcpu *vcpu) { kvm_apic_update_ppr(vcpu); diff --git a/arch/x86/kvm/vmx/x86_ops.h b/arch/x86/kvm/vmx/x86_ops.h index d09abeac2b56..2978b6506ac6 100644 --- a/arch/x86/kvm/vmx/x86_ops.h +++ b/arch/x86/kvm/vmx/x86_ops.h @@ -74,6 +74,7 @@ void vmx_set_idt(struct kvm_vcpu *vcpu, struct desc_ptr *dt); void vmx_get_gdt(struct kvm_vcpu *vcpu, struct desc_ptr *dt); void vmx_set_gdt(struct kvm_vcpu *vcpu, struct desc_ptr *dt); void vmx_set_dr7(struct kvm_vcpu *vcpu, unsigned long val); +void vmx_refresh_pending_dbg_exceptions(struct kvm_vcpu *vcpu); void vmx_sync_dirty_debug_regs(struct kvm_vcpu *vcpu); void vmx_cache_reg(struct kvm_vcpu *vcpu, enum kvm_reg reg); unsigned long vmx_get_rflags(struct kvm_vcpu *vcpu); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 7352c2114bab..9167393cc0cc 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -9232,7 +9232,12 @@ static int kvm_vcpu_check_hw_bp(unsigned long addr, u32 type, u32 dr7, static int kvm_vcpu_do_singlestep(struct kvm_vcpu *vcpu) { - return kvm_inject_emulated_db(vcpu, DR6_BS); + int r; + + r = kvm_inject_emulated_db(vcpu, DR6_BS); + if (r) + kvm_x86_call(refresh_pending_dbg_exceptions)(vcpu); + return r; } int kvm_skip_emulated_instruction(struct kvm_vcpu *vcpu) -- 2.31.1 Similar to the global disable test case in x86's debug_regs test, use 'KVM_FEP' to trigger instruction emulation in order to verify the guest debug DR7.GD checking during instruction emulation. Signed-off-by: Hou Wenlong --- tools/testing/selftests/kvm/x86/debug_regs.c | 23 +++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/kvm/x86/debug_regs.c b/tools/testing/selftests/kvm/x86/debug_regs.c index 2d814c1d1dc4..bd34cf2a96b7 100644 --- a/tools/testing/selftests/kvm/x86/debug_regs.c +++ b/tools/testing/selftests/kvm/x86/debug_regs.c @@ -19,6 +19,7 @@ uint32_t guest_value; extern unsigned char sw_bp, hw_bp, write_data, ss_start, bd_start; +extern unsigned char fep_bd_start; static void guest_code(void) { @@ -64,6 +65,10 @@ static void guest_code(void) /* DR6.BD test */ asm volatile("bd_start: mov %%dr0, %%rax" : : : "rax"); + + if (is_forced_emulation_enabled) + asm volatile(KVM_FEP "fep_bd_start: mov %%dr0, %%rax" : : : "rax"); + GUEST_DONE(); } @@ -185,7 +190,7 @@ int main(void) target_dr6); } - /* Finally test global disable */ + /* test global disable */ memset(&debug, 0, sizeof(debug)); debug.control = KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_HW_BP; debug.arch.debugreg[7] = 0x400 | DR7_GD; @@ -202,6 +207,22 @@ int main(void) run->debug.arch.pc, target_rip, run->debug.arch.dr6, target_dr6); + /* test global disable in emulation */ + if (is_forced_emulation_enabled) { + /* Skip the 3-bytes "mov dr0" */ + vcpu_skip_insn(vcpu, 3); + vcpu_run(vcpu); + TEST_ASSERT(run->exit_reason == KVM_EXIT_DEBUG && + run->debug.arch.exception == DB_VECTOR && + run->debug.arch.pc == CAST_TO_RIP(fep_bd_start) && + run->debug.arch.dr6 == target_dr6, + "DR7.GD: exit %d exception %d rip 0x%llx " + "(should be 0x%llx) dr6 0x%llx (should be 0x%llx)", + run->exit_reason, run->debug.arch.exception, + run->debug.arch.pc, target_rip, run->debug.arch.dr6, + target_dr6); + } + /* Disable all debug controls, run to the end */ memset(&debug, 0, sizeof(debug)); vcpu_guest_debug_set(vcpu, &debug); -- 2.31.1 In the x86's debug_regs test, add a test case to cover the scenario where single-step with STI in VMX sets the 'BS' bit in pending debug exceptions state for #DB interception and instruction emulation in both cases. Signed-off-by: Hou Wenlong --- .../selftests/kvm/include/x86/processor.h | 3 +- tools/testing/selftests/kvm/x86/debug_regs.c | 53 +++++++++++++++++-- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h index 57d62a425109..1992d98016d4 100644 --- a/tools/testing/selftests/kvm/include/x86/processor.h +++ b/tools/testing/selftests/kvm/include/x86/processor.h @@ -36,7 +36,8 @@ extern uint64_t guest_tsc_khz; const char *ex_str(int vector); -#define X86_EFLAGS_FIXED (1u << 1) +#define X86_EFLAGS_FIXED (1u << 1) +#define X86_EFLAGS_TF (1u << 8) #define X86_CR4_VME (1ul << 0) #define X86_CR4_PVI (1ul << 1) diff --git a/tools/testing/selftests/kvm/x86/debug_regs.c b/tools/testing/selftests/kvm/x86/debug_regs.c index bd34cf2a96b7..84d3103a05fc 100644 --- a/tools/testing/selftests/kvm/x86/debug_regs.c +++ b/tools/testing/selftests/kvm/x86/debug_regs.c @@ -15,11 +15,35 @@ #define IRQ_VECTOR 0xAA +#define CAST_TO_RIP(v) ((unsigned long long)&(v)) + /* For testing data access debug BP */ uint32_t guest_value; extern unsigned char sw_bp, hw_bp, write_data, ss_start, bd_start; -extern unsigned char fep_bd_start; +extern unsigned char fep_bd_start, fep_sti_start, fep_sti_end; + +static void guest_db_handler(struct ex_regs *regs) +{ + static int count; + unsigned long target_rips[2] = { + CAST_TO_RIP(fep_sti_start), + CAST_TO_RIP(fep_sti_end), + }; + + __GUEST_ASSERT(regs->rip == target_rips[count], "STI: unexpected rip 0x%lx (should be 0x%lx)", + regs->rip, target_rips[count]); + regs->rflags &= ~X86_EFLAGS_TF; + count++; +} + +static void guest_irq_handler(struct ex_regs *regs) +{ + unsigned long target_rip = CAST_TO_RIP(fep_bd_start); + + __GUEST_ASSERT(regs->rip == target_rip, "IRQ: unexpected rip 0x%lx (should be 0x%lx)", + regs->rip, target_rip); +} static void guest_code(void) { @@ -66,14 +90,27 @@ static void guest_code(void) /* DR6.BD test */ asm volatile("bd_start: mov %%dr0, %%rax" : : : "rax"); - if (is_forced_emulation_enabled) + if (is_forced_emulation_enabled) { asm volatile(KVM_FEP "fep_bd_start: mov %%dr0, %%rax" : : : "rax"); + /* pending debug exceptions for emulation */ + asm volatile("pushf\n\t" + "orq $" __stringify(X86_EFLAGS_TF) ", (%rsp)\n\t" + "popf\n\t" + "sti\n\t" + "fep_sti_start:" + "cli\n\t" + "pushf\n\t" + "orq $" __stringify(X86_EFLAGS_TF) ", (%rsp)\n\t" + "popf\n\t" + KVM_FEP "sti\n\t" + "fep_sti_end:" + "cli\n\t"); + } + GUEST_DONE(); } -#define CAST_TO_RIP(v) ((unsigned long long)&(v)) - static void vcpu_skip_insn(struct kvm_vcpu *vcpu, int insn_len) { struct kvm_regs regs; @@ -227,6 +264,14 @@ int main(void) memset(&debug, 0, sizeof(debug)); vcpu_guest_debug_set(vcpu, &debug); + vm_install_exception_handler(vm, DB_VECTOR, guest_db_handler); + /* + * Install a dummy IRQ handler, as the single-step #DB delivery after + * STI removes the interrupt shadow, so the pending interrupt will be + * delivered after #DB handling. + */ + vm_install_exception_handler(vm, IRQ_VECTOR, guest_irq_handler); + vcpu_run(vcpu); TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); cmd = get_ucall(vcpu, &uc); -- 2.31.1