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

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3296 bytes --]

On Mon, Apr 19, 2021 at 02:40:35PM +0000, Bruno Piazera Larsen wrote:
> > > > * 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.
> 
> What I was doing was to only register the spr once, and use the
> accel-specific functions to set the relevant attributes, so spr_common
> wouldn't need to where (and if) spr_read_* exists or not.
> Would this work?
> 
> Just ignoring the read and write functions means we still need
> to compile them, or at least stub them, otherwise we'd get linker
> problems.

Not if you use a macro which will simply elide the references in the
!TCG case.  Actually I think even an inline wrapper will do it, I'm
pretty sure the compiler is smart enough to optimize the references
out in that case.

> And ifdef'ing them out of the calls would be quite a
> pain to understand the code later on.
> 
> 
> 
> Bruno Piazera Larsen
> 
> Instituto de Pesquisas ELDORADO<http://clickemailmkt.eldorado.org.br/ls/click?upn=UPoxpeIcHnAcbUZyo7TTaswyiVb1TXP3jEbQqiiJKKGsxOn8hBEs5ZsMLQfXkKuKXZ7MVDg0ij9eG8HV4TXI75dBzDiNGLxQ8Xx5PzCVNt6TpGrzBbU-2Biu0o69X5ce-2FW-2FOk1uUipuK0fZnWXJEgbRw-3D-3DJY4T_wWk-2BG6VvNBoa1YzxYjhCdFS9IfANIaBzDSklR1NyyrKOI1wj0P-2BdBFcuO4FnHcsA1MyHu0ly1Yt3oDMp7KKdJPM68iKuI2jiRH5v4B0d8wf3chU3qy5n5iXWnW1QjSaNFHOgELzxaP-2FnesTeBgJ5dFkjH4f279sVQpOtyjw5xAqj34M6pgNRAxVvuXif4IWDcVzXg1FzfYlEfkKzr9vvpA3Hg8kitwMtlU3zwbQUBCgL30fQoJPcRPMGKyOY8RmoAlXNqTJYDYIvqmfnI7KLUvw6vKB5R-2B5q1FJRAzX7H-2BmF0NnDET6jMLuIqtCcVIch>
> 
> Departamento Computação Embarcada
> 
> Analista de Software Trainee
> 
> Aviso Legal - Disclaimer<https://www.eldorado.org.br/disclaimer.html>
> 
> ________________________________

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]