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

[PATCH v4] 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>
Link: https://github.com/ClangBuiltLinux/linux/issues/786
Link: https://sourceware.org/bugzilla/show_bug.cgi?id=25784
Cc: Fangrui Song <maskray@google.com>
Cc: 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.

v4: Finally we managed to find a Makefile method to do it properly
thanks to Kees. As it's too far from the initial version, I removed
Review & Test tag from Nick and Fangrui and Cc instead.
---
 arch/mips/Makefile             | 12 +++++++++++-
 arch/mips/kernel/vmlinux.lds.S |  2 +-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index e1c44aed8156..18495568f03e 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -288,9 +288,19 @@ 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) \
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 : {
-- 
2.26.0.rc2

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

[Maciej W. Rozycki]

On Mon, 13 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 truncate load address provided by platform
> into 32bit for 32bit kernel.

Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org>

> 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;

 The CONFIG_BOOT_ELF64 cruft right above it looks interesting to me, never 
have ever been used.  We have had the current arrangement since:

commit 923ec3d20eef9e36456868b590873ce39f17fe71
Author: Ralf Baechle <ralf@linux-mips.org>
Date:   Wed Nov 6 22:16:38 2002 +0000

    Define load address in linker script instead of relying on the
    deprecated and notoriously unreliable option -Ttext.

and previously `-Ttext' was used with this script anyway, though not very 
long, as the script was entirely ignored until:

commit 7a782968041ffc4c2d89816238e2f8ea5cceddba
Author: Ralf Baechle <ralf@linux-mips.org>
Date:   Thu Oct 31 23:54:21 2002 +0000

    Merge with Linux 2.5.36.

  Maciej

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

[Jiaxun Yang]

On Mon, 13 Apr 2020 07:59:29 +0100 (BST)
"Maciej W. Rozycki" <macro@linux-mips.org> wrote:

> On Mon, 13 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 truncate load address provided by
> > platform into 32bit for 32bit kernel.  
> 
> Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org>
> 
> > 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;  
> 
>  The CONFIG_BOOT_ELF64 cruft right above it looks interesting to me,
> never have ever been used.  We have had the current arrangement since:

It confused me either.
It's only used by SGI so probably it's time to rename it as
BOOT_SEG_ELF64.

Wish someone could clarify what is it.

Thanks.

> 
> commit 923ec3d20eef9e36456868b590873ce39f17fe71
> Author: Ralf Baechle <ralf@linux-mips.org>
> Date:   Wed Nov 6 22:16:38 2002 +0000
> 
>     Define load address in linker script instead of relying on the
>     deprecated and notoriously unreliable option -Ttext.
> 
> and previously `-Ttext' was used with this script anyway, though not
> very long, as the script was entirely ignored until:
> 
> commit 7a782968041ffc4c2d89816238e2f8ea5cceddba
> Author: Ralf Baechle <ralf@linux-mips.org>
> Date:   Thu Oct 31 23:54:21 2002 +0000
> 
>     Merge with Linux 2.5.36.
> 
>   Maciej

--
Jiaxun Yang

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

[Fangrui Song]

On 2020-04-13, Jiaxun Yang wrote:
>On Mon, 13 Apr 2020 07:59:29 +0100 (BST)
>"Maciej W. Rozycki" <macro@linux-mips.org> wrote:
>
>> On Mon, 13 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 truncate load address provided by
>> > platform into 32bit for 32bit kernel.
>>
>> Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org>
>>
>> > 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;
>>
>>  The CONFIG_BOOT_ELF64 cruft right above it looks interesting to me,
>> never have ever been used.  We have had the current arrangement since:
>
>It confused me either.
>It's only used by SGI so probably it's time to rename it as
>BOOT_SEG_ELF64.
>
>Wish someone could clarify what is it.
>
>Thanks.

Agreed that -Ttext in

arch/mips/boot/compressed/Makefile
100:      cmd_zld = $(LD) $(KBUILD_LDFLAGS) -Ttext $(VMLINUZ_LOAD_ADDRESS) -T $< $(vmlinuzobjs-y) -o $@

and a few other places are brittle. They need to be replaced with Output Section Address:
(https://sourceware.org/binutils/docs/ld/Output-Section-Address.html
https://github.com/llvm/llvm-project/blob/master/lld/docs/ELF/linker_script.rst#output-section-address)

-Ttext changes the address of .text . This can lead to the change of the
address of the text segment (RX), but this is not guaranteed (many
sections can be placed before .text and they are not affected).
See https://reviews.llvm.org/D70468 "[ELF] Error if -Ttext-segment is specified"


Reviewed-by: Fangrui Song <maskray@google.com>

>>
>> commit 923ec3d20eef9e36456868b590873ce39f17fe71
>> Author: Ralf Baechle <ralf@linux-mips.org>
>> Date:   Wed Nov 6 22:16:38 2002 +0000
>>
>>     Define load address in linker script instead of relying on the
>>     deprecated and notoriously unreliable option -Ttext.
>>
>> and previously `-Ttext' was used with this script anyway, though not
>> very long, as the script was entirely ignored until:
>>
>> commit 7a782968041ffc4c2d89816238e2f8ea5cceddba
>> Author: Ralf Baechle <ralf@linux-mips.org>
>> Date:   Thu Oct 31 23:54:21 2002 +0000
>>
>>     Merge with Linux 2.5.36.
>>
>>   Maciej
>
>--
>Jiaxun Yang

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

[Kees Cook]

On Mon, Apr 13, 2020 at 02:26:49PM +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: Kees Cook <keescook@chromium.org>

-Kees

> Link: https://github.com/ClangBuiltLinux/linux/issues/786
> Link: https://sourceware.org/bugzilla/show_bug.cgi?id=25784
> Cc: Fangrui Song <maskray@google.com>
> Cc: 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.
> 
> v4: Finally we managed to find a Makefile method to do it properly
> thanks to Kees. As it's too far from the initial version, I removed
> Review & Test tag from Nick and Fangrui and Cc instead.
> ---
>  arch/mips/Makefile             | 12 +++++++++++-
>  arch/mips/kernel/vmlinux.lds.S |  2 +-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> index e1c44aed8156..18495568f03e 100644
> --- a/arch/mips/Makefile
> +++ b/arch/mips/Makefile
> @@ -288,9 +288,19 @@ 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) \
> 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 : {
> -- 
> 2.26.0.rc2
> 

-- 
Kees Cook

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

[Nathan Chancellor]

Hi Jiaxun,

On Mon, Apr 13, 2020 at 02:26:49PM +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>
> Link: https://github.com/ClangBuiltLinux/linux/issues/786
> Link: https://sourceware.org/bugzilla/show_bug.cgi?id=25784
> Cc: Fangrui Song <maskray@google.com>
> Cc: 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.
> 
> v4: Finally we managed to find a Makefile method to do it properly
> thanks to Kees. As it's too far from the initial version, I removed
> Review & Test tag from Nick and Fangrui and Cc instead.
> ---
>  arch/mips/Makefile             | 12 +++++++++++-
>  arch/mips/kernel/vmlinux.lds.S |  2 +-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> index e1c44aed8156..18495568f03e 100644
> --- a/arch/mips/Makefile
> +++ b/arch/mips/Makefile
> @@ -288,9 +288,19 @@ 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) \
> 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 : {
> -- 
> 2.26.0.rc2
> 

This version does not quite work:

ld.lld: error: section .text at 0xFFFFFFFF80990000 of size 0x388C exceeds available address space
ld.lld: error: section .MIPS.abiflags at 0xFFFFFFFF80993890 of size 0x18 exceeds available address space
ld.lld: error: section .rodata.str1.1 at 0xFFFFFFFF809938A8 of size 0x164 exceeds available address space
ld.lld: error: section .eh_frame at 0xFFFFFFFF80993A0C of size 0x2C exceeds available address space
ld.lld: error: section .data at 0xFFFFFFFF80993A40 of size 0x38EFD0 exceeds available address space
ld.lld: error: section .got at 0xFFFFFFFF80D22A10 of size 0x8 exceeds available address space
ld.lld: error: section .bss at 0xFFFFFFFF80E22A20 of size 0x402010 exceeds available address space
ld.lld: error: section .sbss at 0xFFFFFFFF81224A30 of size 0x8 exceeds available address space
make[2]: *** [/home/nathan/src/linux/arch/mips/boot/compressed/Makefile:104: vmlinuz] Error 1

I think something like this is needed but I do not know if it is proper.

Cheers,
Nathan

diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index 18495568f03e..68c0f22fefc0 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -304,6 +304,7 @@ KBUILD_CPPFLAGS += -DVMLINUX_LOAD_ADDRESS=$(load-y) -DVMLINUX_LINK_ADDRESS=$(loa
 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)"
diff --git a/arch/mips/boot/compressed/Makefile b/arch/mips/boot/compressed/Makefile
index 0df0ee8a298d..3d391256ab7e 100644
--- a/arch/mips/boot/compressed/Makefile
+++ b/arch/mips/boot/compressed/Makefile
@@ -90,7 +90,7 @@ ifneq ($(zload-y),)
 VMLINUZ_LOAD_ADDRESS := $(zload-y)
 else
 VMLINUZ_LOAD_ADDRESS = $(shell $(obj)/calc_vmlinuz_load_addr \
-		$(obj)/vmlinux.bin $(VMLINUX_LOAD_ADDRESS))
+		$(obj)/vmlinux.bin $(VMLINUX_LINK_ADDRESS))
 endif
 UIMAGE_LOADADDR = $(VMLINUZ_LOAD_ADDRESS)
 

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

[Maciej W. Rozycki]

On Mon, 13 Apr 2020, Jiaxun Yang wrote:

> > > 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;  
> > 
> >  The CONFIG_BOOT_ELF64 cruft right above it looks interesting to me,
> > never have ever been used.  We have had the current arrangement since:
> 
> It confused me either.
> It's only used by SGI so probably it's time to rename it as
> BOOT_SEG_ELF64.

 Well, as seen above the first assignment is immediately overridden by the 
second and the rest of the conditional are comments.  So as I say it's not 
used ever, not even any SGI configuration, and from Git history it looks 
like it has never been.

  Maciej

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

[Maciej W. Rozycki]

On Mon, 13 Apr 2020, Fangrui Song wrote:

> >> > 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;
> >>
> >> The CONFIG_BOOT_ELF64 cruft right above it looks interesting to me,
> >> never have ever been used.  We have had the current arrangement since:
> >
> >It confused me either.
> >It's only used by SGI so probably it's time to rename it as
> >BOOT_SEG_ELF64.
> >
> >Wish someone could clarify what is it.
> >
> >Thanks.
> 
> Agreed that -Ttext in
> 
> arch/mips/boot/compressed/Makefile
> 100:      cmd_zld = $(LD) $(KBUILD_LDFLAGS) -Ttext $(VMLINUZ_LOAD_ADDRESS) -T
> $< $(vmlinuzobjs-y) -o $@
> 
> and a few other places are brittle. They need to be replaced with Output
> Section Address:
> (https://sourceware.org/binutils/docs/ld/Output-Section-Address.html
> https://github.com/llvm/llvm-project/blob/master/lld/docs/ELF/linker_script.rst#output-section-address)
> 
> -Ttext changes the address of .text . This can lead to the change of the
> address of the text segment (RX), but this is not guaranteed (many
> sections can be placed before .text and they are not affected).

 That is unrelated, but you're free to clean it up of course.

  Maciej

[PATCH v5] 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>
Link: https://github.com/ClangBuiltLinux/linux/issues/786
Link: https://sourceware.org/bugzilla/show_bug.cgi?id=25784
Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org>
Reviewed-by: Fangrui Song <maskray@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Cc: 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.

v4: Finally we managed to find a Makefile method to do it properly
thanks to Kees. As it's too far from the initial version, I removed
Review & Test tag from Nick and Fangrui and Cc instead.

v5: Care vmlinuz as well.
---
 arch/mips/Makefile                 | 13 ++++++++++++-
 arch/mips/boot/compressed/Makefile |  2 +-
 arch/mips/kernel/vmlinux.lds.S     |  2 +-
 3 files changed, 14 insertions(+), 3 deletions(-)

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)"
diff --git a/arch/mips/boot/compressed/Makefile b/arch/mips/boot/compressed/Makefile
index 0df0ee8a298d..3d391256ab7e 100644
--- a/arch/mips/boot/compressed/Makefile
+++ b/arch/mips/boot/compressed/Makefile
@@ -90,7 +90,7 @@ ifneq ($(zload-y),)
 VMLINUZ_LOAD_ADDRESS := $(zload-y)
 else
 VMLINUZ_LOAD_ADDRESS = $(shell $(obj)/calc_vmlinuz_load_addr \
-		$(obj)/vmlinux.bin $(VMLINUX_LOAD_ADDRESS))
+		$(obj)/vmlinux.bin $(VMLINUX_LINK_ADDRESS))
 endif
 UIMAGE_LOADADDR = $(VMLINUZ_LOAD_ADDRESS)
 
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 : {
-- 
2.26.0.rc2

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

[Nathan Chancellor]

On Wed, Apr 22, 2020 at 10:32:54PM +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>
> Link: https://github.com/ClangBuiltLinux/linux/issues/786
> Link: https://sourceware.org/bugzilla/show_bug.cgi?id=25784
> Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org>
> Reviewed-by: Fangrui Song <maskray@google.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Cc: 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.
> 
> v4: Finally we managed to find a Makefile method to do it properly
> thanks to Kees. As it's too far from the initial version, I removed
> Review & Test tag from Nick and Fangrui and Cc instead.
> 
> v5: Care vmlinuz as well.

Tested-by: Nathan Chancellor <natechancellor@gmail.com>

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@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.

  Maciej

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

[Jiaxun Yang]

于 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

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

[Maciej W. Rozycki]

On Thu, 23 Apr 2020, Jiaxun Yang wrote:

> > 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.

 This only affects CONFIG_32BIT kernels, so SYM32 does not apply; I can't 
comment on KVM.  There's still that bunch of:

$(shell expr $(...) \< 0xffffffff80000000)

constructs I mentioned before, so let's leave your change as it stands at 
this time.  Please do rename the variable as I suggested though, I hope 
that's not a big deal.

  Maciej

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

[Thomas Bogendoerfer]

On Fri, Apr 24, 2020 at 01:22:30PM +0100, Maciej W. Rozycki wrote:
> On Thu, 23 Apr 2020, Jiaxun Yang wrote:
> 
> > > 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.
> 
>  This only affects CONFIG_32BIT kernels, so SYM32 does not apply; I can't 
> comment on KVM.  There's still that bunch of:
> 
> $(shell expr $(...) \< 0xffffffff80000000)
> 
> constructs I mentioned before, so let's leave your change as it stands at 
> this time.  Please do rename the variable as I suggested though, I hope 
> that's not a big deal.

Jiaxun, are you going to send an update with this change ?

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

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

[Jiaxun Yang]

于 2020年5月4日 GMT+08:00 下午11:46:13, Thomas Bogendoerfer <tsbogend@alpha.franken.de> 写到:
>On Fri, Apr 24, 2020 at 01:22:30PM +0100, Maciej W. Rozycki wrote:
>> On Thu, 23 Apr 2020, Jiaxun Yang wrote:
>> 
>> > > 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.
>> 
>>  This only affects CONFIG_32BIT kernels, so SYM32 does not apply; I can't 
>> comment on KVM.  There's still that bunch of:
>> 
>> $(shell expr $(...) \< 0xffffffff80000000)
>> 
>> constructs I mentioned before, so let's leave your change as it stands at 
>> this time.  Please do rename the variable as I suggested though, I hope 
>> that's not a big deal.
>
>Jiaxun, are you going to send an update with this change ?

Sorry my mail server missed Maciej's reply.

Should I send another version or you just fix it at apply time?

>
>Thomas.
>

-- 
Jiaxun Yang

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

[Thomas Bogendoerfer]

On Tue, May 05, 2020 at 12:09:46AM +0800, Jiaxun Yang wrote:
> 
> 于 2020年5月4日 GMT+08:00 下午11:46:13, Thomas Bogendoerfer <tsbogend@alpha.franken.de> 写到:
> >On Fri, Apr 24, 2020 at 01:22:30PM +0100, Maciej W. Rozycki wrote:
> >> On Thu, 23 Apr 2020, Jiaxun Yang wrote:
> >> 
> >> > > 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.
> >> 
> >>  This only affects CONFIG_32BIT kernels, so SYM32 does not apply; I can't 
> >> comment on KVM.  There's still that bunch of:
> >> 
> >> $(shell expr $(...) \< 0xffffffff80000000)
> >> 
> >> constructs I mentioned before, so let's leave your change as it stands at 
> >> this time.  Please do rename the variable as I suggested though, I hope 
> >> that's not a big deal.
> >
> >Jiaxun, are you going to send an update with this change ?
> 
> Sorry my mail server missed Maciej's reply.
> 
> Should I send another version or you just fix it at apply time?

please send a new version, thank you.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]