Skip to content

Commit fb25ee7

Browse files
committed
KVM: x86/mmu: Use CMPXCHG when clearing Accessed bit in the shadow MMU
Use CMPXCHG instead of clear_bit(), which currently emits a LOCK BTR since the to-be-cleared bit isn't a compile-time constant, when aging SPTEs in the shadow MMU to align with the approach taken by the TDP MMU, and because using CMPXCHG is far more robust against bugs in KVM. E.g. if the SPTE is somehow no longer an SPTE due to a KVM bug, CMPXCHG will fail gracefully, whereas clear_bit() would potentially corrupt/clobber memory. Clearing the Accessed bit without atomically ensuring the SPTE is still the old SPTE is "fine", as holding the rmap's lock ensures zapping the old SPTE can't fully complete, which in turn ensures a new, different SPTE can't be installed. But that chain of logic isn't exactly obvious, and there's zero reason to avoid CMPXCHG as its cost on modern hardware is within ~1-2 uops of LOCK BTR (and may even be cheaper on some microarchitectures). Doing a 64-bit CMPXCHG on 32-bit kernels does require a more expensive CMPXCHG8B, but 32-bit KVM is all but dead at this point. Cc: James Houghton <jthoughton@google.com> Reviewed-by: Kai Huang <kai.huang@intel.com> Reviewed-by: James Houghton <jthoughton@google.com> Link: https://patch.msgid.link/20260728002236.869865-3-seanjc@google.com Signed-off-by: Sean Christopherson <seanjc@google.com>
1 parent 3d679b7 commit fb25ee7

1 file changed

Lines changed: 14 additions & 13 deletions

File tree

arch/x86/kvm/mmu/mmu.c

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1718,38 +1718,39 @@ static bool kvm_rmap_age_gfn_range(struct kvm *kvm,
17181718
struct kvm_rmap_head *rmap_head;
17191719
struct rmap_iterator iter;
17201720
unsigned long rmap_val;
1721+
u64 old_spte, new_spte;
17211722
bool young = false;
17221723
u64 *sptep;
17231724
gfn_t gfn;
17241725
int level;
1725-
u64 spte;
17261726

17271727
for (level = PG_LEVEL_4K; level <= KVM_MAX_HUGEPAGE_LEVEL; level++) {
17281728
for (gfn = range->start; gfn < range->end;
17291729
gfn += KVM_PAGES_PER_HPAGE(level)) {
17301730
rmap_head = gfn_to_rmap(gfn, level, range->slot);
17311731
rmap_val = kvm_rmap_lock_readonly(rmap_head);
17321732

1733-
for_each_rmap_spte_lockless(rmap_val, &iter, sptep, spte) {
1734-
if (!is_accessed_spte(spte))
1733+
for_each_rmap_spte_lockless(rmap_val, &iter, sptep, old_spte) {
1734+
if (!is_accessed_spte(old_spte))
17351735
continue;
17361736

17371737
if (test_only) {
17381738
kvm_rmap_unlock_readonly(rmap_head, rmap_val);
17391739
return true;
17401740
}
17411741

1742-
if (spte_ad_enabled(spte))
1743-
clear_bit((ffs(shadow_accessed_mask) - 1),
1744-
(unsigned long *)sptep);
1742+
if (spte_ad_enabled(old_spte))
1743+
new_spte = old_spte & ~shadow_accessed_mask;
17451744
else
1746-
/*
1747-
* If the following cmpxchg fails, the
1748-
* spte is being concurrently modified
1749-
* and should most likely stay young.
1750-
*/
1751-
cmpxchg64(sptep, spte,
1752-
mark_spte_for_access_track(spte));
1745+
new_spte = mark_spte_for_access_track(old_spte);
1746+
1747+
/*
1748+
* Don't bother retrying if the CMPXCHG fails,
1749+
* i.e. if another CPU modified the SPTE. The
1750+
* SPTE is either being zapped or is likely
1751+
* still in-use, i.e. is still young.
1752+
*/
1753+
cmpxchg64(sptep, old_spte, new_spte);
17531754
young = true;
17541755
}
17551756

0 commit comments

Comments
 (0)