* [PATCH 0/4] cr4 optimizations for vmx/ept
@ 2009-12-07 10:47 Avi Kivity
2009-12-07 10:47 ` [PATCH 1/4] KVM: VMX: Move some cr[04] related constants to vmx.c Avi Kivity
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Avi Kivity @ 2009-12-07 10:47 UTC (permalink / raw)
To: Marcelo Tosatti, Sheng Yang; +Cc: kvm
When ept is enabled, we aren't particularly interested in cr4.pge, so allow
the guest to own it. This improves performance in vmap() intensive loads.
Avi Kivity (4):
KVM: VMX: Move some cr[04] related constants to vmx.c
KVM: Add accessor for reading cr4 (or some bits of cr4)
KVM: VMX: Make guest cr4 mask more conservative
KVM: VMX: When using ept, allow the guest to own cr4.pge
arch/x86/include/asm/kvm_host.h | 14 +-------------
arch/x86/kvm/kvm_cache_regs.h | 12 ++++++++++++
arch/x86/kvm/mmu.h | 5 +++--
arch/x86/kvm/vmx.c | 32 ++++++++++++++++++++++++++------
arch/x86/kvm/x86.c | 16 ++++++----------
5 files changed, 48 insertions(+), 31 deletions(-)
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/4] KVM: VMX: Move some cr[04] related constants to vmx.c 2009-12-07 10:47 [PATCH 0/4] cr4 optimizations for vmx/ept Avi Kivity @ 2009-12-07 10:47 ` Avi Kivity 2009-12-07 10:47 ` [PATCH 2/4] KVM: Add accessor for reading cr4 (or some bits of cr4) Avi Kivity ` (3 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Avi Kivity @ 2009-12-07 10:47 UTC (permalink / raw) To: Marcelo Tosatti, Sheng Yang; +Cc: kvm They have no place in common code. Signed-off-by: Avi Kivity <avi@redhat.com> --- arch/x86/include/asm/kvm_host.h | 13 ------------- arch/x86/kvm/vmx.c | 13 +++++++++++++ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 4f865e8..da6dee8 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -38,19 +38,6 @@ #define CR3_L_MODE_RESERVED_BITS (CR3_NONPAE_RESERVED_BITS | \ 0xFFFFFF0000000000ULL) -#define KVM_GUEST_CR0_MASK_UNRESTRICTED_GUEST \ - (X86_CR0_WP | X86_CR0_NE | X86_CR0_NW | X86_CR0_CD) -#define KVM_GUEST_CR0_MASK \ - (KVM_GUEST_CR0_MASK_UNRESTRICTED_GUEST | X86_CR0_PG | X86_CR0_PE) -#define KVM_VM_CR0_ALWAYS_ON_UNRESTRICTED_GUEST \ - (X86_CR0_WP | X86_CR0_NE | X86_CR0_TS | X86_CR0_MP) -#define KVM_VM_CR0_ALWAYS_ON \ - (KVM_VM_CR0_ALWAYS_ON_UNRESTRICTED_GUEST | X86_CR0_PG | X86_CR0_PE) -#define KVM_GUEST_CR4_MASK \ - (X86_CR4_VME | X86_CR4_PSE | X86_CR4_PAE | X86_CR4_PGE | X86_CR4_VMXE) -#define KVM_PMODE_VM_CR4_ALWAYS_ON (X86_CR4_PAE | X86_CR4_VMXE) -#define KVM_RMODE_VM_CR4_ALWAYS_ON (X86_CR4_VME | X86_CR4_PAE | X86_CR4_VMXE) - #define INVALID_PAGE (~(hpa_t)0) #define UNMAPPED_GVA (~(gpa_t)0) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 9a0a2cf..5ef820e 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -61,6 +61,19 @@ module_param_named(unrestricted_guest, static int __read_mostly emulate_invalid_guest_state = 0; module_param(emulate_invalid_guest_state, bool, S_IRUGO); +#define KVM_GUEST_CR0_MASK_UNRESTRICTED_GUEST \ + (X86_CR0_WP | X86_CR0_NE | X86_CR0_NW | X86_CR0_CD) +#define KVM_GUEST_CR0_MASK \ + (KVM_GUEST_CR0_MASK_UNRESTRICTED_GUEST | X86_CR0_PG | X86_CR0_PE) +#define KVM_VM_CR0_ALWAYS_ON_UNRESTRICTED_GUEST \ + (X86_CR0_WP | X86_CR0_NE | X86_CR0_TS | X86_CR0_MP) +#define KVM_VM_CR0_ALWAYS_ON \ + (KVM_VM_CR0_ALWAYS_ON_UNRESTRICTED_GUEST | X86_CR0_PG | X86_CR0_PE) +#define KVM_GUEST_CR4_MASK \ + (X86_CR4_VME | X86_CR4_PSE | X86_CR4_PAE | X86_CR4_PGE | X86_CR4_VMXE) +#define KVM_PMODE_VM_CR4_ALWAYS_ON (X86_CR4_PAE | X86_CR4_VMXE) +#define KVM_RMODE_VM_CR4_ALWAYS_ON (X86_CR4_VME | X86_CR4_PAE | X86_CR4_VMXE) + /* * These 2 parameters are used to config the controls for Pause-Loop Exiting: * ple_gap: upper bound on the amount of time between two successive -- 1.6.5.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/4] KVM: Add accessor for reading cr4 (or some bits of cr4) 2009-12-07 10:47 [PATCH 0/4] cr4 optimizations for vmx/ept Avi Kivity 2009-12-07 10:47 ` [PATCH 1/4] KVM: VMX: Move some cr[04] related constants to vmx.c Avi Kivity @ 2009-12-07 10:47 ` Avi Kivity 2009-12-08 7:57 ` Sheng Yang 2009-12-07 10:47 ` [PATCH 3/4] KVM: VMX: Make guest cr4 mask more conservative Avi Kivity ` (2 subsequent siblings) 4 siblings, 1 reply; 10+ messages in thread From: Avi Kivity @ 2009-12-07 10:47 UTC (permalink / raw) To: Marcelo Tosatti, Sheng Yang; +Cc: kvm 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 <avi@redhat.com> --- 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 <linux/kvm_host.h> +#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)); } if (!(cr0 & X86_CR0_WP)) @@ -2417,6 +2419,7 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) vmcs_writel(CR0_GUEST_HOST_MASK, ~0UL); vmcs_writel(CR4_GUEST_HOST_MASK, KVM_GUEST_CR4_MASK); + vmx->vcpu.arch.cr4_guest_owned_bits = KVM_GUEST_CR4_MASK; tsc_base = vmx->vcpu.kvm->arch.vm_init_tsc; rdtscll(tsc_this); @@ -3047,7 +3050,7 @@ static int handle_dr(struct kvm_vcpu *vcpu) vcpu->arch.eff_db[dr] = val; break; case 4 ... 5: - if (vcpu->arch.cr4 & X86_CR4_DE) + if (kvm_read_cr4_bits(vcpu, X86_CR4_DE)) kvm_queue_exception(vcpu, UD_VECTOR); break; case 6: diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index dd15d7a..4a16337 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -481,7 +481,7 @@ EXPORT_SYMBOL_GPL(kvm_lmsw); void kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4) { - unsigned long old_cr4 = vcpu->arch.cr4; + unsigned long old_cr4 = kvm_read_cr4(vcpu); unsigned long pdptr_bits = X86_CR4_PGE | X86_CR4_PSE | X86_CR4_PAE; if (cr4 & CR4_RESERVED_BITS) { @@ -1899,7 +1899,7 @@ static int kvm_vcpu_ioctl_x86_set_mce(struct kvm_vcpu *vcpu, return 0; if (mce->status & MCI_STATUS_UC) { if ((vcpu->arch.mcg_status & MCG_STATUS_MCIP) || - !(vcpu->arch.cr4 & X86_CR4_MCE)) { + !kvm_read_cr4_bits(vcpu, X86_CR4_MCE)) { printk(KERN_DEBUG "kvm: set_mce: " "injects mce exception while " "previous one is in progress!\n"); @@ -3612,7 +3612,6 @@ unsigned long realmode_get_cr(struct kvm_vcpu *vcpu, int cr) { unsigned long value; - kvm_x86_ops->decache_cr4_guest_bits(vcpu); switch (cr) { case 0: value = vcpu->arch.cr0; @@ -3624,7 +3623,7 @@ unsigned long realmode_get_cr(struct kvm_vcpu *vcpu, int cr) value = vcpu->arch.cr3; break; case 4: - value = vcpu->arch.cr4; + value = kvm_read_cr4(vcpu); break; case 8: value = kvm_get_cr8(vcpu); @@ -3652,7 +3651,7 @@ void realmode_set_cr(struct kvm_vcpu *vcpu, int cr, unsigned long val, kvm_set_cr3(vcpu, val); break; case 4: - kvm_set_cr4(vcpu, mk_cr_64(vcpu->arch.cr4, val)); + kvm_set_cr4(vcpu, mk_cr_64(kvm_read_cr4(vcpu), val)); break; case 8: kvm_set_cr8(vcpu, val & 0xfUL); @@ -4232,11 +4231,10 @@ int kvm_arch_vcpu_ioctl_get_sregs(struct kvm_vcpu *vcpu, sregs->gdt.limit = dt.limit; sregs->gdt.base = dt.base; - kvm_x86_ops->decache_cr4_guest_bits(vcpu); sregs->cr0 = vcpu->arch.cr0; sregs->cr2 = vcpu->arch.cr2; sregs->cr3 = vcpu->arch.cr3; - sregs->cr4 = vcpu->arch.cr4; + sregs->cr4 = kvm_read_cr4(vcpu); sregs->cr8 = kvm_get_cr8(vcpu); sregs->efer = vcpu->arch.shadow_efer; sregs->apic_base = kvm_get_apic_base(vcpu); @@ -4732,13 +4730,11 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu, kvm_x86_ops->set_efer(vcpu, sregs->efer); kvm_set_apic_base(vcpu, sregs->apic_base); - kvm_x86_ops->decache_cr4_guest_bits(vcpu); - mmu_reset_needed |= vcpu->arch.cr0 != sregs->cr0; kvm_x86_ops->set_cr0(vcpu, sregs->cr0); vcpu->arch.cr0 = sregs->cr0; - mmu_reset_needed |= vcpu->arch.cr4 != sregs->cr4; + mmu_reset_needed |= kvm_read_cr4(vcpu) != sregs->cr4; kvm_x86_ops->set_cr4(vcpu, sregs->cr4); if (!is_long_mode(vcpu) && is_pae(vcpu)) { load_pdptrs(vcpu, vcpu->arch.cr3); -- 1.6.5.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] KVM: Add accessor for reading cr4 (or some bits of cr4) 2009-12-07 10:47 ` [PATCH 2/4] KVM: Add accessor for reading cr4 (or some bits of cr4) Avi Kivity @ 2009-12-08 7:57 ` Sheng Yang 2009-12-08 9:36 ` Avi Kivity 0 siblings, 1 reply; 10+ messages in thread From: Sheng Yang @ 2009-12-08 7:57 UTC (permalink / raw) To: Avi Kivity; +Cc: Marcelo Tosatti, kvm 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 <avi@redhat.com> > --- > 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 <linux/kvm_host.h> > +#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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] KVM: Add accessor for reading cr4 (or some bits of cr4) 2009-12-08 7:57 ` Sheng Yang @ 2009-12-08 9:36 ` Avi Kivity 2009-12-08 14:40 ` Sheng Yang 0 siblings, 1 reply; 10+ messages in thread From: Avi Kivity @ 2009-12-08 9:36 UTC (permalink / raw) To: Sheng Yang; +Cc: Marcelo Tosatti, kvm On 12/08/2009 09:57 AM, Sheng Yang wrote: > 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() > > That one is called from vmx_set_cr4(); at that time CR4_READ_SHADOW is not up-to-date and vmx_decache_cr4_guest_bits() will actually corrupt vcpu->arch.cr4 (except it won't be called, since cr4.pae is never guest owned). But you are right, I should have placed a comment. I'll add a patch that inlines ept_update_paging_mode_cr4 into its caller so it can access the cr4 parameter directly instead of vcpu->arch.cr4. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] KVM: Add accessor for reading cr4 (or some bits of cr4) 2009-12-08 9:36 ` Avi Kivity @ 2009-12-08 14:40 ` Sheng Yang 2009-12-08 14:44 ` Avi Kivity 0 siblings, 1 reply; 10+ messages in thread From: Sheng Yang @ 2009-12-08 14:40 UTC (permalink / raw) To: Avi Kivity; +Cc: Marcelo Tosatti, kvm On Tue, Dec 08, 2009 at 11:36:59AM +0200, Avi Kivity wrote: > On 12/08/2009 09:57 AM, Sheng Yang wrote: > >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() > > > > That one is called from vmx_set_cr4(); at that time CR4_READ_SHADOW > is not up-to-date and vmx_decache_cr4_guest_bits() will actually > corrupt vcpu->arch.cr4 (except it won't be called, since cr4.pae is > never guest owned). Yes, you are right... > > But you are right, I should have placed a comment. I'll add a patch > that inlines ept_update_paging_mode_cr4 into its caller so it can > access the cr4 parameter directly instead of vcpu->arch.cr4. Just notice another thing, seems the cr4_guest_owned_bits' initial value is wrong. It should be ~KVM_GUEST_CR4_MASK rather than KVM_GUEST_CR4_MASK in this patch IIUC. -- regards Yang, Sheng ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] KVM: Add accessor for reading cr4 (or some bits of cr4) 2009-12-08 14:40 ` Sheng Yang @ 2009-12-08 14:44 ` Avi Kivity 0 siblings, 0 replies; 10+ messages in thread From: Avi Kivity @ 2009-12-08 14:44 UTC (permalink / raw) To: Sheng Yang; +Cc: Marcelo Tosatti, kvm On 12/08/2009 04:40 PM, Sheng Yang wrote: > >> But you are right, I should have placed a comment. I'll add a patch >> that inlines ept_update_paging_mode_cr4 into its caller so it can >> access the cr4 parameter directly instead of vcpu->arch.cr4. >> > Just notice another thing, seems the cr4_guest_owned_bits' initial value is > wrong. It should be ~KVM_GUEST_CR4_MASK rather than KVM_GUEST_CR4_MASK in this > patch IIUC. > Yes. Will fix. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/4] KVM: VMX: Make guest cr4 mask more conservative 2009-12-07 10:47 [PATCH 0/4] cr4 optimizations for vmx/ept Avi Kivity 2009-12-07 10:47 ` [PATCH 1/4] KVM: VMX: Move some cr[04] related constants to vmx.c Avi Kivity 2009-12-07 10:47 ` [PATCH 2/4] KVM: Add accessor for reading cr4 (or some bits of cr4) Avi Kivity @ 2009-12-07 10:47 ` Avi Kivity 2009-12-07 10:47 ` [PATCH 4/4] KVM: VMX: When using ept, allow the guest to own cr4.pge Avi Kivity 2009-12-15 10:44 ` [PATCH 0/4] cr4 optimizations for vmx/ept Avi Kivity 4 siblings, 0 replies; 10+ messages in thread From: Avi Kivity @ 2009-12-07 10:47 UTC (permalink / raw) To: Marcelo Tosatti, Sheng Yang; +Cc: kvm Instead of specifying the bits which we want to trap on, specify the bits which we allow the guest to change transparently. This is safer wrt future changes to cr4. Signed-off-by: Avi Kivity <avi@redhat.com> --- arch/x86/kvm/vmx.c | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index ae95a0c..d34fdd3 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -69,8 +69,10 @@ module_param(emulate_invalid_guest_state, bool, S_IRUGO); (X86_CR0_WP | X86_CR0_NE | X86_CR0_TS | X86_CR0_MP) #define KVM_VM_CR0_ALWAYS_ON \ (KVM_VM_CR0_ALWAYS_ON_UNRESTRICTED_GUEST | X86_CR0_PG | X86_CR0_PE) -#define KVM_GUEST_CR4_MASK \ - (X86_CR4_VME | X86_CR4_PSE | X86_CR4_PAE | X86_CR4_PGE | X86_CR4_VMXE) +#define KVM_CR4_GUEST_OWNED_BITS \ + (X86_CR4_PVI | X86_CR4_DE | X86_CR4_PCE | X86_CR4_OSFXSR \ + | X86_CR4_OSXMMEXCPT) + #define KVM_PMODE_VM_CR4_ALWAYS_ON (X86_CR4_PAE | X86_CR4_VMXE) #define KVM_RMODE_VM_CR4_ALWAYS_ON (X86_CR4_VME | X86_CR4_PAE | X86_CR4_VMXE) @@ -2418,8 +2420,8 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) vmcs_write32(VM_ENTRY_CONTROLS, vmcs_config.vmentry_ctrl); vmcs_writel(CR0_GUEST_HOST_MASK, ~0UL); - vmcs_writel(CR4_GUEST_HOST_MASK, KVM_GUEST_CR4_MASK); - vmx->vcpu.arch.cr4_guest_owned_bits = KVM_GUEST_CR4_MASK; + vmx->vcpu.arch.cr4_guest_owned_bits = KVM_CR4_GUEST_OWNED_BITS; + vmcs_writel(CR4_GUEST_HOST_MASK, ~vmx->vcpu.arch.cr4_guest_owned_bits); tsc_base = vmx->vcpu.kvm->arch.vm_init_tsc; rdtscll(tsc_this); -- 1.6.5.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/4] KVM: VMX: When using ept, allow the guest to own cr4.pge 2009-12-07 10:47 [PATCH 0/4] cr4 optimizations for vmx/ept Avi Kivity ` (2 preceding siblings ...) 2009-12-07 10:47 ` [PATCH 3/4] KVM: VMX: Make guest cr4 mask more conservative Avi Kivity @ 2009-12-07 10:47 ` Avi Kivity 2009-12-15 10:44 ` [PATCH 0/4] cr4 optimizations for vmx/ept Avi Kivity 4 siblings, 0 replies; 10+ messages in thread From: Avi Kivity @ 2009-12-07 10:47 UTC (permalink / raw) To: Marcelo Tosatti, Sheng Yang; +Cc: kvm We make no use of cr4.pge if ept is enabled, but the guest does (to flush global mappings, as with vmap()), so give the guest ownership of this bit. Signed-off-by: Avi Kivity <avi@redhat.com> --- arch/x86/kvm/vmx.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index d34fdd3..2e47e65 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -2421,6 +2421,8 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) vmcs_writel(CR0_GUEST_HOST_MASK, ~0UL); vmx->vcpu.arch.cr4_guest_owned_bits = KVM_CR4_GUEST_OWNED_BITS; + if (enable_ept) + vmx->vcpu.arch.cr4_guest_owned_bits |= X86_CR4_PGE; vmcs_writel(CR4_GUEST_HOST_MASK, ~vmx->vcpu.arch.cr4_guest_owned_bits); tsc_base = vmx->vcpu.kvm->arch.vm_init_tsc; -- 1.6.5.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/4] cr4 optimizations for vmx/ept 2009-12-07 10:47 [PATCH 0/4] cr4 optimizations for vmx/ept Avi Kivity ` (3 preceding siblings ...) 2009-12-07 10:47 ` [PATCH 4/4] KVM: VMX: When using ept, allow the guest to own cr4.pge Avi Kivity @ 2009-12-15 10:44 ` Avi Kivity 4 siblings, 0 replies; 10+ messages in thread From: Avi Kivity @ 2009-12-15 10:44 UTC (permalink / raw) To: Marcelo Tosatti, Sheng Yang; +Cc: kvm On 12/07/2009 12:47 PM, Avi Kivity wrote: > When ept is enabled, we aren't particularly interested in cr4.pge, so allow > the guest to own it. This improves performance in vmap() intensive loads. > This is now applied, after modifying according to review comments. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2009-12-15 10:44 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-12-07 10:47 [PATCH 0/4] cr4 optimizations for vmx/ept Avi Kivity 2009-12-07 10:47 ` [PATCH 1/4] KVM: VMX: Move some cr[04] related constants to vmx.c Avi Kivity 2009-12-07 10:47 ` [PATCH 2/4] KVM: Add accessor for reading cr4 (or some bits of cr4) Avi Kivity 2009-12-08 7:57 ` Sheng Yang 2009-12-08 9:36 ` Avi Kivity 2009-12-08 14:40 ` Sheng Yang 2009-12-08 14:44 ` Avi Kivity 2009-12-07 10:47 ` [PATCH 3/4] KVM: VMX: Make guest cr4 mask more conservative Avi Kivity 2009-12-07 10:47 ` [PATCH 4/4] KVM: VMX: When using ept, allow the guest to own cr4.pge Avi Kivity 2009-12-15 10:44 ` [PATCH 0/4] cr4 optimizations for vmx/ept Avi Kivity
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.