[PATCH v1 1/2] target/ppc: PMU do not clear MMCR0[FCECE] on performance monitor alert

[PATCH v1 1/2] target/ppc: PMU do not clear MMCR0[FCECE] on performance monitor alert

[Nicholas Piggin]

FCECE does not get cleared according to the ISA v3.1B.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/power8-pmu.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c
index c4c331c6b5..af065115f2 100644
--- a/target/ppc/power8-pmu.c
+++ b/target/ppc/power8-pmu.c
@@ -289,7 +289,6 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu)
     pmu_update_cycles(env);
 
     if (env->spr[SPR_POWER_MMCR0] & MMCR0_FCECE) {
-        env->spr[SPR_POWER_MMCR0] &= ~MMCR0_FCECE;
         env->spr[SPR_POWER_MMCR0] |= MMCR0_FC;
 
         /* Changing MMCR0_FC requires summaries and hflags update */
-- 
2.40.1

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

[Nicholas Piggin]

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

Re: [PATCH v1 1/2] target/ppc: PMU do not clear MMCR0[FCECE] on performance monitor alert

[Daniel Henrique Barboza]

On 5/30/23 10:43, Nicholas Piggin wrote:
> FCECE does not get cleared according to the ISA v3.1B.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

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

And queued.


Daniel

>   target/ppc/power8-pmu.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c
> index c4c331c6b5..af065115f2 100644
> --- a/target/ppc/power8-pmu.c
> +++ b/target/ppc/power8-pmu.c
> @@ -289,7 +289,6 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu)
>       pmu_update_cycles(env);
>   
>       if (env->spr[SPR_POWER_MMCR0] & MMCR0_FCECE) {
> -        env->spr[SPR_POWER_MMCR0] &= ~MMCR0_FCECE;
>           env->spr[SPR_POWER_MMCR0] |= MMCR0_FC;
>   
>           /* Changing MMCR0_FC requires summaries and hflags update */

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)