lkml.org 
[lkml]   [2015]   [Nov]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 05/10] KVM: x86: MMU: Use for_each_rmap_spte macro instead of pte_list_walk()
    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_b1@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);



    \
     
     \ /
      Last update: 2015-11-13 23:21    [W:4.056 / U:0.568 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site