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

[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 <address@hidden>

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.