Messages in this thread | | | Date | Sat, 03 Jul 2010 15:26:03 +0300 | From | Avi Kivity <> | Subject | Re: [PATCH v4 5/6] KVM: MMU: combine guest pte read between walk and pte prefetch |
| |
On 07/03/2010 03:16 PM, Xiao Guangrong wrote: > > Avi Kivity wrote: > >> 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? >> >> > I mean is: while we map the last level, we will directly set to the pfn but > the pfn is got by walk_addr, at this time, the guest mapping may be changed. > > What is the 'We already check the gpte' mean? i think i miss something :-( >
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 have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
| |