diff mbox series

[v2,17/21] accel/tcg: cache single instruction TB on pending replay exception

Message ID 20210210221053.18050-18-alex.bennee@linaro.org
State Superseded
Headers show
Series plugins/next pre-PR (hwprofile, regression fixes, icount count fix) | expand

Commit Message

Alex Bennée Feb. 10, 2021, 10:10 p.m. UTC 
Again there is no reason to jump through the nocache hoops to execute
a single instruction block. We do have to add an additional wrinkle to
the cpu_handle_interrupt case to ensure we let through a TB where we
have specifically disabled icount for the block.

As the last user of cpu_exec_nocache we can now remove the function.
Further clean-up will follow in subsequent patches.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Message-Id: <20210209182749.31323-9-alex.bennee@linaro.org>
---
 accel/tcg/cpu-exec.c | 44 ++++----------------------------------------
 1 file changed, 4 insertions(+), 40 deletions(-)

-- 
2.20.1

Comments

Richard Henderson Feb. 11, 2021, 7:12 p.m. UTC | #1 
On 2/10/21 2:10 PM, Alex Bennée wrote:
> Again there is no reason to jump through the nocache hoops to execute

> a single instruction block. We do have to add an additional wrinkle to

> the cpu_handle_interrupt case to ensure we let through a TB where we

> have specifically disabled icount for the block.


Can you say more about this?  Because...

>      if (unlikely(qatomic_read(&cpu->exit_request))

>          || (icount_enabled()

> +            && (cpu->cflags_next_tb == -1 || cpu->cflags_next_tb & CF_USE_ICOUNT)

>              && cpu_neg(cpu)->icount_decr.u16.low + cpu->icount_extra == 0)) {


... this does not appear to match.  You're checking that icount has been
explicitly *enabled*?  Or am I reading the logic backward and only if icount is
enabled will we take EXCP_INTERRUPT?


r~
Alex Bennée Feb. 11, 2021, 8 p.m. UTC | #2 
Richard Henderson <richard.henderson@linaro.org> writes:

> On 2/10/21 2:10 PM, Alex Bennée wrote:

>> Again there is no reason to jump through the nocache hoops to execute

>> a single instruction block. We do have to add an additional wrinkle to

>> the cpu_handle_interrupt case to ensure we let through a TB where we

>> have specifically disabled icount for the block.

>

> Can you say more about this?  Because...

>

>>      if (unlikely(qatomic_read(&cpu->exit_request))

>>          || (icount_enabled()

>> +            && (cpu->cflags_next_tb == -1 || cpu->cflags_next_tb & CF_USE_ICOUNT)

>>              && cpu_neg(cpu)->icount_decr.u16.low + cpu->icount_extra == 0)) {

>

> ... this does not appear to match.  You're checking that icount has been

> explicitly *enabled*?


If icount has been enabled and we are using the default cflags or
enabled and we have the explicit CF_ICOUNT. The replay exception leg
explicitly disables icount because otherwise we'd never actually execute
the block because we have a budget of 0 cycles left. Previously we ran
that block at the exception handling point - now we fall through and
have to make sure we don't trigger an IRQ.

> Or am I reading the logic backward and only if icount is

> enabled will we take EXCP_INTERRUPT?


Or I guess we have an exit_request which hasn't been handled yet but
there is no EXCP_ pending.

>

>

> r~



-- 
Alex Bennée
diff mbox series

Patch

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 5b6a4fe84b..438fece73b 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -224,40 +224,6 @@  cpu_tb_exec(CPUState *cpu, TranslationBlock *itb, int *tb_exit)
     return last_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,
-                             TranslationBlock *orig_tb, bool ignore_icount)
-{
-    TranslationBlock *tb;
-    uint32_t cflags = curr_cflags() | CF_NOCACHE;
-    int tb_exit;
-
-    if (ignore_icount) {
-        cflags &= ~CF_USE_ICOUNT;
-    }
-
-    /* Should never happen.
-       We only end up here when an existing TB is too long.  */
-    cflags |= MIN(max_cycles, CF_COUNT_MASK);
-
-    mmap_lock();
-    tb = tb_gen_code(cpu, orig_tb->pc, orig_tb->cs_base,
-                     orig_tb->flags, cflags);
-    tb->orig_tb = orig_tb;
-    mmap_unlock();
-
-    /* execute the generated code */
-    trace_exec_tb_nocache(tb, tb->pc);
-    cpu_tb_exec(cpu, tb, &tb_exit);
-
-    mmap_lock();
-    tb_phys_invalidate(tb, -1);
-    mmap_unlock();
-    tcg_tb_remove(tb);
-}
-#endif
 
 static void cpu_exec_enter(CPUState *cpu)
 {
@@ -524,15 +490,12 @@  static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
 #ifndef CONFIG_USER_ONLY
         if (replay_has_exception()
             && cpu_neg(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(cpu, NULL, 0, curr_cflags()), true);
+            /* Execute just one insn to trigger exception pending in the log */
+            cpu->cflags_next_tb = (curr_cflags() & ~CF_USE_ICOUNT) | 1;
         }
 #endif
-        if (cpu->exception_index < 0) {
-            return false;
-        }
+        return false;
     }
-
     if (cpu->exception_index >= EXCP_INTERRUPT) {
         /* exit request from the cpu execution loop */
         *ret = cpu->exception_index;
@@ -688,6 +651,7 @@  static inline bool cpu_handle_interrupt(CPUState *cpu,
     /* Finally, check if we need to exit to the main loop.  */
     if (unlikely(qatomic_read(&cpu->exit_request))
         || (icount_enabled()
+            && (cpu->cflags_next_tb == -1 || cpu->cflags_next_tb & CF_USE_ICOUNT)
             && cpu_neg(cpu)->icount_decr.u16.low + cpu->icount_extra == 0)) {
         qatomic_set(&cpu->exit_request, 0);
         if (cpu->exception_index == -1) {