Re: [GIT PULL] Generic entry for ARM

[Peter Zijlstra <peterz@infradead.org>]

On Mon, Apr 07, 2025 at 04:47:05PM -0700, Josh Poimboeuf wrote:
> I think Thomas, Peter and Steven are the experts here, but I'll give my
> limited understanding.
> 
> On Mon, Apr 07, 2025 at 10:00:54AM -0700, Paul E. McKenney wrote:
> > > cpu_init() is called from e.g.:
> > > asmlinkage void secondary_start_kernel(struct task_struct *task)
> > > OK should this also be noinstr? Or is that just implied because
> > > of asmlinkage?
> > >
> > > <linux/compiler_types.h> will resolve to:
> > > 
> > > #if defined(CC_USING_HOTPATCH)
> > > #define notrace                 __attribute__((hotpatch(0, 0)))
> > > #elif defined(CC_USING_PATCHABLE_FUNCTION_ENTRY)
> > > #define notrace                 __attribute__((patchable_function_entry(0, 0)))
> > > #else
> > > #define notrace                 __attribute__((__no_instrument_function__))
> > > #endif
> > > 
> > > which I read as three different ways of saying "don't patch here".
> > > 
> > > Which is confusingly similar or identical to what noinstr does, I do see that
> > > noinstr pushes the code to separate section but that in turn might
> > > be what __attribute__((__no_instrument_function__)) and
> > > friends does?
> > > 
> > > Are they equivalent?
> 
> noinstr is much broader than notrace.  No instrumentation of any kind
> (ftrace, kprobes, sanitizers, profilers, etc), in the prologue or body
> of the function or its callees, except for regions marked by
> instrumentation_{begin,end}().

Note that noinstr code goes into its own section and various probing
mechanisms (HW breakpoints, kprobes etc..) are explicitly excluded from
touching code in this section.

> noinstr is needed in early/late entry code before/after RCU transitions,
> as instrumentation code might depend on RCU.  Also, entry code tends to
> be sensitive and unable to handle random instrumentation code.

Not strictly RCU, but including RCU. Some of the early entry code might
not have a sane stack or anything much a 'C' environment relies on,
could even not have normal page-tables set up.

noinstr is very much an attempt to stretch the whole 'C-as-assember'
ethos. Only emit the code I write, don't play silly buggers.

Current noinstr usage is indeed interrupt/exception/syscall entry but
also idle loops. Various idle routines disable RCU (which then
prohibits tracing) but some go even further and dismantle much of what a
normal C environment would expect.

> I believe noinstr is not typically needed for boot code, at least for
> the primary CPU.  RCU isn't present yet, and many instrumentations might
> not yet be available.  Or their handlers can detect whether they've been
> initialized yet.  Or they're disabled in .init sections.
> 
> Whether noinstr is needed for *secondary* boot code, I don't know.  It
> may be dependent on the instrumentation implementations, e.g., whether
> they're enabled globally or per-CPU.  At least on x86, start_secondary()
> is notrace.  I don't know enough to say whether that's sufficient.

There might be a case for noinstr there, but as of yet, nobody went and
audited all that. Thus far, I've been the idiot doing all the auditing,
but I think I'll pass on this one for now :-)