Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1651112 > unrolled thread
| Started by | Jiri Slaby <jslaby@suse.cz> |
|---|---|
| First post | 2017-05-26 09:00 +0200 |
| Last post | 2017-05-26 14:20 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH 7/7] DWARF: add the config option Jiri Slaby <jslaby@suse.cz> - 2017-05-26 09:00 +0200
Re: [PATCH 7/7] DWARF: add the config option Jiri Slaby <jslaby@suse.cz> - 2017-05-26 13:40 +0200
Re: [PATCH 7/7] DWARF: add the config option Josh Poimboeuf <jpoimboe@redhat.com> - 2017-05-26 14:20 +0200
| From | Jiri Slaby <jslaby@suse.cz> |
|---|---|
| Date | 2017-05-26 09:00 +0200 |
| Subject | Re: [PATCH 7/7] DWARF: add the config option |
| Message-ID | <tLho6-2vy-11@gated-at.bofh.it> |
On 05/19/2017, 11:35 PM, Josh Poimboeuf wrote:
> https://github.com/jpoimboe/linux/blob/undwarf/arch/x86/kernel/unwind_undwarf.c
JFYI, it crashes in sha1_transform_avx due to crypto changes. You
perhaps missed that this beast uses ebp (not rbp) register for
computations. I had to do:
--- a/arch/x86/crypto/sha1_ssse3_asm.S
+++ b/arch/x86/crypto/sha1_ssse3_asm.S
@@ -37,7 +37,7 @@
#define REG_A %ecx
#define REG_B %esi
#define REG_C %edi
-#define REG_D %ebp
+#define REG_D %r12d
#define REG_E %edx
#define REG_T1 %eax
@@ -74,6 +74,7 @@
SYM_FUNC_START(\name)
push %rbx
+ push %r12
push %rbp
mov %rsp, %rbp
@@ -99,6 +100,7 @@
rep stosq
leaveq # deallocate workspace
+ pop %r12
pop %rbx
ret
I am afraid there are more of these, e.g. in aesni-intel_asm.S.
thanks,
--
js
suse labs
[toc] | [next] | [standalone]
| From | Jiri Slaby <jslaby@suse.cz> |
|---|---|
| Date | 2017-05-26 13:40 +0200 |
| Message-ID | <tLlL3-5kc-3@gated-at.bofh.it> |
| In reply to | #1651112 |
On 05/26/2017, 08:54 AM, Jiri Slaby wrote:
> On 05/19/2017, 11:35 PM, Josh Poimboeuf wrote:
>> https://github.com/jpoimboe/linux/blob/undwarf/arch/x86/kernel/unwind_undwarf.c
>
> JFYI, it crashes in sha1_transform_avx due to crypto changes. You
> perhaps missed that this beast uses ebp (not rbp) register for
> computations. I had to do:
>
> --- a/arch/x86/crypto/sha1_ssse3_asm.S
> +++ b/arch/x86/crypto/sha1_ssse3_asm.S
> @@ -37,7 +37,7 @@
> #define REG_A %ecx
> #define REG_B %esi
> #define REG_C %edi
> -#define REG_D %ebp
> +#define REG_D %r12d
> #define REG_E %edx
>
> #define REG_T1 %eax
> @@ -74,6 +74,7 @@
> SYM_FUNC_START(\name)
>
> push %rbx
> + push %r12
> push %rbp
>
> mov %rsp, %rbp
> @@ -99,6 +100,7 @@
> rep stosq
>
> leaveq # deallocate workspace
> + pop %r12
> pop %rbx
> ret
>
>
> I am afraid there are more of these, e.g. in aesni-intel_asm.S.
aesni-intel_asm.S is OK -- only untouched x86_32 part uses ebp.
But sha1_avx2_x86_64_asm.S is not. They use *all* usable registers
including ebp in the computations hidden behind the
SHA1_PIPELINED_MAIN_BODY macro. The only work around I can see is to
push rbp/pop rbp around the computation as it used to do with rbx:
--- a/arch/x86/crypto/sha1_avx2_x86_64_asm.S
+++ b/arch/x86/crypto/sha1_avx2_x86_64_asm.S
@@ -636,6 +636,7 @@ _loop3:
/* Align stack */
mov %rsp, %rbp
and $~(0x20-1), %rsp
+ push %rbp
sub $RESERVE_STACK, %rsp
avx2_zeroupper
@@ -661,6 +662,7 @@ _loop3:
avx2_zeroupper
add $RESERVE_STACK, %rsp
+ pop %rbp
leaveq
pop %r15
regards,
--
js
suse labs
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2017-05-26 14:20 +0200 |
| Message-ID | <tLmnN-5Oa-21@gated-at.bofh.it> |
| In reply to | #1651325 |
On Fri, May 26, 2017 at 01:29:01PM +0200, Jiri Slaby wrote: > On 05/26/2017, 08:54 AM, Jiri Slaby wrote: > > On 05/19/2017, 11:35 PM, Josh Poimboeuf wrote: > >> https://github.com/jpoimboe/linux/blob/undwarf/arch/x86/kernel/unwind_undwarf.c > > > > JFYI, it crashes in sha1_transform_avx due to crypto changes. You > > perhaps missed that this beast uses ebp (not rbp) register for > > computations. I had to do: > > > > --- a/arch/x86/crypto/sha1_ssse3_asm.S > > +++ b/arch/x86/crypto/sha1_ssse3_asm.S > > @@ -37,7 +37,7 @@ > > #define REG_A %ecx > > #define REG_B %esi > > #define REG_C %edi > > -#define REG_D %ebp > > +#define REG_D %r12d > > #define REG_E %edx > > > > #define REG_T1 %eax > > @@ -74,6 +74,7 @@ > > SYM_FUNC_START(\name) > > > > push %rbx > > + push %r12 > > push %rbp > > > > mov %rsp, %rbp > > @@ -99,6 +100,7 @@ > > rep stosq > > > > leaveq # deallocate workspace > > + pop %r12 > > pop %rbx > > ret > > > > > > I am afraid there are more of these, e.g. in aesni-intel_asm.S. > > aesni-intel_asm.S is OK -- only untouched x86_32 part uses ebp. > > But sha1_avx2_x86_64_asm.S is not. They use *all* usable registers > including ebp in the computations hidden behind the > SHA1_PIPELINED_MAIN_BODY macro. The only work around I can see is to > push rbp/pop rbp around the computation as it used to do with rbx: > > --- a/arch/x86/crypto/sha1_avx2_x86_64_asm.S > +++ b/arch/x86/crypto/sha1_avx2_x86_64_asm.S > @@ -636,6 +636,7 @@ _loop3: > /* Align stack */ > mov %rsp, %rbp > and $~(0x20-1), %rsp > + push %rbp > sub $RESERVE_STACK, %rsp > > avx2_zeroupper > @@ -661,6 +662,7 @@ _loop3: > avx2_zeroupper > > add $RESERVE_STACK, %rsp > + pop %rbp > > leaveq > pop %r15 Thanks, the first fix looks good. Is the second one needed though? It already pushes rbp before it aligns the stack. DWARF/undwarf will be immune to these issues, so I'll be moving a lot of these crypto changes to a separate branch. They were only in this branch because the new-and-improved objtool can now find rbp misusage in leaf functions. It seems that most of the crypto code is frame pointer ignorant. IMO, leaf functions shouldn't be allowed to use rbp because it breaks frame pointers when preempted by an interrupt. GCC seems to agree. I added a check to objtool to find the ones which use rbp badly. Here are the ones I see with my config: arch/x86/crypto/des3_ede-asm_64.o: warning: objtool: des3_ede_x86_64_crypt_blk uses BP as a scratch register arch/x86/crypto/des3_ede-asm_64.o: warning: objtool: des3_ede_x86_64_crypt_blk_3way uses BP as a scratch register arch/x86/crypto/blowfish-x86_64-asm_64.o: warning: objtool: __blowfish_enc_blk_4way uses BP as a scratch register arch/x86/crypto/blowfish-x86_64-asm_64.o: warning: objtool: blowfish_dec_blk_4way uses BP as a scratch register arch/x86/crypto/twofish-x86_64-asm_64-3way.o: warning: objtool: __twofish_enc_blk_3way uses BP as a scratch register arch/x86/crypto/twofish-x86_64-asm_64-3way.o: warning: objtool: twofish_dec_blk_3way uses BP as a scratch register arch/x86/crypto/sha256-avx2-asm.o: warning: objtool: sha256_transform_rorx uses BP as a scratch register arch/x86/crypto/cast5-avx-x86_64-asm_64.o: warning: objtool: __cast5_enc_blk16 uses BP as a scratch register arch/x86/crypto/cast5-avx-x86_64-asm_64.o: warning: objtool: __cast5_dec_blk16 uses BP as a scratch register arch/x86/crypto/cast6-avx-x86_64-asm_64.o: warning: objtool: __cast6_enc_blk8 uses BP as a scratch register arch/x86/crypto/cast6-avx-x86_64-asm_64.o: warning: objtool: __cast6_dec_blk8 uses BP as a scratch register arch/x86/crypto/twofish-avx-x86_64-asm_64.o: warning: objtool: __twofish_enc_blk8 uses BP as a scratch register arch/x86/crypto/twofish-avx-x86_64-asm_64.o: warning: objtool: __twofish_dec_blk8 uses BP as a scratch register (And that doesn't include the ones which misuse ebp.) It may be a challenge to fix some of those which use all available registers. -- Josh
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web