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

From: Jiaxun Yang <jiaxun.yang@flygoat.com>
To: "Maciej W. Rozycki" <macro@linux-mips.org>
Cc: linux-mips@vger.kernel.org, clang-built-linux@googlegroups.com,
	Fangrui Song <maskray@google.com>,
	Kees Cook <keescook@chromium.org>,
	Nathan Chancellor <natechancellor@gmail.com>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Paul Burton <paulburton@kernel.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Jouni Hogander <jouni.hogander@unikie.com>,
	Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>,
	Borislav Petkov <bp@suse.de>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5] MIPS: Truncate link address into 32bit for 32bit kernel
Date: Thu, 23 Apr 2020 13:42:52 +0800	[thread overview]
Message-ID: <B307BFAC-9973-4444-B69A-40B054210E84@flygoat.com> (raw)
In-Reply-To: <alpine.LFD.2.21.2004230036480.851719@eddie.linux-mips.org>

于 2020年4月23日 GMT+08:00 上午8:10:12, "Maciej W. Rozycki" <macro@linux-mips.org> 写到:
>On Wed, 22 Apr 2020, Jiaxun Yang wrote:
>
>> Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org>
>
> 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.

Limited experiments showed it should be fine...

But MIPS kernel has some design I'm not really familiar with like SYM32 for
64-bit kernel and special address space design for Trap-and-emul KVM.

I'm not 100 sure it's safe so I didn't do so.

>
>  Maciej

-- 
Jiaxun Yang