qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl
@ 2015-12-18  6:26 Chen Gang
  2015-12-18  6:51 ` Chen Gang
  2015-12-18  9:37 ` Laurent Vivier
  0 siblings, 2 replies; 11+ messages in thread
From: Chen Gang @ 2015-12-18  6:26 UTC (permalink / raw)
  To: riku.voipio; +Cc: Peter Maydell, qemu-devel, Richard Henderson


For fcntl, it always needs to notice about it, just like do_fcntl() has
done, or it will cause issue (e.g. alpha host run i386 guest).

Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 linux-user/syscall.c |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 0f8adeb..1a60e6f 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9007,7 +9007,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             if (((CPUARMState *)cpu_env)->eabi) {
                 if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
                     goto efault;
-                fl.l_type = tswap16(target_efl->l_type);
+                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
+                                                   flock_tbl);
                 fl.l_whence = tswap16(target_efl->l_whence);
                 fl.l_start = tswap64(target_efl->l_start);
                 fl.l_len = tswap64(target_efl->l_len);
@@ -9018,7 +9019,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             {
                 if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
                     goto efault;
-                fl.l_type = tswap16(target_fl->l_type);
+                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
+                                                   flock_tbl);
                 fl.l_whence = tswap16(target_fl->l_whence);
                 fl.l_start = tswap64(target_fl->l_start);
                 fl.l_len = tswap64(target_fl->l_len);
@@ -9031,7 +9033,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                 if (((CPUARMState *)cpu_env)->eabi) {
                     if (!lock_user_struct(VERIFY_WRITE, target_efl, arg3, 0)) 
                         goto efault;
-                    target_efl->l_type = tswap16(fl.l_type);
+                    target_efl->l_type = host_to_target_bitmask(
+                                                 tswap16(fl.l_type), flock_tbl);
                     target_efl->l_whence = tswap16(fl.l_whence);
                     target_efl->l_start = tswap64(fl.l_start);
                     target_efl->l_len = tswap64(fl.l_len);
@@ -9042,7 +9045,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                 {
                     if (!lock_user_struct(VERIFY_WRITE, target_fl, arg3, 0)) 
                         goto efault;
-                    target_fl->l_type = tswap16(fl.l_type);
+                    target_fl->l_type = host_to_target_bitmask(
+                                                 tswap16(fl.l_type), flock_tbl);
                     target_fl->l_whence = tswap16(fl.l_whence);
                     target_fl->l_start = tswap64(fl.l_start);
                     target_fl->l_len = tswap64(fl.l_len);
@@ -9058,7 +9062,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             if (((CPUARMState *)cpu_env)->eabi) {
                 if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
                     goto efault;
-                fl.l_type = tswap16(target_efl->l_type);
+                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
+                                                   flock_tbl);
                 fl.l_whence = tswap16(target_efl->l_whence);
                 fl.l_start = tswap64(target_efl->l_start);
                 fl.l_len = tswap64(target_efl->l_len);
@@ -9069,7 +9074,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             {
                 if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
                     goto efault;
-                fl.l_type = tswap16(target_fl->l_type);
+                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
+                                                   flock_tbl);
                 fl.l_whence = tswap16(target_fl->l_whence);
                 fl.l_start = tswap64(target_fl->l_start);
                 fl.l_len = tswap64(target_fl->l_len);
-- 
1.7.3.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl
  2015-12-18  6:26 [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl Chen Gang
@ 2015-12-18  6:51 ` Chen Gang
  2015-12-18  9:41   ` Laurent Vivier
  2015-12-18  9:37 ` Laurent Vivier
  1 sibling, 1 reply; 11+ messages in thread
From: Chen Gang @ 2015-12-18  6:51 UTC (permalink / raw)
  To: riku.voipio; +Cc: Peter Maydell, qemu-devel, Richard Henderson


I found this issue during my working time, it is about sw_64 (almost the
same as alpha) host running i386 wine programs.

I also found another issue, but I am not quite sure whether it is worth
enough for our upstream: The related fix patch is below, which will let
the initialization slower, but for most archs, they have no this issue.

    linux-user/mmap.c: Always zero MAP_ANONYMOUS memory in target_mmap()
    
    In some architectures, they have no policy to zero MAP_ANONYMOUS memory,
    which will cause issue for qemu target_mmap.
    
    Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 7b459d5..9c9152d 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -567,6 +567,10 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
     printf("\n");
 #endif
     tb_invalidate_phys_range(start, start + len);
+    if ((prot & PROT_WRITE) && (flags & MAP_ANONYMOUS)
+        && ((flags & MAP_PRIVATE) || (fd == -1))) {
+        memset(g2h(start), 0, len);
+    }
     mmap_unlock();
     return start;
 fail:


Thanks.

On 2015年12月18日 14:26, Chen Gang wrote:
> 
> For fcntl, it always needs to notice about it, just like do_fcntl() has
> done, or it will cause issue (e.g. alpha host run i386 guest).
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
>  linux-user/syscall.c |   18 ++++++++++++------
>  1 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 0f8adeb..1a60e6f 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9007,7 +9007,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              if (((CPUARMState *)cpu_env)->eabi) {
>                  if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
>                      goto efault;
> -                fl.l_type = tswap16(target_efl->l_type);
> +                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
> +                                                   flock_tbl);
>                  fl.l_whence = tswap16(target_efl->l_whence);
>                  fl.l_start = tswap64(target_efl->l_start);
>                  fl.l_len = tswap64(target_efl->l_len);
> @@ -9018,7 +9019,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              {
>                  if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
>                      goto efault;
> -                fl.l_type = tswap16(target_fl->l_type);
> +                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
> +                                                   flock_tbl);
>                  fl.l_whence = tswap16(target_fl->l_whence);
>                  fl.l_start = tswap64(target_fl->l_start);
>                  fl.l_len = tswap64(target_fl->l_len);
> @@ -9031,7 +9033,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>                  if (((CPUARMState *)cpu_env)->eabi) {
>                      if (!lock_user_struct(VERIFY_WRITE, target_efl, arg3, 0)) 
>                          goto efault;
> -                    target_efl->l_type = tswap16(fl.l_type);
> +                    target_efl->l_type = host_to_target_bitmask(
> +                                                 tswap16(fl.l_type), flock_tbl);
>                      target_efl->l_whence = tswap16(fl.l_whence);
>                      target_efl->l_start = tswap64(fl.l_start);
>                      target_efl->l_len = tswap64(fl.l_len);
> @@ -9042,7 +9045,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>                  {
>                      if (!lock_user_struct(VERIFY_WRITE, target_fl, arg3, 0)) 
>                          goto efault;
> -                    target_fl->l_type = tswap16(fl.l_type);
> +                    target_fl->l_type = host_to_target_bitmask(
> +                                                 tswap16(fl.l_type), flock_tbl);
>                      target_fl->l_whence = tswap16(fl.l_whence);
>                      target_fl->l_start = tswap64(fl.l_start);
>                      target_fl->l_len = tswap64(fl.l_len);
> @@ -9058,7 +9062,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              if (((CPUARMState *)cpu_env)->eabi) {
>                  if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
>                      goto efault;
> -                fl.l_type = tswap16(target_efl->l_type);
> +                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
> +                                                   flock_tbl);
>                  fl.l_whence = tswap16(target_efl->l_whence);
>                  fl.l_start = tswap64(target_efl->l_start);
>                  fl.l_len = tswap64(target_efl->l_len);
> @@ -9069,7 +9074,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              {
>                  if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
>                      goto efault;
> -                fl.l_type = tswap16(target_fl->l_type);
> +                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
> +                                                   flock_tbl);
>                  fl.l_whence = tswap16(target_fl->l_whence);
>                  fl.l_start = tswap64(target_fl->l_start);
>                  fl.l_len = tswap64(target_fl->l_len);
> 

-- 
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl
  2015-12-18  6:26 [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl Chen Gang
  2015-12-18  6:51 ` Chen Gang
@ 2015-12-18  9:37 ` Laurent Vivier
  2015-12-18 21:40   ` Chen Gang
  1 sibling, 1 reply; 11+ messages in thread
From: Laurent Vivier @ 2015-12-18  9:37 UTC (permalink / raw)
  To: Chen Gang, riku.voipio; +Cc: Peter Maydell, qemu-devel, Richard Henderson


Le 18/12/2015 07:26, Chen Gang a écrit :
> 
> For fcntl, it always needs to notice about it, just like do_fcntl() has
> done, or it will cause issue (e.g. alpha host run i386 guest).
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
>  linux-user/syscall.c |   18 ++++++++++++------
>  1 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 0f8adeb..1a60e6f 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9007,7 +9007,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              if (((CPUARMState *)cpu_env)->eabi) {
>                  if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
>                      goto efault;
> -                fl.l_type = tswap16(target_efl->l_type);
> +                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
> +                                                   flock_tbl);
>                  fl.l_whence = tswap16(target_efl->l_whence);
>                  fl.l_start = tswap64(target_efl->l_start);
>                  fl.l_len = tswap64(target_efl->l_len);
> @@ -9018,7 +9019,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              {
>                  if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
>                      goto efault;
> -                fl.l_type = tswap16(target_fl->l_type);
> +                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
> +                                                   flock_tbl);
>                  fl.l_whence = tswap16(target_fl->l_whence);
>                  fl.l_start = tswap64(target_fl->l_start);
>                  fl.l_len = tswap64(target_fl->l_len);
> @@ -9031,7 +9033,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>                  if (((CPUARMState *)cpu_env)->eabi) {
>                      if (!lock_user_struct(VERIFY_WRITE, target_efl, arg3, 0)) 
>                          goto efault;
> -                    target_efl->l_type = tswap16(fl.l_type);
> +                    target_efl->l_type = host_to_target_bitmask(
> +                                                 tswap16(fl.l_type), flock_tbl);
>                      target_efl->l_whence = tswap16(fl.l_whence);
>                      target_efl->l_start = tswap64(fl.l_start);
>                      target_efl->l_len = tswap64(fl.l_len);
> @@ -9042,7 +9045,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>                  {
>                      if (!lock_user_struct(VERIFY_WRITE, target_fl, arg3, 0)) 
>                          goto efault;
> -                    target_fl->l_type = tswap16(fl.l_type);
> +                    target_fl->l_type = host_to_target_bitmask(
> +                                                 tswap16(fl.l_type), flock_tbl);
>                      target_fl->l_whence = tswap16(fl.l_whence);
>                      target_fl->l_start = tswap64(fl.l_start);
>                      target_fl->l_len = tswap64(fl.l_len);
> @@ -9058,7 +9062,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              if (((CPUARMState *)cpu_env)->eabi) {
>                  if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
>                      goto efault;
> -                fl.l_type = tswap16(target_efl->l_type);
> +                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
> +                                                   flock_tbl);
>                  fl.l_whence = tswap16(target_efl->l_whence);
>                  fl.l_start = tswap64(target_efl->l_start);
>                  fl.l_len = tswap64(target_efl->l_len);
> @@ -9069,7 +9074,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              {
>                  if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
>                      goto efault;
> -                fl.l_type = tswap16(target_fl->l_type);
> +                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
> +                                                   flock_tbl);
>                  fl.l_whence = tswap16(target_fl->l_whence);
>                  fl.l_start = tswap64(target_fl->l_start);
>                  fl.l_len = tswap64(target_fl->l_len);
> 

This patch looks good to me, except that script/checkpatch.pl complains
about "DOS line ending" and "line over 80 characters".

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl
  2015-12-18  6:51 ` Chen Gang
@ 2015-12-18  9:41   ` Laurent Vivier
  2015-12-18 10:47     ` Chen Gang
  0 siblings, 1 reply; 11+ messages in thread
From: Laurent Vivier @ 2015-12-18  9:41 UTC (permalink / raw)
  To: Chen Gang, riku.voipio; +Cc: Peter Maydell, qemu-devel, Richard Henderson



Le 18/12/2015 07:51, Chen Gang a écrit :
> 
> I found this issue during my working time, it is about sw_64 (almost the
> same as alpha) host running i386 wine programs.
> 
> I also found another issue, but I am not quite sure whether it is worth
> enough for our upstream: The related fix patch is below, which will let
> the initialization slower, but for most archs, they have no this issue.
> 
>     linux-user/mmap.c: Always zero MAP_ANONYMOUS memory in target_mmap()
>     
>     In some architectures, they have no policy to zero MAP_ANONYMOUS memory,
>     which will cause issue for qemu target_mmap.
>     
>     Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> 
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index 7b459d5..9c9152d 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -567,6 +567,10 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
>      printf("\n");
>  #endif
>      tb_invalidate_phys_range(start, start + len);
> +    if ((prot & PROT_WRITE) && (flags & MAP_ANONYMOUS)
> +        && ((flags & MAP_PRIVATE) || (fd == -1))) {
> +        memset(g2h(start), 0, len);
> +    }

IMHO, their kernel needs a fix, mmap(2):

MAP_ANONYMOUS
        The mapping is not backed by any file; its contents are initial‐
        ized to zero.


>      mmap_unlock();
>      return start;
>  fail:
> 
> 
> Thanks.

Laurent

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl
  2015-12-18  9:41   ` Laurent Vivier
@ 2015-12-18 10:47     ` Chen Gang
  2015-12-21  2:47       ` Chen Gang
  0 siblings, 1 reply; 11+ messages in thread
From: Chen Gang @ 2015-12-18 10:47 UTC (permalink / raw)
  To: Laurent Vivier, riku.voipio; +Cc: Peter Maydell, qemu-devel, Richard Henderson


On 2015年12月18日 17:41, Laurent Vivier wrote:
> 
> 
> Le 18/12/2015 07:51, Chen Gang a écrit :
>>
>> I found this issue during my working time, it is about sw_64 (almost the
>> same as alpha) host running i386 wine programs.
>>
>> I also found another issue, but I am not quite sure whether it is worth
>> enough for our upstream: The related fix patch is below, which will let
>> the initialization slower, but for most archs, they have no this issue.
>>
>>     linux-user/mmap.c: Always zero MAP_ANONYMOUS memory in target_mmap()
>>     
>>     In some architectures, they have no policy to zero MAP_ANONYMOUS memory,
>>     which will cause issue for qemu target_mmap.
>>     
>>     Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
>>
>> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
>> index 7b459d5..9c9152d 100644
>> --- a/linux-user/mmap.c
>> +++ b/linux-user/mmap.c
>> @@ -567,6 +567,10 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
>>      printf("\n");
>>  #endif
>>      tb_invalidate_phys_range(start, start + len);
>> +    if ((prot & PROT_WRITE) && (flags & MAP_ANONYMOUS)
>> +        && ((flags & MAP_PRIVATE) || (fd == -1))) {
>> +        memset(g2h(start), 0, len);
>> +    }
> 
> IMHO, their kernel needs a fix, mmap(2):
> 
> MAP_ANONYMOUS
>         The mapping is not backed by any file; its contents are initial‐
>         ized to zero.
> 

OK, Thanks.


-- 
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl
  2015-12-18  9:37 ` Laurent Vivier
@ 2015-12-18 21:40   ` Chen Gang
  2015-12-18 21:58     ` Laurent Vivier
  0 siblings, 1 reply; 11+ messages in thread
From: Chen Gang @ 2015-12-18 21:40 UTC (permalink / raw)
  To: Laurent Vivier, Chen Gang, riku.voipio
  Cc: Peter Maydell, qemu-devel, Richard Henderson


On 12/18/15 17:37, Laurent Vivier wrote:
> 
> Le 18/12/2015 07:26, Chen Gang a écrit :
>>
>> For fcntl, it always needs to notice about it, just like do_fcntl() has
>> done, or it will cause issue (e.g. alpha host run i386 guest).
>>
>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
>> ---
>>  linux-user/syscall.c |   18 ++++++++++++------
>>  1 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 0f8adeb..1a60e6f 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -9007,7 +9007,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>              if (((CPUARMState *)cpu_env)->eabi) {
>>                  if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
>>                      goto efault;
>> -                fl.l_type = tswap16(target_efl->l_type);
>> +                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
>> +                                                   flock_tbl);
>>                  fl.l_whence = tswap16(target_efl->l_whence);
>>                  fl.l_start = tswap64(target_efl->l_start);
>>                  fl.l_len = tswap64(target_efl->l_len);
>> @@ -9018,7 +9019,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>              {
>>                  if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
>>                      goto efault;
>> -                fl.l_type = tswap16(target_fl->l_type);
>> +                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
>> +                                                   flock_tbl);
>>                  fl.l_whence = tswap16(target_fl->l_whence);
>>                  fl.l_start = tswap64(target_fl->l_start);
>>                  fl.l_len = tswap64(target_fl->l_len);
>> @@ -9031,7 +9033,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>                  if (((CPUARMState *)cpu_env)->eabi) {
>>                      if (!lock_user_struct(VERIFY_WRITE, target_efl, arg3, 0)) 
>>                          goto efault;
>> -                    target_efl->l_type = tswap16(fl.l_type);
>> +                    target_efl->l_type = host_to_target_bitmask(
>> +                                                 tswap16(fl.l_type), flock_tbl);
>>                      target_efl->l_whence = tswap16(fl.l_whence);
>>                      target_efl->l_start = tswap64(fl.l_start);
>>                      target_efl->l_len = tswap64(fl.l_len);
>> @@ -9042,7 +9045,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>                  {
>>                      if (!lock_user_struct(VERIFY_WRITE, target_fl, arg3, 0)) 
>>                          goto efault;
>> -                    target_fl->l_type = tswap16(fl.l_type);
>> +                    target_fl->l_type = host_to_target_bitmask(
>> +                                                 tswap16(fl.l_type), flock_tbl);
>>                      target_fl->l_whence = tswap16(fl.l_whence);
>>                      target_fl->l_start = tswap64(fl.l_start);
>>                      target_fl->l_len = tswap64(fl.l_len);
>> @@ -9058,7 +9062,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>              if (((CPUARMState *)cpu_env)->eabi) {
>>                  if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
>>                      goto efault;
>> -                fl.l_type = tswap16(target_efl->l_type);
>> +                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
>> +                                                   flock_tbl);
>>                  fl.l_whence = tswap16(target_efl->l_whence);
>>                  fl.l_start = tswap64(target_efl->l_start);
>>                  fl.l_len = tswap64(target_efl->l_len);
>> @@ -9069,7 +9074,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>              {
>>                  if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
>>                      goto efault;
>> -                fl.l_type = tswap16(target_fl->l_type);
>> +                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
>> +                                                   flock_tbl);
>>                  fl.l_whence = tswap16(target_fl->l_whence);
>>                  fl.l_start = tswap64(target_fl->l_start);
>>                  fl.l_len = tswap64(target_fl->l_len);
>>
> 
> This patch looks good to me, except that script/checkpatch.pl complains
> about "DOS line ending" and "line over 80 characters".
> 

I did not get any script/checkpatch.pl complains, originally.

Does my email client configuration is incorrect, then cause incorrect
mail format? I guess not. The related reason is below.

 - I copy your full reply mail contents to a new file (diff.patch).

 - Remove all '> ' in vi editor (1,% s/^> //g) (so get the original
   patch contents).

 - ./script/checkpatch.pl diff.patch, it has no any complains.

> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> 

Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl
  2015-12-18 21:40   ` Chen Gang
@ 2015-12-18 21:58     ` Laurent Vivier
  2015-12-18 22:12       ` Chen Gang
  0 siblings, 1 reply; 11+ messages in thread
From: Laurent Vivier @ 2015-12-18 21:58 UTC (permalink / raw)
  To: Chen Gang, Chen Gang, riku.voipio
  Cc: Peter Maydell, qemu-devel, Richard Henderson



Le 18/12/2015 22:40, Chen Gang a écrit :
> 
> On 12/18/15 17:37, Laurent Vivier wrote:
>>
>> Le 18/12/2015 07:26, Chen Gang a écrit :
>>>
>>> For fcntl, it always needs to notice about it, just like do_fcntl() has
>>> done, or it will cause issue (e.g. alpha host run i386 guest).
>>>
>>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
>>> ---
>>>  linux-user/syscall.c |   18 ++++++++++++------
>>>  1 files changed, 12 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>> index 0f8adeb..1a60e6f 100644
>>> --- a/linux-user/syscall.c
>>> +++ b/linux-user/syscall.c
>>> @@ -9007,7 +9007,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>>              if (((CPUARMState *)cpu_env)->eabi) {
>>>                  if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
>>>                      goto efault;
>>> -                fl.l_type = tswap16(target_efl->l_type);
>>> +                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
>>> +                                                   flock_tbl);
>>>                  fl.l_whence = tswap16(target_efl->l_whence);
>>>                  fl.l_start = tswap64(target_efl->l_start);
>>>                  fl.l_len = tswap64(target_efl->l_len);
>>> @@ -9018,7 +9019,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>>              {
>>>                  if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
>>>                      goto efault;
>>> -                fl.l_type = tswap16(target_fl->l_type);
>>> +                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
>>> +                                                   flock_tbl);
>>>                  fl.l_whence = tswap16(target_fl->l_whence);
>>>                  fl.l_start = tswap64(target_fl->l_start);
>>>                  fl.l_len = tswap64(target_fl->l_len);
>>> @@ -9031,7 +9033,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>>                  if (((CPUARMState *)cpu_env)->eabi) {
>>>                      if (!lock_user_struct(VERIFY_WRITE, target_efl, arg3, 0)) 
>>>                          goto efault;
>>> -                    target_efl->l_type = tswap16(fl.l_type);
>>> +                    target_efl->l_type = host_to_target_bitmask(
>>> +                                                 tswap16(fl.l_type), flock_tbl);
>>>                      target_efl->l_whence = tswap16(fl.l_whence);
>>>                      target_efl->l_start = tswap64(fl.l_start);
>>>                      target_efl->l_len = tswap64(fl.l_len);
>>> @@ -9042,7 +9045,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>>                  {
>>>                      if (!lock_user_struct(VERIFY_WRITE, target_fl, arg3, 0)) 
>>>                          goto efault;
>>> -                    target_fl->l_type = tswap16(fl.l_type);
>>> +                    target_fl->l_type = host_to_target_bitmask(
>>> +                                                 tswap16(fl.l_type), flock_tbl);
>>>                      target_fl->l_whence = tswap16(fl.l_whence);
>>>                      target_fl->l_start = tswap64(fl.l_start);
>>>                      target_fl->l_len = tswap64(fl.l_len);
>>> @@ -9058,7 +9062,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>>              if (((CPUARMState *)cpu_env)->eabi) {
>>>                  if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
>>>                      goto efault;
>>> -                fl.l_type = tswap16(target_efl->l_type);
>>> +                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
>>> +                                                   flock_tbl);
>>>                  fl.l_whence = tswap16(target_efl->l_whence);
>>>                  fl.l_start = tswap64(target_efl->l_start);
>>>                  fl.l_len = tswap64(target_efl->l_len);
>>> @@ -9069,7 +9074,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>>              {
>>>                  if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
>>>                      goto efault;
>>> -                fl.l_type = tswap16(target_fl->l_type);
>>> +                fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
>>> +                                                   flock_tbl);
>>>                  fl.l_whence = tswap16(target_fl->l_whence);
>>>                  fl.l_start = tswap64(target_fl->l_start);
>>>                  fl.l_len = tswap64(target_fl->l_len);
>>>
>>
>> This patch looks good to me, except that script/checkpatch.pl complains
>> about "DOS line ending" and "line over 80 characters".
>>
> 
> I did not get any script/checkpatch.pl complains, originally.
> 
> Does my email client configuration is incorrect, then cause incorrect
> mail format? I guess not. The related reason is below.
> 
>  - I copy your full reply mail contents to a new file (diff.patch).

If you copy, you loose the special characters. I do a "Save as".

> 
>  - Remove all '> ' in vi editor (1,% s/^> //g) (so get the original
>    patch contents).
> 
>  - ./script/checkpatch.pl diff.patch, it has no any complains.

If I run "file" on the saved file, I have:

    $ file orig.eml
    orig.eml: SMTP mail, ASCII text, with CRLF line terminators

I can convert it with "tr":

    $ tr "\r" "\n" < orig.eml > new.eml

    $ file new.eml
    new.eml: SMTP mail, ASCII text

    $ ./scripts/checkpatch.pl new.eml
    total: 0 errors, 0 warnings, 54 lines checked

    new.eml has no obvious style problems and is ready for submission.

Laurent

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl
  2015-12-18 21:58     ` Laurent Vivier
@ 2015-12-18 22:12       ` Chen Gang
  2015-12-18 22:15         ` Laurent Vivier
  0 siblings, 1 reply; 11+ messages in thread
From: Chen Gang @ 2015-12-18 22:12 UTC (permalink / raw)
  To: Laurent Vivier, Chen Gang, riku.voipio
  Cc: Peter Maydell, qemu-devel, Richard Henderson


On 12/19/15 05:58, Laurent Vivier wrote:
> 
> Le 18/12/2015 22:40, Chen Gang a écrit :
>>

[...]

>> I did not get any script/checkpatch.pl complains, originally.
>>
>> Does my email client configuration is incorrect, then cause incorrect
>> mail format? I guess not. The related reason is below.
>>
>>  - I copy your full reply mail contents to a new file (diff.patch).
> 
> If you copy, you loose the special characters. I do a "Save as".
> 
>>
>>  - Remove all '> ' in vi editor (1,% s/^> //g) (so get the original
>>    patch contents).
>>
>>  - ./script/checkpatch.pl diff.patch, it has no any complains.
> 
> If I run "file" on the saved file, I have:
> 
>     $ file orig.eml
>     orig.eml: SMTP mail, ASCII text, with CRLF line terminators
> 
> I can convert it with "tr":
> 
>     $ tr "\r" "\n" < orig.eml > new.eml
> 
>     $ file new.eml
>     new.eml: SMTP mail, ASCII text
> 
>     $ ./scripts/checkpatch.pl new.eml
>     total: 0 errors, 0 warnings, 54 lines checked
> 
>     new.eml has no obvious style problems and is ready for submission.
> 
> Laurent
> 

OK, thank you very much. I shall config my email client again to notice
about it.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl
  2015-12-18 22:12       ` Chen Gang
@ 2015-12-18 22:15         ` Laurent Vivier
  2015-12-18 22:31           ` Chen Gang
  0 siblings, 1 reply; 11+ messages in thread
From: Laurent Vivier @ 2015-12-18 22:15 UTC (permalink / raw)
  To: Chen Gang, Chen Gang, riku.voipio
  Cc: Peter Maydell, qemu-devel, Richard Henderson



Le 18/12/2015 23:12, Chen Gang a écrit :
> 
> On 12/19/15 05:58, Laurent Vivier wrote:
>>
>> Le 18/12/2015 22:40, Chen Gang a écrit :
>>>
> 
> [...]
> 
>>> I did not get any script/checkpatch.pl complains, originally.
>>>
>>> Does my email client configuration is incorrect, then cause incorrect
>>> mail format? I guess not. The related reason is below.
>>>
>>>  - I copy your full reply mail contents to a new file (diff.patch).
>>
>> If you copy, you loose the special characters. I do a "Save as".
>>
>>>
>>>  - Remove all '> ' in vi editor (1,% s/^> //g) (so get the original
>>>    patch contents).
>>>
>>>  - ./script/checkpatch.pl diff.patch, it has no any complains.
>>
>> If I run "file" on the saved file, I have:
>>
>>     $ file orig.eml
>>     orig.eml: SMTP mail, ASCII text, with CRLF line terminators
>>
>> I can convert it with "tr":
>>
>>     $ tr "\r" "\n" < orig.eml > new.eml
>>
>>     $ file new.eml
>>     new.eml: SMTP mail, ASCII text
>>
>>     $ ./scripts/checkpatch.pl new.eml
>>     total: 0 errors, 0 warnings, 54 lines checked
>>
>>     new.eml has no obvious style problems and is ready for submission.
>>
>> Laurent
>>
> 
> OK, thank you very much. I shall config my email client again to notice
> about it.

You should not use your email client to send patches, you should use
"git send-email":

http://wiki.qemu.org/Contribute/SubmitAPatch

Laurent

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl
  2015-12-18 22:15         ` Laurent Vivier
@ 2015-12-18 22:31           ` Chen Gang
  0 siblings, 0 replies; 11+ messages in thread
From: Chen Gang @ 2015-12-18 22:31 UTC (permalink / raw)
  To: Laurent Vivier, Chen Gang, riku.voipio
  Cc: Peter Maydell, qemu-devel, Richard Henderson


On 12/19/15 06:15, Laurent Vivier wrote:
> 
> Le 18/12/2015 23:12, Chen Gang a écrit :
>>

[...]

>>
>> OK, thank you very much. I shall config my email client again to notice
>> about it.
> 
> You should not use your email client to send patches, you should use
> "git send-email":
> 
> http://wiki.qemu.org/Contribute/SubmitAPatch
> 

OK, thanks.

-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl
  2015-12-18 10:47     ` Chen Gang
@ 2015-12-21  2:47       ` Chen Gang
  0 siblings, 0 replies; 11+ messages in thread
From: Chen Gang @ 2015-12-21  2:47 UTC (permalink / raw)
  To: Laurent Vivier, riku.voipio; +Cc: Peter Maydell, qemu-devel, Richard Henderson


On 2015年12月18日 18:47, Chen Gang wrote:
> 
> On 2015年12月18日 17:41, Laurent Vivier wrote:
>>
>> Le 18/12/2015 07:51, Chen Gang a écrit :

[...]

>>>
>>> +++ b/linux-user/mmap.c
>>> @@ -567,6 +567,10 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
>>>      printf("\n");
>>>  #endif
>>>      tb_invalidate_phys_range(start, start + len);
>>> +    if ((prot & PROT_WRITE) && (flags & MAP_ANONYMOUS)
>>> +        && ((flags & MAP_PRIVATE) || (fd == -1))) {
>>> +        memset(g2h(start), 0, len);
>>> +    }
>>
>> IMHO, their kernel needs a fix, mmap(2):
>>
>> MAP_ANONYMOUS
>>         The mapping is not backed by any file; its contents are initial‐
>>         ized to zero.
>>

Oh, sorry, after check again, it is not kernel's issue: mmap() will
always zero the ANONYMOUS memory for kernel 3.8 in sw_64 arch.

It is still our qemu's issue in mmap_frag (do not zero ANONYMOUS memory
fragments), so I sent patch v2 for it, please help check.

Thanks.

> 
> OK, Thanks.
> 
> 

-- 
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-12-21  2:52 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-18  6:26 [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl Chen Gang
2015-12-18  6:51 ` Chen Gang
2015-12-18  9:41   ` Laurent Vivier
2015-12-18 10:47     ` Chen Gang
2015-12-21  2:47       ` Chen Gang
2015-12-18  9:37 ` Laurent Vivier
2015-12-18 21:40   ` Chen Gang
2015-12-18 21:58     ` Laurent Vivier
2015-12-18 22:12       ` Chen Gang
2015-12-18 22:15         ` Laurent Vivier
2015-12-18 22:31           ` Chen Gang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).