On Wed, Jul 28, 2021, Peter Xu wrote:
> On Wed, Jul 28, 2021 at 09:39:02PM +0000, Sean Christopherson wrote:
> > On Fri, Jun 25, 2021, Peter Xu wrote:
> > Why implement this as a generic method with a callback? gcc is suprisingly
> > astute in optimizing callback(), but I don't see the point of adding a complex
> > helper that has a single caller, and is extremely unlikely to gain new callers.
> > Or is there another "zap everything" case I'm missing?
>
> No other case; it's just that pte_list_*() helpers will be more self-contained.
Eh, but this flow is as much about rmaps as it is about pte_list.
> If that'll be a performance concern, no objection to hard code it.
It's more about unnecessary complexity than it is about performance, e.g. gcc-10
generates identical code for both version (which did surprise the heck out of me).
If we really want to isolate pte_list_destroy(), I would vote for something like
this (squashed in). pte_list_remove() already calls mmu_spte_clear_track_bits(),
so that particular separation of concerns has already gone out the window.
-/* Return true if rmap existed and callback called, false otherwise */
-static bool pte_list_destroy(struct kvm_rmap_head *rmap_head,
- void (*callback)(u64 *sptep))
+static bool pte_list_destroy(struct kvm_rmap_head *rmap_head)
{
struct pte_list_desc *desc, *next;
int i;
@@ -1013,20 +1011,16 @@ static bool pte_list_destroy(struct kvm_rmap_head *rmap_head,
return false;
if (!(rmap_head->val & 1)) {
- if (callback)
- callback((u64 *)rmap_head->val);
+ mmu_spte_clear_track_bits((u64 *)rmap_head->val);
goto out;
}
desc = (struct pte_list_desc *)(rmap_head->val & ~1ul);
-
- while (desc) {
- if (callback)
- for (i = 0; i < desc->spte_count; i++)
- callback(desc->sptes[i]);
+ for ( ; desc; desc = next) {
+ for (i = 0; i < desc->spte_count; i++)
+ mmu_spte_clear_track_bits(desc->sptes[i]);
next = desc->more;
mmu_free_pte_list_desc(desc);
- desc = next;
}
out:
/* rmap_head is meaningless now, remember to reset it */
@@ -1422,22 +1416,17 @@ static bool rmap_write_protect(struct kvm_vcpu *vcpu, u64 gfn)
return kvm_mmu_slot_gfn_write_protect(vcpu->kvm, slot, gfn, PG_LEVEL_4K);
}
-static void mmu_spte_clear_track_bits_cb(u64 *sptep)
-{
- mmu_spte_clear_track_bits(sptep);
-}
-
static bool kvm_zap_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
const struct kvm_memory_slot *slot)
{
- return pte_list_destroy(rmap_head, mmu_spte_clear_track_bits_cb);
+ return pte_list_destroy(rmap_head);
}
static bool kvm_unmap_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
struct kvm_memory_slot *slot, gfn_t gfn, int level,
pte_t unused)
{
- return kvm_zap_rmapp(kvm, rmap_head, slot);
+ return pte_list_destroy(rmap_head);
}
static bool kvm_set_pte_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
