[U-Boot] [PATCH] arm: arm64: only use general regs

[U-Boot] [PATCH] arm: arm64: only use general regs

[Peng Fan]

When compiling with android toolchain, there is an instruction
"str     q0, [x8],#16", but x8 is not 16bytes aligned,
this instruction will trigger sync abort.

So, following Linux kernel, only use general regs for arm64.
If not, compiler may use simd registers Q[x]. We need to avoid
using simd registers in U-Boot, because load/store Q[x] has
restriction that 128bits aligned when str/ldr.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 arch/arm/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 5881fdc8e2..4db0398dde 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -18,7 +18,7 @@ arch-$(CONFIG_CPU_ARM1136)	=-march=armv5
 arch-$(CONFIG_CPU_ARM1176)	=-march=armv5t
 arch-$(CONFIG_CPU_V7)		=$(call cc-option, -march=armv7-a, \
 				 $(call cc-option, -march=armv7, -march=armv5))
-arch-$(CONFIG_ARM64)		=-march=armv8-a
+arch-$(CONFIG_ARM64)		=-march=armv8-a -mgeneral-regs-only
 
 # On Tegra systems we must build SPL for the armv4 core on the device
 # but otherwise we can use the value in CONFIG_SYS_ARM_ARCH
-- 
2.14.1

[U-Boot] [PATCH] arm: arm64: only use general regs

[Simon Glass]

On 27 November 2017 at 19:09, Peng Fan <peng.fan@nxp.com> wrote:
> When compiling with android toolchain, there is an instruction
> "str     q0, [x8],#16", but x8 is not 16bytes aligned,
> this instruction will trigger sync abort.
>
> So, following Linux kernel, only use general regs for arm64.
> If not, compiler may use simd registers Q[x]. We need to avoid
> using simd registers in U-Boot, because load/store Q[x] has
> restriction that 128bits aligned when str/ldr.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  arch/arm/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH] arm: arm64: only use general regs

[Alexander Graf]

On 28.11.17 03:09, Peng Fan wrote:
> When compiling with android toolchain, there is an instruction
> "str     q0, [x8],#16", but x8 is not 16bytes aligned,
> this instruction will trigger sync abort.
> 
> So, following Linux kernel, only use general regs for arm64.
> If not, compiler may use simd registers Q[x]. We need to avoid
> using simd registers in U-Boot, because load/store Q[x] has
> restriction that 128bits aligned when str/ldr.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>

The compiler should only output 16-byte-alignemnt-requiring instructions
when it can safely assume that the variable in question is 16 byte aligned.

Where did x8 come from? That was probably just an unsafe cast?

For reference, I ran into something similar recently on 32bit ARM:

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82445

And this indeed turned out to be a compiler bug.

FWIW the main reason Linux doesn't want to use FPU registers in kernel
space is simply that it doesn't want to bother saving/restoring them on
syscalls or interrupts. But I don't quite see why we would care in U-Boot.


Alex

[U-Boot] [PATCH] arm: arm64: only use general regs

[Peng Fan]

Hi Alexander,
On Fri, Dec 01, 2017 at 10:46:29PM +0100, Alexander Graf wrote:
>
>
>On 28.11.17 03:09, Peng Fan wrote:
>> When compiling with android toolchain, there is an instruction
>> "str     q0, [x8],#16", but x8 is not 16bytes aligned,
>> this instruction will trigger sync abort.
>> 
>> So, following Linux kernel, only use general regs for arm64.
>> If not, compiler may use simd registers Q[x]. We need to avoid
>> using simd registers in U-Boot, because load/store Q[x] has
>> restriction that 128bits aligned when str/ldr.
>> 
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>
>The compiler should only output 16-byte-alignemnt-requiring instructions
>when it can safely assume that the variable in question is 16 byte aligned.
>
>Where did x8 come from? That was probably just an unsafe cast?

I am not able to resetup my compiler enviorment, it's long time
since we found this issue.

Just objdump and seems it is memset/memcpy,

00000000007ee708 <memset>:
  7ee708:       92400804        and     x4, x0, #0x7
  7ee70c:       aa0003e3        mov     x3, x0

.......

  7ee764:       aa0003e8        mov     x8, x0
  7ee768:       d2800007        mov     x7, #0x0                        // #0
  7ee76c:       3c810500        str     q0, [x8], #16

Thanks,
Peng

>
>For reference, I ran into something similar recently on 32bit ARM:
>
>  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82445
>
>And this indeed turned out to be a compiler bug.
>
>FWIW the main reason Linux doesn't want to use FPU registers in kernel
>space is simply that it doesn't want to bother saving/restoring them on
>syscalls or interrupts. But I don't quite see why we would care in U-Boot.
>
>
>Alex
>_______________________________________________
>U-Boot mailing list
>U-Boot at lists.denx.de
>https://lists.denx.de/listinfo/u-boot

-- 

[U-Boot] [PATCH] arm: arm64: only use general regs

[Alexander Graf]

On 12/07/2017 08:25 AM, Peng Fan wrote:
> Hi Alexander,
> On Fri, Dec 01, 2017 at 10:46:29PM +0100, Alexander Graf wrote:
>>
>> On 28.11.17 03:09, Peng Fan wrote:
>>> When compiling with android toolchain, there is an instruction
>>> "str     q0, [x8],#16", but x8 is not 16bytes aligned,
>>> this instruction will trigger sync abort.
>>>
>>> So, following Linux kernel, only use general regs for arm64.
>>> If not, compiler may use simd registers Q[x]. We need to avoid
>>> using simd registers in U-Boot, because load/store Q[x] has
>>> restriction that 128bits aligned when str/ldr.
>>>
>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> The compiler should only output 16-byte-alignemnt-requiring instructions
>> when it can safely assume that the variable in question is 16 byte aligned.
>>
>> Where did x8 come from? That was probably just an unsafe cast?
> I am not able to resetup my compiler enviorment, it's long time
> since we found this issue.
>
> Just objdump and seems it is memset/memcpy,
>
> 00000000007ee708 <memset>:
>    7ee708:       92400804        and     x4, x0, #0x7
>    7ee70c:       aa0003e3        mov     x3, x0
>
> .......
>
>    7ee764:       aa0003e8        mov     x8, x0
>    7ee768:       d2800007        mov     x7, #0x0                        // #0
>    7ee76c:       3c810500        str     q0, [x8], #16

I would assume this 16-byte aligned store only happens when the pointer 
is also 16-byte aligned.

Really, I am fairly sure if you ran into issues, we're looking at a 
compiler bug or an unsafe cast. We shouldn't disable vector register 
usage in U-Boot altogether because of either.


Alex

[U-Boot] arm: arm64: only use general regs

[Tom Rini]

On Tue, Nov 28, 2017 at 10:09:37AM +0800, Peng Fan wrote:

> When compiling with android toolchain, there is an instruction
> "str     q0, [x8],#16", but x8 is not 16bytes aligned,
> this instruction will trigger sync abort.
> 
> So, following Linux kernel, only use general regs for arm64.
> If not, compiler may use simd registers Q[x]. We need to avoid
> using simd registers in U-Boot, because load/store Q[x] has
> restriction that 128bits aligned when str/ldr.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

This breaks s32v234evb building, please look into that, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20171202/97f51135/attachment.sig>

[U-Boot] arm: arm64: only use general regs

[Peng Fan]

> -----Original Message-----
> From: Tom Rini [mailto:trini at konsulko.com]
> Sent: Sunday, December 03, 2017 12:02 PM
> To: Peng Fan <peng.fan@nxp.com>
> Cc: albert.u.boot at aribaud.net; sjg at chromium.org; York Sun
> <york.sun@nxp.com>; u-boot at lists.denx.de
> Subject: Re: [U-Boot] arm: arm64: only use general regs
> 
> On Tue, Nov 28, 2017 at 10:09:37AM +0800, Peng Fan wrote:
> 
> > When compiling with android toolchain, there is an instruction
> > "str     q0, [x8],#16", but x8 is not 16bytes aligned,
> > this instruction will trigger sync abort.
> >
> > So, following Linux kernel, only use general regs for arm64.
> > If not, compiler may use simd registers Q[x]. We need to avoid using
> > simd registers in U-Boot, because load/store Q[x] has restriction that
> > 128bits aligned when str/ldr.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> This breaks s32v234evb building, please look into that, thanks!

I have no idea why use float here.

board/freescale/s32v234evb/clock.c: In function 'program_pll.constprop':
board/freescale/s32v234evb/clock.c:91:7: error: '-mgeneral-regs-only' is incompatible with floating-point code
  fvco =
  ~~~~~^
      (refclk_freq / plldv_prediv) * (plldv_mfd +
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
          pllfd_mfn / (float)20480);
          ~~~~~~~~~~~~~~~~~~~~~~~~~

Eddy,
 Could you help explain why use float to cast 20480? Could this cast be removed?

Thanks,
Peng.

> 
> --
> Tom