On Thu, Nov 12, 2015 at 08:52:45PM +0900, Takuya Yoshikawa wrote:
> kvm_mmu_mark_parents_unsync() alone uses pte_list_walk(), witch does
> nearly the same as the for_each_rmap_spte macro. The only difference
> is that is_shadow_present_pte() checks cannot be placed there because
> kvm_mmu_mark_parents_unsync() can be called with a new parent pointer
> whose entry is not set yet.
>
> By calling mark_unsync() separately for the parent and adding the parent
> pointer to the parent_ptes chain later in kvm_mmu_get_page(), the macro
> works with no problem.
>
> Signed-off-by: Takuya Yoshikawa <yoshikawa_takuya...@lab.ntt.co.jp>
> ---
> arch/x86/kvm/mmu.c | 36 +++++++++++++-----------------------
> 1 file changed, 13 insertions(+), 23 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index e8cfdc4..1691171 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -1007,26 +1007,6 @@ static void pte_list_remove(u64 *spte, unsigned long
> *pte_list)
> }
> }
>
> -typedef void (*pte_list_walk_fn) (u64 *spte);
> -static void pte_list_walk(unsigned long *pte_list, pte_list_walk_fn fn)
> -{
> - struct pte_list_desc *desc;
> - int i;
> -
> - if (!*pte_list)
> - return;
> -
> - if (!(*pte_list & 1))
> - return fn((u64 *)*pte_list);
> -
> - desc = (struct pte_list_desc *)(*pte_list & ~1ul);
> - while (desc) {
> - for (i = 0; i < PTE_LIST_EXT && desc->sptes[i]; ++i)
> - fn(desc->sptes[i]);
> - desc = desc->more;
> - }
> -}
> -
> static unsigned long *__gfn_to_rmap(gfn_t gfn, int level,
> struct kvm_memory_slot *slot)
> {
> @@ -1741,7 +1721,12 @@ static struct kvm_mmu_page *kvm_mmu_alloc_page(struct
> kvm_vcpu *vcpu,
> static void mark_unsync(u64 *spte);
> static void kvm_mmu_mark_parents_unsync(struct kvm_mmu_page *sp)
> {
> - pte_list_walk(&sp->parent_ptes, mark_unsync);
> + u64 *sptep;
> + struct rmap_iterator iter;
> +
> + for_each_rmap_spte(&sp->parent_ptes, &iter, sptep) {
> + mark_unsync(sptep);
> + }
> }
>
> static void mark_unsync(u64 *spte)
> @@ -2111,12 +2096,17 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct
> kvm_vcpu *vcpu,
Faulting a spte, and one of the levels of sptes, either
spte-1
spte-2
spte-3
has present bit clear. So we're searching for a guest page to shadow, with
gfn "gfn".
> if (sp->unsync && kvm_sync_page_transient(vcpu, sp))
> break;
If a shadow for gfn exists, but is unsync, sync guest-page ---to--> kvm
sptes.
> - mmu_page_add_parent_pte(vcpu, sp, parent_pte);
add "gfn" (actually its "struct kvm_mmu_page *sp" pointer) to
the parent.
> if (sp->unsync_children) {
> kvm_make_request(KVM_REQ_MMU_SYNC, vcpu);
> kvm_mmu_mark_parents_unsync(sp);
kvm_mmu_mark_parents_unsync relied on the links from current level all
the way to top level to mark all levels unsync, so that on guest entry,
KVM_REQ_MMU_SYNC is processed and any level is brought from guest -->
kvm pages. This now fails, because you removed "mmu_page_add_parent_pte"
(the link is not formed all the way to root).
Unless i am missing something, this is not correct.
> - } else if (sp->unsync)
> + if (parent_pte)
> + mark_unsync(parent_pte);
> + } else if (sp->unsync) {
> kvm_mmu_mark_parents_unsync(sp);
> + if (parent_pte)
> + mark_unsync(parent_pte);
> + }
> + mmu_page_add_parent_pte(vcpu, sp, parent_pte);
>
> __clear_sp_write_flooding_count(sp);
> trace_kvm_mmu_get_page(sp, false);
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
