ARM: makefile: pass -march=armv4 to assembler even on CPU32v3

Per the discussion about half-way down in [1], the kernel doesn't actually support the ARMv3 ISA, but selects it for some ARMv4 ISA hardware that benefits from ARMv3 code generation. Such a consideration, then, only applies to the compiler but not to the assembler. This commit passes -march=armv4 to the assembler in those cases, so that code written for ARMv4 will continue to compile and run fine, without needing module-specific asflags-y overrides.

[1] https://lore.kernel.org/lkml/CAKv+Gu9FoFQymp2-=rUeh14CkUKON389OCE7stYCOFwKZpaCrg at mail.gmail.com/

Signed-off-by Jason A. Donenfeld <[email protected]>

Cc Ard Biesheuvel <[email protected]>

---
I don't have too much familiarity with hardware this old, nor access to testing systems, so please do carefully evaluate the assertions above before merging this, since I'm not sure I have a full understanding of the Linux ARMv3 situation.

Hi Arnd,

Apologies for the delay in getting back to you. I had some MTA issues and stupidly assumed ARM developers were taking the day off instead...

Review

Quoted 2 lines · Arnd Bergmann

(quoted diff) +1 -1

-arch-$(CONFIG_CPU_32v3)                =-D__LINUX_ARM_ARCH__=3 -march=armv3
+arch-$(CONFIG_CPU_32v3)                =-D__LINUX_ARM_ARCH__=3 -march=armv3m

Unfortunately this doesn't really cut it in my case, as it's not only those multiplications:
chacha20-arm.S:402: Error: selected processor does not support `bxeq lr' in ARM mode

I think we're going to wind up playing whack-a-mole in silly ways. The fact of the matter is that the ARM assembly I'm adding to the tree is for ARMv4 and up, and not for ARMv3.

I think there are three options to work around this issue:

1) Not build my assembly when CONFIG_CPU_32v3 via a Kconfig "depends".
2) Set asflags-$(CONFIG_CPU_32v3) inside my module locally to select -march=armv4.
3) This patch.

My initial plan was (1). ArdB recommended I do (2) instead. I thought that was a bit too nuanced and submitted (3).

It sounds like in light of the bus issues, (1) might be the best solution after all?

Let me know, and I'll follow your direction.

Regards,
Jason

Hi Arnd,

Apologies for the delay in getting back to you. I had some MTA issues and stupidly assumed ARM developers were taking the day off instead...

Review

Quoted 2 lines · Arnd Bergmann

(quoted diff) +1 -1

-arch-$(CONFIG_CPU_32v3)                =-D__LINUX_ARM_ARCH__=3 -march=armv3
+arch-$(CONFIG_CPU_32v3)                =-D__LINUX_ARM_ARCH__=3 -march=armv3m

Unfortunately this doesn't really cut it in my case, as it's not only those multiplications:
chacha20-arm.S:402: Error: selected processor does not support `bxeq lr' in ARM mode

I think we're going to wind up playing whack-a-mole in silly ways. The fact of the matter is that the ARM assembly I'm adding to the tree is for ARMv4 and up, and not for ARMv3.

I think there are three options to work around this issue:

1) Not build my assembly when CONFIG_CPU_32v3 via a Kconfig "depends".
2) Set asflags-$(CONFIG_CPU_32v3) inside my module locally to select -march=armv4.
3) This patch.

My initial plan was (1). ArdB recommended I do (2) instead. I thought that was a bit too nuanced and submitted (3).

Quoted 15 lines · Jason A. Donenfeld

Hi Arnd,

Apologies for the delay in getting back to you. I had some MTA issues and stupidly assumed ARM developers were taking the day off instead...

Review

Quoted 2 lines · Arnd Bergmann

(quoted diff) +1 -1

-arch-$(CONFIG_CPU_32v3)                =-D__LINUX_ARM_ARCH__=3 -march=armv3
+arch-$(CONFIG_CPU_32v3)                =-D__LINUX_ARM_ARCH__=3 -march=armv3m

Unfortunately this doesn't really cut it in my case, as it's not only those multiplications:
chacha20-arm.S:402: Error: selected processor does not support `bxeq lr' in ARM mode

Quoted 3 lines

I think we're going to wind up playing whack-a-mole in silly ways. The fact of the matter is that the ARM assembly I'm adding to the tree is for ARMv4 and up, and not for ARMv3.

I don't see what issues remain. The 'reteq lr' that Ard mentioned is definitely the correct way to return from assembly (you also need that for plain armv4, as 'bx' was added in armv4t), and Russell confirmed that using -march=armv3m is something we want anyway for mach-rpc.

Quoted 3 lines

I think we're going to wind up playing whack-a-mole in silly ways. The fact of the matter is that the ARM assembly I'm adding to the tree is for ARMv4 and up, and not for ARMv3.

I don't see what issues remain. The 'reteq lr' that Ard mentioned is definitely the correct way to return from assembly (you also need that for plain armv4, as 'bx' was added in armv4t), and Russell confirmed that using -march=armv3m is something we want anyway for mach-rpc.

Quoted 9 lines

I think there are three options to work around this issue:

1) Not build my assembly when CONFIG_CPU_32v3 via a Kconfig "depends".
2) Set asflags-$(CONFIG_CPU_32v3) inside my module locally to select -march=armv4.
3) This patch.

My initial plan was (1). ArdB recommended I do (2) instead. I thought that was a bit too nuanced and submitted (3).

I suspect all three of the above fail to work for armv4.

Hi Arnd,

Quoted 9 lines · Arnd Bergmann

Quoted 3 lines

I think we're going to wind up playing whack-a-mole in silly ways. The fact of the matter is that the ARM assembly I'm adding to the tree is for ARMv4 and up, and not for ARMv3.

I don't see what issues remain. The 'reteq lr' that Ard mentioned is definitely the correct way to return from assembly (you also need that for plain armv4, as 'bx' was added in armv4t), and Russell confirmed that using -march=armv3m is something we want anyway for mach-rpc.

I'll do that. I can confirm that after changing it to reteq lr, everything works well with armv3m.

Quoted 7 lines

Quoted 7 lines

I think there are three options to work around this issue:

1) Not build my assembly when CONFIG_CPU_32v3 via a Kconfig "depends".
2) Set asflags-$(CONFIG_CPU_32v3) inside my module locally to select -march=armv4.
3) This patch.

Quoted 4 lines

Quoted 2 lines

My initial plan was (1). ArdB recommended I do (2) instead. I thought that was a bit too nuanced and submitted (3).

I suspect all three of the above fail to work for armv4.

Armv4 does actually work in this configuration, in fact.

But anyway,
I'll go with your primary suggestion and we therefore can move ahead with changing the global cflag to march=armv3m. Would you like me to submit the patch for this, or would yo like to handle it?

It builds but it doesn't run, at least not when built into the kernel proper (which will be the case after random.c moves to this library)

Your toolchain is implicitly passing --fix-v4bx to the assembler, which causes it to permit bx instructions in ARMv4 object code, but tag them with special R_ARM_V4BX ELF relocations. The ARM module loader does take these into account, so built as a module, it works. However, when built into the core kernel, we have to rely on the linker to patch this instruction into the ARMv4 equivalent, and a quick check reveals that that is currently not the case.

Bottom line: let's not go there.

Quoted 6 lines

But anyway,
I'll go with your primary suggestion and we therefore can move ahead with changing the global cflag to march=armv3m. Would you like me to submit the patch for this, or would yo like to handle it?

Yes please go ahead.

The purpose of CONFIG_CPU_32v3 is to avoid ldrh/strh on the RiscPC, which is pretty much an ARMv4 device, except its bus will choke on the half-words. The way to make the C compiler not output ldrh/strh is with -march=armv3, which doesn't support them in the ISA. However, this prevents certain cryptography code from working that uses instructions like umull. Fortunately there's also -march=armv3m that does support those, making it possible to continue assembling optimized cryptography routines for our beloved RiscPC.

Signed-off-by Jason A. Donenfeld <[email protected]>

Cc Ard Biesheuvel <[email protected]>

Cc Russell King <[email protected]>

Cc Arnd Bergmann <[email protected]>

---
 arch/arm/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm/Makefile +1 -2

index d1516f85f25d..7fd4bcaf0721 100644
@@ -74,7 +74,7 @@ endif
 arch-$(CONFIG_CPU_32v5)                =-D__LINUX_ARM_ARCH__=5 $(call cc-option,-march=armv5te,-march=armv4t)
 arch-$(CONFIG_CPU_32v4T)       =-D__LINUX_ARM_ARCH__=4 -march=armv4t
 arch-$(CONFIG_CPU_32v4)                =-D__LINUX_ARM_ARCH__=4 -march=armv4
-arch-$(CONFIG_CPU_32v3)                =-D__LINUX_ARM_ARCH__=3 -march=armv3
+arch-$(CONFIG_CPU_32v3)                =-D__LINUX_ARM_ARCH__=3 -march=armv3m
 
 # Evaluate arch cc-option calls now
 arch-y := $(arch-y)
--

2.19.0

Quoted 9 lines · Jason A. Donenfeld

The purpose of CONFIG_CPU_32v3 is to avoid ldrh/strh on the RiscPC, which is pretty much an ARMv4 device, except its bus will choke on the half-words. The way to make the C compiler not output ldrh/strh is with -march=armv3, which doesn't support them in the ISA. However, this prevents certain cryptography code from working that uses instructions like umull. Fortunately there's also -march=armv3m that does support those, making it possible to continue assembling optimized cryptography routines for our beloved RiscPC.

Quoted 4 lines

Signed-off-by Jason A. Donenfeld <[email protected]>

Cc Ard Biesheuvel <[email protected]>

Cc Russell King <[email protected]>

Cc Arnd Bergmann <[email protected]>

Acked-by Ard Biesheuvel <[email protected]>

Please add this to Russell's patch tracker for inclusion at
http://www.arm.linux.org.uk/developer/patches/

Per the discussion about half-way down in [1], the kernel doesn't actually support the ARMv3 ISA, but selects it for some ARMv4 ISA hardware that benefits from ARMv3 code generation.

Such a consideration,
then, only applies to the compiler but not to the assembler. This commit passes -march=armv4 to the assembler in those cases, so that code written for ARMv4 will continue to compile and run fine, without needing module-specific asflags-y overrides.

Quoted 3 lines · Jason A. Donenfeld

Per the discussion about half-way down in [1], the kernel doesn't actually support the ARMv3 ISA, but selects it for some ARMv4 ISA hardware that benefits from ARMv3 code generation.

The issue is to do with the half-word stores in the ARMv4 ISA, which need to be avoided on StrongARM RiscPC - the bus from the processor card (which was designed for ARM610 and ARM710) does not support anything except 8-bit and 32-bit accesses, so the 16-bit load/store instructions don't work correctly.

Obviously, the reason for having the compiler use ARMv3 is to avoid those instructions which we have no other way to prevent - however, the use of ARMv3 with the assembler ensures that ldrh/strh are not accidentally used.

We could argue that the ARMv3 assembly files are now stable, so the chances of ldrh/strh being introduced is low, which would make this change tolerable, but the commit message needs to spell out that we lose this protection.

Quoted 5 lines

Such a consideration,
then, only applies to the compiler but not to the assembler. This commit passes -march=armv4 to the assembler in those cases, so that code written for ARMv4 will continue to compile and run fine, without needing module-specific asflags-y overrides.

Note that "code written for ARMv4" will not be usable on this platform if it makes use of ldrh/strh, so depending on which instructions the assembler is complaining about, it could very well be a real "you're doing something wrong" case.

The side effect of this patch is that such cases will now be hidden rather than evaluated on a case-by-case basis.

Quoted 27 lines · Russell King - ARM Linux

Quoted 3 lines · Jason A. Donenfeld

Per the discussion about half-way down in [1], the kernel doesn't actually support the ARMv3 ISA, but selects it for some ARMv4 ISA hardware that benefits from ARMv3 code generation.

The issue is to do with the half-word stores in the ARMv4 ISA, which need to be avoided on StrongARM RiscPC - the bus from the processor card (which was designed for ARM610 and ARM710) does not support anything except 8-bit and 32-bit accesses, so the 16-bit load/store instructions don't work correctly.

Obviously, the reason for having the compiler use ARMv3 is to avoid those instructions which we have no other way to prevent - however, the use of ARMv3 with the assembler ensures that ldrh/strh are not accidentally used.

We could argue that the ARMv3 assembly files are now stable, so the chances of ldrh/strh being introduced is low, which would make this change tolerable, but the commit message needs to spell out that we lose this protection.

Quoted 5 lines

Such a consideration,
then, only applies to the compiler but not to the assembler. This commit passes -march=armv4 to the assembler in those cases, so that code written for ARMv4 will continue to compile and run fine, without needing module-specific asflags-y overrides.

Quoted 8 lines

Note that "code written for ARMv4" will not be usable on this platform if it makes use of ldrh/strh, so depending on which instructions the assembler is complaining about, it could very well be a real "you're doing something wrong" case.

The side effect of this patch is that such cases will now be hidden rather than evaluated on a case-by-case basis.

Thanks for the insight.

So Arnd's suggestion to switch to armv3-m would actually be feasible then? The code in question does not use ldrh only umull, and so it should build with armv3m as well. And we will even generate some better code for RiscPC if we apply it to both the cflags and the asflags.

So Arnd's suggestion to switch to armv3-m would actually be feasible then? The code in question does not use ldrh only umull, and so it should build with armv3m as well. And we will even generate some better code for RiscPC if we apply it to both the cflags and the asflags.

We could argue that the ARMv3 assembly files are now stable, so the chances of ldrh/strh being introduced is low, which would make this change tolerable, but the commit message needs to spell out that we lose this protection.