diff mbox

[AArch64] Disable pcrelative_literal_loads with fix-cortex-a53-843419

Message ID CAKdteOayTsTMm96VwcW9OpRZGJ5EfqnJPvDohZ0qa58GDLyE8A@mail.gmail.com
State New
Headers show

Commit Message

Christophe Lyon March 10, 2016, 12:37 p.m. UTC 
On 10 March 2016 at 12:43, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> On Tue, Jan 26, 2016 at 03:43:36PM +0100, Christophe Lyon wrote:

>> With the attachment....

>>

>>

>> On 26 January 2016 at 15:42, Christophe Lyon <christophe.lyon@linaro.org> wrote:

>> > Hi,

>> >

>> > This is a followup to PR63304.

>> >

>> > As discussed in bugzilla, this patch disables pcrelative_literal_loads

>> > when -mfix-cortex-a53-843419 (or its default configure option) is

>> > used.

>> >

>> > I copied the behavior of -mfix-cortex-a53-835769 (e.g. in

>> > aarch64_can_inline_p), and I have tested by building the Linux kernel

>> > using -mfix-cortex-a53-843419 and checked that

>> > R_AARCH64_ADR_PREL_PG_HI21 relocations are not emitted anymore (under

>> > CONFIG_ARM64_ERRATUM_843419).

>> >

>> > For reference, this is motivated by:

>> > https://bugs.linaro.org/show_bug.cgi?id=1994

>> > and further details on Launchpad:

>> > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1533009

>> >

>> > OK for trunk?

>

> Thanks, this looks like a clear regression from GCC 5 (we can no longer

> build the kernel, so this workaround is fine to go in now). Please remember

> to add the link to the relevant PR in the ChangeLog.

>

> I'd also really appreciate a nice big comment over this code:

>

>> +  /* If it is not set on the command line, we default to no pc

>> +     relative literal loads, unless the workaround for Cortex-A53

>> +     erratum 843419 is in effect.  */

>> +  if (opts->x_nopcrelative_literal_loads == 2

>> +      && !TARGET_FIX_ERR_A53_843419)

>

> Explaining why this is important (i.e. some summary of the discussion

> in PR63304 regarding the kernel module loader).

>

> Can you repost with that comment added? I don't have any other objections

> to the patch.

>


OK, here is an updated version.


> Thanks,

> James

>

>
2016-03-10  Christophe Lyon  <christophe.lyon@linaro.org>

	PR target/70113.
	* config/aarch64/aarch64.h (TARGET_FIX_ERR_A53_843419_DEFAULT):
	Always define to 0 or 1.
	(TARGET_FIX_ERR_A53_843419): New macro.
	* config/aarch64/aarch64-elf-raw.h
	(TARGET_FIX_ERR_A53_843419_DEFAULT): Update for above changes.
	* config/aarch64/aarch64-linux.h: Likewise.
	* config/aarch64/aarch64.c
	(aarch64_override_options_after_change_1): Do not default
	aarch64_nopcrelative_literal_loads to true if Cortex-A53 erratum
	843419 is on.
	(aarch64_attributes): Handle fix-cortex-a53-843419.
	(aarch64_can_inline_p): Likewise.
	* config/aarch64/aarch64.opt (aarch64_fix_a53_err843419): Save.

Comments

James Greenhalgh March 10, 2016, 1:24 p.m. UTC | #1 
On Thu, Mar 10, 2016 at 01:37:50PM +0100, Christophe Lyon wrote:
> On 10 March 2016 at 12:43, James Greenhalgh <james.greenhalgh@arm.com> wrote:

> > On Tue, Jan 26, 2016 at 03:43:36PM +0100, Christophe Lyon wrote:

> >> With the attachment....

> >>

> >>

> >> On 26 January 2016 at 15:42, Christophe Lyon <christophe.lyon@linaro.org> wrote:

> >> > Hi,

> >> >

> >> > This is a followup to PR63304.

> >> >

> >> > As discussed in bugzilla, this patch disables pcrelative_literal_loads

> >> > when -mfix-cortex-a53-843419 (or its default configure option) is

> >> > used.

> >> >

> >> > I copied the behavior of -mfix-cortex-a53-835769 (e.g. in

> >> > aarch64_can_inline_p), and I have tested by building the Linux kernel

> >> > using -mfix-cortex-a53-843419 and checked that

> >> > R_AARCH64_ADR_PREL_PG_HI21 relocations are not emitted anymore (under

> >> > CONFIG_ARM64_ERRATUM_843419).

> >> >

> >> > For reference, this is motivated by:

> >> > https://bugs.linaro.org/show_bug.cgi?id=1994

> >> > and further details on Launchpad:

> >> > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1533009

> >> >

> >> > OK for trunk?

> >

> > Thanks, this looks like a clear regression from GCC 5 (we can no longer

> > build the kernel, so this workaround is fine to go in now). Please remember

> > to add the link to the relevant PR in the ChangeLog.

> >

> > I'd also really appreciate a nice big comment over this code:

> >

> >> +  /* If it is not set on the command line, we default to no pc

> >> +     relative literal loads, unless the workaround for Cortex-A53

> >> +     erratum 843419 is in effect.  */

> >> +  if (opts->x_nopcrelative_literal_loads == 2

> >> +      && !TARGET_FIX_ERR_A53_843419)

> >

> > Explaining why this is important (i.e. some summary of the discussion

> > in PR63304 regarding the kernel module loader).

> >

> > Can you repost with that comment added? I don't have any other objections

> > to the patch.

> >

> 

> OK, here is an updated version.


Thanks.

This is OK for trunk.

James
diff mbox

Patch

diff --git a/gcc/config/aarch64/aarch64-elf-raw.h b/gcc/config/aarch64/aarch64-elf-raw.h
index 2dcb6d4..9097017 100644
--- a/gcc/config/aarch64/aarch64-elf-raw.h
+++ b/gcc/config/aarch64/aarch64-elf-raw.h
@@ -35,7 +35,7 @@ 
   " %{mfix-cortex-a53-835769:--fix-cortex-a53-835769}"
 #endif
 
-#ifdef TARGET_FIX_ERR_A53_843419_DEFAULT
+#if TARGET_FIX_ERR_A53_843419_DEFAULT
 #define CA53_ERR_843419_SPEC \
   " %{!mno-fix-cortex-a53-843419:--fix-cortex-a53-843419}"
 #else
diff --git a/gcc/config/aarch64/aarch64-linux.h b/gcc/config/aarch64/aarch64-linux.h
index 6064b26..5fcaa59 100644
--- a/gcc/config/aarch64/aarch64-linux.h
+++ b/gcc/config/aarch64/aarch64-linux.h
@@ -53,7 +53,7 @@ 
   " %{mfix-cortex-a53-835769:--fix-cortex-a53-835769}"
 #endif
 
-#ifdef TARGET_FIX_ERR_A53_843419_DEFAULT
+#if TARGET_FIX_ERR_A53_843419_DEFAULT
 #define CA53_ERR_843419_SPEC \
   " %{!mno-fix-cortex-a53-843419:--fix-cortex-a53-843419}"
 #else
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 801f95a..51dfe79 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -8132,9 +8132,18 @@  aarch64_override_options_after_change_1 (struct gcc_options *opts)
   if (opts->x_nopcrelative_literal_loads == 1)
     aarch64_nopcrelative_literal_loads = false;
 
-  /* If it is not set on the command line, we default to no
-     pc relative literal loads.  */
-  if (opts->x_nopcrelative_literal_loads == 2)
+  /* If it is not set on the command line, we default to no pc
+     relative literal loads, unless the workaround for Cortex-A53
+     erratum 843419 is in effect.  */
+  /* This is PR70113. When building the Linux kernel with
+     CONFIG_ARM64_ERRATUM_843419, support for relocations
+     R_AARCH64_ADR_PREL_PG_HI21 and R_AARCH64_ADR_PREL_PG_HI21_NC is
+     removed from the kernel to avoid loading objects with possibly
+     offending sequences. With nopcrelative_literal_loads, we would
+     generate such relocations, preventing the kernel build from
+     succeeding.  */
+  if (opts->x_nopcrelative_literal_loads == 2
+      && !TARGET_FIX_ERR_A53_843419)
     aarch64_nopcrelative_literal_loads = true;
 
   /* In the tiny memory model it makes no sense
@@ -8818,6 +8827,8 @@  static const struct aarch64_attribute_info aarch64_attributes[] =
      OPT_mgeneral_regs_only },
   { "fix-cortex-a53-835769", aarch64_attr_bool, true, NULL,
      OPT_mfix_cortex_a53_835769 },
+  { "fix-cortex-a53-843419", aarch64_attr_bool, true, NULL,
+     OPT_mfix_cortex_a53_843419 },
   { "cmodel", aarch64_attr_enum, false, NULL, OPT_mcmodel_ },
   { "strict-align", aarch64_attr_mask, false, NULL, OPT_mstrict_align },
   { "omit-leaf-frame-pointer", aarch64_attr_bool, true, NULL,
@@ -9232,6 +9243,12 @@  aarch64_can_inline_p (tree caller, tree callee)
 	  2, TARGET_FIX_ERR_A53_835769_DEFAULT))
     return false;
 
+  if (!aarch64_tribools_ok_for_inlining_p (
+	  caller_opts->x_aarch64_fix_a53_err843419,
+	  callee_opts->x_aarch64_fix_a53_err843419,
+	  2, TARGET_FIX_ERR_A53_843419))
+    return false;
+
   /* If the user explicitly specified -momit-leaf-frame-pointer for the
      caller and calle and they don't match up, reject inlining.  */
   if (!aarch64_tribools_ok_for_inlining_p (
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 8b463c9..ec96ce3 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -179,6 +179,20 @@  extern unsigned aarch64_architecture_version;
   ((aarch64_fix_a53_err835769 == 2)	\
   ? TARGET_FIX_ERR_A53_835769_DEFAULT : aarch64_fix_a53_err835769)
 
+/* Make sure this is always defined so we don't have to check for ifdefs
+   but rather use normal ifs.  */
+#ifndef TARGET_FIX_ERR_A53_843419_DEFAULT
+#define TARGET_FIX_ERR_A53_843419_DEFAULT 0
+#else
+#undef TARGET_FIX_ERR_A53_843419_DEFAULT
+#define TARGET_FIX_ERR_A53_843419_DEFAULT 1
+#endif
+
+/* Apply the workaround for Cortex-A53 erratum 843419.  */
+#define TARGET_FIX_ERR_A53_843419	\
+  ((aarch64_fix_a53_err843419 == 2)	\
+  ? TARGET_FIX_ERR_A53_843419_DEFAULT : aarch64_fix_a53_err843419)
+
 /* ARMv8.1 Adv.SIMD support.  */
 #define TARGET_SIMD_RDMA (TARGET_SIMD && AARCH64_ISA_RDMA)
 
diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index 49ef0c6..c637ff4 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -73,7 +73,7 @@  Target Report Var(aarch64_fix_a53_err835769) Init(2) Save
 Workaround for ARM Cortex-A53 Erratum number 835769.
 
 mfix-cortex-a53-843419
-Target Report
+Target Report Var(aarch64_fix_a53_err843419) Init(2) Save
 Workaround for ARM Cortex-A53 Erratum number 843419.
 
 mlittle-endian