[v5,3/6] target: [tcg] Add generic translation framework

Commit Message

Lluís Vilanova Dec. 28, 2016, 4:28 p.m. UTC

Signed-off-by: Lluís Vilanova <[e-mail]>
---
 include/exec/gen-icount.h             |    2 
 include/exec/translate-all_template.h |   73 ++++++++++++
 include/qom/cpu.h                     |   22 ++++
 translate-all_template.h              |  204 +++++++++++++++++++++++++++++++++
 4 files changed, 300 insertions(+), 1 deletion(-)
 create mode 100644 include/exec/translate-all_template.h
 create mode 100644 translate-all_template.h

Comments

Richard Henderson Jan. 11, 2017, 2:50 a.m. UTC | #1

On 12/28/2016 08:28 AM, Lluís Vilanova wrote:
> +typedef enum DisasJumpType {
> +    DJ_NEXT,
> +    DJ_TOO_MANY,
> +    DJ_TARGET,
> +} DisasJumpType;

I wonder if enums like DJ_TARGET_{0..N} wouldn't be better, rather than doing 
addition in the target-specific names.

> +typedef struct DisasContextBase {
> +    TranslationBlock *tb;
> +    bool singlestep_enabled;
> +    target_ulong pc_first;
> +    target_ulong pc_next;
> +    DisasJumpType jmp_type;
> +    unsigned int num_insns;
> +} DisasContextBase;

Sort the bool to the end to minimize padding.

> +/* Get first breakpoint matching a PC */
> +static inline CPUBreakpoint *cpu_breakpoint_get(CPUState *cpu, vaddr pc,
> +                                                CPUBreakpoint *bp)
> +{
> +    if (likely(bp == NULL)) {
> +        if (unlikely(!QTAILQ_EMPTY(&cpu->breakpoints))) {
> +            QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) {
> +                if (bp->pc == pc) {
> +                    return bp;
> +                }
> +            }
> +        }
> +    } else {
> +        QTAILQ_FOREACH_CONTINUE(bp, entry) {
> +            if (bp->pc == pc) {
> +                return bp;
> +            }
> +        }
> +    }
> +    return NULL;
> +}

Any reason not to put the QTAILQ_FOREACH directly into gen_intermediate_code, 
rather than indirect it like this?  I don't see this abstraction as an improvement.


r~

Lluís Vilanova Jan. 16, 2017, 3:41 p.m. UTC | #2

Richard Henderson writes:

> On 12/28/2016 08:28 AM, Lluís Vilanova wrote:
>> +typedef enum DisasJumpType {
>> +    DJ_NEXT,
>> +    DJ_TOO_MANY,
>> +    DJ_TARGET,
>> +} DisasJumpType;

> I wonder if enums like DJ_TARGET_{0..N} wouldn't be better, rather than doing
> addition in the target-specific names.

I'm not sure. ARM has 12 target-specific flags (while x86 only has 2), and
adding so many "unspecified" values to the generic enum does not seem right to
me.

I you feel strongly against the current state, I'll change it; re-defining
existing enum values could, for example, have benefits in switch/case ranges and
compiler warnings.


>> +typedef struct DisasContextBase {
>> +    TranslationBlock *tb;
>> +    bool singlestep_enabled;
>> +    target_ulong pc_first;
>> +    target_ulong pc_next;
>> +    DisasJumpType jmp_type;
>> +    unsigned int num_insns;
>> +} DisasContextBase;

> Sort the bool to the end to minimize padding.

Done.


>> +/* Get first breakpoint matching a PC */
>> +static inline CPUBreakpoint *cpu_breakpoint_get(CPUState *cpu, vaddr pc,
>> +                                                CPUBreakpoint *bp)
>> +{
>> +    if (likely(bp == NULL)) {
>> +        if (unlikely(!QTAILQ_EMPTY(&cpu->breakpoints))) {
>> +            QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) {
>> +                if (bp->pc == pc) {
>> +                    return bp;
>> +                }
>> +            }
>> +        }
>> +    } else {
>> +        QTAILQ_FOREACH_CONTINUE(bp, entry) {
>> +            if (bp->pc == pc) {
>> +                return bp;
>> +            }
>> +        }
>> +    }
>> +    return NULL;
>> +}

> Any reason not to put the QTAILQ_FOREACH directly into gen_intermediate_code,
> rather than indirect it like this?  I don't see this abstraction as an
> improvement.

I thought this belonged here to maintain encapsulation of how breakpoint lists
are implemented (just as we already have insert/remove/test functions right
above this one).

Having it as a separate function (wherever it is) also helps readability. Again,
if you feel strongly against it, I can move that function into the translate
template header.


Thanks,
  Lluis

Patch

diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h
index 050de59b38..c91ac95ed7 100644
--- a/include/exec/gen-icount.h
+++ b/include/exec/gen-icount.h
@@ -45,7 +45,7 @@  static inline void gen_tb_start(TranslationBlock *tb)
     tcg_temp_free_i32(count);
 }
 
-static void gen_tb_end(TranslationBlock *tb, int num_insns)
+static inline void gen_tb_end(TranslationBlock *tb, int num_insns)
 {
     gen_set_label(exitreq_label);
     tcg_gen_exit_tb((uintptr_t)tb + TB_EXIT_REQUESTED);
diff --git a/include/exec/translate-all_template.h b/include/exec/translate-all_template.h
new file mode 100644
index 0000000000..ea507f90c6
--- /dev/null
+++ b/include/exec/translate-all_template.h
@@ -0,0 +1,73 @@ 
+/*
+ * Generic intermediate code generation.
+ *
+ * Copyright (C) 2016 Lluís Vilanova <[e-mail]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef EXEC__TRANSLATE_ALL_TEMPLATE_H
+#define EXEC__TRANSLATE_ALL_TEMPLATE_H
+
+/*
+ * Include this header from a target-specific file, and add a
+ *
+ *     DisasContextBase base;
+ *
+ * member in your target-specific DisasContext.
+ */
+
+
+#include "exec/exec-all.h"
+
+
+/**
+ * BreakpointHitType:
+ * @BH_MISS: No hit
+ * @BH_HIT_INSN: Hit, but continue translating instruction
+ * @BH_HIT_TB: Hit, stop translating TB
+ *
+ * How to react to a breakpoint hit.
+ */
+typedef enum BreakpointHitType {
+    BH_MISS,
+    BH_HIT_INSN,
+    BH_HIT_TB,
+} BreakpointHitType;
+
+/**
+ * DisasJumpType:
+ * @DJ_NEXT: Next instruction in program order
+ * @DJ_TOO_MANY: Too many instructions executed
+ * @DJ_TARGET: Start of target-specific conditions
+ *
+ * What instruction to disassemble next.
+ */
+typedef enum DisasJumpType {
+    DJ_NEXT,
+    DJ_TOO_MANY,
+    DJ_TARGET,
+} DisasJumpType;
+
+/**
+ * DisasContextBase:
+ * @tb: Translation block for this disassembly.
+ * @singlestep_enabled: "Hardware" single stepping enabled.
+ * @pc_first: Address of first guest instruction in this TB.
+ * @pc_next: Address of next guest instruction in this TB (current during
+ *           disassembly).
+ * @num_insns: Number of translated instructions (including current).
+ *
+ * Architecture-agnostic disassembly context.
+ */
+typedef struct DisasContextBase {
+    TranslationBlock *tb;
+    bool singlestep_enabled;
+    target_ulong pc_first;
+    target_ulong pc_next;
+    DisasJumpType jmp_type;
+    unsigned int num_insns;
+} DisasContextBase;
+
+#endif  /* EXEC__TRANSLATE_ALL_TEMPLATE_H */
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 3f79a8e955..64a288b066 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -948,6 +948,28 @@  static inline bool cpu_breakpoint_test(CPUState *cpu, vaddr pc, int mask)
     return false;
 }
 
+/* Get first breakpoint matching a PC */
+static inline CPUBreakpoint *cpu_breakpoint_get(CPUState *cpu, vaddr pc,
+                                                CPUBreakpoint *bp)
+{
+    if (likely(bp == NULL)) {
+        if (unlikely(!QTAILQ_EMPTY(&cpu->breakpoints))) {
+            QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) {
+                if (bp->pc == pc) {
+                    return bp;
+                }
+            }
+        }
+    } else {
+        QTAILQ_FOREACH_CONTINUE(bp, entry) {
+            if (bp->pc == pc) {
+                return bp;
+            }
+        }
+    }
+    return NULL;
+}
+
 int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len,
                           int flags, CPUWatchpoint **watchpoint);
 int cpu_watchpoint_remove(CPUState *cpu, vaddr addr,
diff --git a/translate-all_template.h b/translate-all_template.h
new file mode 100644
index 0000000000..6208916d08
--- /dev/null
+++ b/translate-all_template.h
@@ -0,0 +1,204 @@ 
+/*
+ * Generic intermediate code generation.
+ *
+ * Copyright (C) 2016 Lluís Vilanova <[e-mail]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef TRANSLATE_ALL_TEMPLATE_H
+#define TRANSLATE_ALL_TEMPLATE_H
+
+/*
+ * Include this header from a target-specific file, which must define the
+ * target-specific functions declared below.
+ *
+ * These must be paired with instructions in "exec/translate-all_template.h".
+ */
+
+
+#include "cpu.h"
+#include "qemu/error-report.h"
+
+
+static void gen_intermediate_code_target_init_disas_context(
+    DisasContext *dc, CPUArchState *env);
+
+static void gen_intermediate_code_target_init_globals(
+    DisasContext *dc, CPUArchState *env);
+
+static void gen_intermediate_code_target_tb_start(
+    DisasContext *dc, CPUArchState *env);
+
+static void gen_intermediate_code_target_insn_start(
+    DisasContext *dc, CPUArchState *env);
+
+static BreakpointHitType gen_intermediate_code_target_breakpoint_hit(
+    DisasContext *dc, CPUArchState *env,
+    const CPUBreakpoint *bp);
+
+static target_ulong gen_intermediate_code_target_disas_insn(
+    DisasContext *dc, CPUArchState *env);
+
+static DisasJumpType gen_intermediate_code_target_stop_check(
+    DisasContext *dc, CPUArchState *env);
+
+static void gen_intermediate_code_target_stop(
+    DisasContext *dc, CPUArchState *env);
+
+static int gen_intermediate_code_target_get_disas_flags(
+    const DisasContext *dc);
+
+
+static inline void gen_intermediate_code_tcg_check(const DisasContext *dc)
+{
+    if (tcg_check_temp_count()) {
+        error_report("warning: TCG temporary leaks before "TARGET_FMT_lx,
+                     dc->base.pc_next);
+    }
+}
+
+void gen_intermediate_code(CPUState *cpu, struct TranslationBlock *tb)
+{
+    CPUArchState *env = cpu->env_ptr;
+    DisasContext dc1, *dc = &dc1;
+    int max_insns;
+
+    /* Initialize DisasContext */
+    dc->base.tb = tb;
+    dc->base.singlestep_enabled = cpu->singlestep_enabled;
+    dc->base.pc_first = tb->pc;
+    dc->base.pc_next = dc->base.pc_first;
+    dc->base.jmp_type = DJ_NEXT;
+    dc->base.num_insns = 0;
+    gen_intermediate_code_target_init_disas_context(dc, env);
+
+    /* Initialize globals */
+    gen_intermediate_code_target_init_globals(dc, env);
+    tcg_clear_temp_count();
+
+    /* Instruction counting */
+    max_insns = dc->base.tb->cflags & CF_COUNT_MASK;
+    if (max_insns == 0) {
+        max_insns = CF_COUNT_MASK;
+    }
+    if (max_insns > TCG_MAX_INSNS) {
+        max_insns = TCG_MAX_INSNS;
+    }
+    if (dc->base.singlestep_enabled || singlestep) {
+        max_insns = 1;
+    }
+
+    /* Start translating */
+    gen_tb_start(dc->base.tb);
+    gen_intermediate_code_target_tb_start(dc, env);
+
+    while (true) {
+        CPUBreakpoint *bp;
+
+        dc->base.num_insns++;
+        gen_intermediate_code_target_insn_start(dc, env);
+
+        /* Early exit before breakpoint checks */
+        if (unlikely(dc->base.jmp_type != DJ_NEXT)) {
+            break;
+        }
+
+        /* Pass breakpoint hits to target for further processing */
+        bp = NULL;
+        do {
+            bp = cpu_breakpoint_get(cpu, dc->base.pc_next, bp);
+            if (unlikely(bp)) {
+                BreakpointHitType bh =
+                    gen_intermediate_code_target_breakpoint_hit(dc, env, bp);
+                if (bh == BH_HIT_INSN) {
+                    /* Hit, keep translating */
+                    /*
+                     * TODO: if we're never going to have more than one BP in a
+                     *       single address, we can simply use a bool here.
+                     */
+                    break;
+                } else if (bh == BH_HIT_TB) {
+                    goto done_generating;
+                }
+            }
+        } while (bp != NULL);
+
+        /* Accept I/O on last instruction */
+        if (dc->base.num_insns == max_insns &&
+            (dc->base.tb->cflags & CF_LAST_IO)) {
+            gen_io_start();
+        }
+
+        /* Disassemble one instruction */
+        dc->base.pc_next = gen_intermediate_code_target_disas_insn(dc, env);
+
+        /**************************************************/
+        /* Conditions to stop translation                 */
+        /**************************************************/
+
+        /* Disassembly already set a stop condition */
+        if (dc->base.jmp_type >= DJ_TARGET) {
+            break;
+        }
+
+        /* Target-specific conditions */
+        dc->base.jmp_type = gen_intermediate_code_target_stop_check(dc, env);
+        if (dc->base.jmp_type >= DJ_TARGET) {
+            break;
+        }
+
+        /* Too many instructions */
+        if (tcg_op_buf_full() || dc->base.num_insns >= max_insns) {
+            dc->base.jmp_type = DJ_TOO_MANY;
+            break;
+        }
+
+        /*
+         * Check if next instruction is on next page, which can cause an
+         * exception.
+         *
+         * NOTE: Target-specific code must check a single instruction does not
+         *       cross page boundaries; the first in the TB is always allowed to
+         *       cross pages (never goes through this check).
+         */
+        if ((dc->base.pc_first & TARGET_PAGE_MASK)
+            != (dc->base.pc_next & TARGET_PAGE_MASK)) {
+            dc->base.jmp_type = DJ_TOO_MANY;
+            break;
+        }
+
+        gen_intermediate_code_tcg_check(dc);
+    }
+
+    gen_intermediate_code_target_stop(dc, env);
+
+    if (dc->base.tb->cflags & CF_LAST_IO) {
+        gen_io_end();
+    }
+
+done_generating:
+    gen_tb_end(dc->base.tb, dc->base.num_insns);
+
+    gen_intermediate_code_tcg_check(dc);
+
+#ifdef DEBUG_DISAS
+    if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM) &&
+        qemu_log_in_addr_range(dc->base.pc_first)) {
+        qemu_log_lock();
+        qemu_log("----------------\n");
+        qemu_log("IN: %s\n", lookup_symbol(dc->base.pc_first));
+        log_target_disas(cpu, dc->base.pc_first,
+                         dc->base.pc_next - dc->base.pc_first,
+                         gen_intermediate_code_target_get_disas_flags(dc));
+        qemu_log("\n");
+        qemu_log_unlock();
+    }
+#endif
+
+    dc->base.tb->size = dc->base.pc_next - dc->base.pc_first;
+    dc->base.tb->icount = dc->base.num_insns;
+}
+
+#endif  /* TRANSLATE_ALL_TEMPLATE_H */