Re: [patch 3/3] KVM: MMU: prepopulate the shadow on invlpg

Avi Kivity
2008-10-26T11:48:25Z

Marcelo Tosatti wrote:
> If the guest executes invlpg on a non present entry, peek into th
> pagetable and attempt to prepopulate the shadow entry.
>
> Also stop dirty fault updates from interfering with the fork dete
>
> For RHEL3 and 32-bit Vista guests the success rate is high. With Win2003
> there is a 1:2 success/fail ratio, but even then compilation benchmarks
> are slightly faster. 2000 and XP rarely execute invlpg on non present
> entries.
>
> 2% improvement on RHEL3/AIM7.
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> Index: kvm/arch/x86/kvm/mmu.c
> ===================================================================
> --- kvm.orig/arch/x86/kvm/mmu.c
> +++ kvm/arch/x86/kvm/mmu.c
> @@ -2365,7 +2365,8 @@ static void kvm_mmu_access_page(struct k
> }
>
> void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
> - const u8 *new, int bytes)
> + const u8 *new, int bytes,
> + bool speculative)
>

kvm_mmu_pte_write()s are always speculative. Maybe this is misnamed?

Perhaps 'guest_initiated' (with the opposite meaning).

> @@ -222,7 +223,7 @@ walk:
> if (ret)
> goto walk;
> pte |= PT_DIRTY_MASK;
> - kvm_mmu_pte_write(vcpu, pte_gpa, (u8 *)&pte, sizeof(pte));
> + kvm_mmu_pte_write(vcpu, pte_gpa, (u8 *)&pte, sizeof(pte), 1);
>

This is definitely not a speculative write. But it is !guest_initiated.

> @@ -467,10 +468,18 @@ static int FNAME(shadow_invlpg_entry)(st
> struct kvm_vcpu *vcpu, u64 addr,
> u64 *sptep, int level)
> {
> + struct shadow_walker *sw =
> + container_of(_sw, struct shadow_walker, walker);
>
> if (level == PT_PAGE_TABLE_LEVEL) {
> - if (is_shadow_present_pte(*sptep))
> + struct kvm_mmu_page *sp = page_header(__pa(sptep));
>

blank line after declarations. Or move to head of function.

> + sw->pte_gpa = (sp->gfn << PAGE_SHIFT);
> + sw->pte_gpa += (sptep - sp->spt) * sizeof(pt_element_t);
> +
> + if (is_shadow_present_pte(*sptep)) {
> rmap_remove(vcpu->kvm, sptep);
> + sw->pte_gpa = -1;
>

Why? The pte could have heen replaced (for example, a write access to a
cow page).

--
error compiling committee.c: too many arguments to function