Re: SLES 10 hangs

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1090 bytes --]

Avi Kivity wrote:
> Thomas Glanzmann wrote:
>   
>> Hello Pelle,
>>
>>   
>>     
>>> Hey, I recognize that problem, although my display isn't corrupted, just 
>>> the mouse. It was resistant to X restarts. It first surfaced with kvm-21 
>>> and debian edgy on my machine. I don't believe there was an opera update 
>>> recently.
>>>     
>>>       
>> for me the same thing. Debian ETCH. And the display is only corrupted
>> for a short time, as soon as I switch applications or do a X redraw
>> everything is fine again expect for the mouse. And it popped always up
>> when I was using opera. But that just could be coincedence. I don't
>> believe in coincedence.
>>   
>>     
>
> I've just reproduced this, it's fe7dc1f2c0c3d0c21abf9dfa4387f0b748080688
> (vmx lazy fpu).
>
> I'm looking into it.
>
>
>   

The attached patch (against HEAD) fixes this issue.  It's still not
committed as I have to rework it into smaller patches (and to backport
it to the 2.6.22 patchqueue... sigh).

Please test.

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


[-- Attachment #2: fpu-fix.patch --]
[-- Type: text/x-patch, Size: 8811 bytes --]

diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h
index 9c20d5d..4d7f7c4 100644
--- a/drivers/kvm/kvm.h
+++ b/drivers/kvm/kvm.h
@@ -39,9 +39,9 @@
 
 #define KVM_GUEST_CR0_MASK \
 	(CR0_PG_MASK | CR0_PE_MASK | CR0_WP_MASK | CR0_NE_MASK \
-	 | CR0_NW_MASK | CR0_CD_MASK)
+	 | CR0_NW_MASK | CR0_CD_MASK | CR0_TS_MASK)
 #define KVM_VM_CR0_ALWAYS_ON \
-	(CR0_PG_MASK | CR0_PE_MASK | CR0_WP_MASK | CR0_NE_MASK)
+	(CR0_PG_MASK | CR0_PE_MASK | CR0_WP_MASK | CR0_NE_MASK | CR0_TS_MASK)
 #define KVM_GUEST_CR4_MASK \
 	(CR4_PSE_MASK | CR4_PAE_MASK | CR4_PGE_MASK | CR4_VMXE_MASK | CR4_VME_MASK)
 #define KVM_PMODE_VM_CR4_ALWAYS_ON (CR4_VMXE_MASK | CR4_PAE_MASK)
@@ -60,7 +60,7 @@
 
 #define FX_IMAGE_SIZE 512
 #define FX_IMAGE_ALIGN 16
-#define FX_BUF_SIZE (2 * FX_IMAGE_SIZE + FX_IMAGE_ALIGN)
+#define FX_BUF_SIZE (FX_IMAGE_SIZE + FX_IMAGE_ALIGN)
 
 #define DE_VECTOR 0
 #define NM_VECTOR 7
@@ -301,7 +301,6 @@ struct kvm_vcpu {
 	struct kvm_guest_debug guest_debug;
 
 	char fx_buf[FX_BUF_SIZE];
-	char *host_fx_image;
 	char *guest_fx_image;
 	int fpu_active;
 
diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index a3723dd..7c5dede 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -40,6 +40,7 @@
 #include <linux/file.h>
 #include <linux/fs.h>
 #include <linux/mount.h>
+#include <asm/i387.h>
 
 #include "x86_emulate.h"
 #include "segment_descriptor.h"
@@ -615,10 +616,10 @@ void fx_init(struct kvm_vcpu *vcpu)
 
 	} *fx_image;
 
-	fx_save(vcpu->host_fx_image);
+	kernel_fpu_begin();
 	fpu_init();
 	fx_save(vcpu->guest_fx_image);
-	fx_restore(vcpu->host_fx_image);
+	kernel_fpu_end();
 
 	fx_image = (struct fx_image_s *)vcpu->guest_fx_image;
 	fx_image->mxcsr = 0x1f80;
@@ -2315,9 +2316,8 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, int n)
 		goto out_free_run;
 	vcpu->pio_data = page_address(page);
 
-	vcpu->host_fx_image = (char*)ALIGN((hva_t)vcpu->fx_buf,
+	vcpu->guest_fx_image = (char*)ALIGN((hva_t)vcpu->fx_buf,
 					   FX_IMAGE_ALIGN);
-	vcpu->guest_fx_image = vcpu->host_fx_image + FX_IMAGE_SIZE;
 	vcpu->cr0 = 0x10;
 
 	r = kvm_arch_ops->vcpu_create(vcpu);
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
index b621403..99ab4ce 100644
--- a/drivers/kvm/svm.c
+++ b/drivers/kvm/svm.c
@@ -20,6 +20,7 @@
 #include <linux/highmem.h>
 #include <linux/profile.h>
 #include <asm/desc.h>
+#include <asm/i387.h>
 
 #include "kvm_svm.h"
 #include "x86_emulate.h"
@@ -1505,7 +1506,7 @@ again:
 	}
 
 	if (vcpu->fpu_active) {
-		fx_save(vcpu->host_fx_image);
+		kernel_fpu_begin();
 		fx_restore(vcpu->guest_fx_image);
 	}
 
@@ -1620,7 +1621,7 @@ again:
 
 	if (vcpu->fpu_active) {
 		fx_save(vcpu->guest_fx_image);
-		fx_restore(vcpu->host_fx_image);
+		kernel_fpu_end();
 	}
 
 	if ((vcpu->svm->vmcb->save.dr7 & 0xff))
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index 19edb34..95c5262 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -24,6 +24,7 @@
 #include <linux/profile.h>
 #include <asm/io.h>
 #include <asm/desc.h>
+#include <asm/i387.h>
 
 #include "segment_descriptor.h"
 
@@ -332,6 +333,40 @@ static void vmx_inject_gp(struct kvm_vcpu *vcpu, unsigned error_code)
 		     INTR_INFO_VALID_MASK);
 }
 
+static void update_exception_bitmap(struct kvm_vcpu *vcpu)
+{
+	u32 eb = 0;
+
+	if (vcpu->rmode.active)
+		eb |= ~0u;
+	else
+		eb |= (1u << PF_VECTOR) | (1 << NM_VECTOR);
+	if (vcpu->fpu_active)
+		eb &= ~(1u << NM_VECTOR);
+	vmcs_write32(EXCEPTION_BITMAP, eb);
+}
+
+static void vmx_activate_fpu(struct kvm_vcpu *vcpu)
+{
+	if (vcpu->fpu_active)
+		return;
+	vcpu->fpu_active = 1;
+	vmcs_clear_bits(GUEST_CR0, CR0_TS_MASK);
+	if (vcpu->cr0 & CR0_TS_MASK)
+		vmcs_set_bits(GUEST_CR0, CR0_TS_MASK);
+	update_exception_bitmap(vcpu);
+}
+
+static void vmx_deactivate_fpu(struct kvm_vcpu *vcpu)
+{
+	if (!vcpu->fpu_active || !(vcpu->cr0 & CR0_PE_MASK))
+		return;
+	vcpu->fpu_active = 0;
+	vmcs_set_bits(GUEST_CR0, CR0_TS_MASK);
+	vmcs_set_bits(EXCEPTION_BITMAP, 1 << NM_VECTOR);
+	update_exception_bitmap(vcpu);
+}
+
 /*
  * Set up the vmcs to automatically save and restore system
  * msrs.  Don't touch the 64-bit msrs if the guest is in legacy
@@ -538,10 +573,8 @@ static void vcpu_put_rsp_rip(struct kvm_vcpu *vcpu)
 static int set_guest_debug(struct kvm_vcpu *vcpu, struct kvm_debug_guest *dbg)
 {
 	unsigned long dr7 = 0x400;
-	u32 exception_bitmap;
 	int old_singlestep;
 
-	exception_bitmap = vmcs_read32(EXCEPTION_BITMAP);
 	old_singlestep = vcpu->guest_debug.singlestep;
 
 	vcpu->guest_debug.enabled = dbg->enabled;
@@ -557,11 +590,13 @@ static int set_guest_debug(struct kvm_vcpu *vcpu, struct kvm_debug_guest *dbg)
 			dr7 |= 0 << (i*4+16); /* execution breakpoint */
 		}
 
-		exception_bitmap |= (1u << 1);  /* Trap debug exceptions */
+		/* Trap debug exceptions */
+		vmcs_set_bits(EXCEPTION_BITMAP, 1u << 1);
 
 		vcpu->guest_debug.singlestep = dbg->singlestep;
 	} else {
-		exception_bitmap &= ~(1u << 1); /* Ignore debug exceptions */
+		/* Ignore debug exceptions */
+		vmcs_clear_bits(EXCEPTION_BITMAP, 1u << 1);
 		vcpu->guest_debug.singlestep = 0;
 	}
 
@@ -573,7 +608,6 @@ static int set_guest_debug(struct kvm_vcpu *vcpu, struct kvm_debug_guest *dbg)
 		vmcs_writel(GUEST_RFLAGS, flags);
 	}
 
-	vmcs_write32(EXCEPTION_BITMAP, exception_bitmap);
 	vmcs_writel(GUEST_DR7, dr7);
 
 	return 0;
@@ -687,14 +721,6 @@ static __exit void hardware_unsetup(void)
 	free_kvm_area();
 }
 
-static void update_exception_bitmap(struct kvm_vcpu *vcpu)
-{
-	if (vcpu->rmode.active)
-		vmcs_write32(EXCEPTION_BITMAP, ~0);
-	else
-		vmcs_write32(EXCEPTION_BITMAP, 1 << PF_VECTOR);
-}
-
 static void fix_pmode_dataseg(int seg, struct kvm_save_segment *save)
 {
 	struct kvm_vmx_segment_field *sf = &kvm_vmx_segment_fields[seg];
@@ -860,26 +886,21 @@ static void vmx_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
 	}
 #endif
 
-	if (!(cr0 & CR0_TS_MASK)) {
-		vcpu->fpu_active = 1;
-		vmcs_clear_bits(EXCEPTION_BITMAP, CR0_TS_MASK);
-	}
+	vmx_deactivate_fpu(vcpu);
 
 	vmcs_writel(CR0_READ_SHADOW, cr0);
 	vmcs_writel(GUEST_CR0,
 		    (cr0 & ~KVM_GUEST_CR0_MASK) | KVM_VM_CR0_ALWAYS_ON);
 	vcpu->cr0 = cr0;
+
+	if (!(cr0 & CR0_TS_MASK))
+		vmx_activate_fpu(vcpu);
 }
 
 static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
 {
 	vmcs_writel(GUEST_CR3, cr3);
-
-	if (!(vcpu->cr0 & CR0_TS_MASK)) {
-		vcpu->fpu_active = 0;
-		vmcs_set_bits(GUEST_CR0, CR0_TS_MASK);
-		vmcs_set_bits(EXCEPTION_BITMAP, 1 << NM_VECTOR);
-	}
+	vmx_deactivate_fpu(vcpu);
 }
 
 static void vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
@@ -1249,6 +1270,7 @@ static int vmx_vcpu_setup(struct kvm_vcpu *vcpu)
 #ifdef CONFIG_X86_64
 	vmx_set_efer(vcpu, 0);
 #endif
+	vmx_activate_fpu(vcpu);
 
 	return 0;
 
@@ -1408,10 +1430,7 @@ static int handle_exception(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	}
 
 	if (is_no_device(intr_info)) {
-		vcpu->fpu_active = 1;
-		vmcs_clear_bits(EXCEPTION_BITMAP, 1 << NM_VECTOR);
-		if (!(vcpu->cr0 & CR0_TS_MASK))
-			vmcs_clear_bits(GUEST_CR0, CR0_TS_MASK);
+		vmx_activate_fpu(vcpu);
 		return 1;
 	}
 
@@ -1603,12 +1622,10 @@ static int handle_cr(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 		break;
 	case 2: /* clts */
 		vcpu_load_rsp_rip(vcpu);
-		vcpu->fpu_active = 1;
-		vmcs_clear_bits(EXCEPTION_BITMAP, 1 << NM_VECTOR);
-		vmcs_clear_bits(GUEST_CR0, CR0_TS_MASK);
 		vcpu->cr0 &= ~CR0_TS_MASK;
 		vmcs_writel(CR0_READ_SHADOW, vcpu->cr0);
 		skip_emulated_instruction(vcpu);
+		vmx_activate_fpu(vcpu);
 		return 1;
 	case 1: /*mov from cr*/
 		switch (cr) {
@@ -1824,8 +1841,10 @@ static int vmx_vcpu_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	u16 fs_sel, gs_sel, ldt_sel;
 	int fs_gs_ldt_reload_needed;
 	int r;
+	int guest_fpu_loaded;
 
 preempted:
+	guest_fpu_loaded = 0;
 	/*
 	 * Set host fs and gs selectors.  Unfortunately, 22.2.3 does not
 	 * allow segment selectors with cpl > 0 or ti == 1.
@@ -1861,11 +1880,6 @@ preempted:
 	if (vcpu->guest_debug.enabled)
 		kvm_guest_debug_pre(vcpu);
 
-	if (vcpu->fpu_active) {
-		fx_save(vcpu->host_fx_image);
-		fx_restore(vcpu->guest_fx_image);
-	}
-
 #ifdef CONFIG_X86_64
 	if (is_long_mode(vcpu)) {
 		save_msrs(vcpu->host_msrs + msr_offset_kernel_gs_base, 1);
@@ -1874,6 +1888,12 @@ preempted:
 #endif
 
 again:
+	if (vcpu->fpu_active && !guest_fpu_loaded) {
+		kernel_fpu_begin();
+		fx_restore(vcpu->guest_fx_image);
+		guest_fpu_loaded = 1;
+	}
+
 	asm (
 		/* Store host registers */
 		"pushf \n\t"
@@ -2059,9 +2079,10 @@ out:
 	}
 #endif
 
-	if (vcpu->fpu_active) {
+	if (guest_fpu_loaded) {
 		fx_save(vcpu->guest_fx_image);
-		fx_restore(vcpu->host_fx_image);
+		kernel_fpu_end();
+		guest_fpu_loaded = 0;
 	}
 
 	if (r > 0) {
@@ -2141,7 +2162,6 @@ static int vmx_create_vcpu(struct kvm_vcpu *vcpu)
 	vmcs_clear(vmcs);
 	vcpu->vmcs = vmcs;
 	vcpu->launched = 0;
-	vcpu->fpu_active = 1;
 
 	return 0;
 

[-- Attachment #3: Type: text/plain, Size: 286 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[-- Attachment #4: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel