Re: [PATCH 2/7] tcg: Remove softmmu code_gen_buffer fixed address

[Thomas Huth <thuth@redhat.com>]

On 18/12/2019 04.19, Richard Henderson wrote:
> The commentary talks about "in concert with the addresses
> assigned in the relevant linker script", except there is no
> linker script for softmmu, nor has there been for some time.
> 
> (Do not confuse the user-only linker script editing that was
> removed in the previous patch, because user-only does not
> use this code_gen_buffer allocation method.)
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  accel/tcg/translate-all.c | 37 +++++--------------------------------
>  1 file changed, 5 insertions(+), 32 deletions(-)
> 
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 9f48da9472..88468a1c08 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -1032,47 +1032,20 @@ static inline void *alloc_code_gen_buffer(void)
>  {
>      int prot = PROT_WRITE | PROT_READ | PROT_EXEC;
>      int flags = MAP_PRIVATE | MAP_ANONYMOUS;
> -    uintptr_t start = 0;
>      size_t size = tcg_ctx->code_gen_buffer_size;
>      void *buf;
>  
> -    /* Constrain the position of the buffer based on the host cpu.
> -       Note that these addresses are chosen in concert with the
> -       addresses assigned in the relevant linker script file.  */
> -# if defined(__PIE__) || defined(__PIC__)
> -    /* Don't bother setting a preferred location if we're building
> -       a position-independent executable.  We're more likely to get
> -       an address near the main executable if we let the kernel
> -       choose the address.  */
> -# elif defined(__x86_64__) && defined(MAP_32BIT)
> -    /* Force the memory down into low memory with the executable.
> -       Leave the choice of exact location with the kernel.  */
> -    flags |= MAP_32BIT;
> -    /* Cannot expect to map more than 800MB in low memory.  */
> -    if (size > 800u * 1024 * 1024) {
> -        tcg_ctx->code_gen_buffer_size = size = 800u * 1024 * 1024;
> -    }
> -# elif defined(__sparc__)
> -    start = 0x40000000ul;
> -# elif defined(__s390x__)
> -    start = 0x90000000ul;
> -# elif defined(__mips__)
> -#  if _MIPS_SIM == _ABI64
> -    start = 0x128000000ul;
> -#  else
> -    start = 0x08000000ul;
> -#  endif
> -# endif
> -
> -    buf = mmap((void *)start, size, prot, flags, -1, 0);
> +    buf = mmap(NULL, size, prot, flags, -1, 0);
>      if (buf == MAP_FAILED) {
>          return NULL;
>      }
>  
>  #ifdef __mips__
>      if (cross_256mb(buf, size)) {
> -        /* Try again, with the original still mapped, to avoid re-acquiring
> -           that 256mb crossing.  This time don't specify an address.  */
> +        /*
> +         * Try again, with the original still mapped, to avoid re-acquiring
> +         * the same 256mb crossing.
> +         */
>          size_t size2;
>          void *buf2 = mmap(NULL, size, prot, flags, -1, 0);
>          switch ((int)(buf2 != MAP_FAILED)) {
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>