Re: [PATCH v2 08/11] arm64: debug: split hardware watchpoint exception entry

[Will Deacon <will@kernel.org>]

On Wed, May 28, 2025 at 04:42:46PM +0100, Mark Rutland wrote:
> On Wed, May 28, 2025 at 02:47:52PM +0100, Ada Couprie Diaz wrote:
> > On 20/05/2025 17:59, Will Deacon wrote:
> > 
> > > On Mon, May 12, 2025 at 06:43:23PM +0100, Ada Couprie Diaz wrote:
> > > > [...]
> > > > diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
> > > > index 8814ad24e707..6e70130d2741 100644
> > > > --- a/arch/arm64/kernel/entry-common.c
> > > > +++ b/arch/arm64/kernel/entry-common.c
> > > > @@ -530,10 +530,20 @@ static void noinstr el1_softstp(struct pt_regs *regs, unsigned long esr)
> > > >   	arm64_exit_el1_dbg(regs);
> > > >   }
> > > > -static void noinstr el1_dbg(struct pt_regs *regs, unsigned long esr)
> > > > +static void noinstr el1_watchpt(struct pt_regs *regs, unsigned long esr)
> > > >   {
> > > > +	/* Only watchpoints write FAR_EL1 */
> > > nit: But maybe scope the comment (here and in the el0 handler) for debug
> > > exceptions?
> > > e.g.
> > > 
> > > 	/* Watchpoints are the only debug exception to write FAR_EL1 */
> > > 
> > > ?
> > > 
> > 
> > Good point. The comment felt somewhat off to me and that's exactly why.
> > 
> > Updated the wording for v3, thanks.
> 
> More of a question for Will, but could we drop the comment entirely?
> 
> Historically the same comment in the common el0_dbg() function was a
> useful warning because we were forced to read FAR even for
> non-watchpoints, but as of this restructuring that awkwardness is gone.
> 
> As of this series, for the other debug exceptions, the
> el{0,1}_${EXCEPTIONNAME}() handlers don't currently read FAR, and their
> callees don't have a 'far' argument.
> 
> I'm happy either way, in case you'd prefer that it stays.

I'd prefer to keep it, just in case somebody smart reworks the code and
we end up trying to use the FAR for other debug exceptions.

Will