diff mbox series

[v6,02/16] linux-user/host/ppc64: Use r11 for signal_pending address

Message ID 20211123173759.1383510-3-richard.henderson@linaro.org
State New
Headers show
Series linux-user: simplify safe signal handling | expand

Commit Message

Richard Henderson Nov. 23, 2021, 5:37 p.m. UTC
We don't need a register that can live across the syscall;
we only need a register that can live until the syscall.
Use call-clobbered r11 instead of call-saved r14.
Eliminate the save and restore of r14 from the stack frame.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/host/ppc64/safe-syscall.inc.S | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Comments

Peter Maydell Nov. 29, 2021, 11:01 a.m. UTC | #1
On Tue, 23 Nov 2021 at 17:40, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We don't need a register that can live across the syscall;
> we only need a register that can live until the syscall.

What about the case where:
 * we execute the sc instruction (r11 trashed)
 * the syscall is one that from the host kernel point of
   view is restartable
 * the kernel arranges to restart the syscall by rewinding the
   PC to point to the start of the 'sc' instruction
 * our rewind_if_in_safe_syscall() rewinds PC further to
   point at safe_syscall_start
 * we want to use r11 again, but it was trashed in step 1
?

Put another way, this patch is effectively a revert of
commit 5d9f3ea081721, which was a fix to an observed bug.

-- PMM
Richard Henderson Nov. 29, 2021, 2:30 p.m. UTC | #2
On 11/29/21 12:01 PM, Peter Maydell wrote:
> On Tue, 23 Nov 2021 at 17:40, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> We don't need a register that can live across the syscall;
>> we only need a register that can live until the syscall.
> 
> What about the case where:
>   * we execute the sc instruction (r11 trashed)
>   * the syscall is one that from the host kernel point of
>     view is restartable
>   * the kernel arranges to restart the syscall by rewinding the
>     PC to point to the start of the 'sc' instruction
>   * our rewind_if_in_safe_syscall() rewinds PC further to
>     point at safe_syscall_start
>   * we want to use r11 again, but it was trashed in step 1
> ?
> 
> Put another way, this patch is effectively a revert of
> commit 5d9f3ea081721, which was a fix to an observed bug.

Whoops.  I forgot about that (a mere 3 years ago).

r~
diff mbox series

Patch

diff --git a/linux-user/host/ppc64/safe-syscall.inc.S b/linux-user/host/ppc64/safe-syscall.inc.S
index 4b57440585..5f19cd193c 100644
--- a/linux-user/host/ppc64/safe-syscall.inc.S
+++ b/linux-user/host/ppc64/safe-syscall.inc.S
@@ -49,9 +49,7 @@  safe_syscall_base:
          *               and returns the result in r3
          * Shuffle everything around appropriately.
          */
-        std     14, 16(1) /* Preserve r14 in SP+16 */
-        .cfi_offset 14, 16
-        mr      14, 3   /* signal_pending */
+        mr      11, 3   /* signal_pending */
         mr      0, 4    /* syscall number */
         mr      3, 5    /* syscall arguments */
         mr      4, 6
@@ -69,13 +67,12 @@  safe_syscall_base:
          */
 safe_syscall_start:
         /* if signal_pending is non-zero, don't do the call */
-        lwz     12, 0(14)
+        lwz     12, 0(11)
         cmpwi   0, 12, 0
         bne-    0f
         sc
 safe_syscall_end:
         /* code path when we did execute the syscall */
-        ld 14, 16(1) /* restore r14 to its original value */
         bnslr+
 
         /* syscall failed; return negative errno */
@@ -84,7 +81,6 @@  safe_syscall_end:
 
         /* code path when we didn't execute the syscall */
 0:      addi    3, 0, -TARGET_ERESTARTSYS
-        ld 14, 16(1) /* restore r14 to its original value */
         blr
         .cfi_endproc