Re: [GIT PULL] Generic entry for ARM

[Paul E. McKenney]

On Mon, Mar 31, 2025 at 11:48:40PM +0200, Linus Walleij wrote:
> On Wed, Mar 12, 2025 at 7:00 PM Paul E. McKenney <paulmck@xxxxxxxxxx> wrote:
> 
> > Once you are confident that you have all the needed "noinstr"
> > and "__always_inline" instances in place, could you please add
> > ARCH_WANTS_NO_INSTR to the list of "select" clauses for "config ARM"
> > in arch/arm/Kconfig?
> 
> I would love to do that, I'm just not sure what this really entails.

I freely confess that I have only seen this done, not done it myself.

> Surely this patchset tags a noinstr on every entry point from
> exceptions and syscall software interrupts.
> Documentation/core-api/entry.rst is pretty good at explaining this.
> 
> But what makes me uncertain are things that are tagged
> "notrace", such as void notrace cpu_init(void) - surely we
> don't trace, but should that be "noinstr"? It's even marked
> "notrace" but not "noinstr" in arm64.

I know that __always_inline does the trick, but I am not sure about
notrace (my guess is "no" on notrace).  I added the objtool maintainers
on CC.

No argument on the usefulness of documentation.  ;-)

							Thanx, Paul

> 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?
> 
> sched_clock_noinstr() is tagged noinstr and sched_clock() is
> tagged notrace, so there must be a difference here.
> 
> secondary_start_kernel() is tagged "notrace" on arm64 but
> not on arm, should it be tagged "noinstr" or "notrace"?
> 
> This kind of stuff makes me uncertain about how this is to be
> done. A "noinstr vs notrace" section in Documentation/core-api/entry.rst
> would help a lot I think!
> 
> Yours,
> Linus Walleij