Re: [RESEND PATCH] arm64: kgdb: fix single stepping

From: Will Deacon
Date: Wed Sep 14 2016 - 10:59:00 EST


Hi Akashi,

On Tue, Apr 21, 2015 at 02:13:13AM +0100, AKASHI Takahiro wrote:
> Could you please review my patch below?
> See also arm64 maintainer's comment:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/313712.html

-ETIMEDOUT waiting for the kdgb folk to comment. Ppeople have reported
that this patch is required for kgdb to work correctly on arm64, so I'm
happy to merge it.

However, as detailed in your comment log:

> This patch
> (1) moves kgdb_disable_single_step() from 'c' command handling to single
> step handler.
> This makes sure that single stepping gets effective at every 's' command.
> Please note that, under the current implementation, single step bit in
> spsr, which is cleared by the first single stepping, will not be set
> again for the consecutive 's' commands because single step bit in mdscr
> is still kept on (that is, kernel_active_single_step() in
> kgdb_arch_handle_exception() is true).
> (2) re-implements kgdb_roundup_cpus() because the current implementation
> enabled interrupts naively. See below.
> (3) removes 'enable_dbg' in el1_dbg.
> Single step bit in mdscr is turned on in do_handle_exception()->
> kgdb_handle_expection() before returning to debugged context, and if
> debug exception is enabled in el1_dbg, we will see unexpected single-
> stepping in el1_dbg.
> Since v3.18, the following patch does the same:
> commit 1059c6bf8534 ("arm64: debug: don't re-enable debug exceptions
> on return from el1_dbg)
> (4) masks interrupts while single-stepping one instruction.
> If an interrupt is caught during processing a single-stepping, debug
> exception is unintentionally enabled by el1_irq's 'enable_dbg' before
> returning to debugged context.
> Thus, like in (2), we will see unexpected single-stepping in el1_irq.

this patch is doing *far* too much in one go. Could you please repost it
as a series of self-contained fixes with clear commit messages, so I can
queue them and cc stable where appropriate?

Thanks,

Will