[v2,06/12] target/ppc: Add PPR32 SPR

PPR32 provides access to the upper half of PPR.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/cpu.h        |  1 +
 target/ppc/spr_common.h |  2 ++
 target/ppc/cpu_init.c   | 12 ++++++++++++
 target/ppc/translate.c  | 16 ++++++++++++++++
 4 files changed, 31 insertions(+)

Reviewed-by: Glenn Miles <milesg@linux.ibm.com>

Thanks,

Glenn

On Tue, 2024-05-21 at 11:30 +1000, Nicholas Piggin wrote:
> PPR32 provides access to the upper half of PPR.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  target/ppc/cpu.h        |  1 +
>  target/ppc/spr_common.h |  2 ++
>  target/ppc/cpu_init.c   | 12 ++++++++++++
>  target/ppc/translate.c  | 16 ++++++++++++++++
>  4 files changed, 31 insertions(+)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 2532408be0..141cbefb4c 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -2120,6 +2120,7 @@ void ppc_compat_add_property(Object *obj, const
> char *name,
>  #define SPR_POWER_MMCRS       (0x37E)
>  #define SPR_WORT              (0x37F)
>  #define SPR_PPR               (0x380)
> +#define SPR_PPR32             (0x382)
>  #define SPR_750_GQR0          (0x390)
>  #define SPR_440_DNV0          (0x390)
>  #define SPR_750_GQR1          (0x391)
> diff --git a/target/ppc/spr_common.h b/target/ppc/spr_common.h
> index eb2561f593..9e40b3b608 100644
> --- a/target/ppc/spr_common.h
> +++ b/target/ppc/spr_common.h
> @@ -203,6 +203,8 @@ void spr_read_tfmr(DisasContext *ctx, int gprn,
> int sprn);
>  void spr_write_tfmr(DisasContext *ctx, int sprn, int gprn);
>  void spr_write_lpcr(DisasContext *ctx, int sprn, int gprn);
>  void spr_read_dexcr_ureg(DisasContext *ctx, int gprn, int sprn);
> +void spr_read_ppr32(DisasContext *ctx, int sprn, int gprn);
> +void spr_write_ppr32(DisasContext *ctx, int sprn, int gprn);
>  #endif
>  
>  void register_low_BATs(CPUPPCState *env);
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 892fb6ce02..7684a59d75 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -5623,6 +5623,14 @@ static void register_HEIR64_spr(CPUPPCState
> *env)
>                   0x00000000);
>  }
>  
> +static void register_power7_common_sprs(CPUPPCState *env)
> +{
> +    spr_register(env, SPR_PPR32, "PPR32",
> +                 &spr_read_ppr32, &spr_write_ppr32,
> +                 &spr_read_ppr32, &spr_write_ppr32,
> +                 0x00000000);
> +}
> +
>  static void register_power8_tce_address_control_sprs(CPUPPCState
> *env)
>  {
>      spr_register_kvm(env, SPR_TAR, "TAR",
> @@ -6118,6 +6126,7 @@ static void init_proc_POWER7(CPUPPCState *env)
>      register_power6_common_sprs(env);
>      register_HEIR32_spr(env);
>      register_power6_dbg_sprs(env);
> +    register_power7_common_sprs(env);
>      register_power7_book4_sprs(env);
>  
>      /* env variables */
> @@ -6264,6 +6273,7 @@ static void init_proc_POWER8(CPUPPCState *env)
>      register_power6_common_sprs(env);
>      register_HEIR32_spr(env);
>      register_power6_dbg_sprs(env);
> +    register_power7_common_sprs(env);
>      register_power8_tce_address_control_sprs(env);
>      register_power8_ids_sprs(env);
>      register_power8_ebb_sprs(env);
> @@ -6431,6 +6441,7 @@ static void init_proc_POWER9(CPUPPCState *env)
>      register_power6_common_sprs(env);
>      register_HEIR32_spr(env);
>      register_power6_dbg_sprs(env);
> +    register_power7_common_sprs(env);
>      register_power8_tce_address_control_sprs(env);
>      register_power8_ids_sprs(env);
>      register_power8_ebb_sprs(env);
> @@ -6625,6 +6636,7 @@ static void init_proc_POWER10(CPUPPCState *env)
>      register_power6_common_sprs(env);
>      register_HEIR64_spr(env);
>      register_power6_dbg_sprs(env);
> +    register_power7_common_sprs(env);
>      register_power8_tce_address_control_sprs(env);
>      register_power8_ids_sprs(env);
>      register_power8_ebb_sprs(env);
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index ca4f4c9371..137370b649 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -1414,6 +1414,22 @@ void spr_read_dexcr_ureg(DisasContext *ctx,
> int gprn, int sprn)
>      gen_load_spr(t0, sprn + 16);
>      tcg_gen_ext32u_tl(cpu_gpr[gprn], t0);
>  }
> +
> +/* The PPR32 SPR accesses the upper 32-bits of PPR */
> +void spr_read_ppr32(DisasContext *ctx, int sprn, int gprn)
> +{
> +    gen_load_spr(cpu_gpr[gprn], SPR_PPR);
> +    tcg_gen_shri_tl(cpu_gpr[gprn], cpu_gpr[gprn], 32);
> +}
> +
> +void spr_write_ppr32(DisasContext *ctx, int sprn, int gprn)
> +{
> +    TCGv t0 = tcg_temp_new();
> +
> +    tcg_gen_shli_tl(t0, cpu_gpr[gprn], 32);
> +    gen_store_spr(SPR_PPR, t0);
> +    spr_store_dump_spr(SPR_PPR);
> +}
>  #endif
>  
>  #define GEN_HANDLER(name, opc1, opc2, opc3, inval,
> type)                      \

On 5/20/24 18:30, Nicholas Piggin wrote:
> +void spr_write_ppr32(DisasContext *ctx, int sprn, int gprn)
> +{
> +    TCGv t0 = tcg_temp_new();
> +
> +    tcg_gen_shli_tl(t0, cpu_gpr[gprn], 32);
> +    gen_store_spr(SPR_PPR, t0);
> +    spr_store_dump_spr(SPR_PPR);
> +}

The documentation isn't clear on whether this zaps the low 32 bits. If the low bits of PPR 
are {reserved, must-be-zero, undefined} or suchlike, this is fine.

If not, then you need a deposit here, to preserve those bits, e.g.:

     gen_load_spr(t0, SPR_PPR);
     tcg_gen_deposit_tl(t0, t0, cpu_gpr[gprn], 32, 32);
     gen_store_spr(SPR_PPR, t0);

Anyway, it might be best to add a comment here re the above.


r~

On Wed May 22, 2024 at 3:40 AM AEST, Richard Henderson wrote:
> On 5/20/24 18:30, Nicholas Piggin wrote:
> > +void spr_write_ppr32(DisasContext *ctx, int sprn, int gprn)
> > +{
> > +    TCGv t0 = tcg_temp_new();
> > +
> > +    tcg_gen_shli_tl(t0, cpu_gpr[gprn], 32);
> > +    gen_store_spr(SPR_PPR, t0);
> > +    spr_store_dump_spr(SPR_PPR);
> > +}
>
> The documentation isn't clear on whether this zaps the low 32 bits. If the low bits of PPR 
> are {reserved, must-be-zero, undefined} or suchlike, this is fine.
>
> If not, then you need a deposit here, to preserve those bits, e.g.:
>
>      gen_load_spr(t0, SPR_PPR);
>      tcg_gen_deposit_tl(t0, t0, cpu_gpr[gprn], 32, 32);
>      gen_store_spr(SPR_PPR, t0);
>
> Anyway, it might be best to add a comment here re the above.

Oh good catch. The other bits are reserved which means they can return 0
but it's not necessary. We implement all the bits though, so we should
not have mtPPR32 zeroing out the other half. In theory we probably can
since they're "undefined", but it doesn't seem nice. Actually now I look
the ISA says reserved bits in SPRs should return 0 for reads in
user-mode which we get wrong in a few places.

Anyway yes, for now I'll go with your deposit. Thank you.

Thanks,
Nick