| Message ID | 149865316837.17063.1608754834009945976.stgit@frigg.lan |
|---|---|
| State | New |
| Headers | show |
On Wed, Jun 28, 2017 at 15:32:48 +0300, Lluís Vilanova wrote: > Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu> > --- > accel/tcg/Makefile.objs | 1 > accel/tcg/translator.c | 153 +++++++++++++++++++++++++++++++++++++++++++++ > include/exec/gen-icount.h | 2 - > include/exec/translator.h | 104 +++++++++++++++++++++++++++++++ (snip) > +/** > + * BreakpointCheckType: > + * @BC_MISS: No hit > + * @BC_HIT_INSN: Hit, but continue translating TB > + * @BC_HIT_TB: Hit, stop translating TB > + * > + * How to react to a breakpoint. A hit means no more breakpoints will be checked > + * for the current instruction. > + * > + * Not all breakpoints associated to an address are necessarily raised by > + * targets (e.g., due to conditions encoded in their flags), so tey can decide s/tey/they/ > + * that a breakpoint missed the address (@BP_MISS). > + */ > +typedef enum BreakpointCheckType { > + BC_MISS, > + BC_HIT_INSN, > + BC_HIT_TB, > +} BreakpointCheckType; > + > /** > * DisasJumpType: > * @DISAS_NEXT: Next instruction in program order. > @@ -33,6 +65,78 @@ typedef enum DisasJumpType { > DISAS_TARGET_9, > DISAS_TARGET_10, > DISAS_TARGET_11, > + DISAS_TARGET_12, > + DISAS_TARGET_13, > + DISAS_TARGET_14, > } DisasJumpType; > > +/** > + * DisasContextBase: > + * @tb: Translation block for this disassembly. > + * @pc_first: Address of first guest instruction in this TB. > + * @pc_next: Address of next guest instruction in this TB (current during > + * disassembly). > + * @is_jmp: What instruction to disassemble next. > + * @num_insns: Number of translated instructions (including current). > + * @singlestep_enabled: "Hardware" single stepping enabled. > + * > + * Architecture-agnostic disassembly context. > + */ > +typedef struct DisasContextBase { > + TranslationBlock *tb; > + target_ulong pc_first; > + target_ulong pc_next; > + DisasJumpType is_jmp; > + unsigned int num_insns; > + bool singlestep_enabled; > +} DisasContextBase; > + > +/** > + * TranslatorOps: > + * @init_disas_context: Initialize a DisasContext struct (DisasContextBase has > + * already been initialized). > + * @init_globals: Initialize global variables. > + * @tb_start: Start translating a new TB. > + * @insn_start: Start translating a new instruction. > + * @breakpoint_check: Check if a breakpoint did hit. When called, the breakpoint > + * has already been checked to match the PC. > + * @disas_insn: Disassemble one instruction an return the PC for the next s/disas_insn/translate_insn/ s/an return/and return/ > + * one. Can set db->is_jmp to DJ_TARGET or above to stop s/DJ_TARGET/DISAS_TARGET/ > + * translation. > + * @tb_stop: Stop translating a TB. > + * @disas_flags: Get flags argument for log_target_disas(). s/disas_flags/disas_log/ > + * > + * Target-specific operations for the generic translator loop. > + */ > +typedef struct TranslatorOps { > + void (*init_disas_context)(DisasContextBase *db, CPUState *cpu); > + void (*init_globals)(DisasContextBase *db, CPUState *cpu); > + void (*tb_start)(DisasContextBase *db, CPUState *cpu); > + void (*insn_start)(DisasContextBase *db, CPUState *cpu); > + BreakpointCheckType (*breakpoint_check)(DisasContextBase *db, CPUState *cpu, > + const CPUBreakpoint *bp); > + target_ulong (*translate_insn)(DisasContextBase *db, CPUState *cpu); > + void (*tb_stop)(DisasContextBase *db, CPUState *cpu); > + void (*disas_log)(const DisasContextBase *db, CPUState *cpu); > +} TranslatorOps; > + > +/** > + * translate_block: > + * @ops: Target-specific operations. > + * @db: Disassembly context. > + * @cpu: Target vCPU. > + * @tb: Translation block. > + * > + * Generic translator loop. > + * > + * Translation will stop in the following cases (in order): > + * - When set by #TranslatorOps::insn_start. > + * - When set by #TranslatorOps::translate_insn. > + * - When the TCG operation buffer is full. > + * - When single-stepping is enabled (system-wide or on the current vCPU). > + * - When too many instructions have been translated. > + */ > +void translate_block(const TranslatorOps *ops, DisasContextBase *db, > + CPUState *cpu, TranslationBlock *tb); I'd rather avoid "block" here. Some alternatives: - tb_translate() - translate_tb() - translate() - translator_gen() - translator_loop() E.
On Wed, Jun 28, 2017 at 15:32:48 +0300, Lluís Vilanova wrote: > Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu> > --- (snip) > /** > * DisasJumpType: > * @DISAS_NEXT: Next instruction in program order. > @@ -33,6 +65,78 @@ typedef enum DisasJumpType { > DISAS_TARGET_9, > DISAS_TARGET_10, > DISAS_TARGET_11, > + DISAS_TARGET_12, > + DISAS_TARGET_13, > + DISAS_TARGET_14, > } DisasJumpType; These belong in patch 3. E.
On 06/29/2017 05:02 PM, Emilio G. Cota wrote: >> +void translate_block(const TranslatorOps *ops, DisasContextBase *db, >> + CPUState *cpu, TranslationBlock *tb); > I'd rather avoid "block" here. Some alternatives: > > - tb_translate() > - translate_tb() > - translate() > - translator_gen() > - translator_loop() I like translator_loop. r~
On 06/28/2017 05:32 AM, Lluís Vilanova wrote: > + void (*init_disas_context)(DisasContextBase *db, CPUState *cpu); > + void (*init_globals)(DisasContextBase *db, CPUState *cpu); > + void (*tb_start)(DisasContextBase *db, CPUState *cpu); > + void (*insn_start)(DisasContextBase *db, CPUState *cpu); > + BreakpointCheckType (*breakpoint_check)(DisasContextBase *db, CPUState *cpu, > + const CPUBreakpoint *bp); > + target_ulong (*translate_insn)(DisasContextBase *db, CPUState *cpu); > + void (*tb_stop)(DisasContextBase *db, CPUState *cpu); > + void (*disas_log)(const DisasContextBase *db, CPUState *cpu); Any reason not to stuff the cpu pointer into the DisasContextBase instead of passing it around separately? Otherwise, Reviewed-by: Richard Henderson <rth@twiddle.net> r~
Richard Henderson writes: > On 06/28/2017 05:32 AM, Lluís Vilanova wrote: >> + void (*init_disas_context)(DisasContextBase *db, CPUState *cpu); >> + void (*init_globals)(DisasContextBase *db, CPUState *cpu); >> + void (*tb_start)(DisasContextBase *db, CPUState *cpu); >> + void (*insn_start)(DisasContextBase *db, CPUState *cpu); >> + BreakpointCheckType (*breakpoint_check)(DisasContextBase *db, CPUState *cpu, >> + const CPUBreakpoint *bp); >> + target_ulong (*translate_insn)(DisasContextBase *db, CPUState *cpu); >> + void (*tb_stop)(DisasContextBase *db, CPUState *cpu); >> + void (*disas_log)(const DisasContextBase *db, CPUState *cpu); > Any reason not to stuff the cpu pointer into the DisasContextBase instead of > passing it around separately? None, really. I'll move it from DisasContext (in targets where it's present) into DisasContextBase, and use that one everywhere. > Otherwise, > Reviewed-by: Richard Henderson <rth@twiddle.net> Thanks, Lluis
On 4 July 2017 at 19:59, Lluís Vilanova <vilanova@ac.upc.edu> wrote: > Richard Henderson writes: > >> On 06/28/2017 05:32 AM, Lluís Vilanova wrote: >>> + void (*init_disas_context)(DisasContextBase *db, CPUState *cpu); >>> + void (*init_globals)(DisasContextBase *db, CPUState *cpu); >>> + void (*tb_start)(DisasContextBase *db, CPUState *cpu); >>> + void (*insn_start)(DisasContextBase *db, CPUState *cpu); >>> + BreakpointCheckType (*breakpoint_check)(DisasContextBase *db, CPUState *cpu, >>> + const CPUBreakpoint *bp); >>> + target_ulong (*translate_insn)(DisasContextBase *db, CPUState *cpu); >>> + void (*tb_stop)(DisasContextBase *db, CPUState *cpu); >>> + void (*disas_log)(const DisasContextBase *db, CPUState *cpu); > >> Any reason not to stuff the cpu pointer into the DisasContextBase instead of >> passing it around separately? > > None, really. I'll move it from DisasContext (in targets where it's present) > into DisasContextBase, and use that one everywhere. I kind of like not having CPUState* in DisasContext, because it enforces the rule that you can't read from fields of it inside the target translate.c code without jumping through a hoop (ie copying the info from CPUState->foo to DisasContext->foo). That then acts as a useful flag in code review (or when writing the code) to confirm that foo really is constant for the life of the simulation (or to recommend using a TB flag instead). thanks -- PMM
On 07/04/2017 09:14 AM, Peter Maydell wrote: > On 4 July 2017 at 19:59, Lluís Vilanova <vilanova@ac.upc.edu> wrote: >> Richard Henderson writes: >>> Any reason not to stuff the cpu pointer into the DisasContextBase instead of >>> passing it around separately? >> >> None, really. I'll move it from DisasContext (in targets where it's present) >> into DisasContextBase, and use that one everywhere. > > I kind of like not having CPUState* in DisasContext, because > it enforces the rule that you can't read from fields of > it inside the target translate.c code without jumping through > a hoop (ie copying the info from CPUState->foo to > DisasContext->foo). That then acts as a useful flag in code > review (or when writing the code) to confirm that foo really > is constant for the life of the simulation (or to recommend > using a TB flag instead). I don't see how the spelling "cpu" vs "dc->cpu" really affects that. More practically, I don't see that "cpu" will actually be used by most of those hooks. But because of things like cpu->some_target_feature, it's kind of hard to predict. r~
On 4 July 2017 at 23:31, Richard Henderson <rth@twiddle.net> wrote: > On 07/04/2017 09:14 AM, Peter Maydell wrote: >> I kind of like not having CPUState* in DisasContext, because >> it enforces the rule that you can't read from fields of >> it inside the target translate.c code without jumping through >> a hoop (ie copying the info from CPUState->foo to >> DisasContext->foo). That then acts as a useful flag in code >> review (or when writing the code) to confirm that foo really >> is constant for the life of the simulation (or to recommend >> using a TB flag instead). > > > I don't see how the spelling "cpu" vs "dc->cpu" really affects that. If you put it in dc->cpu then everywhere that gets the dc gets the cpu, unavoidably (which is why I'm suggesting it would be nicer not to do that). If you don't put it in dc then you can structure your translate code so that pretty much all of it gets the dc but not the cpu. target/arm/translate-a64.c does this, for instance. thanks -- PMM
diff --git a/accel/tcg/Makefile.objs b/accel/tcg/Makefile.objs index f173cd5397..3a5da5357c 100644 --- a/accel/tcg/Makefile.objs +++ b/accel/tcg/Makefile.objs @@ -1,3 +1,4 @@ obj-$(CONFIG_SOFTMMU) += tcg-all.o obj-$(CONFIG_SOFTMMU) += cputlb.o obj-y += cpu-exec.o cpu-exec-common.o translate-all.o translate-common.o +obj-y += translator.o diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c new file mode 100644 index 0000000000..b941ebdc62 --- /dev/null +++ b/accel/tcg/translator.c @@ -0,0 +1,153 @@ +/* + * Generic intermediate code generation. + * + * Copyright (C) 2016-2017 Lluís Vilanova <vilanova@ac.upc.edu> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include "qemu/osdep.h" +#include "qemu-common.h" +#include "qemu/error-report.h" +#include "cpu.h" +#include "tcg/tcg.h" +#include "tcg/tcg-op.h" +#include "exec/exec-all.h" +#include "exec/gen-icount.h" +#include "exec/log.h" +#include "exec/translator.h" + + +static inline void translate_block_tcg_check(const DisasContextBase *db) +{ + if (tcg_check_temp_count()) { + error_report("warning: TCG temporary leaks before "TARGET_FMT_lx, + db->pc_next); + } +} + +void translate_block(const TranslatorOps *ops, DisasContextBase *db, + CPUState *cpu, TranslationBlock *tb) +{ + int max_insns; + + /* Initialize DisasContext */ + db->tb = tb; + db->pc_first = tb->pc; + db->pc_next = db->pc_first; + db->is_jmp = DISAS_NEXT; + db->num_insns = 0; + db->singlestep_enabled = cpu->singlestep_enabled; + ops->init_disas_context(db, cpu); + + /* Initialize globals */ + ops->init_globals(db, cpu); + tcg_clear_temp_count(); + + /* Instruction counting */ + max_insns = db->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 (db->singlestep_enabled || singlestep) { + max_insns = 1; + } + + /* Start translating */ + gen_tb_start(db->tb); + ops->tb_start(db, cpu); + + while (true) { + db->num_insns++; + ops->insn_start(db, cpu); + + /* Early exit before breakpoint checks */ + if (unlikely(db->is_jmp != DISAS_NEXT)) { + break; + } + + /* Pass breakpoint hits to target for further processing */ + if (unlikely(!QTAILQ_EMPTY(&cpu->breakpoints))) { + CPUBreakpoint *bp; + QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) { + if (bp->pc == db->pc_next) { + BreakpointCheckType bp_check = + ops->breakpoint_check(db, cpu, bp); + switch (bp_check) { + case BC_MISS: + /* Target ignored this breakpoint, go to next */ + break; + case BC_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. + */ + goto done_breakpoints; + case BC_HIT_TB: + /* Hit, end TB */ + goto done_generating; + default: + g_assert_not_reached(); + } + } + } + } + done_breakpoints: + + /* Accept I/O on last instruction */ + if (db->num_insns == max_insns && (db->tb->cflags & CF_LAST_IO)) { + gen_io_start(); + } + + /* Disassemble one instruction */ + db->pc_next = ops->translate_insn(db, cpu); + + /**************************************************/ + /* Conditions to stop translation */ + /**************************************************/ + + /* Target-specific conditions set by disassembly */ + if (db->is_jmp != DISAS_NEXT) { + break; + } + + /* Too many instructions */ + if (tcg_op_buf_full() || db->num_insns >= max_insns) { + db->is_jmp = DISAS_TOO_MANY; + break; + } + + translate_block_tcg_check(db); + } + + ops->tb_stop(db, cpu); + + if (db->tb->cflags & CF_LAST_IO) { + gen_io_end(); + } + +done_generating: + gen_tb_end(db->tb, db->num_insns); + + translate_block_tcg_check(db); + +#ifdef DEBUG_DISAS + if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM) + && qemu_log_in_addr_range(db->pc_first)) { + qemu_log_lock(); + qemu_log("----------------\n"); + ops->disas_log(db, cpu); + qemu_log("\n"); + qemu_log_unlock(); + } +#endif + + db->tb->size = db->pc_next - db->pc_first; + db->tb->icount = db->num_insns; +} diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h index 3fb17d435a..d66ca09f60 100644 --- a/include/exec/gen-icount.h +++ b/include/exec/gen-icount.h @@ -44,7 +44,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) { if (tb->cflags & CF_USE_ICOUNT) { /* Update the num_insn immediate parameter now that we know diff --git a/include/exec/translator.h b/include/exec/translator.h index 1ffab03a53..c6fe64f988 100644 --- a/include/exec/translator.h +++ b/include/exec/translator.h @@ -10,6 +10,38 @@ #ifndef EXEC__TRANSLATOR_H #define EXEC__TRANSLATOR_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" +#include "tcg/tcg.h" + + +/** + * BreakpointCheckType: + * @BC_MISS: No hit + * @BC_HIT_INSN: Hit, but continue translating TB + * @BC_HIT_TB: Hit, stop translating TB + * + * How to react to a breakpoint. A hit means no more breakpoints will be checked + * for the current instruction. + * + * Not all breakpoints associated to an address are necessarily raised by + * targets (e.g., due to conditions encoded in their flags), so tey can decide + * that a breakpoint missed the address (@BP_MISS). + */ +typedef enum BreakpointCheckType { + BC_MISS, + BC_HIT_INSN, + BC_HIT_TB, +} BreakpointCheckType; + /** * DisasJumpType: * @DISAS_NEXT: Next instruction in program order. @@ -33,6 +65,78 @@ typedef enum DisasJumpType { DISAS_TARGET_9, DISAS_TARGET_10, DISAS_TARGET_11, + DISAS_TARGET_12, + DISAS_TARGET_13, + DISAS_TARGET_14, } DisasJumpType; +/** + * DisasContextBase: + * @tb: Translation block for this disassembly. + * @pc_first: Address of first guest instruction in this TB. + * @pc_next: Address of next guest instruction in this TB (current during + * disassembly). + * @is_jmp: What instruction to disassemble next. + * @num_insns: Number of translated instructions (including current). + * @singlestep_enabled: "Hardware" single stepping enabled. + * + * Architecture-agnostic disassembly context. + */ +typedef struct DisasContextBase { + TranslationBlock *tb; + target_ulong pc_first; + target_ulong pc_next; + DisasJumpType is_jmp; + unsigned int num_insns; + bool singlestep_enabled; +} DisasContextBase; + +/** + * TranslatorOps: + * @init_disas_context: Initialize a DisasContext struct (DisasContextBase has + * already been initialized). + * @init_globals: Initialize global variables. + * @tb_start: Start translating a new TB. + * @insn_start: Start translating a new instruction. + * @breakpoint_check: Check if a breakpoint did hit. When called, the breakpoint + * has already been checked to match the PC. + * @disas_insn: Disassemble one instruction an return the PC for the next + * one. Can set db->is_jmp to DJ_TARGET or above to stop + * translation. + * @tb_stop: Stop translating a TB. + * @disas_flags: Get flags argument for log_target_disas(). + * + * Target-specific operations for the generic translator loop. + */ +typedef struct TranslatorOps { + void (*init_disas_context)(DisasContextBase *db, CPUState *cpu); + void (*init_globals)(DisasContextBase *db, CPUState *cpu); + void (*tb_start)(DisasContextBase *db, CPUState *cpu); + void (*insn_start)(DisasContextBase *db, CPUState *cpu); + BreakpointCheckType (*breakpoint_check)(DisasContextBase *db, CPUState *cpu, + const CPUBreakpoint *bp); + target_ulong (*translate_insn)(DisasContextBase *db, CPUState *cpu); + void (*tb_stop)(DisasContextBase *db, CPUState *cpu); + void (*disas_log)(const DisasContextBase *db, CPUState *cpu); +} TranslatorOps; + +/** + * translate_block: + * @ops: Target-specific operations. + * @db: Disassembly context. + * @cpu: Target vCPU. + * @tb: Translation block. + * + * Generic translator loop. + * + * Translation will stop in the following cases (in order): + * - When set by #TranslatorOps::insn_start. + * - When set by #TranslatorOps::translate_insn. + * - When the TCG operation buffer is full. + * - When single-stepping is enabled (system-wide or on the current vCPU). + * - When too many instructions have been translated. + */ +void translate_block(const TranslatorOps *ops, DisasContextBase *db, + CPUState *cpu, TranslationBlock *tb); + #endif /* EXEC__TRANSLATOR_H */
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu> --- accel/tcg/Makefile.objs | 1 accel/tcg/translator.c | 153 +++++++++++++++++++++++++++++++++++++++++++++ include/exec/gen-icount.h | 2 - include/exec/translator.h | 104 +++++++++++++++++++++++++++++++ 4 files changed, 259 insertions(+), 1 deletion(-) create mode 100644 accel/tcg/translator.c