Re: SLES 10 hangs

Avi Kivity
2007-05-02T10:21:59Z

[-- 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