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

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

[Wirth, Allan]

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 <awirth@akamai.com>
---
 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
         {
-- 
1.9.1

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

[Peter Maydell]

On 1 July 2016 at 12:59, Wirth, Allan <awirth@akamai.com> 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 <awirth@akamai.com>
> ---
>  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/

thanks
-- PMM

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

[Wirth, Allan]

Thanks for the feedback. I didn’t find that patch before when I searched, so
apologies for the duplicate submission.

The proposed fix certainly does seem cleaner and more general. Does it
imply though that this patch is incorrect? It fixes the emulation bug
in my use case, and AFAICT does not introduce new emulation bugs.

Cheers,
Allan Wirth

On 7/1/16, 9:35 AM, "Peter Maydell" <peter.maydell@linaro.org> wrote:

>On 1 July 2016 at 12:59, Wirth, Allan <awirth@akamai.com> 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 <awirth@akamai.com>
>> ---
>>  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/
>
>thanks
>-- PMM

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

[Peter Maydell]

On 1 July 2016 at 16:34, Wirth, Allan <awirth@akamai.com> wrote:
> Thanks for the feedback. I didn’t find that patch before when I searched, so
> apologies for the duplicate submission.
>
> The proposed fix certainly does seem cleaner and more general. Does it
> imply though that this patch is incorrect? It fixes the emulation bug
> in my use case, and AFAICT does not introduce new emulation bugs.

Well, it depends what you mean by "incorrect". It's pretty common
in dealing with a large and old code base to find good opportunities
for small refactorings when you investigate a bug. If we allow bugs
to be fixed with the smallest and most expedient change, then
problems gradually pile up and the codebase becomes unmaintainable.
So we often ask patch submitters to do a bit of cleanup in the
process of fixing their bug.

In this case, fixing the bug in the way that I suggest will
fix it for all architectures, not just x86-64, improve the
code by deleting an #ifdef, and remove a trap that will otherwise
be waiting for the next new architecture that has support
contributed to it.

thanks
-- PMM

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

[Laurent Vivier]

Le 01/07/2016 à 15:35, Peter Maydell a écrit :
> On 1 July 2016 at 12:59, Wirth, Allan <awirth@akamai.com> 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 <awirth@akamai.com>
>> ---
>>  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.

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
         {

Laurent

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

[Peter Maydell]

On 2 July 2016 at 09:20, Laurent Vivier <laurent@vivier.eu> wrote:
>
>
> Le 01/07/2016 à 15:35, Peter Maydell a écrit :
>> On 1 July 2016 at 12:59, Wirth, Allan <awirth@akamai.com> 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 <awirth@akamai.com>
>>> ---
>>>  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.

thanks
-- PMM

Re: [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 <laurent@vivier.eu> wrote:
>>
>>
>> Le 01/07/2016 à 15:35, Peter Maydell a écrit :
>>> On 1 July 2016 at 12:59, Wirth, Allan <awirth@akamai.com> 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 <awirth@akamai.com>
>>>> ---
>>>>  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

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

[Peter Maydell]

On 2 July 2016 at 17:41, Laurent Vivier <laurent@vivier.eu> wrote:
> 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      |
> ------------+----------------+-----------------+

Hmm. Looking at current Linux git master, I get
slightly different results. The only architectures which
define __ARCH_WANT_SYS_OLD_SELECT are:
 arm, m68k, mn10300, x86
and no others use sys_old_select.

So I think we have the following behaviours:

(1) Define neither NR_select nor NR__newselect
 (and use pselect6 syscall for select):
 aarch64, openrisc, tilegx, unicore32, presumably any future arch

(2) only define NR__newselect, it is new select:
 mips, mips64, sh, s390

(3) Only define NR_select, want that to be new select:
 alpha, x86_64, s390x

(4) NR__newselect is new select, NR_select is old_select:
 i386, m68k, arm if kernel is not CONFIG_AEABI

(5) NR__newselect is new select, NR_select is defined but
 if called returns ENOSYS:
 microblaze, arm if CONFIG_AEABI, ppc64

(6) NR__newselect is new select, NR_select is a bonkers custom
 thing that tries to autodetect the calling convention:
http://lxr.free-electrons.com/source/arch/powerpc/kernel/syscalls.c#L86
 ppc32 (but only native 32-bit; 32-bit compat support
 on a ppc64 kernel is category 5, so I vote for ignoring
 this weirdness and calling ppc category 5)

(7) NR_select and NR__newselect are different numbers
 but both are new select:
 cris, sparc, sparc64

which is a pretty confusing mess, but I think it equates to:
(0) if defined, NR__newselect is always new select
(1) if NR_select is defined, the choices are:
 (a) NR_select is old_select:
   i386, m68k, arm
 (b) NR_select is defined but should ENOSYS:
   microblaze, ppc
 (c) NR_select defined and is new select:
   everything else (alpha, x86-64, s390x, cris, sparc, sparc64)

and I think we should handle that by having the code in syscall.c
be something like:

#ifdef TARGET_NR_select
    case TARGET_NR_select:
#if defined(TARGET_WANT_NI_OLD_SELECT)
        /* some architectures used to have old_select here
         * but now ENOSYS it.
         */
        ret = -TARGET_ENOSYS;
        break;
#elif defined(TARGET_WANT_OLD_SYS_SELECT)
        /* code for old select here; maybe factored out to
         * its own function: ret = do_old_select() ?
         */
#else
        /* select is new style select */
        ret = do_select(...);
#endif
#endif

where TARGET_WANT_NI_OLD_SELECT and
TARGET_WANT_OLD_SYS_SELECT are #defined in
linux-user/$(ARCH)/target_syscall.h by those
architectures that need that behaviour
(microblaze, ppc for the first; i386, m68k, arm
for the second).

We could just not define TARGET_NR_select for
microblaze and ppc, of course, but that might
be confusing and easily accidentally reverted.

For openrisc, sh and tilegx we incorrectly define
a TARGET_NR_select which the kernel doesn't, so
we should delete that from our headers.

I think overall that produces a reasonable separation
of "what behaviour does my architecture want" from
the implementation of the various behaviours, and
means the default will be correct for any architectures
we add later (only the oddball legacy cases need
to request special behaviour).

thanks
-- PMM

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

[Laurent Vivier]

Le 02/07/2016 à 22:12, Peter Maydell a écrit :
> On 2 July 2016 at 17:41, Laurent Vivier <laurent@vivier.eu> wrote:
>> 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      |
>> ------------+----------------+-----------------+
> 
> Hmm. Looking at current Linux git master, I get
> slightly different results. The only architectures which
> define __ARCH_WANT_SYS_OLD_SELECT are:

Where is defined this __ARCH_WANT_SYS_OLD_SELECT?

>  arm, m68k, mn10300, x86
> and no others use sys_old_select.

You're right, NR_select is sys_ni_syscall for:

microblaze, mips32, sh4, ppc64

arch/microblaze/kernel/syscall_table.S:	.long sys_ni_syscall		/*
old_select */
arch/mips/kernel/scall32-o32.S:	PTR	sys_ni_syscall			/* old_select */
arch/sh/kernel/syscalls_32.S:	.long sys_ni_syscall	/* sys_oldselect */
arch/pwoerpc/include/asm/systbl.h:SYSX(sys_ni_syscall,sys_ni_syscall,ppc_select)

but I have supposed that it was set to sys_old_select for older kernel.
[but in 1.3.48, it is already sys_ni_syscall for mips... so we must
really manage that as ENOSYS)

In x86, old_select is used for the 32bit version, not for the 64bit:

entry/syscalls/syscall_32.tbl

82      i386    select                  sys_old_select
compat_sys_old_select


> So I think we have the following behaviours:
> 
> (1) Define neither NR_select nor NR__newselect
>  (and use pselect6 syscall for select):
>  aarch64, openrisc, tilegx, unicore32, presumably any future arch

They use:

kernel/sys.c:

#undef __SYSCALL
#define __SYSCALL(nr, call) [nr] = (call),

void *sys_call_table[__NR_syscalls] = {
#include <asm/unistd.h>
};

It's not very clear, but I think they use NR_select with sys_select:

include/uapi/asm-generic/unistd.h

#define __ARCH_WANT_SYS_SELECT
__SYSCALL(__NR_select, sys_select)

> (2) only define NR__newselect, it is new select:
>  mips, mips64, sh, s390
> 
> (3) Only define NR_select, want that to be new select:
>  alpha, x86_64, s390x
> 
> (4) NR__newselect is new select, NR_select is old_select:
>  i386, m68k, arm if kernel is not CONFIG_AEABI
> 
> (5) NR__newselect is new select, NR_select is defined but
>  if called returns ENOSYS:
>  microblaze, arm if CONFIG_AEABI, ppc64
> 
> (6) NR__newselect is new select, NR_select is a bonkers custom
>  thing that tries to autodetect the calling convention:
> http://lxr.free-electrons.com/source/arch/powerpc/kernel/syscalls.c#L86
>  ppc32 (but only native 32-bit; 32-bit compat support
>  on a ppc64 kernel is category 5, so I vote for ignoring
>  this weirdness and calling ppc category 5)
> 
> (7) NR_select and NR__newselect are different numbers
>  but both are new select:
>  cris, sparc, sparc64
> 
> which is a pretty confusing mess, but I think it equates to:
> (0) if defined, NR__newselect is always new select
> (1) if NR_select is defined, the choices are:
>  (a) NR_select is old_select:
>    i386, m68k, arm
>  (b) NR_select is defined but should ENOSYS:
>    microblaze, ppc
>  (c) NR_select defined and is new select:
>    everything else (alpha, x86-64, s390x, cris, sparc, sparc64)
> 
> and I think we should handle that by having the code in syscall.c
> be something like:
> 
> #ifdef TARGET_NR_select
>     case TARGET_NR_select:
> #if defined(TARGET_WANT_NI_OLD_SELECT)
>         /* some architectures used to have old_select here
>          * but now ENOSYS it.
>          */
>         ret = -TARGET_ENOSYS;
>         break;
> #elif defined(TARGET_WANT_OLD_SYS_SELECT)
>         /* code for old select here; maybe factored out to
>          * its own function: ret = do_old_select() ?
>          */
> #else
>         /* select is new style select */
>         ret = do_select(...);
> #endif
> #endif
> 
> where TARGET_WANT_NI_OLD_SELECT and
> TARGET_WANT_OLD_SYS_SELECT are #defined in
> linux-user/$(ARCH)/target_syscall.h by those
> architectures that need that behaviour
> (microblaze, ppc for the first; i386, m68k, arm
> for the second).
> We could just not define TARGET_NR_select for
> microblaze and ppc, of course, but that might
> be confusing and easily accidentally reverted.
> 
> For openrisc, sh and tilegx we incorrectly define
> a TARGET_NR_select which the kernel doesn't, so
> we should delete that from our headers.

I think they really exist (from asm-generic/unistd.h)

> I think overall that produces a reasonable separation
> of "what behaviour does my architecture want" from
> the implementation of the various behaviours, and
> means the default will be correct for any architectures
> we add later (only the oddball legacy cases need
> to request special behaviour).

I agree.

Thanks,
Laurent

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

[Peter Maydell]

On 2 July 2016 at 22:17, Laurent Vivier <laurent@vivier.eu> wrote:
> Le 02/07/2016 à 22:12, Peter Maydell a écrit :
>> (1) Define neither NR_select nor NR__newselect
>>  (and use pselect6 syscall for select):
>>  aarch64, openrisc, tilegx, unicore32, presumably any future arch
>
> They use:
>
> kernel/sys.c:
>
> #undef __SYSCALL
> #define __SYSCALL(nr, call) [nr] = (call),
>
> void *sys_call_table[__NR_syscalls] = {
> #include <asm/unistd.h>
> };
>
> It's not very clear, but I think they use NR_select with sys_select:
>
> include/uapi/asm-generic/unistd.h
>
> #define __ARCH_WANT_SYS_SELECT
> __SYSCALL(__NR_select, sys_select)

This is inside an #ifdef __ARCH_WANT_SYSCALL_DEPRECATED.
Only arch/score defines that; most architectures using
the asm-generic syscall numbers don't want these
obsolete syscalls to exist.

>> For openrisc, sh and tilegx we incorrectly define
>> a TARGET_NR_select which the kernel doesn't, so
>> we should delete that from our headers.
>
> I think they really exist (from asm-generic/unistd.h)

See above.

thanks
-- PMM

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

[Laurent Vivier]

Le 02/07/2016 à 23:20, Peter Maydell a écrit :
> On 2 July 2016 at 22:17, Laurent Vivier <laurent@vivier.eu> wrote:
>> Le 02/07/2016 à 22:12, Peter Maydell a écrit :
>>> (1) Define neither NR_select nor NR__newselect
>>>  (and use pselect6 syscall for select):
>>>  aarch64, openrisc, tilegx, unicore32, presumably any future arch
>>
>> They use:
>>
>> kernel/sys.c:
>>
>> #undef __SYSCALL
>> #define __SYSCALL(nr, call) [nr] = (call),
>>
>> void *sys_call_table[__NR_syscalls] = {
>> #include <asm/unistd.h>
>> };
>>
>> It's not very clear, but I think they use NR_select with sys_select:
>>
>> include/uapi/asm-generic/unistd.h
>>
>> #define __ARCH_WANT_SYS_SELECT
>> __SYSCALL(__NR_select, sys_select)
> 
> This is inside an #ifdef __ARCH_WANT_SYSCALL_DEPRECATED.
> Only arch/score defines that; most architectures using
> the asm-generic syscall numbers don't want these
> obsolete syscalls to exist.

I've missed that... so you're right on everything.

Thanks,
Laurent

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

[Riku Voipio]

On Sat, Jul 02, 2016 at 09:12:09PM +0100, Peter Maydell wrote:
> On 2 July 2016 at 17:41, Laurent Vivier <laurent@vivier.eu> wrote:
> > 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      |
> > ------------+----------------+-----------------+
> 
> Hmm. Looking at current Linux git master, I get
> slightly different results. The only architectures which
> define __ARCH_WANT_SYS_OLD_SELECT are:
>  arm, m68k, mn10300, x86
> and no others use sys_old_select.
> 
> So I think we have the following behaviours:
> 
> (1) Define neither NR_select nor NR__newselect
>  (and use pselect6 syscall for select):
>  aarch64, openrisc, tilegx, unicore32, presumably any future arch
> 
> (2) only define NR__newselect, it is new select:
>  mips, mips64, sh, s390
> 
> (3) Only define NR_select, want that to be new select:
>  alpha, x86_64, s390x
> 
> (4) NR__newselect is new select, NR_select is old_select:
>  i386, m68k, arm if kernel is not CONFIG_AEABI
> 
> (5) NR__newselect is new select, NR_select is defined but
>  if called returns ENOSYS:
>  microblaze, arm if CONFIG_AEABI, ppc64
> 
> (6) NR__newselect is new select, NR_select is a bonkers custom
>  thing that tries to autodetect the calling convention:
> http://lxr.free-electrons.com/source/arch/powerpc/kernel/syscalls.c#L86
>  ppc32 (but only native 32-bit; 32-bit compat support
>  on a ppc64 kernel is category 5, so I vote for ignoring
>  this weirdness and calling ppc category 5)
> 
> (7) NR_select and NR__newselect are different numbers
>  but both are new select:
>  cris, sparc, sparc64
> 
> which is a pretty confusing mess, but I think it equates to:
> (0) if defined, NR__newselect is always new select
> (1) if NR_select is defined, the choices are:
>  (a) NR_select is old_select:
>    i386, m68k, arm
>  (b) NR_select is defined but should ENOSYS:
>    microblaze, ppc
>  (c) NR_select defined and is new select:
>    everything else (alpha, x86-64, s390x, cris, sparc, sparc64)
> 
> and I think we should handle that by having the code in syscall.c
> be something like:
> 
> #ifdef TARGET_NR_select
>     case TARGET_NR_select:
> #if defined(TARGET_WANT_NI_OLD_SELECT)
>         /* some architectures used to have old_select here
>          * but now ENOSYS it.
>          */
>         ret = -TARGET_ENOSYS;
>         break;
> #elif defined(TARGET_WANT_OLD_SYS_SELECT)
>         /* code for old select here; maybe factored out to
>          * its own function: ret = do_old_select() ?
>          */
> #else
>         /* select is new style select */
>         ret = do_select(...);
> #endif
> #endif

I agree, this seems to be the best way to fix select properly.

> where TARGET_WANT_NI_OLD_SELECT and
> TARGET_WANT_OLD_SYS_SELECT are #defined in
> linux-user/$(ARCH)/target_syscall.h by those
> architectures that need that behaviour
> (microblaze, ppc for the first; i386, m68k, arm
> for the second).
> 
> We could just not define TARGET_NR_select for
> microblaze and ppc, of course, but that might
> be confusing and easily accidentally reverted.
> 
> For openrisc, sh and tilegx we incorrectly define
> a TARGET_NR_select which the kernel doesn't, so
> we should delete that from our headers.
> 
> I think overall that produces a reasonable separation
> of "what behaviour does my architecture want" from
> the implementation of the various behaviours, and
> means the default will be correct for any architectures
> we add later (only the oddball legacy cases need
> to request special behaviour).
> 
> thanks
> -- PMM

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

[Laurent Vivier]

Le 07/07/2016 à 20:49, Riku Voipio a écrit :
> On Sat, Jul 02, 2016 at 09:12:09PM +0100, Peter Maydell wrote:
>> On 2 July 2016 at 17:41, Laurent Vivier <laurent@vivier.eu> wrote:
>>> 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      |
>>> ------------+----------------+-----------------+
>>
>> Hmm. Looking at current Linux git master, I get
>> slightly different results. The only architectures which
>> define __ARCH_WANT_SYS_OLD_SELECT are:
>>  arm, m68k, mn10300, x86
>> and no others use sys_old_select.
>>
>> So I think we have the following behaviours:
>>
>> (1) Define neither NR_select nor NR__newselect
>>  (and use pselect6 syscall for select):
>>  aarch64, openrisc, tilegx, unicore32, presumably any future arch
>>
>> (2) only define NR__newselect, it is new select:
>>  mips, mips64, sh, s390
>>
>> (3) Only define NR_select, want that to be new select:
>>  alpha, x86_64, s390x
>>
>> (4) NR__newselect is new select, NR_select is old_select:
>>  i386, m68k, arm if kernel is not CONFIG_AEABI
>>
>> (5) NR__newselect is new select, NR_select is defined but
>>  if called returns ENOSYS:
>>  microblaze, arm if CONFIG_AEABI, ppc64
>>
>> (6) NR__newselect is new select, NR_select is a bonkers custom
>>  thing that tries to autodetect the calling convention:
>> http://lxr.free-electrons.com/source/arch/powerpc/kernel/syscalls.c#L86
>>  ppc32 (but only native 32-bit; 32-bit compat support
>>  on a ppc64 kernel is category 5, so I vote for ignoring
>>  this weirdness and calling ppc category 5)
>>
>> (7) NR_select and NR__newselect are different numbers
>>  but both are new select:
>>  cris, sparc, sparc64
>>
>> which is a pretty confusing mess, but I think it equates to:
>> (0) if defined, NR__newselect is always new select
>> (1) if NR_select is defined, the choices are:
>>  (a) NR_select is old_select:
>>    i386, m68k, arm
>>  (b) NR_select is defined but should ENOSYS:
>>    microblaze, ppc
>>  (c) NR_select defined and is new select:
>>    everything else (alpha, x86-64, s390x, cris, sparc, sparc64)
>>
>> and I think we should handle that by having the code in syscall.c
>> be something like:
>>
>> #ifdef TARGET_NR_select
>>     case TARGET_NR_select:
>> #if defined(TARGET_WANT_NI_OLD_SELECT)
>>         /* some architectures used to have old_select here
>>          * but now ENOSYS it.
>>          */
>>         ret = -TARGET_ENOSYS;
>>         break;
>> #elif defined(TARGET_WANT_OLD_SYS_SELECT)
>>         /* code for old select here; maybe factored out to
>>          * its own function: ret = do_old_select() ?
>>          */
>> #else
>>         /* select is new style select */
>>         ret = do_select(...);
>> #endif
>> #endif
> 
> I agree, this seems to be the best way to fix select properly.

Ok, if no one is already working on that, I'm going to send a patch
according to Peter's comments.

Laurent

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

[Wirth, Allan]

On 7/7/16, 3:02 PM, "Laurent Vivier" <laurent@vivier.eu> wrote:

>
>
>Le 07/07/2016 à 20:49, Riku Voipio a écrit :
>> On Sat, Jul 02, 2016 at 09:12:09PM +0100, Peter Maydell wrote:
>>> On 2 July 2016 at 17:41, Laurent Vivier <laurent@vivier.eu> wrote:
>>>> 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      |
>>>> ------------+----------------+-----------------+
>>>
>>> Hmm. Looking at current Linux git master, I get
>>> slightly different results. The only architectures which
>>> define __ARCH_WANT_SYS_OLD_SELECT are:
>>>  arm, m68k, mn10300, x86
>>> and no others use sys_old_select.
>>>
>>> So I think we have the following behaviours:
>>>
>>> (1) Define neither NR_select nor NR__newselect
>>>  (and use pselect6 syscall for select):
>>>  aarch64, openrisc, tilegx, unicore32, presumably any future arch
>>>
>>> (2) only define NR__newselect, it is new select:
>>>  mips, mips64, sh, s390
>>>
>>> (3) Only define NR_select, want that to be new select:
>>>  alpha, x86_64, s390x
>>>
>>> (4) NR__newselect is new select, NR_select is old_select:
>>>  i386, m68k, arm if kernel is not CONFIG_AEABI
>>>
>>> (5) NR__newselect is new select, NR_select is defined but
>>>  if called returns ENOSYS:
>>>  microblaze, arm if CONFIG_AEABI, ppc64
>>>
>>> (6) NR__newselect is new select, NR_select is a bonkers custom
>>>  thing that tries to autodetect the calling convention:
>>> http://lxr.free-electrons.com/source/arch/powerpc/kernel/syscalls.c#L86
>>>  ppc32 (but only native 32-bit; 32-bit compat support
>>>  on a ppc64 kernel is category 5, so I vote for ignoring
>>>  this weirdness and calling ppc category 5)
>>>
>>> (7) NR_select and NR__newselect are different numbers
>>>  but both are new select:
>>>  cris, sparc, sparc64
>>>
>>> which is a pretty confusing mess, but I think it equates to:
>>> (0) if defined, NR__newselect is always new select
>>> (1) if NR_select is defined, the choices are:
>>>  (a) NR_select is old_select:
>>>    i386, m68k, arm
>>>  (b) NR_select is defined but should ENOSYS:
>>>    microblaze, ppc
>>>  (c) NR_select defined and is new select:
>>>    everything else (alpha, x86-64, s390x, cris, sparc, sparc64)
>>>
>>> and I think we should handle that by having the code in syscall.c
>>> be something like:
>>>
>>> #ifdef TARGET_NR_select
>>>     case TARGET_NR_select:
>>> #if defined(TARGET_WANT_NI_OLD_SELECT)
>>>         /* some architectures used to have old_select here
>>>          * but now ENOSYS it.
>>>          */
>>>         ret = -TARGET_ENOSYS;
>>>         break;
>>> #elif defined(TARGET_WANT_OLD_SYS_SELECT)
>>>         /* code for old select here; maybe factored out to
>>>          * its own function: ret = do_old_select() ?
>>>          */
>>> #else
>>>         /* select is new style select */
>>>         ret = do_select(...);
>>> #endif
>>> #endif
>> 
>> I agree, this seems to be the best way to fix select properly.
>
>Ok, if no one is already working on that, I'm going to send a patch
>according to Peter's comments.
>
>Laurent

I was hoping to, but I do not think that I will get around to it anytime soon.

-Allan

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

[Laurent Vivier]

Le 07/07/2016 à 21:04, Wirth, Allan a écrit :
> 
> 
> On 7/7/16, 3:02 PM, "Laurent Vivier" <laurent@vivier.eu> wrote:
> 
>>
>>
>> Le 07/07/2016 à 20:49, Riku Voipio a écrit :
>>> On Sat, Jul 02, 2016 at 09:12:09PM +0100, Peter Maydell wrote:
>>>> On 2 July 2016 at 17:41, Laurent Vivier <laurent@vivier.eu> wrote:
>>>>> 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      |
>>>>> ------------+----------------+-----------------+
>>>>
>>>> Hmm. Looking at current Linux git master, I get
>>>> slightly different results. The only architectures which
>>>> define __ARCH_WANT_SYS_OLD_SELECT are:
>>>>  arm, m68k, mn10300, x86
>>>> and no others use sys_old_select.
>>>>
>>>> So I think we have the following behaviours:
>>>>
>>>> (1) Define neither NR_select nor NR__newselect
>>>>  (and use pselect6 syscall for select):
>>>>  aarch64, openrisc, tilegx, unicore32, presumably any future arch
>>>>
>>>> (2) only define NR__newselect, it is new select:
>>>>  mips, mips64, sh, s390
>>>>
>>>> (3) Only define NR_select, want that to be new select:
>>>>  alpha, x86_64, s390x
>>>>
>>>> (4) NR__newselect is new select, NR_select is old_select:
>>>>  i386, m68k, arm if kernel is not CONFIG_AEABI
>>>>
>>>> (5) NR__newselect is new select, NR_select is defined but
>>>>  if called returns ENOSYS:
>>>>  microblaze, arm if CONFIG_AEABI, ppc64
>>>>
>>>> (6) NR__newselect is new select, NR_select is a bonkers custom
>>>>  thing that tries to autodetect the calling convention:
>>>> http://lxr.free-electrons.com/source/arch/powerpc/kernel/syscalls.c#L86
>>>>  ppc32 (but only native 32-bit; 32-bit compat support
>>>>  on a ppc64 kernel is category 5, so I vote for ignoring
>>>>  this weirdness and calling ppc category 5)
>>>>
>>>> (7) NR_select and NR__newselect are different numbers
>>>>  but both are new select:
>>>>  cris, sparc, sparc64
>>>>
>>>> which is a pretty confusing mess, but I think it equates to:
>>>> (0) if defined, NR__newselect is always new select
>>>> (1) if NR_select is defined, the choices are:
>>>>  (a) NR_select is old_select:
>>>>    i386, m68k, arm
>>>>  (b) NR_select is defined but should ENOSYS:
>>>>    microblaze, ppc
>>>>  (c) NR_select defined and is new select:
>>>>    everything else (alpha, x86-64, s390x, cris, sparc, sparc64)
>>>>
>>>> and I think we should handle that by having the code in syscall.c
>>>> be something like:
>>>>
>>>> #ifdef TARGET_NR_select
>>>>     case TARGET_NR_select:
>>>> #if defined(TARGET_WANT_NI_OLD_SELECT)
>>>>         /* some architectures used to have old_select here
>>>>          * but now ENOSYS it.
>>>>          */
>>>>         ret = -TARGET_ENOSYS;
>>>>         break;
>>>> #elif defined(TARGET_WANT_OLD_SYS_SELECT)
>>>>         /* code for old select here; maybe factored out to
>>>>          * its own function: ret = do_old_select() ?
>>>>          */
>>>> #else
>>>>         /* select is new style select */
>>>>         ret = do_select(...);
>>>> #endif
>>>> #endif
>>>
>>> I agree, this seems to be the best way to fix select properly.
>>
>> Ok, if no one is already working on that, I'm going to send a patch
>> according to Peter's comments.
>>
>> Laurent
> 
> I was hoping to, but I do not think that I will get around to it anytime soon.

If I provide a patch, could you test it for your test case?

Thanks,
Laurent

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

[Wirth, Allan]

On 7/7/16, 3:09 PM, "Laurent Vivier" <laurent@vivier.eu> wrote:

>
>
>Le 07/07/2016 à 21:04, Wirth, Allan a écrit :
>> 
>> 
>> On 7/7/16, 3:02 PM, "Laurent Vivier" <laurent@vivier.eu> wrote:
>> 
>>>
>>>
>>> Le 07/07/2016 à 20:49, Riku Voipio a écrit :
>>>> On Sat, Jul 02, 2016 at 09:12:09PM +0100, Peter Maydell wrote:
>>>>> On 2 July 2016 at 17:41, Laurent Vivier <laurent@vivier.eu> wrote:
>>>>>> 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      |
>>>>>> ------------+----------------+-----------------+
>>>>>
>>>>> Hmm. Looking at current Linux git master, I get
>>>>> slightly different results. The only architectures which
>>>>> define __ARCH_WANT_SYS_OLD_SELECT are:
>>>>>  arm, m68k, mn10300, x86
>>>>> and no others use sys_old_select.
>>>>>
>>>>> So I think we have the following behaviours:
>>>>>
>>>>> (1) Define neither NR_select nor NR__newselect
>>>>>  (and use pselect6 syscall for select):
>>>>>  aarch64, openrisc, tilegx, unicore32, presumably any future arch
>>>>>
>>>>> (2) only define NR__newselect, it is new select:
>>>>>  mips, mips64, sh, s390
>>>>>
>>>>> (3) Only define NR_select, want that to be new select:
>>>>>  alpha, x86_64, s390x
>>>>>
>>>>> (4) NR__newselect is new select, NR_select is old_select:
>>>>>  i386, m68k, arm if kernel is not CONFIG_AEABI
>>>>>
>>>>> (5) NR__newselect is new select, NR_select is defined but
>>>>>  if called returns ENOSYS:
>>>>>  microblaze, arm if CONFIG_AEABI, ppc64
>>>>>
>>>>> (6) NR__newselect is new select, NR_select is a bonkers custom
>>>>>  thing that tries to autodetect the calling convention:
>>>>> http://lxr.free-electrons.com/source/arch/powerpc/kernel/syscalls.c#L86
>>>>>  ppc32 (but only native 32-bit; 32-bit compat support
>>>>>  on a ppc64 kernel is category 5, so I vote for ignoring
>>>>>  this weirdness and calling ppc category 5)
>>>>>
>>>>> (7) NR_select and NR__newselect are different numbers
>>>>>  but both are new select:
>>>>>  cris, sparc, sparc64
>>>>>
>>>>> which is a pretty confusing mess, but I think it equates to:
>>>>> (0) if defined, NR__newselect is always new select
>>>>> (1) if NR_select is defined, the choices are:
>>>>>  (a) NR_select is old_select:
>>>>>    i386, m68k, arm
>>>>>  (b) NR_select is defined but should ENOSYS:
>>>>>    microblaze, ppc
>>>>>  (c) NR_select defined and is new select:
>>>>>    everything else (alpha, x86-64, s390x, cris, sparc, sparc64)
>>>>>
>>>>> and I think we should handle that by having the code in syscall.c
>>>>> be something like:
>>>>>
>>>>> #ifdef TARGET_NR_select
>>>>>     case TARGET_NR_select:
>>>>> #if defined(TARGET_WANT_NI_OLD_SELECT)
>>>>>         /* some architectures used to have old_select here
>>>>>          * but now ENOSYS it.
>>>>>          */
>>>>>         ret = -TARGET_ENOSYS;
>>>>>         break;
>>>>> #elif defined(TARGET_WANT_OLD_SYS_SELECT)
>>>>>         /* code for old select here; maybe factored out to
>>>>>          * its own function: ret = do_old_select() ?
>>>>>          */
>>>>> #else
>>>>>         /* select is new style select */
>>>>>         ret = do_select(...);
>>>>> #endif
>>>>> #endif
>>>>
>>>> I agree, this seems to be the best way to fix select properly.
>>>
>>> Ok, if no one is already working on that, I'm going to send a patch
>>> according to Peter's comments.
>>>
>>> Laurent
>> 
>> I was hoping to, but I do not think that I will get around to it anytime soon.
>
>If I provide a patch, could you test it for your test case?
>
>Thanks,
>Laurent

Definitely I would be happy to. I would be very grateful if you did.

Thanks,
Allan