[RFC PATCH 1/2] arm64: hw_breakpoint: convert CPU hotplug notifier to new infrastructure
Lorenzo Pieralisi
lorenzo.pieralisi at arm.com
Wed Aug 31 08:35:16 PDT 2016
On Fri, Aug 26, 2016 at 11:35:57AM +0100, Will Deacon wrote:
> The arm64 hw_breakpoint implementation uses a CPU hotplug notifier to
> reset the {break,watch}point registers when CPUs come online.
>
> This patch converts the code to the new hotplug mechanism, whilst moving
> the invocation earlier to remove the need to disable IRQs explicitly in
> the driver (which could cause havok if we trip a watchpoint in an IRQ
> handler whilst restoring the debug register state).
>
> Cc: Sebastian Andrzej Siewior <bigeasy at linutronix.de>
> Signed-off-by: Will Deacon <will.deacon at arm.com>
> ---
> arch/arm64/kernel/hw_breakpoint.c | 48 +++++++++++++--------------------------
> arch/arm64/kernel/suspend.c | 10 ++++----
> include/linux/cpuhotplug.h | 1 +
> 3 files changed, 23 insertions(+), 36 deletions(-)
For both patches:
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi at arm.com>
> diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
> index 26a6bf77d272..948b73148d56 100644
> --- a/arch/arm64/kernel/hw_breakpoint.c
> +++ b/arch/arm64/kernel/hw_breakpoint.c
> @@ -857,7 +857,7 @@ void hw_breakpoint_thread_switch(struct task_struct *next)
> /*
> * CPU initialisation.
> */
> -static void hw_breakpoint_reset(void *unused)
> +static int hw_breakpoint_reset(unsigned int cpu)
> {
> int i;
> struct perf_event **slots;
> @@ -888,28 +888,14 @@ static void hw_breakpoint_reset(void *unused)
> write_wb_reg(AARCH64_DBG_REG_WVR, i, 0UL);
> }
> }
> -}
>
> -static int hw_breakpoint_reset_notify(struct notifier_block *self,
> - unsigned long action,
> - void *hcpu)
> -{
> - if ((action & ~CPU_TASKS_FROZEN) == CPU_ONLINE) {
> - local_irq_disable();
> - hw_breakpoint_reset(NULL);
> - local_irq_enable();
> - }
> - return NOTIFY_OK;
> + return 0;
> }
>
> -static struct notifier_block hw_breakpoint_reset_nb = {
> - .notifier_call = hw_breakpoint_reset_notify,
> -};
> -
> #ifdef CONFIG_CPU_PM
> -extern void cpu_suspend_set_dbg_restorer(void (*hw_bp_restore)(void *));
> +extern void cpu_suspend_set_dbg_restorer(int (*hw_bp_restore)(unsigned int));
> #else
> -static inline void cpu_suspend_set_dbg_restorer(void (*hw_bp_restore)(void *))
> +static inline void cpu_suspend_set_dbg_restorer(int (*hw_bp_restore)(unsigned int))
> {
> }
> #endif
> @@ -919,36 +905,34 @@ static inline void cpu_suspend_set_dbg_restorer(void (*hw_bp_restore)(void *))
> */
> static int __init arch_hw_breakpoint_init(void)
> {
> + int ret;
> +
> core_num_brps = get_num_brps();
> core_num_wrps = get_num_wrps();
>
> pr_info("found %d breakpoint and %d watchpoint registers.\n",
> core_num_brps, core_num_wrps);
>
> - cpu_notifier_register_begin();
> -
> - /*
> - * Reset the breakpoint resources. We assume that a halting
> - * debugger will leave the world in a nice state for us.
> - */
> - smp_call_function(hw_breakpoint_reset, NULL, 1);
> - hw_breakpoint_reset(NULL);
> -
> /* Register debug fault handlers. */
> hook_debug_fault_code(DBG_ESR_EVT_HWBP, breakpoint_handler, SIGTRAP,
> TRAP_HWBKPT, "hw-breakpoint handler");
> hook_debug_fault_code(DBG_ESR_EVT_HWWP, watchpoint_handler, SIGTRAP,
> TRAP_HWBKPT, "hw-watchpoint handler");
>
> - /* Register hotplug notifier. */
> - __register_cpu_notifier(&hw_breakpoint_reset_nb);
> -
> - cpu_notifier_register_done();
> + /*
> + * Reset the breakpoint resources. We assume that a halting
> + * debugger will leave the world in a nice state for us.
> + */
> + ret = cpuhp_setup_state(CPUHP_AP_PERF_ARM_HW_BREAKPOINT_STARTING,
> + "CPUHP_AP_PERF_ARM_HW_BREAKPOINT_STARTING",
> + hw_breakpoint_reset, NULL);
> + if (ret)
> + pr_err("failed to register CPU hotplug notifier: %d\n", ret);
>
> /* Register cpu_suspend hw breakpoint restore hook */
> cpu_suspend_set_dbg_restorer(hw_breakpoint_reset);
>
> - return 0;
> + return ret;
> }
> arch_initcall(arch_hw_breakpoint_init);
>
> diff --git a/arch/arm64/kernel/suspend.c b/arch/arm64/kernel/suspend.c
> index b616e365cee3..ad734142070d 100644
> --- a/arch/arm64/kernel/suspend.c
> +++ b/arch/arm64/kernel/suspend.c
> @@ -23,8 +23,8 @@ unsigned long *sleep_save_stash;
> * time the notifier runs debug exceptions might have been enabled already,
> * with HW breakpoints registers content still in an unknown state.
> */
> -static void (*hw_breakpoint_restore)(void *);
> -void __init cpu_suspend_set_dbg_restorer(void (*hw_bp_restore)(void *))
> +static int (*hw_breakpoint_restore)(unsigned int);
> +void __init cpu_suspend_set_dbg_restorer(int (*hw_bp_restore)(unsigned int))
> {
> /* Prevent multiple restore hook initializations */
> if (WARN_ON(hw_breakpoint_restore))
> @@ -34,6 +34,8 @@ void __init cpu_suspend_set_dbg_restorer(void (*hw_bp_restore)(void *))
>
> void notrace __cpu_suspend_exit(void)
> {
> + unsigned int cpu = smp_processor_id();
> +
> /*
> * We are resuming from reset with the idmap active in TTBR0_EL1.
> * We must uninstall the idmap and restore the expected MMU
> @@ -45,7 +47,7 @@ void notrace __cpu_suspend_exit(void)
> * Restore per-cpu offset before any kernel
> * subsystem relying on it has a chance to run.
> */
> - set_my_cpu_offset(per_cpu_offset(smp_processor_id()));
> + set_my_cpu_offset(per_cpu_offset(cpu));
>
> /*
> * Restore HW breakpoint registers to sane values
> @@ -53,7 +55,7 @@ void notrace __cpu_suspend_exit(void)
> * through local_dbg_restore.
> */
> if (hw_breakpoint_restore)
> - hw_breakpoint_restore(NULL);
> + hw_breakpoint_restore(cpu);
> }
>
> /*
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index 242bf530edfc..3758fe6d5968 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -45,6 +45,7 @@ enum cpuhp_state {
> CPUHP_AP_PERF_METAG_STARTING,
> CPUHP_AP_MIPS_OP_LOONGSON3_STARTING,
> CPUHP_AP_ARM_VFP_STARTING,
> + CPUHP_AP_PERF_ARM_HW_BREAKPOINT_STARTING,
> CPUHP_AP_PERF_ARM_STARTING,
> CPUHP_AP_ARM_L2X0_STARTING,
> CPUHP_AP_ARM_ARCH_TIMER_STARTING,
> --
> 2.1.4
>
More information about the linux-arm-kernel
mailing list