linux-kernel - Re: [GIT PULL] Generic entry for ARM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4157fe23-8be8-4fd1-a69a-c59383b9516d@paulmck-laptop>
Date: Mon, 7 Apr 2025 10:00:54 -0700
From: "Paul E. McKenney" <paulmck@...nel.org>
To: Linus Walleij <linus.walleij@...aro.org>
Cc: Russell King <rmk+kernel@...linux.org.uk>,
	Linux ARM <linux-arm-kernel@...ts.infradead.org>,
	Arnd Bergmann <arnd@...db.de>, Thomas Gleixner <tglx@...utronix.de>,
	linux-kernel <linux-kernel@...r.kernel.org>, frederic@...nel.org,
	jpoimboe@...nel.org, peterz@...radead.org
Subject: Re: [GIT PULL] Generic entry for ARM

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@...nel.org> 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