Re: [Qemu-devel] [PATCH 1/2] Support for MIPS64 user mode emulation

[Aurelien Jarno <aurelien@aurel32.net>]

[I don't know very well linux-user, it would be nice to Cc: Riku Voipio,
 the linux-user maintainer for the next version.]

On Sat, Apr 09, 2011 at 04:02:31PM +0500, Khansa Butt wrote:
> From e96e20e50cada1c9e1b65de5925281cdd5659746 Mon Sep 17 00:00:00 2001
> From: Ehsan-ul-Haq & Khansa Butt <khansa@kics.edu.pk>
> Date: Sat, 9 Apr 2011 10:51:22 +0500
> Subject: [PATCH 1/2] Support for MIPS64 user mode emulation
> 
> 
> Signed-off-by: Ehsan-ul-Haq, Abdul Qadeer, Abdul Waheed, Khansa Butt <
> khansa@kics.edu.pk>
> ---
>  configure                             |    1 +
>  default-configs/mips64-linux-user.mak |    1 +
>  linux-user/elfload.c                  |    2 +-
>  linux-user/main.c                     |   29 +++++++++++++++++++++++++++--
>  linux-user/mips64/syscall.h           |    3 +++
>  linux-user/signal.c                   |    3 ++-
>  target-mips/translate.c               |    1 +
>  7 files changed, 36 insertions(+), 4 deletions(-)
>  create mode 100644 default-configs/mips64-linux-user.mak
> 
> diff --git a/configure b/configure
> index ae97e11..d1f7867 100755
> --- a/configure
> +++ b/configure
> @@ -1039,6 +1039,7 @@ m68k-linux-user \
>  microblaze-linux-user \
>  microblazeel-linux-user \
>  mips-linux-user \
> +mips64-linux-user \
>  mipsel-linux-user \
>  ppc-linux-user \
>  ppc64-linux-user \
> diff --git a/default-configs/mips64-linux-user.mak
> b/default-configs/mips64-linux-user.mak
> new file mode 100644
> index 0000000..1598bfc
> --- /dev/null
> +++ b/default-configs/mips64-linux-user.mak
> @@ -0,0 +1 @@
> +# Default configuration for mips64-linux-user
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index fe5410e..2832a33 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -1384,7 +1384,7 @@ static void load_elf_image(const char *image_name, int
> image_fd,
>              vaddr_po = TARGET_ELF_PAGEOFFSET(vaddr);
>              vaddr_ps = TARGET_ELF_PAGESTART(vaddr);
> 
> -            error = target_mmap(vaddr_ps, eppnt->p_filesz + vaddr_po,
> +            error = target_mmap(vaddr_ps, eppnt->p_memsz + vaddr_po,

What is the goal of this change? If the mmapped aread is bigger than the
file size rounded up to te page size, it will cause a SIGBUS.

>                                  elf_prot, MAP_PRIVATE | MAP_FIXED,
>                                  image_fd, eppnt->p_offset - vaddr_po);
>              if (error == -1) {
> diff --git a/linux-user/main.c b/linux-user/main.c
> index e651bfd..a7f4955 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -1937,6 +1937,14 @@ static int do_store_exclusive(CPUMIPSState *env)
>      int d;
> 
>      addr = env->lladdr;
> +#if defined(TARGET_MIPS64)
> +/* For MIPS64 on 32 bit host there is a need to make
> +* the page accessible to which the above 'addr' is belonged */
> +#if HOST_LONG_BITS == 32
> +    int flag = PAGE_VALID | PAGE_READ | PAGE_WRITE | PAGE_WRITE_ORG;
> +    page_set_flags(addr, addr + 4096, flag);
> +#endif
> +#endif

I don't really see the reason why this should be done that way. Are you
trying to run MIPS32 binaries compiled for 8kB page size?

>      page_addr = addr & TARGET_PAGE_MASK;
>      start_exclusive();
>      mmap_lock();
> @@ -1978,7 +1986,8 @@ static int do_store_exclusive(CPUMIPSState *env)
>  void cpu_loop(CPUMIPSState *env)
>  {
>      target_siginfo_t info;
> -    int trapnr, ret;
> +    int trapnr;
> +    abi_long ret;
>      unsigned int syscall_num;
> 
>      for(;;) {
> @@ -1987,7 +1996,11 @@ void cpu_loop(CPUMIPSState *env)
>          cpu_exec_end(env);
>          switch(trapnr) {
>          case EXCP_SYSCALL:
> +#if defined(TARGET_MIPS64)
> +            syscall_num = env->active_tc.gpr[2] - 5000;
> +#else
>              syscall_num = env->active_tc.gpr[2] - 4000;
> +#endif
>              env->active_tc.PC += 4;
>              if (syscall_num >= sizeof(mips_syscall_args)) {

I don't think you can do it like that. mips_syscall_args corresponds to
a table of 32-bit syscalls, which have totally different numbers than
64-bit syscalls. It's not a simple shift by 1000. See for example
pread64 is 4200 is 32-bit, 5016 in 64-bit.

>                  ret = -ENOSYS;
> @@ -2008,12 +2021,22 @@ void cpu_loop(CPUMIPSState *env)
>                  default:
>                      break;
>                  }
> +#if defined(TARGET_MIPS64)
> +                ret = do_syscall(env, env->active_tc.gpr[2],
> +                                 env->active_tc.gpr[4],
> +                                 env->active_tc.gpr[5],
> +                                 env->active_tc.gpr[6],
> +                                 env->active_tc.gpr[7],
> +                                 env->active_tc.gpr[8],
> +                                 env->active_tc.gpr[9]);
> +#else
>                  ret = do_syscall(env, env->active_tc.gpr[2],
>                                   env->active_tc.gpr[4],
>                                   env->active_tc.gpr[5],
>                                   env->active_tc.gpr[6],
>                                   env->active_tc.gpr[7],
>                                   arg5, arg6/*, arg7, arg8*/);
> +#endif
>              }
>              if (ret == -TARGET_QEMU_ESIGRETURN) {
>                  /* Returning from a successful sigreturn syscall.
> @@ -2935,7 +2958,9 @@ int main(int argc, char **argv, char **envp)
>  #endif
>  #elif defined(TARGET_MIPS)
>  #if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64)
> -        cpu_model = "20Kc";
> +        /* we use this model so that we can decode MIPS64r2
> +           reserved instruction */
> +        cpu_model = "MIPS64R2-generic";
>  #else
>          cpu_model = "24Kf";
>  #endif
> diff --git a/linux-user/mips64/syscall.h b/linux-user/mips64/syscall.h
> index 668a2b9..ec65653 100644
> --- a/linux-user/mips64/syscall.h
> +++ b/linux-user/mips64/syscall.h
> @@ -218,4 +218,7 @@ struct target_pt_regs {
> 
> 
> 
> +/* Nasty hack: define a fake errno value for use by sigreturn.  */
> +#define TARGET_QEMU_ESIGRETURN 255
> +
>  #define UNAME_MACHINE "mips64"
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index ce033e9..a3d49dd 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -2413,7 +2413,8 @@ void sparc64_get_context(CPUSPARCState *env)
>  #endif
>  #elif defined(TARGET_ABI_MIPSN64)
> 
> -# warning signal handling not implemented
> +/* This line is commented out to avoid compile time error */
> +/* # warning signal handling not implemented */

Wouldn't it be possible to implement signal handling instead?

>  static void setup_frame(int sig, struct target_sigaction *ka,
>   target_sigset_t *set, CPUState *env)
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 0f93e2a..63c2563 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -12694,6 +12694,7 @@ void cpu_reset (CPUMIPSState *env)
>          env->hflags |= MIPS_HFLAG_FPU;
>      }
>  #ifdef TARGET_MIPS64
> +    env->hflags |=  MIPS_HFLAG_UX;
>      if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
>          env->hflags |= MIPS_HFLAG_F64;
>      }
> -- 
> 1.7.3.4


-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net