Re: [Qemu-devel] [PATCH] improve emulation correctness

[Dmitry Poletaev]

25.04.2014, 21:09, "Richard Henderson" <address@hidden>:

>  On 04/25/2014 01:13 AM, Dmitry Poletaev wrote:
>>   There is a set of test, that checks QEMU CPU for similar behavior with 
>> real hardware (http://roberto.greyhats.it/projects/pills.html). Test 
>> reg/pill2579.c can detect, that program is execute in emulated environment. 
>> It is related with behavior of rcl instruction. If the number of shifted 
>> bits more than 1, OF of eflags become undefined. Real CPUs does not change 
>> OF, if it is undefined. QEMU do it anyway.
>>   Emulated program can execute that test and after that can understand 
>> environment not real.
>>
>>   Signed-off-by: Dmitry Poletaev <address@hidden>
>>
>>   diff --git a/target-i386/shift_helper_template.h 
>> b/target-i386/shift_helper_template.h
>>   index cf91a2d..d5bd321 100644
>>   --- a/target-i386/shift_helper_template.h
>>   +++ b/target-i386/shift_helper_template.h
>>   @@ -64,8 +64,10 @@ target_ulong glue(helper_rcl, SUFFIX)(CPUX86State *env, 
>> target_ulong t0,
>>            }
>>            t0 = res;
>>            env->cc_src = (eflags & ~(CC_C | CC_O)) |
>>   -            (lshift(src ^ t0, 11 - (DATA_BITS - 1)) & CC_O) |
>>                ((src >> (DATA_BITS - count)) & CC_C);
>>   +        if (count == 1) {
>>   +            env->cc_src |= (lshift(src ^ t0, 11 - (DATA_BITS - 1)) & 
>> CC_O);
>>   +        }
>  This doesn't do what you say it does.  It doesn't leave O unchanged,
>  it always resets it to 0, and only sets it back to 1 if count == 1.
>
>  r~

Of course, you are right. It is more correct now.

Signed-off-by: Dmitry Poletaev <address@hidden>

diff --git a/target-i386/shift_helper_template.h 
b/target-i386/shift_helper_template.h
index cf91a2d..4fcdc19 100644
--- a/target-i386/shift_helper_template.h
+++ b/target-i386/shift_helper_template.h
@@ -63,9 +63,11 @@ target_ulong glue(helper_rcl, SUFFIX)(CPUX86State *env, 
target_ulong t0,
             res |= t0 >> (DATA_BITS + 1 - count);
         }
         t0 = res;
-        env->cc_src = (eflags & ~(CC_C | CC_O)) |
-            (lshift(src ^ t0, 11 - (DATA_BITS - 1)) & CC_O) |
+        env->cc_src = (eflags & ~CC_C) |
             ((src >> (DATA_BITS - count)) & CC_C);
+        if (count == 1) {
+            env->cc_src |= (lshift(src ^ t0, 11 - (DATA_BITS - 1)) & CC_O);
+        }
     }
     return t0;
 }
@@ -93,9 +95,11 @@ target_ulong glue(helper_rcr, SUFFIX)(CPUX86State *env, 
target_ulong t0,
             res |= t0 << (DATA_BITS + 1 - count);
         }
         t0 = res;
-        env->cc_src = (eflags & ~(CC_C | CC_O)) |
-            (lshift(src ^ t0, 11 - (DATA_BITS - 1)) & CC_O) |
+        env->cc_src = (eflags & ~CC_C) |
             ((src >> (count - 1)) & CC_C);
+        if (count == 1) {
+            env->cc_src |= (lshift(src ^ t0, 11 - (DATA_BITS - 1)) & CC_O);
+        }
     }
     return t0;
 }