linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64/crypto: remove non-standard notation
@ 2018-08-20 22:40 Nick Desaulniers
  2018-08-20 22:43 ` Nick Desaulniers
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Nick Desaulniers @ 2018-08-20 22:40 UTC (permalink / raw)
  To: ard.biesheuvel
  Cc: Nick Desaulniers, Herbert Xu, David S. Miller, Catalin Marinas,
	Will Deacon, linux-crypto, linux-arm-kernel, linux-kernel

It seems that:
ldr q8, =0x30000000200000001

is a GNU as convience notation for:
ldr q8, .Lconstant
.Lconstant
.word 0x00000001
.word 0x00000002
.word 0x00000003
.word 0x00000000

based on this comment in binutils' source [0]. I've asked for this
non-standard convience notation to be added to other assemblers [1], but
until then, we can remove it and get equivalent disassembly:

before:
00000000000009d4 <neon_aes_ctr_encrypt>:
...
     a48:       9c000ac8        ldr     q8, ba0 <neon_aes_ctr_encrypt+0x1cc>
...
     ba0:       00000001        .word   0x00000001
     ba4:       00000002        .word   0x00000002
     ba8:       00000003        .word   0x00000003
     bac:       00000000        .word   0x00000000

after:

00000000000009d4 <neon_aes_ctr_encrypt>:
...
     a48:       9c000aa8        ldr     q8, b9c <neon_aes_ctr_encrypt+0x1c8>
...
     b9c:       00000001        .word   0x00000001
     ba0:       00000002        .word   0x00000002
     ba4:       00000003        .word   0x00000003
     ba8:       00000000        .word   0x00000000

[0] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gas/testsuite/gas/aarch64/programmer-friendly.s;h=6254c6476efdc848648b05068be0574e7addc85d;hb=HEAD#l11
[1] https://bugs.llvm.org/show_bug.cgi?id=38642

Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/arm64/crypto/aes-modes.S | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/crypto/aes-modes.S b/arch/arm64/crypto/aes-modes.S
index 483a7130cf0e..9288c5b0eca2 100644
--- a/arch/arm64/crypto/aes-modes.S
+++ b/arch/arm64/crypto/aes-modes.S
@@ -232,7 +232,7 @@ AES_ENTRY(aes_ctr_encrypt)
 	bmi		.Lctr1x
 	cmn		w6, #4			/* 32 bit overflow? */
 	bcs		.Lctr1x
-	ldr		q8, =0x30000000200000001	/* addends 1,2,3[,0] */
+	ldr		q8, .Laddends /* addends 1,2,3[,0] */
 	dup		v7.4s, w6
 	mov		v0.16b, v4.16b
 	add		v7.4s, v7.4s, v8.4s
@@ -295,6 +295,12 @@ AES_ENTRY(aes_ctr_encrypt)
 	rev		x7, x7
 	ins		v4.d[0], x7
 	b		.Lctrcarrydone
+
+.Laddends:
+	.word	0x00000001
+	.word	0x00000002
+	.word	0x00000003
+	.word	0x00000000
 AES_ENDPROC(aes_ctr_encrypt)
 	.ltorg
 
-- 
2.18.0.865.gffc8e1a3cd6-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64/crypto: remove non-standard notation
  2018-08-20 22:40 [PATCH] arm64/crypto: remove non-standard notation Nick Desaulniers
@ 2018-08-20 22:43 ` Nick Desaulniers
  2018-08-21  0:04 ` Will Deacon
  2018-08-21 12:23 ` Ard Biesheuvel
  2 siblings, 0 replies; 8+ messages in thread
From: Nick Desaulniers @ 2018-08-20 22:43 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Herbert Xu, David S. Miller, Catalin Marinas, Will Deacon,
	linux-crypto, Linux ARM, LKML

On Mon, Aug 20, 2018 at 3:41 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> It seems that:
> ldr q8, =0x30000000200000001
>
> is a GNU as convience notation for:
> ldr q8, .Lconstant
> .Lconstant
> .word 0x00000001
> .word 0x00000002
> .word 0x00000003
> .word 0x00000000
>
> based on this comment in binutils' source [0]. I've asked for this
> non-standard convience notation to be added to other assemblers [1], but
> until then, we can remove it and get equivalent disassembly:
>
> before:
> 00000000000009d4 <neon_aes_ctr_encrypt>:
> ...
>      a48:       9c000ac8        ldr     q8, ba0 <neon_aes_ctr_encrypt+0x1cc>
> ...
>      ba0:       00000001        .word   0x00000001
>      ba4:       00000002        .word   0x00000002
>      ba8:       00000003        .word   0x00000003
>      bac:       00000000        .word   0x00000000
>
> after:
>
> 00000000000009d4 <neon_aes_ctr_encrypt>:
> ...
>      a48:       9c000aa8        ldr     q8, b9c <neon_aes_ctr_encrypt+0x1c8>
> ...
>      b9c:       00000001        .word   0x00000001
>      ba0:       00000002        .word   0x00000002
>      ba4:       00000003        .word   0x00000003
>      ba8:       00000000        .word   0x00000000
>
> [0] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gas/testsuite/gas/aarch64/programmer-friendly.s;h=6254c6476efdc848648b05068be0574e7addc85d;hb=HEAD#l11
> [1] https://bugs.llvm.org/show_bug.cgi?id=38642
>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  arch/arm64/crypto/aes-modes.S | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/crypto/aes-modes.S b/arch/arm64/crypto/aes-modes.S
> index 483a7130cf0e..9288c5b0eca2 100644
> --- a/arch/arm64/crypto/aes-modes.S
> +++ b/arch/arm64/crypto/aes-modes.S
> @@ -232,7 +232,7 @@ AES_ENTRY(aes_ctr_encrypt)
>         bmi             .Lctr1x
>         cmn             w6, #4                  /* 32 bit overflow? */
>         bcs             .Lctr1x
> -       ldr             q8, =0x30000000200000001        /* addends 1,2,3[,0] */
> +       ldr             q8, .Laddends /* addends 1,2,3[,0] */
>         dup             v7.4s, w6
>         mov             v0.16b, v4.16b
>         add             v7.4s, v7.4s, v8.4s
> @@ -295,6 +295,12 @@ AES_ENTRY(aes_ctr_encrypt)
>         rev             x7, x7
>         ins             v4.d[0], x7
>         b               .Lctrcarrydone
> +
> +.Laddends:
> +       .word   0x00000001
> +       .word   0x00000002
> +       .word   0x00000003
> +       .word   0x00000000
>  AES_ENDPROC(aes_ctr_encrypt)
>         .ltorg
>
> --
> 2.18.0.865.gffc8e1a3cd6-goog
>

I can't spell. s/convience/convenience/g

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64/crypto: remove non-standard notation
  2018-08-20 22:40 [PATCH] arm64/crypto: remove non-standard notation Nick Desaulniers
  2018-08-20 22:43 ` Nick Desaulniers
@ 2018-08-21  0:04 ` Will Deacon
  2018-08-21 12:23 ` Ard Biesheuvel
  2 siblings, 0 replies; 8+ messages in thread
From: Will Deacon @ 2018-08-21  0:04 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: ard.biesheuvel, Herbert Xu, David S. Miller, Catalin Marinas,
	linux-crypto, linux-arm-kernel, linux-kernel

On Mon, Aug 20, 2018 at 03:40:32PM -0700, Nick Desaulniers wrote:
> It seems that:
> ldr q8, =0x30000000200000001
> 
> is a GNU as convience notation for:
> ldr q8, .Lconstant
> .Lconstant
> .word 0x00000001
> .word 0x00000002
> .word 0x00000003
> .word 0x00000000

Does still this work correctly for a big-endian build?

Will

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64/crypto: remove non-standard notation
  2018-08-20 22:40 [PATCH] arm64/crypto: remove non-standard notation Nick Desaulniers
  2018-08-20 22:43 ` Nick Desaulniers
  2018-08-21  0:04 ` Will Deacon
@ 2018-08-21 12:23 ` Ard Biesheuvel
  2018-08-21 16:50   ` Nick Desaulniers
  2 siblings, 1 reply; 8+ messages in thread
From: Ard Biesheuvel @ 2018-08-21 12:23 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Herbert Xu, David S. Miller, Catalin Marinas, Will Deacon,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
	linux-arm-kernel, Linux Kernel Mailing List

Hi Nick,

On 21 August 2018 at 00:40, Nick Desaulniers <ndesaulniers@google.com> wrote:
> It seems that:
> ldr q8, =0x30000000200000001
>
> is a GNU as convience notation for:
> ldr q8, .Lconstant
> .Lconstant
> .word 0x00000001
> .word 0x00000002
> .word 0x00000003
> .word 0x00000000
>
> based on this comment in binutils' source [0]. I've asked for this
> non-standard convience notation to be added to other assemblers [1], but
> until then, we can remove it and get equivalent disassembly:
>

What do you mean by 'non-standard convenience notation'? Which asm
'standard' does Clang actually claim to implement?

This 'GCC/binutils is broken and we are reluctant to subvert Clang'
attitude is getting a bit old tbh. In the future, could you please
mention clang explicitly in the patch subject so it is obvious what
the purpose of the patch is? I think we should accommodate Clang in
Linux, but the attitude has got to go.


> before:
> 00000000000009d4 <neon_aes_ctr_encrypt>:
> ...
>      a48:       9c000ac8        ldr     q8, ba0 <neon_aes_ctr_encrypt+0x1cc>
> ...
>      ba0:       00000001        .word   0x00000001
>      ba4:       00000002        .word   0x00000002
>      ba8:       00000003        .word   0x00000003
>      bac:       00000000        .word   0x00000000
>
> after:
>
> 00000000000009d4 <neon_aes_ctr_encrypt>:
> ...
>      a48:       9c000aa8        ldr     q8, b9c <neon_aes_ctr_encrypt+0x1c8>
> ...
>      b9c:       00000001        .word   0x00000001
>      ba0:       00000002        .word   0x00000002
>      ba4:       00000003        .word   0x00000003
>      ba8:       00000000        .word   0x00000000
>
> [0] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gas/testsuite/gas/aarch64/programmer-friendly.s;h=6254c6476efdc848648b05068be0574e7addc85d;hb=HEAD#l11
> [1] https://bugs.llvm.org/show_bug.cgi?id=38642
>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  arch/arm64/crypto/aes-modes.S | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/crypto/aes-modes.S b/arch/arm64/crypto/aes-modes.S
> index 483a7130cf0e..9288c5b0eca2 100644
> --- a/arch/arm64/crypto/aes-modes.S
> +++ b/arch/arm64/crypto/aes-modes.S
> @@ -232,7 +232,7 @@ AES_ENTRY(aes_ctr_encrypt)
>         bmi             .Lctr1x
>         cmn             w6, #4                  /* 32 bit overflow? */
>         bcs             .Lctr1x
> -       ldr             q8, =0x30000000200000001        /* addends 1,2,3[,0] */
> +       ldr             q8, .Laddends /* addends 1,2,3[,0] */
>         dup             v7.4s, w6
>         mov             v0.16b, v4.16b
>         add             v7.4s, v7.4s, v8.4s
> @@ -295,6 +295,12 @@ AES_ENTRY(aes_ctr_encrypt)
>         rev             x7, x7
>         ins             v4.d[0], x7
>         b               .Lctrcarrydone
> +
> +.Laddends:
> +       .word   0x00000001
> +       .word   0x00000002
> +       .word   0x00000003
> +       .word   0x00000000

As Will points out, this breaks BE builds.

>  AES_ENDPROC(aes_ctr_encrypt)
>         .ltorg

You can drop this .ltorg if you get rid of all =xxx instances.

Actually, though, I would prefer it if we could use some clever short
sequence of movi+add instructions, and get rid of the literal
altogether. Or otherwise, please use something like

ldr_l q8, .Laddends, <temp register>

instead, and move the literal into the .rodata section (and use .octa
so you don't break BE)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64/crypto: remove non-standard notation
  2018-08-21 12:23 ` Ard Biesheuvel
@ 2018-08-21 16:50   ` Nick Desaulniers
  2018-08-21 17:00     ` Ard Biesheuvel
  0 siblings, 1 reply; 8+ messages in thread
From: Nick Desaulniers @ 2018-08-21 16:50 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Herbert Xu, David S. Miller, Catalin Marinas, Will Deacon,
	linux-crypto, Linux ARM, LKML

On Tue, Aug 21, 2018 at 5:23 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> Hi Nick,
>
> On 21 August 2018 at 00:40, Nick Desaulniers <ndesaulniers@google.com> wrote:
> > It seems that:
> > ldr q8, =0x30000000200000001
> >
> > is a GNU as convience notation for:
> > ldr q8, .Lconstant
> > .Lconstant
> > .word 0x00000001
> > .word 0x00000002
> > .word 0x00000003
> > .word 0x00000000
> >
> > based on this comment in binutils' source [0]. I've asked for this
> > non-standard convience notation to be added to other assemblers [1], but
> > until then, we can remove it and get equivalent disassembly:
> >
>
> What do you mean by 'non-standard convenience notation'? Which asm
> 'standard' does Clang actually claim to implement?

Well, for assembly 'standard' is a bit nebulous.  But it's frustrating
when you can't find what the `=0x...` notation means in either the ARM
or GNU as manuals.  The source of truth happened to be a comment in
the source [0] that explained that this was a "programmer friendly
notation."  Sure, if you can find it.

>
> This 'GCC/binutils is broken and we are reluctant to subvert Clang'
> attitude is getting a bit old tbh. In the future, could you please
> mention clang explicitly in the patch subject so it is obvious what
> the purpose of the patch is? I think we should accommodate Clang in
> Linux, but the attitude has got to go.

'Reluctant?' Please assume good faith; I filed the bug/noticed
yesterday [1]. And I'm in favor of supporting the shorthand.  That's
not my argument at all; Strawman.

>
>
> > before:
> > 00000000000009d4 <neon_aes_ctr_encrypt>:
> > ...
> >      a48:       9c000ac8        ldr     q8, ba0 <neon_aes_ctr_encrypt+0x1cc>
> > ...
> >      ba0:       00000001        .word   0x00000001
> >      ba4:       00000002        .word   0x00000002
> >      ba8:       00000003        .word   0x00000003
> >      bac:       00000000        .word   0x00000000
> >
> > after:
> >
> > 00000000000009d4 <neon_aes_ctr_encrypt>:
> > ...
> >      a48:       9c000aa8        ldr     q8, b9c <neon_aes_ctr_encrypt+0x1c8>
> > ...
> >      b9c:       00000001        .word   0x00000001
> >      ba0:       00000002        .word   0x00000002
> >      ba4:       00000003        .word   0x00000003
> >      ba8:       00000000        .word   0x00000000
> >
> > [0] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gas/testsuite/gas/aarch64/programmer-friendly.s;h=6254c6476efdc848648b05068be0574e7addc85d;hb=HEAD#l11
> > [1] https://bugs.llvm.org/show_bug.cgi?id=38642
> >
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > ---
> >  arch/arm64/crypto/aes-modes.S | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/crypto/aes-modes.S b/arch/arm64/crypto/aes-modes.S
> > index 483a7130cf0e..9288c5b0eca2 100644
> > --- a/arch/arm64/crypto/aes-modes.S
> > +++ b/arch/arm64/crypto/aes-modes.S
> > @@ -232,7 +232,7 @@ AES_ENTRY(aes_ctr_encrypt)
> >         bmi             .Lctr1x
> >         cmn             w6, #4                  /* 32 bit overflow? */
> >         bcs             .Lctr1x
> > -       ldr             q8, =0x30000000200000001        /* addends 1,2,3[,0] */
> > +       ldr             q8, .Laddends /* addends 1,2,3[,0] */
> >         dup             v7.4s, w6
> >         mov             v0.16b, v4.16b
> >         add             v7.4s, v7.4s, v8.4s
> > @@ -295,6 +295,12 @@ AES_ENTRY(aes_ctr_encrypt)
> >         rev             x7, x7
> >         ins             v4.d[0], x7
> >         b               .Lctrcarrydone
> > +
> > +.Laddends:
> > +       .word   0x00000001
> > +       .word   0x00000002
> > +       .word   0x00000003
> > +       .word   0x00000000
>


> As Will points out, this breaks BE builds.

Looks like -EB and -EL are the flags for me to test with in GNU as
(looks like a few different flags for this).  Is there a target triple
ABI for BE?  I'll ask around here too, and post my findings.

>
> >  AES_ENDPROC(aes_ctr_encrypt)
> >         .ltorg
>
> You can drop this .ltorg if you get rid of all =xxx instances.
>
> Actually, though, I would prefer it if we could use some clever short
> sequence of movi+add instructions, and get rid of the literal
> altogether. Or otherwise, please use something like
>
> ldr_l q8, .Laddends, <temp register>
>
> instead, and move the literal into the .rodata section (and use .octa
> so you don't break BE)

Ah, .octa, neat.  Ok, I'll take a look for v2.

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64/crypto: remove non-standard notation
  2018-08-21 16:50   ` Nick Desaulniers
@ 2018-08-21 17:00     ` Ard Biesheuvel
  2018-08-21 17:25       ` Robin Murphy
  0 siblings, 1 reply; 8+ messages in thread
From: Ard Biesheuvel @ 2018-08-21 17:00 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Herbert Xu, David S. Miller, Catalin Marinas, Will Deacon,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Linux ARM, LKML

On 21 August 2018 at 18:50, Nick Desaulniers <ndesaulniers@google.com> wrote:
> On Tue, Aug 21, 2018 at 5:23 AM Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>>
>> Hi Nick,
>>
>> On 21 August 2018 at 00:40, Nick Desaulniers <ndesaulniers@google.com> wrote:
>> > It seems that:
>> > ldr q8, =0x30000000200000001
>> >
>> > is a GNU as convience notation for:
>> > ldr q8, .Lconstant
>> > .Lconstant
>> > .word 0x00000001
>> > .word 0x00000002
>> > .word 0x00000003
>> > .word 0x00000000
>> >
>> > based on this comment in binutils' source [0]. I've asked for this
>> > non-standard convience notation to be added to other assemblers [1], but
>> > until then, we can remove it and get equivalent disassembly:
>> >
>>
>> What do you mean by 'non-standard convenience notation'? Which asm
>> 'standard' does Clang actually claim to implement?
>
> Well, for assembly 'standard' is a bit nebulous.  But it's frustrating
> when you can't find what the `=0x...` notation means in either the ARM
> or GNU as manuals.  The source of truth happened to be a comment in
> the source [0] that explained that this was a "programmer friendly
> notation."  Sure, if you can find it.
>

Well, it is documented in the ARM manuals as the 'ldr pseudo
instruction' for 32-bit ARM, and originally, it would only emit a
literal if the value could not be loaded using a ordinary mov.

For AArch64, I don't think that occurs any longer, i.e., a ldr is
always emitted as an ldr. However, it is used widely in the ARM code
(although not as much in arch/arm64) for loading compile time
constants and even symbol addresses into general purpose registers.
The FP variant may actually be a GNU invention, but given that there
is no standard to begin with, playing the 'GNU is non-standard' card
again is just a bit annoying.

>>
>> This 'GCC/binutils is broken and we are reluctant to subvert Clang'
>> attitude is getting a bit old tbh. In the future, could you please
>> mention clang explicitly in the patch subject so it is obvious what
>> the purpose of the patch is? I think we should accommodate Clang in
>> Linux, but the attitude has got to go.
>
> 'Reluctant?' Please assume good faith; I filed the bug/noticed
> yesterday [1]. And I'm in favor of supporting the shorthand.  That's
> not my argument at all; Strawman.
>

OK, fair enough. But note that Clang already supports the syntax for GPRs.

>>
>>
>> > before:
>> > 00000000000009d4 <neon_aes_ctr_encrypt>:
>> > ...
>> >      a48:       9c000ac8        ldr     q8, ba0 <neon_aes_ctr_encrypt+0x1cc>
>> > ...
>> >      ba0:       00000001        .word   0x00000001
>> >      ba4:       00000002        .word   0x00000002
>> >      ba8:       00000003        .word   0x00000003
>> >      bac:       00000000        .word   0x00000000
>> >
>> > after:
>> >
>> > 00000000000009d4 <neon_aes_ctr_encrypt>:
>> > ...
>> >      a48:       9c000aa8        ldr     q8, b9c <neon_aes_ctr_encrypt+0x1c8>
>> > ...
>> >      b9c:       00000001        .word   0x00000001
>> >      ba0:       00000002        .word   0x00000002
>> >      ba4:       00000003        .word   0x00000003
>> >      ba8:       00000000        .word   0x00000000
>> >
>> > [0] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gas/testsuite/gas/aarch64/programmer-friendly.s;h=6254c6476efdc848648b05068be0574e7addc85d;hb=HEAD#l11
>> > [1] https://bugs.llvm.org/show_bug.cgi?id=38642
>> >
>> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>> > ---
>> >  arch/arm64/crypto/aes-modes.S | 8 +++++++-
>> >  1 file changed, 7 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/arch/arm64/crypto/aes-modes.S b/arch/arm64/crypto/aes-modes.S
>> > index 483a7130cf0e..9288c5b0eca2 100644
>> > --- a/arch/arm64/crypto/aes-modes.S
>> > +++ b/arch/arm64/crypto/aes-modes.S
>> > @@ -232,7 +232,7 @@ AES_ENTRY(aes_ctr_encrypt)
>> >         bmi             .Lctr1x
>> >         cmn             w6, #4                  /* 32 bit overflow? */
>> >         bcs             .Lctr1x
>> > -       ldr             q8, =0x30000000200000001        /* addends 1,2,3[,0] */
>> > +       ldr             q8, .Laddends /* addends 1,2,3[,0] */
>> >         dup             v7.4s, w6
>> >         mov             v0.16b, v4.16b
>> >         add             v7.4s, v7.4s, v8.4s
>> > @@ -295,6 +295,12 @@ AES_ENTRY(aes_ctr_encrypt)
>> >         rev             x7, x7
>> >         ins             v4.d[0], x7
>> >         b               .Lctrcarrydone
>> > +
>> > +.Laddends:
>> > +       .word   0x00000001
>> > +       .word   0x00000002
>> > +       .word   0x00000003
>> > +       .word   0x00000000
>>
>
>
>> As Will points out, this breaks BE builds.
>
> Looks like -EB and -EL are the flags for me to test with in GNU as
> (looks like a few different flags for this).  Is there a target triple
> ABI for BE?  I'll ask around here too, and post my findings.
>
>>
>> >  AES_ENDPROC(aes_ctr_encrypt)
>> >         .ltorg
>>
>> You can drop this .ltorg if you get rid of all =xxx instances.
>>
>> Actually, though, I would prefer it if we could use some clever short
>> sequence of movi+add instructions, and get rid of the literal
>> altogether. Or otherwise, please use something like
>>
>> ldr_l q8, .Laddends, <temp register>
>>
>> instead, and move the literal into the .rodata section (and use .octa
>> so you don't break BE)
>
> Ah, .octa, neat.  Ok, I'll take a look for v2.
>

Please check my solution first. I just sent it out.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64/crypto: remove non-standard notation
  2018-08-21 17:00     ` Ard Biesheuvel
@ 2018-08-21 17:25       ` Robin Murphy
  2018-08-21 18:13         ` Nick Desaulniers
  0 siblings, 1 reply; 8+ messages in thread
From: Robin Murphy @ 2018-08-21 17:25 UTC (permalink / raw)
  To: Ard Biesheuvel, Nick Desaulniers
  Cc: Herbert Xu, Catalin Marinas, Will Deacon, LKML,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE, David S. Miller,
	Linux ARM

On 21/08/18 18:00, Ard Biesheuvel wrote:
> On 21 August 2018 at 18:50, Nick Desaulniers <ndesaulniers@google.com> wrote:
>> On Tue, Aug 21, 2018 at 5:23 AM Ard Biesheuvel
>> <ard.biesheuvel@linaro.org> wrote:
>>>
>>> Hi Nick,
>>>
>>> On 21 August 2018 at 00:40, Nick Desaulniers <ndesaulniers@google.com> wrote:
>>>> It seems that:
>>>> ldr q8, =0x30000000200000001
>>>>
>>>> is a GNU as convience notation for:
>>>> ldr q8, .Lconstant
>>>> .Lconstant
>>>> .word 0x00000001
>>>> .word 0x00000002
>>>> .word 0x00000003
>>>> .word 0x00000000
>>>>
>>>> based on this comment in binutils' source [0]. I've asked for this
>>>> non-standard convience notation to be added to other assemblers [1], but
>>>> until then, we can remove it and get equivalent disassembly:
>>>>
>>>
>>> What do you mean by 'non-standard convenience notation'? Which asm
>>> 'standard' does Clang actually claim to implement?
>>
>> Well, for assembly 'standard' is a bit nebulous.  But it's frustrating
>> when you can't find what the `=0x...` notation means in either the ARM
>> or GNU as manuals.  The source of truth happened to be a comment in
>> the source [0] that explained that this was a "programmer friendly
>> notation."  Sure, if you can find it.
>>
> 
> Well, it is documented in the ARM manuals as the 'ldr pseudo
> instruction' for 32-bit ARM, and originally, it would only emit a
> literal if the value could not be loaded using a ordinary mov.

FWIW It's certainly well-documented by common A64 assemblers too:

http://infocenter.arm.com/help/topic/com.arm.doc.100069_0610_01_en/dom1395139540926.html

https://sourceware.org/binutils/docs/as/AArch64-Opcodes.html#AArch64-Opcodes

> For AArch64, I don't think that occurs any longer, i.e., a ldr is
> always emitted as an ldr. However, it is used widely in the ARM code
> (although not as much in arch/arm64) for loading compile time
> constants and even symbol addresses into general purpose registers.
> The FP variant may actually be a GNU invention, but given that there
> is no standard to begin with, playing the 'GNU is non-standard' card
> again is just a bit annoying.

I note that GAS just says "register" rather than explicitly calling out 
GPRs as armasm does, so in some ways it's not even all that unexpected 
that FP registers also work there, even if it is a bit funky.

Robin.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64/crypto: remove non-standard notation
  2018-08-21 17:25       ` Robin Murphy
@ 2018-08-21 18:13         ` Nick Desaulniers
  0 siblings, 0 replies; 8+ messages in thread
From: Nick Desaulniers @ 2018-08-21 18:13 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Ard Biesheuvel, Herbert Xu, Catalin Marinas, Will Deacon, LKML,
	linux-crypto, David S. Miller, Linux ARM

On Tue, Aug 21, 2018 at 10:25 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 21/08/18 18:00, Ard Biesheuvel wrote:
> > On 21 August 2018 at 18:50, Nick Desaulniers <ndesaulniers@google.com> wrote:
> >> On Tue, Aug 21, 2018 at 5:23 AM Ard Biesheuvel
> >> <ard.biesheuvel@linaro.org> wrote:
> >>>
> >>> Hi Nick,
> >>>
> >>> On 21 August 2018 at 00:40, Nick Desaulniers <ndesaulniers@google.com> wrote:
> >>>> It seems that:
> >>>> ldr q8, =0x30000000200000001
> >>>>
> >>>> is a GNU as convience notation for:
> >>>> ldr q8, .Lconstant
> >>>> .Lconstant
> >>>> .word 0x00000001
> >>>> .word 0x00000002
> >>>> .word 0x00000003
> >>>> .word 0x00000000
> >>>>
> >>>> based on this comment in binutils' source [0]. I've asked for this
> >>>> non-standard convience notation to be added to other assemblers [1], but
> >>>> until then, we can remove it and get equivalent disassembly:
> >>>>
> >>>
> >>> What do you mean by 'non-standard convenience notation'? Which asm
> >>> 'standard' does Clang actually claim to implement?
> >>
> >> Well, for assembly 'standard' is a bit nebulous.  But it's frustrating
> >> when you can't find what the `=0x...` notation means in either the ARM
> >> or GNU as manuals.  The source of truth happened to be a comment in
> >> the source [0] that explained that this was a "programmer friendly
> >> notation."  Sure, if you can find it.
> >>
> >
> > Well, it is documented in the ARM manuals as the 'ldr pseudo
> > instruction' for 32-bit ARM, and originally, it would only emit a
> > literal if the value could not be loaded using a ordinary mov.
>
> FWIW It's certainly well-documented by common A64 assemblers too:
>
> http://infocenter.arm.com/help/topic/com.arm.doc.100069_0610_01_en/dom1395139540926.html
>
> https://sourceware.org/binutils/docs/as/AArch64-Opcodes.html#AArch64-Opcodes

Thanks for those links, I've added them to the LLVM bug to show that
this is documented behavior.
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-08-21 18:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-20 22:40 [PATCH] arm64/crypto: remove non-standard notation Nick Desaulniers
2018-08-20 22:43 ` Nick Desaulniers
2018-08-21  0:04 ` Will Deacon
2018-08-21 12:23 ` Ard Biesheuvel
2018-08-21 16:50   ` Nick Desaulniers
2018-08-21 17:00     ` Ard Biesheuvel
2018-08-21 17:25       ` Robin Murphy
2018-08-21 18:13         ` Nick Desaulniers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).