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

[Max Filippov]

On Mon, Apr 9, 2018 at 5:23 PM, Richard Henderson
 wrote:
> On 04/06/2018 11:36 AM, Max Filippov wrote:
>> +static void target_to_host_low_high(abi_ulong tlow,
>> +abi_ulong thigh,
>> +unsigned long *hlow,
>> +unsigned long *hhigh)
>> +{
>> +unsigned long long off = tlow |
>> +((unsigned long long)thigh << TARGET_LONG_BITS / 2) <<
>> +TARGET_LONG_BITS / 2;
>
> Use uint64_t instead of unsigned long long.

Ok.

>> +
>> +*hlow = (unsigned long)off;
>> +*hhigh = (unsigned long)((off >> HOST_LONG_BITS / 2) >>
>> + HOST_LONG_BITS / 2);
>
> The casts here are unnecessary and are implied by the assignment.

Did that to avoid value truncation warning. Will drop.

> Otherwise,
> Reviewed-by: Richard Henderson 

-- 
Thanks.
-- Max

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

[Richard Henderson]

On 04/06/2018 11:36 AM, Max Filippov wrote:
> +static void target_to_host_low_high(abi_ulong tlow,
> +abi_ulong thigh,
> +unsigned long *hlow,
> +unsigned long *hhigh)
> +{
> +unsigned long long off = tlow |
> +((unsigned long long)thigh << TARGET_LONG_BITS / 2) <<
> +TARGET_LONG_BITS / 2;

Use uint64_t instead of unsigned long long.

> +
> +*hlow = (unsigned long)off;
> +*hhigh = (unsigned long)((off >> HOST_LONG_BITS / 2) >>
> + HOST_LONG_BITS / 2);

The casts here are unnecessary and are implied by the assignment.

Otherwise,
Reviewed-by: Richard Henderson 


r~

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

[Laurent Vivier]

Le 06/04/2018 à 03:36, 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 
> ---
> Changes v3->v4:
> - recombine offset into unsigned long long and split it into two longs.
> 
> 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 | 24 ++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 

Reviewed-by: Laurent Vivier 

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

[Max Filippov]

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 
---
Changes v3->v4:
- recombine offset into unsigned long long and split it into two longs.

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 | 24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 5ef517613577..eb9a31f1e092 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3386,6 +3386,20 @@ 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)
+{
+unsigned long long off = tlow |
+((unsigned long long)thigh << TARGET_LONG_BITS / 2) <<
+TARGET_LONG_BITS / 2;
+
+*hlow = (unsigned long)off;
+*hhigh = (unsigned long)((off >> HOST_LONG_BITS / 2) >>
+ HOST_LONG_BITS / 2);
+}
+
 static struct iovec *lock_iovec(int type, abi_ulong target_addr,
 abi_ulong count, int copy)
 {
@@ -10449,7 +10463,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 {
 struct iovec *vec = lock_iovec(VERIFY_WRITE, arg2, arg3, 0);
 if (vec != NULL) {
-ret = get_errno(safe_preadv(arg1, vec, arg3, arg4, arg5));
+unsigned long low, high;
+
+target_to_host_low_high(arg4, arg5, , );
+ret = get_errno(safe_preadv(arg1, vec, arg3, low, high));
 unlock_iovec(vec, arg2, arg3, 1);
 } else {
 ret = -host_to_target_errno(errno);
@@ -10462,7 +10479,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 {
 struct iovec *vec = lock_iovec(VERIFY_READ, arg2, arg3, 1);
 if (vec != NULL) {
-ret = get_errno(safe_pwritev(arg1, vec, arg3, arg4, arg5));
+unsigned long low, high;
+
+target_to_host_low_high(arg4, arg5, , );
+ret = get_errno(safe_pwritev(arg1, vec, arg3, low, high));
 unlock_iovec(vec, arg2, arg3, 0);
 } else {
 ret = -host_to_target_errno(errno);
-- 
2.11.0