This is an archive of the discontinued LLVM Phabricator instance.

Add missing AArch64 data synchronization barrier (dsb) to __clear_cache
ClosedPublic

Authored by srhines on Jun 11 2021, 12:23 AM.

Details

Summary

https://developer.arm.com/documentation/den0024/a/Caches/Cache-maintenance
covers how to properly clear caches on AArch64, and the builtin
implementation was missing a dsb ish after clearing the icache for the
selected range.

Diff Detail

Event Timeline

srhines requested review of this revision.Jun 11 2021, 12:23 AM
srhines created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJun 11 2021, 12:23 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript
kristof.beyls accepted this revision.Jun 11 2021, 12:53 AM

Thank you Stephen; LGTM!

This revision is now accepted and ready to land.Jun 11 2021, 12:53 AM
pcc added a subscriber: pcc.Jun 14 2021, 10:20 AM

Do we still need the DSB on line 119?

Do we still need the DSB on line 119?

I believe so.
Quoting from the ArmARM (ARM DDI 0487G.a), "B2.3.8 Ordering of instruction fetches":

Ordering of instruction fetches
For two memory locations A and B, if A has been written to and been made coherent with the instruction fetches of
the shareability domain, before an update to B by an observer in the same shareability domain, then the instruction
stream of each observer in the shareability domain will not see the updated value of B without also seeing the
updated value of A.
A write has been made coherent with an instruction fetch of a shareability domain when:

CTR_EL0.{DIC, IDC} == {0, 0}

The location written to has been cleaned to the Point of unification (PoU) from the data cache, and
that clean is complete for the shareability domain. Subsequently the location has been invalidated
to the Point of unification (PoU) from the instruction cache, and that invalidation is complete for
the shareability domain.

The last paragraph ("..., and that clean is complete for the shareability domain. Subsequently...") implies that dsb on line 119 is needed.