From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934282AbcIOHtb (ORCPT ); Thu, 15 Sep 2016 03:49:31 -0400 Received: from mail-pa0-f44.google.com ([209.85.220.44]:35857 "EHLO mail-pa0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756452AbcIOHt1 (ORCPT ); Thu, 15 Sep 2016 03:49:27 -0400 Date: Thu, 15 Sep 2016 16:56:18 +0900 From: AKASHI Takahiro To: Will Deacon Cc: "jason.wessel@windriver.com" , Catalin Marinas , "broonie@kernel.org" , "yong.zhao@amd.com" , "Vijaya.Kumar@caviumnetworks.com" , "kgdb-bugreport@lists.sourceforge.net" , "linux-arm-kernel@lists.infradead.org" , "linaro-kernel@lists.linaro.org" , "linux-kernel@vger.kernel.org" Subject: Re: [RESEND PATCH] arm64: kgdb: fix single stepping Message-ID: <20160915075617.GJ16712@linaro.org> Mail-Followup-To: AKASHI Takahiro , Will Deacon , "jason.wessel@windriver.com" , Catalin Marinas , "broonie@kernel.org" , "yong.zhao@amd.com" , "Vijaya.Kumar@caviumnetworks.com" , "kgdb-bugreport@lists.sourceforge.net" , "linux-arm-kernel@lists.infradead.org" , "linaro-kernel@lists.linaro.org" , "linux-kernel@vger.kernel.org" References: <1429578793-3971-1-git-send-email-takahiro.akashi@linaro.org> <20160914145850.GE19622@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-2022-jp Content-Disposition: inline In-Reply-To: <20160914145850.GE19622@arm.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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