From: Paolo Bonzini <pbonzini@redhat.com>
To: guangrong.xiao@linux.intel.com
Cc: gleb@kernel.org, mtosatti@redhat.com, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/9] KVM: MTRR fixes and some cleanups
Date: Thu, 07 May 2015 18:53:31 +0200 [thread overview]
Message-ID: <554B988B.1000205@redhat.com> (raw)
In-Reply-To: <1430389490-24602-1-git-send-email-guangrong.xiao@linux.intel.com>
On 30/04/2015 12:24, guangrong.xiao@linux.intel.com wrote:
> From: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>
> This are some MTRR bugs if legacy IOMMU device is used on Intel's CPU:
> - In current code, whenever guest MTRR registers are changed
> kvm_mmu_reset_context is called to switch to the new root shadow page
> table, however, it's useless since:
> 1) the cache type is not cached into shadow page's attribute so that the
> original root shadow page will be reused
>
> 2) the cache type is set on the last spte, that means we should sync the
> last sptes when MTRR is changed
>
> We can fix it by dropping all the spte in the gfn range which is
> being updated by MTRR
>
> - some bugs are in get_mtrr_type();
> 1: bit 2 of mtrr_state->enabled is corresponding bit 11 of IA32_MTRR_DEF_TYPE
> MSR which completely control MTRR's enablement that means other bits are
> ignored if it is cleared
>
> 2: the fixed MTRR ranges are controlled by bit 1 of mtrr_state->enabled (bit
> 10 of IA32_MTRR_DEF_TYPE)
>
> 3: if MTRR is disabled, UC is applied to all of physical memory rather than
> mtrr_state->def_type
>
> - we need not to reset mmu once cache policy is changed since shadow page table
> does not virtualize any cache policy
>
> Also, these are some cleanups to make current MMU code more cleaner and help
> us fixing the bug more easier.
>
> Xiao Guangrong (9):
> KVM: MMU: fix decoding cache type from MTRR
> KVM: MMU: introduce slot_handle_level() and its helper
> KVM: MMU: use slot_handle_level and its helper to clean up the code
> KVM: MMU: introduce for_each_rmap_spte()
> KVM: MMU: KVM: introduce for_each_slot_rmap
> KVM: MMU: introduce kvm_zap_rmapp
> KVM: MMU: introduce kvm_zap_gfn_range()
> KVM: MMU: fix MTRR update
> KVM: x86: do not reset mmu if CR0.CD and CR0.NW are changed
>
> arch/x86/include/asm/kvm_host.h | 2 +
> arch/x86/kvm/mmu.c | 407 ++++++++++++++++++++++------------------
> arch/x86/kvm/mmu.h | 1 +
> arch/x86/kvm/mmu_audit.c | 4 +-
> arch/x86/kvm/svm.c | 5 +
> arch/x86/kvm/vmx.c | 58 ++++++
> arch/x86/kvm/x86.c | 5 +-
> 7 files changed, 294 insertions(+), 188 deletions(-)
>
Very nice cleanups. I made a couple comments on the patch ordering and
the API of iterator macros, but the basic ideas are great. I'm going to
apply patch 1, and do some testing with kvm_arch_has_noncoherent_dma
forced to return true.
Paolo
prev parent reply other threads:[~2015-05-07 16:53 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-30 10:24 [PATCH 0/9] KVM: MTRR fixes and some cleanups guangrong.xiao
2015-04-30 10:24 ` [PATCH 1/9] KVM: MMU: fix decoding cache type from MTRR guangrong.xiao
2015-04-30 10:24 ` [PATCH 2/9] KVM: MMU: introduce slot_handle_level() and its helper guangrong.xiao
2015-05-07 12:04 ` Paolo Bonzini
2015-05-11 13:00 ` Xiao Guangrong
2015-04-30 10:24 ` [PATCH 3/9] KVM: MMU: use slot_handle_level and its helper to clean up the code guangrong.xiao
2015-04-30 10:24 ` [PATCH 4/9] KVM: MMU: introduce for_each_rmap_spte() guangrong.xiao
2015-04-30 10:24 ` [PATCH 5/9] KVM: MMU: KVM: introduce for_each_slot_rmap guangrong.xiao
2015-04-30 10:24 ` [PATCH 6/9] KVM: MMU: introduce kvm_zap_rmapp guangrong.xiao
2015-04-30 10:24 ` [PATCH 7/9] KVM: MMU: introduce kvm_zap_gfn_range() guangrong.xiao
2015-04-30 10:24 ` [PATCH 8/9] KVM: MMU: fix MTRR update guangrong.xiao
2015-05-06 21:36 ` David Matlack
2015-05-07 1:57 ` Xiao Guangrong
2015-05-07 16:53 ` Paolo Bonzini
2015-05-11 13:02 ` Xiao Guangrong
2015-04-30 10:24 ` [PATCH 9/9] KVM: x86: do not reset mmu if CR0.CD and CR0.NW are changed guangrong.xiao
2015-04-30 10:24 ` [PATCH 0/9] KVM: MTRR fixes and some cleanups guangrong.xiao
2015-04-30 10:24 ` [PATCH 1/9] KVM: MMU: fix decoding cache type from MTRR guangrong.xiao
2015-05-06 21:42 ` David Matlack
2015-05-07 2:07 ` Xiao Guangrong
2015-04-30 10:24 ` [PATCH 2/9] KVM: MMU: introduce slot_handle_level() and its helper guangrong.xiao
2015-04-30 10:24 ` [PATCH 3/9] KVM: MMU: use slot_handle_level and its helper to clean up the code guangrong.xiao
2015-04-30 10:24 ` [PATCH 4/9] KVM: MMU: introduce for_each_rmap_spte() guangrong.xiao
2015-04-30 10:24 ` [PATCH 5/9] KVM: MMU: KVM: introduce for_each_slot_rmap guangrong.xiao
2015-04-30 10:24 ` [PATCH 6/9] KVM: MMU: introduce kvm_zap_rmapp guangrong.xiao
2015-04-30 10:24 ` [PATCH 7/9] KVM: MMU: introduce kvm_zap_gfn_range() guangrong.xiao
2015-04-30 10:24 ` [PATCH 8/9] KVM: MMU: fix MTRR update guangrong.xiao
2015-04-30 10:24 ` [PATCH 9/9] KVM: x86: do not reset mmu if CR0.CD and CR0.NW are changed guangrong.xiao
2015-05-07 16:53 ` Paolo Bonzini [this message]
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=554B988B.1000205@redhat.com \
--to=pbonzini@redhat.com \
--cc=gleb@kernel.org \
--cc=guangrong.xiao@linux.intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mtosatti@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).