[PATCH v11 0/4] PMU-EBB support for PPC64 TCG

[PATCH v11 0/4] PMU-EBB support for PPC64 TCG

[Daniel Henrique Barboza]

Hi,

This new version makes a few modifications to make the EBB support more
generic.

A new patch (3) was added to implement PPC_INTERRUPT_EBB and its two
internal exceptions described by ISA v3.1: POWERPC_EXCP_PERFM_EBB and
POWERPC_EXCP_EXTERNAL_EBB. When receiving an EBB interrupt we check
BESCR bits to fire the appropriate exception. They are doing the same
thing ATM (clear GE and enter the branch with env->nip = SPR_EBBHR).
PPC_INTERRUPT_EBB will be used for the future XIVE IC EBB lane as well.

Patch 4 (previous 3) contains the helpers used by the PMU to fire the
PERFM_EBB exception, but now we're checking msr_pr and either throwing
the exception immediately or queueing it up for later via
PPC_INTERRUPT_EBB.  This change covers all the race conditions that the
kernel EBB selftests seems to trigger, and without using ppc_set_irq()
to handle BQL.

Changes from v10:
- patch 1:
  * added David's r-b
- patch 3 (new):
  * add PPC_INTERRUPT_EBB, POWERPC_EXCP_PERFM_EBB and
    POWERPC_EXCP_EXTERNAL_EBB
- patch 4:
  * all EBB bits are now being checked in the helper
  * a new static do_ebb() helper was created to handle the common
    EBB logic
  * we're now checking msr_pr and either throwing the exception immediately
    or queueing it for later using PPC_INTERRUPT_EBB
  * ppc_set_irq() call was removed
- v10 link: https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg01856.html


Daniel Henrique Barboza (4):
  target/ppc: fix indent of function parameters
  target/ppc: finalize pre-EBB PMU logic
  target/ppc: add PPC_INTERRUPT_EBB and EBB exceptions
  target/ppc: trigger PERFM EBBs from power8-pmu.c

 target/ppc/cpu.h         |  5 ++-
 target/ppc/cpu_init.c    |  4 ++
 target/ppc/excp_helper.c | 89 ++++++++++++++++++++++++++++++++++++++--
 target/ppc/helper.h      |  1 +
 target/ppc/power8-pmu.c  | 39 ++++++++++++++++--
 5 files changed, 129 insertions(+), 9 deletions(-)

-- 
2.34.1

[PATCH v11 1/4] target/ppc: fix indent of function parameters

[Daniel Henrique Barboza]

Fix indentation of powerpc_set_excp_state() and ppc_excp_apply_ail()
parameters.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 target/ppc/excp_helper.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index c107953dec..8a49a4ab90 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -265,9 +265,9 @@ static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState *env, int excp,
  * +--------------------------------------------------------------------+
  */
 static void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp,
-                                      target_ulong msr,
-                                      target_ulong *new_msr,
-                                      target_ulong *vector)
+                               target_ulong msr,
+                               target_ulong *new_msr,
+                               target_ulong *vector)
 {
 #if defined(TARGET_PPC64)
     CPUPPCState *env = &cpu->env;
@@ -362,7 +362,7 @@ static void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp,
 }
 
 static void powerpc_set_excp_state(PowerPCCPU *cpu,
-                                          target_ulong vector, target_ulong msr)
+                                   target_ulong vector, target_ulong msr)
 {
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
-- 
2.34.1

[PATCH v11 2/4] target/ppc: finalize pre-EBB PMU logic

[Daniel Henrique Barboza]

There are still PMU exclusive bits to handle in fire_PMC_interrupt()
before implementing the EBB support. Let's finalize it now to avoid
dealing with PMU and EBB logic at the same time in the next patches.

fire_PMC_interrupt() will fire an Performance Monitor alert depending on
MMCR0_PMAE. If we are required to freeze the timers (MMCR0_FCECE) we'll
also need to update summaries and delete the existing overflow timers.
In all cases we're going to update the cycle counters.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 target/ppc/power8-pmu.c | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c
index 236e8e66e9..d245663158 100644
--- a/target/ppc/power8-pmu.c
+++ b/target/ppc/power8-pmu.c
@@ -222,6 +222,20 @@ static void pmu_update_overflow_timers(CPUPPCState *env)
     }
 }
 
+static void pmu_delete_timers(CPUPPCState *env)
+{
+    QEMUTimer *pmc_overflow_timer;
+    int sprn;
+
+    for (sprn = SPR_POWER_PMC1; sprn <= SPR_POWER_PMC6; sprn++) {
+        pmc_overflow_timer = get_cyc_overflow_timer(env, sprn);
+
+        if (pmc_overflow_timer) {
+            timer_del(pmc_overflow_timer);
+        }
+    }
+}
+
 void helper_store_mmcr0(CPUPPCState *env, target_ulong value)
 {
     bool hflags_pmcc0 = (value & MMCR0_PMCC0) != 0;
@@ -271,8 +285,26 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu)
 {
     CPUPPCState *env = &cpu->env;
 
-    if (!(env->spr[SPR_POWER_MMCR0] & MMCR0_EBE)) {
-        return;
+    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 a new HFLAGS_INSN_CNT calc */
+        pmu_update_summaries(env);
+
+        /*
+         * Delete all pending timers if we need to freeze
+         * the PMC. We'll restart them when the PMC starts
+         * running again.
+         */
+        pmu_delete_timers(env);
+    }
+
+    if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMAE) {
+        env->spr[SPR_POWER_MMCR0] &= ~MMCR0_PMAE;
+        env->spr[SPR_POWER_MMCR0] |= MMCR0_PMAO;
     }
 
     /* PMC interrupt not implemented yet */
-- 
2.34.1

[PATCH v11 3/4] target/ppc: add PPC_INTERRUPT_EBB and EBB exceptions

[Daniel Henrique Barboza]

PPC_INTERRUPT_EBB is a new interrupt that will be used to deliver EBB
exceptions that had to be postponed because the thread wasn't in problem
state at the time the event-based branch was supposed to occur.

ISA 3.1 also defines two EBB exceptions: Performance Monitor EBB
exception and External EBB exception. They are being added as
POWERPC_EXCP_PERFM_EBB and POWERPC_EXCP_EXTERNAL_EBB.

PPC_INTERRUPT_EBB will check BESCR bits to see the EBB type that
occurred and trigger the appropriate exception. Both exceptions are
doing the same thing in this first implementation: clear BESCR_GE and
enter the branch with env->nip retrieved from SPR_EBBHR.

The checks being done by the interrupt code are msr_pr and BESCR_GE
states. All other checks (EBB facility check, BESCR_PME bit, specific
bits related to the event type) must be done beforehand.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 target/ppc/cpu.h         |  5 ++++-
 target/ppc/cpu_init.c    |  4 ++++
 target/ppc/excp_helper.c | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index dcd83b503c..3962c8f6f4 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -129,8 +129,10 @@ enum {
     /* ISA 3.00 additions */
     POWERPC_EXCP_HVIRT    = 101,
     POWERPC_EXCP_SYSCALL_VECTORED = 102, /* scv exception                     */
+    POWERPC_EXCP_PERFM_EBB = 103,    /* Performance Monitor EBB Exception    */
+    POWERPC_EXCP_EXTERNAL_EBB = 104, /* External EBB Exception               */
     /* EOL                                                                   */
-    POWERPC_EXCP_NB       = 103,
+    POWERPC_EXCP_NB       = 105,
     /* QEMU exceptions: special cases we want to stop translation            */
     POWERPC_EXCP_SYSCALL_USER = 0x203, /* System call in user mode only      */
 };
@@ -2453,6 +2455,7 @@ enum {
     PPC_INTERRUPT_HMI,            /* Hypervisor Maintenance interrupt    */
     PPC_INTERRUPT_HDOORBELL,      /* Hypervisor Doorbell interrupt        */
     PPC_INTERRUPT_HVIRT,          /* Hypervisor virtualization interrupt  */
+    PPC_INTERRUPT_EBB,            /* Event-based Branch exception         */
 };
 
 /* Processor Compatibility mask (PCR) */
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index bf60529d37..136d8ca8b5 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -2336,6 +2336,10 @@ static void init_excp_POWER8(CPUPPCState *env)
     env->excp_vectors[POWERPC_EXCP_FU]       = 0x00000F60;
     env->excp_vectors[POWERPC_EXCP_HV_FU]    = 0x00000F80;
     env->excp_vectors[POWERPC_EXCP_SDOOR_HV] = 0x00000E80;
+
+    /* Userland exceptions without vector value in PowerISA v3.1 */
+    env->excp_vectors[POWERPC_EXCP_PERFM_EBB] = 0x0;
+    env->excp_vectors[POWERPC_EXCP_EXTERNAL_EBB] = 0x0;
 #endif
 }
 
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 8a49a4ab90..ad40a0f8e6 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -990,6 +990,21 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
         new_msr |= (target_ulong)MSR_HVB;
         new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
         break;
+    case POWERPC_EXCP_PERFM_EBB:        /* Performance Monitor EBB Exception  */
+    case POWERPC_EXCP_EXTERNAL_EBB:     /* External EBB Exception             */
+        env->spr[SPR_BESCR] &= ~BESCR_GE;
+
+        /*
+         * Save NIP for rfebb insn in SPR_EBBRR. Next nip is
+         * stored in the EBB Handler SPR_EBBHR.
+         */
+        env->spr[SPR_EBBRR] = env->nip;
+        powerpc_set_excp_state(cpu, env->spr[SPR_EBBHR], env->msr);
+
+        /*
+         * This exception is handled in userspace. No need to proceed.
+         */
+        return;
     case POWERPC_EXCP_THERM:     /* Thermal interrupt                        */
     case POWERPC_EXCP_PERFM:     /* Embedded performance monitor interrupt   */
     case POWERPC_EXCP_VPUA:      /* Vector assist exception                  */
@@ -1681,6 +1696,24 @@ static void ppc_hw_interrupt(CPUPPCState *env)
             powerpc_excp(cpu, POWERPC_EXCP_THERM);
             return;
         }
+        /* EBB exception */
+        if (env->pending_interrupts & (1 << PPC_INTERRUPT_EBB)) {
+            /*
+             * EBB exception must be taken in problem state and
+             * with BESCR_GE set.
+             */
+            if (msr_pr == 1 && env->spr[SPR_BESCR] & BESCR_GE) {
+                env->pending_interrupts &= ~(1 << PPC_INTERRUPT_EBB);
+
+                if (env->spr[SPR_BESCR] & BESCR_PMEO) {
+                    powerpc_excp(cpu, POWERPC_EXCP_PERFM_EBB);
+                } else if (env->spr[SPR_BESCR] & BESCR_EEO) {
+                    powerpc_excp(cpu, POWERPC_EXCP_EXTERNAL_EBB);
+                }
+
+                return;
+            }
+        }
     }
 
     if (env->resume_as_sreset) {
-- 
2.34.1

[PATCH v11 4/4] target/ppc: trigger PERFM EBBs from power8-pmu.c

[Daniel Henrique Barboza]

This patch adds the EBB exception support that are triggered by
Performance Monitor alerts. This happens when a Performance Monitor
alert occurs and MMCR0_EBE, BESCR_PME and BESCR_GE are set.

fire_PMC_interrupt() will execute a new ebb_perfm_excp() helper that
will check for MMCR0_EBE, BESCR_PME and BESCR_GE bits. If all bits are
set, do_ebb() will attempt to trigger a PERFM EBB event.

If the EBB facility is enabled in both FSCR and HFSCR we consider that
the EBB is valid and set BESCR_PMEO. After that, if we're running in
problem state, fire a POWERPC_EXCP_PERM_EBB immediately. Otherwise we'll
queue a PPC_INTERRUPT_EBB.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 target/ppc/excp_helper.c | 48 ++++++++++++++++++++++++++++++++++++++++
 target/ppc/helper.h      |  1 +
 target/ppc/power8-pmu.c  |  3 +--
 3 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index ad40a0f8e6..0c031e67b1 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1948,6 +1948,54 @@ void helper_rfebb(CPUPPCState *env, target_ulong s)
         env->spr[SPR_BESCR] &= ~BESCR_GE;
     }
 }
+
+/*
+ * Triggers or queues an 'ebb_excp' EBB exception. All checks
+ * but FSCR, HFSCR and msr_pr must be done beforehand.
+ *
+ * PowerISA v3.1 isn't clear about whether an EBB should be
+ * postponed or cancelled if the EBB facility is unavailable.
+ * Our assumption here is that the EBB is cancelled if both
+ * FSCR and HFSCR EBB facilities aren't available.
+ */
+static void do_ebb(CPUPPCState *env, int ebb_excp)
+{
+    PowerPCCPU *cpu = env_archcpu(env);
+    CPUState *cs = CPU(cpu);
+
+    /*
+     * FSCR_EBB and FSCR_IC_EBB are the same bits used with
+     * HFSCR.
+     */
+    helper_fscr_facility_check(env, FSCR_EBB, 0, FSCR_IC_EBB);
+    helper_hfscr_facility_check(env, FSCR_EBB, "EBB", FSCR_IC_EBB);
+
+    if (ebb_excp == POWERPC_EXCP_PERFM_EBB) {
+        env->spr[SPR_BESCR] |= BESCR_PMEO;
+    } else if (ebb_excp == POWERPC_EXCP_EXTERNAL_EBB) {
+        env->spr[SPR_BESCR] |= BESCR_EEO;
+    }
+
+    if (msr_pr == 1) {
+        powerpc_excp(cpu, ebb_excp);
+    } else {
+        env->pending_interrupts |= 1 << PPC_INTERRUPT_EBB;
+        cpu_interrupt(cs, CPU_INTERRUPT_HARD);
+    }
+}
+
+void helper_ebb_perfm_excp(CPUPPCState *env)
+{
+    bool perfm_ebb_enabled = env->spr[SPR_POWER_MMCR0] & MMCR0_EBE &&
+                             env->spr[SPR_BESCR] & BESCR_PME &&
+                             env->spr[SPR_BESCR] & BESCR_GE;
+
+    if (!perfm_ebb_enabled) {
+        return;
+    }
+
+    do_ebb(env, POWERPC_EXCP_PERFM_EBB);
+}
 #endif
 
 /*****************************************************************************/
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index f2e5060910..adc31235a8 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -19,6 +19,7 @@ DEF_HELPER_1(rfid, void, env)
 DEF_HELPER_1(rfscv, void, env)
 DEF_HELPER_1(hrfid, void, env)
 DEF_HELPER_2(rfebb, void, env, tl)
+DEF_HELPER_1(ebb_perfm_excp, 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)
diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c
index d245663158..38e1ecb782 100644
--- a/target/ppc/power8-pmu.c
+++ b/target/ppc/power8-pmu.c
@@ -307,8 +307,7 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu)
         env->spr[SPR_POWER_MMCR0] |= MMCR0_PMAO;
     }
 
-    /* PMC interrupt not implemented yet */
-    return;
+    helper_ebb_perfm_excp(env);
 }
 
 /* This helper assumes that the PMC is running. */
-- 
2.34.1

Re: [PATCH v11 3/4] target/ppc: add PPC_INTERRUPT_EBB and EBB exceptions

[Cédric Le Goater]

On 2/11/22 19:33, Daniel Henrique Barboza wrote:
> PPC_INTERRUPT_EBB is a new interrupt that will be used to deliver EBB
> exceptions that had to be postponed because the thread wasn't in problem
> state at the time the event-based branch was supposed to occur.
> 
> ISA 3.1 also defines two EBB exceptions: Performance Monitor EBB
> exception and External EBB exception. They are being added as
> POWERPC_EXCP_PERFM_EBB and POWERPC_EXCP_EXTERNAL_EBB.
> 
> PPC_INTERRUPT_EBB will check BESCR bits to see the EBB type that
> occurred and trigger the appropriate exception. Both exceptions are
> doing the same thing in this first implementation: clear BESCR_GE and
> enter the branch with env->nip retrieved from SPR_EBBHR.
> 
> The checks being done by the interrupt code are msr_pr and BESCR_GE
> states. All other checks (EBB facility check, BESCR_PME bit, specific
> bits related to the event type) must be done beforehand.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>

It looks correct.

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Next step is to modify the POWER9 input pins and these routines :

   xive_tctx_realize()
   xive_tctx_output()
   power9_set_irq()

to add an EBB "wire" between the IC and the CPU.

Thanks,

C.


> ---
>   target/ppc/cpu.h         |  5 ++++-
>   target/ppc/cpu_init.c    |  4 ++++
>   target/ppc/excp_helper.c | 33 +++++++++++++++++++++++++++++++++
>   3 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index dcd83b503c..3962c8f6f4 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -129,8 +129,10 @@ enum {
>       /* ISA 3.00 additions */
>       POWERPC_EXCP_HVIRT    = 101,
>       POWERPC_EXCP_SYSCALL_VECTORED = 102, /* scv exception                     */
> +    POWERPC_EXCP_PERFM_EBB = 103,    /* Performance Monitor EBB Exception    */
> +    POWERPC_EXCP_EXTERNAL_EBB = 104, /* External EBB Exception               */
>       /* EOL                                                                   */
> -    POWERPC_EXCP_NB       = 103,
> +    POWERPC_EXCP_NB       = 105,
>       /* QEMU exceptions: special cases we want to stop translation            */
>       POWERPC_EXCP_SYSCALL_USER = 0x203, /* System call in user mode only      */
>   };
> @@ -2453,6 +2455,7 @@ enum {
>       PPC_INTERRUPT_HMI,            /* Hypervisor Maintenance interrupt    */
>       PPC_INTERRUPT_HDOORBELL,      /* Hypervisor Doorbell interrupt        */
>       PPC_INTERRUPT_HVIRT,          /* Hypervisor virtualization interrupt  */
> +    PPC_INTERRUPT_EBB,            /* Event-based Branch exception         */
>   };
>   
>   /* Processor Compatibility mask (PCR) */
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index bf60529d37..136d8ca8b5 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -2336,6 +2336,10 @@ static void init_excp_POWER8(CPUPPCState *env)
>       env->excp_vectors[POWERPC_EXCP_FU]       = 0x00000F60;
>       env->excp_vectors[POWERPC_EXCP_HV_FU]    = 0x00000F80;
>       env->excp_vectors[POWERPC_EXCP_SDOOR_HV] = 0x00000E80;
> +
> +    /* Userland exceptions without vector value in PowerISA v3.1 */
> +    env->excp_vectors[POWERPC_EXCP_PERFM_EBB] = 0x0;
> +    env->excp_vectors[POWERPC_EXCP_EXTERNAL_EBB] = 0x0;
>   #endif
>   }
>   
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 8a49a4ab90..ad40a0f8e6 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -990,6 +990,21 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>           new_msr |= (target_ulong)MSR_HVB;
>           new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
>           break;
> +    case POWERPC_EXCP_PERFM_EBB:        /* Performance Monitor EBB Exception  */
> +    case POWERPC_EXCP_EXTERNAL_EBB:     /* External EBB Exception             */
> +        env->spr[SPR_BESCR] &= ~BESCR_GE;
> +
> +        /*
> +         * Save NIP for rfebb insn in SPR_EBBRR. Next nip is
> +         * stored in the EBB Handler SPR_EBBHR.
> +         */
> +        env->spr[SPR_EBBRR] = env->nip;
> +        powerpc_set_excp_state(cpu, env->spr[SPR_EBBHR], env->msr);
> +
> +        /*
> +         * This exception is handled in userspace. No need to proceed.
> +         */
> +        return;
>       case POWERPC_EXCP_THERM:     /* Thermal interrupt                        */
>       case POWERPC_EXCP_PERFM:     /* Embedded performance monitor interrupt   */
>       case POWERPC_EXCP_VPUA:      /* Vector assist exception                  */
> @@ -1681,6 +1696,24 @@ static void ppc_hw_interrupt(CPUPPCState *env)
>               powerpc_excp(cpu, POWERPC_EXCP_THERM);
>               return;
>           }
> +        /* EBB exception */
> +        if (env->pending_interrupts & (1 << PPC_INTERRUPT_EBB)) {
> +            /*
> +             * EBB exception must be taken in problem state and
> +             * with BESCR_GE set.
> +             */
> +            if (msr_pr == 1 && env->spr[SPR_BESCR] & BESCR_GE) {
> +                env->pending_interrupts &= ~(1 << PPC_INTERRUPT_EBB);
> +
> +                if (env->spr[SPR_BESCR] & BESCR_PMEO) {
> +                    powerpc_excp(cpu, POWERPC_EXCP_PERFM_EBB);
> +                } else if (env->spr[SPR_BESCR] & BESCR_EEO) {
> +                    powerpc_excp(cpu, POWERPC_EXCP_EXTERNAL_EBB);
> +                }
> +
> +                return;
> +            }
> +        }
>       }
>   
>       if (env->resume_as_sreset) {
> 

Re: [PATCH v11 3/4] target/ppc: add PPC_INTERRUPT_EBB and EBB exceptions

[Daniel Henrique Barboza]

On 2/14/22 14:34, Cédric Le Goater wrote:
> On 2/11/22 19:33, Daniel Henrique Barboza wrote:
>> PPC_INTERRUPT_EBB is a new interrupt that will be used to deliver EBB
>> exceptions that had to be postponed because the thread wasn't in problem
>> state at the time the event-based branch was supposed to occur.
>>
>> ISA 3.1 also defines two EBB exceptions: Performance Monitor EBB
>> exception and External EBB exception. They are being added as
>> POWERPC_EXCP_PERFM_EBB and POWERPC_EXCP_EXTERNAL_EBB.
>>
>> PPC_INTERRUPT_EBB will check BESCR bits to see the EBB type that
>> occurred and trigger the appropriate exception. Both exceptions are
>> doing the same thing in this first implementation: clear BESCR_GE and
>> enter the branch with env->nip retrieved from SPR_EBBHR.
>>
>> The checks being done by the interrupt code are msr_pr and BESCR_GE
>> states. All other checks (EBB facility check, BESCR_PME bit, specific
>> bits related to the event type) must be done beforehand.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> 
> It looks correct.
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> 
> Next step is to modify the POWER9 input pins and these routines :
> 
>    xive_tctx_realize()
>    xive_tctx_output()
>    power9_set_irq()
> 
> to add an EBB "wire" between the IC and the CPU.

Got it. I'll see if I can get this EBB lane up from the IC and CPU. Any suggestions how I
should test it?


Thanks,



Daniel




> 
> Thanks,
> 
> C.
> 
> 
>> ---
>>   target/ppc/cpu.h         |  5 ++++-
>>   target/ppc/cpu_init.c    |  4 ++++
>>   target/ppc/excp_helper.c | 33 +++++++++++++++++++++++++++++++++
>>   3 files changed, 41 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
>> index dcd83b503c..3962c8f6f4 100644
>> --- a/target/ppc/cpu.h
>> +++ b/target/ppc/cpu.h
>> @@ -129,8 +129,10 @@ enum {
>>       /* ISA 3.00 additions */
>>       POWERPC_EXCP_HVIRT    = 101,
>>       POWERPC_EXCP_SYSCALL_VECTORED = 102, /* scv exception                     */
>> +    POWERPC_EXCP_PERFM_EBB = 103,    /* Performance Monitor EBB Exception    */
>> +    POWERPC_EXCP_EXTERNAL_EBB = 104, /* External EBB Exception               */
>>       /* EOL                                                                   */
>> -    POWERPC_EXCP_NB       = 103,
>> +    POWERPC_EXCP_NB       = 105,
>>       /* QEMU exceptions: special cases we want to stop translation            */
>>       POWERPC_EXCP_SYSCALL_USER = 0x203, /* System call in user mode only      */
>>   };
>> @@ -2453,6 +2455,7 @@ enum {
>>       PPC_INTERRUPT_HMI,            /* Hypervisor Maintenance interrupt    */
>>       PPC_INTERRUPT_HDOORBELL,      /* Hypervisor Doorbell interrupt        */
>>       PPC_INTERRUPT_HVIRT,          /* Hypervisor virtualization interrupt  */
>> +    PPC_INTERRUPT_EBB,            /* Event-based Branch exception         */
>>   };
>>   /* Processor Compatibility mask (PCR) */
>> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
>> index bf60529d37..136d8ca8b5 100644
>> --- a/target/ppc/cpu_init.c
>> +++ b/target/ppc/cpu_init.c
>> @@ -2336,6 +2336,10 @@ static void init_excp_POWER8(CPUPPCState *env)
>>       env->excp_vectors[POWERPC_EXCP_FU]       = 0x00000F60;
>>       env->excp_vectors[POWERPC_EXCP_HV_FU]    = 0x00000F80;
>>       env->excp_vectors[POWERPC_EXCP_SDOOR_HV] = 0x00000E80;
>> +
>> +    /* Userland exceptions without vector value in PowerISA v3.1 */
>> +    env->excp_vectors[POWERPC_EXCP_PERFM_EBB] = 0x0;
>> +    env->excp_vectors[POWERPC_EXCP_EXTERNAL_EBB] = 0x0;
>>   #endif
>>   }
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 8a49a4ab90..ad40a0f8e6 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -990,6 +990,21 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>>           new_msr |= (target_ulong)MSR_HVB;
>>           new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
>>           break;
>> +    case POWERPC_EXCP_PERFM_EBB:        /* Performance Monitor EBB Exception  */
>> +    case POWERPC_EXCP_EXTERNAL_EBB:     /* External EBB Exception             */
>> +        env->spr[SPR_BESCR] &= ~BESCR_GE;
>> +
>> +        /*
>> +         * Save NIP for rfebb insn in SPR_EBBRR. Next nip is
>> +         * stored in the EBB Handler SPR_EBBHR.
>> +         */
>> +        env->spr[SPR_EBBRR] = env->nip;
>> +        powerpc_set_excp_state(cpu, env->spr[SPR_EBBHR], env->msr);
>> +
>> +        /*
>> +         * This exception is handled in userspace. No need to proceed.
>> +         */
>> +        return;
>>       case POWERPC_EXCP_THERM:     /* Thermal interrupt                        */
>>       case POWERPC_EXCP_PERFM:     /* Embedded performance monitor interrupt   */
>>       case POWERPC_EXCP_VPUA:      /* Vector assist exception                  */
>> @@ -1681,6 +1696,24 @@ static void ppc_hw_interrupt(CPUPPCState *env)
>>               powerpc_excp(cpu, POWERPC_EXCP_THERM);
>>               return;
>>           }
>> +        /* EBB exception */
>> +        if (env->pending_interrupts & (1 << PPC_INTERRUPT_EBB)) {
>> +            /*
>> +             * EBB exception must be taken in problem state and
>> +             * with BESCR_GE set.
>> +             */
>> +            if (msr_pr == 1 && env->spr[SPR_BESCR] & BESCR_GE) {
>> +                env->pending_interrupts &= ~(1 << PPC_INTERRUPT_EBB);
>> +
>> +                if (env->spr[SPR_BESCR] & BESCR_PMEO) {
>> +                    powerpc_excp(cpu, POWERPC_EXCP_PERFM_EBB);
>> +                } else if (env->spr[SPR_BESCR] & BESCR_EEO) {
>> +                    powerpc_excp(cpu, POWERPC_EXCP_EXTERNAL_EBB);
>> +                }
>> +
>> +                return;
>> +            }
>> +        }
>>       }
>>       if (env->resume_as_sreset) {
>>
> 

Re: [PATCH v11 3/4] target/ppc: add PPC_INTERRUPT_EBB and EBB exceptions

[Cédric Le Goater]

On 2/14/22 18:52, Daniel Henrique Barboza wrote:
> 
> 
> On 2/14/22 14:34, Cédric Le Goater wrote:
>> On 2/11/22 19:33, Daniel Henrique Barboza wrote:
>>> PPC_INTERRUPT_EBB is a new interrupt that will be used to deliver EBB
>>> exceptions that had to be postponed because the thread wasn't in problem
>>> state at the time the event-based branch was supposed to occur.
>>>
>>> ISA 3.1 also defines two EBB exceptions: Performance Monitor EBB
>>> exception and External EBB exception. They are being added as
>>> POWERPC_EXCP_PERFM_EBB and POWERPC_EXCP_EXTERNAL_EBB.
>>>
>>> PPC_INTERRUPT_EBB will check BESCR bits to see the EBB type that
>>> occurred and trigger the appropriate exception. Both exceptions are
>>> doing the same thing in this first implementation: clear BESCR_GE and
>>> enter the branch with env->nip retrieved from SPR_EBBHR.
>>>
>>> The checks being done by the interrupt code are msr_pr and BESCR_GE
>>> states. All other checks (EBB facility check, BESCR_PME bit, specific
>>> bits related to the event type) must be done beforehand.
>>>
>>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>>
>> It looks correct.
>>
>> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>>
>> Next step is to modify the POWER9 input pins and these routines :
>>
>>    xive_tctx_realize()
>>    xive_tctx_output()
>>    power9_set_irq()
>>
>> to add an EBB "wire" between the IC and the CPU.
> 
> Got it. I'll see if I can get this EBB lane up from the IC and CPU. 
> Any suggestions how I should test it?

Without a software stack configuring the IC and handling the EBB,
you can not.

C.

Re: [PATCH v11 4/4] target/ppc: trigger PERFM EBBs from power8-pmu.c

[Cédric Le Goater]

On 2/11/22 19:33, Daniel Henrique Barboza wrote:
> This patch adds the EBB exception support that are triggered by
> Performance Monitor alerts. This happens when a Performance Monitor
> alert occurs and MMCR0_EBE, BESCR_PME and BESCR_GE are set.
> 
> fire_PMC_interrupt() will execute a new ebb_perfm_excp() helper that
> will check for MMCR0_EBE, BESCR_PME and BESCR_GE bits. If all bits are
> set, do_ebb() will attempt to trigger a PERFM EBB event.
> 
> If the EBB facility is enabled in both FSCR and HFSCR we consider that
> the EBB is valid and set BESCR_PMEO. After that, if we're running in
> problem state, fire a POWERPC_EXCP_PERM_EBB immediately. Otherwise we'll
> queue a PPC_INTERRUPT_EBB.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Looks good. One comment below.


> ---
>   target/ppc/excp_helper.c | 48 ++++++++++++++++++++++++++++++++++++++++
>   target/ppc/helper.h      |  1 +
>   target/ppc/power8-pmu.c  |  3 +--
>   3 files changed, 50 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index ad40a0f8e6..0c031e67b1 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1948,6 +1948,54 @@ void helper_rfebb(CPUPPCState *env, target_ulong s)
>           env->spr[SPR_BESCR] &= ~BESCR_GE;
>       }
>   }
> +
> +/*
> + * Triggers or queues an 'ebb_excp' EBB exception. All checks
> + * but FSCR, HFSCR and msr_pr must be done beforehand.
> + *
> + * PowerISA v3.1 isn't clear about whether an EBB should be
> + * postponed or cancelled if the EBB facility is unavailable.
> + * Our assumption here is that the EBB is cancelled if both
> + * FSCR and HFSCR EBB facilities aren't available.
> + */
> +static void do_ebb(CPUPPCState *env, int ebb_excp)
> +{
> +    PowerPCCPU *cpu = env_archcpu(env);
> +    CPUState *cs = CPU(cpu);
> +
> +    /*
> +     * FSCR_EBB and FSCR_IC_EBB are the same bits used with
> +     * HFSCR.
> +     */
> +    helper_fscr_facility_check(env, FSCR_EBB, 0, FSCR_IC_EBB);
> +    helper_hfscr_facility_check(env, FSCR_EBB, "EBB", FSCR_IC_EBB);
> +
> +    if (ebb_excp == POWERPC_EXCP_PERFM_EBB) {
> +        env->spr[SPR_BESCR] |= BESCR_PMEO;
> +    } else if (ebb_excp == POWERPC_EXCP_EXTERNAL_EBB) {
> +        env->spr[SPR_BESCR] |= BESCR_EEO;
> +    }
> +
> +    if (msr_pr == 1) {
> +        powerpc_excp(cpu, ebb_excp);
> +    } else {
> +        env->pending_interrupts |= 1 << PPC_INTERRUPT_EBB;
> +        cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> +    }

Don't you need to lock the iothread ?

Thanks,

C.

> +}
> +
> +void helper_ebb_perfm_excp(CPUPPCState *env)
> +{
> +    bool perfm_ebb_enabled = env->spr[SPR_POWER_MMCR0] & MMCR0_EBE &&
> +                             env->spr[SPR_BESCR] & BESCR_PME &&
> +                             env->spr[SPR_BESCR] & BESCR_GE;
> +
> +    if (!perfm_ebb_enabled) {
> +        return;
> +    }
> +
> +    do_ebb(env, POWERPC_EXCP_PERFM_EBB);
> +}
>   #endif
>   
>   /*****************************************************************************/
> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> index f2e5060910..adc31235a8 100644
> --- a/target/ppc/helper.h
> +++ b/target/ppc/helper.h
> @@ -19,6 +19,7 @@ DEF_HELPER_1(rfid, void, env)
>   DEF_HELPER_1(rfscv, void, env)
>   DEF_HELPER_1(hrfid, void, env)
>   DEF_HELPER_2(rfebb, void, env, tl)
> +DEF_HELPER_1(ebb_perfm_excp, 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)
> diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c
> index d245663158..38e1ecb782 100644
> --- a/target/ppc/power8-pmu.c
> +++ b/target/ppc/power8-pmu.c
> @@ -307,8 +307,7 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu)
>           env->spr[SPR_POWER_MMCR0] |= MMCR0_PMAO;
>       }
>   
> -    /* PMC interrupt not implemented yet */
> -    return;
> +    helper_ebb_perfm_excp(env);
>   }
>   
>   /* This helper assumes that the PMC is running. */

Re: [PATCH v11 4/4] target/ppc: trigger PERFM EBBs from power8-pmu.c

[Daniel Henrique Barboza]

On 2/15/22 13:37, Cédric Le Goater wrote:
> On 2/11/22 19:33, Daniel Henrique Barboza wrote:
>> This patch adds the EBB exception support that are triggered by
>> Performance Monitor alerts. This happens when a Performance Monitor
>> alert occurs and MMCR0_EBE, BESCR_PME and BESCR_GE are set.
>>
>> fire_PMC_interrupt() will execute a new ebb_perfm_excp() helper that
>> will check for MMCR0_EBE, BESCR_PME and BESCR_GE bits. If all bits are
>> set, do_ebb() will attempt to trigger a PERFM EBB event.
>>
>> If the EBB facility is enabled in both FSCR and HFSCR we consider that
>> the EBB is valid and set BESCR_PMEO. After that, if we're running in
>> problem state, fire a POWERPC_EXCP_PERM_EBB immediately. Otherwise we'll
>> queue a PPC_INTERRUPT_EBB.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> 
> Looks good. One comment below.
> 
> 
>> ---
>>   target/ppc/excp_helper.c | 48 ++++++++++++++++++++++++++++++++++++++++
>>   target/ppc/helper.h      |  1 +
>>   target/ppc/power8-pmu.c  |  3 +--
>>   3 files changed, 50 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index ad40a0f8e6..0c031e67b1 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -1948,6 +1948,54 @@ void helper_rfebb(CPUPPCState *env, target_ulong s)
>>           env->spr[SPR_BESCR] &= ~BESCR_GE;
>>       }
>>   }
>> +
>> +/*
>> + * Triggers or queues an 'ebb_excp' EBB exception. All checks
>> + * but FSCR, HFSCR and msr_pr must be done beforehand.
>> + *
>> + * PowerISA v3.1 isn't clear about whether an EBB should be
>> + * postponed or cancelled if the EBB facility is unavailable.
>> + * Our assumption here is that the EBB is cancelled if both
>> + * FSCR and HFSCR EBB facilities aren't available.
>> + */
>> +static void do_ebb(CPUPPCState *env, int ebb_excp)
>> +{
>> +    PowerPCCPU *cpu = env_archcpu(env);
>> +    CPUState *cs = CPU(cpu);
>> +
>> +    /*
>> +     * FSCR_EBB and FSCR_IC_EBB are the same bits used with
>> +     * HFSCR.
>> +     */
>> +    helper_fscr_facility_check(env, FSCR_EBB, 0, FSCR_IC_EBB);
>> +    helper_hfscr_facility_check(env, FSCR_EBB, "EBB", FSCR_IC_EBB);
>> +
>> +    if (ebb_excp == POWERPC_EXCP_PERFM_EBB) {
>> +        env->spr[SPR_BESCR] |= BESCR_PMEO;
>> +    } else if (ebb_excp == POWERPC_EXCP_EXTERNAL_EBB) {
>> +        env->spr[SPR_BESCR] |= BESCR_EEO;
>> +    }
>> +
>> +    if (msr_pr == 1) {
>> +        powerpc_excp(cpu, ebb_excp);
>> +    } else {
>> +        env->pending_interrupts |= 1 << PPC_INTERRUPT_EBB;
>> +        cpu_interrupt(cs, CPU_INTERRUPT_HARD);
>> +    }
> 
> Don't you need to lock the iothread ?

I did in the previous version but now, after doing the msr_pr handling like I'm doing
here, handling BQL wasn't necessary. I suppose this change in the logic handled the
race condition in a way that the lock isn't being exercised as before.


Thanks,


Daniel

> 
> Thanks,
> 
> C.
> 
>> +}
>> +
>> +void helper_ebb_perfm_excp(CPUPPCState *env)
>> +{
>> +    bool perfm_ebb_enabled = env->spr[SPR_POWER_MMCR0] & MMCR0_EBE &&
>> +                             env->spr[SPR_BESCR] & BESCR_PME &&
>> +                             env->spr[SPR_BESCR] & BESCR_GE;
>> +
>> +    if (!perfm_ebb_enabled) {
>> +        return;
>> +    }
>> +
>> +    do_ebb(env, POWERPC_EXCP_PERFM_EBB);
>> +}
>>   #endif
>>   /*****************************************************************************/
>> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
>> index f2e5060910..adc31235a8 100644
>> --- a/target/ppc/helper.h
>> +++ b/target/ppc/helper.h
>> @@ -19,6 +19,7 @@ DEF_HELPER_1(rfid, void, env)
>>   DEF_HELPER_1(rfscv, void, env)
>>   DEF_HELPER_1(hrfid, void, env)
>>   DEF_HELPER_2(rfebb, void, env, tl)
>> +DEF_HELPER_1(ebb_perfm_excp, 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)
>> diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c
>> index d245663158..38e1ecb782 100644
>> --- a/target/ppc/power8-pmu.c
>> +++ b/target/ppc/power8-pmu.c
>> @@ -307,8 +307,7 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu)
>>           env->spr[SPR_POWER_MMCR0] |= MMCR0_PMAO;
>>       }
>> -    /* PMC interrupt not implemented yet */
>> -    return;
>> +    helper_ebb_perfm_excp(env);
>>   }
>>   /* This helper assumes that the PMC is running. */
>