Re: KVM: VMX: cache exit_intr_info

[Marcelo Tosatti <mtosatti@redhat.com>]

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));