From: Marcelo Tosatti <mtosatti@redhat.com>
To: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
Cc: Avi Kivity <avi@redhat.com>, KVM list <kvm@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 6/10] KVM MMU: don't write-protect if have new mapping to unsync page
Date: Thu, 22 Apr 2010 16:29:47 -0300 [thread overview]
Message-ID: <20100422192947.GA2280@amt.cnet> (raw)
In-Reply-To: <4BCFE8F0.7010509@cn.fujitsu.com>
On Thu, Apr 22, 2010 at 02:13:04PM +0800, Xiao Guangrong wrote:
> If have new mapping to the unsync page(i.e, add a new parent), just
> update the page from sp->gfn but not write-protect gfn, and if need
> create new shadow page form sp->gfn, we should sync it
>
> Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
> ---
> arch/x86/kvm/mmu.c | 27 +++++++++++++++++++--------
> 1 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index fd027a6..8607a64 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -1196,16 +1196,20 @@ static void kvm_unlink_unsync_page(struct kvm *kvm, struct kvm_mmu_page *sp)
>
> static int kvm_mmu_zap_page(struct kvm *kvm, struct kvm_mmu_page *sp);
>
> -static int kvm_sync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp)
> +static int kvm_sync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
> + bool clear_unsync)
> {
> if (sp->role.cr4_pae != !!is_pae(vcpu)) {
> kvm_mmu_zap_page(vcpu->kvm, sp);
> return 1;
> }
>
> - if (rmap_write_protect(vcpu->kvm, sp->gfn))
> - kvm_flush_remote_tlbs(vcpu->kvm);
> - kvm_unlink_unsync_page(vcpu->kvm, sp);
> + if (clear_unsync) {
> + if (rmap_write_protect(vcpu->kvm, sp->gfn))
> + kvm_flush_remote_tlbs(vcpu->kvm);
> + kvm_unlink_unsync_page(vcpu->kvm, sp);
> + }
> +
> if (vcpu->arch.mmu.sync_page(vcpu, sp)) {
> kvm_mmu_zap_page(vcpu->kvm, sp);
> return 1;
> @@ -1293,7 +1297,7 @@ static void mmu_sync_children(struct kvm_vcpu *vcpu,
> kvm_flush_remote_tlbs(vcpu->kvm);
>
> for_each_sp(pages, sp, parents, i) {
> - kvm_sync_page(vcpu, sp);
> + kvm_sync_page(vcpu, sp, true);
> mmu_pages_clear_parents(&parents);
> }
> cond_resched_lock(&vcpu->kvm->mmu_lock);
> @@ -1313,7 +1317,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
> unsigned index;
> unsigned quadrant;
> struct hlist_head *bucket;
> - struct kvm_mmu_page *sp;
> + struct kvm_mmu_page *sp, *unsync_sp = NULL;
> struct hlist_node *node, *tmp;
>
> role = vcpu->arch.mmu.base_role;
> @@ -1332,12 +1336,16 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
> hlist_for_each_entry_safe(sp, node, tmp, bucket, hash_link)
> if (sp->gfn == gfn) {
> if (sp->unsync)
> - if (kvm_sync_page(vcpu, sp))
> - continue;
> + unsync_sp = sp;
Xiao,
I don't see a reason why you can't create a new mapping to an unsync
page. The code already creates shadow pte entries using unsync
pagetables.
So all you need would be to kvm_sync_pages before write protecting.
Also make sure kvm_sync_pages is in place here before enabling multiple
unsync shadows, in the patch series.
>
> if (sp->role.word != role.word)
> continue;
>
> + if (unsync_sp && kvm_sync_page(vcpu, unsync_sp, false)) {
> + unsync_sp = NULL;
> + continue;
> + }
> +
> mmu_page_add_parent_pte(vcpu, sp, parent_pte);
> if (sp->unsync_children) {
> set_bit(KVM_REQ_MMU_SYNC, &vcpu->requests);
> @@ -1346,6 +1354,9 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
> trace_kvm_mmu_get_page(sp, false);
> return sp;
> }
> + if (unsync_sp)
> + kvm_sync_page(vcpu, unsync_sp, true);
> +
> ++vcpu->kvm->stat.mmu_cache_miss;
> sp = kvm_mmu_alloc_page(vcpu, parent_pte);
> if (!sp)
> --
> 1.6.1.2
>
next prev parent reply other threads:[~2010-04-22 19:29 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <4BCFE581.8050305@cn.fujitsu.com>
2010-04-22 6:12 ` [PATCH 5/10] KVM MMU: cleanup invlpg code Xiao Guangrong
2010-04-22 6:13 ` [PATCH 6/10] KVM MMU: don't write-protect if have new mapping to unsync page Xiao Guangrong
2010-04-22 19:29 ` Marcelo Tosatti [this message]
2010-04-23 3:35 ` Xiao Guangrong
2010-04-23 11:35 ` Avi Kivity
2010-04-22 6:13 ` [PATCH 7/10] KVM MMU: allow more page become unsync at gfn mapping time Xiao Guangrong
2010-04-22 6:13 ` [PATCH 8/10] KVM MMU: allow more page become unsync at getting sp time Xiao Guangrong
2010-04-23 12:08 ` Avi Kivity
2010-04-22 6:13 ` [PATCH 9/10] KVM MMU: separate invlpg code form kvm_mmu_pte_write() Xiao Guangrong
2010-04-22 6:14 ` [PATCH 10/10] KVM MMU: optimize sync/update unsync-page Xiao Guangrong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100422192947.GA2280@amt.cnet \
--to=mtosatti@redhat.com \
--cc=avi@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=xiaoguangrong@cn.fujitsu.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox