Re: [PATCH 5/7] KVM: MMU: Add validate_indirect_spte() helper

[Avi Kivity <avi@redhat.com>]

On 07/12/2010 08:12 AM, Xiao Guangrong wrote:
>
> Avi Kivity wrote:
>    
>> Move the code to validate an indirect shadow page (by verifying that the gpte
>> has not changed since it was fetched) into a helper.
>>
>> Signed-off-by: Avi Kivity<avi@redhat.com>
>> ---
>>   arch/x86/kvm/paging_tmpl.h |   29 ++++++++++++++++++++---------
>>   1 files changed, 20 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
>> index cacb4f2..72f54fe 100644
>> --- a/arch/x86/kvm/paging_tmpl.h
>> +++ b/arch/x86/kvm/paging_tmpl.h
>> @@ -299,6 +299,23 @@ static void FNAME(update_pte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
>>   		     gpte_to_gfn(gpte), pfn, true, true);
>>   }
>>
>> +static bool FNAME(validate_indirect_spte)(struct kvm_vcpu *vcpu,
>> +					  u64 *sptep, struct kvm_mmu_page *sp,
>> +					  struct guest_walker *gw, int level)
>> +{
>> +	int r;
>> +	pt_element_t curr_pte;
>> +
>> +	r = kvm_read_guest_atomic(vcpu->kvm,
>> +				  gw->pte_gpa[level - 2],
>> +				&curr_pte, sizeof(curr_pte));
>> +	if (r || curr_pte != gw->ptes[level - 2]) {
>> +		kvm_mmu_put_page(sp, sptep);
>> +		return false;
>>      
> I think it's 'level - 1' here for checking 'level''s mapping
>    

Well, I'm just moving code around.  If it needs to be fixed, that's 
another patch.

I think the logic is, we just write-protected the shadow page for level 
L-1, so we need to verify that the gpte we write protected hasn't 
changed.  That gpte is in level  L - 1, and it is stored in 
gw->gptes[level - 2].

Of course, that logic is wrong, we need to check all gptes, since 
between the guest walk and FNAME(fetch)() the shadow pages and gptes may 
have changed several times.  I'll fix that later.

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