[PATCH v2 10/12] drivers/firmware/scmi: disable ftrace for Clang Thumb2 builds

[Ard Biesheuvel]

On Mon, 31 Jan 2022 at 19:37, Nick Desaulniers <ndesaulniers at google.com> wrote:
>
> On Mon, Jan 31, 2022 at 9:04 AM Ard Biesheuvel <ardb at kernel.org> wrote:
> >
> > The SMC calling convention designates R0-R7 as input registers in
> > AArch32 mode, and this conflicts with the compiler's use of R7 as a
> > frame pointer when building in Thumb2 mode. Generally, we don't enable
> > the frame pointer, and GCC happily enables the -pg profiling hooks
> > without them. However, Clang refuses, and errors out with the message
> > below:
> >
> > drivers/firmware/arm_scmi/smc.c:152:2: error: write to reserved register 'R7'
> >         arm_smccc_1_1_invoke(scmi_info->func_id, 0, 0, 0, 0, 0, 0, 0, &res);
> >         ^
> > include/linux/arm-smccc.h:550:4: note: expanded from macro 'arm_smccc_1_1_invoke'
> >                         arm_smccc_1_1_smc(__VA_ARGS__);                 \
> >                         ^
> > Let's just disable ftrace for the compilation unit when building this
> > configuration.
>
> Thanks for the patch!
> Reviewed-by: Nick Desaulniers <ndesaulniers at google.com>
>
> Would it make sense for clang not to warn in such cases? (i.e. write
> to r7 for thumb when -fomit-frame-pointers is specified) If so, we can
> file a feature request and at least add a link in the comment added in
> the Makefile.
>

This is a bit tricky. The ftrace code does not rely on a frame pointer
at all, which is why this is only an issue on Clang, as GCC happily
ignores the use of R7. On Clang, the frame pointer is mandatory with
-pg:

clang: error: invalid argument '-fomit-frame-pointer' not allowed with '-pg'

However, the way the unwinder is wired up in Linux means that only R7
is usable as an unwind anchor (apart from SP), which means that
clobbering R7 even temporarily is risky. Note that the same applies to
FP in places where we preserve/restore it currently. The fact that -pg
implies -fno-omit-frame-pointer on Clang makes this even worse,
because it means that all emitted unwind info will use R7 rather than
SP.

So it would help if we could relax the requirement for
-fno-omit-frame-pointer with -pg on ARM (provided that there is no
reason it is needed that I have missed). That would improve codegen as
well.

But it doesn't fix the underlying issue that clobbering R7 is risky,
and we should whether the clobber is sufficient to discourage the
compiler from using it as the unwind anchor. If this is the case, we
should probably pass -fno-unwind-tables to each function that uses R7
or FP in this manner.


> >
> > Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> > ---
> >  drivers/firmware/arm_scmi/Makefile | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
> > index 1dcf123d64ab..52b9078bfe96 100644
> > --- a/drivers/firmware/arm_scmi/Makefile
> > +++ b/drivers/firmware/arm_scmi/Makefile
> > @@ -11,3 +11,10 @@ scmi-module-objs := $(scmi-bus-y) $(scmi-driver-y) $(scmi-protocols-y) \
> >                     $(scmi-transport-y)
> >  obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o
> >  obj-$(CONFIG_ARM_SCMI_POWER_DOMAIN) += scmi_pm_domain.o
> > +
> > +ifeq ($(CONFIG_THUMB2_KERNEL)$(CONFIG_CC_IS_CLANG),yy)
> > +# The use of R7 in the SMCCC conflicts with the compiler's use of R7 as a frame
> > +# pointer in Thumb2 mode, which is forcibly enabled by Clang when profiling
> > +# hooks are inserted via the -pg switch.
> > +CFLAGS_REMOVE_smc.o += $(CC_FLAGS_FTRACE)
> > +endif
> > --
> > 2.30.2
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers