Re: [PATCH v2 2/9] KVM: x86: Set guest DR6 by kvm_queue_exception_p() in instruction emulation

Re: [PATCH v2 2/9] KVM: x86: Set guest DR6 by kvm_queue_exception_p() in instruction emulation

[Sean Christopherson]

On Thu, Dec 18, 2025, Hou Wenlong wrote:
> 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 <houwenlong.hwl@antgroup.com>
> ---
>  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;

Please split the introduction of the union to a separate patch, mainly so that
the effectively zeroing of ctxt.exception.address in init_emulate_ctxt() is
isolated, e.g. in case it somehow causes problems.  But that will also allow
introducing the inject_emulated_exception() change separately from the
check_dr_read() change.

> +	};
>  	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);

This should be kvm_queue_exception_p().  I also think pivoting on DB_VECTOR is
the wrong approach.  Rather than key off the vector, add payload_valid (to match
error_code_valid), and then do:

	struct x86_exception *ex = &vcpu->arch.emulate_ctxt->exception;

	WARN_ON_ONCE(ex->vector != PF_VECTOR && ex->payload_valid &&
		     ex->error_code_valid);

	if (ex->vector == PF_VECTOR)
		kvm_inject_emulated_page_fault(vcpu, ex);
	else if (ex->payload_valid)
		kvm_queue_exception_p(vcpu, DB_VECTOR, ex->payload);
	else if (ex->error_code_valid)
		kvm_queue_exception_e(vcpu, ex->vector, ex->error_code);
	else
		kvm_queue_exception(vcpu, ex->vector);

PF_VECTOR is special because it has both an error code and a payload, and because
it needs additional handling on multiple fronts.

> +	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
> 

Re: [PATCH v2 2/9] KVM: x86: Set guest DR6 by kvm_queue_exception_p() in instruction emulation

[Sean Christopherson]

On Mon, May 11, 2026, Sean Christopherson wrote:
> On Thu, Dec 18, 2025, Hou Wenlong wrote:
> > 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);
> 
> This should be kvm_queue_exception_p().  I also think pivoting on DB_VECTOR is
> the wrong approach. 

Gah, never mind, didn't look at the next patch.

Re: [PATCH v2 2/9] KVM: x86: Set guest DR6 by kvm_queue_exception_p() in instruction emulation

[Sean Christopherson]

On Mon, May 11, 2026, Sean Christopherson wrote:
> On Mon, May 11, 2026, Sean Christopherson wrote:
> > On Thu, Dec 18, 2025, Hou Wenlong wrote:
> > > 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);
> > 
> > This should be kvm_queue_exception_p().  I also think pivoting on DB_VECTOR is
> > the wrong approach. 
> 
> Gah, never mind, didn't look at the next patch.

Actually, that's a good excuse to provide kvm_inject_emulated_db() in this patch,
even though it doesn't become truly necessary until the next patch.  Eliminating
some of the code movement in the next patch will yield a smaller diff, and make
it easier to see that there's change in the !KVM_GUESTDBG_USE_HW_BP case.

@@ -8976,23 +8998,36 @@ static void toggle_interruptibility(struct kvm_vcpu *vcpu, u32 mask)
        }
 }
 
-static void kvm_inject_emulated_db(struct kvm_vcpu *vcpu, unsigned long dr6)
+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 void inject_emulated_exception(struct kvm_vcpu *vcpu)
+static int inject_emulated_exception(struct kvm_vcpu *vcpu)
 {
        struct x86_exception *ex = &vcpu->arch.emulate_ctxt->exception;
 
        if (ex->vector == DB_VECTOR)
-               kvm_inject_emulated_db(vcpu, 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)

Re: [PATCH v2 0/9] KVM: x86: Improve the handling of debug exceptions during instruction emulation

[Sean Christopherson]

On Thu, Dec 18, 2025, Hou Wenlong wrote:
> Hou Wenlong (9):
>   KVM: x86: Capture "struct x86_exception" in
>     inject_emulated_exception()
>   KVM: x86: Set guest DR6 by kvm_queue_exception_p() in instruction
>     emulation
>   KVM: x86: Check guest debug in DR access instruction emulation
>   KVM: x86: Only check effective code breakpoint in emulation
>   KVM: x86: Consolidate KVM_GUESTDBG_SINGLESTEP check into the
>     kvm_inject_emulated_db()
>   KVM: x86: Move kvm_set_rflags() up before kvm_vcpu_do_singlestep()
>   KVM: VMX: Refresh 'PENDING_DBG_EXCEPTIONS.BS' bit during instruction
>     emulation
>   KVM: selftests: Verify guest debug DR7.GD checking during instruction
>     emulation
>   KVM: selftests: Verify 'BS' bit checking in pending debug exception
>     during VM entry
> 
>  arch/x86/include/asm/kvm-x86-ops.h            |   1 +
>  arch/x86/include/asm/kvm_host.h               |   1 +
>  arch/x86/kvm/emulate.c                        |  14 +--
>  arch/x86/kvm/kvm_emulate.h                    |   7 +-
>  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                            | 116 ++++++++++--------
>  arch/x86/kvm/x86.h                            |   7 ++
>  .../selftests/kvm/include/x86/processor.h     |   3 +-
>  tools/testing/selftests/kvm/x86/debug_regs.c  |  72 ++++++++++-
>  11 files changed, 178 insertions(+), 68 deletions(-)

One goof and some nits on patch 3, but I'll fix them up when applying, i.e. no
need for a v3.  I'll probably also add some comments and elaborate on some of
the changelogs.

Re: [PATCH v2 9/9] KVM: selftests: Verify 'BS' bit checking in pending debug exception state during VM-Entry

[Sean Christopherson]

On Thu, Dec 18, 2025, Hou Wenlong wrote:
> 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.

...

> +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);

This is wrong, and the test fails with your series applied verbatim.  The IRQ
will arrive at fep_sti_start, because RFLAGS.IF=0 from the CLI at the end of the
ss_start block, and remains that way across fep_bd_start.   And to make things
even more confusing, the IRQ arrives on the CLI even though it's in an STI shadow
due to #DBs not being subjected to _STI_ shadows on Intel, only to MOV-SS/POP-SS
shadows.  I.e. the #DB lands on CLI, pushes RFLAGS.IF=1 on the stack, and then
the subsequent IRET from guest_db_handler() fully unmasks IRQs, and voila.

Re: [PATCH v2 9/9] KVM: selftests: Verify 'BS' bit checking in pending debug exception state during VM-Entry

[Hou Wenlong]

On Wed, May 13, 2026 at 04:14:14PM -0700, Sean Christopherson wrote:
> On Thu, Dec 18, 2025, Hou Wenlong wrote:
> > 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.
> 
> ...
> 
> > +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);
> 
> This is wrong, and the test fails with your series applied verbatim.  The IRQ
> will arrive at fep_sti_start, because RFLAGS.IF=0 from the CLI at the end of the
> ss_start block, and remains that way across fep_bd_start.   And to make things
> even more confusing, the IRQ arrives on the CLI even though it's in an STI shadow
> due to #DBs not being subjected to _STI_ shadows on Intel, only to MOV-SS/POP-SS
> shadows.  I.e. the #DB lands on CLI, pushes RFLAGS.IF=1 on the stack, and then
> the subsequent IRET from guest_db_handler() fully unmasks IRQs, and voila.

Yes, the RIP here should be 'fep_sti_start'. This was a stupid mistake,
and I’m very sorry about that. In my reply to v1[0] I explained why an IRQ
handler is needed, so in v2 I added comments and an additional check,
and I tested it before sending. However, it looks like the patch differs
from what I had locally—my code management was a bit messy.

As you pointed out, this pending interrupt can make the test somewhat
confusing. So it might be better to consume the IRQ before running the
test, e.g.:

```
diff --git a/tools/testing/selftests/kvm/x86/debug_regs.c
b/tools/testing/selftests/kvm/x86/debug_regs.c
index 3ec2724697de..030d5cb58cc2 100644
--- a/tools/testing/selftests/kvm/x86/debug_regs.c
+++ b/tools/testing/selftests/kvm/x86/debug_regs.c
@@ -21,7 +21,7 @@
 u32 guest_value;

 extern unsigned char sw_bp, hw_bp, write_data, ss_start, bd_start;
-extern unsigned char fep_bd_start, fep_sti_start, fep_sti_end;
+extern unsigned char fep_irq, fep_bd_start, fep_sti_start, fep_sti_end;

 static void guest_db_handler(struct ex_regs *regs)
 {
@@ -39,7 +39,7 @@ static void guest_db_handler(struct ex_regs *regs)

 static void guest_irq_handler(struct ex_regs *regs)
 {
-       unsigned long target_rip = CAST_TO_RIP(fep_bd_start);
+       unsigned long target_rip = CAST_TO_RIP(fep_irq);

        __GUEST_ASSERT(regs->rip == target_rip, "IRQ: unexpected rip
0x%lx (should be 0x%lx)",
                       regs->rip, target_rip);
@@ -93,6 +93,14 @@ static void guest_code(void)
        if (is_forced_emulation_enabled) {
                asm volatile(KVM_FEP "fep_bd_start: mov %%dr0, %%rax" :
: : "rax");

+               /*
+                * Consume the pending interrupt, as the single-step #DB
delivery after
+                * STI removes the interrupt shadow, so the pending
interrupt will be
+                * delivered after guest #DB handling.
+                */
+               asm volatile("sti; nop\n\t"
+                            "fep_irq:");
+
                /* pending debug exceptions for emulation */
                asm volatile("pushf\n\t"
                             "orq $" __stringify(X86_EFLAGS_TF) ",
(%rsp)\n\t"
@@ -264,13 +272,10 @@ 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);
+       if (is_forced_emulation_enabled) {
+               vm_install_exception_handler(vm, DB_VECTOR,
guest_db_handler);
+               vm_install_exception_handler(vm, IRQ_VECTOR,
guest_irq_handler);
+       }

        vcpu_run(vcpu);
        TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
```

I have tested it on both Intel and AMD. Please let me know if I should
resend a v3 of this patch.

[0]: https://lore.kernel.org/kvm/20251218134026.GA118671@k08j02272.eu95sqa/

Thanks!