Page MenuHomeFreeBSD

arm64 pmap: correct the condition for determining when to flush the icache
ClosedPublic

Authored by alc on Sat, Aug 29, 4:56 PM.
Tags
None
Referenced Files
F170983497: D59265.id185799.diff
Tue, Sep 8, 12:28 AM
F170978920: D59265.diff
Mon, Sep 7, 11:51 PM
Unknown Object (File)
Sun, Sep 6, 10:12 PM
Unknown Object (File)
Sun, Sep 6, 2:21 PM
Unknown Object (File)
Sun, Sep 6, 2:21 PM
Unknown Object (File)
Sun, Sep 6, 10:15 AM
Unknown Object (File)
Sun, Sep 6, 9:21 AM
Unknown Object (File)
Sat, Sep 5, 7:49 PM
Subscribers

Details

Summary

Whenever we create a user-space mapping, we always set ATTR_S1_PXN in the PTE, which blocks execution of user-space code while running in kernel mode. However, when seeking to determine whether we need to perform an icache flush before installing the new PTE, we test whether sometimes the old PTE or other times the new PTE has ATTR_S1_XN set. The trouble is that ATTR_S1_XN is defined as the bitwise OR of ATTR_S1_PXN and ATTR_S1_UXN and the test for whether ATTR_S1_XN is set is satisfied if either of its constituent bits is set, i.e., we write (l3e & ATTR_S1_XN) != 0, so the test is always true.

In practice, I believe that the ill consequences of this bug are limited: In pmap_enter(), in rare circumstances, e.g., wiring a code page, an unnecessary icache flush will be performed. In pmap_enter_l2() and pmap_enter_l3c(), no icache flush will be performed. However, typically an icache flush would have already been performed on each of the constituent base pages.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

alc requested review of this revision.Sat, Aug 29, 4:56 PM
alc added reviewers: kib, markj.
This revision is now accepted and ready to land.Sat, Aug 29, 6:05 PM

I uncovered this while resurrecting an old patch for reducing the number of icache flushes. If all goes well, I will post that in a week or two.

I'm testing this patch on an EC2 a1.4xlarge (16x Cortex A72) machine to see the greatest impact. (This is the entire machine, so there isn't any variance due to other VMs on the machine.) I added a COUNTER_U64 to track icache flushes by the pmap. As expected, this patch results in an increased icache flush count, since we were not flushing on creating executable superpage mappings. During a -j16 buildworld on a -NODEBUG kernel, PRODUCTION malloc, and LLVM with assertions disabled, the number of flushes goes from ~190M to ~230M and wall clock time goes from ~2:09:10 to ~2:10:00. In particular, system time increased by ~6%.