Kernel KVM virtualization development
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	 Hou Wenlong <houwenlong.hwl@antgroup.com>,
	Lai Jiangshan <jiangshan.ljs@antgroup.com>
Subject: Re: [PATCH v3 10/10] KVM: selftests: Verify VMX's GUEST_PENDING_DBG_EXCEPTIONS.BS Consistency Check
Date: Wed, 20 May 2026 09:19:19 -0700	[thread overview]
Message-ID: <ag3fB0wB8-DHjUMC@google.com> (raw)
In-Reply-To: <20260515222638.1949982-11-seanjc@google.com>

On Fri, May 15, 2026, Sean Christopherson wrote:
> From: Hou Wenlong <houwenlong.hwl@antgroup.com>
> 
> In x86's debug_regs test, add a test case to cover the scenario where a
> single-step #DB occurs in an STI-shadow, in which case KVM needs to stuff
> vmcs.GUEST_PENDING_DBG_EXCEPTIONS.BS in order to satisfy a flawed VM-Entry
> Consistency Check.
> 
> Wire up an IRQ handler to gain a bit of bonus coverage, as the subsequent
> IRET from the #DB sets RFLAGS.IF, but *without* STI-blocking, and so the
> pending IRQ is expected on the instruction immediately following STI.
> 
> Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
> [sean: expect the IRQ on the CLI, and explain why]
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  tools/testing/selftests/kvm/x86/debug_regs.c | 64 ++++++++++++++++++--
>  1 file changed, 60 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/x86/debug_regs.c b/tools/testing/selftests/kvm/x86/debug_regs.c
> index ee9d0f3a5807..6299e921dc27 100644
> --- a/tools/testing/selftests/kvm/x86/debug_regs.c
> +++ b/tools/testing/selftests/kvm/x86/debug_regs.c
> @@ -15,11 +15,46 @@
>  
>  #define IRQ_VECTOR 0xAA
>  
> +#define  CAST_TO_RIP(v)  ((unsigned long long)&(v))
> +
>  /* For testing data access debug BP */
>  u32 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[%u]: unexpected rip 0x%lx (should be 0x%lx)",
> +		       count, regs->rip, target_rips[count]);
> +	regs->rflags &= ~X86_EFLAGS_TF;
> +	count++;
> +}
> +
> +static void guest_irq_handler(struct ex_regs *regs)
> +{
> +	/*
> +	 * The pending IRQ should finally be take when KVM_GUESTDBG_BLOCKIRQ is
> +	 * cleared and IRQs are enabled.  Note, the IRQ is expected to arrive
> +	 * on the instruction immediately after STI, even though its in an STI
> +	 * shadow.  Because the next instruction has a coincident #DB, and #DBs
> +	 * are not subject to STI-blocking, the #DB will push RFLAGS.IF=1 on
> +	 * the stack, and the eventual IRET will unmask IRQs and obliterate the
> +	 * STI shadow in the process.
> +	 */
> +	unsigned long target_rip = CAST_TO_RIP(fep_sti_start);
> +
> +	__GUEST_ASSERT(regs->rip == target_rip,
> +		       "IRQ: unexpected rip 0x%lx (should be 0x%lx)",
> +		       regs->rip, target_rip);

From Sashiko:

 : Is an End of Interrupt (EOI) acknowledgment required here?
 : 
 : The handler is triggered by an APIC interrupt injected earlier in the test,
 : but does not write to the APIC_EOI register to acknowledge it.
 : 
 : While it might not fail this specific test since it ends immediately after,
 : does failing to send an EOI leave the interrupt permanently marked as
 : in-service in the local APIC state machine?

Yes, this will leave the IRQ dangling in the ISR.  It doesn't really matter, but
I'll throw in a:

	x2apic_write_reg(APIC_EOI, 0);

when applying.

> +}
>  
>  static void guest_code(void)
>  {
> @@ -66,14 +101,32 @@ static void guest_code(void)
>  	/* DR6.BD test */
>  	asm volatile("bd_start: mov %%dr0, %%rax" : : : "rax");
>  
> -	if (is_forced_emulation_enabled)
> +	/*
> +	 * Note, the IRET from the #DB that occurs in the below STI-shadow will
> +	 * unmask IRQs, i.e. the pending interrupt will be delivered after #DB
> +	 * handling, on the CLI!
> +	 */
> +	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");
> +	}
> +

Also from Sashiko:

 : Does the test verify that the expected #DB and IRQ events actually occurred?
 : 
 : The assertions are placed inside guest_db_handler() and guest_irq_handler(),
 : but if a bug causes the exceptions to be dropped completely, the guest
 : will simply proceed to execute GUEST_DONE() without error.
 : 
 : Should there be a shared state variable to assert that the handlers fired
 : the expected number of times before concluding the test?

Another "yes".  I'll add an assert that the IRQ actually fired.


      reply	other threads:[~2026-05-20 16:19 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-15 22:26 [PATCH v3 00/10] KVM: x86: Improve #DB handling in the emulator Sean Christopherson
2026-05-15 22:26 ` [PATCH v3 01/10] KVM: VMX: Refresh GUEST_PENDING_DBG_EXCEPTIONS.BS on all injected #DBs Sean Christopherson
2026-05-18  8:17   ` Hou Wenlong
2026-05-20 16:11   ` Sean Christopherson
2026-05-15 22:26 ` [PATCH v3 02/10] KVM: x86: Capture "struct x86_exception" in inject_emulated_exception() Sean Christopherson
2026-05-18 18:01   ` Yosry Ahmed
2026-05-15 22:26 ` [PATCH v3 03/10] KVM: x86: Set guest DR6 by kvm_queue_exception_p() in instruction emulation Sean Christopherson
2026-05-18 18:13   ` Yosry Ahmed
2026-05-15 22:26 ` [PATCH v3 04/10] KVM: x86: Honor KVM_GUESTDBG_USE_HW_BP when emulating MOV DR (in emulator) Sean Christopherson
2026-05-18 18:17   ` Yosry Ahmed
2026-05-15 22:26 ` [PATCH v3 05/10] KVM: x86: Honor KVM_GUESTDBG_USE_HW_BP when checking for code breakpoints in emulation Sean Christopherson
2026-05-15 22:26 ` [PATCH v3 06/10] KVM: x86: Move KVM_GUESTDBG_SINGLESTEP handling into kvm_inject_emulated_db() Sean Christopherson
2026-05-18 18:22   ` Yosry Ahmed
2026-05-15 22:26 ` [PATCH v3 07/10] KVM: x86: Drop kvm_vcpu_do_singlestep() now that it's been gutted Sean Christopherson
2026-05-18 18:22   ` Yosry Ahmed
2026-05-15 22:26 ` [PATCH v3 08/10] KVM: selftests: Add all (known) EFLAGS bit definitions Sean Christopherson
2026-05-15 22:26 ` [PATCH v3 09/10] KVM: selftests: Verify guest debug DR7.GD checking during instruction emulation Sean Christopherson
2026-05-20 16:13   ` Sean Christopherson
2026-05-15 22:26 ` [PATCH v3 10/10] KVM: selftests: Verify VMX's GUEST_PENDING_DBG_EXCEPTIONS.BS Consistency Check Sean Christopherson
2026-05-20 16:19   ` Sean Christopherson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ag3fB0wB8-DHjUMC@google.com \
    --to=seanjc@google.com \
    --cc=houwenlong.hwl@antgroup.com \
    --cc=jiangshan.ljs@antgroup.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox