From: Gleb Natapov <gleb@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH 2/7] KVM: mmu: remove uninteresting MMU "new_cr3" callbacks
Date: Thu, 3 Oct 2013 13:23:05 +0300 [thread overview]
Message-ID: <20131003102305.GC17294@redhat.com> (raw)
In-Reply-To: <1380725776-14948-3-git-send-email-pbonzini@redhat.com>
On Wed, Oct 02, 2013 at 04:56:11PM +0200, Paolo Bonzini wrote:
> The new_cr3 MMU callback has been a wrapper for mmu_free_roots since commit
> e676505 (KVM: MMU: Force cr3 reload with two dimensional paging on mov
> cr3 emulation, 2012-07-08).
>
> The commit message mentioned that "mmu_free_roots() is somewhat of an overkill,
> but fixing that is more complicated and will be done after this minimal fix".
> One year has passed, and no one really felt the need to do a different fix.
> Wrap the call with a kvm_mmu_new_cr3 function for clarity, but remove the
> callback.
>
Well the reason for it is because whoever knew about the problem found
other things to do meanwhile :) But do we really need something smarter
here anyway? The situation described in e676505 should be extremely rare
even on non unrestricted guest HW and it happens during slow emulation
anyway, so optimization will likely not be noticeable.
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> arch/x86/include/asm/kvm_host.h | 2 +-
> arch/x86/kvm/mmu.c | 13 +------------
> arch/x86/kvm/x86.c | 2 +-
> 3 files changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 3ea4d40..00a0e4c 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -253,7 +253,6 @@ struct kvm_pio_request {
> * mode.
> */
> struct kvm_mmu {
> - void (*new_cr3)(struct kvm_vcpu *vcpu);
> void (*set_cr3)(struct kvm_vcpu *vcpu, unsigned long root);
> unsigned long (*get_cr3)(struct kvm_vcpu *vcpu);
> u64 (*get_pdptr)(struct kvm_vcpu *vcpu, int index);
> @@ -921,6 +920,7 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu);
> int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t gva, u32 error_code,
> void *insn, int insn_len);
> void kvm_mmu_invlpg(struct kvm_vcpu *vcpu, gva_t gva);
> +void kvm_mmu_new_cr3(struct kvm_vcpu *vcpu);
>
> void kvm_enable_tdp(void);
> void kvm_disable_tdp(void);
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 1c4d580..dff856c 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -2570,11 +2570,6 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
> kvm_release_pfn_clean(pfn);
> }
>
> -static void nonpaging_new_cr3(struct kvm_vcpu *vcpu)
> -{
> - mmu_free_roots(vcpu);
> -}
> -
> static pfn_t pte_prefetch_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn,
> bool no_dirty_log)
> {
> @@ -3427,7 +3422,6 @@ out_unlock:
> static int nonpaging_init_context(struct kvm_vcpu *vcpu,
> struct kvm_mmu *context)
> {
> - context->new_cr3 = nonpaging_new_cr3;
> context->page_fault = nonpaging_page_fault;
> context->gva_to_gpa = nonpaging_gva_to_gpa;
> context->sync_page = nonpaging_sync_page;
> @@ -3448,9 +3442,8 @@ void kvm_mmu_flush_tlb(struct kvm_vcpu *vcpu)
> }
> EXPORT_SYMBOL_GPL(kvm_mmu_flush_tlb);
>
> -static void paging_new_cr3(struct kvm_vcpu *vcpu)
> +void kvm_mmu_new_cr3(struct kvm_vcpu *vcpu)
> {
> - pgprintk("%s: cr3 %lx\n", __func__, kvm_read_cr3(vcpu));
> mmu_free_roots(vcpu);
> }
>
> @@ -3666,7 +3659,6 @@ static int paging64_init_context_common(struct kvm_vcpu *vcpu,
> update_last_pte_bitmap(vcpu, context);
>
> ASSERT(is_pae(vcpu));
> - context->new_cr3 = paging_new_cr3;
> context->page_fault = paging64_page_fault;
> context->gva_to_gpa = paging64_gva_to_gpa;
> context->sync_page = paging64_sync_page;
> @@ -3694,7 +3686,6 @@ static int paging32_init_context(struct kvm_vcpu *vcpu,
> update_permission_bitmask(vcpu, context, false);
> update_last_pte_bitmap(vcpu, context);
>
> - context->new_cr3 = paging_new_cr3;
> context->page_fault = paging32_page_fault;
> context->gva_to_gpa = paging32_gva_to_gpa;
> context->sync_page = paging32_sync_page;
> @@ -3717,7 +3708,6 @@ static int init_kvm_tdp_mmu(struct kvm_vcpu *vcpu)
> struct kvm_mmu *context = vcpu->arch.walk_mmu;
>
> context->base_role.word = 0;
> - context->new_cr3 = nonpaging_new_cr3;
> context->page_fault = tdp_page_fault;
> context->sync_page = nonpaging_sync_page;
> context->invlpg = nonpaging_invlpg;
> @@ -3792,7 +3782,6 @@ int kvm_init_shadow_ept_mmu(struct kvm_vcpu *vcpu, struct kvm_mmu *context,
> context->shadow_root_level = kvm_x86_ops->get_tdp_level();
>
> context->nx = true;
> - context->new_cr3 = paging_new_cr3;
> context->page_fault = ept_page_fault;
> context->gva_to_gpa = ept_gva_to_gpa;
> context->sync_page = ept_sync_page;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 187f824..4a33fdb 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -684,7 +684,7 @@ int kvm_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
>
> vcpu->arch.cr3 = cr3;
> __set_bit(VCPU_EXREG_CR3, (ulong *)&vcpu->arch.regs_avail);
> - vcpu->arch.mmu.new_cr3(vcpu);
> + kvm_mmu_new_cr3(vcpu);
> return 0;
> }
> EXPORT_SYMBOL_GPL(kvm_set_cr3);
> --
> 1.8.3.1
>
--
Gleb.
next prev parent reply other threads:[~2013-10-03 10:23 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-02 14:56 [PATCH 0/7] KVM: x86: small MMU cleanups Paolo Bonzini
2013-10-02 14:56 ` [PATCH 1/7] KVM: mmu: remove uninteresting MMU "free" callbacks Paolo Bonzini
2013-10-02 14:56 ` [PATCH 2/7] KVM: mmu: remove uninteresting MMU "new_cr3" callbacks Paolo Bonzini
2013-10-03 10:23 ` Gleb Natapov [this message]
2013-10-03 11:56 ` Paolo Bonzini
2013-10-02 14:56 ` [PATCH 3/7] KVM: mmu: unify destroy_kvm_mmu with kvm_mmu_unload Paolo Bonzini
2013-10-02 14:56 ` [PATCH 4/7] KVM: mmu: change useless int return types to void Paolo Bonzini
2013-10-02 14:56 ` [PATCH 5/7] KVM: mmu: remove argument to kvm_init_shadow_mmu and kvm_init_shadow_ept_mmu Paolo Bonzini
2013-10-03 11:25 ` Gleb Natapov
2013-10-03 11:51 ` Paolo Bonzini
2013-10-03 12:11 ` Gleb Natapov
2013-10-02 14:56 ` [PATCH 6/7] KVM: mmu: remove ASSERT(vcpu) Paolo Bonzini
2013-10-02 14:56 ` [PATCH 7/7] KVM: mmu: replace assertions with MMU_WARN_ON, a conditional WARN_ON Paolo Bonzini
2013-10-03 12:46 ` [PATCH 0/7] KVM: x86: small MMU cleanups Gleb Natapov
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=20131003102305.GC17294@redhat.com \
--to=gleb@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.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;
as well as URLs for NNTP newsgroup(s).