Re: [PATCH 06/12] x86/crypto: Fix RBP usage in sha1_avx2_x86_64_asm.S

[Tim Chen]

On 08/29/2017 11:05 AM, Josh Poimboeuf wrote:
> Using RBP as a temporary register breaks frame pointer convention and
> breaks stack traces when unwinding from an interrupt in the crypto code.
> 
> Use R11 instead of RBP.  Since R11 isn't a callee-saved register, it
> doesn't need to be saved and restored on the stack.

These changes seem okay.

Thanks.

Tim

> 
> Reported-by: Eric Biggers 
> Reported-by: Peter Zijlstra 
> Signed-off-by: Josh Poimboeuf 
> ---
>  arch/x86/crypto/sha1_avx2_x86_64_asm.S | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/arch/x86/crypto/sha1_avx2_x86_64_asm.S 
> b/arch/x86/crypto/sha1_avx2_x86_64_asm.S
> index 1eab79c9ac48..9f712a7dfd79 100644
> --- a/arch/x86/crypto/sha1_avx2_x86_64_asm.S
> +++ b/arch/x86/crypto/sha1_avx2_x86_64_asm.S
> @@ -89,7 +89,7 @@
>  #define  REG_RE  %rdx
>  #define  REG_RTA %r12
>  #define  REG_RTB %rbx
> -#define  REG_T1  %ebp
> +#define  REG_T1  %r11d
>  #define  xmm_mov vmovups
>  #define  avx2_zeroupper  vzeroupper
>  #define  RND_F1  1
> @@ -637,7 +637,6 @@ _loop3:
>   ENTRY(\name)
>  
>   push%rbx
> - push%rbp
>   push%r12
>   push%r13
>   push%r14
> @@ -673,7 +672,6 @@ _loop3:
>   pop %r14
>   pop %r13
>   pop %r12
> - pop %rbp
>   pop %rbx
>  
>   ret
> 

Re: [PATCH 06/12] x86/crypto: Fix RBP usage in sha1_avx2_x86_64_asm.S

[Tim Chen]

On 08/29/2017 11:05 AM, Josh Poimboeuf wrote:
> Using RBP as a temporary register breaks frame pointer convention and
> breaks stack traces when unwinding from an interrupt in the crypto code.
> 
> Use R11 instead of RBP.  Since R11 isn't a callee-saved register, it
> doesn't need to be saved and restored on the stack.

These changes seem okay.

Thanks.

Tim

> 
> Reported-by: Eric Biggers 
> Reported-by: Peter Zijlstra 
> Signed-off-by: Josh Poimboeuf 
> ---
>  arch/x86/crypto/sha1_avx2_x86_64_asm.S | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/arch/x86/crypto/sha1_avx2_x86_64_asm.S 
> b/arch/x86/crypto/sha1_avx2_x86_64_asm.S
> index 1eab79c9ac48..9f712a7dfd79 100644
> --- a/arch/x86/crypto/sha1_avx2_x86_64_asm.S
> +++ b/arch/x86/crypto/sha1_avx2_x86_64_asm.S
> @@ -89,7 +89,7 @@
>  #define  REG_RE  %rdx
>  #define  REG_RTA %r12
>  #define  REG_RTB %rbx
> -#define  REG_T1  %ebp
> +#define  REG_T1  %r11d
>  #define  xmm_mov vmovups
>  #define  avx2_zeroupper  vzeroupper
>  #define  RND_F1  1
> @@ -637,7 +637,6 @@ _loop3:
>   ENTRY(\name)
>  
>   push%rbx
> - push%rbp
>   push%r12
>   push%r13
>   push%r14
> @@ -673,7 +672,6 @@ _loop3:
>   pop %r14
>   pop %r13
>   pop %r12
> - pop %rbp
>   pop %rbx
>  
>   ret
> 

[PATCH 06/12] x86/crypto: Fix RBP usage in sha1_avx2_x86_64_asm.S

[Josh Poimboeuf]

Using RBP as a temporary register breaks frame pointer convention and
breaks stack traces when unwinding from an interrupt in the crypto code.

Use R11 instead of RBP.  Since R11 isn't a callee-saved register, it
doesn't need to be saved and restored on the stack.

Reported-by: Eric Biggers 
Reported-by: Peter Zijlstra 
Signed-off-by: Josh Poimboeuf 
---
 arch/x86/crypto/sha1_avx2_x86_64_asm.S | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/x86/crypto/sha1_avx2_x86_64_asm.S 
b/arch/x86/crypto/sha1_avx2_x86_64_asm.S
index 1eab79c9ac48..9f712a7dfd79 100644
--- a/arch/x86/crypto/sha1_avx2_x86_64_asm.S
+++ b/arch/x86/crypto/sha1_avx2_x86_64_asm.S
@@ -89,7 +89,7 @@
 #defineREG_RE  %rdx
 #defineREG_RTA %r12
 #defineREG_RTB %rbx
-#defineREG_T1  %ebp
+#defineREG_T1  %r11d
 #definexmm_mov vmovups
 #defineavx2_zeroupper  vzeroupper
 #defineRND_F1  1
@@ -637,7 +637,6 @@ _loop3:
ENTRY(\name)
 
push%rbx
-   push%rbp
push%r12
push%r13
push%r14
@@ -673,7 +672,6 @@ _loop3:
pop %r14
pop %r13
pop %r12
-   pop %rbp
pop %rbx
 
ret
-- 
2.13.5

[PATCH 06/12] x86/crypto: Fix RBP usage in sha1_avx2_x86_64_asm.S

[Josh Poimboeuf]

Using RBP as a temporary register breaks frame pointer convention and
breaks stack traces when unwinding from an interrupt in the crypto code.

Use R11 instead of RBP.  Since R11 isn't a callee-saved register, it
doesn't need to be saved and restored on the stack.

Reported-by: Eric Biggers 
Reported-by: Peter Zijlstra 
Signed-off-by: Josh Poimboeuf 
---
 arch/x86/crypto/sha1_avx2_x86_64_asm.S | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/x86/crypto/sha1_avx2_x86_64_asm.S 
b/arch/x86/crypto/sha1_avx2_x86_64_asm.S
index 1eab79c9ac48..9f712a7dfd79 100644
--- a/arch/x86/crypto/sha1_avx2_x86_64_asm.S
+++ b/arch/x86/crypto/sha1_avx2_x86_64_asm.S
@@ -89,7 +89,7 @@
 #defineREG_RE  %rdx
 #defineREG_RTA %r12
 #defineREG_RTB %rbx
-#defineREG_T1  %ebp
+#defineREG_T1  %r11d
 #definexmm_mov vmovups
 #defineavx2_zeroupper  vzeroupper
 #defineRND_F1  1
@@ -637,7 +637,6 @@ _loop3:
ENTRY(\name)
 
push%rbx
-   push%rbp
push%r12
push%r13
push%r14
@@ -673,7 +672,6 @@ _loop3:
pop %r14
pop %r13
pop %r12
-   pop %rbp
pop %rbx
 
ret
-- 
2.13.5