KVM: VMX: cache exit_intr_info

KVM: VMX: cache exit_intr_info

[Marcelo Tosatti]

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>

--- kvm.orig/arch/x86/kvm/vmx.c
+++ kvm/arch/x86/kvm/vmx.c
@@ -60,6 +60,7 @@ struct vcpu_vmx {
 	int                   launched;
 	u8                    fail;
 	u32                   idt_vectoring_info;
+	u32                   exit_intr_info;
 	struct kvm_msr_entry *guest_msrs;
 	struct kvm_msr_entry *host_msrs;
 	int                   nmsrs;
@@ -2250,7 +2251,7 @@ static int handle_exception(struct kvm_v
 	enum emulation_result er;
 
 	vect_info = vmx->idt_vectoring_info;
-	intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
+	intr_info = vmx->exit_intr_info;
 
 	if ((vect_info & VECTORING_INFO_VALID_MASK) &&
 						!is_page_fault(intr_info))
@@ -2818,7 +2819,7 @@ static void vmx_intr_assist(struct kvm_v
 	update_tpr_threshold(vcpu);
 
 	intr_info_field = vmcs_read32(VM_ENTRY_INTR_INFO_FIELD);
-	exit_intr_info_field = vmcs_read32(VM_EXIT_INTR_INFO);
+	exit_intr_info_field = vmx->exit_intr_info;
 	idtv_info_field = vmx->idt_vectoring_info;
 	if (intr_info_field & INTR_INFO_VALID_MASK) {
 		if (idtv_info_field & INTR_INFO_VALID_MASK) {
@@ -2927,7 +2928,6 @@ static void vmx_flush_regs(struct kvm_vc
 static void vmx_vcpu_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
-	u32 intr_info;
 
 	vmx_flush_regs(vcpu);
 
@@ -3062,11 +3062,11 @@ static void vmx_vcpu_run(struct kvm_vcpu
 	asm("mov %0, %%ds; mov %0, %%es" : : "r"(__USER_DS));
 	vmx->launched = 1;
 
-	intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
+	vmx->exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
 
 	/* We need to handle NMIs before interrupts are enabled */
-	if ((intr_info & INTR_INFO_INTR_TYPE_MASK) == 0x200 &&
-	    (intr_info & INTR_INFO_VALID_MASK)) {
+	if ((vmx->exit_intr_info & INTR_INFO_INTR_TYPE_MASK) == 0x200 &&
+	    (vmx->exit_intr_info & INTR_INFO_VALID_MASK)) {
 		KVMTRACE_0D(NMI, vcpu, handler);
 		asm("int $2");
 	}

Re: KVM: VMX: cache exit_intr_info

[Yang, Sheng]

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.

--
Thanks
Yang, Sheng

Re: KVM: VMX: cache exit_intr_info

[Marcelo Tosatti]

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

Re: KVM: VMX: cache exit_intr_info

[Yang, Sheng]

On Saturday 28 June 2008 13:35:27 Marcelo Tosatti wrote:
> 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?

I think we may include more MSRs, though not all of them in the 
critical path. GUEST_INTERRUPTIABILITY_INFO is on the critical path, 
as well as VM_ENTRY_INTR_INFO_FIELD. The GUEST_RFLAGS and 
CPU_BASED_VM_EXEC_CONTROL also been used very frequently. Of course 
the latter three MSR I mentioned need write cache support, I'd like 
to go the similar way as kvm_cache_regs did.

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

I will test if we include these write cache MSR, how much benefit we 
can get. Can you provide some detail on how can you get the 
performance data? :)

-- 
Thanks
Yang, Sheng