Skip to content

Commit

Permalink
aarch64: force TLB flush when mprotect changes permission
Browse files Browse the repository at this point in the history
When testing the tst-map.cc and tst-elf-permissions.cc with the upcoming
patch to add signals support on aarch64, I noticed that sometimes they
would kind of "hang" for a while and eventually complete successfully.
This would happen especially when running in non-SMP mode (1 CPU). After more
investigation I discovered that the tests would actually get into a page fault
"loop" after calling mprotect() in the signal handler and stay like so
until the page table changes were flushed which I am explaining below.

Analysis of the mmu::protect() and the protection vma operation made
me realize that it has an optimization to trigger full TLB flush only
if pemissions are reduced for any of the relevant pages (see
eefcb08). The problem is that on ARM
any change to the page entry table (regardless if reduction or
expansion) needs a forced completion of writes achieved by the sequence
of `dsb ishst` followed by `isb`. Full flush of TLB on ARM does that but
on top of the expensive `tlbi vmalle1is`.

So to fix this problem, this patch changes change_perm() to return
true if permission changes on aarch64 which would then trigger TLB
flush when the protection vma operation completes. In future we may
optimize it.

Signed-off-by: Waldemar Kozaczuk <[email protected]>
  • Loading branch information
wkozaczuk committed May 4, 2022
1 parent a26f0b2 commit fd24359
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions core/mmu.cc
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 242,15 @@ bool change_perm(hw_ptep<N> ptep, unsigned int perm)
pte.set_rsvd_bit(0, !perm);
ptep.write(pte);

#ifdef __x86_64__
return old & ~perm;
#endif
#ifdef __aarch64__
//TODO: This will trigger full tlb flush in slightly more cases than on x64
//and in future we should investigate more precise and hopefully lighter
//mechanism. But for now it will do it.
return old != perm;
#endif
}

template<int N>
Expand Down

0 comments on commit fd24359

Please sign in to comment.