RE: [PATCH 1/4] target/ppc: Code motion required to build disabling tcg

[Fabiano Rosas <farosas@linux.ibm.com>]

Bruno Piazera Larsen <bruno.larsen@eldorado.org.br> writes:

>> I'm actually not sure if we'll want translate_init.c for !tcg builds.
>> It's *primarily* for TCG, but we still need at least some of the cpu
>> state structure for KVM, and some of that is initialized in
>> translate_init.
>>
>> I think it will probably make more sense to leave it in for a first
>> cut.  Later refinement might end up removing it.
>>
>> The whole #include translate_init.c.inc thing might make for some
>> awkward fiddling in this, of course.
>
> I just checked, there is going to be some shuffling of functions
> around, as there are some static variables defined on translate.c,
> and used in translate_init.c.inc, some functions needed for KVM
> on translate.c and some TCG only functions in the
> translate_init.c.inc.
>
> The trivial path is to:
> * rename translate_init.c.inc to cpu_init.c (since it has to do with
> initial definitions for CPUs, and it's not related to translating
> anymore);

Below I'm assuming we have one place for TCG stuff and other for KVM
stuff, whatever this particular discussion ends up producing.

> * move gen_write_xer and gen_read_xer into cpu_init.c, as they're
> used for some sprs, and whatever needs to be moved with it

I'd leave them where they are currently. Instead what I think we should
do is to find a way to not need the uea/oea/hea|read/write callbacks
with KVM.

Maybe extract a function from _spr_register that sets what is common for
both tcg and kvm (num, name, initial_value, AFAICT). Then alter the
gen_spr* functions to first create all registers and then call both
configs to supplement:

//tcg.c
static void tcg_gen_spr_generic(CPUPPCState *env)
{
    // these only set the callbacks        
    spr_register(env, SPR_FOO,
                 SPR_NOACCESS, SPR_NOACCESS,
                 &spr_read_foo, &spr_write_foo);
    spr_register(env, SPR_BAR,
                 SPR_NOACCESS, SPR_NOACCESS,
                 &spr_read_bar, &spr_write_bar);
}

//kvm.c
static void kvm_gen_spr_generic(CPUPPCState *env)
{
    // these only set one_reg_id
    spr_register_kvm(env, SPR_FOO, KVM_REG_PPC_FOO);
    spr_register_kvm(env, SPR_BAR, KVM_REG_PPC_BAR);
}

//common.c
static void gen_spr_generic(CPUPPCState *env)
{
    // these only set name, num, initial value
    spr_register(env, SPR_FOO, "FOO", 0xf00);
    spr_register(env, SPR_BAR, "BAR", 0xb4d);
    ...

    // have these stubbed if not chosen via config
    tcg_gen_spr_generic(env);
    kvm_gen_spr_generic(env);
}

init_ppc_proc()
{
        ...
        gen_spr_generic(env);
        ...
}

Can anyone see a better way? That would be much easier if we could
afford to say that TCG and KVM are mutually exclusive for a given build,
but I don't think they are.

> * move is_indirect_opcode and ind_table to translate.c, since they
> are used to translate ppc instructions, and the things defined for
> these functions

Makes sense. This and the other part below about callback tables would
be mostly about moving code so it's a candidate for coming soon.

> * Figure out what needs to be added to the includes for both
> files to compile
> * move opcodes and invalid_handler into cpu_init.c, because they
> are only used by stuff in this file.
>
> I'm just not sure about this last point. The stuff that use opcodes
> create the callback tables for TCG, AFAICT. The better plan would
> be to move all of that to tanslate.c, but might be a lot.

translate.c seems like a better place indeed.