Re: [PATCH v5] MIPS: Truncate link address into 32bit for 32bit kernel

[Maciej W. Rozycki]

On Wed, 22 Apr 2020, Jiaxun Yang wrote:

> Reviewed-by: Maciej W. Rozycki <macro@xxxxxxxxxxxxxx>

 Hmm, that was for an earlier version of the patch, and reviews obviously 
do not automatically carry over to subsequent versions, as it cannot be 
claimed that they are as good in the reviewer's eyes as the actual version 
reviewed was.

> diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> index e1c44aed8156..68c0f22fefc0 100644
> --- a/arch/mips/Makefile
> +++ b/arch/mips/Makefile
> @@ -288,12 +288,23 @@ ifdef CONFIG_64BIT
>    endif
>  endif
>  
> +# When linking a 32-bit executable the LLVM linker cannot cope with a
> +# 32-bit load address that has been sign-extended to 64 bits.  Simply
> +# remove the upper 32 bits then, as it is safe to do so with other
> +# linkers.
> +ifdef CONFIG_64BIT
> +	load-ld			= $(load-y)
> +else
> +	load-ld			= $(subst 0xffffffff,0x,$(load-y))
> +endif
> +
>  KBUILD_AFLAGS	+= $(cflags-y)
>  KBUILD_CFLAGS	+= $(cflags-y)
> -KBUILD_CPPFLAGS += -DVMLINUX_LOAD_ADDRESS=$(load-y)
> +KBUILD_CPPFLAGS += -DVMLINUX_LOAD_ADDRESS=$(load-y) -DVMLINUX_LINK_ADDRESS=$(load-ld)
>  KBUILD_CPPFLAGS += -DDATAOFFSET=$(if $(dataoffset-y),$(dataoffset-y),0)
>  
>  bootvars-y	= VMLINUX_LOAD_ADDRESS=$(load-y) \
> +		  VMLINUX_LINK_ADDRESS=$(load-ld) \
>  		  VMLINUX_ENTRY_ADDRESS=$(entry-y) \
>  		  PLATFORM="$(platform-y)" \
>  		  ITS_INPUTS="$(its-y)"

 Hmm, to be honest I find the nomenclature confusing: VMLINUX_LOAD_ADDRESS 
and VMLINUX_LINK_ADDRESS sound like synonyms to me and also look very 
similar, so I expect people will be confused and scratch their heads in 
the future.  Due to the obscurity of the problem I think there is little 
room for manoeuvre here really, but how about using LINKER_LOAD_ADDRESS 
for the new variable?

 Alternatively, have you made any attempt to verify if actually replacing 
the setting for VMLINUX_LOAD_ADDRESS would be safe?  Glancing over its use 
there do not appear to be many places.

  Maciej