Re: [PATCH] arm64: asmlinkage: Enable use of BTI_C macro in SYM_CODE

From: Mark Brown <broonie@kernel.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	Ard Biesheuvel <ardb@kernel.org>
Subject: Re: [PATCH] arm64: asmlinkage: Enable use of BTI_C macro in SYM_CODE
Date: Mon, 6 Dec 2021 19:31:43 +0000	[thread overview]
Message-ID: <Ya5lH38JrBkPbsQu@sirena.org.uk> (raw)
In-Reply-To: <Ya5YttvIclJLDAfp@FVFF77S0Q05N>

[-- Attachment #1.1: Type: text/plain, Size: 2719 bytes --]

On Mon, Dec 06, 2021 at 06:38:46PM +0000, Mark Rutland wrote:
> On Mon, Dec 06, 2021 at 06:05:44PM +0000, Mark Brown wrote:
> > On Mon, Dec 06, 2021 at 04:30:41PM +0000, Mark Rutland wrote:

> > If you mean that the macro should unconditionally emit BTI when the
> > macro is used then the main reason I didn't do that was for consistency
> > with C code - that will only have BTIs added if we're building with BTI
> > enabled.  There is at least one implementation out there which implements
> > unknown HINTs with a performance overhead compared to NOPs so that is
> > one of the reasons why the kernel might be built with BTI disabled.

> I appreciate that rationale; I'm saying we should re-evaluate that.

...

> If this does actually matter in practice, then I'm happy to make it
> conditional, I just think we should err on the side of simplicity otherwise.
> Especially as we already have on case where we add the instructions
> unconditionally, in what is arguably fast-path code!

Hrm.  I'm not sure I see that as a substantial win TBH - it does make
the assembler output more consistent but it's also not what I'd expect
to happen if we turn off BTI for the kernel.  I guess it's just the
usage in the AES code that's making you think of this - all the issues
have been missing landing pads in SYM_CODE?  

I'm not exactly against it, I'm just unclear on the upside which makes
me feel like I'm missing something.  It feels like this comes from the
current conflation of conditionally providing the macro with
conditionally using it in sites where it can be omitted.  My instinct is
more to do something like define your BTI C macro unconditionally but
still have a conditional thing like we have currently at least for
SYM_FUNC, possibly renamed though.

Actually now I think about it we may also want to think about pointer
auth for SYM_FUNC, but that's definitely a separate and more substantial
thing that needs much more consideration and is out of scope here.

> > Yes, even seems to do the right thing without complaint if the assembler
> > is configured v8.5 and so understands v8.5.  My main worry would be the
> > potential confusion caused when people see what looks like it should be
> > a v8.5 instruction in code which is supposed to build configured for
> > v8.0, it looks wrong to see something that looks like a more modern
> > instruction in code that should be buildable with a toolchain that
> > doesn't understand it.

> Sure, but that's already the case for SB and ESB, so I'm not sure that matters.

Oh, ick.  I may perhaps be more sensitive to this than most due to
working with the FP code which has more hand assembly than average.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel