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

[Will Deacon]

On Thu, Feb 20, 2014 at 06:58:25AM +0000, Vijay Kilari wrote:
> Hi Will,

Hi Vijay,

> On Wed, Feb 19, 2014 at 9:42 PM, Will Deacon <will.deacon at arm.com> wrote:
> > The reply from Vijay suggested that everything is confined to a single CPU.
> 
>   This issue is seen only when kgdb test is triggered on secondary cpu's.
> when executed on cpu0 issue is not seen. Though cpu0 triggers kgdb tests
> the do_fork test might trigger debug exception on other cpu's as well,
> which fails.
> 
> For the patch 1 you suggested me to call local_dbg_enable() from
> clear_os_mask() function
> as below
> 
> static void clear_os_lock(void *unused)
> {
>         asm volatile("msr oslar_el1, %0" : : "r" (0));
>         isb();
>         local_dbg_enable();
> }
> 
> This is an SMP call. So when more than one core is enabled, this
> local_dbg_enable() is called
> from SMP call context, which is el1_irq context for secondary cpu's.
> So PSTATE.D flag is unmasked
> only in irq context but not in normal context.

Aha, well spotted!

> For CPU0 this clear_os_lock() is not called from smp call. it is
> directly called in debug_monitors_init()
> call. So PSTATE.D is unmasked for CPU0 and hence kgdb tests passed
> when triggered only on
> cpu0.
> 
> static int debug_monitors_init(void)
> {
>         /* Clear the OS lock. */
>         smp_call_function(clear_os_lock, NULL, 1);
>         clear_os_lock(NULL);
> 
>         /* Register hotplug handler. */
>         register_cpu_notifier(&os_lock_nb);
>         return 0;
> }
> 
> When I made below patch it works. I have tested with 4 cores on foundation model

The main change here is that we enable debug exceptions before we unlock the
os lock. That should be fine, since everything apart from software
breakpoint exceptions are masked when the lock is locked.

> --- 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?

Will