Skip to contentSkip to conversation

SVM: Trap on correct IP

PATCH In review Alexander Graf 2 participants 3 messages Jun 18, 2008

lore
Share & tools
lore

Alexander Graf

author
#0 lore raw

PATCH 1/1 SVM: Trap on correct IP

Hi,

Currently HLT always traps at the IP after the hlt instruction. SVM requires its trap on the instruction's IP though, which breaks the current implementation as soon as an HLT intercept occurs.

This patch does the SVM intercept with an IP value before the HLT instruction.

Alex

Signed-off-by Alexander Graf <[email protected]>
helper.h +1 -1
Index: target-i386/helper.h
===================================================================
@@ -60,7 +60,7 @@
 DEF_HELPER(void, helper_syscall, (int next_eip_addend))
 DEF_HELPER(void, helper_sysret, (int dflag))
 #endif
-DEF_HELPER(void, helper_hlt, (void))
+DEF_HELPER(void, helper_hlt, (int next_eip_addend))
 DEF_HELPER(void, helper_monitor, (target_ulong ptr))
 DEF_HELPER(void, helper_mwait, (void))
 DEF_HELPER(void, helper_debug, (void))
op_helper.c +3 -2
Index: target-i386/op_helper.c
===================================================================
@@ -4547,9 +4547,10 @@
 }
 #endif
 
-void helper_hlt(void)
+void helper_hlt(int next_eip_addend)
 {
     helper_svm_check_intercept_param(SVM_EXIT_HLT, 0);
+    EIP+=next_eip_addend;
     
     env->hflags &= ~HF_INHIBIT_IRQ_MASK; /* needed if sti is just before */
     env->halted = 1;

@@ -4575,7 +4576,7 @@
         /* more than one CPU: do not sleep because another CPU may
            wake this one */
     } else {
-        helper_hlt();
+        helper_hlt(0);
     }
 }
translate.c +2 -2
Index: target-i386/translate.c
===================================================================
@@ -6420,8 +6420,8 @@
         } else {
             if (s->cc_op != CC_OP_DYNAMIC)
                 gen_op_set_cc_op(s->cc_op);
-            gen_jmp_im(s->pc - s->cs_base);
-            tcg_gen_helper_0_0(helper_hlt);
+            gen_jmp_im(pc_start - s->cs_base);
+            tcg_gen_helper_0_1(helper_hlt, tcg_const_i32(s->pc - pc_start));
             s->is_jmp = 3;
         }
         break;
author
#1 lore raw
Quoted 6 lines · Alexander Graf

Hi,

Currently HLT always traps at the IP after the hlt instruction. SVM requires its trap on the instruction's IP though, which breaks the current implementation as soon as an HLT intercept occurs.

Quoted 8 lines

This patch does the SVM intercept with an IP value before the HLT instruction.

Alex

Signed-off-by Alexander Graf <[email protected]>

This version should also fix MWAIT.

Signed-off-by Alexander Graf <[email protected]>
helper.h +2 -2
Index: target-i386/helper.h
===================================================================
@@ -60,9 +60,9 @@
 DEF_HELPER(void, helper_syscall, (int next_eip_addend))
 DEF_HELPER(void, helper_sysret, (int dflag))
 #endif
-DEF_HELPER(void, helper_hlt, (void))
+DEF_HELPER(void, helper_hlt, (int next_eip_addend))
 DEF_HELPER(void, helper_monitor, (target_ulong ptr))
-DEF_HELPER(void, helper_mwait, (void))
+DEF_HELPER(void, helper_mwait, (int next_eip_addend))
 DEF_HELPER(void, helper_debug, (void))
 DEF_HELPER(void, helper_raise_interrupt, (int intno, int next_eip_addend))
 DEF_HELPER(void, helper_raise_exception, (int exception_index))
op_helper.c +5 -3
Index: target-i386/op_helper.c
===================================================================
@@ -4547,9 +4547,10 @@
 }
 #endif
 
-void helper_hlt(void)
+void helper_hlt(int next_eip_addend)
 {
     helper_svm_check_intercept_param(SVM_EXIT_HLT, 0);
+    EIP+=next_eip_addend;
     
     env->hflags &= ~HF_INHIBIT_IRQ_MASK; /* needed if sti is just before */
     env->halted = 1;

@@ -4565,7 +4566,7 @@
     helper_svm_check_intercept_param(SVM_EXIT_MONITOR, 0);
 }
 
-void helper_mwait(void)
+void helper_mwait(int next_eip_addend)
 {
     if ((uint32_t)ECX != 0)
         raise_exception(EXCP0D_GPF);

@@ -4575,7 +4576,8 @@
         /* more than one CPU: do not sleep because another CPU may
            wake this one */
     } else {
-        helper_hlt();
+        /* XXX: is it ok to intercept HLT when MWAIT is not intercepted? */
+        helper_hlt(next_eip_addend);
     }
 }
translate.c +4 -4
Index: target-i386/translate.c
===================================================================
@@ -6420,8 +6420,8 @@
         } else {
             if (s->cc_op != CC_OP_DYNAMIC)
                 gen_op_set_cc_op(s->cc_op);
-            gen_jmp_im(s->pc - s->cs_base);
-            tcg_gen_helper_0_0(helper_hlt);
+            gen_jmp_im(pc_start - s->cs_base);
+            tcg_gen_helper_0_1(helper_hlt, tcg_const_i32(s->pc - pc_start));
             s->is_jmp = 3;
         }
         break;

@@ -6541,8 +6541,8 @@
                         gen_op_set_cc_op(s->cc_op);
                         s->cc_op = CC_OP_DYNAMIC;
                     }
-                    gen_jmp_im(s->pc - s->cs_base);
-                    tcg_gen_helper_0_0(helper_mwait);
+                    gen_jmp_im(pc_start - s->cs_base);
+                    tcg_gen_helper_0_1(helper_mwait, tcg_const_i32(s->pc - pc_start));
                     gen_eob(s);
                     break;
                 default:
#2 lore raw

Your patch introduces a bug in mwait. Try my commited patch instead.

Fabrice.

Quoted 20 lines · Alexander Graf
Quoted 6 lines · Alexander Graf

Hi,

Currently HLT always traps at the IP after the hlt instruction. SVM requires its trap on the instruction's IP though, which breaks the current implementation as soon as an HLT intercept occurs.

Quoted 8 lines

This patch does the SVM intercept with an IP value before the HLT instruction.

Alex

Signed-off-by Alexander Graf <[email protected]>

This version should also fix MWAIT.

Signed-off-by Alexander Graf <[email protected]>