diff mbox

[3/6] linux-user: Provide safe_syscall for arm

Message ID 1465854326-19160-4-git-send-email-rth@twiddle.net
State New
Headers show

Commit Message

Richard Henderson June 13, 2016, 9:45 p.m. UTC
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/host/arm/hostdep.h          | 34 ++++++++++++++
 linux-user/host/arm/safe-syscall.inc.S | 86 ++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+)
 create mode 100644 linux-user/host/arm/hostdep.h
 create mode 100644 linux-user/host/arm/safe-syscall.inc.S

Comments

Peter Maydell June 14, 2016, 12:04 p.m. UTC | #1
On 13 June 2016 at 22:45, Richard Henderson <rth@twiddle.net> wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  linux-user/host/arm/hostdep.h          | 34 ++++++++++++++
>  linux-user/host/arm/safe-syscall.inc.S | 86 ++++++++++++++++++++++++++++++++++
>  2 files changed, 120 insertions(+)
>  create mode 100644 linux-user/host/arm/hostdep.h
>  create mode 100644 linux-user/host/arm/safe-syscall.inc.S
>
> diff --git a/linux-user/host/arm/hostdep.h b/linux-user/host/arm/hostdep.h
> new file mode 100644
> index 0000000..1426fb6
> --- /dev/null
> +++ b/linux-user/host/arm/hostdep.h
> @@ -0,0 +1,34 @@
> +/*
> + * hostdep.h : things which are dependent on the host architecture
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef QEMU_HOSTDEP_H
> +#define QEMU_HOSTDEP_H
> +
> +/* We have a safe-syscall.inc.S */
> +#define HAVE_SAFE_SYSCALL
> +
> +#ifndef __ASSEMBLER__
> +
> +/* These are defined by the safe-syscall.inc.S file */
> +extern char safe_syscall_start[];
> +extern char safe_syscall_end[];
> +
> +/* Adjust the signal context to rewind out of safe-syscall if we're in it */
> +static inline void rewind_if_in_safe_syscall(void *puc)
> +{
> +    struct ucontext *uc = puc;
> +    unsigned long *pcreg = &uc->uc_mcontext.arm_pc;
> +
> +    if (*pcreg > (uintptr_t)safe_syscall_start
> +        && *pcreg < (uintptr_t)safe_syscall_end) {
> +        *pcreg = (uintptr_t)safe_syscall_start;
> +    }
> +}
> +
> +#endif /* __ASSEMBLER__ */
> +
> +#endif
> diff --git a/linux-user/host/arm/safe-syscall.inc.S b/linux-user/host/arm/safe-syscall.inc.S
> new file mode 100644
> index 0000000..52f8883
> --- /dev/null
> +++ b/linux-user/host/arm/safe-syscall.inc.S
> @@ -0,0 +1,86 @@
> +/*
> + * safe-syscall.inc.S : host-specific assembly fragment
> + * to handle signals occurring at the same time as system calls.
> + * This is intended to be included by linux-user/safe-syscall.S
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +       .global safe_syscall_base
> +       .global safe_syscall_start
> +       .global safe_syscall_end
> +       .type   safe_syscall_base, %function
> +
> +       .cfi_sections   .debug_frame
> +
> +       .text
> +       .syntax unified
> +       .arm

Do we need a ".align 2" here? glibc has one.

> +
> +       /* This is the entry point for making a system call. The calling
> +        * convention here is that of a C varargs function with the
> +        * first argument an 'int *' to the signal_pending flag, the
> +        * second one the system call number (as a 'long'), and all further
> +        * arguments being syscall arguments (also 'long').
> +        * We return a long which is the syscall's return value, which
> +        * may be negative-errno on failure. Conversion to the
> +        * -1-and-errno-set convention is done by the calling wrapper.
> +        */
> +safe_syscall_base:
> +       .fnstart
> +       .cfi_startproc
> +       mov     ip, sp                  /* save entry stack */

Personally I find the numbered registers like "r12" easier to read than
the named versions like "ip" (I always have to look the latter up
to find out which register they actually are, so it saves effort
to just write r12 in the first place IMHO.)

> +       push    { r4, r5, r6, r7, r8, lr }
> +       .save   { r4, r5, r6, r7, r8, lr }
> +       .cfi_adjust_cfa_offset 24
> +       .cfi_rel_offset r4, 0
> +       .cfi_rel_offset r5, 4
> +       .cfi_rel_offset r6, 8
> +       .cfi_rel_offset r7, 12
> +       .cfi_rel_offset r8, 16
> +       .cfi_rel_offset lr, 20
> +
> +       /* The syscall calling convention isn't the same as the C one:
> +        * we enter with r0 == *signal_pending
> +        *               r1 == syscall number
> +        *               r2, r3, [sp+0] ... [sp+12] == syscall arguments
> +        *               and return the result in r0
> +        * and the syscall instruction needs
> +        *               r7 == syscall number
> +        *               r0 ... r6 == syscall arguments
> +        *               and returns the result in r0
> +        * Shuffle everything around appropriately.
> +        * Note the 16 bytes that we pushed to save registers.
> +        */
> +       mov     r8, r0                  /* copy signal_pending */
> +       mov     r7, r1                  /* syscall number */
> +       mov     r0, r2                  /* syscall args */
> +       mov     r1, r3
> +       ldm     ip, { r2, r3, r4, r5, r6 }
> +
> +       /* This next sequence of code works in conjunction with the
> +        * rewind_if_safe_syscall_function(). If a signal is taken
> +        * and the interrupted PC is anywhere between 'safe_syscall_start'
> +        * and 'safe_syscall_end' then we rewind it to 'safe_syscall_start'.
> +        * The code sequence must therefore be able to cope with this, and
> +        * the syscall instruction must be the final one in the sequence.
> +        */
> +safe_syscall_start:
> +       /* if signal_pending is non-zero, don't do the call */
> +       ldr     ip, [r8]                /* signal_pending */
> +       tst     ip, ip
> +       bne     1f
> +       swi     0
> +safe_syscall_end:
> +       /* code path for having successfully executed the syscall */
> +       pop     { r4, r5, r6, r7, r8, pc }

Worth commenting here that we assume that we're not trying to do
old ARMv4T interworking ?

> +
> +1:
> +       /* code path when we didn't execute the syscall */
> +       ldr     r0, =-TARGET_ERESTARTSYS
> +       pop     { r4, r5, r6, r7, r8, pc }
> +       .fnend
> +       .cfi_endproc
> +
> +       .size   safe_syscall_base, .-safe_syscall_base
> --
> 2.5.5
>

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM
Richard Henderson June 14, 2016, 3:53 p.m. UTC | #2
On 06/14/2016 05:04 AM, Peter Maydell wrote:
> On 13 June 2016 at 22:45, Richard Henderson <rth@twiddle.net> wrote:
>> Signed-off-by: Richard Henderson <rth@twiddle.net>
>> ---
>>  linux-user/host/arm/hostdep.h          | 34 ++++++++++++++
>>  linux-user/host/arm/safe-syscall.inc.S | 86 ++++++++++++++++++++++++++++++++++
>>  2 files changed, 120 insertions(+)
>>  create mode 100644 linux-user/host/arm/hostdep.h
>>  create mode 100644 linux-user/host/arm/safe-syscall.inc.S
>>
>> diff --git a/linux-user/host/arm/hostdep.h b/linux-user/host/arm/hostdep.h
>> new file mode 100644
>> index 0000000..1426fb6
>> --- /dev/null
>> +++ b/linux-user/host/arm/hostdep.h
>> @@ -0,0 +1,34 @@
>> +/*
>> + * hostdep.h : things which are dependent on the host architecture
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#ifndef QEMU_HOSTDEP_H
>> +#define QEMU_HOSTDEP_H
>> +
>> +/* We have a safe-syscall.inc.S */
>> +#define HAVE_SAFE_SYSCALL
>> +
>> +#ifndef __ASSEMBLER__
>> +
>> +/* These are defined by the safe-syscall.inc.S file */
>> +extern char safe_syscall_start[];
>> +extern char safe_syscall_end[];
>> +
>> +/* Adjust the signal context to rewind out of safe-syscall if we're in it */
>> +static inline void rewind_if_in_safe_syscall(void *puc)
>> +{
>> +    struct ucontext *uc = puc;
>> +    unsigned long *pcreg = &uc->uc_mcontext.arm_pc;
>> +
>> +    if (*pcreg > (uintptr_t)safe_syscall_start
>> +        && *pcreg < (uintptr_t)safe_syscall_end) {
>> +        *pcreg = (uintptr_t)safe_syscall_start;
>> +    }
>> +}
>> +
>> +#endif /* __ASSEMBLER__ */
>> +
>> +#endif
>> diff --git a/linux-user/host/arm/safe-syscall.inc.S b/linux-user/host/arm/safe-syscall.inc.S
>> new file mode 100644
>> index 0000000..52f8883
>> --- /dev/null
>> +++ b/linux-user/host/arm/safe-syscall.inc.S
>> @@ -0,0 +1,86 @@
>> +/*
>> + * safe-syscall.inc.S : host-specific assembly fragment
>> + * to handle signals occurring at the same time as system calls.
>> + * This is intended to be included by linux-user/safe-syscall.S
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +       .global safe_syscall_base
>> +       .global safe_syscall_start
>> +       .global safe_syscall_end
>> +       .type   safe_syscall_base, %function
>> +
>> +       .cfi_sections   .debug_frame
>> +
>> +       .text
>> +       .syntax unified
>> +       .arm
> 
> Do we need a ".align 2" here? glibc has one.

It's probably best to have one.

>> +       mov     ip, sp                  /* save entry stack */
> 
> Personally I find the numbered registers like "r12" easier to read than
> the named versions like "ip" (I always have to look the latter up
> to find out which register they actually are, so it saves effort
> to just write r12 in the first place IMHO.)

That's fine.

>> +       /* code path for having successfully executed the syscall */
>> +       pop     { r4, r5, r6, r7, r8, pc }
> 
> Worth commenting here that we assume that we're not trying to do
> old ARMv4T interworking ?

At one time weren't we talking about dropping host support for really old arm
(pre arm5t?).  I seem to recall making a note about some possible cleanups to
tcg/arm/.

If so, then we shouldn't be noting that specifically here, but somewhere else.
Perhaps README (although it forwards most everything to the web site),
or even a configure test.


r~
diff mbox

Patch

diff --git a/linux-user/host/arm/hostdep.h b/linux-user/host/arm/hostdep.h
new file mode 100644
index 0000000..1426fb6
--- /dev/null
+++ b/linux-user/host/arm/hostdep.h
@@ -0,0 +1,34 @@ 
+/*
+ * hostdep.h : things which are dependent on the host architecture
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_HOSTDEP_H
+#define QEMU_HOSTDEP_H
+
+/* We have a safe-syscall.inc.S */
+#define HAVE_SAFE_SYSCALL
+
+#ifndef __ASSEMBLER__
+
+/* These are defined by the safe-syscall.inc.S file */
+extern char safe_syscall_start[];
+extern char safe_syscall_end[];
+
+/* Adjust the signal context to rewind out of safe-syscall if we're in it */
+static inline void rewind_if_in_safe_syscall(void *puc)
+{
+    struct ucontext *uc = puc;
+    unsigned long *pcreg = &uc->uc_mcontext.arm_pc;
+
+    if (*pcreg > (uintptr_t)safe_syscall_start
+        && *pcreg < (uintptr_t)safe_syscall_end) {
+        *pcreg = (uintptr_t)safe_syscall_start;
+    }
+}
+
+#endif /* __ASSEMBLER__ */
+
+#endif
diff --git a/linux-user/host/arm/safe-syscall.inc.S b/linux-user/host/arm/safe-syscall.inc.S
new file mode 100644
index 0000000..52f8883
--- /dev/null
+++ b/linux-user/host/arm/safe-syscall.inc.S
@@ -0,0 +1,86 @@ 
+/*
+ * safe-syscall.inc.S : host-specific assembly fragment
+ * to handle signals occurring at the same time as system calls.
+ * This is intended to be included by linux-user/safe-syscall.S
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+	.global safe_syscall_base
+	.global safe_syscall_start
+	.global safe_syscall_end
+	.type   safe_syscall_base, %function
+
+	.cfi_sections	.debug_frame
+
+	.text
+	.syntax unified
+	.arm
+
+	/* This is the entry point for making a system call. The calling
+	 * convention here is that of a C varargs function with the
+	 * first argument an 'int *' to the signal_pending flag, the
+	 * second one the system call number (as a 'long'), and all further
+	 * arguments being syscall arguments (also 'long').
+	 * We return a long which is the syscall's return value, which
+	 * may be negative-errno on failure. Conversion to the
+	 * -1-and-errno-set convention is done by the calling wrapper.
+	 */
+safe_syscall_base:
+	.fnstart
+	.cfi_startproc
+	mov	ip, sp			/* save entry stack */
+	push	{ r4, r5, r6, r7, r8, lr }
+	.save	{ r4, r5, r6, r7, r8, lr }
+	.cfi_adjust_cfa_offset 24
+	.cfi_rel_offset r4, 0
+	.cfi_rel_offset r5, 4
+	.cfi_rel_offset r6, 8
+	.cfi_rel_offset r7, 12
+	.cfi_rel_offset r8, 16
+	.cfi_rel_offset lr, 20
+
+	/* The syscall calling convention isn't the same as the C one:
+	 * we enter with r0 == *signal_pending
+	 *               r1 == syscall number
+	 *               r2, r3, [sp+0] ... [sp+12] == syscall arguments
+	 *               and return the result in r0
+	 * and the syscall instruction needs
+	 *               r7 == syscall number
+	 *               r0 ... r6 == syscall arguments
+	 *               and returns the result in r0
+	 * Shuffle everything around appropriately.
+	 * Note the 16 bytes that we pushed to save registers.
+	 */
+	mov	r8, r0			/* copy signal_pending */
+	mov	r7, r1			/* syscall number */
+	mov	r0, r2			/* syscall args */
+	mov	r1, r3
+	ldm	ip, { r2, r3, r4, r5, r6 }
+
+	/* This next sequence of code works in conjunction with the
+	 * rewind_if_safe_syscall_function(). If a signal is taken
+	 * and the interrupted PC is anywhere between 'safe_syscall_start'
+	 * and 'safe_syscall_end' then we rewind it to 'safe_syscall_start'.
+	 * The code sequence must therefore be able to cope with this, and
+	 * the syscall instruction must be the final one in the sequence.
+	 */
+safe_syscall_start:
+	/* if signal_pending is non-zero, don't do the call */
+	ldr	ip, [r8]		/* signal_pending */
+	tst	ip, ip
+	bne	1f
+	swi	0
+safe_syscall_end:
+	/* code path for having successfully executed the syscall */
+	pop	{ r4, r5, r6, r7, r8, pc }
+
+1:
+	/* code path when we didn't execute the syscall */
+	ldr	r0, =-TARGET_ERESTARTSYS
+	pop	{ r4, r5, r6, r7, r8, pc }
+	.fnend
+	.cfi_endproc
+
+	.size   safe_syscall_base, .-safe_syscall_base