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/[email protected]/
Thanks!
