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.
prev parent 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