[PATCH v8 1/6] powerpc: Allow clearing and restoring registers independent of saved breakpoint state

[Russell Currey]

On Fri, 2022-10-21 at 16:22 +1100, Benjamin Gray wrote:
> From: Jordan Niethe <jniethe5 at gmail.com>

Hi Ben,

> For the coming temporary mm used for instruction patching, the
> breakpoint registers need to be cleared to prevent them from
> accidentally being triggered. As soon as the patching is done, the
> breakpoints will be restored. The breakpoint state is stored in the
> per
> cpu variable current_brk[]. Add a pause_breakpoints() function which
> will
> clear the breakpoint registers without touching the state in
> current_bkr[]. Add a pair function unpause_breakpoints() which will
 
typo here ^

> move
> the state in current_brk[] back to the registers.
> 
> Signed-off-by: Jordan Niethe <jniethe5 at gmail.com>
> Signed-off-by: Benjamin Gray <bgray at linux.ibm.com>
> ---
>  arch/powerpc/include/asm/debug.h |  2 ++
>  arch/powerpc/kernel/process.c    | 36 +++++++++++++++++++++++++++++-
> --
>  2 files changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/debug.h
> b/arch/powerpc/include/asm/debug.h
> index 86a14736c76c..83f2dc3785e8 100644
> --- a/arch/powerpc/include/asm/debug.h
> +++ b/arch/powerpc/include/asm/debug.h
> @@ -46,6 +46,8 @@ static inline int debugger_fault_handler(struct
> pt_regs *regs) { return 0; }
>  #endif
>  
>  void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk);
> +void pause_breakpoints(void);
> +void unpause_breakpoints(void);

Nitpick, would (clear/suspend)/restore be clearer than pause/unpause?

>  bool ppc_breakpoint_available(void);
>  #ifdef CONFIG_PPC_ADV_DEBUG_REGS
>  extern void do_send_trap(struct pt_regs *regs, unsigned long
> address,
> diff --git a/arch/powerpc/kernel/process.c
> b/arch/powerpc/kernel/process.c
> index 67da147fe34d..7aee1b30e73c 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -685,6 +685,7 @@ DEFINE_INTERRUPT_HANDLER(do_break)
>  
>  static DEFINE_PER_CPU(struct arch_hw_breakpoint,
> current_brk[HBP_NUM_MAX]);
>  
> +

some bonus whitespace here

>  #ifdef CONFIG_PPC_ADV_DEBUG_REGS
>  /*
>   * Set the debug registers back to their default "safe" values.
> @@ -862,10 +863,8 @@ static inline int set_breakpoint_8xx(struct
> arch_hw_breakpoint *brk)
>         return 0;
>  }
>  
> -void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk)
> +static void ____set_breakpoint(int nr, struct arch_hw_breakpoint
> *brk)

Is there a way to refactor this?  The quad underscore is pretty cursed.

>  {
> -       memcpy(this_cpu_ptr(&current_brk[nr]), brk, sizeof(*brk));
> -
>         if (dawr_enabled())
>                 // Power8 or later
>                 set_dawr(nr, brk);
> @@ -879,6 +878,12 @@ void __set_breakpoint(int nr, struct
> arch_hw_breakpoint *brk)
>                 WARN_ON_ONCE(1);
>  }
>  
> +void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk)
> +{
> +       memcpy(this_cpu_ptr(&current_brk[nr]), brk, sizeof(*brk));
> +       ____set_breakpoint(nr, brk);
> +}
> +
>  /* Check if we have DAWR or DABR hardware */
>  bool ppc_breakpoint_available(void)
>  {
> @@ -891,6 +896,31 @@ bool ppc_breakpoint_available(void)
>  }
>  EXPORT_SYMBOL_GPL(ppc_breakpoint_available);
>  
> +/* Disable the breakpoint in hardware without touching current_brk[]
> */
> +void pause_breakpoints(void)
> +{
> +       struct arch_hw_breakpoint brk = {0};
> +       int i;
> +
> +       if (!ppc_breakpoint_available())
> +               return;
> +
> +       for (i = 0; i < nr_wp_slots(); i++)
> +               ____set_breakpoint(i, &brk);
> +}
> +
> +/* Renable the breakpoint in hardware from current_brk[] */
> +void unpause_breakpoints(void)
> +{
> +       int i;
> +
> +       if (!ppc_breakpoint_available())
> +               return;
> +
> +       for (i = 0; i < nr_wp_slots(); i++)
> +               ____set_breakpoint(i, this_cpu_ptr(&current_brk[i]));
> +}
> +
>  #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>  
>  static inline bool tm_enabled(struct task_struct *tsk)