All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Henderson <richard.henderson@linaro.org>
To: "Bruno Larsen (billionai)" <bruno.larsen@eldorado.org.br>,
	qemu-devel@nongnu.org
Cc: farosas@linux.ibm.com, luis.pires@eldorado.org.br,
	lucas.araujo@eldorado.org.br, fernando.valle@eldorado.org.br,
	qemu-ppc@nongnu.org, matheus.ferst@eldorado.org.br,
	david@gibson.dropbear.id.au
Subject: Re: [PATCH v3 1/7] target/ppc: Created !TCG SPR registration macro
Date: Fri, 30 Apr 2021 17:27:13 -0700	[thread overview]
Message-ID: <cb40e011-cf22-1044-43b0-75401d074796@linaro.org> (raw)
In-Reply-To: <20210430193533.82136-2-bruno.larsen@eldorado.org.br>

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

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~

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 4120 bytes --]

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

  reply	other threads:[~2021-05-01  0:29 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-30 19:35 [PATCH v3 0/7] target/ppc: untangle CPU init from translation Bruno Larsen (billionai)
2021-04-30 19:35 ` [PATCH v3 1/7] target/ppc: Created !TCG SPR registration macro Bruno Larsen (billionai)
2021-05-01  0:27   ` Richard Henderson [this message]
2021-05-03  4:37     ` David Gibson
2021-04-30 19:35 ` [PATCH v3 2/7] target/ppc: Isolated SPR read/write callbacks Bruno Larsen (billionai)
2021-05-01  0:55   ` Richard Henderson
2021-05-03 20:07     ` Bruno Piazera Larsen
2021-05-03  4:54   ` David Gibson
2021-05-03 20:15     ` Bruno Piazera Larsen
2021-05-04  0:28       ` David Gibson
2021-04-30 19:35 ` [PATCH v3 3/7] target/ppc: remove unnecessary SPR functions Bruno Larsen (billionai)
2021-05-01  0:58   ` Richard Henderson
2021-05-03  4:39     ` David Gibson
2021-04-30 19:35 ` [PATCH v3 4/7] target/ppc: turned SPR R/W callbacks not static Bruno Larsen (billionai)
2021-05-01  1:05   ` Richard Henderson
2021-05-03 11:28     ` Bruno Piazera Larsen
2021-05-03 15:25       ` Richard Henderson
2021-05-03 20:08         ` Bruno Piazera Larsen
2021-04-30 19:35 ` [PATCH v3 5/7] target/ppc: removed VSCR from SPR registration Bruno Larsen (billionai)
2021-05-01  1:06   ` Richard Henderson
2021-05-03  4:47   ` David Gibson
2021-04-30 19:35 ` [PATCH v3 6/7] target/ppc: renamed SPR registration functions Bruno Larsen (billionai)
2021-05-03  4:49   ` David Gibson
2021-04-30 19:35 ` [PATCH v3 7/7] target/ppc: isolated cpu init from translation logic Bruno Larsen (billionai)
2021-05-01  1:11   ` Richard Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cb40e011-cf22-1044-43b0-75401d074796@linaro.org \
    --to=richard.henderson@linaro.org \
    --cc=bruno.larsen@eldorado.org.br \
    --cc=david@gibson.dropbear.id.au \
    --cc=farosas@linux.ibm.com \
    --cc=fernando.valle@eldorado.org.br \
    --cc=lucas.araujo@eldorado.org.br \
    --cc=luis.pires@eldorado.org.br \
    --cc=matheus.ferst@eldorado.org.br \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.