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

Submitted by Jan Kiszka on Feb. 19, 2010, 5:21 p.m.

Details

Message ID 4B7EC890.8000309@siemens.com
State New
Headers show

Commit Message

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

Comments

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 hide | download patch | download mbox

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;