From: David Gibson <david@gibson.dropbear.id.au>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: qemu-ppc@nongnu.org, luis.pires@eldorado.org.br,
matheus.ferst@eldorado.org.br, qemu-devel@nongnu.org,
bruno.larsen@eldorado.org.br
Subject: Re: [PATCH 1/2] target/ppc: Clean up _spr_register et al
Date: Mon, 3 May 2021 13:47:44 +1000 [thread overview]
Message-ID: <YI9yYIqx0dSLTVlB@yekko> (raw)
In-Reply-To: <20210501022923.1179736-2-richard.henderson@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 9902 bytes --]
On Fri, Apr 30, 2021 at 07:29:22PM -0700, Richard Henderson wrote:
> Introduce 3 helper macros to elide arguments that we cannot supply.
> This reduces the repetition required to get the job done.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Applied to ppc-for-6.1, thanks.
> ---
> target/ppc/translate_init.c.inc | 154 +++++++++++++++-----------------
> 1 file changed, 74 insertions(+), 80 deletions(-)
>
> diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
> index c03a7c4f52..49a92b20b4 100644
> --- a/target/ppc/translate_init.c.inc
> +++ b/target/ppc/translate_init.c.inc
> @@ -721,104 +721,98 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val)
> helper_mtvscr(env, val);
> }
>
> -#ifdef CONFIG_USER_ONLY
> -#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)
> -#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)
> +/**
> + * _spr_register
> + *
> + * Register an SPR with all the callbacks required for tcg,
> + * and the ID number for KVM.
> + *
> + * The reason for the conditional compilation is that the tcg functions
> + * may be compiled out, and the system kvm header may not be available
> + * for supplying the ID numbers. This is ugly, but the best we can do.
> + */
> +
> +#ifdef CONFIG_TCG
> +# define USR_ARG(X) X,
> +# ifdef CONFIG_USER_ONLY
> +# define SYS_ARG(X)
> +# else
> +# define SYS_ARG(X) X,
> +# endif
> #else
> -#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)
> +# define USR_ARG(X)
> +# define SYS_ARG(X)
> +#endif
> +#ifdef CONFIG_KVM
> +# define KVM_ARG(X) X,
> #else
> -#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
> +# define KVM_ARG(X)
> #endif
>
> -#define spr_register(env, num, name, uea_read, uea_write, \
> - oea_read, oea_write, initial_value) \
> - spr_register_kvm(env, num, name, uea_read, uea_write, \
> - oea_read, oea_write, 0, initial_value)
> +typedef void spr_callback(DisasContext *, int, int);
>
> -#define spr_register_hv(env, num, name, uea_read, uea_write, \
> - oea_read, oea_write, hea_read, hea_write, \
> - initial_value) \
> - spr_register_kvm_hv(env, num, name, uea_read, uea_write, \
> - oea_read, oea_write, hea_read, hea_write, \
> - 0, initial_value)
> -
> -static inline void _spr_register(CPUPPCState *env, int num,
> - const char *name,
> - void (*uea_read)(DisasContext *ctx,
> - int gprn, int sprn),
> - void (*uea_write)(DisasContext *ctx,
> - int sprn, int gprn),
> -#if !defined(CONFIG_USER_ONLY)
> -
> - void (*oea_read)(DisasContext *ctx,
> - int gprn, int sprn),
> - void (*oea_write)(DisasContext *ctx,
> - int sprn, int gprn),
> - void (*hea_read)(DisasContext *opaque,
> - int gprn, int sprn),
> - void (*hea_write)(DisasContext *opaque,
> - int sprn, int gprn),
> -#endif
> -#if defined(CONFIG_KVM)
> - uint64_t one_reg_id,
> -#endif
> - target_ulong initial_value)
> +static void _spr_register(CPUPPCState *env, int num, const char *name,
> + USR_ARG(spr_callback *uea_read)
> + USR_ARG(spr_callback *uea_write)
> + SYS_ARG(spr_callback *oea_read)
> + SYS_ARG(spr_callback *oea_write)
> + SYS_ARG(spr_callback *hea_read)
> + SYS_ARG(spr_callback *hea_write)
> + KVM_ARG(uint64_t one_reg_id)
> + target_ulong initial_value)
> {
> - ppc_spr_t *spr;
> + ppc_spr_t *spr = &env->spr_cb[num];
> +
> + /* No SPR should be registered twice. */
> + assert(spr->name == NULL);
> + assert(name != NULL);
>
> - spr = &env->spr_cb[num];
> - if (spr->name != NULL || env->spr[num] != 0x00000000 ||
> -#if !defined(CONFIG_USER_ONLY)
> - spr->oea_read != NULL || spr->oea_write != NULL ||
> -#endif
> - spr->uea_read != NULL || spr->uea_write != NULL) {
> - printf("Error: Trying to register SPR %d (%03x) twice !\n", num, num);
> - exit(1);
> - }
> -#if defined(PPC_DEBUG_SPR)
> - printf("*** register spr %d (%03x) %s val " TARGET_FMT_lx "\n", num, num,
> - name, initial_value);
> -#endif
> spr->name = name;
> + spr->default_value = initial_value;
> + env->spr[num] = initial_value;
> +
> +#ifdef CONFIG_TCG
> spr->uea_read = uea_read;
> spr->uea_write = uea_write;
> -#if !defined(CONFIG_USER_ONLY)
> +# ifndef CONFIG_USER_ONLY
> spr->oea_read = oea_read;
> spr->oea_write = oea_write;
> spr->hea_read = hea_read;
> spr->hea_write = hea_write;
> +# endif
> #endif
> -#if defined(CONFIG_KVM)
> - spr->one_reg_id = one_reg_id,
> +#ifdef CONFIG_KVM
> + spr->one_reg_id = one_reg_id;
> #endif
> - env->spr[num] = spr->default_value = initial_value;
> }
>
> +/* spr_register_kvm_hv passes all required arguments. */
> +#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, \
> + USR_ARG(uea_read) USR_ARG(uea_write) \
> + SYS_ARG(oea_read) SYS_ARG(oea_write) \
> + SYS_ARG(hea_read) SYS_ARG(hea_write) \
> + KVM_ARG(one_reg_id) initial_value)
> +
> +/* spr_register_kvm duplicates the oea callbacks to the hea callbacks. */
> +#define spr_register_kvm(env, num, name, uea_read, uea_write, \
> + oea_read, oea_write, one_reg_id, ival) \
> + spr_register_kvm_hv(env, num, name, uea_read, uea_write, oea_read, \
> + oea_write, oea_read, oea_write, one_reg_id, ival)
> +
> +/* spr_register_hv and spr_register are similar, except there is no kvm id. */
> +#define spr_register_hv(env, num, name, uea_read, uea_write, \
> + oea_read, oea_write, hea_read, hea_write, ival) \
> + spr_register_kvm_hv(env, num, name, uea_read, uea_write, oea_read, \
> + oea_write, hea_read, hea_write, 0, ival)
> +
> +#define spr_register(env, num, name, uea_read, uea_write, \
> + oea_read, oea_write, ival) \
> + spr_register_kvm(env, num, name, uea_read, uea_write, \
> + oea_read, oea_write, 0, ival)
> +
> /* Generic PowerPC SPRs */
> static void gen_spr_generic(CPUPPCState *env)
> {
--
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 --]
next prev parent reply other threads:[~2021-05-03 4:37 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-01 2:29 [PATCH 0/2] target/ppc: Clean up _spr_register Richard Henderson
2021-05-01 2:29 ` [PATCH 1/2] target/ppc: Clean up _spr_register et al Richard Henderson
2021-05-03 3:47 ` David Gibson [this message]
2021-05-01 2:29 ` [PATCH 2/2] target/ppc: Reduce the size of ppc_spr_t Richard Henderson
2021-05-03 3:48 ` David Gibson
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=YI9yYIqx0dSLTVlB@yekko \
--to=david@gibson.dropbear.id.au \
--cc=bruno.larsen@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 \
--cc=richard.henderson@linaro.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.