From: takahiro.akashi@linaro.org (AKASHI Takahiro)
To: linux-arm-kernel@lists.infradead.org
Subject: [RESEND PATCH] arm64: kgdb: fix single stepping
Date: Thu, 15 Sep 2016 16:56:18 +0900 [thread overview]
Message-ID: <20160915075617.GJ16712@linaro.org> (raw)
In-Reply-To: <20160914145850.GE19622@arm.com>
On Wed, Sep 14, 2016 at 03:58:51PM +0100, Will Deacon wrote:
> 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.
I'm happy, too.
> 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?
Sure, but I need to refresh my memory here.
-Takahiro AKASHI
> Thanks,
>
> Will
WARNING: multiple messages have this Message-ID (diff)
From: AKASHI Takahiro <takahiro.akashi@linaro.org>
To: Will Deacon <will.deacon@arm.com>
Cc: "jason.wessel@windriver.com" <jason.wessel@windriver.com>,
Catalin Marinas <Catalin.Marinas@arm.com>,
"broonie@kernel.org" <broonie@kernel.org>,
"yong.zhao@amd.com" <yong.zhao@amd.com>,
"Vijaya.Kumar@caviumnetworks.com"
<Vijaya.Kumar@caviumnetworks.com>,
"kgdb-bugreport@lists.sourceforge.net"
<kgdb-bugreport@lists.sourceforge.net>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linaro-kernel@lists.linaro.org" <linaro-kernel@lists.linaro.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RESEND PATCH] arm64: kgdb: fix single stepping
Date: Thu, 15 Sep 2016 16:56:18 +0900 [thread overview]
Message-ID: <20160915075617.GJ16712@linaro.org> (raw)
In-Reply-To: <20160914145850.GE19622@arm.com>
On Wed, Sep 14, 2016 at 03:58:51PM +0100, Will Deacon wrote:
> 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.
I'm happy, too.
> 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?
Sure, but I need to refresh my memory here.
-Takahiro AKASHI
> Thanks,
>
> Will
next prev parent reply other threads:[~2016-09-15 7:56 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-21 1:13 [RESEND PATCH] arm64: kgdb: fix single stepping AKASHI Takahiro
2015-04-21 1:13 ` AKASHI Takahiro
2016-09-14 12:48 ` Giuseppe CAVALLARO
2016-09-14 14:58 ` Will Deacon
2016-09-14 14:58 ` Will Deacon
2016-09-15 7:56 ` AKASHI Takahiro [this message]
2016-09-15 7:56 ` AKASHI Takahiro
2016-09-15 10:41 ` Daniel Thompson
2016-09-15 10:41 ` Daniel Thompson
2016-09-15 12:32 ` Jason Wessel
2016-09-15 12:32 ` Jason Wessel
2016-09-15 13:04 ` Jason Wessel
2016-09-15 13:04 ` Jason Wessel
2016-09-16 4:32 ` AKASHI Takahiro
2016-09-16 4:32 ` AKASHI Takahiro
2016-09-16 7:45 ` Will Deacon
2016-09-16 7:45 ` Will Deacon
2016-09-19 22:25 ` Jason Wessel
2016-09-19 22:25 ` Jason Wessel
2016-09-19 22:29 ` Jason Wessel
2016-09-19 22:29 ` Jason Wessel
2016-09-20 8:43 ` AKASHI Takahiro
2016-09-20 8:43 ` AKASHI Takahiro
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160915075617.GJ16712@linaro.org \
--to=takahiro.akashi@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.