Message ID | 20211222134520.587877-2-danielhb413@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Re-write PPC64 PMU instruction count using TCG Ops | expand |
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) > { >
On 12/22/21 15:00, Cédric Le Goater wrote: > 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 ? That was my first idea. But then I thought that this code would feel out of place in power8-pmu-regs.c.inc since, at this moment, the file only has SPR read/write implementations that are being declared by spr_tcg.h to be used by cpu_init.c. I guess it's a matter of taste. I'm ok putting this code in power8-pmu-regs.c.inc if you think it's better. Daniel > > 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) >> { >> >
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) {
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. 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