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@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@arm.com
Thanks,
Will