[PATCH v3 1/7] target/ppc: Created !TCG SPR registration macro

Bruno Larsen (billionai) posted 7 patches 4 years, 9 months ago
[PATCH v3 1/7] target/ppc: Created !TCG SPR registration macro
Posted by Bruno Larsen (billionai) 4 years, 9 months ago
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 */
 
 #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
-- 
2.17.1


Re: [PATCH v3 1/7] target/ppc: Created !TCG SPR registration macro
Posted by Richard Henderson 4 years, 9 months ago
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)                       \
Re: [PATCH v3 1/7] target/ppc: Created !TCG SPR registration macro
Posted by David Gibson 4 years, 9 months ago
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.

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