On Mon, Aug 09, 2021 at 10:10:53AM -0300, Daniel Henrique Barboza wrote:
> All performance monitor counters can trigger a counter negative
> condition if the proper MMCR0 bits are set. This patch does that by
> doing the following:
>
> - pmc_counter_negative_enabled() will check whether a given PMC is
> eligible to trigger the counter negative alert;
>
> - get_counter_neg_timeout() will return the timeout for the counter
> negative condition for a given PMC, or -1 if the PMC is not able to
> trigger this alert;
>
> - the existing counter_negative_cond_enabled() now must consider the
> counter negative bit for PMCs 2-6, MMCR0_PMCjCE;
>
> - set_PMU_excp_timer() will now search all existing PMCs for the
> shortest counter negative timeout. The shortest timeout will be used to
> set the PMC interrupt timer.
>
> This change makes most EBB powepc kernel tests pass, validating that the
> existing EBB logic is consistent. There are a few tests that aren't passing
> due to additional PMU bits and perf events that aren't covered yet.
> We'll attempt to cover some of those in the next patches.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb...@gmail.com>
> ---
> target/ppc/cpu.h | 1 +
> target/ppc/pmu_book3s_helper.c | 96 ++++++++++++++++++++++++++++++----
> 2 files changed, 87 insertions(+), 10 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 5c81d459f4..1aa1fd42af 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -351,6 +351,7 @@ typedef struct ppc_v3_pate_t {
> #define MMCR0_FCECE PPC_BIT(38) /* FC on Enabled Cond or Event */
> #define MMCR0_PMCC PPC_BITMASK(44, 45) /* PMC Control */
> #define MMCR0_PMC1CE PPC_BIT(48)
> +#define MMCR0_PMCjCE PPC_BIT(49)
>
> #define MMCR1_PMC1SEL_SHIFT (63 - 39)
> #define MMCR1_PMC1SEL PPC_BITMASK(32, 39)
> diff --git a/target/ppc/pmu_book3s_helper.c b/target/ppc/pmu_book3s_helper.c
> index 7126e9b3d5..c5c5ab38c9 100644
> --- a/target/ppc/pmu_book3s_helper.c
> +++ b/target/ppc/pmu_book3s_helper.c
> @@ -143,22 +143,98 @@ static int64_t get_CYC_timeout(CPUPPCState *env, int
> sprn)
> return muldiv64(remaining_cyc, NANOSECONDS_PER_SECOND, PPC_CPU_FREQ);
> }
>
> -static void set_PMU_excp_timer(CPUPPCState *env)
> +static bool pmc_counter_negative_enabled(CPUPPCState *env, int sprn)
> {
> - uint64_t timeout, now;
> + switch (sprn) {
> + case SPR_POWER_PMC1:
> + return env->spr[SPR_POWER_MMCR0] & MMCR0_PMC1CE;
>
> - if (!(env->spr[SPR_POWER_MMCR0] & MMCR0_PMC1CE)) {
> - return;
> + case SPR_POWER_PMC2:
> + case SPR_POWER_PMC3:
> + case SPR_POWER_PMC4:
> + case SPR_POWER_PMC5:
> + case SPR_POWER_PMC6:
> + return env->spr[SPR_POWER_MMCR0] & MMCR0_PMCjCE;
> +
> + default:
> + break;
> }
>
> - switch (get_PMC_event(env, SPR_POWER_PMC1)) {
> - case 0x2:
> - timeout = get_INST_CMPL_timeout(env, SPR_POWER_PMC1);
> + return false;
> +}
> +
> +static int64_t get_counter_neg_timeout(CPUPPCState *env, int sprn)
> +{
> + int64_t timeout = -1;
> +
> + if (!pmc_counter_negative_enabled(env, sprn)) {
> + return -1;
> + }
> +
> + if (env->spr[sprn] >= COUNTER_NEGATIVE_VAL) {
> + return 0;
> + }
> +
> + switch (sprn) {
> + case SPR_POWER_PMC1:
> + case SPR_POWER_PMC2:
> + case SPR_POWER_PMC3:
> + case SPR_POWER_PMC4:
> + switch (get_PMC_event(env, sprn)) {
> + case 0x2:
> + timeout = get_INST_CMPL_timeout(env, sprn);
> + break;
> + case 0x1E:
> + timeout = get_CYC_timeout(env, sprn);
> + break;
> + }
> +
> break;
> - case 0x1e:
> - timeout = get_CYC_timeout(env, SPR_POWER_PMC1);
> + case SPR_POWER_PMC5:
> + timeout = get_INST_CMPL_timeout(env, sprn);
> + break;
> + case SPR_POWER_PMC6:
> + timeout = get_CYC_timeout(env, sprn);
> break;
> default:
> + break;
> + }
> +
> + return timeout;
> +}
> +
> +static void set_PMU_excp_timer(CPUPPCState *env)
> +{
> + int64_t timeout = -1;
> + uint64_t now;
> + int i;
> +
> + /*
> + * Scroll through all PMCs and check which one is closer to a
> + * counter negative timeout.
I'm wondering if it would be simpler to use a separate timer for each
PMC: after all the core timer logic must have already implemented this
"who fires first" logic.
> + */
> + for (i = SPR_POWER_PMC1; i <= SPR_POWER_PMC6; i++) {
> + int64_t curr_timeout = get_counter_neg_timeout(env, i);
> +
> + if (curr_timeout == -1) {
> + continue;
> + }
> +
> + if (curr_timeout == 0) {
> + timeout = 0;
> + break;
> + }
> +
> + if (timeout == -1 || timeout > curr_timeout) {
> + timeout = curr_timeout;
> + }
> + }
> +
> + /*
> + * This can happen if counter negative conditions were enabled
> + * without any events to be sampled.
> + */
> + if (timeout == -1) {
> return;
> }
>
> @@ -204,7 +280,7 @@ void cpu_ppc_pmu_timer_init(CPUPPCState *env)
>
> static bool counter_negative_cond_enabled(uint64_t mmcr0)
> {
> - return mmcr0 & MMCR0_PMC1CE;
> + return mmcr0 & (MMCR0_PMC1CE | MMCR0_PMCjCE);
> }
>
> void helper_store_mmcr0(CPUPPCState *env, target_ulong value)
--
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
