Re: [PATCH v4 5/6] KVM: MMU: combine guest pte read between walk and pte prefetch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/05/2010 11:45 AM, Xiao Guangrong wrote:


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,

Why? VCPU1 will call kvm_mmu_pte_write (or invlpg) and establishes the HPTE. Then VCPU0 calls mmu_set_pte() and writes the old GPTE.

it also can broke
Point A's judgment and be checked by 'Point C'

Well, the 'continue' in point A means we skip the check.  That's not good.

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.

I'm not convinced we can bypass the checks.  Consider:


VCPU0                 VCPU1

#PF
walk_addr
-> gpml4e0,gpdpe0,gpde0,gpte0

                      replace gpdpe0 with gpdpe1
                      #PF
                      walk_addr
-> gpml4e0,gpdpe1,gpde1,gpte1
                      fetch
                      -> establish hpml4e0,hpdpte1,hpde0,hpte1
fetch
read hpdpe1
if (present(hpdpe1))
    continue;
...
write hpte0 using shadow hieratchy for hpte1

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

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux