Re: [Qemu-devel] [PATCH 3/6] linux-user: Provide safe_syscall for arm

[Richard Henderson]

On 06/14/2016 05:04 AM, Peter Maydell wrote:
> On 13 June 2016 at 22:45, Richard Henderson <address@hidden> wrote:
>> Signed-off-by: Richard Henderson <address@hidden>
>> ---
>>  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~