On 14.03.2016 17:56, Cédric Le Goater wrote: > From: Benjamin Herrenschmidt <b...@kernel.crashing.org> > > The current set of spr_register_* macros only take the user and > supervisor function pointers. To make the transition easy, we > don't change that but we add "_hv" variants that can be used to > register all 3 sets. > > To simplify the transition, users of the "old" macro will set the > hypervisor callback to be the same as the supervisor one. The new > registration function only needs to be used for registers that are > either hypervisor only or behave differently in HV mode. > > Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org> > Reviewed-by: David Gibson <da...@gibson.dropbear.id.au> > --- > target-ppc/translate.c | 26 ++++++++++++++++---------- > target-ppc/translate_init.c | 35 +++++++++++++++++++++++++++++++---- > 2 files changed, 47 insertions(+), 14 deletions(-) > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > index e402ff920314..327f3259b4be 100644 > --- a/target-ppc/translate.c > +++ b/target-ppc/translate.c > @@ -4282,14 +4282,17 @@ static inline void gen_op_mfspr(DisasContext *ctx) > void (*read_cb)(DisasContext *ctx, int gprn, int sprn); > uint32_t sprn = SPR(ctx->opcode); > > -#if !defined(CONFIG_USER_ONLY) > - if (ctx->hv) > +#if defined(CONFIG_USER_ONLY) > + read_cb = ctx->spr_cb[sprn].uea_read; > +#else > + if (ctx->pr) { > + read_cb = ctx->spr_cb[sprn].uea_read; > + } else if (ctx->hv) { > read_cb = ctx->spr_cb[sprn].hea_read; > - else if (!ctx->pr) > + } else if (!ctx->pr) {
That check for !ctx->pr is now superfluous, isn't it? ... because it has already been checked 4 lines earlier. > read_cb = ctx->spr_cb[sprn].oea_read; > - else > + } > #endif > - read_cb = ctx->spr_cb[sprn].uea_read; > if (likely(read_cb != NULL)) { > if (likely(read_cb != SPR_NOACCESS)) { > (*read_cb)(ctx, rD(ctx->opcode), sprn); > @@ -4437,14 +4440,17 @@ static void gen_mtspr(DisasContext *ctx) > void (*write_cb)(DisasContext *ctx, int sprn, int gprn); > uint32_t sprn = SPR(ctx->opcode); > > -#if !defined(CONFIG_USER_ONLY) > - if (ctx->hv) > +#if defined(CONFIG_USER_ONLY) > + write_cb = ctx->spr_cb[sprn].uea_write; > +#else > + if (ctx->pr) { > + write_cb = ctx->spr_cb[sprn].uea_write; > + } else if (ctx->hv) { > write_cb = ctx->spr_cb[sprn].hea_write; > - else if (!ctx->pr) > + } else { Here it is right already :-) > write_cb = ctx->spr_cb[sprn].oea_write; > - else > + } > #endif > - write_cb = ctx->spr_cb[sprn].uea_write; > if (likely(write_cb != NULL)) { > if (likely(write_cb != SPR_NOACCESS)) { > (*write_cb)(ctx, sprn, rS(ctx->opcode)); > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c > index fb206aff29ad..6a11b41206e5 100644 > --- a/target-ppc/translate_init.c > +++ b/target-ppc/translate_init.c > @@ -579,17 +579,33 @@ static inline void vscr_init (CPUPPCState *env, > uint32_t val) > #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) > #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) \ > + 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, initial_value) > + oea_read, oea_write, hea_read, hea_write, initial_value) > #else > #define spr_register_kvm(env, num, name, uea_read, uea_write, > \ > - oea_read, oea_write, one_reg_id, initial_value) \ > + 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, one_reg_id, initial_value) > + oea_read, oea_write, hea_read, hea_write, > \ > + one_reg_id, initial_value) > #endif > #endif > > @@ -598,6 +614,13 @@ static inline void vscr_init (CPUPPCState *env, uint32_t > val) > spr_register_kvm(env, num, name, uea_read, uea_write, > \ > oea_read, oea_write, 0, initial_value) > > +#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), > @@ -606,6 +629,8 @@ static inline void _spr_register(CPUPPCState *env, int > num, > > 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, > @@ -633,6 +658,8 @@ static inline void _spr_register(CPUPPCState *env, int > num, > #if !defined(CONFIG_USER_ONLY) > spr->oea_read = oea_read; > spr->oea_write = oea_write; > + spr->hea_read = hea_read; > + spr->hea_write = hea_write; > #endif > #if defined(CONFIG_KVM) > spr->one_reg_id = one_reg_id, Apart from the one superfluous if-statement, the patch looks fine to me. Thomas