[PATCH] ARM: Build secure_cntvoff.S unconditionally to fix shmobile !SMP build

[PATCH] ARM: Build secure_cntvoff.S unconditionally to fix shmobile !SMP build

[Geert Uytterhoeven]

If CONFIG_SMP=n, building a kernel for R-Car Gen2 fails with:

    arch/arm/mach-shmobile/setup-rcar-gen2.o: In function `rcar_gen2_timer_init':
    setup-rcar-gen2.c:(.init.text+0x30): undefined reference to `secure_cntvoff_init'

Indeed, on R-Car Gen2 SoCs, secure_cntvoff_init() is not only needed for
secondary CPUs, but also for the boot CPU.  This is most visible on SoCs
with Cortex A7 cores (e.g. R-Car E2, cfr. commit 9ce3fa6816c2fb59 ("ARM:
shmobile: rcar-gen2: Add CA7 arch_timer initialization for r8a7794")),
but Cortex A15 is affected, too.

Fix this by always providing secure_cntvoff_init().

Reported-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 7c607944bc657616 ("ARM: smp: Add initialization of CNTVOFF")
Fixes: cad160ed0a94927e ("ARM: shmobile: Convert file to use cntvoff")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/common/Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
index 1e9f7af8f70ff6ba..923a8d08be679399 100644
--- a/arch/arm/common/Makefile
+++ b/arch/arm/common/Makefile
@@ -3,14 +3,13 @@
 # Makefile for the linux kernel.
 #
 
-obj-y				+= firmware.o
+obj-y				+= firmware.o secure_cntvoff.o
 
 obj-$(CONFIG_SA1111)		+= sa1111.o
 obj-$(CONFIG_DMABOUNCE)		+= dmabounce.o
 obj-$(CONFIG_SHARP_LOCOMO)	+= locomo.o
 obj-$(CONFIG_SHARP_PARAM)	+= sharpsl_param.o
 obj-$(CONFIG_SHARP_SCOOP)	+= scoop.o
-obj-$(CONFIG_SMP)		+= secure_cntvoff.o
 obj-$(CONFIG_PCI_HOST_ITE8152)  += it8152.o
 obj-$(CONFIG_MCPM)		+= mcpm_head.o mcpm_entry.o mcpm_platsmp.o vlock.o
 CFLAGS_REMOVE_mcpm_entry.o	= -pg
-- 
2.7.4

[PATCH] ARM: Build secure_cntvoff.S unconditionally to fix shmobile !SMP build

[Maxime Ripard]

On Wed, Jun 06, 2018 at 10:52:34AM +0200, Geert Uytterhoeven wrote:
> If CONFIG_SMP=n, building a kernel for R-Car Gen2 fails with:
> 
>     arch/arm/mach-shmobile/setup-rcar-gen2.o: In function `rcar_gen2_timer_init':
>     setup-rcar-gen2.c:(.init.text+0x30): undefined reference to `secure_cntvoff_init'
> 
> Indeed, on R-Car Gen2 SoCs, secure_cntvoff_init() is not only needed for
> secondary CPUs, but also for the boot CPU.  This is most visible on SoCs
> with Cortex A7 cores (e.g. R-Car E2, cfr. commit 9ce3fa6816c2fb59 ("ARM:
> shmobile: rcar-gen2: Add CA7 arch_timer initialization for r8a7794")),
> but Cortex A15 is affected, too.
> 
> Fix this by always providing secure_cntvoff_init().
> 
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 7c607944bc657616 ("ARM: smp: Add initialization of CNTVOFF")
> Fixes: cad160ed0a94927e ("ARM: shmobile: Convert file to use cntvoff")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180606/92813218/attachment.sig>

[PATCH] ARM: Build secure_cntvoff.S unconditionally to fix shmobile !SMP build

[Russell King - ARM Linux]

On Wed, Jun 06, 2018 at 10:52:34AM +0200, Geert Uytterhoeven wrote:
> If CONFIG_SMP=n, building a kernel for R-Car Gen2 fails with:
> 
>     arch/arm/mach-shmobile/setup-rcar-gen2.o: In function `rcar_gen2_timer_init':
>     setup-rcar-gen2.c:(.init.text+0x30): undefined reference to `secure_cntvoff_init'
> 
> Indeed, on R-Car Gen2 SoCs, secure_cntvoff_init() is not only needed for
> secondary CPUs, but also for the boot CPU.  This is most visible on SoCs
> with Cortex A7 cores (e.g. R-Car E2, cfr. commit 9ce3fa6816c2fb59 ("ARM:
> shmobile: rcar-gen2: Add CA7 arch_timer initialization for r8a7794")),
> but Cortex A15 is affected, too.
> 
> Fix this by always providing secure_cntvoff_init().
> 
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 7c607944bc657616 ("ARM: smp: Add initialization of CNTVOFF")
> Fixes: cad160ed0a94927e ("ARM: shmobile: Convert file to use cntvoff")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

This doesn't look right to me, but I don't have secure_cntvoff in any
tree here that I can look at to check.  What if secure_cntvoff contains
instructions only available on ARMv7 CPUs, and not ARMv4?

It makes no sense (to me) to always build this.

NAK, for the time being until the situation becomes clearer.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH] ARM: Build secure_cntvoff.S unconditionally to fix shmobile !SMP build

[Geert Uytterhoeven]

Hi Russell,

On Wed, Jun 6, 2018 at 11:09 AM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Wed, Jun 06, 2018 at 10:52:34AM +0200, Geert Uytterhoeven wrote:
>> If CONFIG_SMP=n, building a kernel for R-Car Gen2 fails with:
>>
>>     arch/arm/mach-shmobile/setup-rcar-gen2.o: In function `rcar_gen2_timer_init':
>>     setup-rcar-gen2.c:(.init.text+0x30): undefined reference to `secure_cntvoff_init'
>>
>> Indeed, on R-Car Gen2 SoCs, secure_cntvoff_init() is not only needed for
>> secondary CPUs, but also for the boot CPU.  This is most visible on SoCs
>> with Cortex A7 cores (e.g. R-Car E2, cfr. commit 9ce3fa6816c2fb59 ("ARM:
>> shmobile: rcar-gen2: Add CA7 arch_timer initialization for r8a7794")),
>> but Cortex A15 is affected, too.
>>
>> Fix this by always providing secure_cntvoff_init().
>>
>> Reported-by: Arnd Bergmann <arnd@arndb.de>
>> Fixes: 7c607944bc657616 ("ARM: smp: Add initialization of CNTVOFF")

Interestingly, the commit description says:

   "It should be done by the bootloader but it is currently not the case,
    even for boot CPU because this SoC is booting in secure mode."
    ^^^^^^^^^^^^^^^^^

So it should be called for !SMP on sunxi, too?

>> Fixes: cad160ed0a94927e ("ARM: shmobile: Convert file to use cntvoff")
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> This doesn't look right to me, but I don't have secure_cntvoff in any
> tree here that I can look at to check.  What if secure_cntvoff contains
> instructions only available on ARMv7 CPUs, and not ARMv4?

Compiled != called.

> It makes no sense (to me) to always build this.

But you're right, making it depend on CPU_V7 is better.
Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH] ARM: Build secure_cntvoff.S unconditionally to fix shmobile !SMP build

[Russell King - ARM Linux]

On Wed, Jun 06, 2018 at 11:22:41AM +0200, Geert Uytterhoeven wrote:
> Hi Russell,
> 
> On Wed, Jun 6, 2018 at 11:09 AM, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> >> Fixes: cad160ed0a94927e ("ARM: shmobile: Convert file to use cntvoff")
> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > This doesn't look right to me, but I don't have secure_cntvoff in any
> > tree here that I can look at to check.  What if secure_cntvoff contains
> > instructions only available on ARMv7 CPUs, and not ARMv4?
> 
> Compiled != called.

That makes little difference when the assembler validates that the
instructions are possible on the target architecture and errors out
if not.

That's why I qualified my reply by saying that I didn't have the
contents of secure_cntvoff to reference - there's no clue whether
it's a .c or .S file from just this patch, and there's no visibility
what it contains.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH] ARM: Build secure_cntvoff.S unconditionally to fix shmobile !SMP build

[Geert Uytterhoeven]

Hi Russell,

On Wed, Jun 6, 2018 at 11:27 AM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Wed, Jun 06, 2018 at 11:22:41AM +0200, Geert Uytterhoeven wrote:
>> On Wed, Jun 6, 2018 at 11:09 AM, Russell King - ARM Linux
>> <linux@armlinux.org.uk> wrote:
>> >> Fixes: cad160ed0a94927e ("ARM: shmobile: Convert file to use cntvoff")
>> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> >
>> > This doesn't look right to me, but I don't have secure_cntvoff in any
>> > tree here that I can look at to check.  What if secure_cntvoff contains
>> > instructions only available on ARMv7 CPUs, and not ARMv4?
>>
>> Compiled != called.
>
> That makes little difference when the assembler validates that the
> instructions are possible on the target architecture and errors out
> if not.

The file does have ".arch armv7-a".

> That's why I qualified my reply by saying that I didn't have the
> contents of secure_cntvoff to reference - there's no clue whether
> it's a .c or .S file from just this patch, and there's no visibility
> what it contains.

https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git/commit/?id=7c607944bc657616
https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git/commit/?id=cad160ed0a94927e

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds