From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sheng Yang Subject: Re: [PATCH 2/4] KVM: Add accessor for reading cr4 (or some bits of cr4) Date: Tue, 8 Dec 2009 15:57:42 +0800 Message-ID: <200912081557.42822.sheng@linux.intel.com> References: <1260182832-3974-1-git-send-email-avi@redhat.com> <1260182832-3974-3-git-send-email-avi@redhat.com> Mime-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm@vger.kernel.org To: Avi Kivity Return-path: Received: from mga06.intel.com ([134.134.136.21]:54465 "EHLO orsmga101.jf.intel.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1755657AbZLHH5p (ORCPT ); Tue, 8 Dec 2009 02:57:45 -0500 In-Reply-To: <1260182832-3974-3-git-send-email-avi@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Monday 07 December 2009 18:47:10 Avi Kivity wrote: > Some bits of cr4 can be owned by the guest on vmx, so when we read them, > we copy them to the vcpu structure. In preparation for making the set of > guest-owned bits dynamic, use helpers to access these bits so we don't need > to know where the bit resides. > > No changes to svm since all bits are host-owned there. > > Signed-off-by: Avi Kivity > --- > arch/x86/include/asm/kvm_host.h | 1 + > arch/x86/kvm/kvm_cache_regs.h | 12 ++++++++++++ > arch/x86/kvm/mmu.h | 5 +++-- > arch/x86/kvm/vmx.c | 13 ++++++++----- > arch/x86/kvm/x86.c | 16 ++++++---------- > 5 files changed, 30 insertions(+), 17 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h > b/arch/x86/include/asm/kvm_host.h index da6dee8..e9f4f12 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -272,6 +272,7 @@ struct kvm_vcpu_arch { > unsigned long cr2; > unsigned long cr3; > unsigned long cr4; > + unsigned long cr4_guest_owned_bits; > unsigned long cr8; > u32 hflags; > u64 pdptrs[4]; /* pae */ > diff --git a/arch/x86/kvm/kvm_cache_regs.h b/arch/x86/kvm/kvm_cache_regs.h > index 7bcc5b6..35acc36 100644 > --- a/arch/x86/kvm/kvm_cache_regs.h > +++ b/arch/x86/kvm/kvm_cache_regs.h > @@ -38,4 +38,16 @@ static inline u64 kvm_pdptr_read(struct kvm_vcpu *vcpu, > int index) return vcpu->arch.pdptrs[index]; > } > > +static inline ulong kvm_read_cr4_bits(struct kvm_vcpu *vcpu, ulong mask) > +{ > + if (mask & vcpu->arch.cr4_guest_owned_bits) > + kvm_x86_ops->decache_cr4_guest_bits(vcpu); > + return vcpu->arch.cr4 & mask; > +} > + > +static inline ulong kvm_read_cr4(struct kvm_vcpu *vcpu) > +{ > + return kvm_read_cr4_bits(vcpu, ~0UL); > +} > + > #endif > diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h > index 61a1b38..4567d80 100644 > --- a/arch/x86/kvm/mmu.h > +++ b/arch/x86/kvm/mmu.h > @@ -2,6 +2,7 @@ > #define __KVM_X86_MMU_H > > #include > +#include "kvm_cache_regs.h" > > #define PT64_PT_BITS 9 > #define PT64_ENT_PER_PAGE (1 << PT64_PT_BITS) > @@ -64,12 +65,12 @@ static inline int is_long_mode(struct kvm_vcpu *vcpu) > > static inline int is_pae(struct kvm_vcpu *vcpu) > { > - return vcpu->arch.cr4 & X86_CR4_PAE; > + return kvm_read_cr4_bits(vcpu, X86_CR4_PAE); > } > > static inline int is_pse(struct kvm_vcpu *vcpu) > { > - return vcpu->arch.cr4 & X86_CR4_PSE; > + return kvm_read_cr4_bits(vcpu, X86_CR4_PSE); > } > > static inline int is_paging(struct kvm_vcpu *vcpu) > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 5ef820e..ae95a0c 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -1612,8 +1612,10 @@ static void vmx_flush_tlb(struct kvm_vcpu *vcpu) > > static void vmx_decache_cr4_guest_bits(struct kvm_vcpu *vcpu) > { > - vcpu->arch.cr4 &= KVM_GUEST_CR4_MASK; > - vcpu->arch.cr4 |= vmcs_readl(GUEST_CR4) & ~KVM_GUEST_CR4_MASK; > + ulong cr4_guest_owned_bits = vcpu->arch.cr4_guest_owned_bits; > + > + vcpu->arch.cr4 &= ~cr4_guest_owned_bits; > + vcpu->arch.cr4 |= vmcs_readl(GUEST_CR4) & cr4_guest_owned_bits; > } > > static void ept_load_pdptrs(struct kvm_vcpu *vcpu) > @@ -1658,7 +1660,7 @@ static void ept_update_paging_mode_cr0(unsigned long > *hw_cr0, (CPU_BASED_CR3_LOAD_EXITING | > CPU_BASED_CR3_STORE_EXITING)); > vcpu->arch.cr0 = cr0; > - vmx_set_cr4(vcpu, vcpu->arch.cr4); > + vmx_set_cr4(vcpu, kvm_read_cr4(vcpu)); > } else if (!is_paging(vcpu)) { > /* From nonpaging to paging */ > vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, > @@ -1666,7 +1668,7 @@ static void ept_update_paging_mode_cr0(unsigned long > *hw_cr0, ~(CPU_BASED_CR3_LOAD_EXITING | > CPU_BASED_CR3_STORE_EXITING)); > vcpu->arch.cr0 = cr0; > - vmx_set_cr4(vcpu, vcpu->arch.cr4); > + vmx_set_cr4(vcpu, kvm_read_cr4(vcpu)); > } Another place accessed cr4 directly, in ept_update_paging_mode_cr4() > } else if (!(vcpu->arch.cr4 & X86_CR4_PAE)) > *hw_cr4 &= ~X86_CR4_PAE; Others looks fine to me. -- regards Yang, Sheng