KVM MMU: remove unused struct
PATCH 1/6 KVM MMU: remove unused struct
Remove 'struct kvm_unsync_walk' since it's not used now
--- arch/x86/kvm/mmu.c | 5 ----- 1 files changed, 0 insertions(+), 5 deletions(-)
arch/x86/kvm/mmu.c +0 -5
@@ -172,11 +172,6 @@ struct kvm_shadow_walk_iterator {shadow_walk_okay (&(_walker)); \ shadow_walk_next(&(_walker))) - -struct kvm_unsync_walk { -int (*entry) (struct kvm_mmu_page *sp,struct kvm_unsync_walk *walk); -}; -typedef int (*mmu_parent_walk_fn ) (struct kvm_vcpu *vcpu,struct kvm_mmu_page *sp);static struct kmem_cache *pte_chain_cache;
Signature · 1 line
1.6.1.2
PATCH 2/6 KVM MMU: fix kvm_mmu_zap_page() and its calling path
- calculate zapped page number properly in mmu_zap_unsync_children()
- calculate freeed page number properly kvm_mmu_change_mmu_pages()
- restart list walking if have children page zapped
--- arch/x86/kvm/mmu.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
arch/x86/kvm/mmu.c +4 -3
@@ -1483,8 +1483,8 @@ static int mmu_zap_unsync_children(struct kvm *kvm,for_each_sp (pages, sp, parents, i) {kvm_mmu_zap_page (kvm, sp);mmu_pages_clear_parents (&parents); + zapped++; } - zapped += pages.nr;kvm_mmu_pages_init (parent, &parents, &pages); }
@@ -1540,7 +1540,7 @@ void kvm_mmu_change_mmu_pages(struct kvm *kvm, unsigned int kvm_nr_mmu_pages) page =container_of (kvm->arch.active_mmu_pages.prev, struct kvm_mmu_page, link); -kvm_mmu_zap_page (kvm, page); + used_pages -=kvm_mmu_zap_page (kvm, page); used_pages--; } kvm->arch.n_free_mmu_pages =0 ;
@@ -1589,7 +1589,8 @@ static void mmu_unshadow(struct kvm *kvm, gfn_t gfn) && !sp->role.invalid) {pgprintk ("%s: zap %lx %x\n" , __func__, gfn, sp->role.word); -kvm_mmu_zap_page (kvm, sp); +if (kvm_mmu_zap_page (kvm, sp)) + nn = bucket->first; } } }
Signature · 1 line
1.6.1.2
Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path
Quoted 14 lines · Xiao Guangrong
- calculate zapped page number properly in
mmu_zap_unsync_children()
- calculate freeed page number properlykvm_mmu_change_mmu_pages()
- restart list walking if have children page zappedSigned-off-by Xiao Guangrong<[email protected]>--- arch/x86/kvm/mmu.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)arch/x86/kvm/mmu.c +0
-0
arch/x86/kvm/mmu.c static int mmu_zap_unsync_children(struct kvm *kvm, · line 1490
Quoted 9 lines
arch/x86/kvm/mmu.c +1
-1@@ -1483,8 +1483,8 @@ static int mmu_zap_unsync_children(struct kvm *kvm,for_each_sp (pages, sp, parents, i) {kvm_mmu_zap_page (kvm, sp);mmu_pages_clear_parents (&parents); + zapped++; } - zapped += pages.nr;kvm_mmu_pages_init (parent,&parents,&pages); }
This looks correct, I don't understand how we work in the first place. Marcelo?
arch/x86/kvm/mmu.c void kvm_mmu_change_mmu_pages(struct kvm *kvm, unsigned int kvm_nr_mmu_pages) · line 1546
Quoted 9 lines
arch/x86/kvm/mmu.c +1
-1@@ -1540,7 +1540,7 @@ void kvm_mmu_change_mmu_pages(struct kvm *kvm, unsigned int kvm_nr_mmu_pages) page =container_of (kvm->arch.active_mmu_pages.prev, struct kvm_mmu_page, link); -kvm_mmu_zap_page (kvm, page); + used_pages -=kvm_mmu_zap_page (kvm, page); used_pages--; }
This too. Wow.
arch/x86/kvm/mmu.c static void mmu_unshadow(struct kvm *kvm, gfn_t gfn) · line 1596
Quoted 11 lines
kvm->arch.n_free_mmu_pages = 0;arch/x86/kvm/mmu.c +2
-1@@ -1589,7 +1589,8 @@ static void mmu_unshadow(struct kvm *kvm, gfn_t gfn) && !sp->role.invalid) {pgprintk ("%s: zap %lx %x\n" , __func__, gfn, sp->role.word); -kvm_mmu_zap_page (kvm, sp); +if (kvm_mmu_zap_page (kvm, sp)) + nn = bucket->first; } }
I don't understand why this is needed.
Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path
Quoted 15 lines · Avi Kivity
arch/x86/kvm/mmu.c static void mmu_unshadow(struct kvm *kvm, gfn_t · line 1596
Quoted 13 lines
kvm->arch.n_free_mmu_pages = 0;arch/x86/kvm/mmu.c +2
-1@@ -1589,7 +1589,8 @@ static void mmu_unshadow(struct kvm *kvm, gfn_t gfn) && !sp->role.invalid) {pgprintk ("%s: zap %lx %x\n" , __func__, gfn, sp->role.word); -kvm_mmu_zap_page (kvm, sp); +if (kvm_mmu_zap_page (kvm, sp)) + nn = bucket->first; } }I don't understand why this is needed.
There is the code segment in mmu_unshadow():
|hlist_for_each_entry_safe(sp, node, nn, bucket, hash_link) {
| if (sp->gfn == gfn && !sp->role.direct
| && !sp->role.invalid) {
| pgprintk("%s: zap %lx %x\n",
| __func__, gfn, sp->role.word);
| kvm_mmu_zap_page(kvm, sp);
| }
| }in the loop, if nn is zapped, hlist_for_each_entry_safe() access nn will cause crash. and it's checked in other functions, such as kvm_mmu_zap_all(), kvm_mmu_unprotect_page()...
Thanks,
Xiao
Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path
Quoted 27 lines · Xiao Guangrong
Quoted 16 lines
Quoted 5 lines
kvm->arch.n_free_mmu_pages = 0;arch/x86/kvm/mmu.c static void mmu_unshadow(struct kvm *kvm, gfn_t · line 1596
Quoted 9 lines
arch/x86/kvm/mmu.c +2
-1@@ -1589,7 +1589,8 @@ static void mmu_unshadow(struct kvm *kvm, gfn_t gfn) && !sp->role.invalid) {pgprintk ("%s: zap %lx %x\n" , __func__, gfn, sp->role.word); -kvm_mmu_zap_page (kvm, sp); +if (kvm_mmu_zap_page (kvm, sp)) + nn = bucket->first; } }I don't understand why this is needed.
There is the code segment in
mmu_unshadow():|hlist_for_each_entry_safe(sp, node, nn, bucket, hash_link) { | if (sp->gfn == gfn&& !sp->role.direct | && !sp->role.invalid) { | pgprintk("%s: zap %lx %x\n", | __func__, gfn, sp->role.word); | kvm_mmu_zap_page(kvm, sp); | } | }
Quoted 4 lines
in the loop, if nn is zapped,
hlist_for_each_entry_safe()access nn will cause crash. and it's checked in other functions, such askvm_mmu_zap_all(),kvm_mmu_unprotect_page()...
hlist_for_each_entry_safe() is supposed to be be safe against removal of the element that is pointed to by the iteration cursor.
Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path
Hi Avi,
Quoted 2 lines · Avi Kivity
hlist_for_each_entry_safe()is supposed to be be safe against removal of the element that is pointed to by the iteration cursor.
If we destroyed the next point, hlist_for_each_entry_safe() is unsafe.
List hlist_for_each_entry_safe()'s code:
|#define hlist_for_each_entry_safe(tpos, pos, n, head, member) \
| for (pos = (head)->first; \
| pos && ({ n = pos->next; 1; }) && \
| ({ tpos = hlist_entry(pos, typeof(*tpos), member); 1;}); \
| pos = n)if n is destroyed:
'pos = n, n = pos->next'
then it access n again, it's unsafe/illegal for us.
Xiao
Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path
Quoted 18 lines · Xiao Guangrong
Hi Avi,
Quoted 3 lines · Avi Kivity
hlist_for_each_entry_safe()is supposed to be be safe against removal of the element that is pointed to by the iteration cursor.If we destroyed the next point,
hlist_for_each_entry_safe()is unsafe.List hlist_for_each_entry_safe()'s code:
|#define hlist_for_each_entry_safe(tpos, pos, n, head, member) \ | for (pos = (head)->first; \ | pos&& ({ n = pos->next; 1; })&& \ | ({ tpos = hlist_entry(pos, typeof(*tpos), member); 1;}); \ | pos = n)
Quoted 4 lines
if n is destroyed:
'pos = n, n =pos->next'
then it access n again, it's unsafe/illegal for us.
But kvm_mmu_zap_page() will only destroy sp == tpos == pos; n points at pos->next already, so it's safe.
Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path
Quoted 8 lines · Avi Kivity
Quoted 3 lines · Xiao Guangrong
Hi Avi,
Avi Kivity wrote:
But
kvm_mmu_zap_page()will only destroy sp == tpos == pos; n points atpos->nextalready, so it's safe.
kvm_mmu_zap_page(sp) not only zaps sp but also zaps all sp's unsync children pages, if n is just sp's unsyc child, just at the same hlist and just behind sp, it will crash. :-)
Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path
Quoted 9 lines · Xiao Guangrong
Quoted 5 lines
But
kvm_mmu_zap_page()will only destroy sp == tpos == pos; n points atpos->nextalready, so it's safe.
kvm_mmu_zap_page(sp)not only zaps sp but also zaps all sp's unsync children pages, if n is just sp's unsyc child, just at the same hlist and just behind sp, it will crash. :-)
Ouch. I see now, thanks for explaining.
One way to fix it is to make kvm_mmu_zap_page() only zap the page it is given, and use sp->role.invalid on its children. But it's better to fix it now quickly and do the more involved fixes later.
Just change the assignment to a 'goto restart;' please, I don't like playing with list_for_each internals.
Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path
In reply to Xiao GuangrongQuoted 14 lines · Xiao Guangrong
- calculate zapped page number properly in
mmu_zap_unsync_children()
- calculate freeed page number properlykvm_mmu_change_mmu_pages()
- restart list walking if have children page zappedSigned-off-by Xiao Guangrong <[email protected]>--- arch/x86/kvm/mmu.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)arch/x86/kvm/mmu.c +0
-0
arch/x86/kvm/mmu.c static int mmu_zap_unsync_children(struct kvm *kvm, · line 1489
Quoted 8 lines
arch/x86/kvm/mmu.c +1
-1@@ -1483,8 +1483,8 @@ static int mmu_zap_unsync_children(struct kvm *kvm,for_each_sp (pages, sp, parents, i) {kvm_mmu_zap_page (kvm, sp);mmu_pages_clear_parents (&parents); + zapped++; } - zapped += pages.nr;kvm_mmu_pages_init (parent, &parents, &pages); }
Don't see why this is needed? The for_each_sp loop uses pvec.nr.
arch/x86/kvm/mmu.c void kvm_mmu_change_mmu_pages(struct kvm *kvm, unsigned int kvm_nr_mmu_pages) · line 1546
Quoted 9 lines
arch/x86/kvm/mmu.c +1
-1@@ -1540,7 +1540,7 @@ void kvm_mmu_change_mmu_pages(struct kvm *kvm, unsigned int kvm_nr_mmu_pages) page =container_of (kvm->arch.active_mmu_pages.prev, struct kvm_mmu_page, link); -kvm_mmu_zap_page (kvm, page); + used_pages -=kvm_mmu_zap_page (kvm, page); used_pages--; } kvm->arch.n_free_mmu_pages =0 ;
Oops.
arch/x86/kvm/mmu.c static void mmu_unshadow(struct kvm *kvm, gfn_t gfn) · line 1593
Quoted 7 lines
arch/x86/kvm/mmu.c +2
-1@@ -1589,7 +1589,8 @@ static void mmu_unshadow(struct kvm *kvm, gfn_t gfn) && !sp->role.invalid) {pgprintk ("%s: zap %lx %x\n" , __func__, gfn, sp->role.word); -kvm_mmu_zap_page (kvm, sp); +if (kvm_mmu_zap_page (kvm, sp)) + nn = bucket->first;
Oops 2.
Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path
Quoted 11 lines · Marcelo Tosatti
arch/x86/kvm/mmu.c static int mmu_zap_unsync_children(struct kvm *kvm, · line 1489
Quoted 9 lines
arch/x86/kvm/mmu.c +1
-1@@ -1483,8 +1483,8 @@ static int mmu_zap_unsync_children(struct kvm *kvm,for_each_sp (pages, sp, parents, i) {kvm_mmu_zap_page (kvm, sp);mmu_pages_clear_parents (&parents); + zapped++; } - zapped += pages.nr;kvm_mmu_pages_init (parent, &parents, &pages); }Don't see why this is needed? The
for_each_sploop uses pvec.nr.
I think mmu_zap_unsync_children() should return the number of zapped pages then we can adjust the number of free pages in kvm_mmu_change_mmu_pages(), but pages.nr no only includes the unsync/zapped pages but also includes their parents.
Thanks,
Xiao
Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path
Quoted 16 lines · Xiao Guangrong
Quoted 11 lines · Marcelo Tosatti
arch/x86/kvm/mmu.c static int mmu_zap_unsync_children(struct kvm *kvm, · line 1489
Quoted 9 lines
arch/x86/kvm/mmu.c +1
-1@@ -1483,8 +1483,8 @@ static int mmu_zap_unsync_children(struct kvm *kvm,for_each_sp (pages, sp, parents, i) {kvm_mmu_zap_page (kvm, sp);mmu_pages_clear_parents (&parents); + zapped++; } - zapped += pages.nr;kvm_mmu_pages_init (parent, &parents, &pages); }Don't see why this is needed? The
for_each_sploop uses pvec.nr.
Quoted 3 lines
I think
mmu_zap_unsync_children()should return the number of zapped pages then we can adjust the number of free pages inkvm_mmu_change_mmu_pages(), but pages.nr no only includes the unsync/zapped pages but also includes their parents.
Oh i see. I think its safer to check for list_empty then to rely on proper accounting there, like __kvm_mmu_free_some_pages does.
Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path
Quoted 14 lines · Marcelo Tosatti
Quoted 13 lines · Xiao Guangrong
Quoted 10 lines · Marcelo Tosatti
arch/x86/kvm/mmu.c static int mmu_zap_unsync_children(struct kvm *kvm, · line 1489
Quoted 9 lines
arch/x86/kvm/mmu.c +1
-1@@ -1483,8 +1483,8 @@ static int mmu_zap_unsync_children(struct kvm *kvm,for_each_sp (pages, sp, parents, i) {kvm_mmu_zap_page (kvm, sp);mmu_pages_clear_parents (&parents); + zapped++; } - zapped += pages.nr;kvm_mmu_pages_init (parent, &parents, &pages); }Don't see why this is needed? The
for_each_sploop uses pvec.nr.
Quoted 6 lines
Quoted 3 lines
I think
mmu_zap_unsync_children()should return the number of zapped pages then we can adjust the number of free pages inkvm_mmu_change_mmu_pages(), but pages.nr no only includes the unsync/zapped pages but also includes their parents.Oh i see. I think its safer to check for
list_emptythen to rely on proper accounting there, like__kvm_mmu_free_some_pagesdoes.
Do you mean that we'd better add WARN_ON(list_empty()) code in kvm_mmu_zap_page()?
Thanks,
Xiao
Re: [PATCH 2/6] KVM MMU: fix kvm_mmu_zap_page() and its calling path
Quoted 17 lines · Xiao Guangrong
Quoted 14 lines · Marcelo Tosatti
Quoted 13 lines · Xiao Guangrong
Quoted 10 lines · Marcelo Tosatti
arch/x86/kvm/mmu.c static int mmu_zap_unsync_children(struct kvm *kvm, · line 1489
Quoted 9 lines
arch/x86/kvm/mmu.c +1
-1@@ -1483,8 +1483,8 @@ static int mmu_zap_unsync_children(struct kvm *kvm,for_each_sp (pages, sp, parents, i) {kvm_mmu_zap_page (kvm, sp);mmu_pages_clear_parents (&parents); + zapped++; } - zapped += pages.nr;kvm_mmu_pages_init (parent, &parents, &pages); }Don't see why this is needed? The
for_each_sploop uses pvec.nr.
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
I think
mmu_zap_unsync_children()should return the number of zapped pages then we can adjust the number of free pages inkvm_mmu_change_mmu_pages(), but pages.nr no only includes the unsync/zapped pages but also includes their parents.Oh i see. I think its safer to check for
list_emptythen to rely on proper accounting there, like__kvm_mmu_free_some_pagesdoes.Do you mean that we'd better add WARN_ON(list_empty()) code in
kvm_mmu_zap_page()?
Just break out of the loop iflist_empty(&vcpu->kvm->arch.active_mmu_pages).
PATCH 3/6 KVM MMU: optimize/cleanup for marking parent unsync
In reply to Xiao Guangrong- 'vcpu' is not used while mark parent unsync, so remove it
- if it has alread marked unsync, no need to walk it's parent
--- arch/x86/kvm/mmu.c | 69 +++++++++++++++++---------------------------------- 1 files changed, 23 insertions(+), 46 deletions(-)
arch/x86/kvm/mmu.c +23 -46
@@ -172,7 +172,7 @@ struct kvm_shadow_walk_iterator {shadow_walk_okay (&(_walker)); \ shadow_walk_next(&(_walker))) -typedef int (*mmu_parent_walk_fn ) (struct kvm_vcpu *vcpu,struct kvm_mmu_page *sp); +typedef int (*mmu_parent_walk_fn ) (struct kvm_mmu_page *sp,u64 *spte);static struct kmem_cache *pte_chain_cache;static struct kmem_cache *rmap_desc_cache;
@@ -1000,74 +1000,51 @@ static void mmu_page_remove_parent_pte(struct kvm_mmu_page *sp, } -static void mmu_parent_walk (struct kvm_vcpu *vcpu,struct kvm_mmu_page *sp, -mmu_parent_walk_fn fn) +static void mmu_parent_walk (struct kvm_mmu_page *sp,mmu_parent_walk_fn fn) {struct kvm_pte_chain *pte_chain;struct hlist_node *node;struct kvm_mmu_page *parent_sp;int i; -if (!sp->multimapped && sp->parent_pte) { +if (!sp->parent_pte) +return ; + +if (!sp->multimapped) { parent_sp =page_header (__pa (sp->parent_pte)); -fn (vcpu, parent_sp); -mmu_parent_walk (vcpu, parent_sp, fn); +if (fn (parent_sp, sp->parent_pte)) +mmu_parent_walk (parent_sp, fn);return ; } +hlist_for_each_entry (pte_chain, node, &sp->parent_ptes, link)for (i =0 ; i <NR_PTE_CHAIN_ENTRIES ; ++i) { -if (!pte_chain->parent_ptes[i]) +u64 *spte = pte_chain->parent_ptes[i]; +if (!spte)break ; - parent_sp =page_header (__pa (pte_chain->parent_ptes[i])); -fn (vcpu, parent_sp); -mmu_parent_walk (vcpu, parent_sp, fn); + parent_sp =page_header (__pa (spte)); +if (fn (parent_sp, spte)) +mmu_parent_walk (parent_sp, fn); } } -static void kvm_mmu_update_unsync_bitmap (u64 *spte) +static int mark_unsync (struct kvm_mmu_page *sp,u64 *spte) {unsigned int index; -struct kvm_mmu_page *sp =page_header (__pa (spte)); index = spte - sp->spt; -if (!__test_and_set_bit (index, sp->unsync_child_bitmap)) - sp->unsync_children++; -WARN_ON (!sp->unsync_children); -} - -static void kvm_mmu_update_parents_unsync (struct kvm_mmu_page *sp) -{ -struct kvm_pte_chain *pte_chain; -struct hlist_node *node; -int i; - -if (!sp->parent_pte) -return ; - -if (!sp->multimapped) { -kvm_mmu_update_unsync_bitmap (sp->parent_pte); -return ; - } +if (__test_and_set_bit (index, sp->unsync_child_bitmap)) +return 0 ; -hlist_for_each_entry (pte_chain, node, &sp->parent_ptes, link) -for (i =0 ; i <NR_PTE_CHAIN_ENTRIES ; ++i) { -if (!pte_chain->parent_ptes[i]) -break ; -kvm_mmu_update_unsync_bitmap (pte_chain->parent_ptes[i]); - } -} +if (sp->unsync_children++) +return 0 ; -static int unsync_walk_fn (struct kvm_vcpu *vcpu,struct kvm_mmu_page *sp) -{ -kvm_mmu_update_parents_unsync (sp);return 1 ; } -static void kvm_mmu_mark_parents_unsync (struct kvm_vcpu *vcpu, -struct kvm_mmu_page *sp) +static void kvm_mmu_mark_parents_unsync (struct kvm_mmu_page *sp) { -mmu_parent_walk (vcpu, sp, unsync_walk_fn); -kvm_mmu_update_parents_unsync (sp); +mmu_parent_walk (sp, mark_unsync); } static void nonpaging_prefetch_page(struct kvm_vcpu *vcpu,
@@ -1344,7 +1321,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,mmu_page_add_parent_pte (vcpu, sp, parent_pte);if (sp->unsync_children) {set_bit (KVM_REQ_MMU_SYNC , &vcpu->requests); -kvm_mmu_mark_parents_unsync (vcpu, sp); +kvm_mmu_mark_parents_unsync (sp); }trace_kvm_mmu_get_page (sp, false);return sp;
@@ -1756,7 +1733,7 @@ static int kvm_unsync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp) ++vcpu->kvm->stat.mmu_unsync; sp->unsync =1 ; -kvm_mmu_mark_parents_unsync (vcpu, sp); +kvm_mmu_mark_parents_unsync (sp);mmu_convert_notrap (sp);return 0 ;
Signature · 1 line
1.6.1.2
Re: [PATCH 3/6] KVM MMU: optimize/cleanup for marking parent unsync
Quoted 4 lines · Xiao Guangrong
- 'vcpu' is not used while mark parent unsync, so remove it
- if it has alread marked unsync, no need to walk it's parent
Please separate these two changes.
The optimization looks good. Perhaps it can be done even nicer using mutually recursive functions (mmu_parent_walk calls mmu_parent_walk_fn which calls mmu_parent_walk on the parent), but let's not change too many things at a time.
Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [PATCH 3/6] KVM MMU: optimize/cleanup for marking parent unsync
Quoted 6 lines · Avi Kivity
Quoted 4 lines · Xiao Guangrong
- 'vcpu' is not used while mark parent unsync, so remove it
- if it has alread marked unsync, no need to walk it's parent
Quoted 6 lines
Please separate these two changes.
The optimization looks good. Perhaps it can be done even nicer using mutually recursive functions (
mmu_parent_walkcallsmmu_parent_walk_fnwhich callsmmu_parent_walkon the parent), but let's not change too many things at a time.
OK, i'll separate it in the next version
Thanks,
Xiao
Re: [PATCH 3/6] KVM MMU: optimize/cleanup for marking parent unsync
In reply to Xiao GuangrongQuoted 4 lines · Xiao Guangrong
- 'vcpu' is not used while mark parent unsync, so remove it
- if it has alread marked unsync, no need to walk it's parentSigned-off-by Xiao Guangrong <[email protected]>
Xiao,
Did you actually see this codepath as being performance sensitive?
I'd prefer to not touch it.
Re: [PATCH 3/6] KVM MMU: optimize/cleanup for marking parent unsync
Quoted 3 lines · Marcelo Tosatti
Xiao,
Did you actually see this codepath as being performance sensitive?
Actually, i not run benchmarks to contrast the performance before this patch and after this patch.
Quoted 2 lines
I'd prefer to not touch it.
This patch avoids walk all parents and i think this overload is really unnecessary. It has other tricks in this codepath but i not noticed? :-)
Thanks,
Xiao
Re: [PATCH 3/6] KVM MMU: optimize/cleanup for marking parent unsync
Quoted 6 lines · Xiao Guangrong
Quoted 3 lines
I'd prefer to not touch it.
This patch avoids walk all parents and i think this overload is really unnecessary. It has other tricks in this codepath but i not noticed? :-)
There is also the advantage of code size reduction.
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [PATCH 3/6] KVM MMU: optimize/cleanup for marking parent unsync
In reply to Xiao GuangrongQuoted 8 lines · Xiao Guangrong
Quoted 3 lines · Marcelo Tosatti
Xiao,
Did you actually see this codepath as being performance sensitive?
Quoted 8 lines
Actually, i not run benchmarks to contrast the performance before this patch and after this patch.
Quoted 2 lines
I'd prefer to not touch it.
This patch avoids walk all parents and i think this overload is really unnecessary. It has other tricks in this codepath but i not noticed? :-)
My point is that there is no point in optimizing something unless its performance sensitive. And as i recall, mmu_unsync_walk was much more sensitive performance wise than parent walking. Actually, gfn_to_memslot seems more important since its also noticeable on EPT/NPT hosts.
Re: [PATCH 3/6] KVM MMU: optimize/cleanup for marking parent unsync
Quoted 6 lines · Marcelo Tosatti
Quoted 3 lines
Quoted 1 line
I'd prefer to not touch it.
This patch avoids walk all parents and i think this overload is really unnecessary. It has other tricks in this codepath but i not noticed? :-)
My point is that there is no point in optimizing something unless its performance sensitive.
Hi Marcelo,
I think optimizing not only means 'performance' but also means 'smaller code'(maybe 'cleanup' is more suitable) and 'logic optimize'(do little things), i'm not sure this patch whether can improve system performance obviously but it optimize the code logic and reduce code size, and it not harm other code and system performance, right? :-)
Actually, the origin code has a bug, the code segment in mmu_parent_walk():
| if (!sp->multimapped && sp->parent_pte) {
| ......
| return;
| }
| hlist_for_each_entry(pte_chain, node, &sp->parent_ptes, link)
| for (i = 0; i < NR_PTE_CHAIN_ENTRIES; ++i) {
| ......
| }So, if sp->parent_pte == NULL, it's unsafe...
Quoted 3 lines
And as i recall,
mmu_unsync_walkwas much more
sensitive performance wise than parent walking. Actually,gfn_to_memslotseems more important since its also noticeable on EPT/NPT hosts.
Yeah, i also noticed these and i'm looking into these code.
Thanks,
Xiao
Re: [PATCH 3/6] KVM MMU: optimize/cleanup for marking parent unsync
Quoted 12 lines · Xiao Guangrong
Actually, the origin code has a bug, the code segment in
mmu_parent_walk():| if (!sp->multimapped && sp->parent_pte) { | ...... | return; | } | hlist_for_each_entry(pte_chain, node, &sp->parent_ptes, link) | for (i = 0; i < NR_PTE_CHAIN_ENTRIES; ++i) { | ...... | }
Quoted 1 line
So, if
sp->parent_pte== NULL, it's unsafe...
Marcelo, please ignore this, it not a bug, just my mistake, sorry...
Re: [PATCH 3/6] KVM MMU: optimize/cleanup for marking parent unsync
In reply to Xiao GuangrongQuoted 11 lines · Xiao Guangrong
Quoted 6 lines · Marcelo Tosatti
Quoted 3 lines
Quoted 1 line
I'd prefer to not touch it.
This patch avoids walk all parents and i think this overload is really unnecessary. It has other tricks in this codepath but i not noticed? :-)
My point is that there is no point in optimizing something unless its performance sensitive.
Quoted 6 lines
Hi Marcelo,
I think optimizing not only means 'performance' but also means 'smaller code'(maybe 'cleanup' is more suitable) and 'logic optimize'(do little things), i'm not sure this patch whether can improve system performance obviously but it optimize the code logic and reduce code size, and it not harm other code and system performance, right? :-)
Right, but this walking code already is compact and stable. Removing the unused code variables/definitions is fine, but i'd prefer to not change the logic just for the sake of code reduction.
Quoted 11 lines
Actually, the origin code has a bug, the code segment in
mmu_parent_walk():| if (!sp->multimapped && sp->parent_pte) { | ...... | return; | } | hlist_for_each_entry(pte_chain, node, &sp->parent_ptes, link) | for (i = 0; i < NR_PTE_CHAIN_ENTRIES; ++i) { | ...... | }
Quoted 7 lines
So, if
sp->parent_pte== NULL, it's unsafe...Quoted 3 lines
And as i recall,
mmu_unsync_walkwas much more
sensitive performance wise than parent walking. Actually,gfn_to_memslotseems more important since its also noticeable on EPT/NPT hosts.Yeah, i also noticed these and i'm looking into these code.
Great.
PATCH 4/6 KVM MMU: optimize for writing cr4
In reply to Xiao GuangrongUsually, OS changes CR4.PGE bit to flush all global page, under this case, no need reset mmu and just flush tlb
--- arch/x86/kvm/x86.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
arch/x86/kvm/x86.c +9 -0
@@ -463,6 +463,15 @@ void kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)unsigned long old_cr4 =kvm_read_cr4 (vcpu);unsigned long pdptr_bits =X86_CR4_PGE |X86_CR4_PSE |X86_CR4_PAE ; +if (cr4 == old_cr4) +return ; + +if ((cr4 ^ old_cr4) ==X86_CR4_PGE ) { +kvm_mmu_sync_roots (vcpu); +kvm_mmu_flush_tlb (vcpu); +return ; + } +if (cr4 &CR4_RESERVED_BITS ) {kvm_inject_gp (vcpu,0 );return ;
Signature · 1 line
1.6.1.2
Re: [PATCH 4/6] KVM MMU: optimize for writing cr4
Quoted 19 lines · Xiao Guangrong
Usually, OS changes CR4.PGE bit to flush all global page, under this case, no need reset mmu and just flush tlb
Signed-off-by Xiao Guangrong<[email protected]>--- arch/x86/kvm/x86.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)arch/x86/kvm/x86.c +2
-0@@ -463,6 +463,15 @@ void kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)unsigned long old_cr4 =kvm_read_cr4 (vcpu);unsigned long pdptr_bits =X86_CR4_PGE |X86_CR4_PSE |X86_CR4_PAE ; +if (cr4 == old_cr4) +return ;
arch/x86/kvm/x86.c void kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4) · line 477
Quoted 10 lines
arch/x86/kvm/x86.c +7
-0+ +if ((cr4 ^ old_cr4) ==X86_CR4_PGE ) { +kvm_mmu_sync_roots (vcpu); +kvm_mmu_flush_tlb (vcpu); +return ; + } +if (cr4&CR4_RESERVED_BITS ) {kvm_inject_gp (vcpu,0 );return ;
Later we have:
Quoted 4 lines
kvm_x86_ops->set_cr4(vcpu, cr4); vcpu->arch.cr4 = cr4; vcpu->arch.mmu.base_role.cr4_pge = (cr4 & X86_CR4_PGE) &&!tdp_enabled;
All of which depend on cr4.
Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [PATCH 4/6] KVM MMU: optimize for writing cr4
Hi Avi,
Thanks for your comments.
Quoted 8 lines · Avi Kivity
Later we have:
Quoted 4 lines
kvm_x86_ops->set_cr4(vcpu, cr4); vcpu->arch.cr4 = cr4; vcpu->arch.mmu.base_role.cr4_pge = (cr4 & X86_CR4_PGE) &&!tdp_enabled;
All of which depend on cr4.
Oh, destroy_kvm_mmu() is not really destroyed cr3 and we can reload it later form shadow page cache, so, maybe this patch is unnecessary.
But, i have a another question here, why we need encode 'cr4 & X86_CR4_PGE' into base_role.cr4_gpe? Why we need allocation different shadow page for global page and no-global page?
As i know, global page is not static in TLB, and x86 cpu also may flush them form TLB, maybe we no need treat global page specially... Am i miss something? :-(
Xiao
Re: [PATCH 4/6] KVM MMU: optimize for writing cr4
Quoted 15 lines · Xiao Guangrong
Hi Avi,
Thanks for your comments.
Quoted 8 lines · Avi Kivity
Later we have:
Quoted 5 lines
kvm_x86_ops->set_cr4(vcpu, cr4); vcpu->arch.cr4 = cr4; vcpu->arch.mmu.base_role.cr4_pge = (cr4& X86_CR4_PGE)&&!tdp_enabled;
Quoted 9 lines
Quoted 2 lines
All of which depend on cr4.
Oh,
destroy_kvm_mmu()is not really destroyed cr3 and we can reload it later form shadow page cache, so, maybe this patch is unnecessary.But, i have a another question here, why we need encode 'cr4&
X86_CR4_PGE' into base_role.cr4_gpe? Why we need allocation different shadow page for global page and no-global page?
See 6364a3918cb. It was reverted later due to a problem with the implementation. I'm not sure whether I want to fix the bug and restore that patch, or to drop it altogether and give the guest ownership of cr4.pge. See cr4_guest_owned_bits (currently only used on ept).
Quoted 3 lines
As i know, global page is not static in TLB, and x86 cpu also may flush them form TLB, maybe we no need treat global page specially... Am i miss something? :-(
You can't read reverted patches? :)
Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [PATCH 4/6] KVM MMU: optimize for writing cr4
Quoted 5 lines · Avi Kivity
See
6364a3918cb. It was reverted later due to a problem with the implementation. I'm not sure whether I want to fix the bug and restore that patch, or to drop it altogether and give the guest ownership of cr4.pge. Seecr4_guest_owned_bits(currently only used on ept).
Oh, i see, thanks very much.
Quoted 6 lines
Quoted 4 lines
As i know, global page is not static in TLB, and x86 cpu also may flush them form TLB,
maybe we no need treat global page specially... Am i miss something? :-(You can't read reverted patches? :)
I usually use 'get blame' to look into source, and not noticed reverted patches, i'll pay more attention on those.
Below code still confused me:
| vcpu->arch.mmu.base_role.cr4_pge = (cr4& X86_CR4_PGE)&&!tdp_enabled;
And i found the commit 87778d60ee:
| KVM: MMU: Segregate mmu pages created with different cr4.pge settings | | Don't allow a vcpu with cr4.pge cleared to use a shadow page created with | cr4.pge set; this might cause a cr3 switch not to sync ptes that have the | global bit set (the global bit has no effect if !cr4.pge). | | This can only occur on smp with different cr4.pge settings for different | vcpus (since a cr4 change will resync the shadow ptes), but there's no | cost to being correct here.
In current code, cr3 switch will sync all unsync shadow pages(regardless it's global or not) and this issue not live now, so, do we need also revert this patch?
Xiao
Re: [PATCH 4/6] KVM MMU: optimize for writing cr4
Quoted 13 lines · Xiao Guangrong
And i found the commit
87778d60ee:| KVM: MMU: Segregate mmu pages created with different cr4.pge settings | | Don't allow a vcpu with cr4.pge cleared to use a shadow page created with | cr4.pge set; this might cause a cr3 switch not to sync ptes that have the | global bit set (the global bit has no effect if !cr4.pge). | | This can only occur on smp with different cr4.pge settings for different | vcpus (since a cr4 change will resync the shadow ptes), but there's no | cost to being correct here.
Quoted 4 lines
In current code, cr3 switch will sync all unsync shadow pages(regardless it's global or not) and this issue not live now, so, do we need also revert this patch?
One path is to revert this patch. The other is to restore the optimization that relies on it. I'm not sure which is best.
Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
PATCH 5/6 KVM MMU: reduce kvm_mmu_page size
In reply to Xiao Guangrong'multimapped' and 'unsync' in 'struct kvm_mmu_page' are just indication field, we can use flag bits instand of them
--- arch/x86/include/asm/kvm_host.h | 5 ++- arch/x86/kvm/mmu.c | 65 ++++++++++++++++++++++++++++----------- arch/x86/kvm/mmutrace.h | 7 ++-- arch/x86/kvm/paging_tmpl.h | 2 +- 4 files changed, 55 insertions(+), 24 deletions(-)
arch/x86/include/asm/kvm_host.h +3 -2
@@ -202,9 +202,10 @@ struct kvm_mmu_page {* in this shadow page. */ DECLARE_BITMAP (slot_bitmap,KVM_MEMORY_SLOTS +KVM_PRIVATE_MEM_SLOTS ); -int multimapped;/* More than one parent_pte? */ int root_count;/* Currently serving as active root */ -bool unsync; +#define MMU_PAGE_MULTIMAPPED 0x1/* More than one parent_pte? */ +#define MMU_PAGE_UNSYNC 0x2 +unsigned int flags;unsigned int unsync_children;union {u64 *parent_pte;/* !multimapped */
arch/x86/kvm/mmu.c +47 -18
161 lines not on this page · raw message
arch/x86/kvm/mmutrace.h +4 -3
@@ -11,13 +11,13 @@ __field(__u64, gfn) \ __field(__u32, role) \ __field(__u32, root_count) \ - __field(__u32, unsync) + __field(__u32, flags)#define KVM_MMU_PAGE_ASSIGN (sp) \ __entry->gfn = sp->gfn; \ __entry->role = sp->role.word; \ __entry->root_count = sp->root_count; \ - __entry->unsync = sp->unsync; + __entry->flags = sp->flags;#define KVM_MMU_PAGE_PRINTK () ({ \ const char *ret = p->buffer + p->len; \
@@ -38,7 +38,8 @@ role.cr4_pge ?"" :"!" , \ role.nxe ?"" :"!" , \ __entry->root_count, \ - __entry->unsync ? "unsync" : "sync", 0); \ + __entry->flags &MMU_PAGE_UNSYNC ? \ + "unsync" : "sync", 0); \ ret; \ })
arch/x86/kvm/paging_tmpl.h +1 -1
@@ -263,7 +263,7 @@ static void FNAME(update_pte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *page, gpte = *(const pt_element_t *)pte;if (~gpte & (PT_PRESENT_MASK |PT_ACCESSED_MASK )) {if (!is_present_gpte (gpte)) { -if (page->unsync) +if (mmu_page_is_unsync (page)) new_spte = shadow_trap_nonpresent_pte;else new_spte = shadow_notrap_nonpresent_pte;
Signature · 1 line
1.6.1.2
Re: [PATCH 5/6] KVM MMU: reduce kvm_mmu_page size
Quoted 6 lines · Xiao Guangrong
'multimapped' and 'unsync' in 'struct
kvm_mmu_page' are just indication field, we can use flag bits instand of themarch/x86/include/asm/kvm_host.h +0
-0@@ -202,9 +202,10 @@ struct kvm_mmu_page {* in this shadow page. */
arch/x86/include/asm/kvm_host.h struct kvm_mmu_page { · line 211
Quoted 10 lines
arch/x86/include/asm/kvm_host.h +3
-2DECLARE_BITMAP (slot_bitmap,KVM_MEMORY_SLOTS +KVM_PRIVATE_MEM_SLOTS ); -int multimapped;/* More than one parent_pte? */ int root_count;/* Currently serving as active root */ -bool unsync; +#define MMU_PAGE_MULTIMAPPED 0x1/* More than one parent_pte? */ +#define MMU_PAGE_UNSYNC 0x2 +unsigned int flags;unsigned int unsync_children;union {
Please just use bool for multimapped. If we grow more than 4 flags, we can use 'bool flag : 1;' to reduce space while retaining readability.
Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [PATCH 5/6] KVM MMU: reduce kvm_mmu_page size
Quoted 14 lines · Avi Kivity
Quoted 13 lines · Xiao Guangrong
'multimapped' and 'unsync' in 'struct
kvm_mmu_page' are just indication field, we can use flag bits instand of themarch/x86/include/asm/kvm_host.h +1
-2@@ -202,9 +202,10 @@ struct kvm_mmu_page {* in this shadow page. */ DECLARE_BITMAP (slot_bitmap,KVM_MEMORY_SLOTS +KVM_PRIVATE_MEM_SLOTS ); -int multimapped;/* More than one parent_pte? */ int root_count;/* Currently serving as active root */ -bool unsync; +#define MMU_PAGE_MULTIMAPPED 0x1/* More than one
parent_pte? */
Quoted 9 lines
arch/x86/include/asm/kvm_host.h struct kvm_mmu_page { · line 210
Quoted 5 lines
arch/x86/include/asm/kvm_host.h +2
-0+#define MMU_PAGE_UNSYNC 0x2 +unsigned int flags;unsigned int unsync_children;union {Please just use bool for multimapped. If we grow more than 4 flags, we can use 'bool flag : 1;' to reduce space while retaining readability.
Yeah, good idea, i'll fix it in the next version
Thanks,
Xiao
PATCH 6/6 KVM MMU: optimize synchronization shadow pages
In reply to Xiao Guangrong- chain all unsync shadow pages then we can fetch them quickly
- flush local/remote tlb after all shadow page synced
--- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/mmu.c | 82 ++++++++++++++++++--------------------- 2 files changed, 39 insertions(+), 44 deletions(-)
arch/x86/include/asm/kvm_host.h +1 -0
@@ -207,6 +207,7 @@ struct kvm_mmu_page {#define MMU_PAGE_UNSYNC 0x2unsigned int flags;unsigned int unsync_children; +struct list_head unsync_link;union {u64 *parent_pte;/* !multimapped */ struct hlist_head parent_ptes;/* multimapped, kvm_pte_chain */
arch/x86/kvm/mmu.c +38 -44
142 lines not on this page · raw message
Signature · 1 line
1.6.1.2
Re: [PATCH 6/6] KVM MMU: optimize synchronization shadow pages
Quoted 26 lines · Xiao Guangrong
- chain all unsync shadow pages then we can fetch them quickly
- flush local/remote tlb after all shadow page syncedSigned-off-by Xiao Guangrong<[email protected]>--- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/mmu.c | 82 ++++++++++++++++++--------------------- 2 files changed, 39 insertions(+), 44 deletions(-)arch/x86/include/asm/kvm_host.h +1
-0@@ -207,6 +207,7 @@ struct kvm_mmu_page {#define MMU_PAGE_UNSYNC 0x2unsigned int flags;unsigned int unsync_children; +struct list_head unsync_link;union {u64 *parent_pte;/* !multimapped */ struct hlist_head parent_ptes;/* multimapped, kvm_pte_chain */ arch/x86/kvm/mmu.c +0
-0
arch/x86/kvm/mmu.c typedef int (*mmu_parent_walk_fn) (struct kvm_mmu_page *sp, u64 *spte); · line 183
Quoted 7 lines
arch/x86/kvm/mmu.c +2
-0@@ -177,6 +177,8 @@ typedef int (*mmu_parent_walk_fn) (struct kvm_mmu_page *sp, u64 *spte);static struct kmem_cache *pte_chain_cache;static struct kmem_cache *rmap_desc_cache;static struct kmem_cache *mmu_page_header_cache; +static struct list_head unsync_mmu_page_list = +LIST_HEAD_INIT (unsync_mmu_page_list);
Does this really need to be global? Should be per guest.
Quoted 11 lines
arch/x86/kvm/mmu.c +0
-0static u64 __read_mostly shadow_trap_nonpresent_pte;static u64
__read_mostlyshadow_notrap_nonpresent_pte;arch/x86/kvm/mmu.c +1
-0@@ -950,6 +952,7 @@ static struct kvm_mmu_page *kvm_mmu_alloc_page(struct kvm_vcpu *vcpu,bitmap_zero (sp->slot_bitmap,KVM_MEMORY_SLOTS +KVM_PRIVATE_MEM_SLOTS ); sp->flags =0 ; sp->parent_pte = parent_pte; +INIT_LIST_HEAD (&sp->unsync_link); --vcpu->kvm->arch.n_free_mmu_pages;return sp; }
arch/x86/kvm/mmu.c static void kvm_unlink_unsync_page(struct kvm *kvm, struct kvm_mmu_page *sp) · line 1209
Quoted 7 lines
arch/x86/kvm/mmu.c +1
-0@@ -1200,12 +1203,14 @@ static void kvm_unlink_unsync_page(struct kvm *kvm, struct kvm_mmu_page *sp) {WARN_ON (!mmu_page_is_unsync (sp));mmu_page_clear_unsync (sp); +list_del (&sp->unsync_link); --kvm->stat.mmu_unsync; }
Locking?
arch/x86/kvm/mmu.c static void kvm_mmu_pages_init(struct kvm_mmu_page *parent, · line 1313
Quoted 5 lines
arch/x86/kvm/mmu.c +2
-2-if (protected) -kvm_flush_remote_tlbs (vcpu->kvm); +if (list_empty (&unsync_mmu_page_list)) +return ;
Will this ever happen? In my experience we usually have a ton of unsyncs lying around.
arch/x86/kvm/mmu.c static void kvm_mmu_pages_init(struct kvm_mmu_page *parent, · line 1318
Quoted 6 lines
arch/x86/kvm/mmu.c +4
-0+list_for_each_safe (p, next,&unsync_mmu_page_list) { +struct kvm_mmu_page *sp; + sp = list_entry(p, struct kvm_mmu_page, unsync_link); +__kvm_sync_page (vcpu, sp,&flush_local_tlb,&flush_remote_tlb); }
That's counter to the point of unsync. We only want to sync the minimum number of pages - all pages reachable by the new root. The others we want to keep writeable to reduce our fault count.
Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [PATCH 6/6] KVM MMU: optimize synchronization shadow pages
Quoted 31 lines · Avi Kivity
Quoted 30 lines · Xiao Guangrong
- chain all unsync shadow pages then we can fetch them quickly
- flush local/remote tlb after all shadow page syncedSigned-off-by Xiao Guangrong<[email protected]>--- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/mmu.c | 82 ++++++++++++++++++--------------------- 2 files changed, 39 insertions(+), 44 deletions(-)arch/x86/include/asm/kvm_host.h +1
-0@@ -207,6 +207,7 @@ struct kvm_mmu_page {#define MMU_PAGE_UNSYNC 0x2unsigned int flags;unsigned int unsync_children; +struct list_head unsync_link;union {u64 *parent_pte;/* !multimapped */ struct hlist_head parent_ptes;/* multimapped, kvm_pte_chain */ arch/x86/kvm/mmu.c +0
-0@@ -177,6 +177,8 @@ typedef int (*mmu_parent_walk_fn) (struct kvm_mmu_page *sp, u64 *spte);static struct kmem_cache *pte_chain_cache;
Quoted 9 lines
arch/x86/kvm/mmu.c typedef int (*mmu_parent_walk_fn) (struct kvm_mmu_page *sp, u64 *spte);
Quoted 6 lines
arch/x86/kvm/mmu.c +2
-0static struct kmem_cache *rmap_desc_cache;static struct kmem_cache *mmu_page_header_cache; +static struct list_head unsync_mmu_page_list = +LIST_HEAD_INIT (unsync_mmu_page_list);Does this really need to be global? Should be per guest.
Ah... this patch need more cooking, i'll improve it... thanks for you point it out.
Xiao