From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: cr3 OOS optimisation breaks 32-bit GNU/kFreeBSD guest
Date: Sat, 4 Apr 2009 14:01:43 -0300
Message-ID: <20090404170143.GA3303@amt.cnet>
References: <20090223003305.GW12976@hall.aurel32.net> <20090320231405.GA26415@amt.cnet> <49C60644.2090904@redhat.com> <20090323172725.GA28775@amt.cnet> <49C8AC35.3030803@redhat.com> <20090403214548.GA5394@amt.cnet> <49D73873.2090302@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Aurelien Jarno <aurelien@aurel32.net>, kvm@vger.kernel.org
To: Avi Kivity <avi@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:47551 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753483AbZDDRCX (ORCPT <rfc822;kvm@vger.kernel.org>);
	Sat, 4 Apr 2009 13:02:23 -0400
Content-Disposition: inline
In-Reply-To: <49D73873.2090302@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Sat, Apr 04, 2009 at 01:37:39PM +0300, Avi Kivity wrote:
>> diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
>> index 09782a9..728be72 100644
>> --- a/arch/x86/kvm/paging_tmpl.h
>> +++ b/arch/x86/kvm/paging_tmpl.h
>> @@ -308,8 +308,14 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
>>  			break;
>>  		}
>>  -		if (is_shadow_present_pte(*sptep) && !is_large_pte(*sptep))
>> +		if (is_shadow_present_pte(*sptep) && !is_large_pte(*sptep)) {
>> +			if (level-1 == PT_PAGE_TABLE_LEVEL) {
>> +				shadow_page = page_header(__pa(sptep));
>> +				if (shadow_page->unsync && shadow_page->global)
>> +					kvm_sync_page(vcpu, shadow_page);
>> +			}
>>  			continue;
>> +		}
>>   		if (is_large_pte(*sptep)) {
>>  			rmap_remove(vcpu->kvm, sptep);
>>   
>
> But here the shadow page is already linked?  Isn't the root cause that  
> an invlpg was called when the page wasn't linked, so it wasn't seen by  
> invlpg?
>
> So I thought the best place would be in fetch(), after  
> kvm_mmu_get_page().  If we're linking a page which contains global ptes,  
> they might be unsynced due to invlpgs that we've missed.
>
> Or am I missing something about the root cause?

The problem is when the page is unreachable due to a higher level path
being unlinked. Say:

level 4 -> level 3 . level 2 -> level 1 (global unsync)

The dot there means level 3 is not linked to level 2, so invlpg can't
reach the global unsync at level 1.

kvm_mmu_get_page does sync pages when it finds them, so the code is
already safe for the "linking a page which contains global ptes" case
you mention above.