| Message ID | 20210430193533.82136-2-bruno.larsen@eldorado.org.br (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/ppc: untangle CPU init from translation | expand |
On 4/30/21 12:35 PM, Bruno Larsen (billionai) wrote: > moved RW callback parameters of _spr_register into an ifdef, to support > building without TCG in the future, and added definitions for > spr_register and spr_register_kvm, to keep the same call regardless of > build options > > Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br> > --- > target/ppc/translate_init.c.inc | 26 +++++++++++++++++++++----- > 1 file changed, 21 insertions(+), 5 deletions(-) > > diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc > index 6235eb7536..22b23793fd 100644 > --- a/target/ppc/translate_init.c.inc > +++ b/target/ppc/translate_init.c.inc > @@ -720,6 +720,7 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val) > helper_mtvscr(env, val); > } > > +#ifdef CONFIG_TCG > #ifdef CONFIG_USER_ONLY > #define spr_register_kvm(env, num, name, uea_read, uea_write, \ > oea_read, oea_write, one_reg_id, initial_value) \ > @@ -728,7 +729,7 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val) > oea_read, oea_write, hea_read, hea_write, \ > one_reg_id, initial_value) \ > _spr_register(env, num, name, uea_read, uea_write, initial_value) > -#else > +#else /* CONFIG_USER_ONLY */ > #if !defined(CONFIG_KVM) > #define spr_register_kvm(env, num, name, uea_read, uea_write, \ > oea_read, oea_write, one_reg_id, initial_value) \ > @@ -739,7 +740,7 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val) > one_reg_id, initial_value) \ > _spr_register(env, num, name, uea_read, uea_write, \ > oea_read, oea_write, hea_read, hea_write, initial_value) > -#else > +#else /* CONFIG_KVM */ > #define spr_register_kvm(env, num, name, uea_read, uea_write, \ > oea_read, oea_write, one_reg_id, initial_value) \ > _spr_register(env, num, name, uea_read, uea_write, \ > @@ -751,8 +752,21 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val) > _spr_register(env, num, name, uea_read, uea_write, \ > oea_read, oea_write, hea_read, hea_write, \ > one_reg_id, initial_value) > -#endif > -#endif > +#endif /* CONFIG_KVM */ > +#endif /* CONFIG_USER_ONLY */ > +#else /* CONFIG_TCG */ > +#ifdef CONFIG_KVM /* sanity check. should always enter this */ > +#define spr_register_kvm(env, num, name, uea_read, uea_write, \ > + oea_read, oea_write, one_reg_id, initial_value) \ > + _spr_register(env, num, name, one_reg_id, initial_value) > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, \ > + oea_read, oea_write, hea_read, hea_write, \ > + one_reg_id, initial_value) \ > + _spr_register(env, num, name, one_reg_id, initial_value) > +#else /* CONFIG_KVM */ > +#error "Either TCG or KVM should be configured" > +#endif /* CONFIG_KVM */ > +#endif /* CONFIG_TCG */ I think this ifdef tree, and the repetition, is unnecessarily confusing. How about something like this? r~ diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc index 22b23793fd..1e1539f929 100644 --- a/target/ppc/translate_init.c.inc +++ b/target/ppc/translate_init.c.inc @@ -721,52 +721,34 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val) } #ifdef CONFIG_TCG -#ifdef CONFIG_USER_ONLY +# define U(X) X, +# ifndef CONFIG_USER_ONLY +# define S(X) X, +# else +# define S(X) +# endif +#else +# define U(X) +# define S(X) +#endif +#ifdef CONFIG_KVM +# define K(X) X, +#else +# define K(X) +#endif + #define spr_register_kvm(env, num, name, uea_read, uea_write, \ oea_read, oea_write, one_reg_id, initial_value) \ - _spr_register(env, num, name, uea_read, uea_write, initial_value) + _spr_register(env, num, name, U(uea_read) U(uea_write) \ + S(oea_read) S(oea_write) S(oea_read) S(oea_write) \ + K(one_reg_id) initial_value) + #define spr_register_kvm_hv(env, num, name, uea_read, uea_write, \ oea_read, oea_write, hea_read, hea_write, \ one_reg_id, initial_value) \ - _spr_register(env, num, name, uea_read, uea_write, initial_value) -#else /* CONFIG_USER_ONLY */ -#if !defined(CONFIG_KVM) -#define spr_register_kvm(env, num, name, uea_read, uea_write, \ - oea_read, oea_write, one_reg_id, initial_value) \ - _spr_register(env, num, name, uea_read, uea_write, \ - oea_read, oea_write, oea_read, oea_write, initial_value) -#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, \ - oea_read, oea_write, hea_read, hea_write, \ - one_reg_id, initial_value) \ - _spr_register(env, num, name, uea_read, uea_write, \ - oea_read, oea_write, hea_read, hea_write, initial_value) -#else /* CONFIG_KVM */ -#define spr_register_kvm(env, num, name, uea_read, uea_write, \ - oea_read, oea_write, one_reg_id, initial_value) \ - _spr_register(env, num, name, uea_read, uea_write, \ - oea_read, oea_write, oea_read, oea_write, \ - one_reg_id, initial_value) -#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, \ - oea_read, oea_write, hea_read, hea_write, \ - one_reg_id, initial_value) \ - _spr_register(env, num, name, uea_read, uea_write, \ - oea_read, oea_write, hea_read, hea_write, \ - one_reg_id, initial_value) -#endif /* CONFIG_KVM */ -#endif /* CONFIG_USER_ONLY */ -#else /* CONFIG_TCG */ -#ifdef CONFIG_KVM /* sanity check. should always enter this */ -#define spr_register_kvm(env, num, name, uea_read, uea_write, \ - oea_read, oea_write, one_reg_id, initial_value) \ - _spr_register(env, num, name, one_reg_id, initial_value) -#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, \ - oea_read, oea_write, hea_read, hea_write, \ - one_reg_id, initial_value) \ - _spr_register(env, num, name, one_reg_id, initial_value) -#else /* CONFIG_KVM */ -#error "Either TCG or KVM should be configured" -#endif /* CONFIG_KVM */ -#endif /* CONFIG_TCG */ + _spr_register(env, num, name, U(uea_read) U(uea_write) \ + S(oea_read) S(oea_write) S(hea_read) S(hea_write) \ + K(one_reg_id) initial_value) #define spr_register(env, num, name, uea_read, uea_write, \ oea_read, oea_write, initial_value) \
On Fri, Apr 30, 2021 at 05:27:13PM -0700, Richard Henderson wrote: > On 4/30/21 12:35 PM, Bruno Larsen (billionai) wrote: > > moved RW callback parameters of _spr_register into an ifdef, to support > > building without TCG in the future, and added definitions for > > spr_register and spr_register_kvm, to keep the same call regardless of > > build options > > > > Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br> > > --- > > target/ppc/translate_init.c.inc | 26 +++++++++++++++++++++----- > > 1 file changed, 21 insertions(+), 5 deletions(-) > > > > diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc > > index 6235eb7536..22b23793fd 100644 > > --- a/target/ppc/translate_init.c.inc > > +++ b/target/ppc/translate_init.c.inc > > @@ -720,6 +720,7 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val) > > helper_mtvscr(env, val); > > } > > +#ifdef CONFIG_TCG > > #ifdef CONFIG_USER_ONLY > > #define spr_register_kvm(env, num, name, uea_read, uea_write, \ > > oea_read, oea_write, one_reg_id, initial_value) \ > > @@ -728,7 +729,7 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val) > > oea_read, oea_write, hea_read, hea_write, \ > > one_reg_id, initial_value) \ > > _spr_register(env, num, name, uea_read, uea_write, initial_value) > > -#else > > +#else /* CONFIG_USER_ONLY */ > > #if !defined(CONFIG_KVM) > > #define spr_register_kvm(env, num, name, uea_read, uea_write, \ > > oea_read, oea_write, one_reg_id, initial_value) \ > > @@ -739,7 +740,7 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val) > > one_reg_id, initial_value) \ > > _spr_register(env, num, name, uea_read, uea_write, \ > > oea_read, oea_write, hea_read, hea_write, initial_value) > > -#else > > +#else /* CONFIG_KVM */ > > #define spr_register_kvm(env, num, name, uea_read, uea_write, \ > > oea_read, oea_write, one_reg_id, initial_value) \ > > _spr_register(env, num, name, uea_read, uea_write, \ > > @@ -751,8 +752,21 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val) > > _spr_register(env, num, name, uea_read, uea_write, \ > > oea_read, oea_write, hea_read, hea_write, \ > > one_reg_id, initial_value) > > -#endif > > -#endif > > +#endif /* CONFIG_KVM */ > > +#endif /* CONFIG_USER_ONLY */ > > +#else /* CONFIG_TCG */ > > +#ifdef CONFIG_KVM /* sanity check. should always enter this */ > > +#define spr_register_kvm(env, num, name, uea_read, uea_write, \ > > + oea_read, oea_write, one_reg_id, initial_value) \ > > + _spr_register(env, num, name, one_reg_id, initial_value) > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, \ > > + oea_read, oea_write, hea_read, hea_write, \ > > + one_reg_id, initial_value) \ > > + _spr_register(env, num, name, one_reg_id, initial_value) > > +#else /* CONFIG_KVM */ > > +#error "Either TCG or KVM should be configured" > > +#endif /* CONFIG_KVM */ > > +#endif /* CONFIG_TCG */ > > I think this ifdef tree, and the repetition, is unnecessarily confusing. > How about something like this? I've applied Richard's version of this to my tree now, so you might need to rebase the rest of the series.
diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc index 6235eb7536..22b23793fd 100644 --- a/target/ppc/translate_init.c.inc +++ b/target/ppc/translate_init.c.inc @@ -720,6 +720,7 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val) helper_mtvscr(env, val); } +#ifdef CONFIG_TCG #ifdef CONFIG_USER_ONLY #define spr_register_kvm(env, num, name, uea_read, uea_write, \ oea_read, oea_write, one_reg_id, initial_value) \ @@ -728,7 +729,7 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val) oea_read, oea_write, hea_read, hea_write, \ one_reg_id, initial_value) \ _spr_register(env, num, name, uea_read, uea_write, initial_value) -#else +#else /* CONFIG_USER_ONLY */ #if !defined(CONFIG_KVM) #define spr_register_kvm(env, num, name, uea_read, uea_write, \ oea_read, oea_write, one_reg_id, initial_value) \ @@ -739,7 +740,7 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val) one_reg_id, initial_value) \ _spr_register(env, num, name, uea_read, uea_write, \ oea_read, oea_write, hea_read, hea_write, initial_value) -#else +#else /* CONFIG_KVM */ #define spr_register_kvm(env, num, name, uea_read, uea_write, \ oea_read, oea_write, one_reg_id, initial_value) \ _spr_register(env, num, name, uea_read, uea_write, \ @@ -751,8 +752,21 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val) _spr_register(env, num, name, uea_read, uea_write, \ oea_read, oea_write, hea_read, hea_write, \ one_reg_id, initial_value) -#endif -#endif +#endif /* CONFIG_KVM */ +#endif /* CONFIG_USER_ONLY */ +#else /* CONFIG_TCG */ +#ifdef CONFIG_KVM /* sanity check. should always enter this */ +#define spr_register_kvm(env, num, name, uea_read, uea_write, \ + oea_read, oea_write, one_reg_id, initial_value) \ + _spr_register(env, num, name, one_reg_id, initial_value) +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, \ + oea_read, oea_write, hea_read, hea_write, \ + one_reg_id, initial_value) \ + _spr_register(env, num, name, one_reg_id, initial_value) +#else /* CONFIG_KVM */ +#error "Either TCG or KVM should be configured" +#endif /* CONFIG_KVM */ +#endif /* CONFIG_TCG */ #define spr_register(env, num, name, uea_read, uea_write, \ oea_read, oea_write, initial_value) \ @@ -768,6 +782,7 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val) static inline void _spr_register(CPUPPCState *env, int num, const char *name, +#if defined(CONFIG_TCG) void (*uea_read)(DisasContext *ctx, int gprn, int sprn), void (*uea_write)(DisasContext *ctx, @@ -782,7 +797,8 @@ static inline void _spr_register(CPUPPCState *env, int num, int gprn, int sprn), void (*hea_write)(DisasContext *opaque, int sprn, int gprn), -#endif +#endif /* CONFIG_USER_ONLY */ +#endif /* CONFIG_TCG */ #if defined(CONFIG_KVM) uint64_t one_reg_id, #endif
moved RW callback parameters of _spr_register into an ifdef, to support building without TCG in the future, and added definitions for spr_register and spr_register_kvm, to keep the same call regardless of build options Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br> --- target/ppc/translate_init.c.inc | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-)