Re: [Qemu-devel] [PATCH v11 04/29] target: [tcg] Add generic translation framework

[Peter Maydell]

On 4 July 2017 at 19:59, Lluís Vilanova <address@hidden> 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