From: Gleb Natapov <gleb@kernel.org>
To: Jan Kiszka <jan.kiszka@web.de>
Cc: Paolo Bonzini <pbonzini@redhat.com>, kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: VMX: shadow VM_(ENTRY|EXIT)_CONTROLS vmcs field
Date: Mon, 9 Dec 2013 08:55:51 +0200 [thread overview]
Message-ID: <20131209065550.GB16543@minantech.com> (raw)
In-Reply-To: <52A46D5B.8040100@web.de>
On Sun, Dec 08, 2013 at 02:00:11PM +0100, Jan Kiszka wrote:
> 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 <gleb@redhat.com>
> >> 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);
> >> }
> >>
> >> +static inline void vm_entry_controls_init(struct vcpu_vmx *vmx, u32 val)
> >> +{
> >> + vmcs_write32(VM_ENTRY_CONTROLS, val);
> >> + vmx->vm_entry_controls_shadow = val;
> >> +}
> >> +
> >> +static inline void vm_entry_controls_set(struct vcpu_vmx *vmx, u32 val)
> >> +{
> >> + if (vmx->vm_entry_controls_shadow != 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, u32 val)
> >> +{
> >> + vm_entry_controls_set(vmx, vm_entry_controls_get(vmx) & ~val);
> >> +}
> >> +
> >> +static inline void vm_exit_controls_init(struct vcpu_vmx *vmx, u32 val)
> >> +{
> >> + vmcs_write32(VM_EXIT_CONTROLS, val);
> >> + vmx->vm_exit_controls_shadow = val;
> >> +}
> >> +
> >> +static inline void vm_exit_controls_set(struct vcpu_vmx *vmx, u32 val)
> >> +{
> >> + if (vmx->vm_exit_controls_shadow != 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, u32 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 = 0;
> >> @@ -1410,11 +1468,11 @@ static void update_exception_bitmap(struct kvm_vcpu *vcpu)
> >> vmcs_write32(EXCEPTION_BITMAP, eb);
> >> }
> >>
> >> -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);
> >> }
> >>
> >> static void clear_atomic_switch_msr(struct vcpu_vmx *vmx, unsigned msr)
> >> @@ -1425,14 +1483,15 @@ static void clear_atomic_switch_msr(struct vcpu_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 vcpu_vmx *vmx, unsigned msr)
> >> vmcs_write32(VM_EXIT_MSR_LOAD_COUNT, m->nr);
> >> }
> >>
> >> -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);
> >> }
> >>
> >> static void add_atomic_switch_msr(struct vcpu_vmx *vmx, unsigned msr,
> >> @@ -1472,7 +1532,8 @@ static void add_atomic_switch_msr(struct vcpu_vmx *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_vmx *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 = 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 = 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);
> >>
> >> msr->data = efer & ~EFER_LME;
> >> }
> >> @@ -3217,9 +3274,7 @@ static void enter_lmode(struct kvm_vcpu *vcpu)
> >>
> >> 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);
> >> }
> >>
> >> @@ -4346,10 +4401,11 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
> >> ++vmx->nmsrs;
> >> }
> >>
> >> - vmcs_write32(VM_EXIT_CONTROLS, vmcs_config.vmexit_ctrl);
> >> +
> >> + vm_exit_controls_init(vmx, vmcs_config.vmexit_ctrl);
> >>
> >> /* 22.2.1, 20.8.1 */
> >> - vmcs_write32(VM_ENTRY_CONTROLS, vmcs_config.vmentry_ctrl);
> >> + vm_entry_controls_init(vmx, vmcs_config.vmentry_ctrl);
> >>
> >> vmcs_writel(CR0_GUEST_HOST_MASK, ~0UL);
> >> set_cr4_guest_host_mask(vmx);
> >> @@ -7759,12 +7815,12 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
> >> exit_control = vmcs_config.vmexit_ctrl;
> >> if (vmcs12->pin_based_vm_exec_control & PIN_BASED_VMX_PREEMPTION_TIMER)
> >> exit_control |= VM_EXIT_SAVE_VMX_PREEMPTION_TIMER;
> >> - vmcs_write32(VM_EXIT_CONTROLS, exit_control);
> >> + vm_exit_controls_init(vmx, exit_control);
> >>
> >> /* 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,
> >> (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)
> >>
> >> vmcs12->vm_entry_controls =
> >> (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);
> >>
> >> /* 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 *vcpu)
> >> vcpu->cpu = cpu;
> >> put_cpu();
> >>
> >> + vm_entry_controls_init(vmx, vmcs_read32(VM_ENTRY_CONTROLS));
> >> vmx_segment_cache_clear(vmx);
> >>
> >> /* 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
> >>
> >
> > 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:
>
What about this patch?
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 1d95384..f90320b 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8452,6 +8452,7 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu)
put_cpu();
vm_entry_controls_init(vmx, vmcs_read32(VM_ENTRY_CONTROLS));
+ vm_exit_controls_init(vmx, vmcs_read32(VM_EXIT_CONTROLS));
vmx_segment_cache_clear(vmx);
/* if no vmcs02 cache requested, remove the one we used */
--
Gleb.
next prev parent reply other threads:[~2013-12-09 6:55 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-25 13:37 [PATCH] KVM: VMX: shadow VM_(ENTRY|EXIT)_CONTROLS vmcs field Gleb Natapov
2013-11-27 16:58 ` Paolo Bonzini
2013-12-08 13:00 ` Jan Kiszka
2013-12-09 6:55 ` Gleb Natapov [this message]
2013-12-09 13:48 ` Jan Kiszka
2013-12-12 9:50 ` Paolo Bonzini
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=20131209065550.GB16543@minantech.com \
--to=gleb@kernel.org \
--cc=jan.kiszka@web.de \
--cc=kvm@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