diff mbox series

[v2,04/16] target/ppc: PMU basic cycle count for pseries TCG

Message ID 20210824163032.394099-5-danielhb413@gmail.com
State New
Headers show
Series PMU-EBB support for PPC64 TCG | expand

Commit Message

Daniel Henrique Barboza Aug. 24, 2021, 4:30 p.m. UTC
This patch adds the barebones of the PMU logic by enabling cycle
counting, done via the performance monitor counter 6. The overall logic
goes as follows:

- a helper is added to control the PMU state on each MMCR0 write. This
allows for the PMU to start/stop as the frozen counter bit (MMCR0_FC)
is cleared or set;

- MMCR0 reg initial value is set to 0x80000000 (MMCR0_FC set) to avoid
having to spin the PMU right at system init;

- the intended usage is to freeze the counters by setting MMCR0_FC, do
any additional setting of events to be counted via MMCR1 (not
implemented yet) and enable the PMU by zeroing MMCR0_FC. Software must
freeze counters to read the results - on the fly reading of the PMCs
will return the starting value of each one.

Since there will be more PMU exclusive code to be added next, put the
PMU logic in its own helper to keep all in the same place. The name of
the new helper file, power8_pmu.c, is an indicative that the PMU logic
has been tested with the IBM POWER chip family, POWER8 being the oldest
version tested. This doesn't mean that this PMU logic will break with
any other PPC64 chip that implements Book3s, but since we can't assert
that this PMU will work with all available Book3s emulated processors
we're choosing to be explicit.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 target/ppc/cpu.h        |  6 ++++
 target/ppc/cpu_init.c   |  6 ++--
 target/ppc/helper.h     |  1 +
 target/ppc/meson.build  |  1 +
 target/ppc/power8_pmu.c | 74 +++++++++++++++++++++++++++++++++++++++++
 target/ppc/spr_tcg.h    |  1 +
 target/ppc/translate.c  | 17 +++++++++-
 7 files changed, 102 insertions(+), 4 deletions(-)
 create mode 100644 target/ppc/power8_pmu.c

Comments

David Gibson Aug. 25, 2021, 5:19 a.m. UTC | #1
On Tue, Aug 24, 2021 at 01:30:20PM -0300, Daniel Henrique Barboza wrote:
> This patch adds the barebones of the PMU logic by enabling cycle
> counting, done via the performance monitor counter 6. The overall logic
> goes as follows:
> 
> - a helper is added to control the PMU state on each MMCR0 write. This
> allows for the PMU to start/stop as the frozen counter bit (MMCR0_FC)
> is cleared or set;
> 
> - MMCR0 reg initial value is set to 0x80000000 (MMCR0_FC set) to avoid
> having to spin the PMU right at system init;
> 
> - the intended usage is to freeze the counters by setting MMCR0_FC, do
> any additional setting of events to be counted via MMCR1 (not
> implemented yet) and enable the PMU by zeroing MMCR0_FC. Software must
> freeze counters to read the results - on the fly reading of the PMCs
> will return the starting value of each one.

Ok, I like how this is simpler than the previous version.  Since qemu
is not a cycle-accurate simulator, we basically have a choice in
emulating the PMU:
   1) we can maintain the illusion that the cpu clock goes at the
      advertised speed w.r.t. real time
or 2) we can maintain the illusion that instructions complete roughly
      as fast as we expect w.r.t. the cpu clock

We can't do both at the same time.  Well... in theory we kind of could
(on a time averaged basis at least) if we decouple the guest's notion
of "real time" from actual real time.  But that introduces a bunch of
other complications, so I don't think we want to go there.

Since it's simpler, I think (1) is probably the way to go, which is
what you've done here.

> Since there will be more PMU exclusive code to be added next, put the
> PMU logic in its own helper to keep all in the same place. The name of
> the new helper file, power8_pmu.c, is an indicative that the PMU logic
> has been tested with the IBM POWER chip family, POWER8 being the oldest
> version tested. This doesn't mean that this PMU logic will break with
> any other PPC64 chip that implements Book3s, but since we can't assert
> that this PMU will work with all available Book3s emulated processors
> we're choosing to be explicit.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  target/ppc/cpu.h        |  6 ++++
>  target/ppc/cpu_init.c   |  6 ++--
>  target/ppc/helper.h     |  1 +
>  target/ppc/meson.build  |  1 +
>  target/ppc/power8_pmu.c | 74 +++++++++++++++++++++++++++++++++++++++++
>  target/ppc/spr_tcg.h    |  1 +
>  target/ppc/translate.c  | 17 +++++++++-
>  7 files changed, 102 insertions(+), 4 deletions(-)
>  create mode 100644 target/ppc/power8_pmu.c
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 739005ba29..6878d950de 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1176,6 +1176,12 @@ struct CPUPPCState {
>      uint32_t tm_vscr;
>      uint64_t tm_dscr;
>      uint64_t tm_tar;
> +
> +    /*
> +     * PMU base time value used by the PMU to calculate
> +     * running cycles.
> +     */
> +    uint64_t pmu_base_time;
>  };
>  
>  #define SET_FIT_PERIOD(a_, b_, c_, d_)          \
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 860716da18..71f052b052 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -6821,8 +6821,8 @@ static void register_book3s_pmu_sup_sprs(CPUPPCState *env)
>  {
>      spr_register_kvm(env, SPR_POWER_MMCR0, "MMCR0",
>                       SPR_NOACCESS, SPR_NOACCESS,
> -                     &spr_read_generic, &spr_write_generic,
> -                     KVM_REG_PPC_MMCR0, 0x00000000);
> +                     &spr_read_generic, &spr_write_MMCR0_generic,

s/spr_write_MMCR0_generic/spr_write_MMCR0/

The generic refers to the fact that it's covering any register which
will just read back whatever value is written to it.  Now that you're
applying MMCR0 specific logic, it's not generic any more.

> +                     KVM_REG_PPC_MMCR0, 0x80000000);
>      spr_register_kvm(env, SPR_POWER_MMCR1, "MMCR1",
>                       SPR_NOACCESS, SPR_NOACCESS,
>                       &spr_read_generic, &spr_write_generic,
> @@ -6870,7 +6870,7 @@ static void register_book3s_pmu_user_sprs(CPUPPCState *env)
>      spr_register(env, SPR_POWER_UMMCR0, "UMMCR0",
>                   &spr_read_MMCR0_ureg, &spr_write_MMCR0_ureg,
>                   &spr_read_ureg, &spr_write_ureg,
> -                 0x00000000);
> +                 0x80000000);
>      spr_register(env, SPR_POWER_UMMCR1, "UMMCR1",
>                   &spr_read_ureg, SPR_NOACCESS,
>                   &spr_read_ureg, &spr_write_ureg,
> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> index 4076aa281e..5122632784 100644
> --- a/target/ppc/helper.h
> +++ b/target/ppc/helper.h
> @@ -20,6 +20,7 @@ DEF_HELPER_1(rfscv, void, env)
>  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)
>  #endif
>  DEF_HELPER_1(check_tlb_flush_local, void, env)
>  DEF_HELPER_1(check_tlb_flush_global, void, env)
> diff --git a/target/ppc/meson.build b/target/ppc/meson.build
> index b85f295703..278ce07da9 100644
> --- a/target/ppc/meson.build
> +++ b/target/ppc/meson.build
> @@ -14,6 +14,7 @@ ppc_ss.add(when: 'CONFIG_TCG', if_true: files(
>    'int_helper.c',
>    'mem_helper.c',
>    'misc_helper.c',
> +  'power8_pmu.c',
>    'timebase_helper.c',
>    'translate.c',
>  ))
> diff --git a/target/ppc/power8_pmu.c b/target/ppc/power8_pmu.c
> new file mode 100644
> index 0000000000..47de38a99e
> --- /dev/null
> +++ b/target/ppc/power8_pmu.c
> @@ -0,0 +1,74 @@
> +/*
> + * PMU emulation helpers 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.
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "cpu.h"
> +#include "helper_regs.h"
> +#include "exec/exec-all.h"
> +#include "exec/helper-proto.h"
> +#include "qemu/error-report.h"
> +#include "qemu/main-loop.h"
> +
> +#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
> +
> +static void update_PMC_PM_CYC(CPUPPCState *env, int sprn,
> +                              uint64_t time_delta)
> +{
> +    /*
> +     * The pseries and pvn clock runs at 1Ghz, meaning that
> +     * 1 nanosec equals 1 cycle.
> +     */
> +    env->spr[sprn] += time_delta;
> +}
> +
> +static void update_cycles_PMCs(CPUPPCState *env)
> +{
> +    uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +    uint64_t time_delta = now - env->pmu_base_time;
> +
> +    update_PMC_PM_CYC(env, SPR_POWER_PMC6, time_delta);
> +}
> +
> +void helper_store_mmcr0(CPUPPCState *env, target_ulong value)
> +{
> +    target_ulong curr_value = env->spr[SPR_POWER_MMCR0];
> +    bool curr_FC = curr_value & MMCR0_FC;
> +    bool new_FC = value & MMCR0_FC;
> +
> +    env->spr[SPR_POWER_MMCR0] = value;
> +
> +    /* MMCR0 writes can change HFLAGS_PMCCCLEAR */
> +    if ((curr_value & MMCR0_PMCC) != (value & MMCR0_PMCC)) {
> +        hreg_compute_hflags(env);
> +    }
> +
> +    /*
> +     * In an frozen count (FC) bit change:
> +     *
> +     * - if PMCs were running (curr_FC = false) and we're freezing
> +     * them (new_FC = true), save the PMCs values in the registers.
> +     *
> +     * - if PMCs were frozen (curr_FC = true) and we're activating
> +     * them (new_FC = false), set the new base_time for future cycle
> +     * calculations.
> +     */
> +    if (curr_FC != new_FC) {
> +        if (!curr_FC) {
> +            update_cycles_PMCs(env);
> +        } else {
> +            env->pmu_base_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +        }
> +    }
> +}
> +
> +#endif /* defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY) */
> diff --git a/target/ppc/spr_tcg.h b/target/ppc/spr_tcg.h
> index 5c383dae3d..2c5b056fc1 100644
> --- a/target/ppc/spr_tcg.h
> +++ b/target/ppc/spr_tcg.h
> @@ -25,6 +25,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_generic(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);
>  void spr_read_lr(DisasContext *ctx, int gprn, int sprn);
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index b48eec83e3..e4f75ba380 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -401,6 +401,19 @@ void spr_write_generic(DisasContext *ctx, int sprn, int gprn)
>      spr_store_dump_spr(sprn);
>  }
>  
> +#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
> +void spr_write_MMCR0_generic(DisasContext *ctx, int sprn, int gprn)
> +{
> +    gen_icount_io_start(ctx);

Since you're not using icount any more, do you still need these?

> +    gen_helper_store_mmcr0(cpu_env, cpu_gpr[gprn]);
> +}
> +#else
> +void spr_write_MMCR0_generic(DisasContext *ctx, int sprn, int gprn)
> +{
> +    spr_write_generic(ctx, sprn, gprn);
> +}
> +#endif
> +
>  #if !defined(CONFIG_USER_ONLY)
>  void spr_write_generic32(DisasContext *ctx, int sprn, int gprn)
>  {
> @@ -609,6 +622,8 @@ void spr_write_MMCR0_ureg(DisasContext *ctx, int sprn, int gprn)
>      t0 = tcg_temp_new();
>      t1 = tcg_temp_new();
>  
> +    gen_icount_io_start(ctx);
> +
>      /*
>       * Filter out all bits but FC, PMAO, and PMAE, according
>       * to ISA v3.1, in 10.4.4 Monitor Mode Control Register 0,
> @@ -620,7 +635,7 @@ void spr_write_MMCR0_ureg(DisasContext *ctx, int sprn, int gprn)
>      tcg_gen_andi_tl(t1, t1, ~(MMCR0_FC | MMCR0_PMAO | MMCR0_PMAE));
>      /* Keep all other bits intact */
>      tcg_gen_or_tl(t1, t1, t0);
> -    gen_store_spr(SPR_POWER_MMCR0, t1);
> +    gen_helper_store_mmcr0(cpu_env, t1);

Do you need this change?  Won't the gen_store_spr() basically do the
same thing as the gen_helpersince spr_write_MMCR0() expands to a
gen_helper anyway?

>  
>      tcg_temp_free(t0);
>      tcg_temp_free(t1);
Daniel Henrique Barboza Aug. 25, 2021, 2:05 p.m. UTC | #2
On 8/25/21 2:19 AM, David Gibson wrote:
> On Tue, Aug 24, 2021 at 01:30:20PM -0300, Daniel Henrique Barboza wrote:
>> This patch adds the barebones of the PMU logic by enabling cycle
>> counting, done via the performance monitor counter 6. The overall logic
>> goes as follows:
>>
>> - a helper is added to control the PMU state on each MMCR0 write. This
>> allows for the PMU to start/stop as the frozen counter bit (MMCR0_FC)
>> is cleared or set;
>>
>> - MMCR0 reg initial value is set to 0x80000000 (MMCR0_FC set) to avoid
>> having to spin the PMU right at system init;
>>
>> - the intended usage is to freeze the counters by setting MMCR0_FC, do
>> any additional setting of events to be counted via MMCR1 (not
>> implemented yet) and enable the PMU by zeroing MMCR0_FC. Software must
>> freeze counters to read the results - on the fly reading of the PMCs
>> will return the starting value of each one.
> 
> Ok, I like how this is simpler than the previous version.  Since qemu
> is not a cycle-accurate simulator, we basically have a choice in
> emulating the PMU:
>     1) we can maintain the illusion that the cpu clock goes at the
>        advertised speed w.r.t. real time
> or 2) we can maintain the illusion that instructions complete roughly
>        as fast as we expect w.r.t. the cpu clock
> 
> We can't do both at the same time.  Well... in theory we kind of could
> (on a time averaged basis at least) if we decouple the guest's notion
> of "real time" from actual real time.  But that introduces a bunch of
> other complications, so I don't think we want to go there.
> 
> Since it's simpler, I think (1) is probably the way to go, which is
> what you've done here.
> 
>> Since there will be more PMU exclusive code to be added next, put the
>> PMU logic in its own helper to keep all in the same place. The name of
>> the new helper file, power8_pmu.c, is an indicative that the PMU logic
>> has been tested with the IBM POWER chip family, POWER8 being the oldest
>> version tested. This doesn't mean that this PMU logic will break with
>> any other PPC64 chip that implements Book3s, but since we can't assert
>> that this PMU will work with all available Book3s emulated processors
>> we're choosing to be explicit.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>>   target/ppc/cpu.h        |  6 ++++
>>   target/ppc/cpu_init.c   |  6 ++--
>>   target/ppc/helper.h     |  1 +
>>   target/ppc/meson.build  |  1 +
>>   target/ppc/power8_pmu.c | 74 +++++++++++++++++++++++++++++++++++++++++
>>   target/ppc/spr_tcg.h    |  1 +
>>   target/ppc/translate.c  | 17 +++++++++-
>>   7 files changed, 102 insertions(+), 4 deletions(-)
>>   create mode 100644 target/ppc/power8_pmu.c
>>
>> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
>> index 739005ba29..6878d950de 100644
>> --- a/target/ppc/cpu.h
>> +++ b/target/ppc/cpu.h
>> @@ -1176,6 +1176,12 @@ struct CPUPPCState {
>>       uint32_t tm_vscr;
>>       uint64_t tm_dscr;
>>       uint64_t tm_tar;
>> +
>> +    /*
>> +     * PMU base time value used by the PMU to calculate
>> +     * running cycles.
>> +     */
>> +    uint64_t pmu_base_time;
>>   };
>>   
>>   #define SET_FIT_PERIOD(a_, b_, c_, d_)          \
>> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
>> index 860716da18..71f052b052 100644
>> --- a/target/ppc/cpu_init.c
>> +++ b/target/ppc/cpu_init.c
>> @@ -6821,8 +6821,8 @@ static void register_book3s_pmu_sup_sprs(CPUPPCState *env)
>>   {
>>       spr_register_kvm(env, SPR_POWER_MMCR0, "MMCR0",
>>                        SPR_NOACCESS, SPR_NOACCESS,
>> -                     &spr_read_generic, &spr_write_generic,
>> -                     KVM_REG_PPC_MMCR0, 0x00000000);
>> +                     &spr_read_generic, &spr_write_MMCR0_generic,
> 
> s/spr_write_MMCR0_generic/spr_write_MMCR0/
> 
> The generic refers to the fact that it's covering any register which
> will just read back whatever value is written to it.  Now that you're
> applying MMCR0 specific logic, it's not generic any more.
> 
>> +                     KVM_REG_PPC_MMCR0, 0x80000000);
>>       spr_register_kvm(env, SPR_POWER_MMCR1, "MMCR1",
>>                        SPR_NOACCESS, SPR_NOACCESS,
>>                        &spr_read_generic, &spr_write_generic,
>> @@ -6870,7 +6870,7 @@ static void register_book3s_pmu_user_sprs(CPUPPCState *env)
>>       spr_register(env, SPR_POWER_UMMCR0, "UMMCR0",
>>                    &spr_read_MMCR0_ureg, &spr_write_MMCR0_ureg,
>>                    &spr_read_ureg, &spr_write_ureg,
>> -                 0x00000000);
>> +                 0x80000000);
>>       spr_register(env, SPR_POWER_UMMCR1, "UMMCR1",
>>                    &spr_read_ureg, SPR_NOACCESS,
>>                    &spr_read_ureg, &spr_write_ureg,
>> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
>> index 4076aa281e..5122632784 100644
>> --- a/target/ppc/helper.h
>> +++ b/target/ppc/helper.h
>> @@ -20,6 +20,7 @@ DEF_HELPER_1(rfscv, void, env)
>>   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)
>>   #endif
>>   DEF_HELPER_1(check_tlb_flush_local, void, env)
>>   DEF_HELPER_1(check_tlb_flush_global, void, env)
>> diff --git a/target/ppc/meson.build b/target/ppc/meson.build
>> index b85f295703..278ce07da9 100644
>> --- a/target/ppc/meson.build
>> +++ b/target/ppc/meson.build
>> @@ -14,6 +14,7 @@ ppc_ss.add(when: 'CONFIG_TCG', if_true: files(
>>     'int_helper.c',
>>     'mem_helper.c',
>>     'misc_helper.c',
>> +  'power8_pmu.c',
>>     'timebase_helper.c',
>>     'translate.c',
>>   ))
>> diff --git a/target/ppc/power8_pmu.c b/target/ppc/power8_pmu.c
>> new file mode 100644
>> index 0000000000..47de38a99e
>> --- /dev/null
>> +++ b/target/ppc/power8_pmu.c
>> @@ -0,0 +1,74 @@
>> +/*
>> + * PMU emulation helpers 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.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +
>> +#include "cpu.h"
>> +#include "helper_regs.h"
>> +#include "exec/exec-all.h"
>> +#include "exec/helper-proto.h"
>> +#include "qemu/error-report.h"
>> +#include "qemu/main-loop.h"
>> +
>> +#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
>> +
>> +static void update_PMC_PM_CYC(CPUPPCState *env, int sprn,
>> +                              uint64_t time_delta)
>> +{
>> +    /*
>> +     * The pseries and pvn clock runs at 1Ghz, meaning that
>> +     * 1 nanosec equals 1 cycle.
>> +     */
>> +    env->spr[sprn] += time_delta;
>> +}
>> +
>> +static void update_cycles_PMCs(CPUPPCState *env)
>> +{
>> +    uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>> +    uint64_t time_delta = now - env->pmu_base_time;
>> +
>> +    update_PMC_PM_CYC(env, SPR_POWER_PMC6, time_delta);
>> +}
>> +
>> +void helper_store_mmcr0(CPUPPCState *env, target_ulong value)
>> +{
>> +    target_ulong curr_value = env->spr[SPR_POWER_MMCR0];
>> +    bool curr_FC = curr_value & MMCR0_FC;
>> +    bool new_FC = value & MMCR0_FC;
>> +
>> +    env->spr[SPR_POWER_MMCR0] = value;
>> +
>> +    /* MMCR0 writes can change HFLAGS_PMCCCLEAR */
>> +    if ((curr_value & MMCR0_PMCC) != (value & MMCR0_PMCC)) {
>> +        hreg_compute_hflags(env);
>> +    }
>> +
>> +    /*
>> +     * In an frozen count (FC) bit change:
>> +     *
>> +     * - if PMCs were running (curr_FC = false) and we're freezing
>> +     * them (new_FC = true), save the PMCs values in the registers.
>> +     *
>> +     * - if PMCs were frozen (curr_FC = true) and we're activating
>> +     * them (new_FC = false), set the new base_time for future cycle
>> +     * calculations.
>> +     */
>> +    if (curr_FC != new_FC) {
>> +        if (!curr_FC) {
>> +            update_cycles_PMCs(env);
>> +        } else {
>> +            env->pmu_base_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>> +        }
>> +    }
>> +}
>> +
>> +#endif /* defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY) */
>> diff --git a/target/ppc/spr_tcg.h b/target/ppc/spr_tcg.h
>> index 5c383dae3d..2c5b056fc1 100644
>> --- a/target/ppc/spr_tcg.h
>> +++ b/target/ppc/spr_tcg.h
>> @@ -25,6 +25,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_generic(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);
>>   void spr_read_lr(DisasContext *ctx, int gprn, int sprn);
>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>> index b48eec83e3..e4f75ba380 100644
>> --- a/target/ppc/translate.c
>> +++ b/target/ppc/translate.c
>> @@ -401,6 +401,19 @@ void spr_write_generic(DisasContext *ctx, int sprn, int gprn)
>>       spr_store_dump_spr(sprn);
>>   }
>>   
>> +#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
>> +void spr_write_MMCR0_generic(DisasContext *ctx, int sprn, int gprn)
>> +{
>> +    gen_icount_io_start(ctx);
> 
> Since you're not using icount any more, do you still need these?

I believe we do. We're not using icount, but we should support icount guests.
And if an icount guest runs this code we'll have a 'bad icount read' error
because we're doing operations with the virtual clock.

I explained this rationale in patch 14. I should've mentioned that in this patch
instead.



> 
>> +    gen_helper_store_mmcr0(cpu_env, cpu_gpr[gprn]);
>> +}
>> +#else
>> +void spr_write_MMCR0_generic(DisasContext *ctx, int sprn, int gprn)
>> +{
>> +    spr_write_generic(ctx, sprn, gprn);
>> +}
>> +#endif
>> +
>>   #if !defined(CONFIG_USER_ONLY)
>>   void spr_write_generic32(DisasContext *ctx, int sprn, int gprn)
>>   {
>> @@ -609,6 +622,8 @@ void spr_write_MMCR0_ureg(DisasContext *ctx, int sprn, int gprn)
>>       t0 = tcg_temp_new();
>>       t1 = tcg_temp_new();
>>   
>> +    gen_icount_io_start(ctx);
>> +
>>       /*
>>        * Filter out all bits but FC, PMAO, and PMAE, according
>>        * to ISA v3.1, in 10.4.4 Monitor Mode Control Register 0,
>> @@ -620,7 +635,7 @@ void spr_write_MMCR0_ureg(DisasContext *ctx, int sprn, int gprn)
>>       tcg_gen_andi_tl(t1, t1, ~(MMCR0_FC | MMCR0_PMAO | MMCR0_PMAE));
>>       /* Keep all other bits intact */
>>       tcg_gen_or_tl(t1, t1, t0);
>> -    gen_store_spr(SPR_POWER_MMCR0, t1);
>> +    gen_helper_store_mmcr0(cpu_env, t1);
> 
> Do you need this change?  Won't the gen_store_spr() basically do the
> same thing as the gen_helpersince spr_write_MMCR0() expands to a
> gen_helper anyway?


Never realized that. I'll try it out.


Daniel


> 
>>   
>>       tcg_temp_free(t0);
>>       tcg_temp_free(t1);
>
diff mbox series

Patch

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 739005ba29..6878d950de 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1176,6 +1176,12 @@  struct CPUPPCState {
     uint32_t tm_vscr;
     uint64_t tm_dscr;
     uint64_t tm_tar;
+
+    /*
+     * PMU base time value used by the PMU to calculate
+     * running cycles.
+     */
+    uint64_t pmu_base_time;
 };
 
 #define SET_FIT_PERIOD(a_, b_, c_, d_)          \
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 860716da18..71f052b052 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6821,8 +6821,8 @@  static void register_book3s_pmu_sup_sprs(CPUPPCState *env)
 {
     spr_register_kvm(env, SPR_POWER_MMCR0, "MMCR0",
                      SPR_NOACCESS, SPR_NOACCESS,
-                     &spr_read_generic, &spr_write_generic,
-                     KVM_REG_PPC_MMCR0, 0x00000000);
+                     &spr_read_generic, &spr_write_MMCR0_generic,
+                     KVM_REG_PPC_MMCR0, 0x80000000);
     spr_register_kvm(env, SPR_POWER_MMCR1, "MMCR1",
                      SPR_NOACCESS, SPR_NOACCESS,
                      &spr_read_generic, &spr_write_generic,
@@ -6870,7 +6870,7 @@  static void register_book3s_pmu_user_sprs(CPUPPCState *env)
     spr_register(env, SPR_POWER_UMMCR0, "UMMCR0",
                  &spr_read_MMCR0_ureg, &spr_write_MMCR0_ureg,
                  &spr_read_ureg, &spr_write_ureg,
-                 0x00000000);
+                 0x80000000);
     spr_register(env, SPR_POWER_UMMCR1, "UMMCR1",
                  &spr_read_ureg, SPR_NOACCESS,
                  &spr_read_ureg, &spr_write_ureg,
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 4076aa281e..5122632784 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -20,6 +20,7 @@  DEF_HELPER_1(rfscv, void, env)
 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)
 #endif
 DEF_HELPER_1(check_tlb_flush_local, void, env)
 DEF_HELPER_1(check_tlb_flush_global, void, env)
diff --git a/target/ppc/meson.build b/target/ppc/meson.build
index b85f295703..278ce07da9 100644
--- a/target/ppc/meson.build
+++ b/target/ppc/meson.build
@@ -14,6 +14,7 @@  ppc_ss.add(when: 'CONFIG_TCG', if_true: files(
   'int_helper.c',
   'mem_helper.c',
   'misc_helper.c',
+  'power8_pmu.c',
   'timebase_helper.c',
   'translate.c',
 ))
diff --git a/target/ppc/power8_pmu.c b/target/ppc/power8_pmu.c
new file mode 100644
index 0000000000..47de38a99e
--- /dev/null
+++ b/target/ppc/power8_pmu.c
@@ -0,0 +1,74 @@ 
+/*
+ * PMU emulation helpers 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.
+ */
+
+#include "qemu/osdep.h"
+
+#include "cpu.h"
+#include "helper_regs.h"
+#include "exec/exec-all.h"
+#include "exec/helper-proto.h"
+#include "qemu/error-report.h"
+#include "qemu/main-loop.h"
+
+#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
+
+static void update_PMC_PM_CYC(CPUPPCState *env, int sprn,
+                              uint64_t time_delta)
+{
+    /*
+     * The pseries and pvn clock runs at 1Ghz, meaning that
+     * 1 nanosec equals 1 cycle.
+     */
+    env->spr[sprn] += time_delta;
+}
+
+static void update_cycles_PMCs(CPUPPCState *env)
+{
+    uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+    uint64_t time_delta = now - env->pmu_base_time;
+
+    update_PMC_PM_CYC(env, SPR_POWER_PMC6, time_delta);
+}
+
+void helper_store_mmcr0(CPUPPCState *env, target_ulong value)
+{
+    target_ulong curr_value = env->spr[SPR_POWER_MMCR0];
+    bool curr_FC = curr_value & MMCR0_FC;
+    bool new_FC = value & MMCR0_FC;
+
+    env->spr[SPR_POWER_MMCR0] = value;
+
+    /* MMCR0 writes can change HFLAGS_PMCCCLEAR */
+    if ((curr_value & MMCR0_PMCC) != (value & MMCR0_PMCC)) {
+        hreg_compute_hflags(env);
+    }
+
+    /*
+     * In an frozen count (FC) bit change:
+     *
+     * - if PMCs were running (curr_FC = false) and we're freezing
+     * them (new_FC = true), save the PMCs values in the registers.
+     *
+     * - if PMCs were frozen (curr_FC = true) and we're activating
+     * them (new_FC = false), set the new base_time for future cycle
+     * calculations.
+     */
+    if (curr_FC != new_FC) {
+        if (!curr_FC) {
+            update_cycles_PMCs(env);
+        } else {
+            env->pmu_base_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+        }
+    }
+}
+
+#endif /* defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY) */
diff --git a/target/ppc/spr_tcg.h b/target/ppc/spr_tcg.h
index 5c383dae3d..2c5b056fc1 100644
--- a/target/ppc/spr_tcg.h
+++ b/target/ppc/spr_tcg.h
@@ -25,6 +25,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_generic(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);
 void spr_read_lr(DisasContext *ctx, int gprn, int sprn);
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index b48eec83e3..e4f75ba380 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -401,6 +401,19 @@  void spr_write_generic(DisasContext *ctx, int sprn, int gprn)
     spr_store_dump_spr(sprn);
 }
 
+#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
+void spr_write_MMCR0_generic(DisasContext *ctx, int sprn, int gprn)
+{
+    gen_icount_io_start(ctx);
+    gen_helper_store_mmcr0(cpu_env, cpu_gpr[gprn]);
+}
+#else
+void spr_write_MMCR0_generic(DisasContext *ctx, int sprn, int gprn)
+{
+    spr_write_generic(ctx, sprn, gprn);
+}
+#endif
+
 #if !defined(CONFIG_USER_ONLY)
 void spr_write_generic32(DisasContext *ctx, int sprn, int gprn)
 {
@@ -609,6 +622,8 @@  void spr_write_MMCR0_ureg(DisasContext *ctx, int sprn, int gprn)
     t0 = tcg_temp_new();
     t1 = tcg_temp_new();
 
+    gen_icount_io_start(ctx);
+
     /*
      * Filter out all bits but FC, PMAO, and PMAE, according
      * to ISA v3.1, in 10.4.4 Monitor Mode Control Register 0,
@@ -620,7 +635,7 @@  void spr_write_MMCR0_ureg(DisasContext *ctx, int sprn, int gprn)
     tcg_gen_andi_tl(t1, t1, ~(MMCR0_FC | MMCR0_PMAO | MMCR0_PMAE));
     /* Keep all other bits intact */
     tcg_gen_or_tl(t1, t1, t0);
-    gen_store_spr(SPR_POWER_MMCR0, t1);
+    gen_helper_store_mmcr0(cpu_env, t1);
 
     tcg_temp_free(t0);
     tcg_temp_free(t1);