external module sched_in event

Hello,

My worst longstanding problem with KVM is that as the uptime of my host system increased, my opensuse guest images started to destabilize and lockup at boot. The weird thing was that fresh after boot everything was always perfectly ok, so I thought it was rmmod/insmod or some other sticky effect on the CPU after restarting the guest a few times that triggered the crash. Furthermore if I loaded the cpu a lot (like with a while :; do true;done), the crash would magically disappear. Decreasing cpu frequency and timings didn't help. Debugging wasn't trivial because it required a certain uptime and it didn't always crash.

So I once debugged this more aggressively I figured out KVM was ok, it was the guest that crashed in the tsc clocksource because tsc wasn't monotone. guest was looping in an infinite loop with irq disabled. So I tried to pass "notsc" and that fixed the crash just fine.

Initially I thought it was the tsc_offset logic being wrong but then I figured out that the vcpu_put/load wasn't always executed, this bugcheck triggers with current git and so I recommend to apply this to kvm.git to avoid similar nasty hard-to-detect bugs in the future (Avi says vmx would crash hard in such a condition, svm is much simpler and it somewhat survives the lack of sched_in and only crashes the guest due to not monotone tsc):

index ac876ec..26372fa 100644
@@ -742,6 +742,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 {
+	WARN_ON(vcpu->cpu != smp_processor_id());
 	kvm_x86_ops->vcpu_put(vcpu);
 	kvm_put_guest_fpu(vcpu);
 }

So trying to understand why the ->cpu was wrong, I looked into the preempt notifiers emulation, and it looked quite fragile without a real sched_in hook. I figured out I could provide a real sched_in hook by loading the proper values in the tsk->thread.debugreg[0/7]. Initially I got the hooking points out of objdump -d vmlinux, but Avi preferred no dependency on the vmlinux and he suggested to try to find the sched_in hook in the stack. So that's what I implemented now and this should provide real robustness to the out of tree module compiled against binary kernel images with CONFIG_KVM=n. I tried to be compatible with all kernels down to 2.6.5 but only 2.6.2x host is tested and only on 64bit and only on SVM (no vmx system around here at all).

This fixes my longstanding KVM instability and "-smp 2" now works flawlessy with svm too! -smp 2 -snapshot crashes in qemu userland but that's not kernel related, must be some thread mutex lock recursion or lock inversion in the qcow cow code. Removing -snapshot make -smp 2 stable. Multiple guests UP and SMP seems stable too.

To reproduce my crash easily without waiting ages for the two tsc to deviate with an error larger than the number of cycles it takes for a CPU migration, run write_tsc(0,0) in kernel mode (like in the svm.c init function and then insmod kvm-amd; rmmod kvm-amd and then remove write_tsc and recompile kvm-amd).

#include <stdio.h>
main()

{
	unsigned long x1, x2;
	unsigned int a, d;
	asm volatile("rdtsc" : "=a" (a), "=d" (d));
	x1 = ((unsigned long)d << 32) | a;
	for (;;) {
		asm volatile("rdtsc" : "=a" (a), "=d" (d));
		x2 = ((unsigned long)d << 32) | a;
		if (x2 < x1)
			printf("error %Ld\n", x1-x2);
		else
			printf("good  %Ld\n", x1-x2);
		x1 = x2;
	}
}

(the "good.." printf can be commented out if you run this on the host, but it better stay if you run this in the guest because it helps rescheduling X with SDL that increases the frequency of the CPU-switches of the kvm task)

So in short with the below fix applied, after a write_tsc(0,0), the UP guest never return any error anymore. Previously it would return frequent errors because sched_in wasn't properly invoked by svm.c and it would crash at boot every single time after a write_tsc(0,0).

The SMP guest of course still returns TSC errors but that's ok, the smp host also return TSC errors, that's ok, it's only the UP guest that is forbidden to have a not monotone TSC or the guest would crash like it happened to me.

I'm unsure if special_reload_db7 is needed at all, but it certainly can't hurt so it's the only hack I left.

Finally I can enjoy KVM stability too ;). If you always compiled your host kernel with CONFIG_KVM=y on a recent kernels including the preempt-notifiers, you could never run into this. If you compile your host kernel with CONFIG_KVM=n please try to test this.

index 7993aa2..5187c96 100644
@@ -24,32 +24,6 @@
     printf("MODULE_INFO(version, \"%s\");\n", version)
 }
 
-/^static unsigned long vmcs_readl/ {
-    in_vmcs_read = 1
-}
-
-/ASM_VMX_VMREAD_RDX_RAX/ && in_vmcs_read {
-    printf("\tstart_special_insn();\n")
-}
-
-/return/ && in_vmcs_read {
-    printf("\tend_special_insn();\n");
-    in_vmcs_read = 0
-}
-
-/^static void vmcs_writel/ {
-    in_vmcs_write = 1
-}
-
-/ASM_VMX_VMWRITE_RAX_RDX/ && in_vmcs_write {
-    printf("\tstart_special_insn();\n")
-}
-
-/if/ && in_vmcs_write {
-    printf("\tend_special_insn();\n");
-    in_vmcs_write = 0
-}
-
 /^static void vmx_load_host_state/ {
     vmx_load_host_state = 1
 }

@@ -74,15 +48,6 @@
     print "\tspecial_reload_dr7();"
 }
 
-/static void vcpu_put|static int __vcpu_run|static struct kvm_vcpu \*vmx_create_vcpu/ {
-    in_tricky_func = 1
-}
-
-/preempt_disable|get_cpu/ && in_tricky_func {
-    printf("\tin_special_section();\n");
-    in_tricky_func = 0
-}
-
 /unsigned long flags;/ &&  vmx_load_host_state {
     print "\tunsigned long gsbase;"
 }

@@ -90,4 +55,3 @@
 /local_irq_save/ &&  vmx_load_host_state {
     print "\t\tgsbase = vmcs_readl(HOST_GS_BASE);"
 }
-

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Andrea Arcangeli wrote:

[snip]

It's needed, vmx (and IIRC svm) will clear out db7 so we must reload it.

In fairness we need also reload it if the host had it set; it shouldn't be a hack but part of mainline.

Unfortunately, this fails badly on Intel i386:

vmwrite error means the vmcs pointer was not loaded, probably because the sched_in event did not fire after a vcpu migration.

Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Hmm ok there's a definitive bug that I forgot a int1 kernel->kernel switch on x86 has no special debug stack like on x86-64. This will have a better chance to work, hope I got all offsets right by memory.... At least the offset "32" in the leal and eax + fastcall should all be right or I doubt it could survive the double dereferencing. Likely the one-more-derefence didn't oops there because you likely have >=1g of ram and there was a 25% chance of crashing due the lack of sched-in and 75% chance of crashing in the one-more-dereference in a more meaningful way.

index 7993aa2..5187c96 100644
@@ -24,32 +24,6 @@
     printf("MODULE_INFO(version, \"%s\");\n", version)
 }
 
-/^static unsigned long vmcs_readl/ {
-    in_vmcs_read = 1
-}
-
-/ASM_VMX_VMREAD_RDX_RAX/ && in_vmcs_read {
-    printf("\tstart_special_insn();\n")
-}
-
-/return/ && in_vmcs_read {
-    printf("\tend_special_insn();\n");
-    in_vmcs_read = 0
-}
-
-/^static void vmcs_writel/ {
-    in_vmcs_write = 1
-}
-
-/ASM_VMX_VMWRITE_RAX_RDX/ && in_vmcs_write {
-    printf("\tstart_special_insn();\n")
-}
-
-/if/ && in_vmcs_write {
-    printf("\tend_special_insn();\n");
-    in_vmcs_write = 0
-}
-
 /^static void vmx_load_host_state/ {
     vmx_load_host_state = 1
 }

@@ -74,15 +48,6 @@
     print "\tspecial_reload_dr7();"
 }
 
-/static void vcpu_put|static int __vcpu_run|static struct kvm_vcpu \*vmx_create_vcpu/ {
-    in_tricky_func = 1
-}
-
-/preempt_disable|get_cpu/ && in_tricky_func {
-    printf("\tin_special_section();\n");
-    in_tricky_func = 0
-}
-
 /unsigned long flags;/ &&  vmx_load_host_state {
     print "\tunsigned long gsbase;"
 }

@@ -90,4 +55,3 @@
 /local_irq_save/ &&  vmx_load_host_state {
     print "\t\tgsbase = vmcs_readl(HOST_GS_BASE);"
 }
-

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Now I see lots of

The sched_in notifier needs to enable interrupts (but it must disable preemption to avoid recursion).

Eventually I got this:

BUG: spinlock lockup on CPU#3, qemu-system-x86/4425, c07001cc (Not tainted)
 [<f8c053bb>] __vcpu_clear+0x0/0x4a [kvm_intel]
 [<c04edec8>] _raw_spin_lock+0xb8/0xd9
 [<c0417ac3>] smp_call_function_single+0x9a/0x10c
 [<c0403126>] __switch_to+0x174/0x18e
 [<f8c05614>] vcpu_clear+0x41/0x50 [kvm_intel]
 [<f8c058a5>] vmx_vcpu_load+0x2e/0x103 [kvm_intel]
 [<f8c0516d>] vmx_vcpu_put+0xc0/0xf3 [kvm_intel]
 [<f8c5f744>] kvm_arch_vcpu_load+0x9/0xa [kvm]
 [<f8c6b961>] preempt_notifier_trigger+0x5b/0xe1 [kvm]
 [<f8c6b79a>] pn_int1_handler+0x16/0x26 [kvm]
 [<c061fa14>] __mutex_lock_slowpath+0x45/0x77
 [<c061f8ff>] mutex_lock+0x26/0x29
 [<f8c6a465>] apic_update_ppr+0x17/0x3e [kvm]
 [<f8c650ed>] kvm_mmu_page_fault+0x14/0x9b [kvm]
 [<f8c6a55a>] kvm_get_apic_interrupt+0x3a/0x4f [kvm]
 [<f8c06cdb>] kvm_handle_exit+0x6a/0x86 [kvm_intel]
 [<f8c623cb>] kvm_arch_vcpu_ioctl_run+0x2a4/0x3aa [kvm]
 [<c05e6d63>] tcp_sendmsg+0x913/0xa04
 [<f8c5f246>] kvm_vcpu_ioctl+0xce/0x298 [kvm]
 [<c0420e83>] __activate_task+0x1c/0x29
 [<c0422645>] try_to_wake_up+0x3aa/0x3b4
 [<c06205b5>] _spin_unlock_irq+0x5/0x7
 [<c061ef69>] __sched_text_start+0x999/0xa21
 [<c047d5e5>] core_sys_select+0x218/0x2f3
 [<c043b61a>] futex_wake+0xa6/0xb0
 [<c043c233>] do_futex+0x217/0xb7d
 [<c042faa1>] __dequeue_signal+0xff/0x14e
 [<c0430ca0>] dequeue_signal+0x36/0xae
 [<f8c5ee89>] kvm_vm_ioctl+0x172/0x183 [kvm]
 [<c043a03a>] ktime_get_ts+0x16/0x44
 [<c043a07a>] ktime_get+0x12/0x34
 [<c043698b>] common_timer_get+0xf4/0x130
 [<f8c5f178>] kvm_vcpu_ioctl+0x0/0x298 [kvm]
 [<c047c4a7>] do_ioctl+0x1f/0x62
 [<c047c72e>] vfs_ioctl+0x244/0x256
 [<c04ed2c0>] copy_to_user+0x3c/0x50
 [<c047c78c>] sys_ioctl+0x4c/0x64
 [<c0403f64>] syscall_call+0x7/0xb
 =======================

followed by lockup of the qemu process, but it may be due to interrupts being disabled.

error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Ok this update fixes the smp_call_function deadlock.

index 7993aa2..5187c96 100644
@@ -24,32 +24,6 @@
     printf("MODULE_INFO(version, \"%s\");\n", version)
 }
 
-/^static unsigned long vmcs_readl/ {
-    in_vmcs_read = 1
-}
-
-/ASM_VMX_VMREAD_RDX_RAX/ && in_vmcs_read {
-    printf("\tstart_special_insn();\n")
-}
-
-/return/ && in_vmcs_read {
-    printf("\tend_special_insn();\n");
-    in_vmcs_read = 0
-}
-
-/^static void vmcs_writel/ {
-    in_vmcs_write = 1
-}
-
-/ASM_VMX_VMWRITE_RAX_RDX/ && in_vmcs_write {
-    printf("\tstart_special_insn();\n")
-}
-
-/if/ && in_vmcs_write {
-    printf("\tend_special_insn();\n");
-    in_vmcs_write = 0
-}
-
 /^static void vmx_load_host_state/ {
     vmx_load_host_state = 1
 }

@@ -74,15 +48,6 @@
     print "\tspecial_reload_dr7();"
 }
 
-/static void vcpu_put|static int __vcpu_run|static struct kvm_vcpu \*vmx_create_vcpu/ {
-    in_tricky_func = 1
-}
-
-/preempt_disable|get_cpu/ && in_tricky_func {
-    printf("\tin_special_section();\n");
-    in_tricky_func = 0
-}
-
 /unsigned long flags;/ &&  vmx_load_host_state {
     print "\tunsigned long gsbase;"
 }

@@ -90,4 +55,3 @@
 /local_irq_save/ &&  vmx_load_host_state {
     print "\t\tgsbase = vmcs_readl(HOST_GS_BASE);"
 }
-

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

I was able to boot a 4-way guest on a 2-way i386 host, so looks like this works. Thanks, it will reduce the maintenance burden needed for the external module.

Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/