Patchwork [uq/master] KVM: x86: Fix eflags corruption in kvm mode

login
register
mail settings
Submitter Jan Kiszka
Date Feb. 19, 2010, 5:21 p.m.
Message ID <4B7EC890.8000309@siemens.com>
Download mbox | patch
Permalink /patch/45866/
State New
Headers show

Comments

Jan Kiszka - Feb. 19, 2010, 5:21 p.m.
This should explain a lot of the weird breakages of upstream KVM we've
seen recently (actually we should have seen it much earlier):

Stop translating eflags into TCG format when in kvm mode as we never
translate it back and rather sync this broken state into the kernel.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

qemu-kvm is not affected as it has it own cpu loop - maybe the way to go
for upstream as well on the long-term.

 cpu-exec.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)
Avi Kivity - Feb. 21, 2010, 9:14 a.m.
On 02/19/2010 07:21 PM, Jan Kiszka wrote:
> This should explain a lot of the weird breakages of upstream KVM we've
> seen recently (actually we should have seen it much earlier):
>
> Stop translating eflags into TCG format when in kvm mode as we never
> translate it back and rather sync this broken state into the kernel.
>
>    

Applied to uq/master and uq/stable-0.12, thanks (though realistically 
0.12 kvm users should stick with qemu-kvm).

> qemu-kvm is not affected as it has it own cpu loop - maybe the way to go
> for upstream as well on the long-term.
>
>    

Or use a skeleton main loop and a function pointer table, like kvm does 
with vmx and svm.
Jan Kiszka - Feb. 22, 2010, 8:13 a.m.
Avi Kivity wrote:
> On 02/19/2010 07:21 PM, Jan Kiszka wrote:
>> This should explain a lot of the weird breakages of upstream KVM we've
>> seen recently (actually we should have seen it much earlier):
>>
>> Stop translating eflags into TCG format when in kvm mode as we never
>> translate it back and rather sync this broken state into the kernel.
>>
>>    
> 
> Applied to uq/master and uq/stable-0.12, thanks (though realistically 
> 0.12 kvm users should stick with qemu-kvm).
> 

Actually, I would prefer if more "light" kvm users (no demanding
performance requirements, no dependency on qemu-kvm-only features)
consider upstream instead of qemu-kvm so that it gains way more testing
than just by us when pushing some more bits. So we should keep on
informing people about limitations and known issues. We need them on
both sides.

Jan
Avi Kivity - Feb. 22, 2010, 8:24 a.m.
On 02/22/2010 10:13 AM, Jan Kiszka wrote:
> Avi Kivity wrote:
>    
>> On 02/19/2010 07:21 PM, Jan Kiszka wrote:
>>      
>>> This should explain a lot of the weird breakages of upstream KVM we've
>>> seen recently (actually we should have seen it much earlier):
>>>
>>> Stop translating eflags into TCG format when in kvm mode as we never
>>> translate it back and rather sync this broken state into the kernel.
>>>
>>>
>>>        
>> Applied to uq/master and uq/stable-0.12, thanks (though realistically
>> 0.12 kvm users should stick with qemu-kvm).
>>
>>      
> Actually, I would prefer if more "light" kvm users (no demanding
> performance requirements, no dependency on qemu-kvm-only features)
> consider upstream instead of qemu-kvm so that it gains way more testing
> than just by us when pushing some more bits. So we should keep on
> informing people about limitations and known issues. We need them on
> both sides.
>
>    

Agreed.

Patch

diff --git a/cpu-exec.c b/cpu-exec.c
index 6a290fd..4029ea2 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -228,11 +228,13 @@  int cpu_exec(CPUState *env1)
     env = env1;
 
 #if defined(TARGET_I386)
-    /* put eflags in CPU temporary format */
-    CC_SRC = env->eflags & (CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
-    DF = 1 - (2 * ((env->eflags >> 10) & 1));
-    CC_OP = CC_OP_EFLAGS;
-    env->eflags &= ~(DF_MASK | CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
+    if (!kvm_enabled()) {
+        /* put eflags in CPU temporary format */
+        CC_SRC = env->eflags & (CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
+        DF = 1 - (2 * ((env->eflags >> 10) & 1));
+        CC_OP = CC_OP_EFLAGS;
+        env->eflags &= ~(DF_MASK | CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
+    }
 #elif defined(TARGET_SPARC)
 #elif defined(TARGET_M68K)
     env->cc_op = CC_OP_FLAGS;