Re: [PATCH 1/8] target/ppc: introduce power8-pmu-insn-cnt.c.inc

["Cédric Le Goater" <clg@kaod.org>]

On 12/22/21 14:45, Daniel Henrique Barboza wrote:
> We're going to add a significant amount of TCG ops code for
> instruction counting, eventually getting rid of the 'helper_insn_inc'
> helper entirely.
> 
> Create a new file to avoid putting even more stuff on the already
> crowded target/ppc/translate.c.

We already have a power8-pmu-regs.c.inc file. Couldn't we use it or is it
a bad idea ?

Thanks,

C.


> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>   target/ppc/power8-pmu-insn-cnt.c.inc | 54 ++++++++++++++++++++++++++++
>   target/ppc/translate.c               | 44 ++---------------------
>   2 files changed, 56 insertions(+), 42 deletions(-)
>   create mode 100644 target/ppc/power8-pmu-insn-cnt.c.inc
> 
> diff --git a/target/ppc/power8-pmu-insn-cnt.c.inc b/target/ppc/power8-pmu-insn-cnt.c.inc
> new file mode 100644
> index 0000000000..2febbcc27e
> --- /dev/null
> +++ b/target/ppc/power8-pmu-insn-cnt.c.inc
> @@ -0,0 +1,54 @@
> +/*
> + * PMU instruction counting for TCG IBM POWER chips
> + *
> + * Copyright IBM Corp. 2021
> + *
> + * Authors:
> + *  Daniel Henrique Barboza      <danielhb413@gmail.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#if defined(TARGET_PPC64)
> +static void pmu_count_insns(DisasContext *ctx)
> +{
> +    /*
> +     * Do not bother calling the helper if the PMU isn't counting
> +     * instructions.
> +     */
> +    if (!ctx->pmu_insn_cnt) {
> +        return;
> +    }
> +
> + #if !defined(CONFIG_USER_ONLY)
> +    /*
> +     * The PMU insns_inc() helper stops the internal PMU timer if a
> +     * counter overflows happens. In that case, if the guest is
> +     * running with icount and we do not handle it beforehand,
> +     * the helper can trigger a 'bad icount read'.
> +     */
> +    gen_icount_io_start(ctx);
> +
> +    gen_helper_insns_inc(cpu_env, tcg_constant_i32(ctx->base.num_insns));
> +#else
> +    /*
> +     * User mode can read (but not write) PMC5 and start/stop
> +     * the PMU via MMCR0_FC. In this case just increment
> +     * PMC5 with base.num_insns.
> +     */
> +    TCGv t0 = tcg_temp_new();
> +
> +    gen_load_spr(t0, SPR_POWER_PMC5);
> +    tcg_gen_addi_tl(t0, t0, ctx->base.num_insns);
> +    gen_store_spr(SPR_POWER_PMC5, t0);
> +
> +    tcg_temp_free(t0);
> +#endif /* #if !defined(CONFIG_USER_ONLY) */
> +}
> +#else
> +static void pmu_count_insns(DisasContext *ctx)
> +{
> +    return;
> +}
> +#endif /* #if defined(TARGET_PPC64) */
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 114456148c..44773bc6cd 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4183,48 +4183,8 @@ static inline void gen_update_cfar(DisasContext *ctx, target_ulong nip)
>   #endif
>   }
>   
> -#if defined(TARGET_PPC64)
> -static void pmu_count_insns(DisasContext *ctx)
> -{
> -    /*
> -     * Do not bother calling the helper if the PMU isn't counting
> -     * instructions.
> -     */
> -    if (!ctx->pmu_insn_cnt) {
> -        return;
> -    }
> -
> - #if !defined(CONFIG_USER_ONLY)
> -    /*
> -     * The PMU insns_inc() helper stops the internal PMU timer if a
> -     * counter overflows happens. In that case, if the guest is
> -     * running with icount and we do not handle it beforehand,
> -     * the helper can trigger a 'bad icount read'.
> -     */
> -    gen_icount_io_start(ctx);
> -
> -    gen_helper_insns_inc(cpu_env, tcg_constant_i32(ctx->base.num_insns));
> -#else
> -    /*
> -     * User mode can read (but not write) PMC5 and start/stop
> -     * the PMU via MMCR0_FC. In this case just increment
> -     * PMC5 with base.num_insns.
> -     */
> -    TCGv t0 = tcg_temp_new();
> -
> -    gen_load_spr(t0, SPR_POWER_PMC5);
> -    tcg_gen_addi_tl(t0, t0, ctx->base.num_insns);
> -    gen_store_spr(SPR_POWER_PMC5, t0);
> -
> -    tcg_temp_free(t0);
> -#endif /* #if !defined(CONFIG_USER_ONLY) */
> -}
> -#else
> -static void pmu_count_insns(DisasContext *ctx)
> -{
> -    return;
> -}
> -#endif /* #if defined(TARGET_PPC64) */
> +/* For pmu_count_insns */
> +#include "target/ppc/power8-pmu-insn-cnt.c.inc"
>   
>   static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest)
>   {
>