Re: KVM: VMX: cache exit_intr_info

Marcelo Tosatti
2008-06-28T05:35:52Z

On Sat, Jun 28, 2008 at 11:20:47AM +0800, Yang, Sheng wrote:
> On Saturday 28 June 2008 02:05:19 Marcelo Tosatti wrote:
> > exit_intr_info is read-only in nature, so once read it can be
> > cached similarly to idtv_vectoring_inf.
> >
> > Reduces guest re-entry in about 50 cycles on my machine (the
> > exception path should be similar, but haven't measured).
> >
> > Applies on top of register accessor patch.
> >
> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> >
> Thanks for the patches! :)
>
> And I realized there are also too much vmcs_read32
> (CPU_BASED_VM_EXEC_CONTROL)(though not read only). I'd like to post
> another patch to optimize it later.

GUEST_INTERRUPTIBILITY_INFO is also a candidate, with significant wins
(used by skip_emulated_instruction which is often used in the exit
handlers).

GUEST_RFLAGS is another register read multiple times in the fast path,
but seems trickier.

Do you have a better suggestion instead of
vmcs_cache_read32/vmcs_cache_write32 below for this caching
optimizations?

With these three patches applied gettimeofday() microbenchmark is 5%
faster.

Index: kvm.speed/arch/x86/kvm/vmx.c
===================================================================
--- kvm.speed.orig/arch/x86/kvm/vmx.c
+++ kvm.speed/arch/x86/kvm/vmx.c
@@ -61,6 +61,7 @@ struct vcpu_vmx {
u8 fail;
u32 idt_vectoring_info;
u32 exit_intr_info;
+ u32 interruptibility;
struct kvm_msr_entry *guest_msrs;
struct kvm_msr_entry *host_msrs;
int nmsrs;
@@ -415,6 +416,17 @@ static u64 vmcs_read64(unsigned long fie
#endif
}

+static u32 vmcs_cache_read32(struct kvm_vcpu *vcpu, unsigned long field)
+{
+ struct vcpu_vmx *vmx = to_vmx(vcpu);
+ switch(field) {
+ case GUEST_INTERRUPTIBILITY_INFO:
+ return vmx->interruptibility;
+ default:
+ BUG();
+ }
+}
+
static noinline void vmwrite_error(unsigned long field, unsigned long value)
{
printk(KERN_ERR "vmwrite error: reg %lx value %lx (err %d)\n",
@@ -451,6 +463,22 @@ static void vmcs_write64(unsigned long f
#endif
}

+static void vmcs_cache_write32(struct kvm_vcpu *vcpu, unsigned long field,
+ u32 value)
+{
+ struct vcpu_vmx *vmx = to_vmx(vcpu);
+
+ vmcs_write32(field, value);
+
+ switch (field) {
+ case GUEST_INTERRUPTIBILITY_INFO:
+ vmx->interruptibility = value;
+ break;
+ default:
+ ;
+ }
+}
+
static void vmcs_clear_bits(unsigned long field, u32 mask)
{
vmcs_writel(field, vmcs_readl(field) & ~mask);
@@ -717,9 +745,9 @@ static void skip_emulated_instruction(st
* We emulated an instruction, so temporary interrupt blocking
* should be removed, if set.
*/
- interruptibility = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO);
+ interruptibility = vmcs_cache_read32(vcpu, GUEST_INTERRUPTIBILITY_INFO);
if (interruptibility & 3)
- vmcs_write32(GUEST_INTERRUPTIBILITY_INFO,
+ vmcs_cache_write32(vcpu, GUEST_INTERRUPTIBILITY_INFO,
interruptibility & ~3);
vcpu->arch.interrupt_window_open = 1;
}
@@ -2079,7 +2107,7 @@ static int vmx_vcpu_reset(struct kvm_vcp
vmcs_write32(GUEST_IDTR_LIMIT, 0xffff);

vmcs_write32(GUEST_ACTIVITY_STATE, 0);
- vmcs_write32(GUEST_INTERRUPTIBILITY_INFO, 0);
+ vmcs_cache_write32(vcpu, GUEST_INTERRUPTIBILITY_INFO, 0);
vmcs_write32(GUEST_PENDING_DBG_EXCEPTIONS, 0);

guest_write_tsc(0);
@@ -2169,7 +2197,7 @@ static void do_interrupt_requests(struct

vcpu->arch.interrupt_window_open =
((vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_IF) &&
- (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & 3) == 0);
+ (vmcs_cache_read32(vcpu, GUEST_INTERRUPTIBILITY_INFO) & 3) == 0);

if (vcpu->arch.interrupt_window_open &&
vcpu->arch.irq_summary &&
@@ -2788,7 +2816,7 @@ static void enable_nmi_window(struct kvm

static int vmx_nmi_enabled(struct kvm_vcpu *vcpu)
{
- u32 guest_intr = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO);
+ u32 guest_intr = vmcs_cache_read32(vcpu, GUEST_INTERRUPTIBILITY_INFO);
return !(guest_intr & (GUEST_INTR_STATE_NMI |
GUEST_INTR_STATE_MOV_SS |
GUEST_INTR_STATE_STI));
@@ -2796,7 +2824,7 @@ static int vmx_nmi_enabled(struct kvm_vc

static int vmx_irq_enabled(struct kvm_vcpu *vcpu)
{
- u32 guest_intr = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO);
+ u32 guest_intr = vmcs_cache_read32(vcpu, GUEST_INTERRUPTIBILITY_INFO);
return (!(guest_intr & (GUEST_INTR_STATE_MOV_SS |
GUEST_INTR_STATE_STI)) &&
(vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_IF));
@@ -2850,8 +2878,8 @@ static void vmx_intr_assist(struct kvm_v
*/
if ((idtv_info_field & VECTORING_INFO_TYPE_MASK)
== INTR_TYPE_NMI_INTR && cpu_has_virtual_nmis())
- vmcs_write32(GUEST_INTERRUPTIBILITY_INFO,
- vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) &
+ vmcs_cache_write32(vcpu, GUEST_INTERRUPTIBILITY_INFO,
+ vmcs_cache_read32(vcpu, GUEST_INTERRUPTIBILITY_INFO) &
~GUEST_INTR_STATE_NMI);

vmcs_write32(VM_ENTRY_INTR_INFO_FIELD, idtv_info_field
@@ -2873,8 +2901,8 @@ static void vmx_intr_assist(struct kvm_v
*/
if ((exit_intr_info_field & INTR_INFO_UNBLOCK_NMI) &&
(exit_intr_info_field & INTR_INFO_VECTOR_MASK) != 8)
- vmcs_write32(GUEST_INTERRUPTIBILITY_INFO,
- vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) |
+ vmcs_cache_write32(vcpu, GUEST_INTERRUPTIBILITY_INFO,
+ vmcs_cache_read32(vcpu, GUEST_INTERRUPTIBILITY_INFO) |
GUEST_INTR_STATE_NMI);
else if (vcpu->arch.nmi_pending) {
if (vmx_nmi_enabled(vcpu))
@@ -3055,8 +3083,9 @@ static void vmx_vcpu_run(struct kvm_vcpu
if (vmx->rmode.irq.pending)
fixup_rmode_irq(vmx);

+ vmx->interruptibility = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO);
vcpu->arch.interrupt_window_open =
- (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) &
+ (vmx->interruptibility &
(GUEST_INTR_STATE_STI | GUEST_INTR_STATE_MOV_SS)) == 0;

asm("mov %0, %%ds; mov %0, %%es" : : "r"(__USER_DS));