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

[David Gibson]

On Wed, Nov 11, 2015 at 11:27:19AM +1100, Benjamin Herrenschmidt wrote:
> 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 <address@hidden>

Reviewed-by: David Gibson <address@hidden>

> ---
>  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 e18d204..a2fe1b5 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -4299,14 +4299,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);
> @@ -4450,14 +4453,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 e88dc7f..30a03ce 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -578,17 +578,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
>  
> @@ -597,6 +613,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),
> @@ -605,6 +628,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,
> @@ -632,6 +657,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,

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

signature.asc
Description: PGP signature