From mboxrd@z Thu Jan  1 00:00:00 1970
From: will.deacon@arm.com (Will Deacon)
Date: Mon, 5 Jun 2017 17:29:19 +0100
Subject: [PATCH v3 2/4] arm64: kgdb: disable interrupts while a software
 step is enabled
In-Reply-To: <20170523043058.5463-3-takahiro.akashi@linaro.org>
References: <20170523043058.5463-1-takahiro.akashi@linaro.org>
 <20170523043058.5463-3-takahiro.akashi@linaro.org>
Message-ID: <20170605162919.GN21944@arm.com>
To: linux-arm-kernel@lists.infradead.org
List-Id: linux-arm-kernel.lists.infradead.org

On Tue, May 23, 2017 at 01:30:56PM +0900, AKASHI Takahiro wrote:
> After entering kgdb mode, 'stepi' may unexpectedly breaks the execution
> somewhere in el1_irq.
> 
> This happens because a debug exception is always enabled in el1_irq
> due to the following commit merged in v3.16:
>   commit 2a2830703a23 ("arm64: debug: avoid accessing mdscr_el1 on fault
> 			paths where possible")
> A pending interrupt can be taken after kgdb has enabled a software step,
> but before a debug exception is actually taken.
> 
> This patch enforces interrupts to be masked while single stepping.

The desired behaviour here boils down to whether or not KGDB expects to step
into or over interrupts in response a stepi instruction. What do other
architectures do? What happens if the instruction being stepped faults?

Will

> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Jason Wessel <jason.wessel@windriver.com>
> ---
>  arch/arm64/kernel/kgdb.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
> index b9176b324e5a..fddbc6be3780 100644
> --- a/arch/arm64/kernel/kgdb.c
> +++ b/arch/arm64/kernel/kgdb.c
> @@ -28,6 +28,7 @@
>  
>  #include <asm/debug-monitors.h>
>  #include <asm/insn.h>
> +#include <asm/ptrace.h>
>  #include <asm/traps.h>
>  
>  struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] = {
> @@ -111,6 +112,8 @@ struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] = {
>  	{ "fpcr", 4, -1 },
>  };
>  
> +static DEFINE_PER_CPU(unsigned int, kgdb_pstate);
> +
>  char *dbg_get_reg(int regno, void *mem, struct pt_regs *regs)
>  {
>  	if (regno >= DBG_MAX_REG_NUM || regno < 0)
> @@ -200,6 +203,10 @@ int kgdb_arch_handle_exception(int exception_vector, int signo,
>  		err = 0;
>  		break;
>  	case 's':
> +		/* mask interrupts while single stepping */
> +		__this_cpu_write(kgdb_pstate, linux_regs->pstate);
> +		linux_regs->pstate |= PSR_I_BIT;
> +
>  		/*
>  		 * Update step address value with address passed
>  		 * with step packet.
> @@ -242,11 +249,20 @@ NOKPROBE_SYMBOL(kgdb_compiled_brk_fn);
>  
>  static int kgdb_step_brk_fn(struct pt_regs *regs, unsigned int esr)
>  {
> +	unsigned int pstate;
> +
>  	if (!kgdb_single_step)
>  		return DBG_HOOK_ERROR;
>  
>  	kernel_disable_single_step();
>  
> +	/* restore interrupt mask status */
> +	pstate = __this_cpu_read(kgdb_pstate);
> +	if (pstate & PSR_I_BIT)
> +		regs->pstate |= PSR_I_BIT;
> +	else
> +		regs->pstate &= ~PSR_I_BIT;
> +
>  	kgdb_handle_exception(1, SIGTRAP, 0, regs);
>  	return 0;
>  }
> -- 
> 2.11.1
>