* [PATCH 0/2] target/ppc: Clean up _spr_register @ 2021-05-01 2:29 Richard Henderson 2021-05-01 2:29 ` [PATCH 1/2] target/ppc: Clean up _spr_register et al Richard Henderson 2021-05-01 2:29 ` [PATCH 2/2] target/ppc: Reduce the size of ppc_spr_t Richard Henderson 0 siblings, 2 replies; 5+ messages in thread From: Richard Henderson @ 2021-05-01 2:29 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-ppc, luis.pires, matheus.ferst, bruno.larsen, david This is a fleshed out version of the pseudo-patch I posted in response to Bruno Larsen's "untangle CPU init" patch set, specifically to replace https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg06754.html Plus, ppc_spr_t can use some love to match. r~ PS, I generated this vs master, so it might not apply without conflict against Bruno's prerequisites. Richard Henderson (2): target/ppc: Clean up _spr_register et al target/ppc: Reduce the size of ppc_spr_t target/ppc/cpu.h | 12 ++- target/ppc/translate_init.c.inc | 154 +++++++++++++++----------------- 2 files changed, 82 insertions(+), 84 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] target/ppc: Clean up _spr_register et al 2021-05-01 2:29 [PATCH 0/2] target/ppc: Clean up _spr_register Richard Henderson @ 2021-05-01 2:29 ` Richard Henderson 2021-05-03 3:47 ` David Gibson 2021-05-01 2:29 ` [PATCH 2/2] target/ppc: Reduce the size of ppc_spr_t Richard Henderson 1 sibling, 1 reply; 5+ messages in thread From: Richard Henderson @ 2021-05-01 2:29 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-ppc, luis.pires, matheus.ferst, bruno.larsen, david 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> --- 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) { -- 2.25.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] target/ppc: Clean up _spr_register et al 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 0 siblings, 0 replies; 5+ messages in thread From: David Gibson @ 2021-05-03 3:47 UTC (permalink / raw) To: Richard Henderson Cc: qemu-ppc, luis.pires, matheus.ferst, qemu-devel, bruno.larsen [-- 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 --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] target/ppc: Reduce the size of ppc_spr_t 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-01 2:29 ` Richard Henderson 2021-05-03 3:48 ` David Gibson 1 sibling, 1 reply; 5+ messages in thread From: Richard Henderson @ 2021-05-01 2:29 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-ppc, luis.pires, matheus.ferst, bruno.larsen, david We elide values when registering sprs, we might as well save space in the array as well. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/ppc/cpu.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index e73416da68..b704261b49 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -192,17 +192,21 @@ typedef struct ppc_hash_pte64 ppc_hash_pte64_t; /* SPR access micro-ops generations callbacks */ struct ppc_spr_t { + const char *name; + target_ulong default_value; +#ifndef CONFIG_USER_ONLY + unsigned int gdb_id; +#endif +#ifdef CONFIG_TCG void (*uea_read)(DisasContext *ctx, int gpr_num, int spr_num); void (*uea_write)(DisasContext *ctx, int spr_num, int gpr_num); -#if !defined(CONFIG_USER_ONLY) +# ifndef CONFIG_USER_ONLY void (*oea_read)(DisasContext *ctx, int gpr_num, int spr_num); void (*oea_write)(DisasContext *ctx, int spr_num, int gpr_num); void (*hea_read)(DisasContext *ctx, int gpr_num, int spr_num); void (*hea_write)(DisasContext *ctx, int spr_num, int gpr_num); - unsigned int gdb_id; +# endif #endif - const char *name; - target_ulong default_value; #ifdef CONFIG_KVM /* * We (ab)use the fact that all the SPRs will have ids for the -- 2.25.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] target/ppc: Reduce the size of ppc_spr_t 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 0 siblings, 0 replies; 5+ messages in thread From: David Gibson @ 2021-05-03 3:48 UTC (permalink / raw) To: Richard Henderson Cc: qemu-ppc, luis.pires, matheus.ferst, qemu-devel, bruno.larsen [-- Attachment #1: Type: text/plain, Size: 1731 bytes --] On Fri, Apr 30, 2021 at 07:29:23PM -0700, Richard Henderson wrote: > We elide values when registering sprs, we might as well > save space in the array as well. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Applied to ppc-for-6.1, thanks. > --- > target/ppc/cpu.h | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > index e73416da68..b704261b49 100644 > --- a/target/ppc/cpu.h > +++ b/target/ppc/cpu.h > @@ -192,17 +192,21 @@ typedef struct ppc_hash_pte64 ppc_hash_pte64_t; > > /* SPR access micro-ops generations callbacks */ > struct ppc_spr_t { > + const char *name; > + target_ulong default_value; > +#ifndef CONFIG_USER_ONLY > + unsigned int gdb_id; > +#endif > +#ifdef CONFIG_TCG > void (*uea_read)(DisasContext *ctx, int gpr_num, int spr_num); > void (*uea_write)(DisasContext *ctx, int spr_num, int gpr_num); > -#if !defined(CONFIG_USER_ONLY) > +# ifndef CONFIG_USER_ONLY > void (*oea_read)(DisasContext *ctx, int gpr_num, int spr_num); > void (*oea_write)(DisasContext *ctx, int spr_num, int gpr_num); > void (*hea_read)(DisasContext *ctx, int gpr_num, int spr_num); > void (*hea_write)(DisasContext *ctx, int spr_num, int gpr_num); > - unsigned int gdb_id; > +# endif > #endif > - const char *name; > - target_ulong default_value; > #ifdef CONFIG_KVM > /* > * We (ab)use the fact that all the SPRs will have ids for the -- 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 --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-05-03 4:37 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).