From mboxrd@z Thu Jan  1 00:00:00 1970
From: will.deacon@arm.com (Will Deacon)
Date: Tue, 29 Mar 2016 16:06:50 +0100
Subject: [PATCH] arm64: mm: Add trace_irqflags annotations to
 do_debug_exception()
In-Reply-To: <1458580446-32474-1-git-send-email-james.morse@arm.com>
References: <1458580446-32474-1-git-send-email-james.morse@arm.com>
Message-ID: <20160329150650.GE6745@arm.com>
To: linux-arm-kernel@lists.infradead.org
List-Id: linux-arm-kernel.lists.infradead.org

On Mon, Mar 21, 2016 at 05:14:06PM +0000, James Morse wrote:
> With CONFIG_PROVE_LOCKING, CONFIG_DEBUG_LOCKDEP and CONFIG_TRACE_IRQFLAGS
> enabled, lockdep will compare current->hardirqs_enabled with the flags from
> local_irq_save().
> 
> When a debug exception occurs, interrupts are disabled in entry.S, but
> lockdep isn't told, resulting in:
> DEBUG_LOCKS_WARN_ON(current->hardirqs_enabled)
> ------------[ cut here ]------------
> WARNING: at ../kernel/locking/lockdep.c:3523
> Modules linked in:
> CPU: 3 PID: 1752 Comm: perf Not tainted 4.5.0-rc4+ #2204
> Hardware name: ARM Juno development board (r1) (DT)
> task: ffffffc974868000 ti: ffffffc975f40000 task.ti: ffffffc975f40000
> PC is at check_flags.part.35+0x17c/0x184
> LR is at check_flags.part.35+0x17c/0x184
> pc : [<ffffff80080fc93c>] lr : [<ffffff80080fc93c>] pstate: 600003c5
> [...]
> ---[ end trace 74631f9305ef5020 ]---
> Call trace:
> [<ffffff80080fc93c>] check_flags.part.35+0x17c/0x184
> [<ffffff80080ffe30>] lock_acquire+0xa8/0xc4
> [<ffffff8008093038>] breakpoint_handler+0x118/0x288
> [<ffffff8008082434>] do_debug_exception+0x3c/0xa8
> [<ffffff80080854b4>] el1_dbg+0x18/0x6c
> [<ffffff80081e82f4>] do_filp_open+0x64/0xdc
> [<ffffff80081d6e60>] do_sys_open+0x140/0x204
> [<ffffff80081d6f58>] SyS_openat+0x10/0x18
> [<ffffff8008085d30>] el0_svc_naked+0x24/0x28
> possible reason: unannotated irqs-off.
> irq event stamp: 65857
> hardirqs last  enabled at (65857): [<ffffff80081fb1c0>] lookup_mnt+0xf4/0x1b4
> hardirqs last disabled at (65856): [<ffffff80081fb188>] lookup_mnt+0xbc/0x1b4
> softirqs last  enabled at (65790): [<ffffff80080bdca4>] __do_softirq+0x1f8/0x290
> softirqs last disabled at (65757): [<ffffff80080be038>] irq_exit+0x9c/0xd0
> 
> This patch adds the annotations to do_debug_exception(), while trying not
> to call trace_hardirqs_off() if el1_dbg() interrupted a task that already
> had irqs disabled.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
>  arch/arm64/mm/fault.c | 33 +++++++++++++++++++++++----------
>  1 file changed, 23 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 44e56de23f79..d8b9c1da2630 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -548,20 +548,33 @@ asmlinkage int __exception do_debug_exception(unsigned long addr,
>  {
>  	const struct fault_info *inf = debug_fault_info + DBG_ESR_EVT(esr);
>  	struct siginfo info;
> +	int rv;
>  
> -	if (!inf->fn(addr, esr, regs))
> -		return 1;
> +	/*
> +	 * If we came in from el0_dbg, we disabled irqs. From el1_dbg,
> +	 * we need to test pstate.
> +	 */
> +	if (user_mode(regs) || !(regs->pstate & PSR_I_BIT))
> +		trace_hardirqs_off();

Can you use interrupts_enabled(regs) for this?

Will