Re: [Qemu-trivial] [Qemu-devel] [PATCH] linux-user: fix signal() syscall on x86_64

[Laurent Vivier]

Le 02/07/2016 à 11:56, Peter Maydell a écrit :
> On 2 July 2016 at 09:20, Laurent Vivier <address@hidden> wrote:
>>
>>
>> Le 01/07/2016 à 15:35, Peter Maydell a écrit :
>>> On 1 July 2016 at 12:59, Wirth, Allan <address@hidden> wrote:
>>>> Linux on X86_64 does not use sel_arg_struct for select(), the args are
>>>> passed directly. This patch switches a define so X86_64 uses the correct
>>>> calling convention.
>>>>
>>>> Signed-off-by: Allan Wirth <address@hidden>
>>>> ---
>>>>  linux-user/syscall.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>>> index 8bf6205..209b2a7 100644
>>>> --- a/linux-user/syscall.c
>>>> +++ b/linux-user/syscall.c
>>>> @@ -8002,7 +8002,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
>>>> arg1,
>>>>          break;
>>>>  #if defined(TARGET_NR_select)
>>>>      case TARGET_NR_select:
>>>> -#if defined(TARGET_S390X) || defined(TARGET_ALPHA)
>>>> +#if defined(TARGET_S390X) || defined(TARGET_ALPHA) || 
>>>> defined(TARGET_X86_64)
>>>>          ret = do_select(arg1, arg2, arg3, arg4, arg5);
>>>>  #else
>>>>          {
>>>
>>> There is a cleaner approach which we should use to fix this:
>>> see my comments in reply to this recent patch trying to do
>>> a similar thing:
>>> https://patchwork.kernel.org/patch/9185927/
>>
>> syscall_nr.h are copies of unistd.h from kernel, so kernel uses also
>> __NR_select and __NR__newselect.
> 
> Ugh, this is complicated. The syscall functions are sys_oldselect
> and sys_select, but the syscall numbers are __NR_select and
> __NR__newselect, and I'm not sure all the architectures are
> using them consistently. For instance alpha in the kernel has
> syscall 358 as __NR_select, but the syscall table directs it
> to sys_select(), not sys_oldselect().
> 
>> I think the fix can be as simple as:
>>
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -8372,7 +8372,7 @@ abi_long do_syscall(void *cpu_env, int num,
>> abi_long arg1,
>>          break;
>>  #if defined(TARGET_NR_select)
>>      case TARGET_NR_select:
>> -#if defined(TARGET_S390X) || defined(TARGET_ALPHA)
>> +#if !defined(TARGET_NR__new_select)
>>          ret = do_select(arg1, arg2, arg3, arg4, arg5);
>>  #else
>>          {
> 
> This looks promising but I guess we need to fish through
> all the kernel architectures comparing their syscall numbers
> and which functions they dispatch to in their syscall tables.

Sadly, this can't work:

sparc/sparc64/cris use sys_select for NR_select AND NR_newselect.

Not sure all is correct, but it's what I've found:

            | __NR_select    | __NR__newselect
------------+----------------+-----------------+
arm         | sys_old_select | sys_select      |
------------+----------------+-----------------+
aarch64     | sys_select     |        -        |
------------+----------------+-----------------+
alpha       | sys_select     |        -        |
------------+----------------+-----------------+
cris        | sys_select     | sys_select      |
------------+----------------+-----------------+
m68k        | sys_old_select | sys_select      |
------------+----------------+-----------------+
microblaze  | sys_old_select | sys_select      |
------------+----------------+-----------------+
mips        | sys_old_select | sys_select      |
------------+----------------+-----------------+
mips64      | sys_select     |        -        |
------------+----------------+-----------------+
openrisc    | sys_select     |        -        |
------------+----------------+-----------------+
ppc         | sys_old_select | sys_select      |
------------+----------------+-----------------+
s390x       | sys_select     |        -        |
------------+----------------+-----------------+
sh4         | sys_old_select | sys_select      |
------------+----------------+-----------------+
sparc       | sys_select     | sys_select      |
------------+----------------+-----------------+
sparc64     | sys_select     | sys_select      |
------------+----------------+-----------------+
tilegx      | sys_select     |        -        |
------------+----------------+-----------------+
unicore32   | sys_select     |        -        |
------------+----------------+-----------------+
x86_64      | sys_select     |        -        |
------------+----------------+-----------------+
i386        | sys_old_select | sys_select      |
------------+----------------+-----------------+

Laurent