Prev: [PATCH padmux.c] Fix typo in kerneldoc
Next: New ACL format for better NFSv4 acl interoperability
From: Xiao Guangrong on 3 Jul 2010 08:40


Avi Kivity wrote:

>>
>
> if (!direct) {
> 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(shadow_page, sptep);
> kvm_release_pfn_clean(pfn);
> sptep = NULL;
> break;
> }
> }
>
> the code you moved... under what scenario is it not sufficient?
>

I not move those code, just use common function instead, that it's
FNAME(check_level_mapping)(), there are do the same work.

And this check is not sufficient, since it's only checked if the
mapping is zapped or not exist, for other words only when broken this
judgment:
is_shadow_present_pte(*sptep) && !is_large_pte(*sptep)

but if the middle level is present and it's not the large mapping,
this check is skipped.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Avi Kivity on 3 Jul 2010 08:50
On 07/03/2010 03:44 PM, Avi Kivity wrote:
> On 07/03/2010 03:31 PM, Xiao Guangrong wrote:
>>
>> Avi Kivity wrote:
>>
>>>>
>>> if (!direct) {
>>> 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(shadow_page, sptep);
>>> kvm_release_pfn_clean(pfn);
>>> sptep = NULL;
>>> break;
>>> }
>>> }
>>>
>>> the code you moved... under what scenario is it not sufficient?
>>>
>> I not move those code, just use common function instead, that it's
>> FNAME(check_level_mapping)(), there are do the same work.
>>
>> And this check is not sufficient, since it's only checked if the
>> mapping is zapped or not exist, for other words only when broken this
>> judgment:
>> is_shadow_present_pte(*sptep)&& !is_large_pte(*sptep)
>>
>> but if the middle level is present and it's not the large mapping,
>> this check is skipped.
>
>
> Well, in the description, it looked like everything was using small
> pages (in kvm, level=1 means PTE level, we need to change this one
> day). Please describe again and say exactly when the guest or host
> uses large pages.
>
>

Oh, I see what you mean.

Regarding the patch, is it possible just to move the check before,
instead of adding the 'check' variable?

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Xiao Guangrong on 3 Jul 2010 09:10


Avi Kivity wrote:

>>
>>
>> Well, in the description, it looked like everything was using small
>> pages (in kvm, level=1 means PTE level, we need to change this one
>> day). Please describe again and say exactly when the guest or host
>> uses large pages.
>>
>>
>
> Oh, I see what you mean.
>
> Regarding the patch, is it possible just to move the check before,
> instead of adding the 'check' variable?
>

Umm, if we move the check before the judgment, it'll check every level,
actually, only the opened mapping and the laster level need checked, so
for the performance reason, maybe it's better to keep two check-point.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Avi Kivity on 4 Jul 2010 10:40
On 07/03/2010 03:57 PM, Xiao Guangrong wrote:
>
> Avi Kivity wrote:
>
>
>>> And this check is not sufficient, since it's only checked if the
>>> mapping is zapped or not exist, for other words only when broken this
>>> judgment:
>>> is_shadow_present_pte(*sptep)&& !is_large_pte(*sptep)
>>>
>>> but if the middle level is present and it's not the large mapping,
>>> this check is skipped.
>>>
>>>
>>
>> Well, in the description, it looked like everything was using small
>> pages (in kvm, level=1 means PTE level, we need to change this one
>> day). Please describe again and say exactly when the guest or host uses
>> large pages.
>>
>>
> Avi, sorry for my poor English, i not mean "everything was using small", i don't
> know where cause you confused :-(
>

Well, LVL1 got me to assume those are small pages.

Can you explain more precisely? Use the processor terms like PTE and
PDE. Don't worry about your English :)

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Avi Kivity on 4 Jul 2010 10:40
On 07/03/2010 04:03 PM, Xiao Guangrong wrote:
>
> Avi Kivity wrote:
>
>
>>>
>>> Well, in the description, it looked like everything was using small
>>> pages (in kvm, level=1 means PTE level, we need to change this one
>>> day). Please describe again and say exactly when the guest or host
>>> uses large pages.
>>>
>>>
>>>
>> Oh, I see what you mean.
>>
>> Regarding the patch, is it possible just to move the check before,
>> instead of adding the 'check' variable?
>>
>>
> Umm, if we move the check before the judgment, it'll check every level,
> actually, only the opened mapping and the laster level need checked, so
> for the performance reason, maybe it's better to keep two check-point.
>

What exactly are the conditions when you want to check?

Perhaps we do need to check every level. A write to a PDE (or higher
level) will clear the corresponding spte, but a fault immediately
afterwards can re-establish the spte to point to the new page.

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
First  |  Prev  |  Next  |  Last
Pages: 1 2 3 4
Prev: [PATCH padmux.c] Fix typo in kerneldoc
Next: New ACL format for better NFSv4 acl interoperability