From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH] KVM: VMX: shadow VM_(ENTRY|EXIT)_CONTROLS vmcs field Date: Sun, 08 Dec 2013 14:00:11 +0100 Message-ID: <52A46D5B.8040100@web.de> References: <20131125133713.GB959@redhat.com> <529624BE.9090404@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="T40gn7qQus195oDadgxuBEjIKF8XIt1du" Cc: kvm@vger.kernel.org To: Paolo Bonzini , Gleb Natapov Return-path: Received: from mout.web.de ([212.227.17.11]:60564 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752672Ab3LHNAQ (ORCPT ); Sun, 8 Dec 2013 08:00:16 -0500 Received: from mchn199C.mchp.siemens.de ([95.157.58.223]) by smtp.web.de (mrweb103) with ESMTPSA (Nemesis) id 0MGA7n-1VkwsD48Xv-00FCbr for ; Sun, 08 Dec 2013 14:00:15 +0100 In-Reply-To: <529624BE.9090404@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --T40gn7qQus195oDadgxuBEjIKF8XIt1du Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2013-11-27 17:58, Paolo Bonzini wrote: > Il 25/11/2013 14:37, Gleb Natapov ha scritto: >> VM_(ENTRY|EXIT)_CONTROLS vmcs fields are read/written on each guest >> entry but most times it can be avoided since values do not changes. >> Keep fields copy in memory to avoid unnecessary reads from vmcs. >> >> Signed-off-by: Gleb Natapov >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index b2fe1c2..29c1c7f 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -418,6 +418,8 @@ struct vcpu_vmx { >> u64 msr_host_kernel_gs_base; >> u64 msr_guest_kernel_gs_base; >> #endif >> + u32 vm_entry_controls_shadow; >> + u32 vm_exit_controls_shadow; >> /* >> * loaded_vmcs points to the VMCS currently used in this vcpu. For a= >> * non-nested (L1) guest, it always points to vmcs01. For a nested >> @@ -1326,6 +1328,62 @@ static void vmcs_set_bits(unsigned long field, = u32 mask) >> vmcs_writel(field, vmcs_readl(field) | mask); >> } >> =20 >> +static inline void vm_entry_controls_init(struct vcpu_vmx *vmx, u32 v= al) >> +{ >> + vmcs_write32(VM_ENTRY_CONTROLS, val); >> + vmx->vm_entry_controls_shadow =3D val; >> +} >> + >> +static inline void vm_entry_controls_set(struct vcpu_vmx *vmx, u32 va= l) >> +{ >> + if (vmx->vm_entry_controls_shadow !=3D val) >> + vm_entry_controls_init(vmx, val); >> +} >> + >> +static inline u32 vm_entry_controls_get(struct vcpu_vmx *vmx) >> +{ >> + return vmx->vm_entry_controls_shadow; >> +} >> + >> + >> +static inline void vm_entry_controls_setbit(struct vcpu_vmx *vmx, u32= val) >> +{ >> + vm_entry_controls_set(vmx, vm_entry_controls_get(vmx) | val); >> +} >> + >> +static inline void vm_entry_controls_clearbit(struct vcpu_vmx *vmx, u= 32 val) >> +{ >> + vm_entry_controls_set(vmx, vm_entry_controls_get(vmx) & ~val); >> +} >> + >> +static inline void vm_exit_controls_init(struct vcpu_vmx *vmx, u32 va= l) >> +{ >> + vmcs_write32(VM_EXIT_CONTROLS, val); >> + vmx->vm_exit_controls_shadow =3D val; >> +} >> + >> +static inline void vm_exit_controls_set(struct vcpu_vmx *vmx, u32 val= ) >> +{ >> + if (vmx->vm_exit_controls_shadow !=3D val) >> + vm_exit_controls_init(vmx, val); >> +} >> + >> +static inline u32 vm_exit_controls_get(struct vcpu_vmx *vmx) >> +{ >> + return vmx->vm_exit_controls_shadow; >> +} >> + >> + >> +static inline void vm_exit_controls_setbit(struct vcpu_vmx *vmx, u32 = val) >> +{ >> + vm_exit_controls_set(vmx, vm_exit_controls_get(vmx) | val); >> +} >> + >> +static inline void vm_exit_controls_clearbit(struct vcpu_vmx *vmx, u3= 2 val) >> +{ >> + vm_exit_controls_set(vmx, vm_exit_controls_get(vmx) & ~val); >> +} >> + >> static void vmx_segment_cache_clear(struct vcpu_vmx *vmx) >> { >> vmx->segment_cache.bitmask =3D 0; >> @@ -1410,11 +1468,11 @@ static void update_exception_bitmap(struct kvm= _vcpu *vcpu) >> vmcs_write32(EXCEPTION_BITMAP, eb); >> } >> =20 >> -static void clear_atomic_switch_msr_special(unsigned long entry, >> - unsigned long exit) >> +static void clear_atomic_switch_msr_special(struct vcpu_vmx *vmx, >> + unsigned long entry, unsigned long exit) >> { >> - vmcs_clear_bits(VM_ENTRY_CONTROLS, entry); >> - vmcs_clear_bits(VM_EXIT_CONTROLS, exit); >> + vm_entry_controls_clearbit(vmx, entry); >> + vm_exit_controls_clearbit(vmx, exit); >> } >> =20 >> static void clear_atomic_switch_msr(struct vcpu_vmx *vmx, unsigned ms= r) >> @@ -1425,14 +1483,15 @@ static void clear_atomic_switch_msr(struct vcp= u_vmx *vmx, unsigned msr) >> switch (msr) { >> case MSR_EFER: >> if (cpu_has_load_ia32_efer) { >> - clear_atomic_switch_msr_special(VM_ENTRY_LOAD_IA32_EFER, >> + clear_atomic_switch_msr_special(vmx, >> + VM_ENTRY_LOAD_IA32_EFER, >> VM_EXIT_LOAD_IA32_EFER); >> return; >> } >> break; >> case MSR_CORE_PERF_GLOBAL_CTRL: >> if (cpu_has_load_perf_global_ctrl) { >> - clear_atomic_switch_msr_special( >> + clear_atomic_switch_msr_special(vmx, >> VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL, >> VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL); >> return; >> @@ -1453,14 +1512,15 @@ static void clear_atomic_switch_msr(struct vcp= u_vmx *vmx, unsigned msr) >> vmcs_write32(VM_EXIT_MSR_LOAD_COUNT, m->nr); >> } >> =20 >> -static void add_atomic_switch_msr_special(unsigned long entry, >> - unsigned long exit, unsigned long guest_val_vmcs, >> - unsigned long host_val_vmcs, u64 guest_val, u64 host_val) >> +static void add_atomic_switch_msr_special(struct vcpu_vmx *vmx, >> + unsigned long entry, unsigned long exit, >> + unsigned long guest_val_vmcs, unsigned long host_val_vmcs, >> + u64 guest_val, u64 host_val) >> { >> vmcs_write64(guest_val_vmcs, guest_val); >> vmcs_write64(host_val_vmcs, host_val); >> - vmcs_set_bits(VM_ENTRY_CONTROLS, entry); >> - vmcs_set_bits(VM_EXIT_CONTROLS, exit); >> + vm_entry_controls_setbit(vmx, entry); >> + vm_exit_controls_setbit(vmx, exit); >> } >> =20 >> static void add_atomic_switch_msr(struct vcpu_vmx *vmx, unsigned msr,= >> @@ -1472,7 +1532,8 @@ static void add_atomic_switch_msr(struct vcpu_vm= x *vmx, unsigned msr, >> switch (msr) { >> case MSR_EFER: >> if (cpu_has_load_ia32_efer) { >> - add_atomic_switch_msr_special(VM_ENTRY_LOAD_IA32_EFER, >> + add_atomic_switch_msr_special(vmx, >> + VM_ENTRY_LOAD_IA32_EFER, >> VM_EXIT_LOAD_IA32_EFER, >> GUEST_IA32_EFER, >> HOST_IA32_EFER, >> @@ -1482,7 +1543,7 @@ static void add_atomic_switch_msr(struct vcpu_vm= x *vmx, unsigned msr, >> break; >> case MSR_CORE_PERF_GLOBAL_CTRL: >> if (cpu_has_load_perf_global_ctrl) { >> - add_atomic_switch_msr_special( >> + add_atomic_switch_msr_special(vmx, >> VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL, >> VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL, >> GUEST_IA32_PERF_GLOBAL_CTRL, >> @@ -3182,14 +3243,10 @@ static void vmx_set_efer(struct kvm_vcpu *vcpu= , u64 efer) >> vmx_load_host_state(to_vmx(vcpu)); >> vcpu->arch.efer =3D efer; >> if (efer & EFER_LMA) { >> - vmcs_write32(VM_ENTRY_CONTROLS, >> - vmcs_read32(VM_ENTRY_CONTROLS) | >> - VM_ENTRY_IA32E_MODE); >> + vm_entry_controls_setbit(to_vmx(vcpu), VM_ENTRY_IA32E_MODE); >> msr->data =3D efer; >> } else { >> - vmcs_write32(VM_ENTRY_CONTROLS, >> - vmcs_read32(VM_ENTRY_CONTROLS) & >> - ~VM_ENTRY_IA32E_MODE); >> + vm_entry_controls_clearbit(to_vmx(vcpu), VM_ENTRY_IA32E_MODE); >> =20 >> msr->data =3D efer & ~EFER_LME; >> } >> @@ -3217,9 +3274,7 @@ static void enter_lmode(struct kvm_vcpu *vcpu) >> =20 >> static void exit_lmode(struct kvm_vcpu *vcpu) >> { >> - vmcs_write32(VM_ENTRY_CONTROLS, >> - vmcs_read32(VM_ENTRY_CONTROLS) >> - & ~VM_ENTRY_IA32E_MODE); >> + vm_entry_controls_clearbit(to_vmx(vcpu), VM_ENTRY_IA32E_MODE); >> vmx_set_efer(vcpu, vcpu->arch.efer & ~EFER_LMA); >> } >> =20 >> @@ -4346,10 +4401,11 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx= ) >> ++vmx->nmsrs; >> } >> =20 >> - vmcs_write32(VM_EXIT_CONTROLS, vmcs_config.vmexit_ctrl); >> + >> + vm_exit_controls_init(vmx, vmcs_config.vmexit_ctrl); >> =20 >> /* 22.2.1, 20.8.1 */ >> - vmcs_write32(VM_ENTRY_CONTROLS, vmcs_config.vmentry_ctrl); >> + vm_entry_controls_init(vmx, vmcs_config.vmentry_ctrl); >> =20 >> vmcs_writel(CR0_GUEST_HOST_MASK, ~0UL); >> set_cr4_guest_host_mask(vmx); >> @@ -7759,12 +7815,12 @@ static void prepare_vmcs02(struct kvm_vcpu *vc= pu, struct vmcs12 *vmcs12) >> exit_control =3D vmcs_config.vmexit_ctrl; >> if (vmcs12->pin_based_vm_exec_control & PIN_BASED_VMX_PREEMPTION_TIM= ER) >> exit_control |=3D VM_EXIT_SAVE_VMX_PREEMPTION_TIMER; >> - vmcs_write32(VM_EXIT_CONTROLS, exit_control); >> + vm_exit_controls_init(vmx, exit_control); >> =20 >> /* vmcs12's VM_ENTRY_LOAD_IA32_EFER and VM_ENTRY_IA32E_MODE are >> * emulated by vmx_set_efer(), below. >> */ >> - vmcs_write32(VM_ENTRY_CONTROLS, >> + vm_entry_controls_init(vmx,=20 >> (vmcs12->vm_entry_controls & ~VM_ENTRY_LOAD_IA32_EFER & >> ~VM_ENTRY_IA32E_MODE) | >> (vmcs_config.vmentry_ctrl & ~VM_ENTRY_IA32E_MODE)); >> @@ -8186,7 +8242,7 @@ static void prepare_vmcs12(struct kvm_vcpu *vcpu= , struct vmcs12 *vmcs12) >> =20 >> vmcs12->vm_entry_controls =3D >> (vmcs12->vm_entry_controls & ~VM_ENTRY_IA32E_MODE) | >> - (vmcs_read32(VM_ENTRY_CONTROLS) & VM_ENTRY_IA32E_MODE); >> + (vm_entry_controls_get(to_vmx(vcpu)) & VM_ENTRY_IA32E_MODE); >> =20 >> /* TODO: These cannot have changed unless we have MSR bitmaps and >> * the relevant bit asks not to trap the change */ >> @@ -8390,6 +8446,7 @@ static void nested_vmx_vmexit(struct kvm_vcpu *v= cpu) >> vcpu->cpu =3D cpu; >> put_cpu(); >> =20 >> + vm_entry_controls_init(vmx, vmcs_read32(VM_ENTRY_CONTROLS)); >> vmx_segment_cache_clear(vmx); >> =20 >> /* if no vmcs02 cache requested, remove the one we used */ >> -- >> Gleb. >> -- >> To unsubscribe from this list: send the line "unsubscribe kvm" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >=20 > Applied to kvm/queue, thanks. Seems to have or cause an issue with nVMX. The (still not merged) vmx unit test breaks here after apply this commit: enabling apic paging enabled cr0 =3D 80010011 cr3 =3D 7fff000 cr4 =3D 20 PASS: test vmx capability PASS: test vmxon PASS: test vmptrld PASS: test vmclear PASS: test vmptrst Hello World, this is null_guest_main! PASS: test vmxoff Test suite : vmenter PASS: test vmlaunch PASS: test vmresume Test suite : preemption timer KVM: entry failed, hardware error 0x7 RAX=3D000000002ced133a RBX=3D0000000000000000 RCX=3D0000000000000000 RDX=3D= 0000000000000000 RSI=3D0000000000000000 RDI=3D0000000000000000 RBP=3D0000000000000000 RSP=3D= 000000000044f9a0 R8 =3D0000000000000000 R9 =3D0000000000000000 R10=3D0000000000000000 R11=3D= 0000000000000000 R12=3D0000000000000000 R13=3D0000000000000000 R14=3D0000000000000000 R15=3D= 0000000000000000 RIP=3D0000000000400c63 RFL=3D00000002 [-------] CPL=3D0 II=3D0 A20=3D1 SM= M=3D0 HLT=3D0 ES =3D0010 0000000000000000 ffffffff 00c09300 DPL=3D0 DS [-WA] CS =3D0008 0000000000000000 ffffffff 00a09b00 DPL=3D0 CS64 [-RA] SS =3D0010 0000000000000000 ffffffff 00c09300 DPL=3D0 DS [-WA] DS =3D0010 0000000000000000 ffffffff 00c09300 DPL=3D0 DS [-WA] FS =3D0010 0000000000000000 ffffffff 00c09300 DPL=3D0 DS [-WA] GS =3D0010 0000000000000000 ffffffff 00c09300 DPL=3D0 DS [-WA] LDT=3D0000 0000000000000000 0000ffff 00008200 DPL=3D0 LDT TR =3D0048 000000000040b052 00000067 00008b00 DPL=3D0 TSS64-busy GDT=3D 000000000040b000 00000447 IDT=3D 0000000000400296 00000fff CR0=3D80010031 CR2=3D0000000000000000 CR3=3D0000000007fff000 CR4=3D000020= 21 DR0=3D0000000000000000 DR1=3D0000000000000000 DR2=3D0000000000000000 DR3=3D= 0000000000000000=20 DR6=3D00000000ffff0ff0 DR7=3D0000000000000400 EFER=3D0000000000000500 Code=3D34 25 00 0c 45 00 9d 0f 01 c2 eb 03 0f 01 c3 0f 96 44 24 0c <48> 8= 7 04 25 40 0c 45 00 48 87 1c 25 48 0c 45 00 48 87 0c 25 50 0c 45 00 48 87= 14 25 58 0c Jan --T40gn7qQus195oDadgxuBEjIKF8XIt1du Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlKkbV4ACgkQitSsb3rl5xS5lACgjJ/LztvzGeITd+0G3t4RNFoF G7EAoLpn5FTdRmaJxYmbnDtwwblWxriw =zg9+ -----END PGP SIGNATURE----- --T40gn7qQus195oDadgxuBEjIKF8XIt1du--