Re: [Qemu-devel] [PATCH 02/17] ppc: Add macros to register hypervisor mode SPRs

[Thomas Huth]

On 14.03.2016 17:56, Cédric Le Goater wrote:
> From: Benjamin Herrenschmidt 
> 
> 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 
> Reviewed-by: David Gibson 
> ---
>  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, 

[Qemu-devel] [PATCH 02/17] ppc: Add macros to register hypervisor mode SPRs

[Cédric Le Goater]

From: Benjamin Herrenschmidt 

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 
Reviewed-by: David Gibson 
---
 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) {
 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 {
 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,