Re: [Qemu-devel] [PATCH v3] linux-user: fix preadv/pwritev offsets

[Laurent Vivier]

Le 05/04/2018 à 15:47, Max Filippov a écrit :
> preadv/pwritev accept low and high parts of file offset in two separate
> parameters. When host bitness doesn't match guest bitness these parts
> must be appropriately recombined.
> Introduce target_to_host_low_high that does this recombination and use
> it in preadv/pwritev syscalls.
> 
> This fixes glibc testsuite test misc/tst-preadvwritev64.
> 
> Signed-off-by: Max Filippov <address@hidden>
> ---
> Changes v2->v3:
> - rename helper to target_to_host_low_high;
> - handle cases TARGET_LONG_BITS * 2 > HOST_LONG_BITS and
>   TARGET_LONG_BITS < 2 * HOST_LONG_BITS correctly.
> 
> Changes v1->v2:
> - fix host high computation in TARGET_LONG_BITS > HOST_LONG_BITS case
> 
>  linux-user/syscall.c | 34 ++++++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 5ef517613577..13ad572e0d3b 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -3386,6 +3386,30 @@ static abi_long do_getsockopt(int sockfd, int level, 
> int optname,
>      return ret;
>  }
>  
> +static void target_to_host_low_high(abi_ulong tlow,
> +                                    abi_ulong thigh,
> +                                    unsigned long *hlow,
> +                                    unsigned long *hhigh)
> +{
> +#if TARGET_LONG_BITS == HOST_LONG_BITS
> +        *hlow = tlow;
> +        *hhigh = thigh;
> +#elif TARGET_LONG_BITS < HOST_LONG_BITS
> +        *hlow = tlow | (unsigned long)thigh << TARGET_LONG_BITS;
> +#if TARGET_LONG_BITS * 2 <= HOST_LONG_BITS
> +        *hhigh = 0;
> +#else
> +        *hhigh = (unsigned long)thigh >> (HOST_LONG_BITS - TARGET_LONG_BITS);
> +#endif
> +#else
> +        *hlow = (unsigned long)tlow;
> +        *hhigh = (unsigned long)(tlow >> HOST_LONG_BITS);
> +#if TARGET_LONG_BITS < 2 * HOST_LONG_BITS
> +        *hhigh |= (unsigned long)thigh << (TARGET_LONG_BITS - 
> HOST_LONG_BITS);
> +#endif
> +#endif
> +}

Why don't you try to de-construct then re-construct the offset?

Kernel commit
  601cc11d054a "Make non-compat preadv/pwritev use native register size"
is interesting.

static inline abi_llong target_pos_from_hilo(abi_ulong high,
                                             abi_ulong low)
{
#define TARGET_HALF_LONG_BITS (TARGET_LONG_BITS / 2)
   return (((abi_llong)high << TARGET_HALF_LONG_BITS) <<
                             TARGET_HALF_LONG_BITS) | low;
}

#define HOST_HALF_LONG_BITS (HOST_LONG_BITS / 2)

 abi_llong pos = target_pos_from_hilo(arg4, arg5);
 ret = get_errno(safe_preadv(arg1, vec, arg3,
          pos,
          (pos >> HOST_HALF_LONG_BITS) >> HOST_HALF_LONG_BITS));

It looks simpler, but perhaps I miss something
(it's just cut&paste, I don't pretend to understand that code...)?

Richard?

Thanks,
Laurent