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

From: [email-masked] (Catalin Marinas)
Date: Thu, 20 Feb 2014 16:27:10 +0000

On Thu, Feb 20, 2014 at 03:56:00PM +0000, Will Deacon wrote:
> On Thu, Feb 20, 2014 at 12:22:14PM +0000, Vijay Kilari wrote:
> > On Thu, Feb 20, 2014 at 5:12 PM, Will Deacon <[email-masked]> 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 <[email-masked]>

Vijay, could you please post a commit log together with the above patch
(and Will's ack) so that I can merge it on top of your other patches?

Thanks.

-- 
Catalin