[kvm-unit-tests,RFC] x86: Fix "debug" test on AMD uArch

Commit Message

Ravi Bangoria Feb. 24, 2025, 11:26 a.m. UTC

As per the AMD APM[1], DR6[BusLockDetect] bit is unmodified for any source
of #DB exception other than Bus Lock (and AMD HW is working correctly as
per the spec).

KUT debug test initializes DR6[BusLockDetect] with 0 before executing each
test and thus the bit remains 0 at the #DB exception for sources other
than Bus Lock. Since DR6[BusLockDetect] bit has opposite polarity, as in,
value 0 indicates the condition, KUT tests are interpreting it as #DB due
to Bus Lock and thus they are failing.

Fix this by initializing DR6 with a valid default value before running the
test.

[1]: AMD64 Architecture Programmer's Manual Pub. 40332, Rev. 4.07 - June
     2023, Vol 2, 13.1.3.6 Bus Lock Trap
     https://bugzilla.kernel.org/attachment.cgi?id=304653

Note: Although Intel SDM (December 2024, Vol 3B, 19.2.3 Debug Status
Register (DR6)) stats the same "Other debug exceptions do not modify this
bit", HW seems to be resetting DR6[BusLockDetect] to 1 for #DB with
non-BusLock sources? In any case, initializing DR6 with correct value is
semantically correct on Intel as well, so this should not have any
regression on Intel platforms.

Reported-by: Sean Christopherson <[e-mail]>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219787#c13
Signed-off-by: Ravi Bangoria <[e-mail]>
---
 x86/debug.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

Comments

Sean Christopherson Feb. 24, 2025, 3:28 p.m. UTC | #1

On Mon, Feb 24, 2025, Ravi Bangoria wrote:
> As per the AMD APM[1], DR6[BusLockDetect] bit is unmodified for any source
> of #DB exception other than Bus Lock (and AMD HW is working correctly as
> per the spec).
> 
> KUT debug test initializes DR6[BusLockDetect] with 0 before executing each
> test and thus the bit remains 0 at the #DB exception for sources other
> than Bus Lock. Since DR6[BusLockDetect] bit has opposite polarity, as in,
> value 0 indicates the condition, KUT tests are interpreting it as #DB due
> to Bus Lock and thus they are failing.
> 
> Fix this by initializing DR6 with a valid default value before running the
> test.

The test is weird, but as-is it's correct.  The APM does a poor job of stating
the exact behavior, but DR6[11] should never go to '0' if BusLockTrap is disabled,
even if software explicitly writes '0'.  Any other behavior would break backwards
compatibility with existing software (as evidenced by the test failing).

Editing to omit irrelevant snippets: 

  Software enables bus lock trap by setting DebugCtl MSR[BLCKDB] (bit 2) to 1
  When bus lock trap is enabled, ... The processor indicates that this #DB was
  caused by a bus lock by clearing DR6[BLD] (bit 11). DR6[11] previously had
  been defined to be always 1.
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The test fails because the host leaves DebugCtl.BLCKDB, a.k.a. BusLockDetect,
enabled.  With my to-be-posted change to manually clear DebugCtl prior to VMRUN,
the test passes.

Patch

diff --git a/x86/debug.c b/x86/debug.c
index f493567c..2b1084a2 100644
--- a/x86/debug.c
+++ b/x86/debug.c
@@ -93,7 +93,7 @@  static void __run_single_step_db_test(db_test_fn test, db_report_fn report_fn)
 	bool ign;
 
 	n = 0;
-	write_dr6(0);
+	write_dr6(DR6_ACTIVE_LOW);
 
 	start = test();
 	report_fn(start, "");
@@ -107,7 +107,7 @@  static void __run_single_step_db_test(db_test_fn test, db_report_fn report_fn)
 		return;
 
 	n = 0;
-	write_dr6(0);
+	write_dr6(DR6_ACTIVE_LOW);
 
 	/*
 	 * Run the test in usermode.  Use the expected start RIP from the first
@@ -339,6 +339,7 @@  static noinline unsigned long singlestep_with_movss_blocking_and_dr7_gd(void)
 	 * General Detect #DB.
 	 */
 	asm volatile(
+		"mov $0xffff0ff0, %%rcx\n\t"
 		"xor %0, %0\n\t"
 		"pushf\n\t"
 		"pop %%rax\n\t"
@@ -347,7 +348,7 @@  static noinline unsigned long singlestep_with_movss_blocking_and_dr7_gd(void)
 		"mov %%ss, %%ax\n\t"
 		"popf\n\t"
 		"mov %%ax, %%ss\n\t"
-		"1: mov %0, %%dr6\n\t"
+		"1: mov %%rcx, %%dr6\n\t"
 		"and $~(1<<8),%%rax\n\t"
 		"push %%rax\n\t"
 		"popf\n\t"
@@ -433,7 +434,7 @@  int main(int ac, char **av)
 	write_cr4(cr4 | X86_CR4_DE);
 	read_dr4();
 	report(got_ud, "DR4 read got #UD with CR4.DE == 1");
-	write_dr6(0);
+	write_dr6(DR6_ACTIVE_LOW);
 
 	extern unsigned char sw_bp;
 	asm volatile("int3; sw_bp:");
@@ -460,7 +461,7 @@  int main(int ac, char **av)
 	n = 0;
 	extern unsigned char hw_bp2;
 	write_dr0(&hw_bp2);
-	write_dr6(DR6_BS | DR6_TRAP1);
+	write_dr6(DR6_BS | DR6_TRAP1 | DR6_ACTIVE_LOW);
 	asm volatile("hw_bp2: nop");
 	report(n == 1 &&
 	       db_addr[0] == ((unsigned long)&hw_bp2) &&
@@ -477,7 +478,7 @@  int main(int ac, char **av)
 
 	n = 0;
 	write_dr1((void *)&value);
-	write_dr6(DR6_BS);
+	write_dr6(DR6_BS | DR6_ACTIVE_LOW);
 	write_dr7(0x00d0040a); // 4-byte write
 
 	extern unsigned char hw_wp1;
@@ -491,7 +492,7 @@  int main(int ac, char **av)
 	       "hw watchpoint (test that dr6.BS is not cleared)");
 
 	n = 0;
-	write_dr6(0);
+	write_dr6(DR6_ACTIVE_LOW);
 
 	extern unsigned char hw_wp2;
 	asm volatile(
@@ -504,7 +505,7 @@  int main(int ac, char **av)
 	       "hw watchpoint (test that dr6.BS is not set)");
 
 	n = 0;
-	write_dr6(0);
+	write_dr6(DR6_ACTIVE_LOW);
 	extern unsigned char sw_icebp;
 	asm volatile(".byte 0xf1; sw_icebp:");
 	report(n == 1 &&