Re: [PATCH v2 05/11] arm64: entry: Add entry and exit functions for debug exceptions

[Will Deacon <will@kernel.org>]

On Wed, May 28, 2025 at 03:08:35PM +0100, Ada Couprie Diaz wrote:
> On 20/05/2025 16:36, Will Deacon wrote:
> 
> > On Mon, May 12, 2025 at 06:43:20PM +0100, Ada Couprie Diaz wrote:
> > > Duplicate the `debug_exception_entry()` and `debug_exception_exit()`
> > nit: it's debug_exception_enter().
> Thanks,fixed for v3.
> > > functions from mm/fault.c, as they are needed to split
> > > the debug exceptions entry paths from the current unified one.
> > > The original will be removed in a cleanup patch.
> > > 
> > > Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
> > > ---
> > >   arch/arm64/kernel/entry-common.c | 22 ++++++++++++++++++++++
> > >   1 file changed, 22 insertions(+)
> > > 
> > > diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
> > > index b260ddc4d3e9..92d78b329e67 100644
> > > --- a/arch/arm64/kernel/entry-common.c
> > > +++ b/arch/arm64/kernel/entry-common.c
> > > @@ -418,6 +418,28 @@ static inline void fp_user_discard(void)
> > >   	}
> > >   }
> > > +/*
> > > + * In debug exception context, we explicitly disable preemption despite
> > > + * having interrupts disabled.
> > > + * This serves two purposes: it makes it much less likely that we would
> > > + * accidentally schedule in exception context and it will force a warning
> > > + * if we somehow manage to schedule by accident.
> > > + */
> > > +static void debug_exception_enter(struct pt_regs *regs)
> > > +{
> > > +	preempt_disable();
> > > +
> > > +	/* This code is a bit fragile.  Test it. */
> > > +	RCU_LOCKDEP_WARN(!rcu_is_watching(), "exception_enter didn't work");
> > > +}
> > > +NOKPROBE_SYMBOL(debug_exception_enter);
> > > +
> > > +static void debug_exception_exit(struct pt_regs *regs)
> > > +{
> > > +	preempt_enable_no_resched();
> > > +}
> > > +NOKPROBE_SYMBOL(debug_exception_exit);
> > Could you make these externally visible (i.e. drop the 'static') so that
> > you can remove the definitions from mm/fault.c at the same time? Then
> > you could add the 'static' back at the end when there are no more
> > external callers.
> > 
> If you are happy with some header noise, adding `debug_exception_enter()`
> and `debug_exception_exit()` to include/asm/exception.h` so that they can
> be visible in `mm/fault.c` during the clean up and removing them when
> done, I would much prefer doing it that way !
> 
> It felt strange to just have some duplicate code, but I didn't know
> what would be acceptable.
> I will make the change for v3, unless the header noise mentioned above is
> an issue.

Thanks, that sounds great.

Will