* [PATCH] MIPS: malta: Set load address for 32bit kernel correctly @ 2020-04-05 8:24 Jiaxun Yang 2020-04-05 16:47 ` Maciej W. Rozycki 2020-04-07 8:06 ` [PATCH v2] MIPS: Truncate load-y into 32bit for 32bit kernel Jiaxun Yang 0 siblings, 2 replies; 14+ messages in thread From: Jiaxun Yang @ 2020-04-05 8:24 UTC (permalink / raw) To: linux-mips Cc: Jiaxun Yang, Fangrui Song, Nathan Chancellor, Thomas Bogendoerfer, linux-kernel 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] MIPS: malta: Set load address for 32bit kernel correctly 2020-04-05 8:24 [PATCH] MIPS: malta: Set load address for 32bit kernel correctly Jiaxun Yang @ 2020-04-05 16:47 ` Maciej W. Rozycki 2020-04-05 16:53 ` Jiaxun Yang 2020-04-07 8:06 ` [PATCH v2] MIPS: Truncate load-y into 32bit for 32bit kernel Jiaxun Yang 1 sibling, 1 reply; 14+ messages in thread From: Maciej W. Rozycki @ 2020-04-05 16:47 UTC (permalink / raw) To: Jiaxun Yang Cc: linux-mips, Fangrui Song, Nathan Chancellor, Thomas Bogendoerfer, linux-kernel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] MIPS: malta: Set load address for 32bit kernel correctly 2020-04-05 16:47 ` Maciej W. Rozycki @ 2020-04-05 16:53 ` Jiaxun Yang 2020-04-05 17:23 ` Maciej W. Rozycki 0 siblings, 1 reply; 14+ messages in thread From: Jiaxun Yang @ 2020-04-05 16:53 UTC (permalink / raw) To: Maciej W. Rozycki Cc: linux-mips, Fangrui Song, Nathan Chancellor, Thomas Bogendoerfer, linux-kernel 于 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] MIPS: malta: Set load address for 32bit kernel correctly 2020-04-05 16:53 ` Jiaxun Yang @ 2020-04-05 17:23 ` Maciej W. Rozycki 2020-04-06 10:57 ` YunQiang Su 0 siblings, 1 reply; 14+ messages in thread From: Maciej W. Rozycki @ 2020-04-05 17:23 UTC (permalink / raw) To: Jiaxun Yang Cc: linux-mips, Fangrui Song, Nathan Chancellor, Thomas Bogendoerfer, linux-kernel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] MIPS: malta: Set load address for 32bit kernel correctly 2020-04-05 17:23 ` Maciej W. Rozycki @ 2020-04-06 10:57 ` YunQiang Su 2020-04-06 11:10 ` Jiaxun Yang 0 siblings, 1 reply; 14+ messages in thread From: YunQiang Su @ 2020-04-06 10:57 UTC (permalink / raw) To: Maciej W. Rozycki Cc: Jiaxun Yang, linux-mips, Fangrui Song, Nathan Chancellor, Thomas Bogendoerfer, LKML 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] MIPS: malta: Set load address for 32bit kernel correctly 2020-04-06 10:57 ` YunQiang Su @ 2020-04-06 11:10 ` Jiaxun Yang 2020-04-06 16:43 ` Fangrui Song 0 siblings, 1 reply; 14+ messages in thread From: Jiaxun Yang @ 2020-04-06 11:10 UTC (permalink / raw) To: YunQiang Su, Maciej W. Rozycki Cc: linux-mips, Fangrui Song, Nathan Chancellor, Thomas Bogendoerfer, LKML 于 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] MIPS: malta: Set load address for 32bit kernel correctly 2020-04-06 11:10 ` Jiaxun Yang @ 2020-04-06 16:43 ` Fangrui Song 0 siblings, 0 replies; 14+ messages in thread From: Fangrui Song @ 2020-04-06 16:43 UTC (permalink / raw) To: Jiaxun Yang Cc: YunQiang Su, Maciej W. Rozycki, linux-mips, Nathan Chancellor, Thomas Bogendoerfer, LKML 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] MIPS: Truncate load-y into 32bit for 32bit kernel 2020-04-05 8:24 [PATCH] MIPS: malta: Set load address for 32bit kernel correctly Jiaxun Yang 2020-04-05 16:47 ` Maciej W. Rozycki @ 2020-04-07 8:06 ` Jiaxun Yang 2020-04-07 17:21 ` Nick Desaulniers 2020-04-10 9:06 ` [PATCH v3] MIPS: Truncate link address " Jiaxun Yang 1 sibling, 2 replies; 14+ messages in thread From: Jiaxun Yang @ 2020-04-07 8:06 UTC (permalink / raw) To: linux-mips Cc: Jiaxun Yang, clang-built-linux, Fangrui Song, Nathan Chancellor, Thomas Bogendoerfer, linux-kernel 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] MIPS: Truncate load-y into 32bit for 32bit kernel 2020-04-07 8:06 ` [PATCH v2] MIPS: Truncate load-y into 32bit for 32bit kernel Jiaxun Yang @ 2020-04-07 17:21 ` Nick Desaulniers 2020-04-07 18:00 ` Fangrui Song 2020-04-07 18:10 ` Maciej W. Rozycki 2020-04-10 9:06 ` [PATCH v3] MIPS: Truncate link address " Jiaxun Yang 1 sibling, 2 replies; 14+ messages in thread From: Nick Desaulniers @ 2020-04-07 17:21 UTC (permalink / raw) To: Jiaxun Yang Cc: linux-mips, clang-built-linux, Fangrui Song, Nathan Chancellor, Thomas Bogendoerfer, LKML 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] MIPS: Truncate load-y into 32bit for 32bit kernel 2020-04-07 17:21 ` Nick Desaulniers @ 2020-04-07 18:00 ` Fangrui Song 2020-04-07 18:10 ` Maciej W. Rozycki 1 sibling, 0 replies; 14+ messages in thread From: Fangrui Song @ 2020-04-07 18:00 UTC (permalink / raw) To: Nick Desaulniers Cc: Jiaxun Yang, linux-mips, clang-built-linux, Nathan Chancellor, Thomas Bogendoerfer, LKML 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] MIPS: Truncate load-y into 32bit for 32bit kernel 2020-04-07 17:21 ` Nick Desaulniers 2020-04-07 18:00 ` Fangrui Song @ 2020-04-07 18:10 ` Maciej W. Rozycki 1 sibling, 0 replies; 14+ messages in thread From: Maciej W. Rozycki @ 2020-04-07 18:10 UTC (permalink / raw) To: Nick Desaulniers Cc: Jiaxun Yang, linux-mips, clang-built-linux, Fangrui Song, Nathan Chancellor, Thomas Bogendoerfer, LKML 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3] MIPS: Truncate link address into 32bit for 32bit kernel 2020-04-07 8:06 ` [PATCH v2] MIPS: Truncate load-y into 32bit for 32bit kernel Jiaxun Yang 2020-04-07 17:21 ` Nick Desaulniers @ 2020-04-10 9:06 ` Jiaxun Yang 2020-04-10 20:45 ` Kees Cook 1 sibling, 1 reply; 14+ messages in thread From: Jiaxun Yang @ 2020-04-10 9:06 UTC (permalink / raw) To: linux-mips Cc: macro, Jiaxun Yang, Fangrui Song, Nathan Chancellor, Thomas Bogendoerfer, Paul Burton, Borislav Petkov, Kees Cook, Heiko Carstens, Masahiro Yamada, Greg Kroah-Hartman, linux-kernel, clang-built-linux 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3] MIPS: Truncate link address into 32bit for 32bit kernel 2020-04-10 9:06 ` [PATCH v3] MIPS: Truncate link address " Jiaxun Yang @ 2020-04-10 20:45 ` Kees Cook 2020-04-10 23:40 ` Maciej W. Rozycki 0 siblings, 1 reply; 14+ messages in thread From: Kees Cook @ 2020-04-10 20:45 UTC (permalink / raw) To: Jiaxun Yang Cc: linux-mips, macro, Fangrui Song, Nathan Chancellor, Thomas Bogendoerfer, Paul Burton, Borislav Petkov, Heiko Carstens, Masahiro Yamada, Greg Kroah-Hartman, linux-kernel, clang-built-linux 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] MIPS: Truncate link address into 32bit for 32bit kernel 2020-04-10 20:45 ` Kees Cook @ 2020-04-10 23:40 ` Maciej W. Rozycki 0 siblings, 0 replies; 14+ messages in thread From: Maciej W. Rozycki @ 2020-04-10 23:40 UTC (permalink / raw) To: Kees Cook Cc: Jiaxun Yang, linux-mips, Fangrui Song, Nathan Chancellor, Thomas Bogendoerfer, Paul Burton, Borislav Petkov, Heiko Carstens, Masahiro Yamada, Greg Kroah-Hartman, linux-kernel, clang-built-linux 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-04-10 23:40 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-05 8:24 [PATCH] MIPS: malta: Set load address for 32bit kernel correctly Jiaxun Yang 2020-04-05 16:47 ` Maciej W. Rozycki 2020-04-05 16:53 ` Jiaxun Yang 2020-04-05 17:23 ` Maciej W. Rozycki 2020-04-06 10:57 ` YunQiang Su 2020-04-06 11:10 ` Jiaxun Yang 2020-04-06 16:43 ` Fangrui Song 2020-04-07 8:06 ` [PATCH v2] MIPS: Truncate load-y into 32bit for 32bit kernel Jiaxun Yang 2020-04-07 17:21 ` Nick Desaulniers 2020-04-07 18:00 ` Fangrui Song 2020-04-07 18:10 ` Maciej W. Rozycki 2020-04-10 9:06 ` [PATCH v3] MIPS: Truncate link address " Jiaxun Yang 2020-04-10 20:45 ` Kees Cook 2020-04-10 23:40 ` Maciej W. Rozycki
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.