Re: [PATCH 3/7] openrisc: restore call-saved regs on sigreturn

[Stafford Horne]

On Mon, 19 Sep 2016, Stafford Horne wrote:




On Mon, 19 Sep 2016, Jonas Bonn wrote:


 On 09/16/2016 04:43 PM, Stafford Horne wrote:
>   From: Jonas Bonn 
> 
>   Return to userspace via _resume_userspace instead of via syscall return

>   path for the rt_sigreturn syscall.
> 
>   I'll rework this comment more later, but this patch needs testing.


 This whole patch was reworked later... what happened to that work? I
 recall it being posted for testing without feedback...

 In any case, the patch comment needs cleaning up.


Hi Jonas,
When working through the changelogs to pull out these patches I didnt find 
anything more recent.  However, I did only notice the commit message needed 
cleanup after posting the patch set.  It was on my todo list to clean it up.


I will have another look for a later patch, I didnt find at first. But thanks 
for the info.


Hi Jonas,

I checked up on this and cannot find any more recent patch.  Do you have 
any details? It seems this patch was discussed off mailing lists as I 
wasn't able to find it anywhere other than a 3 way conversation sent to 
me by Stefan.


I can understand what this change is doing, but I find that ptrace 
is not working so well after applying it. Which might be related to what 
Sebastian Macke was seeing when he found strace was not working.


In the mean time I am trying to fix the issue, but if you have the revised 
patch it would be helpful.


-Stafford

> 
>   Old comment from previous patch:
> 
>   The sigreturn syscall is more like a context switch than a function 
>   call;

>   it entails a return from one context (the signal handler) to another
>   (the process in question).  For a context switch like this there are
>   effectively no call-saved regs that remain constant across the 
>   transition.
> 
>   This patch restores the call-saved regs from pt_regs before returning 
>   from
>   the syscall, effectively restoring the context that the process had 
>   before

>   being interrupted by the signal handler.  Restoring the call-saved regs
>   in this way allows us to return to userspace via the usual syscall fast
>   path.
> 
>   Reported-by: Sebastian Macke 

>   Signed-off-by: Jonas Bonn 
>   Signed-off-by: Stafford Horne 
>   ---
> arch/openrisc/kernel/entry.S | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
> 
>   diff --git a/arch/openrisc/kernel/entry.S 
>   b/arch/openrisc/kernel/entry.S

>   index fec8bf9..572d223 100644
>   --- a/arch/openrisc/kernel/entry.S
>   +++ b/arch/openrisc/kernel/entry.S
>   @@ -1101,8 +1101,16 @@ ENTRY(__sys_fork)
>   l.addi   r3,r1,0
> 
> ENTRY(sys_rt_sigreturn)

>   -l.j _sys_rt_sigreturn
>   +l.jal   _sys_rt_sigreturn
> l.addi r3,r1,0
>   +l.sfne  r30,r0
>   +l.bnf   _no_syscall_trace
>   + l.nop
>   +l.jal   do_syscall_trace_leave
>   + l.addi r3,r1,0
>   +_no_syscall_trace:
>   +l.j _resume_userspace
>   + l.nop
> 
> /* This is a catch-all syscall for atomic instructions for the 
> OpenRISC

> 1000.
>  * The functions takes a variable number of parameters depending on
>  which

Re: [PATCH 3/7] openrisc: restore call-saved regs on sigreturn

[Stafford Horne]

On Mon, 19 Sep 2016, Stafford Horne wrote:




On Mon, 19 Sep 2016, Jonas Bonn wrote:


 On 09/16/2016 04:43 PM, Stafford Horne wrote:
>   From: Jonas Bonn 
> 
>   Return to userspace via _resume_userspace instead of via syscall return

>   path for the rt_sigreturn syscall.
> 
>   I'll rework this comment more later, but this patch needs testing.


 This whole patch was reworked later... what happened to that work? I
 recall it being posted for testing without feedback...

 In any case, the patch comment needs cleaning up.


Hi Jonas,
When working through the changelogs to pull out these patches I didnt find 
anything more recent.  However, I did only notice the commit message needed 
cleanup after posting the patch set.  It was on my todo list to clean it up.


I will have another look for a later patch, I didnt find at first. But thanks 
for the info.


Hi Jonas,

I checked up on this and cannot find any more recent patch.  Do you have 
any details? It seems this patch was discussed off mailing lists as I 
wasn't able to find it anywhere other than a 3 way conversation sent to 
me by Stefan.


I can understand what this change is doing, but I find that ptrace 
is not working so well after applying it. Which might be related to what 
Sebastian Macke was seeing when he found strace was not working.


In the mean time I am trying to fix the issue, but if you have the revised 
patch it would be helpful.


-Stafford

> 
>   Old comment from previous patch:
> 
>   The sigreturn syscall is more like a context switch than a function 
>   call;

>   it entails a return from one context (the signal handler) to another
>   (the process in question).  For a context switch like this there are
>   effectively no call-saved regs that remain constant across the 
>   transition.
> 
>   This patch restores the call-saved regs from pt_regs before returning 
>   from
>   the syscall, effectively restoring the context that the process had 
>   before

>   being interrupted by the signal handler.  Restoring the call-saved regs
>   in this way allows us to return to userspace via the usual syscall fast
>   path.
> 
>   Reported-by: Sebastian Macke 

>   Signed-off-by: Jonas Bonn 
>   Signed-off-by: Stafford Horne 
>   ---
> arch/openrisc/kernel/entry.S | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
> 
>   diff --git a/arch/openrisc/kernel/entry.S 
>   b/arch/openrisc/kernel/entry.S

>   index fec8bf9..572d223 100644
>   --- a/arch/openrisc/kernel/entry.S
>   +++ b/arch/openrisc/kernel/entry.S
>   @@ -1101,8 +1101,16 @@ ENTRY(__sys_fork)
>   l.addi   r3,r1,0
> 
> ENTRY(sys_rt_sigreturn)

>   -l.j _sys_rt_sigreturn
>   +l.jal   _sys_rt_sigreturn
> l.addi r3,r1,0
>   +l.sfne  r30,r0
>   +l.bnf   _no_syscall_trace
>   + l.nop
>   +l.jal   do_syscall_trace_leave
>   + l.addi r3,r1,0
>   +_no_syscall_trace:
>   +l.j _resume_userspace
>   + l.nop
> 
> /* This is a catch-all syscall for atomic instructions for the 
> OpenRISC

> 1000.
>  * The functions takes a variable number of parameters depending on
>  which

Re: [PATCH 3/7] openrisc: restore call-saved regs on sigreturn

[Stafford Horne]

On Mon, 19 Sep 2016, Jonas Bonn wrote:


On 09/16/2016 04:43 PM, Stafford Horne wrote:

 From: Jonas Bonn 

 Return to userspace via _resume_userspace instead of via syscall return
 path for the rt_sigreturn syscall.

 I'll rework this comment more later, but this patch needs testing.


This whole patch was reworked later... what happened to that work? I recall 
it being posted for testing without feedback...


In any case, the patch comment needs cleaning up.


Hi Jonas,
When working through the changelogs to pull out these patches I didnt find 
anything more recent.  However, I did only notice the commit message 
needed cleanup after posting the patch set.  It was on my todo list to 
clean it up.


I will have another look for a later patch, I didnt find at first. But 
thanks for the info.


-Stafford



 Old comment from previous patch:

 The sigreturn syscall is more like a context switch than a function call;
 it entails a return from one context (the signal handler) to another
 (the process in question).  For a context switch like this there are
 effectively no call-saved regs that remain constant across the transition.

 This patch restores the call-saved regs from pt_regs before returning from
 the syscall, effectively restoring the context that the process had before
 being interrupted by the signal handler.  Restoring the call-saved regs
 in this way allows us to return to userspace via the usual syscall fast
 path.

 Reported-by: Sebastian Macke 
 Signed-off-by: Jonas Bonn 
 Signed-off-by: Stafford Horne 
 ---
   arch/openrisc/kernel/entry.S | 10 +-
   1 file changed, 9 insertions(+), 1 deletion(-)

 diff --git a/arch/openrisc/kernel/entry.S b/arch/openrisc/kernel/entry.S
 index fec8bf9..572d223 100644
 --- a/arch/openrisc/kernel/entry.S
 +++ b/arch/openrisc/kernel/entry.S
 @@ -1101,8 +1101,16 @@ ENTRY(__sys_fork)
 l.addi r3,r1,0

   ENTRY(sys_rt_sigreturn)
 -  l.j _sys_rt_sigreturn
 +  l.jal   _sys_rt_sigreturn
 l.addi r3,r1,0
 +  l.sfne  r30,r0
 +  l.bnf   _no_syscall_trace
 +   l.nop
 +  l.jal   do_syscall_trace_leave
 +   l.addi r3,r1,0
 +_no_syscall_trace:
 +  l.j _resume_userspace
 +   l.nop

   /* This is a catch-all syscall for atomic instructions for the OpenRISC
   1000.
* The functions takes a variable number of parameters depending on
which

Re: [PATCH 3/7] openrisc: restore call-saved regs on sigreturn

[Stafford Horne]

On Mon, 19 Sep 2016, Jonas Bonn wrote:


On 09/16/2016 04:43 PM, Stafford Horne wrote:

 From: Jonas Bonn 

 Return to userspace via _resume_userspace instead of via syscall return
 path for the rt_sigreturn syscall.

 I'll rework this comment more later, but this patch needs testing.


This whole patch was reworked later... what happened to that work? I recall 
it being posted for testing without feedback...


In any case, the patch comment needs cleaning up.


Hi Jonas,
When working through the changelogs to pull out these patches I didnt find 
anything more recent.  However, I did only notice the commit message 
needed cleanup after posting the patch set.  It was on my todo list to 
clean it up.


I will have another look for a later patch, I didnt find at first. But 
thanks for the info.


-Stafford



 Old comment from previous patch:

 The sigreturn syscall is more like a context switch than a function call;
 it entails a return from one context (the signal handler) to another
 (the process in question).  For a context switch like this there are
 effectively no call-saved regs that remain constant across the transition.

 This patch restores the call-saved regs from pt_regs before returning from
 the syscall, effectively restoring the context that the process had before
 being interrupted by the signal handler.  Restoring the call-saved regs
 in this way allows us to return to userspace via the usual syscall fast
 path.

 Reported-by: Sebastian Macke 
 Signed-off-by: Jonas Bonn 
 Signed-off-by: Stafford Horne 
 ---
   arch/openrisc/kernel/entry.S | 10 +-
   1 file changed, 9 insertions(+), 1 deletion(-)

 diff --git a/arch/openrisc/kernel/entry.S b/arch/openrisc/kernel/entry.S
 index fec8bf9..572d223 100644
 --- a/arch/openrisc/kernel/entry.S
 +++ b/arch/openrisc/kernel/entry.S
 @@ -1101,8 +1101,16 @@ ENTRY(__sys_fork)
 l.addi r3,r1,0

   ENTRY(sys_rt_sigreturn)
 -  l.j _sys_rt_sigreturn
 +  l.jal   _sys_rt_sigreturn
 l.addi r3,r1,0
 +  l.sfne  r30,r0
 +  l.bnf   _no_syscall_trace
 +   l.nop
 +  l.jal   do_syscall_trace_leave
 +   l.addi r3,r1,0
 +_no_syscall_trace:
 +  l.j _resume_userspace
 +   l.nop

   /* This is a catch-all syscall for atomic instructions for the OpenRISC
   1000.
* The functions takes a variable number of parameters depending on
which

Re: [PATCH 3/7] openrisc: restore call-saved regs on sigreturn

[Jonas Bonn]

On 09/16/2016 04:43 PM, Stafford Horne wrote:

From: Jonas Bonn 

Return to userspace via _resume_userspace instead of via syscall return
path for the rt_sigreturn syscall.

I'll rework this comment more later, but this patch needs testing.


This whole patch was reworked later... what happened to that work? I 
recall it being posted for testing without feedback...


In any case, the patch comment needs cleaning up.

/Jonas



Old comment from previous patch:

The sigreturn syscall is more like a context switch than a function call;
it entails a return from one context (the signal handler) to another
(the process in question).  For a context switch like this there are
effectively no call-saved regs that remain constant across the transition.

This patch restores the call-saved regs from pt_regs before returning from
the syscall, effectively restoring the context that the process had before
being interrupted by the signal handler.  Restoring the call-saved regs
in this way allows us to return to userspace via the usual syscall fast
path.

Reported-by: Sebastian Macke 
Signed-off-by: Jonas Bonn 
Signed-off-by: Stafford Horne 
---
  arch/openrisc/kernel/entry.S | 10 +-
  1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/openrisc/kernel/entry.S b/arch/openrisc/kernel/entry.S
index fec8bf9..572d223 100644
--- a/arch/openrisc/kernel/entry.S
+++ b/arch/openrisc/kernel/entry.S
@@ -1101,8 +1101,16 @@ ENTRY(__sys_fork)
 l.addi r3,r1,0
  
  ENTRY(sys_rt_sigreturn)

-   l.j _sys_rt_sigreturn
+   l.jal   _sys_rt_sigreturn
 l.addi r3,r1,0
+   l.sfne  r30,r0
+   l.bnf   _no_syscall_trace
+l.nop
+   l.jal   do_syscall_trace_leave
+l.addi r3,r1,0
+_no_syscall_trace:
+   l.j _resume_userspace
+l.nop
  
  /* This is a catch-all syscall for atomic instructions for the OpenRISC 1000.

   * The functions takes a variable number of parameters depending on which

Re: [PATCH 3/7] openrisc: restore call-saved regs on sigreturn

[Jonas Bonn]

On 09/16/2016 04:43 PM, Stafford Horne wrote:

From: Jonas Bonn 

Return to userspace via _resume_userspace instead of via syscall return
path for the rt_sigreturn syscall.

I'll rework this comment more later, but this patch needs testing.


This whole patch was reworked later... what happened to that work? I 
recall it being posted for testing without feedback...


In any case, the patch comment needs cleaning up.

/Jonas



Old comment from previous patch:

The sigreturn syscall is more like a context switch than a function call;
it entails a return from one context (the signal handler) to another
(the process in question).  For a context switch like this there are
effectively no call-saved regs that remain constant across the transition.

This patch restores the call-saved regs from pt_regs before returning from
the syscall, effectively restoring the context that the process had before
being interrupted by the signal handler.  Restoring the call-saved regs
in this way allows us to return to userspace via the usual syscall fast
path.

Reported-by: Sebastian Macke 
Signed-off-by: Jonas Bonn 
Signed-off-by: Stafford Horne 
---
  arch/openrisc/kernel/entry.S | 10 +-
  1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/openrisc/kernel/entry.S b/arch/openrisc/kernel/entry.S
index fec8bf9..572d223 100644
--- a/arch/openrisc/kernel/entry.S
+++ b/arch/openrisc/kernel/entry.S
@@ -1101,8 +1101,16 @@ ENTRY(__sys_fork)
 l.addi r3,r1,0
  
  ENTRY(sys_rt_sigreturn)

-   l.j _sys_rt_sigreturn
+   l.jal   _sys_rt_sigreturn
 l.addi r3,r1,0
+   l.sfne  r30,r0
+   l.bnf   _no_syscall_trace
+l.nop
+   l.jal   do_syscall_trace_leave
+l.addi r3,r1,0
+_no_syscall_trace:
+   l.j _resume_userspace
+l.nop
  
  /* This is a catch-all syscall for atomic instructions for the OpenRISC 1000.

   * The functions takes a variable number of parameters depending on which