Re: [Qemu-devel] [PATCH 02/23] bsd-user: add initial support for mips/mips64

[Peter Maydell]

On 24 June 2013 03:03, Stacey Son <address@hidden> wrote:
> Add the main cpu loop, cpu_loop(), for mips and mips64 architecture.  Set the
> cpu model. Add some stubs for future code.
>
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -2,6 +2,7 @@
>   *  qemu user main
>   *
>   *  Copyright (c) 2003-2008 Fabrice Bellard
> + *  Copyright (c) 2013 Stacey Son
>   *
>   *  This program is free software; you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License as published by
> @@ -23,6 +24,7 @@
>  #include <errno.h>
>  #include <unistd.h>
>  #include <machine/trap.h>
> +#include <sys/syscall.h>
>  #include <sys/types.h>
>  #include <sys/mman.h>
>
> @@ -387,6 +389,172 @@ void cpu_loop(CPUX86State *env)
>  }
>  #endif
>
> +#if defined(TARGET_MIPS)
> +
> +/* Compare to sys/mips/mips/trap.c */
> +
> +void cpu_loop(CPUMIPSState *env)

I suspect you'd do better in the long term to restructure
to pull cpu_loop out into a per-architecture source file
and avoid the ifdefs. (This is something I'd like to do to
the linux-user/ code, which is currently even more ifdef-ridden
than bsd-user/.)

> --- a/target-mips/mips-defs.h
> +++ b/target-mips/mips-defs.h
> @@ -10,8 +10,17 @@
>
>  #if defined(TARGET_MIPS64)
>  #define TARGET_LONG_BITS 64
> -#define TARGET_PHYS_ADDR_SPACE_BITS 36
> -#define TARGET_VIRT_ADDR_SPACE_BITS 42
> +#  if defined(CONFIG_BSD) && defined(CONFIG_USER_ONLY)
> +#    define TARGET_PHYS_ADDR_SPACE_BITS 59
> +#    ifdef TARGET_ABI32
> +#       define TARGET_VIRT_ADDR_SPACE_BITS 32
> +#    else
> +#       define TARGET_VIRT_ADDR_SPACE_BITS 62
> +#    endif
> +#  else
> +#    define TARGET_PHYS_ADDR_SPACE_BITS 36
> +#    define TARGET_VIRT_ADDR_SPACE_BITS 42
> +#  endif
>  #else
>  #define TARGET_LONG_BITS 32
>  #define TARGET_PHYS_ADDR_SPACE_BITS 36

This looks a little fishy -- could you give some rationale?
Why does only BSD need to do this? Why do you need to
change the TARGET_PHYS_ADDR_SPACE_BITS for a -user target?
Where do the numbers come from?

thanks
-- PMM