| Message ID | 20200221180901.15812-1-tony@atomide.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: OMAP2+: Fix compile if CONFIG_HAVE_ARM_SMCCC is not set | expand |
On 2/21/20 1:09 PM, Tony Lindgren wrote: > Recent omap changes added runtime checks to use omap_smccc_smc() > when optee is configured in dts. As the omap-secure code can be > built for ARMv6 only without ARMv7 and use custom smc calls, we > now get a build error: > > omap-secure.c:(.text+0x94): undefined reference to `__arm_smccc_smc' > > Let's just ifdef out omap_smccc_smc() unless the CPU has selected > CONFIG_HAVE_ARM_SMCCC. The other option discussed was to add an > inline function to arm-smccc.h, but we'd still also have to add > ifdef around omap_smccc_smc() to avoid a warning for uninitialized > value for struct arm_smccc_res in omap_smccc_smc(). And we probably > should not start initializing values in arm-smccc.h if disabled. > > Let's also warn on trying to use omap_smccc_smc() if disabled as > suggested by Andrew F. Davis <afd@ti.com>. > > Fixes: 48840e16c299 ("ARM: OMAP2+: Use ARM SMC Calling Convention when OP-TEE is available") > Reported-by: kbuild test robot <lkp@intel.com> > Cc: Aaro Koskinen <aaro.koskinen@iki.fi> > Cc: Andrew F. Davis <afd@ti.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Marc Zyngier <maz@kernel.org> > Cc: Rob Herring <robh@kernel.org> > Cc: Russell King <rmk+kernel@arm.linux.org.uk> > Cc: Steven Price <steven.price@arm.com> > Cc: Will Deacon <will@kernel.org> > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- Looks good to me, Acked-by: Andrew F. Davis <afd@ti.com> > arch/arm/mach-omap2/omap-secure.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/arm/mach-omap2/omap-secure.c b/arch/arm/mach-omap2/omap-secure.c > --- a/arch/arm/mach-omap2/omap-secure.c > +++ b/arch/arm/mach-omap2/omap-secure.c > @@ -77,6 +77,7 @@ u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs, u32 arg1, u32 arg2, > return ret; > } > > +#ifdef CONFIG_HAVE_ARM_SMCCC > void omap_smccc_smc(u32 fn, u32 arg) > { > struct arm_smccc_res res; > @@ -85,6 +86,12 @@ void omap_smccc_smc(u32 fn, u32 arg) > 0, 0, 0, 0, 0, 0, &res); > WARN(res.a0, "Secure function call 0x%08x failed\n", fn); > } > +#else > +void omap_smccc_smc(u32 fn, u32 arg) > +{ > + WARN_ONCE(1, "smccc is disabled\n"); > +} > +#endif > > void omap_smc1(u32 fn, u32 arg) > { >
On Fri, Feb 21, 2020 at 7:09 PM Tony Lindgren <tony@atomide.com> wrote: > > Recent omap changes added runtime checks to use omap_smccc_smc() > when optee is configured in dts. As the omap-secure code can be > built for ARMv6 only without ARMv7 and use custom smc calls, we > now get a build error: > > omap-secure.c:(.text+0x94): undefined reference to `__arm_smccc_smc' > > Let's just ifdef out omap_smccc_smc() unless the CPU has selected > CONFIG_HAVE_ARM_SMCCC. The other option discussed was to add an > inline function to arm-smccc.h, but we'd still also have to add > ifdef around omap_smccc_smc() to avoid a warning for uninitialized > value for struct arm_smccc_res in omap_smccc_smc(). And we probably > should not start initializing values in arm-smccc.h if disabled. > > Let's also warn on trying to use omap_smccc_smc() if disabled as > suggested by Andrew F. Davis <afd@ti.com>. > > +#ifdef CONFIG_HAVE_ARM_SMCCC > void omap_smccc_smc(u32 fn, u32 arg) > { > struct arm_smccc_res res; > @@ -85,6 +86,12 @@ void omap_smccc_smc(u32 fn, u32 arg) > 0, 0, 0, 0, 0, 0, &res); > WARN(res.a0, "Secure function call 0x%08x failed\n", fn); > } > +#else Looking through the callers again, this can and is only called for ARMv7 CPUs, so can't you just avoid the #else path and turn it into a link error if we ever get callers on ARMv6-only builds? Arnd
* Arnd Bergmann <arnd@arndb.de> [200221 20:06]: > On Fri, Feb 21, 2020 at 7:09 PM Tony Lindgren <tony@atomide.com> wrote: > > > > Recent omap changes added runtime checks to use omap_smccc_smc() > > when optee is configured in dts. As the omap-secure code can be > > built for ARMv6 only without ARMv7 and use custom smc calls, we > > now get a build error: > > > > omap-secure.c:(.text+0x94): undefined reference to `__arm_smccc_smc' > > > > Let's just ifdef out omap_smccc_smc() unless the CPU has selected > > CONFIG_HAVE_ARM_SMCCC. The other option discussed was to add an > > inline function to arm-smccc.h, but we'd still also have to add > > ifdef around omap_smccc_smc() to avoid a warning for uninitialized > > value for struct arm_smccc_res in omap_smccc_smc(). And we probably > > should not start initializing values in arm-smccc.h if disabled. > > > > Let's also warn on trying to use omap_smccc_smc() if disabled as > > suggested by Andrew F. Davis <afd@ti.com>. > > > > +#ifdef CONFIG_HAVE_ARM_SMCCC > > void omap_smccc_smc(u32 fn, u32 arg) > > { > > struct arm_smccc_res res; > > @@ -85,6 +86,12 @@ void omap_smccc_smc(u32 fn, u32 arg) > > 0, 0, 0, 0, 0, 0, &res); > > WARN(res.a0, "Secure function call 0x%08x failed\n", fn); > > } > > +#else > > Looking through the callers again, this can and is only called > for ARMv7 CPUs, so can't you just avoid the #else path and > turn it into a link error if we ever get callers on ARMv6-only builds? Hmm yeah maybe. If ARMv6 machines (basically n8x0) is not using omap-smc.S at all, it's unlike it would need omap-secure.c either. Regards, Tony
* Tony Lindgren <tony@atomide.com> [200221 20:16]: > * Arnd Bergmann <arnd@arndb.de> [200221 20:06]: > > On Fri, Feb 21, 2020 at 7:09 PM Tony Lindgren <tony@atomide.com> wrote: > > > > > > Recent omap changes added runtime checks to use omap_smccc_smc() > > > when optee is configured in dts. As the omap-secure code can be > > > built for ARMv6 only without ARMv7 and use custom smc calls, we > > > now get a build error: > > > > > > omap-secure.c:(.text+0x94): undefined reference to `__arm_smccc_smc' > > > > > > Let's just ifdef out omap_smccc_smc() unless the CPU has selected > > > CONFIG_HAVE_ARM_SMCCC. The other option discussed was to add an > > > inline function to arm-smccc.h, but we'd still also have to add > > > ifdef around omap_smccc_smc() to avoid a warning for uninitialized > > > value for struct arm_smccc_res in omap_smccc_smc(). And we probably > > > should not start initializing values in arm-smccc.h if disabled. > > > > > > Let's also warn on trying to use omap_smccc_smc() if disabled as > > > suggested by Andrew F. Davis <afd@ti.com>. > > > > > > +#ifdef CONFIG_HAVE_ARM_SMCCC > > > void omap_smccc_smc(u32 fn, u32 arg) > > > { > > > struct arm_smccc_res res; > > > @@ -85,6 +86,12 @@ void omap_smccc_smc(u32 fn, u32 arg) > > > 0, 0, 0, 0, 0, 0, &res); > > > WARN(res.a0, "Secure function call 0x%08x failed\n", fn); > > > } > > > +#else > > > > Looking through the callers again, this can and is only called > > for ARMv7 CPUs, so can't you just avoid the #else path and > > turn it into a link error if we ever get callers on ARMv6-only builds? > > Hmm yeah maybe. If ARMv6 machines (basically n8x0) is not using > omap-smc.S at all, it's unlike it would need omap-secure.c either. Yeah we never built secure-common for omap2 earlier, this got recently added. My guess is that n8x0 only has a signed bootloader and no smc calls at all, or at least we don't use any smc calls for n8x0 in the kernel. I'll send out v2 of the patch. Regards, Tony
diff --git a/arch/arm/mach-omap2/omap-secure.c b/arch/arm/mach-omap2/omap-secure.c --- a/arch/arm/mach-omap2/omap-secure.c +++ b/arch/arm/mach-omap2/omap-secure.c @@ -77,6 +77,7 @@ u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs, u32 arg1, u32 arg2, return ret; } +#ifdef CONFIG_HAVE_ARM_SMCCC void omap_smccc_smc(u32 fn, u32 arg) { struct arm_smccc_res res; @@ -85,6 +86,12 @@ void omap_smccc_smc(u32 fn, u32 arg) 0, 0, 0, 0, 0, 0, &res); WARN(res.a0, "Secure function call 0x%08x failed\n", fn); } +#else +void omap_smccc_smc(u32 fn, u32 arg) +{ + WARN_ONCE(1, "smccc is disabled\n"); +} +#endif void omap_smc1(u32 fn, u32 arg) {
Recent omap changes added runtime checks to use omap_smccc_smc() when optee is configured in dts. As the omap-secure code can be built for ARMv6 only without ARMv7 and use custom smc calls, we now get a build error: omap-secure.c:(.text+0x94): undefined reference to `__arm_smccc_smc' Let's just ifdef out omap_smccc_smc() unless the CPU has selected CONFIG_HAVE_ARM_SMCCC. The other option discussed was to add an inline function to arm-smccc.h, but we'd still also have to add ifdef around omap_smccc_smc() to avoid a warning for uninitialized value for struct arm_smccc_res in omap_smccc_smc(). And we probably should not start initializing values in arm-smccc.h if disabled. Let's also warn on trying to use omap_smccc_smc() if disabled as suggested by Andrew F. Davis <afd@ti.com>. Fixes: 48840e16c299 ("ARM: OMAP2+: Use ARM SMC Calling Convention when OP-TEE is available") Reported-by: kbuild test robot <lkp@intel.com> Cc: Aaro Koskinen <aaro.koskinen@iki.fi> Cc: Andrew F. Davis <afd@ti.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Marc Zyngier <maz@kernel.org> Cc: Rob Herring <robh@kernel.org> Cc: Russell King <rmk+kernel@arm.linux.org.uk> Cc: Steven Price <steven.price@arm.com> Cc: Will Deacon <will@kernel.org> Signed-off-by: Tony Lindgren <tony@atomide.com> --- arch/arm/mach-omap2/omap-secure.c | 7 +++++++ 1 file changed, 7 insertions(+)