1. Patchew
  2. QEMU
  3. View series

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

James Clarke posted 1 patch 6 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170915193313.86362-1-jrtc27@jrtc27.com
Test checkpatch passed
Test docker passed
Test s390x passed
linux-user/syscall.c | 36 +++++++++++++++++++++++++-----------
1 file changed, 25 insertions(+), 11 deletions(-)
[Qemu-devel] [PATCH v3] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64
Posted by James Clarke 6 years, 7 months ago 
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
Posted by Eric Blake 6 years, 7 months ago 
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
Re: [Qemu-devel] [PATCH v3] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64
Posted by Laurent Vivier 6 years, 7 months ago 
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
Posted by Richard Henderson 6 years, 7 months ago 
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
Posted by John Paul Adrian Glaubitz 6 years, 7 months ago 
-----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
Posted by John Paul Adrian Glaubitz 6 years, 7 months ago 
(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
Posted by Philippe Mathieu-Daudé 6 years, 7 months ago 
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
Posted by John Paul Adrian Glaubitz 6 years, 6 months ago 
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
Posted by John Paul Adrian Glaubitz 6 years, 5 months ago 
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
Posted by Riku Voipio 6 years, 5 months ago 
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
Posted by James Clarke 6 years, 5 months ago 
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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.