Re: [Qemu-devel] [PATCH 3/4] target: [tcg] Add generic translation framework

[Richard Henderson]

On 07/15/2016 09:42 PM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova <address@hidden>
> ---
>  include/exec/translate-all_template.h |   58 ++++++++++++
>  include/qom/cpu.h                     |   21 ++++
>  translate-all_template.h              |  160 +++++++++++++++++++++++++++++++++
>  3 files changed, 239 insertions(+)
>  create mode 100644 include/exec/translate-all_template.h
>  create mode 100644 translate-all_template.h
>
> diff --git a/include/exec/translate-all_template.h 
> b/include/exec/translate-all_template.h
> new file mode 100644
> index 0000000..9e0c361
> --- /dev/null
> +++ b/include/exec/translate-all_template.h
> @@ -0,0 +1,58 @@
> +/*
> + * Generic intermediate code generation.
> + *
> + * Copyright (C) 2016 Lluís Vilanova <address@hidden>
> + *
> + * 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"
> +
> +
> +/**
> + * 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;

I think you might as well add the common cases: exit tb via exception, exit via 
goto_tb, exit via indirect jump (pc updated), exit for state change (pc not 
updated).

See the set used for alpha.

> +void gen_intermediate_code(CPUState *cpu, struct TranslationBlock *tb)
> +{
> +    CPUArchState *env = cpu->env_ptr;
> +    DisasContext dc1, *dc = &dc1;
> +    int num_insns;
> +    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;
> +    gen_intermediate_code_target_init_disas_context(dc, env);
> +
> +    /* Target-specific globals */
> +    gen_intermediate_code_target_init_globals(dc, env);
> +
> +    /* Instruction counting */
> +    num_insns = 0;
> +    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;
> +    }

I've started adding the singlestep check here, outside the loop, setting 
max_insns to 1.

> +
> +    /* Start translating */
> +    gen_tb_start(dc->base.tb);
> +
> +    while (true) {
> +        CPUBreakpoint *bp;
> +
> +        tcg_gen_insn_start(dc->base.pc_next, dc->cc_op);

You've probably discovered that this will have to be its own hook.

> +        num_insns++;
> +
> +        /* Pass breakpoint hits to target for further processing */
> +        bp = NULL;
> +        do {
> +            bp = cpu_breakpoint_get(cpu, dc->base.pc_next, bp);
> +            if (unlikely(bp)) {
> +                if (gen_intermediate_code_target_breakpoint_hit(dc, env, bp)) {
> +                    goto done_generating;
> +                }
> +            }
> +        } while (bp != NULL);

Why would you need to loop here?


r~