[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/4] target/ppc: Code motion required to build disabling tcg
From: |
David Gibson |
Subject: |
Re: [PATCH 1/4] target/ppc: Code motion required to build disabling tcg |
Date: |
Mon, 19 Apr 2021 15:23:17 +1000 |
On Tue, Apr 13, 2021 at 06:38:57PM -0300, Fabiano Rosas wrote:
> 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);
> }
I really dislike the idea above - it'd be way too easy for KVM and TCG
to get out of sync. Instead make spr_register() itself a macro if
necessary, so it just ignores the access functions in the !TCG case.
> //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.
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
RE: [PATCH 1/4] target/ppc: Code motion required to build disabling tcg, Bruno Piazera Larsen, 2021/04/19
RE: [PATCH 1/4] target/ppc: Code motion required to build disabling tcg, Bruno Piazera Larsen, 2021/04/20
RE: [PATCH 1/4] target/ppc: Code motion required to build disabling tcg, Bruno Piazera Larsen, 2021/04/22