On Tue, Dec 06, 2022 at 05:36:00PM +0000, Ben Gardon wrote:
> Move the various rmap zap functions to rmap.c. These functions are less
> "pure" rmap operations in that they also contain some SPTE manipulation,
> however they're mostly about rmap / pte list manipulation.
>
> No functional change intended.
>
> Signed-off-by: Ben Gardon <bgardon@xxxxxxxxxx>
> ---
[...]
> -static void kvm_zap_one_rmap_spte(struct kvm *kvm,
> - struct kvm_rmap_head *rmap_head, u64 *sptep)
> -{
> - mmu_spte_clear_track_bits(kvm, sptep);
> - pte_list_remove(sptep, rmap_head);
> -}
> -
> -/* Return true if at least one SPTE was zapped, false otherwise */
> -static bool kvm_zap_all_rmap_sptes(struct kvm *kvm,
> - struct kvm_rmap_head *rmap_head)
> -{
> - struct pte_list_desc *desc, *next;
> - int i;
> -
> - if (!rmap_head->val)
> - return false;
> -
> - if (!(rmap_head->val & 1)) {
> - mmu_spte_clear_track_bits(kvm, (u64 *)rmap_head->val);
> - goto out;
> - }
> -
> - desc = (struct pte_list_desc *)(rmap_head->val & ~1ul);
> -
> - for (; desc; desc = next) {
> - for (i = 0; i < desc->spte_count; i++)
> - mmu_spte_clear_track_bits(kvm, desc->sptes[i]);
> - next = desc->more;
> - free_pte_list_desc(desc);
> - }
> -out:
> - /* rmap_head is meaningless now, remember to reset it */
> - rmap_head->val = 0;
> - return true;
> -}
I don't like moving the rmap zap functions into rmap.c, because they are
more mmu.c functions, as you note in the commit description. e.g. It's
odd to have kvm_zap_all_rmap_sptes() in rmap.c but not, say
__rmap_clear_dirty().
I get your point though that kvm_zap_all_rmap_sptes() has to know
intimate details of the pte_list_desc structure. It would be nice to
keep those details isolated to rmap.c.
What about keeping the zap functions mmu.c and just provide a better API
for kvm_zap_all_rmap_sptes() to process the rmap entries?
e.g.
mmu.c:
static bool kvm_zap_all_rmap_sptes(struct kvm *kvm,
struct kvm_rmap_head *rmap_head)
{
struct rmap_iterator iter;
bool flush = false;
u64 *sptep;
for_each_rmap_spte(rmap_head, &iter, sptep) {
mmu_spte_clear_track_bits(kvm, sptep);
flush = true;
}
pte_list_free_all(rmap_head); // <-- implemented in rmap.c
return flush;
}
This should be about as efficient as the current approach (same big-O
notation at least) and maintain the separation of pte_list_desc
internals in rmap.c.
