Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

2017-09-15 Thread Laurent Vivier
Le 15/09/2017 à 17:41, Philippe Mathieu-Daudé a écrit :
> On 09/15/2017 03:58 AM, James Clarke wrote:
>> Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
>> Signed-off-by: James Clarke 
> 
> Congratulations! You have won yourself a R: entry (Designated reviewer)
> in the "Linux user" and "SH4" sections of MAINTAINERS!
> 
>> ---
>>   linux-user/syscall.c | 12 
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 9b6364a266..24d6a81c21 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -10495,20 +10495,32 @@ abi_long do_syscall(void *cpu_env, int num,
>> abi_long arg1,
>>   #endif
>>   #ifdef TARGET_NR_pread64
>>   case TARGET_NR_pread64:
>> +#if defined(TARGET_SH4)
>> +    /* SH4 doesn't align register pairs, except for
>> p{read,write}64 */
>> +    arg4 = arg5;
>> +    arg5 = arg6;
>> +#else
>>   if (regpairs_aligned(cpu_env)) {
>>   arg4 = arg5;
>>   arg5 = arg6;
>>   }
>> +#endif
> 
> I'd rather use arch_type from "sysemu/arch_init.h":
> 
>   case TARGET_NR_pwrite64:
>   /* SH4 doesn't align register pairs, except for p{read,write}64 */
>   if (arch_type == QEMU_ARCH_SH4 || regpairs_aligned(cpu_env)) {
>   arg4 = arg5;
>   arg5 = arg6;
>   }
> 
> What do you think?

For the moment, arch_type is only available for system targets.

Laurent



Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

2017-09-15 Thread Richard Henderson
On 09/15/2017 08:41 AM, Philippe Mathieu-Daudé wrote:
> On 09/15/2017 03:58 AM, James Clarke wrote:
>> Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
>> Signed-off-by: James Clarke 
> 
> Congratulations! You have won yourself a R: entry (Designated reviewer) in the
> "Linux user" and "SH4" sections of MAINTAINERS!
> 
>> ---
>>   linux-user/syscall.c | 12 
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 9b6364a266..24d6a81c21 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -10495,20 +10495,32 @@ abi_long do_syscall(void *cpu_env, int num,
>> abi_long arg1,
>>   #endif
>>   #ifdef TARGET_NR_pread64
>>   case TARGET_NR_pread64:
>> +#if defined(TARGET_SH4)
>> +    /* SH4 doesn't align register pairs, except for p{read,write}64 */
>> +    arg4 = arg5;
>> +    arg5 = arg6;
>> +#else
>>   if (regpairs_aligned(cpu_env)) {
>>   arg4 = arg5;
>>   arg5 = arg6;
>>   }
>> +#endif
> 
> I'd rather use arch_type from "sysemu/arch_init.h":
> 
>   case TARGET_NR_pwrite64:
>   /* SH4 doesn't align register pairs, except for p{read,write}64 */
>   if (arch_type == QEMU_ARCH_SH4 || regpairs_aligned(cpu_env)) {
>   arg4 = arg5;
>   arg5 = arg6;
>   }
> 
> What do you think?

I'd rather change the interface of regpairs_aligned to take the syscall number,
and add an instance for SH4 above.


r~



Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

2017-09-15 Thread John Paul Adrian Glaubitz

On 09/15/2017 05:41 PM, Philippe Mathieu-Daudé wrote:

I'd rather use arch_type from "sysemu/arch_init.h":

   case TARGET_NR_pwrite64:
   /* SH4 doesn't align register pairs, except for p{read,write}64 */
   if (arch_type == QEMU_ARCH_SH4 || regpairs_aligned(cpu_env)) {
   arg4 = arg5;
   arg5 = arg6;
   }

What do you think?


I agree. That looks a bit cleaner. Was actually thinking about something like 
that.


  if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0)))
  goto efault;
  ret = get_errno(pread64(arg1, p, arg3, target_offset64(arg4, arg5)));
  unlock_user(p, arg2, ret);
  break;
  case TARGET_NR_pwrite64:
+#if defined(TARGET_SH4)
+    /* SH4 doesn't align register pairs, except for p{read,write}64 */
+    arg4 = arg5;
+    arg5 = arg6;
+#else
  if (regpairs_aligned(cpu_env)) {
  arg4 = arg5;
  arg5 = arg6;
  }
+#endif


same here.


Dito.

--
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913



Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

2017-09-15 Thread Philippe Mathieu-Daudé

On 09/15/2017 03:58 AM, James Clarke wrote:

Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
Signed-off-by: James Clarke 


Congratulations! You have won yourself a R: entry (Designated reviewer) 
in the "Linux user" and "SH4" sections of MAINTAINERS!



---
  linux-user/syscall.c | 12 
  1 file changed, 12 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9b6364a266..24d6a81c21 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -10495,20 +10495,32 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
  #endif
  #ifdef TARGET_NR_pread64
  case TARGET_NR_pread64:
+#if defined(TARGET_SH4)
+/* SH4 doesn't align register pairs, except for p{read,write}64 */
+arg4 = arg5;
+arg5 = arg6;
+#else
  if (regpairs_aligned(cpu_env)) {
  arg4 = arg5;
  arg5 = arg6;
  }
+#endif


I'd rather use arch_type from "sysemu/arch_init.h":

  case TARGET_NR_pwrite64:
  /* SH4 doesn't align register pairs, except for p{read,write}64 */
  if (arch_type == QEMU_ARCH_SH4 || regpairs_aligned(cpu_env)) {
  arg4 = arg5;
  arg5 = arg6;
  }

What do you think?


  if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0)))
  goto efault;
  ret = get_errno(pread64(arg1, p, arg3, target_offset64(arg4, arg5)));
  unlock_user(p, arg2, ret);
  break;
  case TARGET_NR_pwrite64:
+#if defined(TARGET_SH4)
+/* SH4 doesn't align register pairs, except for p{read,write}64 */
+arg4 = arg5;
+arg5 = arg6;
+#else
  if (regpairs_aligned(cpu_env)) {
  arg4 = arg5;
  arg5 = arg6;
  }
+#endif


same here.


  if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))
  goto efault;
  ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg4, arg5)));
--
2.13.2




Regards,

Phil.



Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

2017-09-15 Thread John Paul Adrian Glaubitz

On 09/15/2017 08:58 AM, James Clarke wrote:

Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
Signed-off-by: James Clarke 
---
  linux-user/syscall.c | 12 
  1 file changed, 12 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9b6364a266..24d6a81c21 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -10495,20 +10495,32 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
  #endif
  #ifdef TARGET_NR_pread64
  case TARGET_NR_pread64:
+#if defined(TARGET_SH4)
+/* SH4 doesn't align register pairs, except for p{read,write}64 */
+arg4 = arg5;
+arg5 = arg6;
+#else
  if (regpairs_aligned(cpu_env)) {
  arg4 = arg5;
  arg5 = arg6;
  }
+#endif
  if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0)))
  goto efault;
  ret = get_errno(pread64(arg1, p, arg3, target_offset64(arg4, arg5)));
  unlock_user(p, arg2, ret);
  break;
  case TARGET_NR_pwrite64:
+#if defined(TARGET_SH4)
+/* SH4 doesn't align register pairs, except for p{read,write}64 */
+arg4 = arg5;
+arg5 = arg6;
+#else
  if (regpairs_aligned(cpu_env)) {
  arg4 = arg5;
  arg5 = arg6;
  }
+#endif
  if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))
  goto efault;
  ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg4, arg5)));
--
2.13.2


Tested-By: John Paul Adrian Glaubitz 

--
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913



Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

2017-09-15 Thread Laurent Vivier
Le 15/09/2017 à 08:58, James Clarke a écrit :
> Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
> Signed-off-by: James Clarke 
> ---
>  linux-user/syscall.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 9b6364a266..24d6a81c21 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -10495,20 +10495,32 @@ abi_long do_syscall(void *cpu_env, int num, 
> abi_long arg1,
>  #endif
>  #ifdef TARGET_NR_pread64
>  case TARGET_NR_pread64:
> +#if defined(TARGET_SH4)
> +/* SH4 doesn't align register pairs, except for p{read,write}64 */
> +arg4 = arg5;
> +arg5 = arg6;
> +#else
>  if (regpairs_aligned(cpu_env)) {
>  arg4 = arg5;
>  arg5 = arg6;
>  }
> +#endif
>  if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0)))
>  goto efault;
>  ret = get_errno(pread64(arg1, p, arg3, target_offset64(arg4, arg5)));
>  unlock_user(p, arg2, ret);
>  break;
>  case TARGET_NR_pwrite64:
> +#if defined(TARGET_SH4)
> +/* SH4 doesn't align register pairs, except for p{read,write}64 */
> +arg4 = arg5;
> +arg5 = arg6;
> +#else
>  if (regpairs_aligned(cpu_env)) {
>  arg4 = arg5;
>  arg5 = arg6;
>  }
> +#endif
>  if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))
>  goto efault;
>  ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg4, 
> arg5)));
> --
> 2.13.2
> 

Reviewed-by: Laurent Vivier 




[Qemu-devel] [PATCH] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

2017-09-15 Thread James Clarke
Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
Signed-off-by: James Clarke 
---
 linux-user/syscall.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9b6364a266..24d6a81c21 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -10495,20 +10495,32 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 #endif
 #ifdef TARGET_NR_pread64
 case TARGET_NR_pread64:
+#if defined(TARGET_SH4)
+/* SH4 doesn't align register pairs, except for p{read,write}64 */
+arg4 = arg5;
+arg5 = arg6;
+#else
 if (regpairs_aligned(cpu_env)) {
 arg4 = arg5;
 arg5 = arg6;
 }
+#endif
 if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0)))
 goto efault;
 ret = get_errno(pread64(arg1, p, arg3, target_offset64(arg4, arg5)));
 unlock_user(p, arg2, ret);
 break;
 case TARGET_NR_pwrite64:
+#if defined(TARGET_SH4)
+/* SH4 doesn't align register pairs, except for p{read,write}64 */
+arg4 = arg5;
+arg5 = arg6;
+#else
 if (regpairs_aligned(cpu_env)) {
 arg4 = arg5;
 arg5 = arg6;
 }
+#endif
 if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))
 goto efault;
 ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg4, arg5)));
--
2.13.2