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

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

[James Clarke]

Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
Signed-off-by: James Clarke <jrtc27@jrtc27.com>
---
 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

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

[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 <jrtc27@jrtc27.com>
> ---
>  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 <laurent@vivier.eu>

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

[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 <jrtc27@jrtc27.com>
> ---
>   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 <glaubitz@physik.fu-berlin.de>

-- 
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@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

[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 <jrtc27@jrtc27.com>

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

[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 - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@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

[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 <jrtc27@jrtc27.com>
> 
> 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

[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 <jrtc27@jrtc27.com>
> 
> 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

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

[James Clarke]

Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
Signed-off-by: James Clarke <jrtc27@jrtc27.com>
---

Changes since v1:
 * Removed all changes in v1 :)
 * Added syscall num argument to regpairs_aligned
 * Added SH4-specific implementation of regpairs_aligned to return 1 for
   p{read,write}64

 linux-user/syscall.c | 34 +++++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9b6364a266..492c654970 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -667,18 +667,30 @@ static inline int next_free_host_timer(void)
 
 /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */
 #ifdef TARGET_ARM
-static inline int regpairs_aligned(void *cpu_env) {
+static inline int regpairs_aligned(void *cpu_env, int num) {
     return ((((CPUARMState *)cpu_env)->eabi) == 1) ;
 }
 #elif defined(TARGET_MIPS) && (TARGET_ABI_BITS == 32)
-static inline int regpairs_aligned(void *cpu_env) { return 1; }
+static inline int regpairs_aligned(void *cpu_env, int num) { return 1; }
 #elif defined(TARGET_PPC) && !defined(TARGET_PPC64)
 /* SysV AVI for PPC32 expects 64bit parameters to be passed on odd/even pairs
  * of registers which translates to the same as ARM/MIPS, because we start with
  * r3 as arg1 */
-static inline int regpairs_aligned(void *cpu_env) { return 1; }
+static inline int regpairs_aligned(void *cpu_env, int num) { return 1; }
+#elif defined(TARGET_SH4)
+/* SH4 doesn't align register pairs, except for p{read,write}64 */
+static inline int regpairs_aligned(void *cpu_env, int num) {
+    switch (num) {
+    case TARGET_NR_pread64:
+    case TARGET_NR_pwrite64:
+        return 1;
+
+    default:
+        return 0;
+    }
+}
 #else
-static inline int regpairs_aligned(void *cpu_env) { return 0; }
+static inline int regpairs_aligned(void *cpu_env, int num) { return 0; }
 #endif
 
 #define ERRNO_TABLE_SIZE 1200
@@ -6857,7 +6869,7 @@ static inline abi_long target_truncate64(void *cpu_env, const char *arg1,
                                          abi_long arg3,
                                          abi_long arg4)
 {
-    if (regpairs_aligned(cpu_env)) {
+    if (regpairs_aligned(cpu_env, TARGET_NR_truncate64)) {
         arg2 = arg3;
         arg3 = arg4;
     }
@@ -6871,7 +6883,7 @@ static inline abi_long target_ftruncate64(void *cpu_env, abi_long arg1,
                                           abi_long arg3,
                                           abi_long arg4)
 {
-    if (regpairs_aligned(cpu_env)) {
+    if (regpairs_aligned(cpu_env, TARGET_NR_ftruncate64)) {
         arg2 = arg3;
         arg3 = arg4;
     }
@@ -10495,7 +10507,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_pread64
     case TARGET_NR_pread64:
-        if (regpairs_aligned(cpu_env)) {
+        if (regpairs_aligned(cpu_env, num)) {
             arg4 = arg5;
             arg5 = arg6;
         }
@@ -10505,7 +10517,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         unlock_user(p, arg2, ret);
         break;
     case TARGET_NR_pwrite64:
-        if (regpairs_aligned(cpu_env)) {
+        if (regpairs_aligned(cpu_env, num)) {
             arg4 = arg5;
             arg5 = arg6;
         }
@@ -11275,7 +11287,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         arg6 = ret;
 #else
         /* 6 args: fd, offset (high, low), len (high, low), advice */
-        if (regpairs_aligned(cpu_env)) {
+        if (regpairs_aligned(cpu_env, num)) {
             /* offset is in (3,4), len in (5,6) and advice in 7 */
             arg2 = arg3;
             arg3 = arg4;
@@ -11294,7 +11306,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #ifdef TARGET_NR_fadvise64
     case TARGET_NR_fadvise64:
         /* 5 args: fd, offset (high, low), len, advice */
-        if (regpairs_aligned(cpu_env)) {
+        if (regpairs_aligned(cpu_env, num)) {
             /* offset is in (3,4), len in 5 and advice in 6 */
             arg2 = arg3;
             arg3 = arg4;
@@ -11407,7 +11419,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #ifdef TARGET_NR_readahead
     case TARGET_NR_readahead:
 #if TARGET_ABI_BITS == 32
-        if (regpairs_aligned(cpu_env)) {
+        if (regpairs_aligned(cpu_env, num)) {
             arg2 = arg3;
             arg3 = arg4;
             arg4 = arg5;
-- 
2.13.2

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

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH v2] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64
Message-id: 20170915190748.82389-1-jrtc27@jrtc27.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/1504888905-22396-1-git-send-email-chugh.ishani@research.iiit.ac.in -> patchew/1504888905-22396-1-git-send-email-chugh.ishani@research.iiit.ac.in
Switched to a new branch 'test'
5c4a338e12 linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

=== OUTPUT BEGIN ===
Checking PATCH 1/1: linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64...
ERROR: open brace '{' following function declarations go on the next line
#20: FILE: linux-user/syscall.c:670:
+static inline int regpairs_aligned(void *cpu_env, int num) {

ERROR: open brace '{' following function declarations go on the next line
#34: FILE: linux-user/syscall.c:682:
+static inline int regpairs_aligned(void *cpu_env, int num) {

total: 2 errors, 0 warnings, 90 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

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

[James Clarke]

Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
Signed-off-by: James Clarke <jrtc27@jrtc27.com>
---

Changes since v2:
 * Fixed opening curly brace formatting, both for my new SH4-specific
   regpairs_aligned function, as well as the Arm one I touched, to appease
   checkpatch.pl

Changes since v1:
 * Removed all changes in v1 :)
 * Added syscall num argument to regpairs_aligned
 * Added SH4-specific implementation of regpairs_aligned to return 1 for
   p{read,write}64

 linux-user/syscall.c | 36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9b6364a266..0c1bd80bed 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -667,18 +667,32 @@ static inline int next_free_host_timer(void)
 
 /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */
 #ifdef TARGET_ARM
-static inline int regpairs_aligned(void *cpu_env) {
+static inline int regpairs_aligned(void *cpu_env, int num)
+{
     return ((((CPUARMState *)cpu_env)->eabi) == 1) ;
 }
 #elif defined(TARGET_MIPS) && (TARGET_ABI_BITS == 32)
-static inline int regpairs_aligned(void *cpu_env) { return 1; }
+static inline int regpairs_aligned(void *cpu_env, int num) { return 1; }
 #elif defined(TARGET_PPC) && !defined(TARGET_PPC64)
 /* SysV AVI for PPC32 expects 64bit parameters to be passed on odd/even pairs
  * of registers which translates to the same as ARM/MIPS, because we start with
  * r3 as arg1 */
-static inline int regpairs_aligned(void *cpu_env) { return 1; }
+static inline int regpairs_aligned(void *cpu_env, int num) { return 1; }
+#elif defined(TARGET_SH4)
+/* SH4 doesn't align register pairs, except for p{read,write}64 */
+static inline int regpairs_aligned(void *cpu_env, int num)
+{
+    switch (num) {
+    case TARGET_NR_pread64:
+    case TARGET_NR_pwrite64:
+        return 1;
+
+    default:
+        return 0;
+    }
+}
 #else
-static inline int regpairs_aligned(void *cpu_env) { return 0; }
+static inline int regpairs_aligned(void *cpu_env, int num) { return 0; }
 #endif
 
 #define ERRNO_TABLE_SIZE 1200
@@ -6857,7 +6871,7 @@ static inline abi_long target_truncate64(void *cpu_env, const char *arg1,
                                          abi_long arg3,
                                          abi_long arg4)
 {
-    if (regpairs_aligned(cpu_env)) {
+    if (regpairs_aligned(cpu_env, TARGET_NR_truncate64)) {
         arg2 = arg3;
         arg3 = arg4;
     }
@@ -6871,7 +6885,7 @@ static inline abi_long target_ftruncate64(void *cpu_env, abi_long arg1,
                                           abi_long arg3,
                                           abi_long arg4)
 {
-    if (regpairs_aligned(cpu_env)) {
+    if (regpairs_aligned(cpu_env, TARGET_NR_ftruncate64)) {
         arg2 = arg3;
         arg3 = arg4;
     }
@@ -10495,7 +10509,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_pread64
     case TARGET_NR_pread64:
-        if (regpairs_aligned(cpu_env)) {
+        if (regpairs_aligned(cpu_env, num)) {
             arg4 = arg5;
             arg5 = arg6;
         }
@@ -10505,7 +10519,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         unlock_user(p, arg2, ret);
         break;
     case TARGET_NR_pwrite64:
-        if (regpairs_aligned(cpu_env)) {
+        if (regpairs_aligned(cpu_env, num)) {
             arg4 = arg5;
             arg5 = arg6;
         }
@@ -11275,7 +11289,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         arg6 = ret;
 #else
         /* 6 args: fd, offset (high, low), len (high, low), advice */
-        if (regpairs_aligned(cpu_env)) {
+        if (regpairs_aligned(cpu_env, num)) {
             /* offset is in (3,4), len in (5,6) and advice in 7 */
             arg2 = arg3;
             arg3 = arg4;
@@ -11294,7 +11308,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #ifdef TARGET_NR_fadvise64
     case TARGET_NR_fadvise64:
         /* 5 args: fd, offset (high, low), len, advice */
-        if (regpairs_aligned(cpu_env)) {
+        if (regpairs_aligned(cpu_env, num)) {
             /* offset is in (3,4), len in 5 and advice in 6 */
             arg2 = arg3;
             arg3 = arg4;
@@ -11407,7 +11421,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #ifdef TARGET_NR_readahead
     case TARGET_NR_readahead:
 #if TARGET_ABI_BITS == 32
-        if (regpairs_aligned(cpu_env)) {
+        if (regpairs_aligned(cpu_env, num)) {
             arg2 = arg3;
             arg3 = arg4;
             arg4 = arg5;
-- 
2.13.2

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 811 bytes --]

On 09/15/2017 02:33 PM, James Clarke wrote:
> Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
> Signed-off-by: James Clarke <jrtc27@jrtc27.com>
> ---
> 
> Changes since v2:
>  * Fixed opening curly brace formatting, both for my new SH4-specific
>    regpairs_aligned function, as well as the Arm one I touched, to appease
>    checkpatch.pl

It's better to post your v3 as a top-level post rather than in-reply-to
v1 and v2; our automated tooling doesn't always find buried patches as
easily as new threads.  No need to resend for now, but food for thought
if you have to do a v4.

Other patch submission hints at http://wiki.qemu.org/Contribute/SubmitAPatch

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

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

[Laurent Vivier]

Le 15/09/2017 à 21:33, James Clarke a écrit :
> Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
> Signed-off-by: James Clarke <jrtc27@jrtc27.com>
> ---
> 
> Changes since v2:
>  * Fixed opening curly brace formatting, both for my new SH4-specific
>    regpairs_aligned function, as well as the Arm one I touched, to appease
>    checkpatch.pl
> 
> Changes since v1:
>  * Removed all changes in v1 :)
>  * Added syscall num argument to regpairs_aligned
>  * Added SH4-specific implementation of regpairs_aligned to return 1 for
>    p{read,write}64
> 
>  linux-user/syscall.c | 36 +++++++++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 11 deletions(-)

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

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

[Richard Henderson]

On 09/15/2017 12:33 PM, James Clarke wrote:
> Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
> Signed-off-by: James Clarke <jrtc27@jrtc27.com>
> ---
> 
> Changes since v2:
>  * Fixed opening curly brace formatting, both for my new SH4-specific
>    regpairs_aligned function, as well as the Arm one I touched, to appease
>    checkpatch.pl
> 
> Changes since v1:
>  * Removed all changes in v1 :)
>  * Added syscall num argument to regpairs_aligned
>  * Added SH4-specific implementation of regpairs_aligned to return 1 for
>    p{read,write}64
> 
>  linux-user/syscall.c | 36 +++++++++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 11 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

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

[John Paul Adrian Glaubitz]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 09/15/2017 09:33 PM, James Clarke wrote:
> Fixes: https://bugs.launchpad.net/qemu/+bug/1716767 Signed-off-by: James Clarke <jrtc27@jrtc27.com> ---
> 
> Changes since v2: * Fixed opening curly brace formatting, both for my new SH4-specific regpairs_aligned function, as well as the Arm one I touched, to
> appease checkpatch.pl
> 
> Changes since v1: * Removed all changes in v1 :) * Added syscall num argument to regpairs_aligned * Added SH4-specific implementation of regpairs_aligned
> to return 1 for p{read,write}64
> 
> linux-user/syscall.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 9b6364a266..0c1bd80bed 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@
> -667,18 +667,32 @@ static inline int next_free_host_timer(void)
> 
> /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */ #ifdef TARGET_ARM -static inline int regpairs_aligned(void *cpu_env) { 
> +static inline int regpairs_aligned(void *cpu_env, int num) +{ return ((((CPUARMState *)cpu_env)->eabi) == 1) ; } #elif defined(TARGET_MIPS) &&
> (TARGET_ABI_BITS == 32) -static inline int regpairs_aligned(void *cpu_env) { return 1; } +static inline int regpairs_aligned(void *cpu_env, int num) {
> return 1; } #elif defined(TARGET_PPC) && !defined(TARGET_PPC64) /* SysV AVI for PPC32 expects 64bit parameters to be passed on odd/even pairs * of
> registers which translates to the same as ARM/MIPS, because we start with * r3 as arg1 */ -static inline int regpairs_aligned(void *cpu_env) { return 1; } 
> +static inline int regpairs_aligned(void *cpu_env, int num) { return 1; } +#elif defined(TARGET_SH4) +/* SH4 doesn't align register pairs, except for
> p{read,write}64 */ +static inline int regpairs_aligned(void *cpu_env, int num) +{ +    switch (num) { +    case TARGET_NR_pread64: +    case
> TARGET_NR_pwrite64: +        return 1; + +    default: +        return 0; +    } +} #else -static inline int regpairs_aligned(void *cpu_env) { return 0; } 
> +static inline int regpairs_aligned(void *cpu_env, int num) { return 0; } #endif
> 
> #define ERRNO_TABLE_SIZE 1200 @@ -6857,7 +6871,7 @@ static inline abi_long target_truncate64(void *cpu_env, const char *arg1, abi_long arg3, abi_long
> arg4) { -    if (regpairs_aligned(cpu_env)) { +    if (regpairs_aligned(cpu_env, TARGET_NR_truncate64)) { arg2 = arg3; arg3 = arg4; } @@ -6871,7 +6885,7 @@
> static inline abi_long target_ftruncate64(void *cpu_env, abi_long arg1, abi_long arg3, abi_long arg4) { -    if (regpairs_aligned(cpu_env)) { +    if
> (regpairs_aligned(cpu_env, TARGET_NR_ftruncate64)) { arg2 = arg3; arg3 = arg4; } @@ -10495,7 +10509,7 @@ abi_long do_syscall(void *cpu_env, int num,
> abi_long arg1, #endif #ifdef TARGET_NR_pread64 case TARGET_NR_pread64: -        if (regpairs_aligned(cpu_env)) { +        if (regpairs_aligned(cpu_env,
> num)) { arg4 = arg5; arg5 = arg6; } @@ -10505,7 +10519,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, unlock_user(p, arg2, ret); break; 
> case TARGET_NR_pwrite64: -        if (regpairs_aligned(cpu_env)) { +        if (regpairs_aligned(cpu_env, num)) { arg4 = arg5; arg5 = arg6; } @@ -11275,7
> +11289,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, arg6 = ret; #else /* 6 args: fd, offset (high, low), len (high, low), advice */ -
> if (regpairs_aligned(cpu_env)) { +        if (regpairs_aligned(cpu_env, num)) { /* offset is in (3,4), len in (5,6) and advice in 7 */ arg2 = arg3; arg3 =
> arg4; @@ -11294,7 +11308,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, #ifdef TARGET_NR_fadvise64 case TARGET_NR_fadvise64: /* 5 args:
> fd, offset (high, low), len, advice */ -        if (regpairs_aligned(cpu_env)) { +        if (regpairs_aligned(cpu_env, num)) { /* offset is in (3,4), len
> in 5 and advice in 6 */ arg2 = arg3; arg3 = arg4; @@ -11407,7 +11421,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, #ifdef
> TARGET_NR_readahead case TARGET_NR_readahead: #if TARGET_ABI_BITS == 32 -        if (regpairs_aligned(cpu_env)) { +        if (regpairs_aligned(cpu_env,
> num)) { arg2 = arg3; arg3 = arg4; arg4 = arg5;
> 

Tested-By: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

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

iQIzBAEBCAAdFiEEYv+KdYTgKVaVRgAGdCY7N/W1+RMFAlm8OtoACgkQdCY7N/W1
+RNU1RAAjlDrSn79gJNhsCXBykjVU5i5GrOS5kGSxvs1P494ntprMyraMpcKfFlj
r2jbn9UjTshiHi9GZjsNKQF6FusNYNOPqKIIiY5Cd63yNjymTEvF+p9vlhNhr4TS
fjDZKKWJ9Xs3hzUqRTu6rfbLaG+56Yzd1pkE9iocfGT/r0zXHaSWPnKIBe0uPkkp
AR6L8lGLLeX0I4i8E2Bp9OZM8oWqn+PMQgajgPVAgaTVAWneAIwYZW2m1Ci0bSM2
7UE+/fQ8spX/MT7cOO5Yhpr+KQk9zbYWNaqyj20gbuA4TMKeH106VomZL1SrVCWg
fIUhS4Hc8AA5DXhi3Ed4o9fuZklModBw1EFzssBEVTtdmO8P6kA931AcSF0XIjLV
zhcXJLlXpjiHY9AfugapUY2JVVMUbUAX3HLZCgLqH0hqK8bjiUquioTHIwaK3FvD
pZgUj2kF5gMFFslq9Rx2DW1FlxHjHyUooaUtkmlUGcz3pVTdrmY9ngEyaI5Enh6o
Mfr0ptfJ43Zarg5FdGYur7WbunX5TSdeeCek/PKj8Xxin7y3l4/eFvAwsuhXOPWi
+kFF4dYFZsNKGwIeiPpT/YPPqf0g0tYcicG9OTR0NYy4ngPNGq0NEvI7Yuow6gAk
XZLEgzwLJLLq6kTETdMEJPCWCLGa7JIREv66drphV7EZ4sxBo0k=
=4pba
-----END PGP SIGNATURE-----

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

[John Paul Adrian Glaubitz]

(re-sent because GPG messed up the line endings)

On 09/15/2017 09:33 PM, James Clarke wrote:
> Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
> Signed-off-by: James Clarke <jrtc27@jrtc27.com>
> ---
> 
> Changes since v2:
>  * Fixed opening curly brace formatting, both for my new SH4-specific
>    regpairs_aligned function, as well as the Arm one I touched, to appease
>    checkpatch.pl
> 
> Changes since v1:
>  * Removed all changes in v1 :)
>  * Added syscall num argument to regpairs_aligned
>  * Added SH4-specific implementation of regpairs_aligned to return 1 for
>    p{read,write}64
> 
>  linux-user/syscall.c | 36 +++++++++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 9b6364a266..0c1bd80bed 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -667,18 +667,32 @@ static inline int next_free_host_timer(void)
>  
>  /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */
>  #ifdef TARGET_ARM
> -static inline int regpairs_aligned(void *cpu_env) {
> +static inline int regpairs_aligned(void *cpu_env, int num)
> +{
>      return ((((CPUARMState *)cpu_env)->eabi) == 1) ;
>  }
>  #elif defined(TARGET_MIPS) && (TARGET_ABI_BITS == 32)
> -static inline int regpairs_aligned(void *cpu_env) { return 1; }
> +static inline int regpairs_aligned(void *cpu_env, int num) { return 1; }
>  #elif defined(TARGET_PPC) && !defined(TARGET_PPC64)
>  /* SysV AVI for PPC32 expects 64bit parameters to be passed on odd/even pairs
>   * of registers which translates to the same as ARM/MIPS, because we start with
>   * r3 as arg1 */
> -static inline int regpairs_aligned(void *cpu_env) { return 1; }
> +static inline int regpairs_aligned(void *cpu_env, int num) { return 1; }
> +#elif defined(TARGET_SH4)
> +/* SH4 doesn't align register pairs, except for p{read,write}64 */
> +static inline int regpairs_aligned(void *cpu_env, int num)
> +{
> +    switch (num) {
> +    case TARGET_NR_pread64:
> +    case TARGET_NR_pwrite64:
> +        return 1;
> +
> +    default:
> +        return 0;
> +    }
> +}
>  #else
> -static inline int regpairs_aligned(void *cpu_env) { return 0; }
> +static inline int regpairs_aligned(void *cpu_env, int num) { return 0; }
>  #endif
>  
>  #define ERRNO_TABLE_SIZE 1200
> @@ -6857,7 +6871,7 @@ static inline abi_long target_truncate64(void *cpu_env, const char *arg1,
>                                           abi_long arg3,
>                                           abi_long arg4)
>  {
> -    if (regpairs_aligned(cpu_env)) {
> +    if (regpairs_aligned(cpu_env, TARGET_NR_truncate64)) {
>          arg2 = arg3;
>          arg3 = arg4;
>      }
> @@ -6871,7 +6885,7 @@ static inline abi_long target_ftruncate64(void *cpu_env, abi_long arg1,
>                                            abi_long arg3,
>                                            abi_long arg4)
>  {
> -    if (regpairs_aligned(cpu_env)) {
> +    if (regpairs_aligned(cpu_env, TARGET_NR_ftruncate64)) {
>          arg2 = arg3;
>          arg3 = arg4;
>      }
> @@ -10495,7 +10509,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>  #endif
>  #ifdef TARGET_NR_pread64
>      case TARGET_NR_pread64:
> -        if (regpairs_aligned(cpu_env)) {
> +        if (regpairs_aligned(cpu_env, num)) {
>              arg4 = arg5;
>              arg5 = arg6;
>          }
> @@ -10505,7 +10519,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>          unlock_user(p, arg2, ret);
>          break;
>      case TARGET_NR_pwrite64:
> -        if (regpairs_aligned(cpu_env)) {
> +        if (regpairs_aligned(cpu_env, num)) {
>              arg4 = arg5;
>              arg5 = arg6;
>          }
> @@ -11275,7 +11289,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>          arg6 = ret;
>  #else
>          /* 6 args: fd, offset (high, low), len (high, low), advice */
> -        if (regpairs_aligned(cpu_env)) {
> +        if (regpairs_aligned(cpu_env, num)) {
>              /* offset is in (3,4), len in (5,6) and advice in 7 */
>              arg2 = arg3;
>              arg3 = arg4;
> @@ -11294,7 +11308,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>  #ifdef TARGET_NR_fadvise64
>      case TARGET_NR_fadvise64:
>          /* 5 args: fd, offset (high, low), len, advice */
> -        if (regpairs_aligned(cpu_env)) {
> +        if (regpairs_aligned(cpu_env, num)) {
>              /* offset is in (3,4), len in 5 and advice in 6 */
>              arg2 = arg3;
>              arg3 = arg4;
> @@ -11407,7 +11421,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>  #ifdef TARGET_NR_readahead
>      case TARGET_NR_readahead:
>  #if TARGET_ABI_BITS == 32
> -        if (regpairs_aligned(cpu_env)) {
> +        if (regpairs_aligned(cpu_env, num)) {
>              arg2 = arg3;
>              arg3 = arg4;
>              arg4 = arg5;
> 

Tested-By: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

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

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

[Philippe Mathieu-Daudé]

On 09/15/2017 04:33 PM, James Clarke wrote:
> Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
> Signed-off-by: James Clarke <jrtc27@jrtc27.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
> 
> Changes since v2:
>   * Fixed opening curly brace formatting, both for my new SH4-specific
>     regpairs_aligned function, as well as the Arm one I touched, to appease
>     checkpatch.pl
> 
> Changes since v1:
>   * Removed all changes in v1 :)
>   * Added syscall num argument to regpairs_aligned
>   * Added SH4-specific implementation of regpairs_aligned to return 1 for
>     p{read,write}64

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

[John Paul Adrian Glaubitz]

Hi!

Any chance that this patch gets merged soon?

Looks like it has been reviewed by at least Richard and Philippe.

Adrian

On 09/15/2017 09:33 PM, James Clarke wrote:
> Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
> Signed-off-by: James Clarke <jrtc27@jrtc27.com>
> ---
> 
> Changes since v2:
>   * Fixed opening curly brace formatting, both for my new SH4-specific
>     regpairs_aligned function, as well as the Arm one I touched, to appease
>     checkpatch.pl
> 
> Changes since v1:
>   * Removed all changes in v1 :)
>   * Added syscall num argument to regpairs_aligned
>   * Added SH4-specific implementation of regpairs_aligned to return 1 for
>     p{read,write}64
> 
>   linux-user/syscall.c | 36 +++++++++++++++++++++++++-----------
>   1 file changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 9b6364a266..0c1bd80bed 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -667,18 +667,32 @@ static inline int next_free_host_timer(void)
>   
>   /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */
>   #ifdef TARGET_ARM
> -static inline int regpairs_aligned(void *cpu_env) {
> +static inline int regpairs_aligned(void *cpu_env, int num)
> +{
>       return ((((CPUARMState *)cpu_env)->eabi) == 1) ;
>   }
>   #elif defined(TARGET_MIPS) && (TARGET_ABI_BITS == 32)
> -static inline int regpairs_aligned(void *cpu_env) { return 1; }
> +static inline int regpairs_aligned(void *cpu_env, int num) { return 1; }
>   #elif defined(TARGET_PPC) && !defined(TARGET_PPC64)
>   /* SysV AVI for PPC32 expects 64bit parameters to be passed on odd/even pairs
>    * of registers which translates to the same as ARM/MIPS, because we start with
>    * r3 as arg1 */
> -static inline int regpairs_aligned(void *cpu_env) { return 1; }
> +static inline int regpairs_aligned(void *cpu_env, int num) { return 1; }
> +#elif defined(TARGET_SH4)
> +/* SH4 doesn't align register pairs, except for p{read,write}64 */
> +static inline int regpairs_aligned(void *cpu_env, int num)
> +{
> +    switch (num) {
> +    case TARGET_NR_pread64:
> +    case TARGET_NR_pwrite64:
> +        return 1;
> +
> +    default:
> +        return 0;
> +    }
> +}
>   #else
> -static inline int regpairs_aligned(void *cpu_env) { return 0; }
> +static inline int regpairs_aligned(void *cpu_env, int num) { return 0; }
>   #endif
>   
>   #define ERRNO_TABLE_SIZE 1200
> @@ -6857,7 +6871,7 @@ static inline abi_long target_truncate64(void *cpu_env, const char *arg1,
>                                            abi_long arg3,
>                                            abi_long arg4)
>   {
> -    if (regpairs_aligned(cpu_env)) {
> +    if (regpairs_aligned(cpu_env, TARGET_NR_truncate64)) {
>           arg2 = arg3;
>           arg3 = arg4;
>       }
> @@ -6871,7 +6885,7 @@ static inline abi_long target_ftruncate64(void *cpu_env, abi_long arg1,
>                                             abi_long arg3,
>                                             abi_long arg4)
>   {
> -    if (regpairs_aligned(cpu_env)) {
> +    if (regpairs_aligned(cpu_env, TARGET_NR_ftruncate64)) {
>           arg2 = arg3;
>           arg3 = arg4;
>       }
> @@ -10495,7 +10509,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>   #endif
>   #ifdef TARGET_NR_pread64
>       case TARGET_NR_pread64:
> -        if (regpairs_aligned(cpu_env)) {
> +        if (regpairs_aligned(cpu_env, num)) {
>               arg4 = arg5;
>               arg5 = arg6;
>           }
> @@ -10505,7 +10519,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>           unlock_user(p, arg2, ret);
>           break;
>       case TARGET_NR_pwrite64:
> -        if (regpairs_aligned(cpu_env)) {
> +        if (regpairs_aligned(cpu_env, num)) {
>               arg4 = arg5;
>               arg5 = arg6;
>           }
> @@ -11275,7 +11289,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>           arg6 = ret;
>   #else
>           /* 6 args: fd, offset (high, low), len (high, low), advice */
> -        if (regpairs_aligned(cpu_env)) {
> +        if (regpairs_aligned(cpu_env, num)) {
>               /* offset is in (3,4), len in (5,6) and advice in 7 */
>               arg2 = arg3;
>               arg3 = arg4;
> @@ -11294,7 +11308,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>   #ifdef TARGET_NR_fadvise64
>       case TARGET_NR_fadvise64:
>           /* 5 args: fd, offset (high, low), len, advice */
> -        if (regpairs_aligned(cpu_env)) {
> +        if (regpairs_aligned(cpu_env, num)) {
>               /* offset is in (3,4), len in 5 and advice in 6 */
>               arg2 = arg3;
>               arg3 = arg4;
> @@ -11407,7 +11421,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>   #ifdef TARGET_NR_readahead
>       case TARGET_NR_readahead:
>   #if TARGET_ABI_BITS == 32
> -        if (regpairs_aligned(cpu_env)) {
> +        if (regpairs_aligned(cpu_env, num)) {
>               arg2 = arg3;
>               arg3 = arg4;
>               arg4 = arg5;
> 

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

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

[John Paul Adrian Glaubitz]

Ping.

Please don't let this patch fall of the table. It's actually fixing a bug.

Thanks,
Adrian

On 10/04/2017 10:38 AM, John Paul Adrian Glaubitz wrote:
> Hi!
> 
> Any chance that this patch gets merged soon?
> 
> Looks like it has been reviewed by at least Richard and Philippe.
> 
> Adrian
> 
> On 09/15/2017 09:33 PM, James Clarke wrote:
>> Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
>> Signed-off-by: James Clarke <jrtc27@jrtc27.com>
>> ---
>>
>> Changes since v2:
>>   * Fixed opening curly brace formatting, both for my new SH4-specific
>>     regpairs_aligned function, as well as the Arm one I touched, to appease
>>     checkpatch.pl
>>
>> Changes since v1:
>>   * Removed all changes in v1 :)
>>   * Added syscall num argument to regpairs_aligned
>>   * Added SH4-specific implementation of regpairs_aligned to return 1 for
>>     p{read,write}64
>>
>>   linux-user/syscall.c | 36 +++++++++++++++++++++++++-----------
>>   1 file changed, 25 insertions(+), 11 deletions(-)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 9b6364a266..0c1bd80bed 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -667,18 +667,32 @@ static inline int next_free_host_timer(void)
>>     /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */
>>   #ifdef TARGET_ARM
>> -static inline int regpairs_aligned(void *cpu_env) {
>> +static inline int regpairs_aligned(void *cpu_env, int num)
>> +{
>>       return ((((CPUARMState *)cpu_env)->eabi) == 1) ;
>>   }
>>   #elif defined(TARGET_MIPS) && (TARGET_ABI_BITS == 32)
>> -static inline int regpairs_aligned(void *cpu_env) { return 1; }
>> +static inline int regpairs_aligned(void *cpu_env, int num) { return 1; }
>>   #elif defined(TARGET_PPC) && !defined(TARGET_PPC64)
>>   /* SysV AVI for PPC32 expects 64bit parameters to be passed on odd/even pairs
>>    * of registers which translates to the same as ARM/MIPS, because we start with
>>    * r3 as arg1 */
>> -static inline int regpairs_aligned(void *cpu_env) { return 1; }
>> +static inline int regpairs_aligned(void *cpu_env, int num) { return 1; }
>> +#elif defined(TARGET_SH4)
>> +/* SH4 doesn't align register pairs, except for p{read,write}64 */
>> +static inline int regpairs_aligned(void *cpu_env, int num)
>> +{
>> +    switch (num) {
>> +    case TARGET_NR_pread64:
>> +    case TARGET_NR_pwrite64:
>> +        return 1;
>> +
>> +    default:
>> +        return 0;
>> +    }
>> +}
>>   #else
>> -static inline int regpairs_aligned(void *cpu_env) { return 0; }
>> +static inline int regpairs_aligned(void *cpu_env, int num) { return 0; }
>>   #endif
>>     #define ERRNO_TABLE_SIZE 1200
>> @@ -6857,7 +6871,7 @@ static inline abi_long target_truncate64(void *cpu_env, const char *arg1,
>>                                            abi_long arg3,
>>                                            abi_long arg4)
>>   {
>> -    if (regpairs_aligned(cpu_env)) {
>> +    if (regpairs_aligned(cpu_env, TARGET_NR_truncate64)) {
>>           arg2 = arg3;
>>           arg3 = arg4;
>>       }
>> @@ -6871,7 +6885,7 @@ static inline abi_long target_ftruncate64(void *cpu_env, abi_long arg1,
>>                                             abi_long arg3,
>>                                             abi_long arg4)
>>   {
>> -    if (regpairs_aligned(cpu_env)) {
>> +    if (regpairs_aligned(cpu_env, TARGET_NR_ftruncate64)) {
>>           arg2 = arg3;
>>           arg3 = arg4;
>>       }
>> @@ -10495,7 +10509,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>   #endif
>>   #ifdef TARGET_NR_pread64
>>       case TARGET_NR_pread64:
>> -        if (regpairs_aligned(cpu_env)) {
>> +        if (regpairs_aligned(cpu_env, num)) {
>>               arg4 = arg5;
>>               arg5 = arg6;
>>           }
>> @@ -10505,7 +10519,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>           unlock_user(p, arg2, ret);
>>           break;
>>       case TARGET_NR_pwrite64:
>> -        if (regpairs_aligned(cpu_env)) {
>> +        if (regpairs_aligned(cpu_env, num)) {
>>               arg4 = arg5;
>>               arg5 = arg6;
>>           }
>> @@ -11275,7 +11289,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>           arg6 = ret;
>>   #else
>>           /* 6 args: fd, offset (high, low), len (high, low), advice */
>> -        if (regpairs_aligned(cpu_env)) {
>> +        if (regpairs_aligned(cpu_env, num)) {
>>               /* offset is in (3,4), len in (5,6) and advice in 7 */
>>               arg2 = arg3;
>>               arg3 = arg4;
>> @@ -11294,7 +11308,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>   #ifdef TARGET_NR_fadvise64
>>       case TARGET_NR_fadvise64:
>>           /* 5 args: fd, offset (high, low), len, advice */
>> -        if (regpairs_aligned(cpu_env)) {
>> +        if (regpairs_aligned(cpu_env, num)) {
>>               /* offset is in (3,4), len in 5 and advice in 6 */
>>               arg2 = arg3;
>>               arg3 = arg4;
>> @@ -11407,7 +11421,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>   #ifdef TARGET_NR_readahead
>>       case TARGET_NR_readahead:
>>   #if TARGET_ABI_BITS == 32
>> -        if (regpairs_aligned(cpu_env)) {
>> +        if (regpairs_aligned(cpu_env, num)) {
>>               arg2 = arg3;
>>               arg3 = arg4;
>>               arg4 = arg5;
>>
> 


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

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

[Riku Voipio]

On Wed, Oct 04, 2017 at 10:38:50AM +0200, John Paul Adrian Glaubitz wrote:
> Hi!
> 
> Any chance that this patch gets merged soon?
> 
> Looks like it has been reviewed by at least Richard and Philippe.

Sorry, slipped under radar. Applied to linux-user.
 
> Adrian
> 
> On 09/15/2017 09:33 PM, James Clarke wrote:
> >Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
> >Signed-off-by: James Clarke <jrtc27@jrtc27.com>
> >---
> >
> >Changes since v2:
> >  * Fixed opening curly brace formatting, both for my new SH4-specific
> >    regpairs_aligned function, as well as the Arm one I touched, to appease
> >    checkpatch.pl
> >
> >Changes since v1:
> >  * Removed all changes in v1 :)
> >  * Added syscall num argument to regpairs_aligned
> >  * Added SH4-specific implementation of regpairs_aligned to return 1 for
> >    p{read,write}64
> >
> >  linux-user/syscall.c | 36 +++++++++++++++++++++++++-----------
> >  1 file changed, 25 insertions(+), 11 deletions(-)
> >
> >diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> >index 9b6364a266..0c1bd80bed 100644
> >--- a/linux-user/syscall.c
> >+++ b/linux-user/syscall.c
> >@@ -667,18 +667,32 @@ static inline int next_free_host_timer(void)
> >  /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */
> >  #ifdef TARGET_ARM
> >-static inline int regpairs_aligned(void *cpu_env) {
> >+static inline int regpairs_aligned(void *cpu_env, int num)
> >+{
> >      return ((((CPUARMState *)cpu_env)->eabi) == 1) ;
> >  }
> >  #elif defined(TARGET_MIPS) && (TARGET_ABI_BITS == 32)
> >-static inline int regpairs_aligned(void *cpu_env) { return 1; }
> >+static inline int regpairs_aligned(void *cpu_env, int num) { return 1; }
> >  #elif defined(TARGET_PPC) && !defined(TARGET_PPC64)
> >  /* SysV AVI for PPC32 expects 64bit parameters to be passed on odd/even pairs
> >   * of registers which translates to the same as ARM/MIPS, because we start with
> >   * r3 as arg1 */
> >-static inline int regpairs_aligned(void *cpu_env) { return 1; }
> >+static inline int regpairs_aligned(void *cpu_env, int num) { return 1; }
> >+#elif defined(TARGET_SH4)
> >+/* SH4 doesn't align register pairs, except for p{read,write}64 */
> >+static inline int regpairs_aligned(void *cpu_env, int num)
> >+{
> >+    switch (num) {
> >+    case TARGET_NR_pread64:
> >+    case TARGET_NR_pwrite64:
> >+        return 1;
> >+
> >+    default:
> >+        return 0;
> >+    }
> >+}
> >  #else
> >-static inline int regpairs_aligned(void *cpu_env) { return 0; }
> >+static inline int regpairs_aligned(void *cpu_env, int num) { return 0; }
> >  #endif
> >  #define ERRNO_TABLE_SIZE 1200
> >@@ -6857,7 +6871,7 @@ static inline abi_long target_truncate64(void *cpu_env, const char *arg1,
> >                                           abi_long arg3,
> >                                           abi_long arg4)
> >  {
> >-    if (regpairs_aligned(cpu_env)) {
> >+    if (regpairs_aligned(cpu_env, TARGET_NR_truncate64)) {
> >          arg2 = arg3;
> >          arg3 = arg4;
> >      }
> >@@ -6871,7 +6885,7 @@ static inline abi_long target_ftruncate64(void *cpu_env, abi_long arg1,
> >                                            abi_long arg3,
> >                                            abi_long arg4)
> >  {
> >-    if (regpairs_aligned(cpu_env)) {
> >+    if (regpairs_aligned(cpu_env, TARGET_NR_ftruncate64)) {
> >          arg2 = arg3;
> >          arg3 = arg4;
> >      }
> >@@ -10495,7 +10509,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
> >  #endif
> >  #ifdef TARGET_NR_pread64
> >      case TARGET_NR_pread64:
> >-        if (regpairs_aligned(cpu_env)) {
> >+        if (regpairs_aligned(cpu_env, num)) {
> >              arg4 = arg5;
> >              arg5 = arg6;
> >          }
> >@@ -10505,7 +10519,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
> >          unlock_user(p, arg2, ret);
> >          break;
> >      case TARGET_NR_pwrite64:
> >-        if (regpairs_aligned(cpu_env)) {
> >+        if (regpairs_aligned(cpu_env, num)) {
> >              arg4 = arg5;
> >              arg5 = arg6;
> >          }
> >@@ -11275,7 +11289,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
> >          arg6 = ret;
> >  #else
> >          /* 6 args: fd, offset (high, low), len (high, low), advice */
> >-        if (regpairs_aligned(cpu_env)) {
> >+        if (regpairs_aligned(cpu_env, num)) {
> >              /* offset is in (3,4), len in (5,6) and advice in 7 */
> >              arg2 = arg3;
> >              arg3 = arg4;
> >@@ -11294,7 +11308,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
> >  #ifdef TARGET_NR_fadvise64
> >      case TARGET_NR_fadvise64:
> >          /* 5 args: fd, offset (high, low), len, advice */
> >-        if (regpairs_aligned(cpu_env)) {
> >+        if (regpairs_aligned(cpu_env, num)) {
> >              /* offset is in (3,4), len in 5 and advice in 6 */
> >              arg2 = arg3;
> >              arg3 = arg4;
> >@@ -11407,7 +11421,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
> >  #ifdef TARGET_NR_readahead
> >      case TARGET_NR_readahead:
> >  #if TARGET_ABI_BITS == 32
> >-        if (regpairs_aligned(cpu_env)) {
> >+        if (regpairs_aligned(cpu_env, num)) {
> >              arg2 = arg3;
> >              arg3 = arg4;
> >              arg4 = arg5;
> >
> 
> -- 
>  .''`.  John Paul Adrian Glaubitz
> : :' :  Debian Developer - glaubitz@debian.org
> `. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
>   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
> 

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

[James Clarke]

On 6 Nov 2017, at 19:57, Riku Voipio <riku.voipio@iki.fi> wrote:
> 
> On Wed, Oct 04, 2017 at 10:38:50AM +0200, John Paul Adrian Glaubitz wrote:
>> Hi!
>> 
>> Any chance that this patch gets merged soon?
>> 
>> Looks like it has been reviewed by at least Richard and Philippe.
> 
> Sorry, slipped under radar. Applied to linux-user.

Thanks! These things happen.

James