[Qemu-devel] [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL - v2

[Jan Kiszka <jan.kiszka@web.de>]

Fabrice Bellard wrote:
> Jan Kiszka wrote:
>> With the help of SSTEP_INTERNAL, we can overcome CF_SINGLE_INSN and,
>> thus, tb_gen_code.
> 
> Does your code avoid looping if an instruction modifies itself ?

Err, well, of course not. Not completely understanding what I patched, I
missed the meaning of those two CF_SINGLE_INSN checks. Version below
gets this right. Successfully tested with qemu-x86_64 system&userspace.
An updated patch 3 will follow, too. Thanks!

------------

With the help of SSTEP_INTERNAL, we can overcome CF_SINGLE_INSN and,
thus, tb_gen_code with its setup code.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 exec.c |   75 +++++------------------------------------------------------------
 1 file changed, 6 insertions(+), 69 deletions(-)

Index: b/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -723,43 +723,6 @@ static void build_page_bitmap(PageDesc *
     }
 }
 
-#ifdef TARGET_HAS_PRECISE_SMC
-
-static void tb_gen_code(CPUState *env,
-                        target_ulong pc, target_ulong cs_base, int flags,
-                        int cflags)
-{
-    TranslationBlock *tb;
-    uint8_t *tc_ptr;
-    target_ulong phys_pc, phys_page2, virt_page2;
-    int code_gen_size;
-
-    phys_pc = get_phys_addr_code(env, pc);
-    tb = tb_alloc(pc);
-    if (!tb) {
-        /* flush must be done */
-        tb_flush(env);
-        /* cannot fail at this point */
-        tb = tb_alloc(pc);
-    }
-    tc_ptr = code_gen_ptr;
-    tb->tc_ptr = tc_ptr;
-    tb->cs_base = cs_base;
-    tb->flags = flags;
-    tb->cflags = cflags;
-    cpu_gen_code(env, tb, &code_gen_size);
-    code_gen_ptr = (void *)(((unsigned long)code_gen_ptr + code_gen_size + CODE_GEN_ALIGN - 1) & ~(CODE_GEN_ALIGN - 1));
-
-    /* check next page if needed */
-    virt_page2 = (pc + tb->size - 1) & TARGET_PAGE_MASK;
-    phys_page2 = -1;
-    if ((pc & TARGET_PAGE_MASK) != virt_page2) {
-        phys_page2 = get_phys_addr_code(env, virt_page2);
-    }
-    tb_link_phys(tb, phys_pc, phys_page2);
-}
-#endif
-
 /* invalidate all TBs which intersect with the target physical page
    starting in range [start;end[. NOTE: start and end must refer to
    the same physical page. 'is_cpu_write_access' should be true if called
@@ -768,12 +731,11 @@ static void tb_gen_code(CPUState *env,
 void tb_invalidate_phys_page_range(target_phys_addr_t start, target_phys_addr_t end,
                                    int is_cpu_write_access)
 {
-    int n, current_tb_modified, current_tb_not_found, current_flags;
+    int n, current_tb_modified, current_tb_not_found;
     CPUState *env = cpu_single_env;
     PageDesc *p;
     TranslationBlock *tb, *tb_next, *current_tb, *saved_tb;
     target_ulong tb_start, tb_end;
-    target_ulong current_pc, current_cs_base;
 
     p = page_find(start >> TARGET_PAGE_BITS);
     if (!p)
@@ -790,9 +752,6 @@ void tb_invalidate_phys_page_range(targe
     current_tb_not_found = is_cpu_write_access;
     current_tb_modified = 0;
     current_tb = NULL; /* avoid warning */
-    current_pc = 0; /* avoid warning */
-    current_cs_base = 0; /* avoid warning */
-    current_flags = 0; /* avoid warning */
     tb = p->first_tb;
     while (tb != NULL) {
         n = (long)tb & 3;
@@ -819,7 +778,7 @@ void tb_invalidate_phys_page_range(targe
                 }
             }
             if (current_tb == tb &&
-                !(current_tb->cflags & CF_SINGLE_INSN)) {
+                !(env->singlestep_enabled & SSTEP_INTERNAL)) {
                 /* If we are modifying the current TB, we must stop
                 its execution. We could be more precise by checking
                 that the modification is after the current PC, but it
@@ -829,14 +788,6 @@ void tb_invalidate_phys_page_range(targe
                 current_tb_modified = 1;
                 cpu_restore_state(current_tb, env,
                                   env->mem_write_pc, NULL);
-#if defined(TARGET_I386)
-                current_flags = env->hflags;
-                current_flags |= (env->eflags & (IOPL_MASK | TF_MASK | VM_MASK));
-                current_cs_base = (target_ulong)env->segs[R_CS].base;
-                current_pc = current_cs_base + env->eip;
-#else
-#error unsupported CPU
-#endif
             }
 #endif /* TARGET_HAS_PRECISE_SMC */
             /* we need to do that to handle the case where a signal
@@ -870,8 +821,7 @@ void tb_invalidate_phys_page_range(targe
            modifying the memory. It will ensure that it cannot modify
            itself */
         env->current_tb = NULL;
-        tb_gen_code(env, current_pc, current_cs_base, current_flags,
-                    CF_SINGLE_INSN);
+        env->singlestep_enabled |= SSTEP_INTERNAL;
         cpu_resume_from_signal(env, NULL);
     }
 #endif
@@ -910,8 +860,7 @@ static inline void tb_invalidate_phys_pa
 static void tb_invalidate_phys_page(target_phys_addr_t addr,
                                     unsigned long pc, void *puc)
 {
-    int n, current_flags, current_tb_modified;
-    target_ulong current_pc, current_cs_base;
+    int n, current_tb_modified;
     PageDesc *p;
     TranslationBlock *tb, *current_tb;
 #ifdef TARGET_HAS_PRECISE_SMC
@@ -925,9 +874,6 @@ static void tb_invalidate_phys_page(targ
     tb = p->first_tb;
     current_tb_modified = 0;
     current_tb = NULL;
-    current_pc = 0; /* avoid warning */
-    current_cs_base = 0; /* avoid warning */
-    current_flags = 0; /* avoid warning */
 #ifdef TARGET_HAS_PRECISE_SMC
     if (tb && pc != 0) {
         current_tb = tb_find_pc(pc);
@@ -938,7 +884,7 @@ static void tb_invalidate_phys_page(targ
         tb = (TranslationBlock *)((long)tb & ~3);
 #ifdef TARGET_HAS_PRECISE_SMC
         if (current_tb == tb &&
-            !(current_tb->cflags & CF_SINGLE_INSN)) {
+            !(env->singlestep_enabled & SSTEP_INTERNAL)) {
                 /* If we are modifying the current TB, we must stop
                    its execution. We could be more precise by checking
                    that the modification is after the current PC, but it
@@ -947,14 +893,6 @@ static void tb_invalidate_phys_page(targ
 
             current_tb_modified = 1;
             cpu_restore_state(current_tb, env, pc, puc);
-#if defined(TARGET_I386)
-            current_flags = env->hflags;
-            current_flags |= (env->eflags & (IOPL_MASK | TF_MASK | VM_MASK));
-            current_cs_base = (target_ulong)env->segs[R_CS].base;
-            current_pc = current_cs_base + env->eip;
-#else
-#error unsupported CPU
-#endif
         }
 #endif /* TARGET_HAS_PRECISE_SMC */
         tb_phys_invalidate(tb, addr);
@@ -967,8 +905,7 @@ static void tb_invalidate_phys_page(targ
            modifying the memory. It will ensure that it cannot modify
            itself */
         env->current_tb = NULL;
-        tb_gen_code(env, current_pc, current_cs_base, current_flags,
-                    CF_SINGLE_INSN);
+        env->singlestep_enabled |= SSTEP_INTERNAL;
         cpu_resume_from_signal(env, puc);
     }
 #endif