Re: [Qemu-devel] [PATCH 3/7] Pass select() arguments directly to do_select() on x86 platforms

[Peter Maydell]

On 19 June 2016 at 01:15, Timothy Pearson
<address@hidden> wrote:
> This matches the calling conventions in the Linux kernel and
> resolves select() hangs on i386/x86_64 guests.
>
> Signed-off-by: Timothy Pearson <address@hidden>
> ---
>  linux-user/syscall.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 2968b57..644a490 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -4785,6 +4785,7 @@ static abi_long do_ioctl(int fd, int cmd, abi_long arg)
>      void *argptr;
>
>      ie = ioctl_entries;
> +
>      for(;;) {
>          if (ie->target_cmd == 0) {
>              gemu_log("Unsupported ioctl: cmd=0x%04lx\n", (long)cmd);
> @@ -7899,7 +7900,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_I386) 
> || defined(TARGET_X86_64)
>          ret = do_select(arg1, arg2, arg3, arg4, arg5);
>  #else
>          {

I think we can fix this in a cleaner way. This is actually
the result of there being two select syscalls:
 * the one which takes a struct with the params, which
   the kernel calls 'old_select' and which we seem to have
   decided to call TARGET_NR_select
 * the one which takes the arguments separately, which the
   kernel calls 'select' and which we seem to use
   TARGET_NR__newselect for

So I think we should fix this by:
 (1) making sure all archs (including x86, alpha and s390)
     define TARGET_NR_select and TARGET_NR__newselect
     correctly
 (2) removing this ifdef entirely (so TARGET_NR_select is always
     "use the struct" for any arch that defines it, and
     TARGET_NR__newselect is always "direct args"), and
     similarly for the equivalent ifdef in linux-user/syscall.c

I think we should also change our naming to TARGET_NR_oldselect
and TARGET_NR_select to match the kernel.

thanks
-- PMM