----- On May 5, 2020, at 9:49 AM, Thomas Gleixner t...@linutronix.de wrote:
> From: Peter Zijlstra <pet...@infradead.org>
>
> DR6/7 should be handled before nmi_enter() is invoked and restore after
> nmi_exit() to minimize the exposure.
>
> Split it out into helper inlines and bring it into the correct order.
>
[...]
>
> +static __always_inline void debug_enter(unsigned long *dr6, unsigned long
> *dr7)
> +{
> + /*
> + * 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.
> + */
> + get_debugreg(*dr7, 6);
> + set_debugreg(0, 7);
> +
> + /*
> + * The Intel SDM says:
> + *
> + * Certain debug exceptions may clear bits 0-3. The remaining
> + * contents of the DR6 register are never cleared by the
> + * processor. To avoid confusion in identifying debug
> + * exceptions, debug handlers should clear the register before
> + * returning to the interrupted task.
> + *
> + * Keep it simple: clear DR6 immediately.
> + */
> + get_debugreg(*dr6, 6);
> + set_debugreg(0, 6);
> + /* Filter out all the reserved bits which are preset to 1 */
> + *dr6 &= ~DR6_RESERVED;
> +}
> +
> +static __always_inline void debug_exit(unsigned long dr7)
> +{
> + set_debugreg(dr7, 7);
> +}
Out of curiosity, what prevents the compiler from moving instructions
outside of the code regions surrounded by entry/exit ? This is an always
inline, which invokes set_debugreg which is inline for CONFIG_PARAVIRT_XXL=n,
which in turn uses an asm() (not volatile), without any memory clobber.
Also, considering that "inline" is not sufficient to ensure the compiler
does not emit a traceable function, I suspect you'll also want to mark
"native_get_debugreg" and "native_set_debugreg" always inline as well.
Thanks,
Mathieu
> +
> /*
> * Our handling of the processor debug registers is non-trivial.
> * We do not clear them on entry and exit from the kernel. Therefore
> @@ -718,28 +756,13 @@ static bool is_sysenter_singlestep(struc
> dotraplinkage void do_debug(struct pt_regs *regs, long error_code)
> {
> struct task_struct *tsk = current;
> + unsigned long dr6, dr7;
> int user_icebp = 0;
> - unsigned long dr6;
> int si_code;
>
> - nmi_enter();
> -
> - get_debugreg(dr6, 6);
> - /*
> - * The Intel SDM says:
> - *
> - * Certain debug exceptions may clear bits 0-3. The remaining
> - * contents of the DR6 register are never cleared by the
> - * processor. To avoid confusion in identifying debug
> - * exceptions, debug handlers should clear the register before
> - * returning to the interrupted task.
> - *
> - * Keep it simple: clear DR6 immediately.
> - */
> - set_debugreg(0, 6);
> + debug_enter(&dr6, &dr7);
>
> - /* Filter out all the reserved bits which are preset to 1 */
> - dr6 &= ~DR6_RESERVED;
> + nmi_enter();
>
> /*
> * The SDM says "The processor clears the BTF flag when it
> @@ -777,7 +800,7 @@ dotraplinkage void do_debug(struct pt_re
> #endif
>
> if (notify_die(DIE_DEBUG, "debug", regs, (long)&dr6, error_code,
> - SIGTRAP) == NOTIFY_STOP)
> + SIGTRAP) == NOTIFY_STOP)
> goto exit;
>
> /*
> @@ -816,6 +839,7 @@ dotraplinkage void do_debug(struct pt_re
>
> exit:
> nmi_exit();
> + debug_exit(dr7);
> }
> NOKPROBE_SYMBOL(do_debug);
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
