| Message ID | 164e58f5-7dcc-bc9f-286f-98b5de56c1cb@vivier.eu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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
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
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
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
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
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
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
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
--- 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