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/03/2010 01:31 PM, Xiao Guangrong wrote:

See how the pte is reread inside fetch with mmu_lock held.

It looks like something is broken in 'fetch' functions, this patch will
fix it.

Subject: [PATCH] KVM: MMU: fix last level broken in FNAME(fetch)

We read the guest level out of 'mmu_lock', sometimes, the host mapping is
confusion. Consider this case:

VCPU0:                                              VCPU1

Read guest mapping, assume the mapping is:
GLV3 ->  GLV2 ->  GLV1 ->  GFNA,
And in the host, the corresponding mapping is
HLV3 ->  HLV2 ->  HLV1(P=0)

                                                    Write GLV1 and cause the
                                                    mapping point to GFNB
                                                    (May occur in pte_write or
                                                       invlpg path)

Mapping GLV1 to GFNA

This issue only occurs in the last indirect mapping, since if the middle
mapping is changed, the mapping will be zapped, then it will be detected
in the FNAME(fetch) path, but when it map the last level, it not checked.

Fixed by also check the last level.


I don't really see what is fixed. We already check the gpte. What's special about the new scenario?

@@ -322,6 +334,12 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
  		level = iterator.level;
  		sptep = iterator.sptep;
  		if (iterator.level == hlevel) {
+			if (check&&  level == gw->level&&
+			      !FNAME(check_level_mapping)(vcpu, gw, hlevel)) {
+				kvm_release_pfn_clean(pfn);
+				break;
+			}
+

Now we check here...

  			mmu_set_spte(vcpu, sptep, access,
  				     gw->pte_access&  access,
  				     user_fault, write_fault,
@@ -376,10 +394,10 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
  		sp = kvm_mmu_get_page(vcpu, table_gfn, addr, level-1,
  					       direct, access, sptep);
  		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]) {
+			if (hlevel == level - 1)
+				check = false;
+
+			if (!FNAME(check_level_mapping)(vcpu, gw, level - 1)) {

... and here?  Why?


(looking at the code, we have a call to kvm_host_page_size() on every page fault, that takes mmap_sem... that's got to impact scaling)

--
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 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