From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [patch 03/10] KVM: MMU: do not write-protect large mappings
Date: Fri, 19 Sep 2008 17:29:28 -0700
Message-ID: <48D443E8.3090505@redhat.com>
References: <20080918212749.800177179@localhost.localdomain> <20080918213336.633920548@localhost.localdomain>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: Avi Kivity <avi@qumranet.com>, kvm@vger.kernel.org,
	"David S. Ahern" <daahern@cisco.com>
To: Marcelo Tosatti <mtosatti@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:52088 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1750846AbYITA3Y (ORCPT <rfc822;kvm@vger.kernel.org>);
	Fri, 19 Sep 2008 20:29:24 -0400
In-Reply-To: <20080918213336.633920548@localhost.localdomain>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Marcelo Tosatti wrote:
> There is not much point in write protecting large mappings. This
> can only happen when a page is shadowed during the window between
> is_largepage_backed and mmu_lock acquision. Zap the entry instead, so
> the next pagefault will find a shadowed page via is_largepage_backed and
> fallback to 4k translations.
>
> Simplifies out of sync shadow.
>
> 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
> @@ -1180,11 +1180,16 @@ static int set_spte(struct kvm_vcpu *vcp
>  	    || (write_fault && !is_write_protection(vcpu) && !user_fault)) {
>  		struct kvm_mmu_page *shadow;
>  
> +		if (largepage && has_wrprotected_page(vcpu->kvm, gfn)) {
> +			ret = 1;
> +			spte = shadow_trap_nonpresent_pte;
> +			goto set_pte;
> +		}
> +
>  		spte |= PT_WRITABLE_MASK;
>  
>  		shadow = kvm_mmu_lookup_page(vcpu->kvm, gfn);
> -		if (shadow ||
> -		   (largepage && has_wrprotected_page(vcpu->kvm, gfn))) {
> +		if (shadow) {
>  			pgprintk("%s: found shadow page for %lx, marking ro\n",
>  				 __func__, gfn);
>  			ret = 1;
> @@ -1197,6 +1202,7 @@ static int set_spte(struct kvm_vcpu *vcp
>  	if (pte_access & ACC_WRITE_MASK)
>  		mark_page_dirty(vcpu->kvm, gfn);
>  
> +set_pte:
>  	set_shadow_pte(shadow_pte, spte);
>  	return ret;
>  }
>   

Don't we need to drop a reference to the page?

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.