Messages in this thread | | | Subject | Re: [PATCH V4 11/15] KVM/MMU: Replace tlb flush function with range list flush function | From | Paolo Bonzini <> | Date | Mon, 15 Oct 2018 13:51:13 +0200 |
| |
On 13/10/2018 16:54, lantianyu1986@gmail.com wrote: > while (mmu_unsync_walk(parent, &pages)) { > bool protected = false; > + LIST_HEAD(flush_list); > > - for_each_sp(pages, sp, parents, i) > + for_each_sp(pages, sp, parents, i) { > protected |= rmap_write_protect(vcpu, sp->gfn); > + kvm_mmu_queue_flush_request(sp, &flush_list); > + }
Here you already know that the page has to be flushed, because you are dealing with shadow page tables and those always use 4K pages. So the check on is_last_page is unnecessary.
> > pte_access, PT_PAGE_TABLE_LEVEL, > gfn, spte_to_pfn(sp->spt[i]), > true, false, host_writable); > + if (set_spte_ret && kvm_available_flush_tlb_with_range()) > + kvm_mmu_queue_flush_request(sp, &flush_list); > }
This is wrong, I think. sp is always the same throughout the loop, so you are adding it multiple times to flush_list.
Instead, you need to add a separate range for each virtual address (in this case L2 GPA) that is synced; but for each PTE that you call set_spte here for, you could be syncing multiple L2 GPAs if a single page is reused multiple times by the guest's EPT page tables.
And actually I may be missing something, but doesn't this apply to all call sites? For mmu_sync_children you can do the flush in __rmap_write_protect and return false, similar to the first part of the series, but not for kvm_mmu_commit_zap_page and sync_page.
Can you simplify this series to only have hv_remote_flush_tlb_with_range and remove all the flush_list stuff? That first part is safe and well understood, because it uses the rmap and so it's clear that you have L2 GPAs at hand. Most of the remarks I made on the Hyper-V API will still apply.
Paolo
> if (set_spte_ret & SET_SPTE_NEED_REMOTE_TLB_FLUSH) > - kvm_flush_remote_tlbs(vcpu->kvm); > + kvm_flush_remote_tlbs_with_list(vcpu->kvm, &flush_list); > > return nr_present;
| |