Re: [PATCH] arm64: ftrace: add missing BTIs

From: Mark Rutland <mark.rutland@arm.com>
To: Mark Brown <broonie@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com,
	will@kernel.org
Subject: Re: [PATCH] arm64: ftrace: add missing BTIs
Date: Thu, 2 Dec 2021 10:43:39 +0000	[thread overview]
Message-ID: <YaijW+BII2OG/CG/@FVFF77S0Q05N> (raw)
In-Reply-To: <YaUSfpRsm2ZjeudZ@sirena.org.uk>

On Mon, Nov 29, 2021 at 05:48:46PM +0000, Mark Brown wrote:
> On Mon, Nov 29, 2021 at 05:37:51PM +0000, Mark Rutland wrote:
> > On Mon, Nov 29, 2021 at 04:50:39PM +0000, Mark Brown wrote:
> 
> > > > +#ifdef BTI_C
> > > > +	BTI_C
> > > > +#endif
> 
> > > The ifdefs here feel ugly enough that it might be worth doing that right
> > > now TBH.  I'm trying to think of any cases where we might also need a
> > > BTI J but nothing springs to mind right now.
> 
> > Agreed on the ugliness -- I'd like to revisit that with some related
> > cleanup/improvement to our existing SYM_*() macros. I just didn't want to do
> > that as a prerequisite for the fix as it'd make backports painful, e.g. by
> > creating a dependency on commit:
> 
> >   1cbdf60bd1b74e39 ("kasan: arm64: support specialized outlined tag mismatch checks")
> 
> > ... which uses the ifdef pattern above.
> 
> Ugh, that's not great.  But yes, backporting was why I did the Reviwed-by.

Sure, and thanks for that!

> If we knew the names we were going for it'd be easy enough to add a new
> SYM_CODE_ varaint without introducing dependencies but that might be
> getting ahead of things.
> 
> > I'm also not sure what naming/structure we'd like, or whether it's simpler to
> > unconditionally define BTI_C.
> 
> The unconditional definition seems like it might be neater as a
> minimally intrusive thing for backporting, we can always refactor later.

The problem with that is that for consistency we should remove the ifdeferry
from the kasan trampoline at the same time, which either means an unnecessary
dependency as above, or splitting this into three patches, one of which should
not be backported.

Given that, I think that as-is this is the simplest form for backporting.

I completely agree the ifdeferry is ugly, and I do intend to follow up on that
regardless. If you feel strongly that we should get rid of that now, I can spin
a v2 with a second patch to clean that up; otherwise I'll do that as part of a
later series.

Thanks,
Mark.

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