Re: [PATCH 1/4] arm64: kgdb: Fix single-step exception handling oops

From: Doug Anderson <dianders@chromium.org>
To: Wei Li <liwei391@huawei.com>
Cc: Daniel Thompson <daniel.thompson@linaro.org>,
	Jason Wessel <jason.wessel@windriver.com>,
	Marc Zyngier <maz@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	David Miller <davem@davemloft.net>, Will Deacon <will@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	LKML <linux-kernel@vger.kernel.org>,
	liwei1412@163.com
Subject: Re: [PATCH 1/4] arm64: kgdb: Fix single-step exception handling oops
Date: Wed, 13 May 2020 17:21:13 -0700	[thread overview]
Message-ID: <CAD=FV=U+EGz5nRoPXa_V+85FDwHjnAK6-ubH0zv1gGROKNZhPg@mail.gmail.com> (raw)
In-Reply-To: <20200509214159.19680-2-liwei391@huawei.com>

Hi,

On Sat, May 9, 2020 at 6:49 AM Wei Li <liwei391@huawei.com> wrote:
>
> After entering kdb due to breakpoint, when we execute 'ss' or 'go' (will
> delay installing breakpoints, do single-step first), it won't work
> correctly, and it will enter kdb due to oops.
>
> It's because the reason gotten in kdb_stub() is not as expected, and it
> seems that the ex_vector for single-step should be 0, like what arch
> powerpc/sh/parisc has implemented.
>
> Before the patch:
> Entering kdb (current=0xffff8000119e2dc0, pid 0) on processor 0 due to Keyboard Entry
> [0]kdb> bp printk
> Instruction(i) BP #0 at 0xffff8000101486cc (printk)
>     is enabled   addr at ffff8000101486cc, hardtype=0 installed=0
>
> [0]kdb> g
>
> / # echo h > /proc/sysrq-trigger
>
> Entering kdb (current=0xffff0000fa878040, pid 266) on processor 3 due to Breakpoint @ 0xffff8000101486cc
> [3]kdb> ss
>
> Entering kdb (current=0xffff0000fa878040, pid 266) on processor 3 Oops: (null)
> due to oops @ 0xffff800010082ab8
> CPU: 3 PID: 266 Comm: sh Not tainted 5.7.0-rc4-13839-gf0e5ad491718 #6
> Hardware name: linux,dummy-virt (DT)
> pstate: 00000085 (nzcv daIf -PAN -UAO)
> pc : el1_irq+0x78/0x180
> lr : __handle_sysrq+0x80/0x190
> sp : ffff800015003bf0
> x29: ffff800015003d20 x28: ffff0000fa878040
> x27: 0000000000000000 x26: ffff80001126b1f0
> x25: ffff800011b6a0d8 x24: 0000000000000000
> x23: 0000000080200005 x22: ffff8000101486cc
> x21: ffff800015003d30 x20: 0000ffffffffffff
> x19: ffff8000119f2000 x18: 0000000000000000
> x17: 0000000000000000 x16: 0000000000000000
> x15: 0000000000000000 x14: 0000000000000000
> x13: 0000000000000000 x12: 0000000000000000
> x11: 0000000000000000 x10: 0000000000000000
> x9 : 0000000000000000 x8 : ffff800015003e50
> x7 : 0000000000000002 x6 : 00000000380b9990
> x5 : ffff8000106e99e8 x4 : ffff0000fadd83c0
> x3 : 0000ffffffffffff x2 : ffff800011b6a0d8
> x1 : ffff800011b6a000 x0 : ffff80001130c9d8
> Call trace:
>  el1_irq+0x78/0x180
>  printk+0x0/0x84
>  write_sysrq_trigger+0xb0/0x118
>  proc_reg_write+0xb4/0xe0
>  __vfs_write+0x18/0x40
>  vfs_write+0xb0/0x1b8
>  ksys_write+0x64/0xf0
>  __arm64_sys_write+0x14/0x20
>  el0_svc_common.constprop.2+0xb0/0x168
>  do_el0_svc+0x20/0x98
>  el0_sync_handler+0xec/0x1a8
>  el0_sync+0x140/0x180
>
> [3]kdb>
>
> After the patch:
> Entering kdb (current=0xffff8000119e2dc0, pid 0) on processor 0 due to Keyboard Entry
> [0]kdb> bp printk
> Instruction(i) BP #0 at 0xffff8000101486cc (printk)
>     is enabled   addr at ffff8000101486cc, hardtype=0 installed=0
>
> [0]kdb> g
>
> / # echo h > /proc/sysrq-trigger
>
> Entering kdb (current=0xffff0000fa852bc0, pid 268) on processor 0 due to Breakpoint @ 0xffff8000101486cc
> [0]kdb> g
>
> Entering kdb (current=0xffff0000fa852bc0, pid 268) on processor 0 due to Breakpoint @ 0xffff8000101486cc
> [0]kdb> ss
>
> Entering kdb (current=0xffff0000fa852bc0, pid 268) on processor 0 due to SS trap @ 0xffff800010082ab8
> [0]kdb>
>
> Fixes: 44679a4f142b ("arm64: KGDB: Add step debugging support")
> Signed-off-by: Wei Li <liwei391@huawei.com>
> ---
>  arch/arm64/kernel/kgdb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

As far as I can tell this looks right.  Specifically in kdb_stub() I
see that it needs "ks->ex_vector == 0" to get "reason" as
KDB_REASON_SSTEP.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

I did really basic testing of this series as a whole and things that
used to be broken for me around stepping / going past breakpoints are
now fixed.  I'll keep using it more, but it definitely feels like it
makes things happier.

Tested-by: Douglas Anderson <dianders@chromium.org>

-Doug