Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable the Arm Cortex-A53 errata mitigation on aarch64-unknown-none #118095

Merged
merged 1 commit into from Nov 27, 2023

Conversation

jonathanpallant
Copy link
Contributor

Arm Cortex-A53 CPUs have an errata related to a specific sequence of instructions - errata number 843419 (https://documentation-service.arm.com/static/5fa29fddb209f547eebd361d). There is a mitigation that can be applied at link-time which detects the when sequence of instructions exists at a specific alignment. When detected, the linker re-writes those instructions and either changes an ADRP to an ADR, or bounces to a veneer to break the sequence.

The linker argument to enable the mitigation is "--fix-cortex-a53-843419", and this is supported by GNU ld and LLVM lld. The gcc argument to enable the flag is "-mfix-cortex-a53-843419".

Because the aarch64-unknown-none target uses rust-lld directly, this patch causes rustc to emit the "--fix-cortex-a53-843419" argument when calling the linker, just like aarch64-linux-gnu-gcc on Ubuntu 22.04 does.

Failure to enable this mitigation in the linker can cause the production of instruction sequences that do not execute correctly on Arm Cortex-A53.

Arm Cortex-A53 CPUs have an errata related to a specific sequence of instructions - errata number 843419 (https://documentation-service.arm.com/static/5fa29fddb209f547eebd361d). There is a mitigation that can be applied at link-time which detects the when sequence of instructions exists at a specific alignment. When detected, the linker re-writes those instructions and either changes an ADRP to an ADR, or bounces to a veneer to break the sequence.

The linker argument to enable the mitigation is "--fix-cortex-a53-843419", and this is supported by GNU ld and LLVM lld. The gcc argument to enable the flag is "-mfix-cortex-a53-843419".

Because the aarch64-unknown-none target uses rust-lld directly, this patch causes rustc to emit the "--fix-cortex-a53-843419" argument when calling the linker, just like aarch64-linux-gnu-gcc on Ubuntu 22.04 does.

Failure to enable this mitigation in the linker can cause the production of instruction sequences that do not execute correctly on Arm Cortex-A53.
@rustbot
Copy link
Collaborator

rustbot commented Nov 20, 2023

r? @b-naber

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 20, 2023
@rustbot
Copy link
Collaborator

rustbot commented Nov 20, 2023

These commits modify compiler targets.
(See the Target Tier Policy.)

@jonathanpallant
Copy link
Contributor Author

We should perhaps enable this on all the rust-lld linked aarch64 targets?

Also, I note that Aarch64 Linux turns it on by default.

@Mark-Simulacrum
Copy link
Member

Why is this gated in linkers behind a flag? It seems like something that could "just" be done by default - what is the cost? Performance overhead?

@jonathanpallant
Copy link
Contributor Author

jonathanpallant commented Nov 20, 2023

One possible mitigation is cheap - replacing an ADRP instruction with an ADR instruction. If that doesn't work (the PC-relative address is too large) it replace one instruction of the sequence of four with a branch to a veneer, which performs the displaced instruction and then branches back, which is slower. Due to reasons, GNU ld actually emits the veneer in either case, so it costs you code space either way. LLVM's lld seems to always emit the veneer and never does the ADRP to ADR replacement.

The mitigation only applies if the sequence falls on two out of every 1024 code addresses (0x...FF8 and 0x...FFC), so it's rare. But if you don't have a buggy Cortex-A53 you might prefer to not have it?

@jonathanpallant
Copy link
Contributor Author

Here's the output of a piece of code that trips the silicon errata, linked without the flag:

0000000000211ff8 <_start>:
  211ff8: 41 d0 3b d5  	mrs	x1, TPIDR_EL0
  211ffc: 00 01 00 d0  	adrp	x0, 0x233000 <_otherfunc+0x78>
  212000: 21 00 40 f9  	ldr	x1, [x1]
  212004: 00 0c 40 f9  	ldr	x0, [x0, #24]
  212008: c0 03 5f d6  	ret

000000000021200c <_otherfunc>:
  21200c: 00 00 80 d2  	mov	x0, #0
  212010: c0 03 5f d6  	ret

And with the flag:

0000000000211ff8 <_start>:
  211ff8: 41 d0 3b d5  	mrs	x1, TPIDR_EL0
  211ffc: 00 01 00 d0  	adrp	x0, 0x233000 <__CortexA53843419_212004+0x70>
  212000: 21 00 40 f9  	ldr	x1, [x1]
  212004: 04 00 00 14  	b	0x212014 <__CortexA53843419_212004>
  212008: c0 03 5f d6  	ret

000000000021200c <_otherfunc>:
  21200c: 00 00 80 d2  	mov	x0, #0
  212010: c0 03 5f d6  	ret

0000000000212014 <__CortexA53843419_212004>:
  212014: 00 10 40 f9  	ldr	x0, [x0, #32]
  212018: fc ff ff 17  	b	0x212008 <_start+0x10>

@davidtwco
Copy link
Member

r? @davidtwco

@rustbot rustbot assigned davidtwco and unassigned b-naber Nov 27, 2023
@davidtwco
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 27, 2023

📌 Commit 4741f44 has been approved by davidtwco

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 27, 2023
@davidtwco
Copy link
Member

We should perhaps enable this on all the rust-lld linked aarch64 targets?

Also, I note that Aarch64 Linux turns it on by default.

@rustbot ping arm

Interested on whether this would be recommended by those more familiar than ARM than myself. I've approved this PR as-is, because it seems like a straightforward improvement, and we can enable this mitigation more widely as a follow-up if recommended.

@rustbot rustbot added the O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state label Nov 27, 2023
@rustbot
Copy link
Collaborator

rustbot commented Nov 27, 2023

Hey ARM Group! This bug has been identified as a good "ARM candidate".
In case it's useful, here are some instructions for tackling these sorts of
bugs. Maybe take a look?
Thanks! <3

cc @adamgemmell @hug-dev @jacobbramley @JamieCunliffe @joaopaulocarreiro @raw-bin @Stammark

@jacobbramley
Copy link
Contributor

This makes sense to enable by default. There might be cases where someone wants to disable it because they know they aren't going to use an affected A53 and have a performance-critical workload, but they can override it if so.

It might also be nice to disable the workaround if -C target-cpu is set to something that doesn't include A53, but that's probably fiddly and something that can be considered later.

Thanks!

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 27, 2023
Rollup of 4 pull requests

Successful merges:

 - rust-lang#118095 (Enable the Arm Cortex-A53 errata mitigation on aarch64-unknown-none)
 - rust-lang#118340 (Use helper functions in `pretty.rs` instead of accessing the `Cell`s manually)
 - rust-lang#118358 (make const tests independent of std debug assertions)
 - rust-lang#118359 (Suggest swapping the order of `ref` and `box`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7b4eb52 into rust-lang:master Nov 27, 2023
22 checks passed
@rustbot rustbot added this to the 1.76.0 milestone Nov 27, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 27, 2023
Rollup merge of rust-lang#118095 - ferrocene:apply-cortex-a53-fix, r=davidtwco

Enable the Arm Cortex-A53 errata mitigation on aarch64-unknown-none

Arm Cortex-A53 CPUs have an errata related to a specific sequence of instructions - errata number 843419 (https://documentation-service.arm.com/static/5fa29fddb209f547eebd361d). There is a mitigation that can be applied at link-time which detects the when sequence of instructions exists at a specific alignment. When detected, the linker re-writes those instructions and either changes an ADRP to an ADR, or bounces to a veneer to break the sequence.

The linker argument to enable the mitigation is "--fix-cortex-a53-843419", and this is supported by GNU ld and LLVM lld. The gcc argument to enable the flag is "-mfix-cortex-a53-843419".

Because the aarch64-unknown-none target uses rust-lld directly, this patch causes rustc to emit the "--fix-cortex-a53-843419" argument when calling the linker, just like aarch64-linux-gnu-gcc on Ubuntu 22.04 does.

Failure to enable this mitigation in the linker can cause the production of instruction sequences that do not execute correctly on Arm Cortex-A53.
@tshepang tshepang deleted the apply-cortex-a53-fix branch November 28, 2023 08:27
@jonathanpallant
Copy link
Contributor Author

How do you override pre_link_args?

@jacobbramley
Copy link
Contributor

How do you override pre_link_args?

That is a good point. I was just going by the phrase "by default" in the comment, thinking that -C link-arg could do it, but that is additive and linkers don't seem to accept --no-fix-cortex-a53-843419. That, and std will be compiled with this flag (in the absence of build-std).

I might be wrong, but I think that if someone really wanted to turn this off, they'd have to build a modified toolchain (and std). That isn't really ideal, but it's probably still better than shipping a pre-built std without the mitigations.

Even if we ship std with mitigations, we could allow users to disable them in the other project. (I'm not sure how, but hypothetically we could do that.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants