[PATCH v9 1/6] arm64: Add macros to manage processor debug state

[Will Deacon]

On Thu, Feb 20, 2014 at 12:22:14PM +0000, Vijay Kilari wrote:
> On Thu, Feb 20, 2014 at 5:12 PM, Will Deacon <will.deacon at arm.com> wrote:
> > On Thu, Feb 20, 2014 at 06:58:25AM +0000, Vijay Kilari wrote:
> >> --- a/arch/arm64/kernel/smp.c
> >> +++ b/arch/arm64/kernel/smp.c
> >> @@ -160,6 +160,8 @@ asmlinkage void secondary_start_kernel(void)
> >>         set_cpu_online(cpu, true);
> >>         complete(&cpu_running);
> >>
> >> +       local_dbg_enable();
> >>         local_irq_enable();
> >>         local_async_enable();
> >
> > The only thing to add then moving the isb(); local_dbg_enable() out of
> > clear_os_lock and into debug_monitors_init. You can probably make the
> > smp_call_function an on_each_cpu too.
> >
> > Does that make sense?
> 
> Its ok for me.  Isn't require to have isb() after clearing os lock for
> every cpu?
> Just having isb on cpu0 after clearing os lock is enough?

They'll synchronise on return from the IPI.

> --- a/arch/arm64/kernel/debug-monitors.c
> +++ b/arch/arm64/kernel/debug-monitors.c
> @@ -137,8 +137,6 @@ void disable_debug_monitors(enum debug_el el)
>  static void clear_os_lock(void *unused)
>  {
>         asm volatile("msr oslar_el1, %0" : : "r" (0));
> -       isb();
> -       local_dbg_enable();
>  }
> 
>  static int os_lock_notify(struct notifier_block *self,
> @@ -157,8 +155,9 @@ static struct notifier_block os_lock_nb = {
>  static int debug_monitors_init(void)
>  {
>         /* Clear the OS lock. */
> -       smp_call_function(clear_os_lock, NULL, 1);
> -       clear_os_lock(NULL);
> +       on_each_cpu(clear_os_lock, NULL, 1);
> +       isb();
> +       local_dbg_enable();
> 
>         /* Register hotplug handler. */
>         register_cpu_notifier(&os_lock_nb);
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 7cfb92a..5070dc3 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -160,6 +160,7 @@ asmlinkage void secondary_start_kernel(void)
>         set_cpu_online(cpu, true);
>         complete(&cpu_running);
> 
> +       local_dbg_enable();
>         local_irq_enable();
>         local_async_enable();
> 
> Is this ok?

Looks good to me:

  Acked-by: Will Deacon <will.deacon at arm.com>

Thanks,

Will