Hi James, On 29 August 2017 at 21:31, James Greenhalgh <james.greenha...@arm.com> wrote: > On Tue, Jun 27, 2017 at 11:20:02AM +1000, Kugan Vivekanandarajah wrote: >> https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00614.html added this >> workaround to get kernel building with when TARGET_FIX_ERR_A53_843419 >> is enabled. >> >> This was added to support building kernel loadable modules. In kernel, >> when CONFIG_ARM64_ERRATUM_843419 is selected, the relocation needed >> for ADRP/LDR (R_AARCH64_ADR_PREL_PG_HI21 and >> R_AARCH64_ADR_PREL_PG_HI21_NC are removed from the kernel to avoid >> loading objects with possibly offending sequence). Thus, it could only >> support pc relative literal loads. >> >> However, the following patch was posted to kernel to add >> -mpc-relative-literal-loads >> http://www.spinics.net/lists/arm-kernel/msg476149.html >> >> -mpc-relative-literal-loads is unconditionally added to the kernel >> build as can be seen from: >> https://github.com/torvalds/linux/blob/master/arch/arm64/Makefile >> >> Therefore this patch removes the hunk so that applications like >> SPECcpu2017's 521/621.wrf can be built (with LTO in this case) without >> -mno-pc-relative-literal-loads >> >> Bootstrapped and regression tested on aarch64-linux-gnu with no new >> regressions. >> >> Is this OK for trunk? > > Hi Kugan, > > I'm struggling a little to convince myself that this is correct. I think > the argument is that this was a workaround for one very particular issue > with the linux kernel module loader, which hard faults by refusing to > handle these relocations when in a workaround mode for Erratum 843419.
Yes. > Most of these relocations won't occur because the kernel builds with > -mcmodel=large, but literals would always use a small model style > adrp/load, unless we were using -mpc-relative-literal-loads . So, this > workaround enabled -mpc-relative-literal-loads always when we were in > a workaround mode, thus allowing the kernel loader to continue. > > The argument for removing it then, is that with the kernel now always using > -mpc-relative-literal-loads there is no reason for this workaround to stay > in place. The linkers which we will use will apply the workaround if needed. Yes. > Testcases and a detailed problem report of the build failures you had seen in > 521.wrf would have helped me get closer to understanding this, and made > review substantially easier. Sorry for not being clear with this. Unfortunately 521.wrf was showing this error with LTO so I could not reproduce with a small enough test case. > Am I on the right track? > > If so, this is OK for trunk. If not, please can you expand on what is going > on in this patch. Thanks for the review. Kugan > > Thanks, > James > > >> >> Thanks, >> Kugan >> >> gcc/testsuite/ChangeLog: >> >> 2017-06-27 Kugan Vivekanandarajah <kug...@linaro.org> >> >> * gcc.target/aarch64/pr63304_1.c: Remove-mno-fix-cortex-a53-843419. >> >> gcc/ChangeLog: >> >> 2017-06-27 Kugan Vivekanandarajah <kug...@linaro.org> >> >> * config/aarch64/aarch64.c (aarch64_override_options_after_change_1): >> Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341 >> for default. > >