Re: [PATCH v1 2/2] target/ppc: PMU implement PERFM interrupts

[Daniel Henrique Barboza]

On 5/30/23 10:43, Nicholas Piggin wrote:
> The PMU raises a performance monitor exception (causing an interrupt
> when MSR[EE]=1) when MMCR0[PMAO] is set, and lowers it when clear.
>
> Wire this up and implement the interrupt delivery for books. Linux perf
> record can now collect PMI-driven samples.
>
> fire_PMC_interrupt is renamed to perfm_alert, which matches a bit closer
> to the new terminology used in the ISA and distinguishes the alert
> condition (e.g., counter overflow) from the PERFM (or EBB) interrupts.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

And queued.


Daniel

>   target/ppc/excp_helper.c |  2 +-
>   target/ppc/power8-pmu.c  | 20 +++++++++++++-------
>   2 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 0f7ed58673..4925996cf3 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1611,6 +1611,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>       case POWERPC_EXCP_ISEG:      /* Instruction segment exception            
> */
>       case POWERPC_EXCP_TRACE:     /* Trace exception                          
> */
>       case POWERPC_EXCP_SDOOR:     /* Doorbell interrupt                       
> */
> +    case POWERPC_EXCP_PERFM:     /* Performance monitor interrupt            */
>           break;
>       case POWERPC_EXCP_HISI:      /* Hypervisor instruction storage exception 
> */
>           msr |= env->error_code;
> @@ -1664,7 +1665,6 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>            */
>           return;
>       case POWERPC_EXCP_THERM:     /* Thermal interrupt                        
> */
> -    case POWERPC_EXCP_PERFM:     /* Embedded performance monitor interrupt   */
>       case POWERPC_EXCP_VPUA:      /* Vector assist exception                  
> */
>       case POWERPC_EXCP_MAINT:     /* Maintenance exception                    
> */
>       case POWERPC_EXCP_HV_MAINT:  /* Hypervisor Maintenance exception         
> */
> diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c
> index af065115f2..48f2868f8b 100644
> --- a/target/ppc/power8-pmu.c
> +++ b/target/ppc/power8-pmu.c
> @@ -84,8 +84,16 @@ static void pmu_update_summaries(CPUPPCState *env)
>   
>   void pmu_mmcr01_updated(CPUPPCState *env)
>   {
> +    PowerPCCPU *cpu = env_archcpu(env);
> +
>       pmu_update_summaries(env);
>       hreg_update_pmu_hflags(env);
> +
> +    if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMAO)
> +        ppc_set_irq(cpu, PPC_INTERRUPT_PERFM, 1);
> +    else
> +        ppc_set_irq(cpu, PPC_INTERRUPT_PERFM, 0);
> +
>       /*
>        * Should this update overflow timers (if mmcr0 is updated) so they
>        * get set in cpu_post_load?
> @@ -282,7 +290,7 @@ void helper_store_pmc(CPUPPCState *env, uint32_t sprn, 
> uint64_t value)
>       pmc_update_overflow_timer(env, sprn);
>   }
>   
> -static void fire_PMC_interrupt(PowerPCCPU *cpu)
> +static void perfm_alert(PowerPCCPU *cpu)
>   {
>       CPUPPCState *env = &cpu->env;
>   
> @@ -306,6 +314,7 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu)
>           /* These MMCR0 bits do not require summaries or hflags update. */
>           env->spr[SPR_POWER_MMCR0] &= ~MMCR0_PMAE;
>           env->spr[SPR_POWER_MMCR0] |= MMCR0_PMAO;
> +        ppc_set_irq(cpu, PPC_INTERRUPT_PERFM, 1);
>       }
>   
>       raise_ebb_perfm_exception(env);
> @@ -314,20 +323,17 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu)
>   void helper_handle_pmc5_overflow(CPUPPCState *env)
>   {
>       env->spr[SPR_POWER_PMC5] = PMC_COUNTER_NEGATIVE_VAL;
> -    fire_PMC_interrupt(env_archcpu(env));
> +    perfm_alert(env_archcpu(env));
>   }
>   
>   /* This helper assumes that the PMC is running. */
>   void helper_insns_inc(CPUPPCState *env, uint32_t num_insns)
>   {
>       bool overflow_triggered;
> -    PowerPCCPU *cpu;
>   
>       overflow_triggered = pmu_increment_insns(env, num_insns);
> -
>       if (overflow_triggered) {
> -        cpu = env_archcpu(env);
> -        fire_PMC_interrupt(cpu);
> +        perfm_alert(env_archcpu(env));
>       }
>   }
>   
> @@ -335,7 +341,7 @@ static void cpu_ppc_pmu_timer_cb(void *opaque)
>   {
>       PowerPCCPU *cpu = opaque;
>   
> -    fire_PMC_interrupt(cpu);
> +    perfm_alert(cpu);
>   }
>   
>   void cpu_ppc_pmu_init(CPUPPCState *env)