On 06/07/2011 09:01 PM, Xiao Guangrong wrote:
> If dirty bit is not set, we can make the pte access read-only to avoid handing
> dirty bit everywhere

> diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
> index b0c8184..67971da 100644
> --- a/arch/x86/kvm/paging_tmpl.h
> +++ b/arch/x86/kvm/paging_tmpl.h
> @@ -106,6 +106,9 @@ static unsigned FNAME(gpte_access)(struct kvm_vcpu *vcpu, 
> pt_element_t gpte)
>       unsigned access;
>  
>       access = (gpte & (PT_WRITABLE_MASK | PT_USER_MASK)) | ACC_EXEC_MASK;
> +     if (!is_dirty_gpte(gpte))
> +             access &= ~ACC_WRITE_MASK;
> +

Sorry, it can break something: if the gpte is not on the last level and dirty 
bit
is set later, below patch should fix it, i'll merge it into in the next version.

diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index 4287dc8..6ceb5fd 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -101,12 +101,13 @@ static int FNAME(cmpxchg_gpte)(struct kvm_vcpu *vcpu, 
struct kvm_mmu *mmu,
        return (ret != orig_pte);
 }
 
-static unsigned FNAME(gpte_access)(struct kvm_vcpu *vcpu, pt_element_t gpte)
+static unsigned FNAME(gpte_access)(struct kvm_vcpu *vcpu, pt_element_t gpte,
+                                  bool last)
 {
        unsigned access;
 
        access = (gpte & (PT_WRITABLE_MASK | PT_USER_MASK)) | ACC_EXEC_MASK;
-       if (!is_dirty_gpte(gpte))
+       if (last && !is_dirty_gpte(gpte))
                access &= ~ACC_WRITE_MASK;
 
 #if PTTYPE == 64
@@ -230,8 +231,6 @@ walk:
                        pte |= PT_ACCESSED_MASK;
                }
 
-               pte_access = pt_access & FNAME(gpte_access)(vcpu, pte);
-
                walker->ptes[walker->level - 1] = pte;
 
                if ((walker->level == PT_PAGE_TABLE_LEVEL) ||
@@ -266,7 +265,7 @@ walk:
                        break;
                }
 
-               pt_access = pte_access;
+               pt_access &= FNAME(gpte_access)(vcpu, pte, false);
                --walker->level;
        }
 
@@ -290,6 +289,7 @@ walk:
                walker->ptes[walker->level - 1] = pte;
        }
 
+       pte_access = pt_access & FNAME(gpte_access)(vcpu, pte, true);
        walker->pt_access = pt_access;
        walker->pte_access = pte_access;
        pgprintk("%s: pte %llx pte_access %x pt_access %x\n",
@@ -369,7 +369,7 @@ static void FNAME(update_pte)(struct kvm_vcpu *vcpu, struct 
kvm_mmu_page *sp,
                return;
 
        pgprintk("%s: gpte %llx spte %p\n", __func__, (u64)gpte, spte);
-       pte_access = sp->role.access & FNAME(gpte_access)(vcpu, gpte);
+       pte_access = sp->role.access & FNAME(gpte_access)(vcpu, gpte, true);
        pfn = gfn_to_pfn_atomic(vcpu->kvm, gpte_to_gfn(gpte));
        if (mmu_invalid_pfn(pfn)) {
                kvm_release_pfn_clean(pfn);
@@ -444,7 +444,8 @@ static void FNAME(pte_prefetch)(struct kvm_vcpu *vcpu, 
struct guest_walker *gw,
                if (FNAME(prefetch_invalid_gpte)(vcpu, sp, spte, gpte))
                        continue;
 
-               pte_access = sp->role.access & FNAME(gpte_access)(vcpu, gpte);
+               pte_access = sp->role.access & FNAME(gpte_access)(vcpu, gpte,
+                                                                 true);
                gfn = gpte_to_gfn(gpte);
                pfn = pte_prefetch_gfn_to_pfn(vcpu, gfn,
                                      pte_access & ACC_WRITE_MASK);
@@ -790,7 +791,7 @@ static bool FNAME(sync_mmio_spte)(struct kvm_vcpu *vcpu,
        if (unlikely(is_mmio_spte(*sptep))) {
                gfn_t gfn = gpte_to_gfn(gpte);
                unsigned access = sp->role.access & FNAME(gpte_access)(vcpu,
-                                                       gpte);
+                                                       gpte, true);
 
                if (gfn != get_mmio_spte_gfn(*sptep)) {
                        __set_spte(sptep, shadow_trap_nonpresent_pte);
@@ -868,7 +869,8 @@ static int FNAME(sync_page)(struct kvm_vcpu *vcpu, struct 
kvm_mmu_page *sp)
                }
 
                nr_present++;
-               pte_access = sp->role.access & FNAME(gpte_access)(vcpu, gpte);
+               pte_access = sp->role.access & FNAME(gpte_access)(vcpu, gpte,
+                                                                 true);
                host_writable = sp->spt[i] & SPTE_HOST_WRITEABLE;
 
                set_spte(vcpu, &sp->spt[i], pte_access, 0, 0,
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to