All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v6 0/6] tcg: Misc clean-up patches
@ 2016-04-28 22:10 Sergey Fedorov
  2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 1/6] tcg: code_bitmap is not used by user-mode emulation Sergey Fedorov
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Sergey Fedorov @ 2016-04-28 22:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Sergey Fedorov, Paolo Bonzini,
	Peter Crosthwaite, Richard Henderson

From: Sergey Fedorov <serge.fdrv@gmail.com>

This patch series consists of various general TCG clean-up patches extracted
from Paolo's MTTCG tree [1] and Alex's MTTCG base enablement tree [2]. I also
add here a patch from myself to rework tb_invalidated_flag based on the Paolo's
"tcg: move tb_invalidated_flag to CPUState" patch from the original verions of
this series. Another patch of mine cleans up from a misleading 'next_tb'
variable.

The main idea is to review and merge these patches separately from the MTTCG
series to cut the latter and make it easier to review.

This series is based on commit 6d6e9af2ed1c ("tcg: Allow goto_tb to any
target PC in user mode") [3] and is available at [4].

[1] git://github.com/bonzini/qemu.git mttcg
[2] git://github.com/stsquad/qemu.git mttcg/base-patches-v2
[3] git://github.com/sergefdrv/qemu.git tb-chaining-cleanup-v5
[4] git://github.com/sergefdrv/qemu.git tcg-cleanup-v6

Summary of changes:
 v6:
  * Fixed rebase conflicts
 v5:
  * Shortcut non-null tests in [PATCH v5 4/6]
  * Add a patch to move TB chaining to tb_find_fast() [PATCH v5 6/6]
 v4:
  * Add a patch to clean up from 'next_tb' [PATCH v4 4/6]
 v3:
  * Add a patch to rework tb_invalidated_flag from myself [PATCH v3 4/4]
 v2:
  * Complete code_bitmap elimination [PATCH v2 1/3]
  * Take Alex's version of tb_find_physical() reorganization [PATCH v2 2/3]
  * Drop [PATCH 3/5] completely
  * Drop [PATCH 4/5] and [PATCH 5/5] for separate series
  * Take Alex's rebase of Paolo's icount code eliding [PATCH v2 3/3]

Alex Bennée (1):
  tcg: reorganize tb_find_physical loop

Paolo Bonzini (2):
  tcg: code_bitmap is not used by user-mode emulation
  cpu-exec: elide more icount code if CONFIG_USER_ONLY

Sergey Fedorov (3):
  tcg: Clean up from 'next_tb'
  tcg: Rework tb_invalidated_flag
  cpu-exec: Move TB chaining into tb_find_fast()

 cpu-exec.c              | 151 +++++++++++++++++++++++++++---------------------
 include/exec/exec-all.h |   2 -
 include/qom/cpu.h       |   2 +
 tcg/tcg.h               |  19 +++---
 tci.c                   |   6 +-
 trace-events            |   2 +-
 translate-all.c         |  16 ++---
 7 files changed, 111 insertions(+), 87 deletions(-)

-- 
2.8.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH v6 1/6] tcg: code_bitmap is not used by user-mode emulation
  2016-04-28 22:10 [Qemu-devel] [PATCH v6 0/6] tcg: Misc clean-up patches Sergey Fedorov
@ 2016-04-28 22:10 ` Sergey Fedorov
  2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 2/6] tcg: reorganize tb_find_physical loop Sergey Fedorov
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Sergey Fedorov @ 2016-04-28 22:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Sergey Fedorov, Paolo Bonzini,
	Peter Crosthwaite, Richard Henderson, Sergey Fedorov

From: Paolo Bonzini <pbonzini@redhat.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
[Sergey Fedorov: eliminate the field entirely in user-mode]
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
Reviewed-by: Richard Henderson  <rth@twiddle.net>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---

Changes in v2:
 * The field is eliminated entirely in user-mode

 translate-all.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/translate-all.c b/translate-all.c
index d679ad129e10..74fa1b4d271c 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -75,8 +75,9 @@ typedef struct PageDesc {
     /* in order to optimize self modifying code, we count the number
        of lookups we do to a given page to use a bitmap */
     unsigned int code_write_count;
+#ifdef CONFIG_SOFTMMU
     unsigned long *code_bitmap;
-#if defined(CONFIG_USER_ONLY)
+#else
     unsigned long flags;
 #endif
 } PageDesc;
@@ -783,8 +784,10 @@ void tb_free(TranslationBlock *tb)
 
 static inline void invalidate_page_bitmap(PageDesc *p)
 {
+#ifdef CONFIG_SOFTMMU
     g_free(p->code_bitmap);
     p->code_bitmap = NULL;
+#endif
     p->code_write_count = 0;
 }
 
@@ -1028,6 +1031,7 @@ void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr)
     tcg_ctx.tb_ctx.tb_phys_invalidate_count++;
 }
 
+#ifdef CONFIG_SOFTMMU
 static void build_page_bitmap(PageDesc *p)
 {
     int n, tb_start, tb_end;
@@ -1056,6 +1060,7 @@ static void build_page_bitmap(PageDesc *p)
         tb = tb->page_next[n];
     }
 }
+#endif
 
 /* add the tb in the target page and protect it if necessary
  *
@@ -1412,6 +1417,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
 #endif
 }
 
+#ifdef CONFIG_SOFTMMU
 /* len must be <= 8 and start must be a multiple of len */
 void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len)
 {
@@ -1449,8 +1455,7 @@ void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len)
         tb_invalidate_phys_page_range(start, start + len, 1);
     }
 }
-
-#if !defined(CONFIG_SOFTMMU)
+#else
 /* Called with mmap_lock held.  */
 static void tb_invalidate_phys_page(tb_page_addr_t addr,
                                     uintptr_t pc, void *puc,
-- 
2.8.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH v6 2/6] tcg: reorganize tb_find_physical loop
  2016-04-28 22:10 [Qemu-devel] [PATCH v6 0/6] tcg: Misc clean-up patches Sergey Fedorov
  2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 1/6] tcg: code_bitmap is not used by user-mode emulation Sergey Fedorov
@ 2016-04-28 22:10 ` Sergey Fedorov
  2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 3/6] cpu-exec: elide more icount code if CONFIG_USER_ONLY Sergey Fedorov
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Sergey Fedorov @ 2016-04-28 22:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Sergey Fedorov, Paolo Bonzini,
	Peter Crosthwaite, Richard Henderson, Sergey Fedorov

From: Alex Bennée <alex.bennee@linaro.org>

Put some comments and improve code structure. This should help reading
the code.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
[Sergey Fedorov: provide commit message; bring back resetting of
tb_invalidated_flag]
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
Reviewed-by: Richard Henderson  <rth@twiddle.net>
---
 cpu-exec.c | 44 ++++++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index f984dc71cbf1..02a49070b9dd 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -223,10 +223,9 @@ static TranslationBlock *tb_find_physical(CPUState *cpu,
                                           uint32_t flags)
 {
     CPUArchState *env = (CPUArchState *)cpu->env_ptr;
-    TranslationBlock *tb, **ptb1;
+    TranslationBlock *tb, **tb_hash_head, **ptb1;
     unsigned int h;
     tb_page_addr_t phys_pc, phys_page1;
-    target_ulong virt_page2;
 
     tcg_ctx.tb_ctx.tb_invalidated_flag = 0;
 
@@ -234,37 +233,42 @@ static TranslationBlock *tb_find_physical(CPUState *cpu,
     phys_pc = get_page_addr_code(env, pc);
     phys_page1 = phys_pc & TARGET_PAGE_MASK;
     h = tb_phys_hash_func(phys_pc);
-    ptb1 = &tcg_ctx.tb_ctx.tb_phys_hash[h];
-    for(;;) {
-        tb = *ptb1;
-        if (!tb) {
-            return NULL;
-        }
+
+    /* Start at head of the hash entry */
+    ptb1 = tb_hash_head = &tcg_ctx.tb_ctx.tb_phys_hash[h];
+    tb = *ptb1;
+
+    while (tb) {
         if (tb->pc == pc &&
             tb->page_addr[0] == phys_page1 &&
             tb->cs_base == cs_base &&
             tb->flags == flags) {
-            /* check next page if needed */
-            if (tb->page_addr[1] != -1) {
-                tb_page_addr_t phys_page2;
 
-                virt_page2 = (pc & TARGET_PAGE_MASK) +
-                    TARGET_PAGE_SIZE;
-                phys_page2 = get_page_addr_code(env, virt_page2);
+            if (tb->page_addr[1] == -1) {
+                /* done, we have a match */
+                break;
+            } else {
+                /* check next page if needed */
+                target_ulong virt_page2 = (pc & TARGET_PAGE_MASK) +
+                                          TARGET_PAGE_SIZE;
+                tb_page_addr_t phys_page2 = get_page_addr_code(env, virt_page2);
+
                 if (tb->page_addr[1] == phys_page2) {
                     break;
                 }
-            } else {
-                break;
             }
         }
+
         ptb1 = &tb->phys_hash_next;
+        tb = *ptb1;
     }
 
-    /* Move the TB to the head of the list */
-    *ptb1 = tb->phys_hash_next;
-    tb->phys_hash_next = tcg_ctx.tb_ctx.tb_phys_hash[h];
-    tcg_ctx.tb_ctx.tb_phys_hash[h] = tb;
+    if (tb) {
+        /* Move the TB to the head of the list */
+        *ptb1 = tb->phys_hash_next;
+        tb->phys_hash_next = *tb_hash_head;
+        *tb_hash_head = tb;
+    }
     return tb;
 }
 
-- 
2.8.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH v6 3/6] cpu-exec: elide more icount code if CONFIG_USER_ONLY
  2016-04-28 22:10 [Qemu-devel] [PATCH v6 0/6] tcg: Misc clean-up patches Sergey Fedorov
  2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 1/6] tcg: code_bitmap is not used by user-mode emulation Sergey Fedorov
  2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 2/6] tcg: reorganize tb_find_physical loop Sergey Fedorov
@ 2016-04-28 22:10 ` Sergey Fedorov
  2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 4/6] tcg: Clean up from 'next_tb' Sergey Fedorov
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Sergey Fedorov @ 2016-04-28 22:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Sergey Fedorov, Paolo Bonzini,
	Peter Crosthwaite, Richard Henderson, Sergey Fedorov

From: Paolo Bonzini <pbonzini@redhat.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
[Alex Bennée: #ifndef replay code to match elided functions]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---
 cpu-exec.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/cpu-exec.c b/cpu-exec.c
index 02a49070b9dd..bd831b5a12f9 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -192,6 +192,7 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, TranslationBlock *itb)
     return next_tb;
 }
 
+#ifndef CONFIG_USER_ONLY
 /* Execute the code without caching the generated code. An interpreter
    could be used if available. */
 static void cpu_exec_nocache(CPUState *cpu, int max_cycles,
@@ -216,6 +217,7 @@ static void cpu_exec_nocache(CPUState *cpu, int max_cycles,
     tb_phys_invalidate(tb, -1);
     tb_free(tb);
 }
+#endif
 
 static TranslationBlock *tb_find_physical(CPUState *cpu,
                                           target_ulong pc,
@@ -430,12 +432,14 @@ int cpu_exec(CPUState *cpu)
                     }
 #endif
                 }
+#ifndef CONFIG_USER_ONLY
             } else if (replay_has_exception()
                        && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
                 /* try to cause an exception pending in the log */
                 cpu_exec_nocache(cpu, 1, tb_find_fast(cpu), true);
                 ret = -1;
                 break;
+#endif
             }
 
             next_tb = 0; /* force lookup of first TB */
@@ -542,6 +546,9 @@ int cpu_exec(CPUState *cpu)
                     case TB_EXIT_ICOUNT_EXPIRED:
                     {
                         /* Instruction counter expired.  */
+#ifdef CONFIG_USER_ONLY
+                        abort();
+#else
                         int insns_left = cpu->icount_decr.u32;
                         if (cpu->icount_extra && insns_left >= 0) {
                             /* Refill decrementer and continue execution.  */
@@ -561,6 +568,7 @@ int cpu_exec(CPUState *cpu)
                             cpu_loop_exit(cpu);
                         }
                         break;
+#endif
                     }
                     default:
                         break;
-- 
2.8.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH v6 4/6] tcg: Clean up from 'next_tb'
  2016-04-28 22:10 [Qemu-devel] [PATCH v6 0/6] tcg: Misc clean-up patches Sergey Fedorov
                   ` (2 preceding siblings ...)
  2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 3/6] cpu-exec: elide more icount code if CONFIG_USER_ONLY Sergey Fedorov
@ 2016-04-28 22:10 ` Sergey Fedorov
  2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 5/6] tcg: Rework tb_invalidated_flag Sergey Fedorov
  2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 6/6] cpu-exec: Move TB chaining into tb_find_fast() Sergey Fedorov
  5 siblings, 0 replies; 11+ messages in thread
From: Sergey Fedorov @ 2016-04-28 22:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Sergey Fedorov, Paolo Bonzini,
	Peter Crosthwaite, Richard Henderson, Sergey Fedorov,
	Stefan Weil

From: Sergey Fedorov <serge.fdrv@gmail.com>

The value returned from tcg_qemu_tb_exec() is the value passed to the
corresponding tcg_gen_exit_tb() at translation time of the last TB
attempted to execute. It is a little confusing to store it in a variable
named 'next_tb'. In fact, it is a combination of 4-byte aligned pointer
and additional information in its two least significant bits. Break it
down right away into two variables named 'last_tb' and 'tb_exit' which
are a pointer to the last TB attempted to execute and the TB exit
reason, correspondingly. This simplifies the code and improves its
readability.

Correct a misleading documentation comment for tcg_qemu_tb_exec() and
fix logging in cpu_tb_exec(). Also rename a misleading 'next_tb' in
another couple of places.

Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---

Changes in v6:
 * Fixed rebase conflicts
Changes in v5:
 * Shortcut a non-null test

 cpu-exec.c   | 59 ++++++++++++++++++++++++++++++++---------------------------
 tcg/tcg.h    | 19 ++++++++++---------
 tci.c        |  6 +++---
 trace-events |  2 +-
 4 files changed, 46 insertions(+), 40 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index bd831b5a12f9..9407c66f626d 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -136,7 +136,9 @@ static void init_delay_params(SyncClocks *sc, const CPUState *cpu)
 static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, TranslationBlock *itb)
 {
     CPUArchState *env = cpu->env_ptr;
-    uintptr_t next_tb;
+    uintptr_t ret;
+    TranslationBlock *last_tb;
+    int tb_exit;
     uint8_t *tb_ptr = itb->tc_ptr;
 
     qemu_log_mask_and_addr(CPU_LOG_EXEC, itb->pc,
@@ -160,36 +162,37 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, TranslationBlock *itb)
 #endif /* DEBUG_DISAS */
 
     cpu->can_do_io = !use_icount;
-    next_tb = tcg_qemu_tb_exec(env, tb_ptr);
+    ret = tcg_qemu_tb_exec(env, tb_ptr);
     cpu->can_do_io = 1;
-    trace_exec_tb_exit((void *) (next_tb & ~TB_EXIT_MASK),
-                       next_tb & TB_EXIT_MASK);
+    last_tb = (TranslationBlock *)(ret & ~TB_EXIT_MASK);
+    tb_exit = ret & TB_EXIT_MASK;
+    trace_exec_tb_exit(last_tb, tb_exit);
 
-    if ((next_tb & TB_EXIT_MASK) > TB_EXIT_IDX1) {
+    if (tb_exit > TB_EXIT_IDX1) {
         /* We didn't start executing this TB (eg because the instruction
          * counter hit zero); we must restore the guest PC to the address
          * of the start of the TB.
          */
         CPUClass *cc = CPU_GET_CLASS(cpu);
-        TranslationBlock *tb = (TranslationBlock *)(next_tb & ~TB_EXIT_MASK);
-        qemu_log_mask_and_addr(CPU_LOG_EXEC, itb->pc,
+        qemu_log_mask_and_addr(CPU_LOG_EXEC, last_tb->pc,
                                "Stopped execution of TB chain before %p ["
                                TARGET_FMT_lx "] %s\n",
-                               itb->tc_ptr, itb->pc, lookup_symbol(itb->pc));
+                               last_tb->tc_ptr, last_tb->pc,
+                               lookup_symbol(last_tb->pc));
         if (cc->synchronize_from_tb) {
-            cc->synchronize_from_tb(cpu, tb);
+            cc->synchronize_from_tb(cpu, last_tb);
         } else {
             assert(cc->set_pc);
-            cc->set_pc(cpu, tb->pc);
+            cc->set_pc(cpu, last_tb->pc);
         }
     }
-    if ((next_tb & TB_EXIT_MASK) == TB_EXIT_REQUESTED) {
+    if (tb_exit == TB_EXIT_REQUESTED) {
         /* We were asked to stop executing TBs (probably a pending
          * interrupt. We've now stopped, so clear the flag.
          */
         cpu->tcg_exit_req = 0;
     }
-    return next_tb;
+    return ret;
 }
 
 #ifndef CONFIG_USER_ONLY
@@ -358,8 +361,8 @@ int cpu_exec(CPUState *cpu)
     CPUArchState *env = &x86_cpu->env;
 #endif
     int ret, interrupt_request;
-    TranslationBlock *tb;
-    uintptr_t next_tb;
+    TranslationBlock *tb, *last_tb;
+    int tb_exit = 0;
     SyncClocks sc;
 
     /* replay_interrupt may need current_cpu */
@@ -442,7 +445,7 @@ int cpu_exec(CPUState *cpu)
 #endif
             }
 
-            next_tb = 0; /* force lookup of first TB */
+            last_tb = NULL; /* forget the last executed TB after exception */
             for(;;) {
                 interrupt_request = cpu->interrupt_request;
                 if (unlikely(interrupt_request)) {
@@ -487,7 +490,7 @@ int cpu_exec(CPUState *cpu)
                     else {
                         replay_interrupt();
                         if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
-                            next_tb = 0;
+                            last_tb = NULL;
                         }
                     }
                     /* Don't use the cached interrupt_request value,
@@ -496,7 +499,7 @@ int cpu_exec(CPUState *cpu)
                         cpu->interrupt_request &= ~CPU_INTERRUPT_EXITTB;
                         /* ensure that no TB jump will be modified as
                            the program flow was changed */
-                        next_tb = 0;
+                        last_tb = NULL;
                     }
                 }
                 if (unlikely(cpu->exit_request
@@ -513,22 +516,24 @@ int cpu_exec(CPUState *cpu)
                     /* as some TB could have been invalidated because
                        of memory exceptions while generating the code, we
                        must recompute the hash index here */
-                    next_tb = 0;
+                    last_tb = NULL;
                     tcg_ctx.tb_ctx.tb_invalidated_flag = 0;
                 }
                 /* See if we can patch the calling TB. */
-                if (next_tb != 0 && !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
-                    tb_add_jump((TranslationBlock *)(next_tb & ~TB_EXIT_MASK),
-                                next_tb & TB_EXIT_MASK, tb);
+                if (last_tb && !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
+                    tb_add_jump(last_tb, tb_exit, tb);
                 }
                 tb_unlock();
                 if (likely(!cpu->exit_request)) {
+                    uintptr_t ret;
                     trace_exec_tb(tb, tb->pc);
                     /* execute the generated code */
                     cpu->current_tb = tb;
-                    next_tb = cpu_tb_exec(cpu, tb);
+                    ret = cpu_tb_exec(cpu, tb);
                     cpu->current_tb = NULL;
-                    switch (next_tb & TB_EXIT_MASK) {
+                    last_tb = (TranslationBlock *)(ret & ~TB_EXIT_MASK);
+                    tb_exit = ret & TB_EXIT_MASK;
+                    switch (tb_exit) {
                     case TB_EXIT_REQUESTED:
                         /* Something asked us to stop executing
                          * chained TBs; just continue round the main
@@ -541,7 +546,7 @@ int cpu_exec(CPUState *cpu)
                          * or cpu->interrupt_request.
                          */
                         smp_rmb();
-                        next_tb = 0;
+                        last_tb = NULL;
                         break;
                     case TB_EXIT_ICOUNT_EXPIRED:
                     {
@@ -559,12 +564,12 @@ int cpu_exec(CPUState *cpu)
                         } else {
                             if (insns_left > 0) {
                                 /* Execute remaining instructions.  */
-                                tb = (TranslationBlock *)(next_tb & ~TB_EXIT_MASK);
-                                cpu_exec_nocache(cpu, insns_left, tb, false);
+                                cpu_exec_nocache(cpu, insns_left,
+                                                 last_tb, false);
                                 align_clocks(&sc, cpu);
                             }
                             cpu->exception_index = EXCP_INTERRUPT;
-                            next_tb = 0;
+                            last_tb = NULL;
                             cpu_loop_exit(cpu);
                         }
                         break;
diff --git a/tcg/tcg.h b/tcg/tcg.h
index ea4ff02308fc..04e962e0b41d 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -919,7 +919,7 @@ static inline unsigned get_mmuidx(TCGMemOpIdx oi)
 
 /**
  * tcg_qemu_tb_exec:
- * @env: CPUArchState * for the CPU
+ * @env: pointer to CPUArchState for the CPU
  * @tb_ptr: address of generated code for the TB to execute
  *
  * Start executing code from a given translation block.
@@ -930,30 +930,31 @@ static inline unsigned get_mmuidx(TCGMemOpIdx oi)
  * which has not yet been directly linked, or an asynchronous
  * event such as an interrupt needs handling.
  *
- * The return value is a pointer to the next TB to execute
- * (if known; otherwise zero). This pointer is assumed to be
- * 4-aligned, and the bottom two bits are used to return further
- * information:
+ * Return: The return value is the value passed to the corresponding
+ * tcg_gen_exit_tb() at translation time of the last TB attempted to execute.
+ * The value is either zero or a 4-byte aligned pointer to that TB combined
+ * with additional information in its two least significant bits. The
+ * additional information is encoded as follows:
  *  0, 1: the link between this TB and the next is via the specified
  *        TB index (0 or 1). That is, we left the TB via (the equivalent
  *        of) "goto_tb <index>". The main loop uses this to determine
  *        how to link the TB just executed to the next.
  *  2:    we are using instruction counting code generation, and we
  *        did not start executing this TB because the instruction counter
- *        would hit zero midway through it. In this case the next-TB pointer
+ *        would hit zero midway through it. In this case the pointer
  *        returned is the TB we were about to execute, and the caller must
  *        arrange to execute the remaining count of instructions.
  *  3:    we stopped because the CPU's exit_request flag was set
  *        (usually meaning that there is an interrupt that needs to be
- *        handled). The next-TB pointer returned is the TB we were
- *        about to execute when we noticed the pending exit request.
+ *        handled). The pointer returned is the TB we were about to execute
+ *        when we noticed the pending exit request.
  *
  * If the bottom two bits indicate an exit-via-index then the CPU
  * state is correctly synchronised and ready for execution of the next
  * TB (and in particular the guest PC is the address to execute next).
  * Otherwise, we gave up on execution of this TB before it started, and
  * the caller must fix up the CPU state by calling the CPU's
- * synchronize_from_tb() method with the next-TB pointer we return (falling
+ * synchronize_from_tb() method with the TB pointer we return (falling
  * back to calling the CPU's set_pc method with tb->pb if no
  * synchronize_from_tb() method exists).
  *
diff --git a/tci.c b/tci.c
index a8939e6d3c2b..0fdc4e2c127c 100644
--- a/tci.c
+++ b/tci.c
@@ -467,7 +467,7 @@ uintptr_t tcg_qemu_tb_exec(CPUArchState *env, uint8_t *tb_ptr)
 {
     long tcg_temps[CPU_TEMP_BUF_NLONGS];
     uintptr_t sp_value = (uintptr_t)(tcg_temps + CPU_TEMP_BUF_NLONGS);
-    uintptr_t next_tb = 0;
+    uintptr_t ret = 0;
 
     tci_reg[TCG_AREG0] = (tcg_target_ulong)env;
     tci_reg[TCG_REG_CALL_STACK] = sp_value;
@@ -1085,7 +1085,7 @@ uintptr_t tcg_qemu_tb_exec(CPUArchState *env, uint8_t *tb_ptr)
             /* QEMU specific operations. */
 
         case INDEX_op_exit_tb:
-            next_tb = *(uint64_t *)tb_ptr;
+            ret = *(uint64_t *)tb_ptr;
             goto exit;
             break;
         case INDEX_op_goto_tb:
@@ -1243,5 +1243,5 @@ uintptr_t tcg_qemu_tb_exec(CPUArchState *env, uint8_t *tb_ptr)
         tci_assert(tb_ptr == old_code_ptr + op_size);
     }
 exit:
-    return next_tb;
+    return ret;
 }
diff --git a/trace-events b/trace-events
index 83507438789b..ef4da73e19f6 100644
--- a/trace-events
+++ b/trace-events
@@ -1615,7 +1615,7 @@ kvm_failed_spr_get(int str, const char *msg) "Warning: Unable to retrieve SPR %d
 # cpu-exec.c
 disable exec_tb(void *tb, uintptr_t pc) "tb:%p pc=0x%"PRIxPTR
 disable exec_tb_nocache(void *tb, uintptr_t pc) "tb:%p pc=0x%"PRIxPTR
-disable exec_tb_exit(void *next_tb, unsigned int flags) "tb:%p flags=%x"
+disable exec_tb_exit(void *last_tb, unsigned int flags) "tb:%p flags=%x"
 
 # translate-all.c
 translate_block(void *tb, uintptr_t pc, uint8_t *tb_code) "tb:%p, pc:0x%"PRIxPTR", tb_code:%p"
-- 
2.8.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH v6 5/6] tcg: Rework tb_invalidated_flag
  2016-04-28 22:10 [Qemu-devel] [PATCH v6 0/6] tcg: Misc clean-up patches Sergey Fedorov
                   ` (3 preceding siblings ...)
  2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 4/6] tcg: Clean up from 'next_tb' Sergey Fedorov
@ 2016-04-28 22:10 ` Sergey Fedorov
  2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 6/6] cpu-exec: Move TB chaining into tb_find_fast() Sergey Fedorov
  5 siblings, 0 replies; 11+ messages in thread
From: Sergey Fedorov @ 2016-04-28 22:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Sergey Fedorov, Paolo Bonzini,
	Peter Crosthwaite, Richard Henderson, Sergey Fedorov,
	Andreas Färber

From: Sergey Fedorov <serge.fdrv@gmail.com>

'tb_invalidated_flag' was meant to catch two events:
 * some TB has been invalidated by tb_phys_invalidate();
 * the whole translation buffer has been flushed by tb_flush().

Then it was checked:
 * in cpu_exec() to ensure that the last executed TB can be safely
   linked to directly call the next one;
 * in cpu_exec_nocache() to decide if the original TB should be provided
   for further possible invalidation along with the temporarily
   generated TB.

It is always safe to patch an invalidated TB since it is not going to be
used anyway. It is also safe to call tb_phys_invalidate() for an already
invalidated TB. Thus, setting this flag in tb_phys_invalidate() is
simply unnecessary. Moreover, it can prevent from pretty proper linking
of TBs, if any arbitrary TB has been invalidated. So just don't touch it
in tb_phys_invalidate().

If this flag is only used to catch whether tb_flush() has been called
then rename it to 'tb_flushed'. Declare it as 'bool' and stick to using
only 'true' and 'false' to set its value. Also, instead of setting it in
tb_gen_code(), just after tb_flush() has been called, do it right inside
of tb_flush().

In cpu_exec(), this flag is used to track if tb_flush() has been called
and have made 'next_tb' (a reference to the last executed TB) invalid
for linking it to directly call the next TB. tb_flush() can be called
during the CPU execution loop from tb_gen_code(), during TB execution or
by another thread while 'tb_lock' is released. Catch for translation
buffer flush reliably by resetting this flag once before first TB lookup
and each time we find it set before trying to add a direct jump. Don't
touch in in tb_find_physical().

Each vCPU has its own execution loop in multithreaded mode and thus
should have its own copy of the flag to be able to reset it with its own
'next_tb' and don't affect any other vCPU execution thread. So make this
flag per-vCPU and move it to CPUState.

In cpu_exec_nocache(), we only need to check if tb_flush() has been
called from tb_gen_code() called by cpu_exec_nocache() itself. To do
this reliably, preserve the old value of the flag, reset it before
calling tb_gen_code(), check afterwards, and combine the saved value
back to the flag.

This patch is based on the patch "tcg: move tb_invalidated_flag to
CPUState" from Paolo Bonzini <pbonzini@redhat.com>.

Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---

Changes in v6:
 * Fixed rebase conflicts
Changes in v4:
 * Rebased on top of the previous patch

 cpu-exec.c              | 21 +++++++++++----------
 include/exec/exec-all.h |  2 --
 include/qom/cpu.h       |  2 ++
 translate-all.c         |  5 +----
 4 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 9407c66f626d..f49a436e1a5a 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -202,16 +202,20 @@ static void cpu_exec_nocache(CPUState *cpu, int max_cycles,
                              TranslationBlock *orig_tb, bool ignore_icount)
 {
     TranslationBlock *tb;
+    bool old_tb_flushed;
 
     /* Should never happen.
        We only end up here when an existing TB is too long.  */
     if (max_cycles > CF_COUNT_MASK)
         max_cycles = CF_COUNT_MASK;
 
+    old_tb_flushed = cpu->tb_flushed;
+    cpu->tb_flushed = false;
     tb = tb_gen_code(cpu, orig_tb->pc, orig_tb->cs_base, orig_tb->flags,
                      max_cycles | CF_NOCACHE
                          | (ignore_icount ? CF_IGNORE_ICOUNT : 0));
-    tb->orig_tb = tcg_ctx.tb_ctx.tb_invalidated_flag ? NULL : orig_tb;
+    tb->orig_tb = cpu->tb_flushed ? NULL : orig_tb;
+    cpu->tb_flushed |= old_tb_flushed;
     cpu->current_tb = tb;
     /* execute the generated code */
     trace_exec_tb_nocache(tb, tb->pc);
@@ -232,8 +236,6 @@ static TranslationBlock *tb_find_physical(CPUState *cpu,
     unsigned int h;
     tb_page_addr_t phys_pc, phys_page1;
 
-    tcg_ctx.tb_ctx.tb_invalidated_flag = 0;
-
     /* find translated block using physical mappings */
     phys_pc = get_page_addr_code(env, pc);
     phys_page1 = phys_pc & TARGET_PAGE_MASK;
@@ -446,6 +448,7 @@ int cpu_exec(CPUState *cpu)
             }
 
             last_tb = NULL; /* forget the last executed TB after exception */
+            cpu->tb_flushed = false; /* reset before first TB lookup */
             for(;;) {
                 interrupt_request = cpu->interrupt_request;
                 if (unlikely(interrupt_request)) {
@@ -510,14 +513,12 @@ int cpu_exec(CPUState *cpu)
                 }
                 tb_lock();
                 tb = tb_find_fast(cpu);
-                /* Note: we do it here to avoid a gcc bug on Mac OS X when
-                   doing it in tb_find_slow */
-                if (tcg_ctx.tb_ctx.tb_invalidated_flag) {
-                    /* as some TB could have been invalidated because
-                       of memory exceptions while generating the code, we
-                       must recompute the hash index here */
+                if (cpu->tb_flushed) {
+                    /* Ensure that no TB jump will be modified as the
+                     * translation buffer has been flushed.
+                     */
                     last_tb = NULL;
-                    tcg_ctx.tb_ctx.tb_invalidated_flag = 0;
+                    cpu->tb_flushed = false;
                 }
                 /* See if we can patch the calling TB. */
                 if (last_tb && !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 06da1bcc45bf..85528f994167 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -304,8 +304,6 @@ struct TBContext {
     /* statistics */
     int tb_flush_count;
     int tb_phys_invalidate_count;
-
-    int tb_invalidated_flag;
 };
 
 void tb_free(TranslationBlock *tb);
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index b7a10f791acc..c1ae24d1fcbb 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -238,6 +238,7 @@ struct kvm_run;
  * @crash_occurred: Indicates the OS reported a crash (panic) for this CPU
  * @tcg_exit_req: Set to force TCG to stop executing linked TBs for this
  *           CPU and return to its top level loop.
+ * @tb_flushed: Indicates the translation buffer has been flushed.
  * @singlestep_enabled: Flags for single-stepping.
  * @icount_extra: Instructions until next timer event.
  * @icount_decr: Number of cycles left, with interrupt flag in high bit.
@@ -289,6 +290,7 @@ struct CPUState {
     bool stopped;
     bool crash_occurred;
     bool exit_request;
+    bool tb_flushed;
     uint32_t interrupt_request;
     int singlestep_enabled;
     int64_t icount_extra;
diff --git a/translate-all.c b/translate-all.c
index 74fa1b4d271c..ee495ebc025d 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -843,6 +843,7 @@ void tb_flush(CPUState *cpu)
 
     CPU_FOREACH(cpu) {
         memset(cpu->tb_jmp_cache, 0, sizeof(cpu->tb_jmp_cache));
+        cpu->tb_flushed = true;
     }
 
     memset(tcg_ctx.tb_ctx.tb_phys_hash, 0, sizeof(tcg_ctx.tb_ctx.tb_phys_hash));
@@ -1011,8 +1012,6 @@ void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr)
         invalidate_page_bitmap(p);
     }
 
-    tcg_ctx.tb_ctx.tb_invalidated_flag = 1;
-
     /* remove the TB from the hash list */
     h = tb_jmp_cache_hash_func(tb->pc);
     CPU_FOREACH(cpu) {
@@ -1178,8 +1177,6 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
         /* cannot fail at this point */
         tb = tb_alloc(pc);
         assert(tb != NULL);
-        /* Don't forget to invalidate previous TB info.  */
-        tcg_ctx.tb_ctx.tb_invalidated_flag = 1;
     }
 
     gen_code_buf = tcg_ctx.code_gen_ptr;
-- 
2.8.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH v6 6/6] cpu-exec: Move TB chaining into tb_find_fast()
  2016-04-28 22:10 [Qemu-devel] [PATCH v6 0/6] tcg: Misc clean-up patches Sergey Fedorov
                   ` (4 preceding siblings ...)
  2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 5/6] tcg: Rework tb_invalidated_flag Sergey Fedorov
@ 2016-04-28 22:10 ` Sergey Fedorov
  2016-04-29 13:54   ` Alex Bennée
  5 siblings, 1 reply; 11+ messages in thread
From: Sergey Fedorov @ 2016-04-28 22:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Sergey Fedorov, Paolo Bonzini,
	Peter Crosthwaite, Richard Henderson, Sergey Fedorov

From: Sergey Fedorov <serge.fdrv@gmail.com>

Move tb_add_jump() call and surrounding code from cpu_exec() into
tb_find_fast(). That simplifies cpu_exec() a little by hiding the direct
chaining optimization details into tb_find_fast(). It also allows to
move tb_lock()/tb_unlock() pair into tb_find_fast(), putting it closer
to tb_find_slow() which also manipulates the lock.

Suggested-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---

Changes in v6:
 * Fixed rebase conflicts

 cpu-exec.c | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index f49a436e1a5a..5f23c0660d6e 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -320,7 +320,9 @@ found:
     return tb;
 }
 
-static inline TranslationBlock *tb_find_fast(CPUState *cpu)
+static inline TranslationBlock *tb_find_fast(CPUState *cpu,
+                                             TranslationBlock **last_tb,
+                                             int tb_exit)
 {
     CPUArchState *env = (CPUArchState *)cpu->env_ptr;
     TranslationBlock *tb;
@@ -331,11 +333,24 @@ static inline TranslationBlock *tb_find_fast(CPUState *cpu)
        always be the same before a given translated block
        is executed. */
     cpu_get_tb_cpu_state(env, &pc, &cs_base, &flags);
+    tb_lock();
     tb = cpu->tb_jmp_cache[tb_jmp_cache_hash_func(pc)];
     if (unlikely(!tb || tb->pc != pc || tb->cs_base != cs_base ||
                  tb->flags != flags)) {
         tb = tb_find_slow(cpu, pc, cs_base, flags);
     }
+    if (cpu->tb_flushed) {
+        /* Ensure that no TB jump will be modified as the
+         * translation buffer has been flushed.
+         */
+        *last_tb = NULL;
+        cpu->tb_flushed = false;
+    }
+    /* See if we can patch the calling TB. */
+    if (*last_tb && qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
+        tb_add_jump(*last_tb, tb_exit, tb);
+    }
+    tb_unlock();
     return tb;
 }
 
@@ -441,7 +456,8 @@ int cpu_exec(CPUState *cpu)
             } else if (replay_has_exception()
                        && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
                 /* try to cause an exception pending in the log */
-                cpu_exec_nocache(cpu, 1, tb_find_fast(cpu), true);
+                last_tb = NULL; /* Avoid chaining TBs */
+                cpu_exec_nocache(cpu, 1, tb_find_fast(cpu, &last_tb, 0), true);
                 ret = -1;
                 break;
 #endif
@@ -511,20 +527,7 @@ int cpu_exec(CPUState *cpu)
                     cpu->exception_index = EXCP_INTERRUPT;
                     cpu_loop_exit(cpu);
                 }
-                tb_lock();
-                tb = tb_find_fast(cpu);
-                if (cpu->tb_flushed) {
-                    /* Ensure that no TB jump will be modified as the
-                     * translation buffer has been flushed.
-                     */
-                    last_tb = NULL;
-                    cpu->tb_flushed = false;
-                }
-                /* See if we can patch the calling TB. */
-                if (last_tb && !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
-                    tb_add_jump(last_tb, tb_exit, tb);
-                }
-                tb_unlock();
+                tb = tb_find_fast(cpu, &last_tb, tb_exit);
                 if (likely(!cpu->exit_request)) {
                     uintptr_t ret;
                     trace_exec_tb(tb, tb->pc);
-- 
2.8.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH v6 6/6] cpu-exec: Move TB chaining into tb_find_fast()
  2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 6/6] cpu-exec: Move TB chaining into tb_find_fast() Sergey Fedorov
@ 2016-04-29 13:54   ` Alex Bennée
  2016-04-29 13:58     ` Sergey Fedorov
  0 siblings, 1 reply; 11+ messages in thread
From: Alex Bennée @ 2016-04-29 13:54 UTC (permalink / raw)
  To: Sergey Fedorov
  Cc: qemu-devel, Sergey Fedorov, Paolo Bonzini, Peter Crosthwaite,
	Richard Henderson


Sergey Fedorov <sergey.fedorov@linaro.org> writes:

> From: Sergey Fedorov <serge.fdrv@gmail.com>
>
> Move tb_add_jump() call and surrounding code from cpu_exec() into
> tb_find_fast(). That simplifies cpu_exec() a little by hiding the direct
> chaining optimization details into tb_find_fast(). It also allows to
> move tb_lock()/tb_unlock() pair into tb_find_fast(), putting it closer
> to tb_find_slow() which also manipulates the lock.
>
> Suggested-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
> ---
>
> Changes in v6:
>  * Fixed rebase conflicts
>
>  cpu-exec.c | 35 +++++++++++++++++++----------------
>  1 file changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/cpu-exec.c b/cpu-exec.c
> index f49a436e1a5a..5f23c0660d6e 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -320,7 +320,9 @@ found:
>      return tb;
>  }
>
> -static inline TranslationBlock *tb_find_fast(CPUState *cpu)
> +static inline TranslationBlock *tb_find_fast(CPUState *cpu,
> +                                             TranslationBlock **last_tb,
> +                                             int tb_exit)
>  {
>      CPUArchState *env = (CPUArchState *)cpu->env_ptr;
>      TranslationBlock *tb;
> @@ -331,11 +333,24 @@ static inline TranslationBlock *tb_find_fast(CPUState *cpu)
>         always be the same before a given translated block
>         is executed. */
>      cpu_get_tb_cpu_state(env, &pc, &cs_base, &flags);
> +    tb_lock();
>      tb = cpu->tb_jmp_cache[tb_jmp_cache_hash_func(pc)];
>      if (unlikely(!tb || tb->pc != pc || tb->cs_base != cs_base ||
>                   tb->flags != flags)) {
>          tb = tb_find_slow(cpu, pc, cs_base, flags);
>      }
> +    if (cpu->tb_flushed) {
> +        /* Ensure that no TB jump will be modified as the
> +         * translation buffer has been flushed.
> +         */
> +        *last_tb = NULL;
> +        cpu->tb_flushed = false;
> +    }
> +    /* See if we can patch the calling TB. */
> +    if (*last_tb && qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {

This should be !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)

> +        tb_add_jump(*last_tb, tb_exit, tb);
> +    }
> +    tb_unlock();
>      return tb;
>  }
>
> @@ -441,7 +456,8 @@ int cpu_exec(CPUState *cpu)
>              } else if (replay_has_exception()
>                         && cpu->icount_decr.u16.low + cpu->icount_extra == 0) {
>                  /* try to cause an exception pending in the log */
> -                cpu_exec_nocache(cpu, 1, tb_find_fast(cpu), true);
> +                last_tb = NULL; /* Avoid chaining TBs */
> +                cpu_exec_nocache(cpu, 1, tb_find_fast(cpu, &last_tb, 0), true);
>                  ret = -1;
>                  break;
>  #endif
> @@ -511,20 +527,7 @@ int cpu_exec(CPUState *cpu)
>                      cpu->exception_index = EXCP_INTERRUPT;
>                      cpu_loop_exit(cpu);
>                  }
> -                tb_lock();
> -                tb = tb_find_fast(cpu);
> -                if (cpu->tb_flushed) {
> -                    /* Ensure that no TB jump will be modified as the
> -                     * translation buffer has been flushed.
> -                     */
> -                    last_tb = NULL;
> -                    cpu->tb_flushed = false;
> -                }
> -                /* See if we can patch the calling TB. */
> -                if (last_tb && !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
> -                    tb_add_jump(last_tb, tb_exit, tb);
> -                }
> -                tb_unlock();
> +                tb = tb_find_fast(cpu, &last_tb, tb_exit);
>                  if (likely(!cpu->exit_request)) {
>                      uintptr_t ret;
>                      trace_exec_tb(tb, tb->pc);


--
Alex Bennée

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH v6 6/6] cpu-exec: Move TB chaining into tb_find_fast()
  2016-04-29 13:54   ` Alex Bennée
@ 2016-04-29 13:58     ` Sergey Fedorov
  2016-04-29 16:32       ` Richard Henderson
  0 siblings, 1 reply; 11+ messages in thread
From: Sergey Fedorov @ 2016-04-29 13:58 UTC (permalink / raw)
  To: Alex Bennée, Sergey Fedorov
  Cc: qemu-devel, Paolo Bonzini, Peter Crosthwaite, Richard Henderson

On 29/04/16 16:54, Alex Bennée wrote:
> Sergey Fedorov <sergey.fedorov@linaro.org> writes:
>> diff --git a/cpu-exec.c b/cpu-exec.c
>> index f49a436e1a5a..5f23c0660d6e 100644
>> --- a/cpu-exec.c
>> +++ b/cpu-exec.c
>> @@ -320,7 +320,9 @@ found:
>>      return tb;
>>  }
>>
>> -static inline TranslationBlock *tb_find_fast(CPUState *cpu)
>> +static inline TranslationBlock *tb_find_fast(CPUState *cpu,
>> +                                             TranslationBlock **last_tb,
>> +                                             int tb_exit)
>>  {
>>      CPUArchState *env = (CPUArchState *)cpu->env_ptr;
>>      TranslationBlock *tb;
>> @@ -331,11 +333,24 @@ static inline TranslationBlock *tb_find_fast(CPUState *cpu)
>>         always be the same before a given translated block
>>         is executed. */
>>      cpu_get_tb_cpu_state(env, &pc, &cs_base, &flags);
>> +    tb_lock();
>>      tb = cpu->tb_jmp_cache[tb_jmp_cache_hash_func(pc)];
>>      if (unlikely(!tb || tb->pc != pc || tb->cs_base != cs_base ||
>>                   tb->flags != flags)) {
>>          tb = tb_find_slow(cpu, pc, cs_base, flags);
>>      }
>> +    if (cpu->tb_flushed) {
>> +        /* Ensure that no TB jump will be modified as the
>> +         * translation buffer has been flushed.
>> +         */
>> +        *last_tb = NULL;
>> +        cpu->tb_flushed = false;
>> +    }
>> +    /* See if we can patch the calling TB. */
>> +    if (*last_tb && qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
> This should be !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)

Probably, it's mine rebase conflict resolution mistake. Nice catch, thanks!

Kind regards,
Sergey

>
>> +        tb_add_jump(*last_tb, tb_exit, tb);
>> +    }
>> +    tb_unlock();
>>      return tb;
>>  }

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH v6 6/6] cpu-exec: Move TB chaining into tb_find_fast()
  2016-04-29 13:58     ` Sergey Fedorov
@ 2016-04-29 16:32       ` Richard Henderson
  2016-04-29 19:05         ` Sergey Fedorov
  0 siblings, 1 reply; 11+ messages in thread
From: Richard Henderson @ 2016-04-29 16:32 UTC (permalink / raw)
  To: Sergey Fedorov, Alex Bennée, Sergey Fedorov
  Cc: qemu-devel, Paolo Bonzini, Peter Crosthwaite

On 04/29/2016 06:58 AM, Sergey Fedorov wrote:
> On 29/04/16 16:54, Alex Bennée wrote:
>> Sergey Fedorov <sergey.fedorov@linaro.org> writes:
>>> diff --git a/cpu-exec.c b/cpu-exec.c
>>> index f49a436e1a5a..5f23c0660d6e 100644
>>> --- a/cpu-exec.c
>>> +++ b/cpu-exec.c
>>> @@ -320,7 +320,9 @@ found:
>>>      return tb;
>>>  }
>>>
>>> -static inline TranslationBlock *tb_find_fast(CPUState *cpu)
>>> +static inline TranslationBlock *tb_find_fast(CPUState *cpu,
>>> +                                             TranslationBlock **last_tb,
>>> +                                             int tb_exit)
>>>  {
>>>      CPUArchState *env = (CPUArchState *)cpu->env_ptr;
>>>      TranslationBlock *tb;
>>> @@ -331,11 +333,24 @@ static inline TranslationBlock *tb_find_fast(CPUState *cpu)
>>>         always be the same before a given translated block
>>>         is executed. */
>>>      cpu_get_tb_cpu_state(env, &pc, &cs_base, &flags);
>>> +    tb_lock();
>>>      tb = cpu->tb_jmp_cache[tb_jmp_cache_hash_func(pc)];
>>>      if (unlikely(!tb || tb->pc != pc || tb->cs_base != cs_base ||
>>>                   tb->flags != flags)) {
>>>          tb = tb_find_slow(cpu, pc, cs_base, flags);
>>>      }
>>> +    if (cpu->tb_flushed) {
>>> +        /* Ensure that no TB jump will be modified as the
>>> +         * translation buffer has been flushed.
>>> +         */
>>> +        *last_tb = NULL;
>>> +        cpu->tb_flushed = false;
>>> +    }
>>> +    /* See if we can patch the calling TB. */
>>> +    if (*last_tb && qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
>> This should be !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)
> 
> Probably, it's mine rebase conflict resolution mistake. Nice catch, thanks!

Fixed while applying all to tcg-next.


r~

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH v6 6/6] cpu-exec: Move TB chaining into tb_find_fast()
  2016-04-29 16:32       ` Richard Henderson
@ 2016-04-29 19:05         ` Sergey Fedorov
  0 siblings, 0 replies; 11+ messages in thread
From: Sergey Fedorov @ 2016-04-29 19:05 UTC (permalink / raw)
  To: Richard Henderson, Alex Bennée, Sergey Fedorov
  Cc: qemu-devel, Paolo Bonzini, Peter Crosthwaite

On 29/04/16 19:32, Richard Henderson wrote:
> On 04/29/2016 06:58 AM, Sergey Fedorov wrote:
>> On 29/04/16 16:54, Alex Bennée wrote:
>>> Sergey Fedorov <sergey.fedorov@linaro.org> writes:
>>>> diff --git a/cpu-exec.c b/cpu-exec.c
>>>> index f49a436e1a5a..5f23c0660d6e 100644
>>>> --- a/cpu-exec.c
>>>> +++ b/cpu-exec.c
>>>> @@ -320,7 +320,9 @@ found:
>>>>      return tb;
>>>>  }
>>>>
>>>> -static inline TranslationBlock *tb_find_fast(CPUState *cpu)
>>>> +static inline TranslationBlock *tb_find_fast(CPUState *cpu,
>>>> +                                             TranslationBlock **last_tb,
>>>> +                                             int tb_exit)
>>>>  {
>>>>      CPUArchState *env = (CPUArchState *)cpu->env_ptr;
>>>>      TranslationBlock *tb;
>>>> @@ -331,11 +333,24 @@ static inline TranslationBlock *tb_find_fast(CPUState *cpu)
>>>>         always be the same before a given translated block
>>>>         is executed. */
>>>>      cpu_get_tb_cpu_state(env, &pc, &cs_base, &flags);
>>>> +    tb_lock();
>>>>      tb = cpu->tb_jmp_cache[tb_jmp_cache_hash_func(pc)];
>>>>      if (unlikely(!tb || tb->pc != pc || tb->cs_base != cs_base ||
>>>>                   tb->flags != flags)) {
>>>>          tb = tb_find_slow(cpu, pc, cs_base, flags);
>>>>      }
>>>> +    if (cpu->tb_flushed) {
>>>> +        /* Ensure that no TB jump will be modified as the
>>>> +         * translation buffer has been flushed.
>>>> +         */
>>>> +        *last_tb = NULL;
>>>> +        cpu->tb_flushed = false;
>>>> +    }
>>>> +    /* See if we can patch the calling TB. */
>>>> +    if (*last_tb && qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
>>> This should be !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)
>> Probably, it's mine rebase conflict resolution mistake. Nice catch, thanks!
> Fixed while applying all to tcg-next.

Thanks!

Kind regards,
Sergey

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-04-29 19:06 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-28 22:10 [Qemu-devel] [PATCH v6 0/6] tcg: Misc clean-up patches Sergey Fedorov
2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 1/6] tcg: code_bitmap is not used by user-mode emulation Sergey Fedorov
2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 2/6] tcg: reorganize tb_find_physical loop Sergey Fedorov
2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 3/6] cpu-exec: elide more icount code if CONFIG_USER_ONLY Sergey Fedorov
2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 4/6] tcg: Clean up from 'next_tb' Sergey Fedorov
2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 5/6] tcg: Rework tb_invalidated_flag Sergey Fedorov
2016-04-28 22:10 ` [Qemu-devel] [PATCH v6 6/6] cpu-exec: Move TB chaining into tb_find_fast() Sergey Fedorov
2016-04-29 13:54   ` Alex Bennée
2016-04-29 13:58     ` Sergey Fedorov
2016-04-29 16:32       ` Richard Henderson
2016-04-29 19:05         ` Sergey Fedorov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.