Re: [PATCH v8 04/10] target/ppc: PMU: update counters on MMCR1 write

[David Gibson]

On Thu, Nov 25, 2021 at 12:08:11PM -0300, Daniel Henrique Barboza wrote:
> MMCR1 determines the events to be sampled by the PMU. Updating the
> counters at every MMCR1 write ensures that we're not sampling more
> or less events by looking only at MMCR0 and the PMCs.
> 
> It is worth noticing that both the Book3S PowerPC PMU, and this IBM
> Power8+ PMU that we're modeling, also uses MMCRA, MMCR2 and MMCR3 to
> control the PMU. These three registers aren't being handled in this
> initial implementation, so for now we're controlling all the PMU
> aspects using MMCR0, MMCR1 and the PMCs.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  target/ppc/cpu_init.c            |  2 +-
>  target/ppc/helper.h              |  1 +
>  target/ppc/power8-pmu-regs.c.inc | 11 +++++++++++
>  target/ppc/power8-pmu.c          |  7 +++++++
>  target/ppc/spr_tcg.h             |  1 +
>  5 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index a7f47ec322..2d72dde26d 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -6825,7 +6825,7 @@ static void register_book3s_pmu_sup_sprs(CPUPPCState 
> *env)
>                       KVM_REG_PPC_MMCR0, 0x80000000);
>      spr_register_kvm(env, SPR_POWER_MMCR1, "MMCR1",
>                       SPR_NOACCESS, SPR_NOACCESS,
> -                     &spr_read_generic, &spr_write_generic,
> +                     &spr_read_generic, &spr_write_MMCR1,
>                       KVM_REG_PPC_MMCR1, 0x00000000);
>      spr_register_kvm(env, SPR_POWER_MMCRA, "MMCRA",
>                       SPR_NOACCESS, SPR_NOACCESS,
> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> index d7567f75b4..94b4690375 100644
> --- a/target/ppc/helper.h
> +++ b/target/ppc/helper.h
> @@ -21,6 +21,7 @@ DEF_HELPER_1(hrfid, void, env)
>  DEF_HELPER_2(store_lpcr, void, env, tl)
>  DEF_HELPER_2(store_pcr, void, env, tl)
>  DEF_HELPER_2(store_mmcr0, void, env, tl)
> +DEF_HELPER_2(store_mmcr1, void, env, tl)
>  DEF_HELPER_3(store_pmc, void, env, i32, i64)
>  DEF_HELPER_2(read_pmc, tl, env, i32)
>  #endif
> diff --git a/target/ppc/power8-pmu-regs.c.inc 
> b/target/ppc/power8-pmu-regs.c.inc
> index f0c9cc343b..25b13ad564 100644
> --- a/target/ppc/power8-pmu-regs.c.inc
> +++ b/target/ppc/power8-pmu-regs.c.inc
> @@ -255,6 +255,12 @@ void spr_write_MMCR0(DisasContext *ctx, int sprn, int 
> gprn)
>  {
>      write_MMCR0_common(ctx, cpu_gpr[gprn]);
>  }
> +
> +void spr_write_MMCR1(DisasContext *ctx, int sprn, int gprn)
> +{
> +    gen_icount_io_start(ctx);
> +    gen_helper_store_mmcr1(cpu_env, cpu_gpr[gprn]);
> +}
>  #else
>  void spr_read_MMCR0_ureg(DisasContext *ctx, int gprn, int sprn)
>  {
> @@ -301,6 +307,11 @@ void spr_write_MMCR0(DisasContext *ctx, int sprn, int 
> gprn)
>      spr_write_generic(ctx, sprn, gprn);
>  }
>  
> +void spr_write_MMCR1(DisasContext *ctx, int sprn, int gprn)
> +{
> +    spr_write_generic(ctx, sprn, gprn);
> +}
> +
>  void spr_write_PMC(DisasContext *ctx, int sprn, int gprn)
>  {
>      spr_write_generic(ctx, sprn, gprn);
> diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c
> index 5f2623aa25..acdaee7459 100644
> --- a/target/ppc/power8-pmu.c
> +++ b/target/ppc/power8-pmu.c
> @@ -145,6 +145,13 @@ void helper_store_mmcr0(CPUPPCState *env, target_ulong 
> value)
>      }
>  }
>  
> +void helper_store_mmcr1(CPUPPCState *env, uint64_t value)
> +{
> +    pmu_update_cycles(env);
> +
> +    env->spr[SPR_POWER_MMCR1] = value;
> +}
> +
>  target_ulong helper_read_pmc(CPUPPCState *env, uint32_t sprn)
>  {
>      pmu_update_cycles(env);
> diff --git a/target/ppc/spr_tcg.h b/target/ppc/spr_tcg.h
> index 1e79a0522a..1d6521eedc 100644
> --- a/target/ppc/spr_tcg.h
> +++ b/target/ppc/spr_tcg.h
> @@ -26,6 +26,7 @@ void spr_noaccess(DisasContext *ctx, int gprn, int sprn);
>  void spr_read_generic(DisasContext *ctx, int gprn, int sprn);
>  void spr_write_generic(DisasContext *ctx, int sprn, int gprn);
>  void spr_write_MMCR0(DisasContext *ctx, int sprn, int gprn);
> +void spr_write_MMCR1(DisasContext *ctx, int sprn, int gprn);
>  void spr_write_PMC(DisasContext *ctx, int sprn, int gprn);
>  void spr_read_xer(DisasContext *ctx, int gprn, int sprn);
>  void spr_write_xer(DisasContext *ctx, int sprn, int gprn);

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