[PATCH] MIPS: malta: Set load address for 32bit kernel correctly

[PATCH] MIPS: malta: Set load address for 32bit kernel correctly

[Jiaxun Yang]

LLD failed to link vmlinux with 64bit load address for 32bit ELF
while bfd will strip 64bit address into 32bit silently.
To fix LLD build, we should supply a 32bit load address for 32bit
kernel.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Fangrui Song <maskray@google.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
---
 arch/mips/mti-malta/Platform | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/mips/mti-malta/Platform b/arch/mips/mti-malta/Platform
index 2cc72c9b38e3..f9b49cba1764 100644
--- a/arch/mips/mti-malta/Platform
+++ b/arch/mips/mti-malta/Platform
@@ -6,6 +6,10 @@ cflags-$(CONFIG_MIPS_MALTA)	+= -I$(srctree)/arch/mips/include/asm/mach-malta
 ifdef CONFIG_KVM_GUEST
     load-$(CONFIG_MIPS_MALTA)	+= 0x0000000040100000
 else
+ifdef CONFIG_64BIT
     load-$(CONFIG_MIPS_MALTA)	+= 0xffffffff80100000
+else
+    load-$(CONFIG_MIPS_MALTA)	+= 0x80100000
+endif
 endif
 all-$(CONFIG_MIPS_MALTA)	:= $(COMPRESSION_FNAME).bin
-- 
2.26.0.rc2

Re: [PATCH] MIPS: malta: Set load address for 32bit kernel correctly

[Maciej W. Rozycki]

On Sun, 5 Apr 2020, Jiaxun Yang wrote:

> LLD failed to link vmlinux with 64bit load address for 32bit ELF
> while bfd will strip 64bit address into 32bit silently.
> To fix LLD build, we should supply a 32bit load address for 32bit
> kernel.
[...]
> diff --git a/arch/mips/mti-malta/Platform b/arch/mips/mti-malta/Platform
> index 2cc72c9b38e3..f9b49cba1764 100644
> --- a/arch/mips/mti-malta/Platform
> +++ b/arch/mips/mti-malta/Platform
> @@ -6,6 +6,10 @@ cflags-$(CONFIG_MIPS_MALTA)	+= -I$(srctree)/arch/mips/include/asm/mach-malta
>  ifdef CONFIG_KVM_GUEST
>      load-$(CONFIG_MIPS_MALTA)	+= 0x0000000040100000
>  else
> +ifdef CONFIG_64BIT
>      load-$(CONFIG_MIPS_MALTA)	+= 0xffffffff80100000
> +else
> +    load-$(CONFIG_MIPS_MALTA)	+= 0x80100000

 Given the description above I think it should be done uniformly and 
automatically across all platforms by trimming the address supplied with 
$(load-y) to low 8 digits in a single place, that is at the place where 
the variable is consumed.  This will reduce clutter across Makefile 
fragments, avoid inconsistencies and extra work to handle individual 
platforms as the problem is triggered over and over again, and limit the 
risk of mistakes.

 Some error checking might be doable for verifying that the 64-bit address 
truncated is a sign-extended 32-bit value, but that perhaps would be an 
overkill as certainly any 64-bit system that sets the load address to be 
outside the sign-extended 32-bit address range does not support a !64BIT 
configuration anyway.

  Maciej

Re: [PATCH] MIPS: malta: Set load address for 32bit kernel correctly

[Jiaxun Yang]

于 2020年4月6日 GMT+08:00 上午12:47:29, "Maciej W. Rozycki" <macro@linux-mips.org> 写到:
>On Sun, 5 Apr 2020, Jiaxun Yang wrote:
>
>> LLD failed to link vmlinux with 64bit load address for 32bit ELF
>> while bfd will strip 64bit address into 32bit silently.
>> To fix LLD build, we should supply a 32bit load address for 32bit
>> kernel.
>[...]
>> diff --git a/arch/mips/mti-malta/Platform
>b/arch/mips/mti-malta/Platform
>> index 2cc72c9b38e3..f9b49cba1764 100644
>> --- a/arch/mips/mti-malta/Platform
>> +++ b/arch/mips/mti-malta/Platform
>> @@ -6,6 +6,10 @@ cflags-$(CONFIG_MIPS_MALTA)	+=
>-I$(srctree)/arch/mips/include/asm/mach-malta
>>  ifdef CONFIG_KVM_GUEST
>>      load-$(CONFIG_MIPS_MALTA)	+= 0x0000000040100000
>>  else
>> +ifdef CONFIG_64BIT
>>      load-$(CONFIG_MIPS_MALTA)	+= 0xffffffff80100000
>> +else
>> +    load-$(CONFIG_MIPS_MALTA)	+= 0x80100000
>
> Given the description above I think it should be done uniformly and 
>automatically across all platforms by trimming the address supplied
>with 
>$(load-y) to low 8 digits in a single place, that is at the place where
>
>the variable is consumed.  This will reduce clutter across Makefile 
>fragments, avoid inconsistencies and extra work to handle individual 
>platforms as the problem is triggered over and over again, and limit
>the 
>risk of mistakes.

I was intended to do like this but failed to find a proper way.

Makefile isn't designed for any kind of calculation.
And shell variables are 64-bit signed so it can't hold such a huge variable.

Just wish somebody can give me a way to do like:

ifndef CONFIG_64BIT
load-y = $(load-y) & 0xffffffff
endif

In makefiles.

Thanks.
>
>Some error checking might be doable for verifying that the 64-bit
>address 
>truncated is a sign-extended 32-bit value, but that perhaps would be an
>
>overkill as certainly any 64-bit system that sets the load address to
>be 
>outside the sign-extended 32-bit address range does not support a
>!64BIT 
>configuration anyway.
>
>  Maciej

-- 
Jiaxun Yang

Re: [PATCH] MIPS: malta: Set load address for 32bit kernel correctly

[Maciej W. Rozycki]

On Mon, 6 Apr 2020, Jiaxun Yang wrote:

> > Given the description above I think it should be done uniformly and 
> >automatically across all platforms by trimming the address supplied
> >with 
> >$(load-y) to low 8 digits in a single place, that is at the place where
> >
> >the variable is consumed.  This will reduce clutter across Makefile 
> >fragments, avoid inconsistencies and extra work to handle individual 
> >platforms as the problem is triggered over and over again, and limit
> >the 
> >risk of mistakes.
> 
> I was intended to do like this but failed to find a proper way.
> 
> Makefile isn't designed for any kind of calculation.
> And shell variables are 64-bit signed so it can't hold such a huge variable.
> 
> Just wish somebody can give me a way to do like:
> 
> ifndef CONFIG_64BIT
> load-y = $(load-y) & 0xffffffff
> endif

 Use the usual shell tools like `sed', `cut', `awk', or whatever we use in 
the kernel build already for other purposes.  There's no need to do any 
actual calculation here to extract the last 8 characters (and the leading 
`0x' prefix).  At worst you can write a small C program, compile it with 
the build system compiler and run, as we already do for some stuff.

  Maciej

Re: [PATCH] MIPS: malta: Set load address for 32bit kernel correctly

[YunQiang Su]

Maciej W. Rozycki <macro@linux-mips.org> 于2020年4月6日周一 上午1:23写道：
>
> On Mon, 6 Apr 2020, Jiaxun Yang wrote:
>
> > > Given the description above I think it should be done uniformly and
> > >automatically across all platforms by trimming the address supplied
> > >with
> > >$(load-y) to low 8 digits in a single place, that is at the place where
> > >
> > >the variable is consumed.  This will reduce clutter across Makefile
> > >fragments, avoid inconsistencies and extra work to handle individual
> > >platforms as the problem is triggered over and over again, and limit
> > >the
> > >risk of mistakes.
> >
> > I was intended to do like this but failed to find a proper way.
> >
> > Makefile isn't designed for any kind of calculation.
> > And shell variables are 64-bit signed so it can't hold such a huge variable.
> >
> > Just wish somebody can give me a way to do like:
> >
> > ifndef CONFIG_64BIT
> > load-y = $(load-y) & 0xffffffff
> > endif
>
>  Use the usual shell tools like `sed', `cut', `awk', or whatever we use in

perl may be the easiest to use tool here.

ifndef CONFIG_64BIT
  load-y := $(shell $(PERL) -e 'print $(load-y) & 0xffffffff')
endif

Note that it is `:=' instead of '='.

> the kernel build already for other purposes.  There's no need to do any
> actual calculation here to extract the last 8 characters (and the leading
> `0x' prefix).  At worst you can write a small C program, compile it with
> the build system compiler and run, as we already do for some stuff.
>
>   Maciej



-- 
YunQiang Su

Re: [PATCH] MIPS: malta: Set load address for 32bit kernel correctly

[Jiaxun Yang]

于 2020年4月6日 GMT+08:00 下午6:57:18, YunQiang Su <wzssyqa@gmail.com> 写到:
>Maciej W. Rozycki <macro@linux-mips.org> 于2020年4月6日周一 上午1:23写道：
>>
>> On Mon, 6 Apr 2020, Jiaxun Yang wrote:
>>
>> > > Given the description above I think it should be done uniformly
>and
>> > >automatically across all platforms by trimming the address
>supplied
>> > >with
>> > >$(load-y) to low 8 digits in a single place, that is at the place
>where
>> > >
>> > >the variable is consumed.  This will reduce clutter across
>Makefile
>> > >fragments, avoid inconsistencies and extra work to handle
>individual
>> > >platforms as the problem is triggered over and over again, and
>limit
>> > >the
>> > >risk of mistakes.
>> >
>> > I was intended to do like this but failed to find a proper way.
>> >
>> > Makefile isn't designed for any kind of calculation.
>> > And shell variables are 64-bit signed so it can't hold such a huge
>variable.
>> >
>> > Just wish somebody can give me a way to do like:
>> >
>> > ifndef CONFIG_64BIT
>> > load-y = $(load-y) & 0xffffffff
>> > endif
>>
>>  Use the usual shell tools like `sed', `cut', `awk', or whatever we
>use in
>
>perl may be the easiest to use tool here.
>
>ifndef CONFIG_64BIT
>  load-y := $(shell $(PERL) -e 'print $(load-y) & 0xffffffff')
>endif
>
>Note that it is `:=' instead of '='.

It seems like perl is not one of kernel's build dependencies.[1]
I'm comsidering a alternative solution,
write a small hostprog in C to deal with that.

Thanks.

[1]: https://www.kernel.org/doc/html/v5.6/process/changes.html


>
>> the kernel build already for other purposes.  There's no need to do
>any
>> actual calculation here to extract the last 8 characters (and the
>leading
>> `0x' prefix).  At worst you can write a small C program, compile it
>with
>> the build system compiler and run, as we already do for some stuff.
>>
>>   Maciej

-- 
Jiaxun Yang

Re: [PATCH] MIPS: malta: Set load address for 32bit kernel correctly

[Fangrui Song]

On 2020-04-06, Jiaxun Yang wrote:
>
>
>于 2020年4月6日 GMT+08:00 下午6:57:18, YunQiang Su <wzssyqa@gmail.com> 写到:
>>Maciej W. Rozycki <macro@linux-mips.org> 于2020年4月6日周一 上午1:23写道：
>>>
>>> On Mon, 6 Apr 2020, Jiaxun Yang wrote:
>>>
>>> > > Given the description above I think it should be done uniformly
>>and
>>> > >automatically across all platforms by trimming the address
>>supplied
>>> > >with
>>> > >$(load-y) to low 8 digits in a single place, that is at the place
>>where
>>> > >
>>> > >the variable is consumed.  This will reduce clutter across
>>Makefile
>>> > >fragments, avoid inconsistencies and extra work to handle
>>individual
>>> > >platforms as the problem is triggered over and over again, and
>>limit
>>> > >the
>>> > >risk of mistakes.
>>> >
>>> > I was intended to do like this but failed to find a proper way.
>>> >
>>> > Makefile isn't designed for any kind of calculation.
>>> > And shell variables are 64-bit signed so it can't hold such a huge
>>variable.
>>> >
>>> > Just wish somebody can give me a way to do like:
>>> >
>>> > ifndef CONFIG_64BIT
>>> > load-y = $(load-y) & 0xffffffff
>>> > endif
>>>
>>>  Use the usual shell tools like `sed', `cut', `awk', or whatever we
>>use in
>>
>>perl may be the easiest to use tool here.
>>
>>ifndef CONFIG_64BIT
>>  load-y := $(shell $(PERL) -e 'print $(load-y) & 0xffffffff')
>>endif
>>
>>Note that it is `:=' instead of '='.
>
>It seems like perl is not one of kernel's build dependencies.[1]
>I'm comsidering a alternative solution,
>write a small hostprog in C to deal with that.
>
>Thanks.
>
>[1]: https://www.kernel.org/doc/html/v5.6/process/changes.html

load-y := 0xffffffff80100000
load-y := 0x$(shell echo "$(load-y)" | rev | head -c 8 | rev)

>>
>>> the kernel build already for other purposes.  There's no need to do
>>any
>>> actual calculation here to extract the last 8 characters (and the
>>leading
>>> `0x' prefix).  At worst you can write a small C program, compile it
>>with
>>> the build system compiler and run, as we already do for some stuff.
>>>
>>>   Maciej
>
>-- 
>Jiaxun Yang

[PATCH v2] MIPS: Truncate load-y into 32bit for 32bit kernel

[Jiaxun Yang]

LLD failed to link vmlinux with 64bit load address for 32bit ELF
while bfd will strip 64bit address into 32bit silently.
To fix LLD build, we should truncate load address provided by platform
into 32bit for 32bit kernel.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Fangrui Song <maskray@google.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

--
V2: Take MaskRay's shell magic.
---
 arch/mips/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index e1c44aed8156..f8fd3c39fb55 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -286,6 +286,9 @@ ifdef CONFIG_64BIT
       $(error CONFIG_CPU_DADDI_WORKAROUNDS unsupported without -msym32)
     endif
   endif
+else
+	# Truncate address into 32-bit
+	load-y := 0x$(shell echo "$(load-y)" | rev | head -c 8 | rev)
 endif
 
 KBUILD_AFLAGS	+= $(cflags-y)
-- 
2.26.0.rc2

Re: [PATCH v2] MIPS: Truncate load-y into 32bit for 32bit kernel

[Nick Desaulniers]

On Tue, Apr 7, 2020 at 1:07 AM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>
> LLD failed to link vmlinux with 64bit load address for 32bit ELF
> while bfd will strip 64bit address into 32bit silently.
> To fix LLD build, we should truncate load address provided by platform
> into 32bit for 32bit kernel.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Reviewed-by: Fangrui Song <maskray@google.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
>
> --
> V2: Take MaskRay's shell magic.

V2 is way too clever, V1 was much more readable.

Can this tag be added to the commit to help us track when and where it lands?
Link: https://github.com/ClangBuiltLinux/linux/issues/786

> ---
>  arch/mips/Makefile | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> index e1c44aed8156..f8fd3c39fb55 100644
> --- a/arch/mips/Makefile
> +++ b/arch/mips/Makefile
> @@ -286,6 +286,9 @@ ifdef CONFIG_64BIT
>        $(error CONFIG_CPU_DADDI_WORKAROUNDS unsupported without -msym32)
>      endif
>    endif
> +else
> +       # Truncate address into 32-bit
> +       load-y := 0x$(shell echo "$(load-y)" | rev | head -c 8 | rev)
>  endif
>
>  KBUILD_AFLAGS  += $(cflags-y)
> --

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2] MIPS: Truncate load-y into 32bit for 32bit kernel

[Fangrui Song]

On 2020-04-07, Nick Desaulniers wrote:
>On Tue, Apr 7, 2020 at 1:07 AM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>>
>> LLD failed to link vmlinux with 64bit load address for 32bit ELF
>> while bfd will strip 64bit address into 32bit silently.
>> To fix LLD build, we should truncate load address provided by platform
>> into 32bit for 32bit kernel.
>>
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> Reviewed-by: Fangrui Song <maskray@google.com>
>> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
>>
>> --
>> V2: Take MaskRay's shell magic.
>
>V2 is way too clever, V1 was much more readable.

This is difficult:/
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_04
The POSIX shell is only required to do signed long integer arithmetic.
"signed long" can be 32-bit.

awk may not provide precision more than a double ("... decimal-floating-constant token as specified
by the ISO C standard")

   % gawk 'BEGIN {printf("%x", (0xffffffff80101234 % 0x100000000))}' /dev/null
   80101000

>Can this tag be added to the commit to help us track when and where it lands?
>Link: https://github.com/ClangBuiltLinux/linux/issues/786

And this tag for GNU ld enhancement:

Link: https://sourceware.org/bugzilla/show_bug.cgi?id=25784

>> ---
>>  arch/mips/Makefile | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/mips/Makefile b/arch/mips/Makefile
>> index e1c44aed8156..f8fd3c39fb55 100644
>> --- a/arch/mips/Makefile
>> +++ b/arch/mips/Makefile
>> @@ -286,6 +286,9 @@ ifdef CONFIG_64BIT
>>        $(error CONFIG_CPU_DADDI_WORKAROUNDS unsupported without -msym32)
>>      endif
>>    endif
>> +else
>> +       # Truncate address into 32-bit
>> +       load-y := 0x$(shell echo "$(load-y)" | rev | head -c 8 | rev)
>>  endif
>>
>>  KBUILD_AFLAGS  += $(cflags-y)
>> --
>
>-- 
>Thanks,
>~Nick Desaulniers

Re: [PATCH v2] MIPS: Truncate load-y into 32bit for 32bit kernel

[Maciej W. Rozycki]

On Tue, 7 Apr 2020, Nick Desaulniers wrote:

> V2 is way too clever, V1 was much more readable.

 I think V2 is a step in the right direction except it still has some 
issues, and also I'd simplify it as there's surely too much processing 
there.

 OTOH V1 is going to be a maintenance nightmare, as you need to handle all 
platforms individually whether they want different 32-bit and 64-bit load 
addresses or not.

> > diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> > index e1c44aed8156..f8fd3c39fb55 100644
> > --- a/arch/mips/Makefile
> > +++ b/arch/mips/Makefile
> > @@ -286,6 +286,9 @@ ifdef CONFIG_64BIT
> >        $(error CONFIG_CPU_DADDI_WORKAROUNDS unsupported without -msym32)
> >      endif
> >    endif
> > +else
> > +       # Truncate address into 32-bit
> > +       load-y := 0x$(shell echo "$(load-y)" | rev | head -c 8 | rev)

 You cannot just truncate `load-y' in place like this as it will break 
logic with `expr' used elsewhere in this Makefile (your original change 
would too) that does a string comparison on this variable.  So you need to 
define another variable for the sole linker's use, like `load-ld'.

 Then I think there's no need to invoke multiple programs, especially ones 
we don't currently rely on (`rev').  How about:

	load-ld = $(shell echo "$(load-y)" | sed 's/.\{8\}\(.\{8\}\)$/\1/')

 Also this really needs to be placed elsewhere, as it has nothing to do 
with KBUILD_SYM32 it has been attached to with this change, and explain 
why it is done rather than what (it's obvious from the command it's meant 
to truncate the address).

 So use something along the lines of:

# 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 = $(shell echo "$(load-y)" | sed 's/.\{8\}\(.\{8\}\)$/\1/')
endif

just above the use place, and then adjust the place to refer `load-ld' 
rather than `load-y'.

 Put the justification for this change (feel free to reuse observations I 
made here), like why a new variable, in the change description.

  Maciej

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

[Jiaxun Yang]

LLD failed to link vmlinux with 64bit load address for 32bit ELF
while bfd will strip 64bit address into 32bit silently.
To fix LLD build, we should truncate load address provided by platform
into 32bit for 32bit kernel.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Fangrui Song <maskray@google.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

--
V2: Take MaskRay's shell magic.

V3: After spent an hour on dealing with special character issue in
Makefile, I gave up to do shell hacks and write a util in C instead.
Thanks Maciej for pointing out Makefile variable problem.
---
 arch/mips/Makefile             |  2 ++
 arch/mips/kernel/Makefile      | 11 ++++++++++-
 arch/mips/kernel/vmlinux.lds.S |  2 +-
 arch/mips/tools/.gitignore     |  1 +
 arch/mips/tools/Makefile       |  5 +++++
 arch/mips/tools/truncate32.c   | 29 +++++++++++++++++++++++++++++
 6 files changed, 48 insertions(+), 2 deletions(-)
 create mode 100644 arch/mips/tools/truncate32.c

diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index e1c44aed8156..633e9de4d262 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -14,6 +14,7 @@
 
 archscripts: scripts_basic
 	$(Q)$(MAKE) $(build)=arch/mips/tools elf-entry
+	$(Q)$(MAKE) $(build)=arch/mips/tools truncate32
 ifeq ($(CONFIG_CPU_LOONGSON3_WORKAROUNDS),y)
 	$(Q)$(MAKE) $(build)=arch/mips/tools loongson3-llsc-check
 endif
@@ -261,6 +262,7 @@ include arch/mips/Kbuild.platforms
 ifdef CONFIG_PHYSICAL_START
 load-y					= $(CONFIG_PHYSICAL_START)
 endif
+export VMLINUX_LOAD_ADDRESS		:= $(load-y)
 
 entry-y				= $(shell $(objtree)/arch/mips/tools/elf-entry vmlinux)
 cflags-y			+= -I$(srctree)/arch/mips/include/asm/mach-generic
diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile
index d6e97df51cfb..0178f7085317 100644
--- a/arch/mips/kernel/Makefile
+++ b/arch/mips/kernel/Makefile
@@ -112,4 +112,13 @@ obj-$(CONFIG_MIPS_CPC)		+= mips-cpc.o
 obj-$(CONFIG_CPU_PM)		+= pm.o
 obj-$(CONFIG_MIPS_CPS_PM)	+= pm-cps.o
 
-CPPFLAGS_vmlinux.lds		:= $(KBUILD_CFLAGS)
+# 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			= $(VMLINUX_LOAD_ADDRESS)
+else
+	load-ld			= $(shell $(objtree)/arch/mips/tools/truncate32 $(VMLINUX_LOAD_ADDRESS))
+endif
+CPPFLAGS_vmlinux.lds		:= $(KBUILD_CFLAGS) -DVMLINUX_LINK_ADDRESS=$(load-ld)
diff --git a/arch/mips/kernel/vmlinux.lds.S b/arch/mips/kernel/vmlinux.lds.S
index a5f00ec73ea6..5226cd8e4bee 100644
--- a/arch/mips/kernel/vmlinux.lds.S
+++ b/arch/mips/kernel/vmlinux.lds.S
@@ -55,7 +55,7 @@ SECTIONS
 	/* . = 0xa800000000300000; */
 	. = 0xffffffff80300000;
 #endif
-	. = VMLINUX_LOAD_ADDRESS;
+	. = VMLINUX_LINK_ADDRESS;
 	/* read-only */
 	_text = .;	/* Text and read-only data */
 	.text : {
diff --git a/arch/mips/tools/.gitignore b/arch/mips/tools/.gitignore
index 794817dfb389..58ead412c8d3 100644
--- a/arch/mips/tools/.gitignore
+++ b/arch/mips/tools/.gitignore
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 elf-entry
 loongson3-llsc-check
+truncate32
diff --git a/arch/mips/tools/Makefile b/arch/mips/tools/Makefile
index b851e5dcc65a..69debb18bbb4 100644
--- a/arch/mips/tools/Makefile
+++ b/arch/mips/tools/Makefile
@@ -8,3 +8,8 @@ hostprogs += loongson3-llsc-check
 PHONY += loongson3-llsc-check
 loongson3-llsc-check: $(obj)/loongson3-llsc-check
 	@:
+
+hostprogs += truncate32
+PHONY += truncate32
+truncate32: $(obj)/truncate32
+	@:
diff --git a/arch/mips/tools/truncate32.c b/arch/mips/tools/truncate32.c
new file mode 100644
index 000000000000..82c19b4c32da
--- /dev/null
+++ b/arch/mips/tools/truncate32.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+__attribute__((noreturn))
+static void die(const char *msg)
+{
+	fputs(msg, stderr);
+	exit(EXIT_FAILURE);
+}
+
+int main(int argc, const char *argv[])
+{
+	unsigned long long val;
+
+	if (argc != 2)
+		die("Usage: truncate32 <address>\n");
+
+	val = strtoull(argv[1], NULL, 0);
+
+	if ((val & 0xffffffff00000000) != 0xffffffff00000000)
+		die("Invalid input\n");
+
+	val = val & 0xffffffff;
+	printf("0x%08llx\n", val);
+
+	return EXIT_SUCCESS;
+}
-- 
2.26.0.rc2

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

[Kees Cook]

On Fri, Apr 10, 2020 at 05:06:23PM +0800, Jiaxun Yang wrote:
> LLD failed to link vmlinux with 64bit load address for 32bit ELF
> while bfd will strip 64bit address into 32bit silently.
> To fix LLD build, we should truncate load address provided by platform
> into 32bit for 32bit kernel.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Reviewed-by: Fangrui Song <maskray@google.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> 
> --
> V2: Take MaskRay's shell magic.
> 
> V3: After spent an hour on dealing with special character issue in
> Makefile, I gave up to do shell hacks and write a util in C instead.
> Thanks Maciej for pointing out Makefile variable problem.
> ---
>  arch/mips/Makefile             |  2 ++
>  arch/mips/kernel/Makefile      | 11 ++++++++++-
>  arch/mips/kernel/vmlinux.lds.S |  2 +-
>  arch/mips/tools/.gitignore     |  1 +
>  arch/mips/tools/Makefile       |  5 +++++
>  arch/mips/tools/truncate32.c   | 29 +++++++++++++++++++++++++++++
>  6 files changed, 48 insertions(+), 2 deletions(-)
>  create mode 100644 arch/mips/tools/truncate32.c
> 
> diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> index e1c44aed8156..633e9de4d262 100644
> --- a/arch/mips/Makefile
> +++ b/arch/mips/Makefile
> @@ -14,6 +14,7 @@
>  
>  archscripts: scripts_basic
>  	$(Q)$(MAKE) $(build)=arch/mips/tools elf-entry
> +	$(Q)$(MAKE) $(build)=arch/mips/tools truncate32
>  ifeq ($(CONFIG_CPU_LOONGSON3_WORKAROUNDS),y)
>  	$(Q)$(MAKE) $(build)=arch/mips/tools loongson3-llsc-check
>  endif
> @@ -261,6 +262,7 @@ include arch/mips/Kbuild.platforms
>  ifdef CONFIG_PHYSICAL_START
>  load-y					= $(CONFIG_PHYSICAL_START)
>  endif
> +export VMLINUX_LOAD_ADDRESS		:= $(load-y)
>  
>  entry-y				= $(shell $(objtree)/arch/mips/tools/elf-entry vmlinux)
>  cflags-y			+= -I$(srctree)/arch/mips/include/asm/mach-generic
> diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile
> index d6e97df51cfb..0178f7085317 100644
> --- a/arch/mips/kernel/Makefile
> +++ b/arch/mips/kernel/Makefile
> @@ -112,4 +112,13 @@ obj-$(CONFIG_MIPS_CPC)		+= mips-cpc.o
>  obj-$(CONFIG_CPU_PM)		+= pm.o
>  obj-$(CONFIG_MIPS_CPS_PM)	+= pm-cps.o
>  
> -CPPFLAGS_vmlinux.lds		:= $(KBUILD_CFLAGS)
> +# 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			= $(VMLINUX_LOAD_ADDRESS)
> +else
> +	load-ld			= $(shell $(objtree)/arch/mips/tools/truncate32 $(VMLINUX_LOAD_ADDRESS))

This is major overkill. Just use the Makefile's builtin text
manipulation:

load-ld = $(subst 0xffffffff,0x,$(VMLINUX_LOAD_ADDRESS))

And note that a shell failure here would be entirely ignored, so the use
of die() in the proposed helper wouldn't actually stop a build, etc.

> +endif
> +CPPFLAGS_vmlinux.lds		:= $(KBUILD_CFLAGS) -DVMLINUX_LINK_ADDRESS=$(load-ld)
> diff --git a/arch/mips/kernel/vmlinux.lds.S b/arch/mips/kernel/vmlinux.lds.S
> index a5f00ec73ea6..5226cd8e4bee 100644
> --- a/arch/mips/kernel/vmlinux.lds.S
> +++ b/arch/mips/kernel/vmlinux.lds.S
> @@ -55,7 +55,7 @@ SECTIONS
>  	/* . = 0xa800000000300000; */
>  	. = 0xffffffff80300000;
>  #endif
> -	. = VMLINUX_LOAD_ADDRESS;
> +	. = VMLINUX_LINK_ADDRESS;
>  	/* read-only */
>  	_text = .;	/* Text and read-only data */
>  	.text : {
> diff --git a/arch/mips/tools/.gitignore b/arch/mips/tools/.gitignore
> index 794817dfb389..58ead412c8d3 100644
> --- a/arch/mips/tools/.gitignore
> +++ b/arch/mips/tools/.gitignore
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  elf-entry
>  loongson3-llsc-check
> +truncate32
> diff --git a/arch/mips/tools/Makefile b/arch/mips/tools/Makefile
> index b851e5dcc65a..69debb18bbb4 100644
> --- a/arch/mips/tools/Makefile
> +++ b/arch/mips/tools/Makefile
> @@ -8,3 +8,8 @@ hostprogs += loongson3-llsc-check
>  PHONY += loongson3-llsc-check
>  loongson3-llsc-check: $(obj)/loongson3-llsc-check
>  	@:
> +
> +hostprogs += truncate32
> +PHONY += truncate32

Isn't the special variable named ".PHONY"? (And also is that only for
things the don't get written to disk, but truncate32 is...)

> +truncate32: $(obj)/truncate32
> +	@:
> diff --git a/arch/mips/tools/truncate32.c b/arch/mips/tools/truncate32.c
> new file mode 100644
> index 000000000000..82c19b4c32da
> --- /dev/null
> +++ b/arch/mips/tools/truncate32.c
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +__attribute__((noreturn))
> +static void die(const char *msg)
> +{
> +	fputs(msg, stderr);
> +	exit(EXIT_FAILURE);
> +}
> +
> +int main(int argc, const char *argv[])
> +{
> +	unsigned long long val;
> +
> +	if (argc != 2)
> +		die("Usage: truncate32 <address>\n");
> +
> +	val = strtoull(argv[1], NULL, 0);
> +
> +	if ((val & 0xffffffff00000000) != 0xffffffff00000000)
> +		die("Invalid input\n");
> +
> +	val = val & 0xffffffff;
> +	printf("0x%08llx\n", val);
> +
> +	return EXIT_SUCCESS;
> +}
> -- 
> 2.26.0.rc2
> 

-- 
Kees Cook

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

[Maciej W. Rozycki]

On Fri, 10 Apr 2020, Kees Cook wrote:

> > diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile
> > index d6e97df51cfb..0178f7085317 100644
> > --- a/arch/mips/kernel/Makefile
> > +++ b/arch/mips/kernel/Makefile
> > @@ -112,4 +112,13 @@ obj-$(CONFIG_MIPS_CPC)		+= mips-cpc.o
> >  obj-$(CONFIG_CPU_PM)		+= pm.o
> >  obj-$(CONFIG_MIPS_CPS_PM)	+= pm-cps.o
> >  
> > -CPPFLAGS_vmlinux.lds		:= $(KBUILD_CFLAGS)
> > +# 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			= $(VMLINUX_LOAD_ADDRESS)
> > +else
> > +	load-ld			= $(shell $(objtree)/arch/mips/tools/truncate32 $(VMLINUX_LOAD_ADDRESS))
> 
> This is major overkill. Just use the Makefile's builtin text
> manipulation:
> 
> load-ld = $(subst 0xffffffff,0x,$(VMLINUX_LOAD_ADDRESS))

 This looks like the best approach to me, thank you for the hint!  And we 
only ever want to strip 0xffffffff anyway.  (I forgot about this function 
of `make', doh!)

  Maciej