Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8;
 format=flowed
Content-Transfer-Encoding: 8bit
Date:   Tue, 31 Mar 2020 19:33:38 +0800
From:   tingwei@codeaurora.org
To:     Will Deacon <will@kernel.org>
Cc:     Mark Rutland <mark.rutland@arm.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] arm64: hw_breakpoint: don't clear debug registers in halt
 mode
In-Reply-To: <20200331074147.GA25612@willie-the-truck>
References: <20200328083209.21793-1-tingwei@codeaurora.org>
 <20200330123946.GH1309@C02TD0UTHF1T.local>
 <20200330134218.GB10633@willie-the-truck>
 <2f4d076b2b21de3908f0821126d0c61e@codeaurora.org>
 <20200331074147.GA25612@willie-the-truck>
Message-ID: <518d9ca9652c23bfc0e1831306144418@codeaurora.org>
User-Agent: Roundcube Webmail/1.3.9
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

在 2020-03-31 15:41，Will Deacon 写道：
> On Tue, Mar 31, 2020 at 10:39:42AM +0800, tingwei@codeaurora.org wrote:
>> 在 2020-03-30 21:42，Will Deacon 写道：
>> > On Mon, Mar 30, 2020 at 01:39:46PM +0100, Mark Rutland wrote:
>> > > On Sat, Mar 28, 2020 at 04:32:09PM +0800, Tingwei Zhang wrote:
>> > > > If external debugger sets a breakpoint for one Kernel function
>> > > > when device is in bootloader mode and loads Kernel, this breakpoint
>> > > > will be wiped out in hw_breakpoint_reset(). To fix this, check
>> > > > MDSCR_EL1.HDE in hw_breakpoint_reset(). When MDSCR_EL1.HDE is
>> > > > 0b1, halting debug is enabled. Don't reset debug registers in this
>> > case.
>> > >
>> > > I don't think this is sufficient, because the kernel can still
>> > > subsequently mess with breakpoints, and the HW debugger might not be
>> > > attached at this point in time anyhow.
>> > >
>> > > I reckon this should hang off the existing "nodebumon" command line
>> > > option, and we shouldn't use HW breakpoints at all when that is
>> > > passed.
>> > > Then you can pass that to prevent the kernel stomping on the external
>> > > debugger.
>> > >
>> > > Will, thoughts?
>> >
>> > I was going to suggest the same thing, although we will also need to
>> > take
>> > care to reset the registers if "nodebugmon" is toggled at runtime via
>> > the
>> > "debug_enabled" file in debugfs.
>> >
>> Thanks for the suggestion, Mark and Will. It's a great idea to use
>> "nodebugmon". When "nodebugmon" is set, Kernel won't change HW
>> breakpoints.
>> 
>> For reset the registers after "debug_enabled" is toggled, I'm thinking 
>> if
>> we are adding unnecessary complexity here.If we take that approach, we
>> will
>> hook "debug_enabled" interface and use smp_call_function_single() to 
>> call
>> hw_breakpoint_reset() on each CPU. Wait for all CPUs' execution done 
>> and
>> change "debug_enabled". External debugger would clear the breakpoints 
>> when
>> it detaches the device and restores its breakpoints when attaches the
>> device.
>> Assume debug_enabled is changed to one after external debugger 
>> detaches
>> the
>> device. Debugger would already clear the breakpoint registers. If 
>> debgger
>> is
>> still attached, there's nothing Kernel can do to stop it 
>> restores/programs
>> the breakpoint registers.
>> 
>> What do you think of this?
> 
> It's all a bit of a mess. Looking at it some more, why can't the 
> external
> debugger simply trap access to the debug registers using EDSCR.TDA? 
> That
> way, we don't have to change anything in the kernel.
> 
> Will

External debugger has the function to trap access to debug registers 
now.
What do we expect debugger to do after core is stopped? Skip that msr
instruction and continue to run?

Tingwei