Re: [PATCH] KVM: MMU: optimize for set_spte

[Marcelo Tosatti <mtosatti@redhat.com>]

On Tue, Dec 04, 2012 at 07:17:11AM +0800, Xiao Guangrong wrote:
> There are two cases we need to adjust page size in set_spte:
> 1): the one is other vcpu creates new sp in the window between mapping_level()
>     and acquiring mmu-lock.
> 2): the another case is the new sp is created by itself (page-fault path) when
>     guest uses the target gfn as its page table.
> 
> In current code, set_spte drop the spte and emulate the access for these case,
> it works not good:
> - for the case 1, it may destroy the mapping established by other vcpu, and
>   do expensive instruction emulation.
> - for the case 2, it may emulate the access even if the guest is accessing
>   the page which not used as page table. There is a example, 0~2M is used as
>   huge page in guest, in this huge page, only page 3 used as page table, then
>   guest read/writes on other pages can cause instruction emulation.
> 
> Both of these cases can be fixed by allowing guest to retry the access, it
> will refault, then we can establish the mapping by using small page
> 
> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> ---
>  arch/x86/kvm/mmu.c |   16 ++++++++++++----
>  1 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index b875a9e..01d7c2a 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -2382,12 +2382,20 @@ static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
>  	    || (!vcpu->arch.mmu.direct_map && write_fault
>  		&& !is_write_protection(vcpu) && !user_fault)) {
> 
> +		/*
> +		 * There are two cases:
> +		 * - the one is other vcpu creates new sp in the window
> +		 *   between mapping_level() and acquiring mmu-lock.
> +		 * - the another case is the new sp is created by itself
> +		 *   (page-fault path) when guest uses the target gfn as
> +		 *   its page table.
> +		 * Both of these cases can be fixed by allowing guest to
> +		 * retry the access, it will refault, then we can establish
> +		 * the mapping by using small page.
> +		 */
>  		if (level > PT_PAGE_TABLE_LEVEL &&
> -		    has_wrprotected_page(vcpu->kvm, gfn, level)) {
> -			ret = 1;
> -			drop_spte(vcpu->kvm, sptep);
> +		    has_wrprotected_page(vcpu->kvm, gfn, level))
>  			goto done;
> -		}
> 
>  		spte |= PT_WRITABLE_MASK | SPTE_MMU_WRITEABLE;
> 
> -- 
> 1.7.7.6

ACK.

Does it fix your testcase?