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

[Avi Kivity <avi@redhat.com>]

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