diff mbox

[2/4] Set eflags and cr0 prior to calling cpu_x86_load_seg_cache() in smm_helper.c.

Message ID 52b90f04f7c81f5627a9adba9459322d92bf3706.1398802891.git.kevin@koconnor.net
State New
Headers show

Commit Message

Kevin O'Connor April 29, 2014, 8:38 p.m. UTC 
The cpu_x86_load_seg_cache() function inspects cr0 and eflags, so make
sure all changes to eflags and cr0 are done prior to loading the
segment caches.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
---
 target-i386/smm_helper.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

Comments

Richard Henderson April 30, 2014, 4:54 p.m. UTC | #1 
On 04/29/2014 01:38 PM, Kevin O'Connor wrote:
>      cpu_load_eflags(env, 0, ~(CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C |
>                                DF_MASK));
>      env->eip = 0x00008000;
> +    cpu_x86_update_cr0(env,
> +                       env->cr[0] & ~(CR0_PE_MASK | CR0_EM_MASK | CR0_TS_MASK |
> +                                      CR0_PG_MASK));
> +    cpu_x86_update_cr4(env, 0);
> +    env->dr[7] = 0x00000400;
> +    CC_OP = CC_OP_EFLAGS;

Please place the CC_OP update immediately after cpu_load_eflags.


r~
Paolo Bonzini April 30, 2014, 5:07 p.m. UTC | #2 
Il 30/04/2014 18:54, Richard Henderson ha scritto:
> On 04/29/2014 01:38 PM, Kevin O'Connor wrote:
>>      cpu_load_eflags(env, 0, ~(CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C |
>>                                DF_MASK));
>>      env->eip = 0x00008000;
>> +    cpu_x86_update_cr0(env,
>> +                       env->cr[0] & ~(CR0_PE_MASK | CR0_EM_MASK | CR0_TS_MASK |
>> +                                      CR0_PG_MASK));
>> +    cpu_x86_update_cr4(env, 0);
>> +    env->dr[7] = 0x00000400;
>> +    CC_OP = CC_OP_EFLAGS;
>
> Please place the CC_OP update immediately after cpu_load_eflags.

I was going to move it straight inside cpu_load_eflags in a follow-up patch.

Paolo
Richard Henderson April 30, 2014, 9:58 p.m. UTC | #3 
On 04/30/2014 10:07 AM, Paolo Bonzini wrote:
> Il 30/04/2014 18:54, Richard Henderson ha scritto:
>> On 04/29/2014 01:38 PM, Kevin O'Connor wrote:
>>>      cpu_load_eflags(env, 0, ~(CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C |
>>>                                DF_MASK));
>>>      env->eip = 0x00008000;
>>> +    cpu_x86_update_cr0(env,
>>> +                       env->cr[0] & ~(CR0_PE_MASK | CR0_EM_MASK |
>>> CR0_TS_MASK |
>>> +                                      CR0_PG_MASK));
>>> +    cpu_x86_update_cr4(env, 0);
>>> +    env->dr[7] = 0x00000400;
>>> +    CC_OP = CC_OP_EFLAGS;
>>
>> Please place the CC_OP update immediately after cpu_load_eflags.
> 
> I was going to move it straight inside cpu_load_eflags in a follow-up patch.

Fair enough.


r~
diff mbox

Patch

diff --git a/target-i386/smm_helper.c b/target-i386/smm_helper.c
index 35901c9..4841d53 100644
--- a/target-i386/smm_helper.c
+++ b/target-i386/smm_helper.c
@@ -163,6 +163,13 @@  void do_smm_enter(X86CPU *cpu)
     cpu_load_eflags(env, 0, ~(CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C |
                               DF_MASK));
     env->eip = 0x00008000;
+    cpu_x86_update_cr0(env,
+                       env->cr[0] & ~(CR0_PE_MASK | CR0_EM_MASK | CR0_TS_MASK |
+                                      CR0_PG_MASK));
+    cpu_x86_update_cr4(env, 0);
+    env->dr[7] = 0x00000400;
+    CC_OP = CC_OP_EFLAGS;
+
     cpu_x86_load_seg_cache(env, R_CS, (env->smbase >> 4) & 0xffff, env->smbase,
                            0xffffffff, 0);
     cpu_x86_load_seg_cache(env, R_DS, 0, 0, 0xffffffff, 0);
@@ -170,13 +177,6 @@  void do_smm_enter(X86CPU *cpu)
     cpu_x86_load_seg_cache(env, R_SS, 0, 0, 0xffffffff, 0);
     cpu_x86_load_seg_cache(env, R_FS, 0, 0, 0xffffffff, 0);
     cpu_x86_load_seg_cache(env, R_GS, 0, 0, 0xffffffff, 0);
-
-    cpu_x86_update_cr0(env,
-                       env->cr[0] & ~(CR0_PE_MASK | CR0_EM_MASK | CR0_TS_MASK |
-                                      CR0_PG_MASK));
-    cpu_x86_update_cr4(env, 0);
-    env->dr[7] = 0x00000400;
-    CC_OP = CC_OP_EFLAGS;
 }
 
 void helper_rsm(CPUX86State *env)
@@ -191,16 +191,6 @@  void helper_rsm(CPUX86State *env)
 #ifdef TARGET_X86_64
     cpu_load_efer(env, ldq_phys(cs->as, sm_state + 0x7ed0));
 
-    for (i = 0; i < 6; i++) {
-        offset = 0x7e00 + i * 16;
-        cpu_x86_load_seg_cache(env, i,
-                               lduw_phys(cs->as, sm_state + offset),
-                               ldq_phys(cs->as, sm_state + offset + 8),
-                               ldl_phys(cs->as, sm_state + offset + 4),
-                               (lduw_phys(cs->as, sm_state + offset + 2) &
-                                0xf0ff) << 8);
-    }
-
     env->gdt.base = ldq_phys(cs->as, sm_state + 0x7e68);
     env->gdt.limit = ldl_phys(cs->as, sm_state + 0x7e64);
 
@@ -238,6 +228,16 @@  void helper_rsm(CPUX86State *env)
     cpu_x86_update_cr3(env, ldl_phys(cs->as, sm_state + 0x7f50));
     cpu_x86_update_cr0(env, ldl_phys(cs->as, sm_state + 0x7f58));
 
+    for (i = 0; i < 6; i++) {
+        offset = 0x7e00 + i * 16;
+        cpu_x86_load_seg_cache(env, i,
+                               lduw_phys(cs->as, sm_state + offset),
+                               ldq_phys(cs->as, sm_state + offset + 8),
+                               ldl_phys(cs->as, sm_state + offset + 4),
+                               (lduw_phys(cs->as, sm_state + offset + 2) &
+                                0xf0ff) << 8);
+    }
+
     val = ldl_phys(cs->as, sm_state + 0x7efc); /* revision ID */
     if (val & 0x20000) {
         env->smbase = ldl_phys(cs->as, sm_state + 0x7f00) & ~0x7fff;