[PATCH v2 0/6] KVM: x86: Fix emulated MOV DR{4,5} #GP bugs

[PATCH v2 0/6] KVM: x86: Fix emulated MOV DR{4,5} #GP bugs

[Sean Christopherson]

Carlos' fix for a bug where KVM fails to signal #GP on emulated MOV DR to
DR4 or DR5, plus follow-up fixes priority goofs (#GP vs #DB, and #GP versus
#VMEXIT on SVM).  The rest are opportunistic cleanups.

v1: https://lore.kernel.org/all/20260601133320.91479-2-clopez@suse.de

Carlos López (1):
  KVM: x86: Treat any non-zero return from set_dr() as a faulting
    condition

Sean Christopherson (5):
  KVM: x86: Prioritize DR7.GD #DB over #GP due to illegal DR6/7 value
  KVM: x86: Manually check DR4/5 write values to fix SVM intercept
    priority
  KVM: x86: Use kvm_dr{6,7}_valid() to check DR{4,5,6,7} write values in
    emulator
  KVM: x86: WARN if MOV DR emulation hits a "too late" #GP
  KVM: x86: Read CR4.DE in emulator if and only if accessing DR4 or DR5

 arch/x86/kvm/emulate.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)


base-commit: d4bfaa66fa171089b9b9fb2dc17af9245f2b9b34
-- 
2.54.0.1032.g2f8565e1d1-goog

[PATCH v2 1/6] KVM: x86: Treat any non-zero return from set_dr() as a faulting condition

[Sean Christopherson]

From: Carlos López <clopez@suse.de>

When emulating a MOV to a debug register, em_dr_write() calls
@ctxt->ops->set_dr(), which is forwarded to emulator_set_dr() and
then kvm_set_dr(). The latter checks that the written value is valid,
otherwise returning an error, in which case the emulator is supposed to
inject a #GP fault into the guest.

Commit 996ff5429e98 ("KVM: x86: move kvm_inject_gp up from kvm_set_dr
to callers") changed the contract of kvm_set_dr() (and thus
emulator_set_dr()), returning 1 as an error instead of -1, but the
caller in em_dr_write() was never updated, checking only if the returned
value is negative. The end result is that em_dr_write() does not detect
the error, so an invalid write does not generate a #GP, but at the same
time the register value is not updated.

The practical impact is limited, as check_dr_write() already checks DR6
and DR7 manually. However, it misses DR4/DR5, which alias DR6/DR7 when
CR4.DE=0.

Fix the bug by treating any non-zero return from set_dr() as a reason to
inject #GP.

Note, the manual checks on DR6 and DR7 are flawed, as they incorrectly
prioritize the #GP over a DR7.GD=1 #DB (the General Detect #DB has
priority on both Intel and AMD).

Note #2, relying on ->set_dr() to detect #GP is also flawed as all
exceptions have higher priority than the instruction intercept on SVM,
i.e. the manual checks need to be extended to DR4 and DR5 (after the
priority bug is fixed).

Fixes: 996ff5429e98 ("KVM: x86: move kvm_inject_gp up from kvm_set_dr to callers")
Signed-off-by: Carlos López <clopez@suse.de>
Link: https://patch.msgid.link/20260601133320.91479-2-clopez@suse.de
[sean: drop explicit "!= 0", massage changelog]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/emulate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index b566ab5c7515..75cd8b6136aa 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3299,7 +3299,7 @@ static int em_dr_write(struct x86_emulate_ctxt *ctxt)
 		val = ctxt->src.val & ~0U;
 
 	/* #UD condition is already handled. */
-	if (ctxt->ops->set_dr(ctxt, ctxt->modrm_reg, val) < 0)
+	if (ctxt->ops->set_dr(ctxt, ctxt->modrm_reg, val))
 		return emulate_gp(ctxt, 0);
 
 	/* Disable writeback. */
-- 
2.54.0.1032.g2f8565e1d1-goog

[PATCH v2 2/6] KVM: x86: Prioritize DR7.GD #DB over #GP due to illegal DR6/7 value

[Sean Christopherson]

When emulating a MOV DR, specifically a write to DR6 or DR7, treat a #DB
due to DR7.GD (General Detect) as higher priority than a #GP due to an
illegal value.  While neither Intel's SDM nor AMD's APM says anything
about the relative priority, empirical testing on Intel and AMD shows that
the #DB has higher priority.  And for VMX, where the instruction intercept
has priority over *all* exceptions, KVM already treats the #DB as having
higher priority.

Fixes: 3b88e41a4134 ("KVM: SVM: Add intercept check for accessing dr registers")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/emulate.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 75cd8b6136aa..4484c5fa19e3 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3854,11 +3854,16 @@ static int check_dr_write(struct x86_emulate_ctxt *ctxt)
 {
 	u64 new_val = ctxt->src.val64;
 	int dr = ctxt->modrm_reg;
+	int rc;
+
+	rc = check_dr_read(ctxt);
+	if (rc != X86EMUL_CONTINUE)
+		return rc;
 
 	if ((dr == 6 || dr == 7) && (new_val & 0xffffffff00000000ULL))
 		return emulate_gp(ctxt, 0);
 
-	return check_dr_read(ctxt);
+	return X86EMUL_CONTINUE;
 }
 
 static int check_svme(struct x86_emulate_ctxt *ctxt)
-- 
2.54.0.1032.g2f8565e1d1-goog

[PATCH v2 3/6] KVM: x86: Manually check DR4/5 write values to fix SVM intercept priority

[Sean Christopherson]

Manually (pre)check the values being written to DR4/5, i.e. the DR6/DR7
aliases, instead of relying on ->set_dr() => kvm_set_dr() to signal a #GP.
SVM unfortunately prioritizes all exceptions over an instruction intercept,
i.e. nSVM is relying on the emulator to perform *all* exception checks
prior to attempting to execute the instruction.

Fixes: 3b88e41a4134 ("KVM: SVM: Add intercept check for accessing dr registers")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/emulate.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 4484c5fa19e3..a1bccab0eefe 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3853,15 +3853,23 @@ static int check_dr_read(struct x86_emulate_ctxt *ctxt)
 static int check_dr_write(struct x86_emulate_ctxt *ctxt)
 {
 	u64 new_val = ctxt->src.val64;
-	int dr = ctxt->modrm_reg;
 	int rc;
 
 	rc = check_dr_read(ctxt);
 	if (rc != X86EMUL_CONTINUE)
 		return rc;
 
-	if ((dr == 6 || dr == 7) && (new_val & 0xffffffff00000000ULL))
-		return emulate_gp(ctxt, 0);
+	switch (ctxt->modrm_reg) {
+	case 4:
+	case 5:
+	case 6:
+	case 7:
+		if (new_val & 0xffffffff00000000ULL)
+			return emulate_gp(ctxt, 0);
+		break;
+	default:
+		break;
+	}
 
 	return X86EMUL_CONTINUE;
 }
-- 
2.54.0.1032.g2f8565e1d1-goog

[PATCH v2 4/6] KVM: x86: Use kvm_dr{6,7}_valid() to check DR{4,5,6,7} write values in emulator

[Sean Christopherson]

Use kvm_dr{6,7}_valid() to validate the incoming DR{4,5,6,7} value in the
emulator instead of open coding an equivalent check.  In the unlikely event
that the behavior of DR6/7 (and their aliases) changes in the future, using
common helpers will hopefully make it less likely the emulator logic will
be overlooked.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/emulate.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index a1bccab0eefe..1df2dcfd8bef 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3861,10 +3861,13 @@ static int check_dr_write(struct x86_emulate_ctxt *ctxt)
 
 	switch (ctxt->modrm_reg) {
 	case 4:
-	case 5:
 	case 6:
+		if (!kvm_dr6_valid(new_val))
+			return emulate_gp(ctxt, 0);
+		break;
+	case 5:
 	case 7:
-		if (new_val & 0xffffffff00000000ULL)
+		if (!kvm_dr7_valid(new_val))
 			return emulate_gp(ctxt, 0);
 		break;
 	default:
-- 
2.54.0.1032.g2f8565e1d1-goog

[PATCH v2 5/6] KVM: x86: WARN if MOV DR emulation hits a "too late" #GP

[Sean Christopherson]

WARN if ->set_dr() => kvm_set_dr() fails when emulating a MOV DR write,
as the emulator _must_ pre-check for #GPs in order to get the event
priority right when emulating MOV DR for L2 on SVM (all exceptions have
higher priority than the instruction intercept).

Opportunistically update the comment as the blurb about "#UD" being
checked is incomplete and misleading.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/emulate.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 1df2dcfd8bef..9d345ad25ac0 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3298,8 +3298,12 @@ static int em_dr_write(struct x86_emulate_ctxt *ctxt)
 	else
 		val = ctxt->src.val & ~0U;
 
-	/* #UD condition is already handled. */
-	if (ctxt->ops->set_dr(ctxt, ctxt->modrm_reg, val))
+	/*
+	 * A #GP due to an illegal value should be impossible at this point, as
+	 * such #GPs have priority over MOV DR intercepts on SVM, i.e. KVM must
+	 * manually check the value *before* emulating the write.
+	 */
+	if (WARN_ON_ONCE(ctxt->ops->set_dr(ctxt, ctxt->modrm_reg, val)))
 		return emulate_gp(ctxt, 0);
 
 	/* Disable writeback. */
-- 
2.54.0.1032.g2f8565e1d1-goog

[PATCH v2 6/6] KVM: x86: Read CR4.DE in emulator if and only if accessing DR4 or DR5

[Sean Christopherson]

Micro-optimize emulation of MOV DR instructions by checking CR4.DE if and
only if DR4 or DR5 is being accessed.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/emulate.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 9d345ad25ac0..37f3cee4358b 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3839,13 +3839,11 @@ static int check_cr_access(struct x86_emulate_ctxt *ctxt)
 static int check_dr_read(struct x86_emulate_ctxt *ctxt)
 {
 	int dr = ctxt->modrm_reg;
-	u64 cr4;
 
 	if (dr > 7)
 		return emulate_ud(ctxt);
 
-	cr4 = ctxt->ops->get_cr(ctxt, 4);
-	if ((cr4 & X86_CR4_DE) && (dr == 4 || dr == 5))
+	if ((dr == 4 || dr == 5) && (ctxt->ops->get_cr(ctxt, 4) & X86_CR4_DE))
 		return emulate_ud(ctxt);
 
 	if (ctxt->ops->get_effective_dr7(ctxt) & DR7_GD)
-- 
2.54.0.1032.g2f8565e1d1-goog

Re: [PATCH v2 2/6] KVM: x86: Prioritize DR7.GD #DB over #GP due to illegal DR6/7 value

[Maciej W. Rozycki]

On Wed, 3 Jun 2026, Sean Christopherson wrote:

> When emulating a MOV DR, specifically a write to DR6 or DR7, treat a #DB
> due to DR7.GD (General Detect) as higher priority than a #GP due to an
> illegal value.  While neither Intel's SDM nor AMD's APM says anything
> about the relative priority, empirical testing on Intel and AMD shows that
> the #DB has higher priority.  

 It has to.  The primary reason for the existence of DR7.GD is to prevent 
software being debugged such as an OS kernel from clobbering the debug 
register state when executed under the control of an ICE or a JTAG (XDP) 
debug probe.  Under these circumstances #GP remains an ordinary exception 
routed via the IDT to a handler provided by the OS, however #DB traps into 
the ICE or SMM mode instead and the OS handler is never reached.

 This is also the reason why DR7.GD gets cleared at the same time -- so
that the debug firmware can then access the debug state rather than making 
the exception trigger again -- and for the existence of the ICEBP aka INT1 
instruction -- so that #DB can be triggered and control regained by the 
debug firmware via a software breakpoint, since the number of hardware 
breakpoints is limited (and #BP is an ordinary exception).

 It's unsurprising that the priority of these exceptions is not documented 
well in the processor developer's manuals given that x86 hardware debug 
features remain vaguely documented overall in public resources.  Though I 
note the Intel wording for the GD bit is "[it] causes a debug exception to 
be generated prior to any MOV instruction that accesses a debug register." 
so I think it actually implies no action resulting from the execution of 
the affected instruction happens, including in particular any exception it 
might otherwise trigger.

 FWIW,

  Maciej

Re: [PATCH v2 2/6] KVM: x86: Prioritize DR7.GD #DB over #GP due to illegal DR6/7 value

[Sean Christopherson]

On Thu, Jun 04, 2026, Maciej W. Rozycki wrote:
> On Wed, 3 Jun 2026, Sean Christopherson wrote:
> 
> > When emulating a MOV DR, specifically a write to DR6 or DR7, treat a #DB
> > due to DR7.GD (General Detect) as higher priority than a #GP due to an
> > illegal value.  While neither Intel's SDM nor AMD's APM says anything
> > about the relative priority, empirical testing on Intel and AMD shows that
> > the #DB has higher priority.  
> 
>  It has to.  

Not super strictly speaking though, because ucode _could_ check for an illegal
value before enforcing DR7.GD.

OMG, Intel and AMD don't even behave the same.  I _was_ going to say that
"obviously the CPL #GP check has priority over DR7.GD", but on Intel the DR7.GD
#DB has priority over the CPL check, whereas on AMD the CPL check has priority.

Which actually proves my point: hardware/ucode doesn't _have to_ treat DR7.GD
with the absolute highest priority.

Ugh, and KVM's emulator doesn't get the priority between the #UD checks and the
CPL check.  Double ugh, half of the instructions that #GP at CPL>0 don't get the
priority right.

> The primary reason for the existence of DR7.GD is to prevent 
> software being debugged such as an OS kernel from clobbering the debug 
> register state when executed under the control of an ICE or a JTAG (XDP) 
> debug probe.  Under these circumstances #GP remains an ordinary exception 
> routed via the IDT to a handler provided by the OS, however #DB traps into 
> the ICE or SMM mode instead and the OS handler is never reached.
> 
>  This is also the reason why DR7.GD gets cleared at the same time -- so
> that the debug firmware can then access the debug state rather than making 
> the exception trigger again -- and for the existence of the ICEBP aka INT1 
> instruction -- so that #DB can be triggered and control regained by the 
> debug firmware via a software breakpoint, since the number of hardware 
> breakpoints is limited (and #BP is an ordinary exception).
> 
>  It's unsurprising that the priority of these exceptions is not documented 
> well in the processor developer's manuals given that x86 hardware debug 
> features remain vaguely documented overall in public resources.  Though I 
> note the Intel wording for the GD bit is "[it] causes a debug exception to 
> be generated prior to any MOV instruction that accesses a debug register." 

Define "accesses" though.  MOV DR at CPL>0 definitely isn't going to access
anything, and so that statement still holds true if the #GP due to CPL>0 had higher
priority, as there wouldn't be any DR access to prevent.

> so I think it actually implies no action resulting from the execution of 
> the affected instruction happens, including in particular any exception it 
> might otherwise trigger.
> 
>  FWIW,
> 
>   Maciej