Re: [Qemu-devel] [PATCH v14 08/34] tcg: Add generic translation framework

[Lluís Vilanova]

Richard Henderson writes:

> From: Lluís Vilanova <address@hidden>
> Signed-off-by: Lluís Vilanova <address@hidden>
> Message-Id: <address@hidden>
> [rth: Moved max_insns adjustment from tb_start to init_disas_context.
> Removed pc_next return from translate_insn.
> Removed tcg_check_temp_count from generic loop.
> Moved gen_io_end to exactly match gen_io_start.
> Use qemu_log instead of error_report for temporary leaks.
> Moved TB size/icount assignments before disas_log.]
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  include/exec/translator.h | 101 +++++++++++++++++++++++++++++++++++
>  accel/tcg/translator.c    | 133 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  accel/tcg/Makefile.objs   |   1 +
>  3 files changed, 235 insertions(+)
>  create mode 100644 accel/tcg/translator.c

> diff --git a/include/exec/translator.h b/include/exec/translator.h
> index b51b8f8..aa84376 100644
> --- a/include/exec/translator.h
> +++ b/include/exec/translator.h
> @@ -10,6 +10,19 @@
>  #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"
> +
> +
>  /**
>   * DisasJumpType:
>   * @DISAS_NEXT: Next instruction in program order.
> @@ -37,4 +50,92 @@ typedef enum DisasJumpType {
>      DISAS_TARGET_11,
>  } 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 the target-specific portions of DisasContext struct.
> + *      The generic DisasContextBase has already been initialized.
> + *      Return max_insns, modified as necessary by db->tb->flags.
> + *
> + * @tb_start:
> + *      Emit any code required before the start of the main loop,
> + *      after the generic gen_tb_start().
> + *
> + * @insn_start:
> + *      Emit the tcg_gen_insn_start opcode.
> + *
> + * @breakpoint_check:
> + *      When called, the breakpoint has already been checked to match the PC,
> + *      but the target may decide the breakpoint missed the address
> + *      (e.g., due to conditions encoded in their flags).  Return true to
> + *      indicate that the breakpoint did hit, in which case no more 
> breakpoints
> + *      are checked.  If the breakpoint did hit, emit any code required to
> + *      signal the exception, and set db->is_jmp as necessary to terminate
> + *      the main loop.
> + *
> + * @translate_insn:
> + *      Disassemble one instruction and set db->pc_next for the start
> + *      of the following instruction.  Set db->is_jmp as necessary to
> + *      terminate the main loop.
> + *
> + * @tb_stop:
> + *      Emit any opcodes required to exit the TB, based on db->is_jmp.
> + *
> + * @disas_log:
> + *      Print instruction disassembly to log.
> + */
> +typedef struct TranslatorOps {
> +    int (*init_disas_context)(DisasContextBase *db, CPUState *cpu,
> +                              int max_insns);
> +    void (*tb_start)(DisasContextBase *db, CPUState *cpu);
> +    void (*insn_start)(DisasContextBase *db, CPUState *cpu);
> +    bool (*breakpoint_check)(DisasContextBase *db, CPUState *cpu,
> +                             const CPUBreakpoint *bp);
> +    void (*translate_insn)(DisasContextBase *db, CPUState *cpu);
> +    void (*tb_stop)(DisasContextBase *db, CPUState *cpu);
> +    void (*disas_log)(const DisasContextBase *db, CPUState *cpu);
> +} TranslatorOps;
> +
> +/**
> + * translator_loop:
> + * @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 et by #TranslatorOps::insn_start.

Seems untrue; there's a tcg_debug_assert, so this should now probably be
breakpoint_check() instead.

> + * - 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 translator_loop(const TranslatorOps *ops, DisasContextBase *db,
> +                     CPUState *cpu, TranslationBlock *tb);

For the "When set by #TranslatorOps:..." I'd also talk about setting is_jmp for
that, and describe the type of stop that different is_jmp values produce
(TOO_MANY is "delayed" and NORETURN is "immediate" when set in
breakpoint_check; all values are "immediate" when set in translate_insn()).


Thanks,
  Lluis