[RFC,18/32] x86/fred: add a debug fault entry stub for FRED

Commit Message

Li, Xin3 Dec. 20, 2022, 6:36 a.m. UTC

From: "H. Peter Anvin (Intel)" <[e-mail]>

Add a debug fault entry stub for FRED.

On a FRED system, the debug trap status information (DR6) is passed
on the stack, to avoid the problem of transient state. Furthermore,
FRED transitions avoid a lot of ugly corner cases the handling of which
can, and should be, skipped.

The FRED debug trap status information saved on the stack differs from DR6
in both stickiness and polarity; it is exactly what debug_read_clear_dr6()
returns, and exc_debug_user()/exc_debug_kernel() expect.

Signed-off-by: H. Peter Anvin (Intel) <[e-mail]>
Signed-off-by: Xin Li <[e-mail]>
---
 arch/x86/include/asm/fred.h |  1 +
 arch/x86/kernel/traps.c     | 61 ++++++++++++++++++++++++++-----------
 2 files changed, 45 insertions(+), 17 deletions(-)

Comments

Peter Zijlstra Dec. 20, 2022, 9:15 a.m. UTC | #1

On Mon, Dec 19, 2022 at 10:36:44PM -0800, Xin Li wrote:

> +static __always_inline void debug_kernel_common(struct pt_regs *regs,
> +						unsigned long dr6)
>  {
> -	/*
> -	 * Disable breakpoints during exception handling; recursive exceptions
> -	 * are exceedingly 'fun'.
> -	 *
> -	 * Since this function is NOKPROBE, and that also applies to
> -	 * HW_BREAKPOINT_X, we can't hit a breakpoint before this (XXX except a
> -	 * HW_BREAKPOINT_W on our stack)
> -	 *
> -	 * Entry text is excluded for HW_BP_X and cpu_entry_area, which
> -	 * includes the entry stack is excluded for everything.
> -	 */
> -	unsigned long dr7 = local_db_save();
> -	irqentry_state_t irq_state = irqentry_nmi_enter(regs);
>  	instrumentation_begin();
>  
>  	/*
> @@ -1062,7 +1050,8 @@ static __always_inline void exc_debug_kernel(struct pt_regs *regs,
>  	 * Catch SYSENTER with TF set and clear DR_STEP. If this hit a
>  	 * watchpoint at the same time then that will still be handled.
>  	 */
> -	if ((dr6 & DR_STEP) && is_sysenter_singlestep(regs))
> +	if (!cpu_feature_enabled(X86_FEATURE_FRED) &&
> +	    (dr6 & DR_STEP) && is_sysenter_singlestep(regs))
>  		dr6 &= ~DR_STEP;
>  
>  	/*
> @@ -1089,8 +1078,28 @@ static __always_inline void exc_debug_kernel(struct pt_regs *regs,
>  		regs->flags &= ~X86_EFLAGS_TF;
>  out:
>  	instrumentation_end();
> -	irqentry_nmi_exit(regs, irq_state);
> +}

Why doesn't the FRED handler get to to use irqentry_nmi_{enter,exit}() ?

> @@ -1179,6 +1188,24 @@ DEFINE_IDTENTRY_DEBUG_USER(exc_debug)
>  {
>  	exc_debug_user(regs, debug_read_clear_dr6());
>  }
> +
> +# ifdef CONFIG_X86_FRED
> +DEFINE_FRED_HANDLER(fred_exc_debug)
> +{
> +	/*
> +	 * The FRED debug information saved onto stack differs from
> +	 * DR6 in both stickiness and polarity; it is exactly what
> +	 * debug_read_clear_dr6() returns.
> +	 */
> +	unsigned long dr6 = fred_event_data(regs);
> +
> +	if (user_mode(regs))
> +		exc_debug_user(regs, dr6);
> +	else
> +		debug_kernel_common(regs, dr6);
> +}
> +# endif /* CONFIG_X86_FRED */

Patch

diff --git a/arch/x86/include/asm/fred.h b/arch/x86/include/asm/fred.h
index 38a90eae7c0f..3089d1c70771 100644
--- a/arch/x86/include/asm/fred.h
+++ b/arch/x86/include/asm/fred.h
@@ -92,6 +92,7 @@  static __always_inline unsigned long fred_event_data(struct pt_regs *regs)
 #define DEFINE_FRED_HANDLER(f) noinstr DECLARE_FRED_HANDLER(f)
 typedef DECLARE_FRED_HANDLER((*fred_handler));
 
+DECLARE_FRED_HANDLER(fred_exc_debug);
 DECLARE_FRED_HANDLER(fred_exc_page_fault);
 
 #endif /* __ASSEMBLY__ */
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 99386836b02e..b0ee83bab9e6 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -47,6 +47,7 @@ 
 #include <asm/debugreg.h>
 #include <asm/realmode.h>
 #include <asm/text-patching.h>
+#include <asm/fred.h>
 #include <asm/ftrace.h>
 #include <asm/traps.h>
 #include <asm/desc.h>
@@ -1020,22 +1021,9 @@  static bool notify_debug(struct pt_regs *regs, unsigned long *dr6)
 	return false;
 }
 
-static __always_inline void exc_debug_kernel(struct pt_regs *regs,
-					     unsigned long dr6)
+static __always_inline void debug_kernel_common(struct pt_regs *regs,
+						unsigned long dr6)
 {
-	/*
-	 * Disable breakpoints during exception handling; recursive exceptions
-	 * are exceedingly 'fun'.
-	 *
-	 * Since this function is NOKPROBE, and that also applies to
-	 * HW_BREAKPOINT_X, we can't hit a breakpoint before this (XXX except a
-	 * HW_BREAKPOINT_W on our stack)
-	 *
-	 * Entry text is excluded for HW_BP_X and cpu_entry_area, which
-	 * includes the entry stack is excluded for everything.
-	 */
-	unsigned long dr7 = local_db_save();
-	irqentry_state_t irq_state = irqentry_nmi_enter(regs);
 	instrumentation_begin();
 
 	/*
@@ -1062,7 +1050,8 @@  static __always_inline void exc_debug_kernel(struct pt_regs *regs,
 	 * Catch SYSENTER with TF set and clear DR_STEP. If this hit a
 	 * watchpoint at the same time then that will still be handled.
 	 */
-	if ((dr6 & DR_STEP) && is_sysenter_singlestep(regs))
+	if (!cpu_feature_enabled(X86_FEATURE_FRED) &&
+	    (dr6 & DR_STEP) && is_sysenter_singlestep(regs))
 		dr6 &= ~DR_STEP;
 
 	/*
@@ -1089,8 +1078,28 @@  static __always_inline void exc_debug_kernel(struct pt_regs *regs,
 		regs->flags &= ~X86_EFLAGS_TF;
 out:
 	instrumentation_end();
-	irqentry_nmi_exit(regs, irq_state);
+}
+
+static __always_inline void exc_debug_kernel(struct pt_regs *regs,
+					     unsigned long dr6)
+{
+	/*
+	 * Disable breakpoints during exception handling; recursive exceptions
+	 * are exceedingly 'fun'.
+	 *
+	 * Since this function is NOKPROBE, and that also applies to
+	 * HW_BREAKPOINT_X, we can't hit a breakpoint before this (XXX except a
+	 * HW_BREAKPOINT_W on our stack)
+	 *
+	 * Entry text is excluded for HW_BP_X and cpu_entry_area, which
+	 * includes the entry stack is excluded for everything.
+	 */
+	unsigned long dr7 = local_db_save();
+	irqentry_state_t irq_state = irqentry_nmi_enter(regs);
+
+	debug_kernel_common(regs, dr6);
 
+	irqentry_nmi_exit(regs, irq_state);
 	local_db_restore(dr7);
 }
 
@@ -1179,6 +1188,24 @@  DEFINE_IDTENTRY_DEBUG_USER(exc_debug)
 {
 	exc_debug_user(regs, debug_read_clear_dr6());
 }
+
+# ifdef CONFIG_X86_FRED
+DEFINE_FRED_HANDLER(fred_exc_debug)
+{
+	/*
+	 * The FRED debug information saved onto stack differs from
+	 * DR6 in both stickiness and polarity; it is exactly what
+	 * debug_read_clear_dr6() returns.
+	 */
+	unsigned long dr6 = fred_event_data(regs);
+
+	if (user_mode(regs))
+		exc_debug_user(regs, dr6);
+	else
+		debug_kernel_common(regs, dr6);
+}
+# endif /* CONFIG_X86_FRED */
+
 #else
 /* 32 bit does not have separate entry points. */
 DEFINE_IDTENTRY_RAW(exc_debug)