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 <jiangshan.ljs@antgroup.com>
Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
---
arch/x86/kvm/x86.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 5af652916a19..83960214d5d8 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8632,7 +8632,10 @@ 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) {
+ /* Data breakpoints are not supported in emulation for now. */
+ WARN_ON((dr6 & DR6_BS) && (dr6 & DR_TRAP_BITS));
+
+ 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;
@@ -8907,17 +8910,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.1On Wed, Sep 10, 2025, Hou Wenlong wrote:
> 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 <jiangshan.ljs@antgroup.com>
> Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
> ---
> arch/x86/kvm/x86.c | 17 +++++------------
> 1 file changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 5af652916a19..83960214d5d8 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8632,7 +8632,10 @@ 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) {
> + /* Data breakpoints are not supported in emulation for now. */
> + WARN_ON((dr6 & DR6_BS) && (dr6 & DR_TRAP_BITS));
If we keep this, it should be a WARN_ON_ONCE(). We've had at least one case where
a sanity check in the emulator caused major problems because a WARN_ON() spammed
the kernel log to the point where it overloaded things :-)
But I think the WARN will be subject to false positives. KVM doesn't emulate data
#DBs, but it does emulate code #DBs, and fault-like code #DBs can be coincident
with trap-like single-step #DBs. Ah, but kvm_vcpu_check_code_breakpoint() doesn't
account for RFLAGS.TF. That should probably be addressed in this series, especially
since it's consolidating KVM_GUESTDBG_SINGLESTEP handling.
On Fri, Dec 05, 2025 at 09:58:04AM -0800, Sean Christopherson wrote:
> On Wed, Sep 10, 2025, Hou Wenlong wrote:
> > 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 <jiangshan.ljs@antgroup.com>
> > Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
> > ---
> > arch/x86/kvm/x86.c | 17 +++++------------
> > 1 file changed, 5 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 5af652916a19..83960214d5d8 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -8632,7 +8632,10 @@ 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) {
> > + /* Data breakpoints are not supported in emulation for now. */
> > + WARN_ON((dr6 & DR6_BS) && (dr6 & DR_TRAP_BITS));
>
> If we keep this, it should be a WARN_ON_ONCE(). We've had at least one case where
> a sanity check in the emulator caused major problems because a WARN_ON() spammed
> the kernel log to the point where it overloaded things :-)
>
I'll drop it.
> But I think the WARN will be subject to false positives. KVM doesn't emulate data
> #DBs, but it does emulate code #DBs, and fault-like code #DBs can be coincident
> with trap-like single-step #DBs. Ah, but kvm_vcpu_check_code_breakpoint() doesn't
> account for RFLAGS.TF. That should probably be addressed in this series, especially
> since it's consolidating KVM_GUESTDBG_SINGLESTEP handling.
Sorry, I didn't follow it, how fault-like code #DBs can be coincident
with trap-like single-step #DBs, could you provide an example?
Thanks!
On Thu, Dec 11, 2025, Hou Wenlong wrote:
> On Fri, Dec 05, 2025 at 09:58:04AM -0800, Sean Christopherson wrote:
> > But I think the WARN will be subject to false positives. KVM doesn't emulate data
> > #DBs, but it does emulate code #DBs, and fault-like code #DBs can be coincident
> > with trap-like single-step #DBs. Ah, but kvm_vcpu_check_code_breakpoint() doesn't
> > account for RFLAGS.TF. That should probably be addressed in this series, especially
> > since it's consolidating KVM_GUESTDBG_SINGLESTEP handling.
>
> Sorry, I didn't follow it, how fault-like code #DBs can be coincident
> with trap-like single-step #DBs, could you provide an example?
Ya, here's a KUT testcase that applies on top of
https://lore.kernel.org/all/20251126191736.907963-1-seanjc@google.com.
---
x86/debug.c | 43 +++++++++++++++++++++++++++++++++++++++----
1 file changed, 39 insertions(+), 4 deletions(-)
diff --git a/x86/debug.c b/x86/debug.c
index 8177575c..313d854e 100644
--- a/x86/debug.c
+++ b/x86/debug.c
@@ -92,6 +92,7 @@ typedef unsigned long (*db_test_fn)(void);
typedef void (*db_report_fn)(unsigned long, const char *);
static unsigned long singlestep_with_movss_blocking_and_dr7_gd(void);
+static unsigned long singlestep_with_code_db(void);
static unsigned long singlestep_with_sti_hlt(void);
static void __run_single_step_db_test(db_test_fn test, db_report_fn report_fn)
@@ -106,11 +107,12 @@ static void __run_single_step_db_test(db_test_fn test, db_report_fn report_fn)
report_fn(start, "");
/*
- * MOV DR #GPs at CPL>0, don't try to run the DR7.GD test in usermode.
- * Likewise for HLT.
+ * MOV DR #GPs at CPL>0, don't try to run the DR7.GD or code #DB tests
+ * in usermode. Likewise for HLT.
*/
- if (test == singlestep_with_movss_blocking_and_dr7_gd
- || test == singlestep_with_sti_hlt)
+ if (test == singlestep_with_movss_blocking_and_dr7_gd ||
+ test == singlestep_with_code_db ||
+ test == singlestep_with_sti_hlt)
return;
n = 0;
@@ -163,6 +165,38 @@ static noinline unsigned long singlestep_basic(void)
return start;
}
+static void report_singlestep_with_code_db(unsigned long start, const char *usermode)
+{
+ report(n == 3 &&
+ dr6[0] == (DR6_ACTIVE_LOW | DR6_BS | DR6_TRAP2) && db_addr[0] == start &&
+ is_single_step_db(dr6[1]) && db_addr[1] == start + 1 &&
+ is_single_step_db(dr6[2]) && db_addr[2] == start + 1 + 1,
+ "%sSingle-step + code #DB test", usermode);
+}
+
+static noinline unsigned long singlestep_with_code_db(void)
+{
+ unsigned long start;
+
+ asm volatile (
+ "lea 1f(%%rip), %0\n\t"
+ "mov %0, %%dr2\n\t"
+ "mov $" xstr(DR7_FIXED_1 | DR7_EXECUTE_DRx(2) | DR7_GLOBAL_ENABLE_DR2) ", %0\n\t"
+ "mov %0, %%dr7\n\t"
+ "pushf\n\t"
+ "pop %%rax\n\t"
+ "or $(1<<8),%%rax\n\t"
+ "push %%rax\n\t"
+ "popf\n\t"
+ "and $~(1<<8),%%rax\n\t"
+ "1:push %%rax\n\t"
+ "popf\n\t"
+ "lea 1b(%%rip), %0\n\t"
+ : "=r" (start) : : "rax"
+ );
+ return start;
+}
+
static void report_singlestep_emulated_instructions(unsigned long start,
const char *usermode)
{
@@ -517,6 +551,7 @@ int main(int ac, char **av)
n, db_addr[0], dr6[0]);
run_ss_db_test(singlestep_basic);
+ run_ss_db_test(singlestep_with_code_db);
run_ss_db_test(singlestep_emulated_instructions);
run_ss_db_test(singlestep_with_sti_blocking);
run_ss_db_test(singlestep_with_movss_blocking);
base-commit: 23071a886edbe303fb964c5c386750b0b458dbfb
--
On Thu, Dec 11, 2025 at 09:19:39AM -0800, Sean Christopherson wrote:
> On Thu, Dec 11, 2025, Hou Wenlong wrote:
> > On Fri, Dec 05, 2025 at 09:58:04AM -0800, Sean Christopherson wrote:
> > > But I think the WARN will be subject to false positives. KVM doesn't emulate data
> > > #DBs, but it does emulate code #DBs, and fault-like code #DBs can be coincident
> > > with trap-like single-step #DBs. Ah, but kvm_vcpu_check_code_breakpoint() doesn't
> > > account for RFLAGS.TF. That should probably be addressed in this series, especially
> > > since it's consolidating KVM_GUESTDBG_SINGLESTEP handling.
> >
> > Sorry, I didn't follow it, how fault-like code #DBs can be coincident
> > with trap-like single-step #DBs, could you provide an example?
>
> Ya, here's a KUT testcase that applies on top of
> https://lore.kernel.org/all/20251126191736.907963-1-seanjc@google.com.
>
Thanks for your testcase; it really changed my perspective.
> ---
> x86/debug.c | 43 +++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/x86/debug.c b/x86/debug.c
> index 8177575c..313d854e 100644
> --- a/x86/debug.c
> +++ b/x86/debug.c
> @@ -92,6 +92,7 @@ typedef unsigned long (*db_test_fn)(void);
> typedef void (*db_report_fn)(unsigned long, const char *);
>
> static unsigned long singlestep_with_movss_blocking_and_dr7_gd(void);
> +static unsigned long singlestep_with_code_db(void);
> static unsigned long singlestep_with_sti_hlt(void);
>
> static void __run_single_step_db_test(db_test_fn test, db_report_fn report_fn)
> @@ -106,11 +107,12 @@ static void __run_single_step_db_test(db_test_fn test, db_report_fn report_fn)
> report_fn(start, "");
>
> /*
> - * MOV DR #GPs at CPL>0, don't try to run the DR7.GD test in usermode.
> - * Likewise for HLT.
> + * MOV DR #GPs at CPL>0, don't try to run the DR7.GD or code #DB tests
> + * in usermode. Likewise for HLT.
> */
> - if (test == singlestep_with_movss_blocking_and_dr7_gd
> - || test == singlestep_with_sti_hlt)
> + if (test == singlestep_with_movss_blocking_and_dr7_gd ||
> + test == singlestep_with_code_db ||
> + test == singlestep_with_sti_hlt)
> return;
>
> n = 0;
> @@ -163,6 +165,38 @@ static noinline unsigned long singlestep_basic(void)
> return start;
> }
>
> +static void report_singlestep_with_code_db(unsigned long start, const char *usermode)
> +{
> + report(n == 3 &&
> + dr6[0] == (DR6_ACTIVE_LOW | DR6_BS | DR6_TRAP2) && db_addr[0] == start &&
> + is_single_step_db(dr6[1]) && db_addr[1] == start + 1 &&
> + is_single_step_db(dr6[2]) && db_addr[2] == start + 1 + 1,
> + "%sSingle-step + code #DB test", usermode);
> +}
> +
> +static noinline unsigned long singlestep_with_code_db(void)
> +{
> + unsigned long start;
> +
> + asm volatile (
> + "lea 1f(%%rip), %0\n\t"
> + "mov %0, %%dr2\n\t"
> + "mov $" xstr(DR7_FIXED_1 | DR7_EXECUTE_DRx(2) | DR7_GLOBAL_ENABLE_DR2) ", %0\n\t"
> + "mov %0, %%dr7\n\t"
> + "pushf\n\t"
> + "pop %%rax\n\t"
> + "or $(1<<8),%%rax\n\t"
> + "push %%rax\n\t"
> + "popf\n\t"
> + "and $~(1<<8),%%rax\n\t"
In my previous understanding, I thought there would be two #DBs
generated at the instruction boundary. First, the single-step trap #DB
would be handled, and then, when resuming to start the new instruction,
it would check for the code breakpoint and generate a code fault #DB.
However, it turns out that the check for the code breakpoint happened
before the instruction boundary. I also see in the kernel hardware
breakpoint handler that it notes that code breakpoints and single-step
can be detected together. Is this due to instruction prefetch?
If we want to emulate the hardware behavior in the emulator, does that
mean we need to check for code breakpoints in kvm_vcpu_do_single_step()
and set the DR_TRAP_BITS along with the DR6_BS bit?
Thanks!
> + "1:push %%rax\n\t"
> + "popf\n\t"
> + "lea 1b(%%rip), %0\n\t"
> + : "=r" (start) : : "rax"
> + );
> + return start;
> +}
> +
> static void report_singlestep_emulated_instructions(unsigned long start,
> const char *usermode)
> {
> @@ -517,6 +551,7 @@ int main(int ac, char **av)
> n, db_addr[0], dr6[0]);
>
> run_ss_db_test(singlestep_basic);
> + run_ss_db_test(singlestep_with_code_db);
> run_ss_db_test(singlestep_emulated_instructions);
> run_ss_db_test(singlestep_with_sti_blocking);
> run_ss_db_test(singlestep_with_movss_blocking);
>
> base-commit: 23071a886edbe303fb964c5c386750b0b458dbfb
> --
On Fri, Dec 12, 2025, Hou Wenlong wrote:
> On Thu, Dec 11, 2025 at 09:19:39AM -0800, Sean Christopherson wrote:
> > On Thu, Dec 11, 2025, Hou Wenlong wrote:
> > +static noinline unsigned long singlestep_with_code_db(void)
> > +{
> > + unsigned long start;
> > +
> > + asm volatile (
> > + "lea 1f(%%rip), %0\n\t"
> > + "mov %0, %%dr2\n\t"
> > + "mov $" xstr(DR7_FIXED_1 | DR7_EXECUTE_DRx(2) | DR7_GLOBAL_ENABLE_DR2) ", %0\n\t"
> > + "mov %0, %%dr7\n\t"
> > + "pushf\n\t"
> > + "pop %%rax\n\t"
> > + "or $(1<<8),%%rax\n\t"
> > + "push %%rax\n\t"
> > + "popf\n\t"
> > + "and $~(1<<8),%%rax\n\t"
> In my previous understanding, I thought there would be two #DBs
> generated at the instruction boundary. First, the single-step trap #DB
> would be handled, and then, when resuming to start the new instruction,
> it would check for the code breakpoint and generate a code fault #DB.
> However, it turns out that the check for the code breakpoint happened
> before the instruction boundary.
Yeah, that's what I was trying to explain by describing code breakpoint as fault-like.
> I also see in the kernel hardware breakpoint handler that it notes that code
> breakpoints and single-step can be detected together. Is this due to
> instruction prefetch?
Nope, it's just how #DBs work, everything pending gets smushed together. Note,
data #DBs can also be coincident. E.g. it's entirely possible that you could
observe a code breakpoint, a data breakpoint, and a single-step breakpoint in a
single #DB.
> If we want to emulate the hardware behavior in the emulator, does that
> mean we need to check for code breakpoints in kvm_vcpu_do_single_step()
> and set the DR_TRAP_BITS along with the DR6_BS bit?
Hmm, ya, I think so? I don't think the CPU will fetch and merge the imminent
code #DB with the injected single-step #DB.
On Fri, Dec 12, 2025 at 09:53:20AM -0800, Sean Christopherson wrote:
> On Fri, Dec 12, 2025, Hou Wenlong wrote:
> > On Thu, Dec 11, 2025 at 09:19:39AM -0800, Sean Christopherson wrote:
> > > On Thu, Dec 11, 2025, Hou Wenlong wrote:
> > > +static noinline unsigned long singlestep_with_code_db(void)
> > > +{
> > > + unsigned long start;
> > > +
> > > + asm volatile (
> > > + "lea 1f(%%rip), %0\n\t"
> > > + "mov %0, %%dr2\n\t"
> > > + "mov $" xstr(DR7_FIXED_1 | DR7_EXECUTE_DRx(2) | DR7_GLOBAL_ENABLE_DR2) ", %0\n\t"
> > > + "mov %0, %%dr7\n\t"
> > > + "pushf\n\t"
> > > + "pop %%rax\n\t"
> > > + "or $(1<<8),%%rax\n\t"
> > > + "push %%rax\n\t"
> > > + "popf\n\t"
> > > + "and $~(1<<8),%%rax\n\t"
> > In my previous understanding, I thought there would be two #DBs
> > generated at the instruction boundary. First, the single-step trap #DB
> > would be handled, and then, when resuming to start the new instruction,
> > it would check for the code breakpoint and generate a code fault #DB.
> > However, it turns out that the check for the code breakpoint happened
> > before the instruction boundary.
>
> Yeah, that's what I was trying to explain by describing code breakpoint as fault-like.
>
> > I also see in the kernel hardware breakpoint handler that it notes that code
> > breakpoints and single-step can be detected together. Is this due to
> > instruction prefetch?
>
> Nope, it's just how #DBs work, everything pending gets smushed together. Note,
> data #DBs can also be coincident. E.g. it's entirely possible that you could
> observe a code breakpoint, a data breakpoint, and a single-step breakpoint in a
> single #DB.
>
> > If we want to emulate the hardware behavior in the emulator, does that
> > mean we need to check for code breakpoints in kvm_vcpu_do_single_step()
> > and set the DR_TRAP_BITS along with the DR6_BS bit?
>
> Hmm, ya, I think so? I don't think the CPU will fetch and merge the imminent
> code #DB with the injected single-step #DB.
Um, I have one more question: what do you mean when you say that
kvm_vcpu_check_code_breakpoint() doesn't account for RFLAGS.TF?
Thanks!
On Sun, Dec 14, 2025, Hou Wenlong wrote:
> On Fri, Dec 12, 2025 at 09:53:20AM -0800, Sean Christopherson wrote:
> > On Fri, Dec 12, 2025, Hou Wenlong wrote:
> > > On Thu, Dec 11, 2025 at 09:19:39AM -0800, Sean Christopherson wrote:
> > > > On Thu, Dec 11, 2025, Hou Wenlong wrote:
> > > > +static noinline unsigned long singlestep_with_code_db(void)
> > > > +{
> > > > + unsigned long start;
> > > > +
> > > > + asm volatile (
> > > > + "lea 1f(%%rip), %0\n\t"
> > > > + "mov %0, %%dr2\n\t"
> > > > + "mov $" xstr(DR7_FIXED_1 | DR7_EXECUTE_DRx(2) | DR7_GLOBAL_ENABLE_DR2) ", %0\n\t"
> > > > + "mov %0, %%dr7\n\t"
> > > > + "pushf\n\t"
> > > > + "pop %%rax\n\t"
> > > > + "or $(1<<8),%%rax\n\t"
> > > > + "push %%rax\n\t"
> > > > + "popf\n\t"
> > > > + "and $~(1<<8),%%rax\n\t"
> > > In my previous understanding, I thought there would be two #DBs
> > > generated at the instruction boundary. First, the single-step trap #DB
> > > would be handled, and then, when resuming to start the new instruction,
> > > it would check for the code breakpoint and generate a code fault #DB.
> > > However, it turns out that the check for the code breakpoint happened
> > > before the instruction boundary.
> >
> > Yeah, that's what I was trying to explain by describing code breakpoint as fault-like.
> >
> > > I also see in the kernel hardware breakpoint handler that it notes that code
> > > breakpoints and single-step can be detected together. Is this due to
> > > instruction prefetch?
> >
> > Nope, it's just how #DBs work, everything pending gets smushed together. Note,
> > data #DBs can also be coincident. E.g. it's entirely possible that you could
> > observe a code breakpoint, a data breakpoint, and a single-step breakpoint in a
> > single #DB.
> >
> > > If we want to emulate the hardware behavior in the emulator, does that
> > > mean we need to check for code breakpoints in kvm_vcpu_do_single_step()
> > > and set the DR_TRAP_BITS along with the DR6_BS bit?
> >
> > Hmm, ya, I think so? I don't think the CPU will fetch and merge the imminent
> > code #DB with the injected single-step #DB.
> Um, I have one more question: what do you mean when you say that
> kvm_vcpu_check_code_breakpoint() doesn't account for RFLAGS.TF?
I was pointing out that if KVM is emulating multiple instructions without
entering the guest, then I'm pretty sure kvm_vcpu_check_code_breakpoint() would
fail to detect a coincident single-step #DB. kvm_vcpu_check_code_breakpoint()
may not be the right place to try and handle that though.
© 2016 - 2026 Red Hat, Inc.