[Qemu-devel] [PATCH v3 01/15] translate-all.c: Don't pass puc, locked to tb_invalidate_phys_page()

[Stefan Hajnoczi]

From: Peter Maydell <address@hidden>

The user-mode-only function tb_invalidate_phys_page() is only
called from two places:
 * page_unprotect(), which passes in a non-zero pc, a puc pointer
   and the value 'true' for the locked argument
 * page_set_flags(), which passes in a zero pc, a NULL puc pointer
   and a 'false' locked argument

If the pc is non-zero then we may call cpu_resume_from_signal(),
which does a longjmp out of the calling code (and out of the
signal handler); this is to cover the case of a target CPU with
"precise self-modifying code" (currently only x86) executing
a store instruction which modifies code in the same TB as the
store itself. Rather than doing the longjump directly here,
return a flag to the caller which indicates whether the current
TB was modified, and move the longjump to page_unprotect.

Signed-off-by: Peter Maydell <address@hidden>
Reviewed-by: Sergey Fedorov <address@hidden>
Acked-by: Eduardo Habkost <address@hidden>
Acked-by: Riku Voipio <address@hidden>
Message-id: address@hidden
---
 translate-all.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/translate-all.c b/translate-all.c
index c599dc4..2285961 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1439,10 +1439,13 @@ void tb_invalidate_phys_page_fast(tb_page_addr_t start, 
int len)
     }
 }
 #else
-/* Called with mmap_lock held.  */
-static void tb_invalidate_phys_page(tb_page_addr_t addr,
-                                    uintptr_t pc, void *puc,
-                                    bool locked)
+/* Called with mmap_lock held. If pc is not 0 then it indicates the
+ * host PC of the faulting store instruction that caused this invalidate.
+ * Returns true if the caller needs to abort execution of the current
+ * TB (because it was modified by this store and the guest CPU has
+ * precise-SMC semantics).
+ */
+static bool tb_invalidate_phys_page(tb_page_addr_t addr, uintptr_t pc)
 {
     TranslationBlock *tb;
     PageDesc *p;
@@ -1460,7 +1463,7 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
     addr &= TARGET_PAGE_MASK;
     p = page_find(addr >> TARGET_PAGE_BITS);
     if (!p) {
-        return;
+        return false;
     }
     tb = p->first_tb;
 #ifdef TARGET_HAS_PRECISE_SMC
@@ -1499,12 +1502,10 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
            modifying the memory. It will ensure that it cannot modify
            itself */
         tb_gen_code(cpu, current_pc, current_cs_base, current_flags, 1);
-        if (locked) {
-            mmap_unlock();
-        }
-        cpu_resume_from_signal(cpu, puc);
+        return true;
     }
 #endif
+    return false;
 }
 #endif
 
@@ -1902,7 +1903,7 @@ void page_set_flags(target_ulong start, target_ulong end, 
int flags)
         if (!(p->flags & PAGE_WRITE) &&
             (flags & PAGE_WRITE) &&
             p->first_tb) {
-            tb_invalidate_phys_page(addr, 0, NULL, false);
+            tb_invalidate_phys_page(addr, 0);
         }
         p->flags = flags;
     }
@@ -1996,7 +1997,10 @@ int page_unprotect(target_ulong address, uintptr_t pc, 
void *puc)
 
             /* and since the content will be modified, we must invalidate
                the corresponding translated code. */
-            tb_invalidate_phys_page(addr, pc, puc, true);
+            if (tb_invalidate_phys_page(addr, pc)) {
+                mmap_unlock();
+                cpu_resume_from_signal(current_cpu, puc);
+            }
 #ifdef DEBUG_TB_CHECK
             tb_invalidate_check(addr);
 #endif
-- 
2.5.5