Re: [PATCH v3 01/11] KVM: MMU: avoid pte_list_desc running out in kvm_mmu_pte_write
On 09/13/2011 06:50 PM, Avi Kivity wrote: > On 09/13/2011 01:24 PM, Xiao Guangrong wrote: >> On 09/13/2011 05:51 PM, Avi Kivity wrote: >> > On 08/30/2011 05:34 AM, Xiao Guangrong wrote: >> >> kvm_mmu_pte_write is unsafe since we need to alloc pte_list_desc in the >> >> function when spte is prefetched, unfortunately, we can not know how many >> >> spte need to be prefetched on this path, that means we can use out of the >> >> free pte_list_desc object in the cache, and BUG_ON() is triggered, also >> >> some >> >> path does not fill the cache, such as INS instruction emulated that does >> >> not >> >> trigger page fault >> >> >> >> @@ -3716,10 +3735,6 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, >> >> gva_t cr2, u32 error_code, >> >>goto out; >> >>} >> >> >> >> -r = mmu_topup_memory_caches(vcpu); >> >> -if (r) >> >> -goto out; >> >> - >> >>er = x86_emulate_instruction(vcpu, cr2, 0, insn, insn_len); >> >> >> > >> > Suppose we are out of memory, can't this get us in an endless loop? >> > >> > return -ENOMEM breaks as out (and kills the guest, likely). >> > >> >> If memory is not enough, we just clear sptes on pte_write path(not prefetch >> spte), >> the later page fault path can return -1 to let guest crash. Hmm? >> > > Yes. > > btw, is rmap_can_add() sufficent? We allocate more than just rmaps in > mmu_topup_memory_caches(). I guess it is, but this is getting tricky. > rmap_can_add() is used to avoid prefetching sptes more than the number of rmaps in the cache, because we do not know the exact number of sptes to be fetched. :-) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 01/11] KVM: MMU: avoid pte_list_desc running out in kvm_mmu_pte_write
On 09/13/2011 01:24 PM, Xiao Guangrong wrote: On 09/13/2011 05:51 PM, Avi Kivity wrote: > On 08/30/2011 05:34 AM, Xiao Guangrong wrote: >> kvm_mmu_pte_write is unsafe since we need to alloc pte_list_desc in the >> function when spte is prefetched, unfortunately, we can not know how many >> spte need to be prefetched on this path, that means we can use out of the >> free pte_list_desc object in the cache, and BUG_ON() is triggered, also some >> path does not fill the cache, such as INS instruction emulated that does not >> trigger page fault >> >> @@ -3716,10 +3735,6 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u32 error_code, >>goto out; >>} >> >> -r = mmu_topup_memory_caches(vcpu); >> -if (r) >> -goto out; >> - >>er = x86_emulate_instruction(vcpu, cr2, 0, insn, insn_len); >> > > Suppose we are out of memory, can't this get us in an endless loop? > > return -ENOMEM breaks as out (and kills the guest, likely). > If memory is not enough, we just clear sptes on pte_write path(not prefetch spte), the later page fault path can return -1 to let guest crash. Hmm? Yes. btw, is rmap_can_add() sufficent? We allocate more than just rmaps in mmu_topup_memory_caches(). I guess it is, but this is getting tricky. -- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 01/11] KVM: MMU: avoid pte_list_desc running out in kvm_mmu_pte_write
On 09/13/2011 05:51 PM, Avi Kivity wrote: > On 08/30/2011 05:34 AM, Xiao Guangrong wrote: >> kvm_mmu_pte_write is unsafe since we need to alloc pte_list_desc in the >> function when spte is prefetched, unfortunately, we can not know how many >> spte need to be prefetched on this path, that means we can use out of the >> free pte_list_desc object in the cache, and BUG_ON() is triggered, also some >> path does not fill the cache, such as INS instruction emulated that does not >> trigger page fault >> >> @@ -3716,10 +3735,6 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t >> cr2, u32 error_code, >> goto out; >> } >> >> -r = mmu_topup_memory_caches(vcpu); >> -if (r) >> -goto out; >> - >> er = x86_emulate_instruction(vcpu, cr2, 0, insn, insn_len); >> > > Suppose we are out of memory, can't this get us in an endless loop? > > return -ENOMEM breaks as out (and kills the guest, likely). > If memory is not enough, we just clear sptes on pte_write path(not prefetch spte), the later page fault path can return -1 to let guest crash. Hmm? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 01/11] KVM: MMU: avoid pte_list_desc running out in kvm_mmu_pte_write
On 08/30/2011 05:34 AM, Xiao Guangrong wrote: kvm_mmu_pte_write is unsafe since we need to alloc pte_list_desc in the function when spte is prefetched, unfortunately, we can not know how many spte need to be prefetched on this path, that means we can use out of the free pte_list_desc object in the cache, and BUG_ON() is triggered, also some path does not fill the cache, such as INS instruction emulated that does not trigger page fault @@ -3716,10 +3735,6 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u32 error_code, goto out; } - r = mmu_topup_memory_caches(vcpu); - if (r) - goto out; - er = x86_emulate_instruction(vcpu, cr2, 0, insn, insn_len); Suppose we are out of memory, can't this get us in an endless loop? return -ENOMEM breaks as out (and kills the guest, likely). -- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 01/11] KVM: MMU: avoid pte_list_desc running out in kvm_mmu_pte_write
kvm_mmu_pte_write is unsafe since we need to alloc pte_list_desc in the
function when spte is prefetched, unfortunately, we can not know how many
spte need to be prefetched on this path, that means we can use out of the
free pte_list_desc object in the cache, and BUG_ON() is triggered, also some
path does not fill the cache, such as INS instruction emulated that does not
trigger page fault
Signed-off-by: Xiao Guangrong
---
arch/x86/kvm/mmu.c | 25 -
1 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 5d7fbf0..b01afee 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -592,6 +592,11 @@ static int mmu_topup_memory_cache(struct
kvm_mmu_memory_cache *cache,
return 0;
}
+static int mmu_memory_cache_free_objects(struct kvm_mmu_memory_cache *cache)
+{
+ return cache->nobjs;
+}
+
static void mmu_free_memory_cache(struct kvm_mmu_memory_cache *mc,
struct kmem_cache *cache)
{
@@ -969,6 +974,14 @@ static unsigned long *gfn_to_rmap(struct kvm *kvm, gfn_t
gfn, int level)
return &linfo->rmap_pde;
}
+static bool rmap_can_add(struct kvm_vcpu *vcpu)
+{
+ struct kvm_mmu_memory_cache *cache;
+
+ cache = &vcpu->arch.mmu_pte_list_desc_cache;
+ return mmu_memory_cache_free_objects(cache);
+}
+
static int rmap_add(struct kvm_vcpu *vcpu, u64 *spte, gfn_t gfn)
{
struct kvm_mmu_page *sp;
@@ -3585,6 +3598,12 @@ void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
break;
}
+ /*
+* No need to care whether allocation memory is successful
+* or not since pte prefetch is skiped if it does not have
+* enough objects in the cache.
+*/
+ mmu_topup_memory_caches(vcpu);
spin_lock(&vcpu->kvm->mmu_lock);
if (atomic_read(&vcpu->kvm->arch.invlpg_counter) != invlpg_counter)
gentry = 0;
@@ -3655,7 +3674,7 @@ void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
mmu_page_zap_pte(vcpu->kvm, sp, spte);
if (gentry &&
!((sp->role.word ^ vcpu->arch.mmu.base_role.word)
- & mask.word))
+ & mask.word) && rmap_can_add(vcpu))
mmu_pte_write_new_pte(vcpu, sp, spte, &gentry);
if (!remote_flush && need_remote_flush(entry, *spte))
remote_flush = true;
@@ -3716,10 +3735,6 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2,
u32 error_code,
goto out;
}
- r = mmu_topup_memory_caches(vcpu);
- if (r)
- goto out;
-
er = x86_emulate_instruction(vcpu, cr2, 0, insn, insn_len);
switch (er) {
--
1.7.5.4
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
