Re: [PATCH 10/19] target/ppc: PMU Event-Based exception support

[David Gibson]

On Mon, Aug 09, 2021 at 10:10:48AM -0300, Daniel Henrique Barboza wrote:
> From: Gustavo Romero <gromero@linux.ibm.com>
> 
> Following up the rfebb implementation, this patch adds the EBB exception
> support that are triggered by Performance Monitor alerts. This exception
> occurs when an enabled PMU condition or event happens and both MMCR0_EBE
> and BESCR_PME are set.
> 
> The supported PM alerts will consist of counter negative conditions of
> the PMU counters. This will be achieved by a timer mechanism that will
> predict when a counter becomes negative. The PMU timer callback will set
> the appropriate bits in MMCR0 and fire a PMC interrupt. The EBB
> exception code will then set the appropriate BESCR bits, set the next
> instruction pointer to the address pointed by the return register
> (SPR_EBBRR), and redirect execution to the handler (pointed by
> SPR_EBBHR).
> 
> This patch sets the basic structure of interrupts and the timer. The
> following patches will add the counter negative logic for the registers.
> The goal is to use the EBB selftests of the powerpc kernel to validate
> the EBB implementation, thus we'll add more PMU bits as we go along.
> 
> CC: Gustavo Romero <gustavo.romero@linaro.org>
> Signed-off-by: Gustavo Romero <gromero@linux.ibm.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  hw/ppc/spapr_cpu_core.c        |  6 ++++++
>  target/ppc/cpu.h               |  9 +++++++-
>  target/ppc/excp_helper.c       | 28 +++++++++++++++++++++++++
>  target/ppc/pmu_book3s_helper.c | 38 ++++++++++++++++++++++++++++++++++
>  4 files changed, 80 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 4f316a6f9d..41b443bde2 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -20,6 +20,7 @@
>  #include "target/ppc/kvm_ppc.h"
>  #include "hw/ppc/ppc.h"
>  #include "target/ppc/mmu-hash64.h"
> +#include "target/ppc/pmu_book3s_helper.h"
>  #include "sysemu/numa.h"
>  #include "sysemu/reset.h"
>  #include "sysemu/hw_accel.h"
> @@ -266,6 +267,11 @@ static bool spapr_realize_vcpu(PowerPCCPU *cpu, 
> SpaprMachineState *spapr,
>          return false;
>      }
>  
> +    /* Init PMU interrupt timer (TCG only) */
> +    if (!kvm_enabled()) {

I'd prefer this check to be inside the called function.

> +        cpu_ppc_pmu_timer_init(env);
> +    }
> +
>      if (!sc->pre_3_0_migration) {
>          vmstate_register(NULL, cs->cpu_index, &vmstate_spapr_cpu_state,
>                           cpu->machine_data);
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index ae431e65be..1d38b8cf7a 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -129,8 +129,9 @@ enum {
>      /* ISA 3.00 additions */
>      POWERPC_EXCP_HVIRT    = 101,
>      POWERPC_EXCP_SYSCALL_VECTORED = 102, /* scv exception                    
>  */
> +    POWERPC_EXCP_EBB = 103, /* Event-based branch exception                  
> */
>      /* EOL                                                                   
> */
> -    POWERPC_EXCP_NB       = 103,
> +    POWERPC_EXCP_NB       = 104,
>      /* QEMU exceptions: special cases we want to stop translation            
> */
>      POWERPC_EXCP_SYSCALL_USER = 0x203, /* System call in user mode only      
> */
>  };
> @@ -1201,6 +1202,11 @@ struct CPUPPCState {
>       * instructions and cycles.
>       */
>      uint64_t pmu_base_icount;
> +
> +    /*
> +     * Timer used to fire performance monitor alerts and interrupts.
> +     */
> +    QEMUTimer *pmu_intr_timer;
>  };
>  
>  #define SET_FIT_PERIOD(a_, b_, c_, d_)          \
> @@ -2417,6 +2423,7 @@ enum {
>      PPC_INTERRUPT_HMI,            /* Hypervisor Maintenance interrupt    */
>      PPC_INTERRUPT_HDOORBELL,      /* Hypervisor Doorbell interrupt        */
>      PPC_INTERRUPT_HVIRT,          /* Hypervisor virtualization interrupt  */
> +    PPC_INTERRUPT_PMC,            /* Performance Monitor Counter interrupt */
>  };
>  
>  /* Processor Compatibility mask (PCR) */
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index a79a0ed465..b866209b6e 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -821,6 +821,22 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
> excp_model, int excp)
>          cpu_abort(cs, "Non maskable external exception "
>                    "is not implemented yet !\n");
>          break;
> +    case POWERPC_EXCP_EBB:       /* Event-based branch exception             
> */
> +        if ((env->spr[SPR_BESCR] & BESCR_GE) &&
> +            (env->spr[SPR_BESCR] & BESCR_PME)) {
> +            target_ulong nip;
> +
> +            env->spr[SPR_BESCR] &= ~BESCR_GE;   /* Clear GE */
> +            env->spr[SPR_BESCR] |= BESCR_PMEO;  /* Set PMEO */
> +            env->spr[SPR_EBBRR] = env->nip;     /* Save NIP for rfebb insn */
> +            nip = env->spr[SPR_EBBHR];          /* EBB handler */
> +            powerpc_set_excp_state(cpu, nip, env->msr);
> +        }
> +        /*
> +         * This interrupt is handled by userspace. No need
> +         * to proceed.
> +         */
> +        return;
>      default:
>      excp_invalid:
>          cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
> @@ -1068,6 +1084,18 @@ static void ppc_hw_interrupt(CPUPPCState *env)
>              powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_THERM);
>              return;
>          }
> +        /* PMC -> Event-based branch exception */
> +        if (env->pending_interrupts & (1 << PPC_INTERRUPT_PMC)) {
> +            /*
> +             * Performance Monitor event-based exception can only
> +             * occur in problem state.
> +             */
> +            if (msr_pr == 1) {
> +                env->pending_interrupts &= ~(1 << PPC_INTERRUPT_PMC);
> +                powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_EBB);
> +                return;
> +            }
> +        }
>      }
>  
>      if (env->resume_as_sreset) {
> diff --git a/target/ppc/pmu_book3s_helper.c b/target/ppc/pmu_book3s_helper.c
> index 91bb82e699..43cc0eb722 100644
> --- a/target/ppc/pmu_book3s_helper.c
> +++ b/target/ppc/pmu_book3s_helper.c
> @@ -10,12 +10,15 @@
>   * See the COPYING file in the top-level directory.
>   */
>  
> +#include "pmu_book3s_helper.h"
> +
>  #include "qemu/osdep.h"
>  #include "cpu.h"
>  #include "exec/exec-all.h"
>  #include "exec/helper-proto.h"
>  #include "qemu/error-report.h"
>  #include "qemu/main-loop.h"
> +#include "hw/ppc/ppc.h"
>  
>  /*
>   * Set arbitrarily based on clock-frequency values used in PNV
> @@ -96,6 +99,41 @@ static void update_PMCs(CPUPPCState *env, uint64_t 
> icount_delta)
>      update_PMC_PM_CYC(env, SPR_POWER_PMC6, icount_delta);
>  }
>  
> +static void cpu_ppc_pmu_timer_cb(void *opaque)
> +{
> +    PowerPCCPU *cpu = opaque;
> +    CPUPPCState *env = &cpu->env;
> +    uint64_t mmcr0;
> +
> +    mmcr0 = env->spr[SPR_POWER_MMCR0];
> +    if (env->spr[SPR_POWER_MMCR0] & MMCR0_EBE) {
> +        /* freeeze counters if needed */
> +        if (mmcr0 & MMCR0_FCECE) {
> +            mmcr0 &= ~MMCR0_FCECE;
> +            mmcr0 |= MMCR0_FC;
> +        }
> +
> +        /* Clear PMAE and set PMAO */
> +        if (mmcr0 & MMCR0_PMAE) {
> +            mmcr0 &= ~MMCR0_PMAE;
> +            mmcr0 |= MMCR0_PMAO;
> +        }
> +        env->spr[SPR_POWER_MMCR0] = mmcr0;

Don't you need to go through the helper here to make sure the freeze
counter logic runs?

> +        /* Fire the PMC hardware exception */
> +        ppc_set_irq(cpu, PPC_INTERRUPT_PMC, 1);
> +    }
> +}
> +
> +void cpu_ppc_pmu_timer_init(CPUPPCState *env)
> +{
> +    PowerPCCPU *cpu = env_archcpu(env);
> +    QEMUTimer *timer;
> +
> +    timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &cpu_ppc_pmu_timer_cb, cpu);

I don't see where the actual time-until-interrupt is calculated.

> +    env->pmu_intr_timer = timer;
> +}
> +
>  void helper_store_mmcr0(CPUPPCState *env, target_ulong value)
>  {
>      uint64_t curr_icount = (uint64_t)icount_get_raw();

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