[Qemu-devel] [PATCH 1/2] linux-user: ppc: mark as long long aligned

[Qemu-devel] [PATCH 1/2] linux-user: ppc: mark as long long aligned

[Alexander Graf]

The PPC32 ABI dictates that long long (64bit) parameters are pass in odd/even
register pairs. Because unlike ARM and MIPS we start at an odd register number,
we can reuse the same aligning code that ARM and MIPS use.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 linux-user/syscall.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 1a38169..8cd56f2 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -587,12 +587,16 @@ extern int setfsgid(int);
 extern int setgroups(int, gid_t *);
 
 /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */
-#ifdef TARGET_ARM 
+#ifdef TARGET_ARM
 static inline int regpairs_aligned(void *cpu_env) {
     return ((((CPUARMState *)cpu_env)->eabi) == 1) ;
 }
 #elif defined(TARGET_MIPS)
 static inline int regpairs_aligned(void *cpu_env) { return 1; }
+#elif defined(TARGET_PPC) && !defined(TARGET_PPC64)
+/* 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; }
 #else
 static inline int regpairs_aligned(void *cpu_env) { return 0; }
 #endif
-- 
1.6.0.2

[Qemu-devel] [PATCH 2/2] linux-user: register align p{read, write}64

[Alexander Graf]

pread64 and pwrite64 pass 64bit parameters which for some architectures need
to be aligned to special argument pairs, creating a gap argument.

Handle this special case the same way we handle it in other places of the code.

Reported-by: Alex Barcelo <abarcelo@ac.upc.edu>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 linux-user/syscall.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 8cd56f2..7992b1b 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7423,12 +7423,20 @@ 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)) {
+            arg4 = arg5;
+            arg5 = arg6;
+        }
         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 (regpairs_aligned(cpu_env)) {
+            arg4 = arg5;
+            arg5 = arg6;
+        }
         if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))
             goto efault;
         ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg4, arg5)));
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 1/2] linux-user: ppc: mark as long long aligned

[Andreas Färber]

Am 30.09.2012 03:32, schrieb Alexander Graf:
> The PPC32 ABI dictates that long long (64bit) parameters are pass in odd/even
> register pairs. Because unlike ARM and MIPS we start at an odd register number,
> we can reuse the same aligning code that ARM and MIPS use.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  linux-user/syscall.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1a38169..8cd56f2 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -587,12 +587,16 @@ extern int setfsgid(int);
>  extern int setgroups(int, gid_t *);
>  
>  /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */
> -#ifdef TARGET_ARM 
> +#ifdef TARGET_ARM

For anyone wondering, this is dropping a whitespace at end of line. ;)

>  static inline int regpairs_aligned(void *cpu_env) {
>      return ((((CPUARMState *)cpu_env)->eabi) == 1) ;
>  }
>  #elif defined(TARGET_MIPS)
>  static inline int regpairs_aligned(void *cpu_env) { return 1; }
> +#elif defined(TARGET_PPC) && !defined(TARGET_PPC64)
> +/* 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; }
>  #else
>  static inline int regpairs_aligned(void *cpu_env) { return 0; }
>  #endif

It is obvious that this function has been copied unmodified from mips,
but shouldn't new code use bool and true, assuming that there is no
magic performed on the return value? :)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 1/2] linux-user: ppc: mark as long long aligned

[Alexander Graf]

On 01.10.2012, at 15:04, Andreas Färber wrote:

> Am 30.09.2012 03:32, schrieb Alexander Graf:
>> The PPC32 ABI dictates that long long (64bit) parameters are pass in odd/even
>> register pairs. Because unlike ARM and MIPS we start at an odd register number,
>> we can reuse the same aligning code that ARM and MIPS use.
>> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> linux-user/syscall.c |    6 +++++-
>> 1 files changed, 5 insertions(+), 1 deletions(-)
>> 
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 1a38169..8cd56f2 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -587,12 +587,16 @@ extern int setfsgid(int);
>> extern int setgroups(int, gid_t *);
>> 
>> /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */
>> -#ifdef TARGET_ARM 
>> +#ifdef TARGET_ARM
> 
> For anyone wondering, this is dropping a whitespace at end of line. ;)

Yeah, my vi scripts marked it as red, which always makes me nervous :).

> 
>> static inline int regpairs_aligned(void *cpu_env) {
>>     return ((((CPUARMState *)cpu_env)->eabi) == 1) ;
>> }
>> #elif defined(TARGET_MIPS)
>> static inline int regpairs_aligned(void *cpu_env) { return 1; }
>> +#elif defined(TARGET_PPC) && !defined(TARGET_PPC64)
>> +/* 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; }
>> #else
>> static inline int regpairs_aligned(void *cpu_env) { return 0; }
>> #endif
> 
> It is obvious that this function has been copied unmodified from mips,
> but shouldn't new code use bool and true, assuming that there is no
> magic performed on the return value? :)

The asm result should be the same, since they're all getting inlined, no? And it wouldn't tremendously increase readability either. So for now, I'd say either

  a) send a separate cleanup patch

or

  b) leave it as is :)


Alex

Re: [Qemu-devel] [PATCH 1/2] linux-user: ppc: mark as long long aligned

[malc]

On Sun, 30 Sep 2012, Alexander Graf wrote:

> The PPC32 ABI dictates that long long (64bit) parameters are pass in odd/even
> register pairs. Because unlike ARM and MIPS we start at an odd register number,
> we can reuse the same aligning code that ARM and MIPS use.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  linux-user/syscall.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1a38169..8cd56f2 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -587,12 +587,16 @@ extern int setfsgid(int);
>  extern int setgroups(int, gid_t *);
>  
>  /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */
> -#ifdef TARGET_ARM 
> +#ifdef TARGET_ARM
>  static inline int regpairs_aligned(void *cpu_env) {
>      return ((((CPUARMState *)cpu_env)->eabi) == 1) ;
>  }
>  #elif defined(TARGET_MIPS)
>  static inline int regpairs_aligned(void *cpu_env) { return 1; }
> +#elif defined(TARGET_PPC) && !defined(TARGET_PPC64)
> +/* 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 */

This is inaccurate, PPC32 doesn't expect anything, SysV ABI for PPC32,
which linux uses, does. This is linux-user so SysV ABI is a given but
still i'd reword it.

> +static inline int regpairs_aligned(void *cpu_env) { return 1; }
>  #else
>  static inline int regpairs_aligned(void *cpu_env) { return 0; }
>  #endif
> 

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 2/2] linux-user: register align p{read, write}64

[Alex Barcelo]

On Sun, Sep 30, 2012 at 3:32 AM, Alexander Graf <agraf@suse.de> wrote:
> pread64 and pwrite64 pass 64bit parameters which for some architectures need
> to be aligned to special argument pairs, creating a gap argument.
>
> Handle this special case the same way we handle it in other places of the code.
>
> Reported-by: Alex Barcelo <abarcelo@ac.upc.edu>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  linux-user/syscall.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 8cd56f2..7992b1b 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7423,12 +7423,20 @@ 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)) {
> +            arg4 = arg5;
> +            arg5 = arg6;
> +        }
>          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 (regpairs_aligned(cpu_env)) {
> +            arg4 = arg5;
> +            arg5 = arg6;
> +        }
>          if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))
>              goto efault;
>          ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg4, arg5)));
> --
> 1.6.0.2
>
>

Tested-by: Alex Barcelo <abarcelo@ac.upc.edu>

Re: [Qemu-devel] [PATCH 2/2] linux-user: register align p{read, write}64

[Peter Maydell]

On 30 September 2012 02:32, Alexander Graf <agraf@suse.de> wrote:
> pread64 and pwrite64 pass 64bit parameters which for some architectures need
> to be aligned to special argument pairs, creating a gap argument.
>
> Handle this special case the same way we handle it in other places of the code.
>
> Reported-by: Alex Barcelo <abarcelo@ac.upc.edu>
> Signed-off-by: Alexander Graf <agraf@suse.de>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>