lkml.org 
[lkml]   [2010]   [Jul]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 5/6] KVM: MMU: combine guest pte read between walk and pte prefetch


Avi Kivity wrote:

>> Looks into the code more carefully, maybe this code is wrong:
>>
>>
>> if (!direct) {
>> r = kvm_read_guest_atomic(vcpu->kvm,
>> - gw->pte_gpa[level - 2],
>> + gw->pte_gpa[level - 1],
>> &curr_pte, sizeof(curr_pte));
>> - if (r || curr_pte != gw->ptes[level - 2]) {
>> + if (r || curr_pte != gw->ptes[level - 1]) {
>> kvm_mmu_put_page(sp, sptep);
>> kvm_release_pfn_clean(pfn);
>> sptep = NULL;
>>
>> It should check the 'level' mapping not 'level - 1', in the later
>> description
>> i'll explain it.
>>
>
> Right, this fixes the check for the top level, but it removes a check
> from the bottom level.
>

We no need check the bottom level if guest not modify the bottom level,
if guest modify it, the bottom level is no-present, it also can broke
Point A's judgment and be checked by 'Point C'

> We need to move this to the top of the loop so we check all levels. I
> guess this is why you needed to add a new check point. But since we
> loop at least glevels times, we don't need two check points.
>

>
> Ok. So moving the check to before point A, and s/level - 2/level - 1/
> should work, yes?
>
> Should be slightly simpler since we don't need to kvm_mmu_put_page(sp,
> sptep) any more.

Yeah, it can work, but check all levels is really unnecessary, if guest not
modify the level, the check can be avoid.

This is why i choose two check-point, one is behind Point A's judgment, this
point checks the level which modified by guest, and another point is at mapping
last level point, this check is alway need.

>
> Thanks for the detailed explanation.
>

It's really my pleasure :-)



\
 
 \ /
  Last update: 2010-07-05 10:51    [W:0.389 / U:0.208 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site