Re: [PATCH v8 10/10] target/ppc/excp_helper.c: EBB handling adjustments

2021-11-29 Thread David Gibson
On Thu, Nov 25, 2021 at 12:08:17PM -0300, Daniel Henrique Barboza wrote:
> The current logic is only considering event-based exceptions triggered
> by the performance monitor. This is true now, but we might want to add
> support for external event-based exceptions in the future.
> 
> Let's make it a bit easier to do so by adding the bit logic that would
> happen in case we were dealing with an external event-based exception.
> 
> While we're at it, add a few comments explaining why we're setting and
> clearing BESCR bits.
> 
> Signed-off-by: Daniel Henrique Barboza 

Reviewed-by: David Gibson 

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


[PATCH v8 10/10] target/ppc/excp_helper.c: EBB handling adjustments

2021-11-25 Thread Daniel Henrique Barboza
The current logic is only considering event-based exceptions triggered
by the performance monitor. This is true now, but we might want to add
support for external event-based exceptions in the future.

Let's make it a bit easier to do so by adding the bit logic that would
happen in case we were dealing with an external event-based exception.

While we're at it, add a few comments explaining why we're setting and
clearing BESCR bits.

Signed-off-by: Daniel Henrique Barboza 
---
 target/ppc/excp_helper.c | 45 ++--
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index a26d266fe6..42e2fee9c8 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -801,14 +801,47 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
excp_model, int excp)
 break;
 case POWERPC_EXCP_EBB:   /* Event-based branch exception */
 if ((env->spr[SPR_FSCR] & (1ull << FSCR_EBB)) &&
-(env->spr[SPR_BESCR] & BESCR_GE) &&
-(env->spr[SPR_BESCR] & BESCR_PME)) {
+(env->spr[SPR_BESCR] & BESCR_GE)) {
 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 */
+/*
+ * If we have Performance Monitor Event-Based exception
+ * enabled (BESCR_PME) and a Performance Monitor alert
+ * occurred (MMCR0_PMAO), clear BESCR_PME and set BESCR_PMEO
+ * (Performance Monitor Event-Based Exception Occurred).
+ *
+ * Software is responsible for clearing both BESCR_PMEO and
+ * MMCR0_PMAO after the event has been handled.
+ */
+if ((env->spr[SPR_BESCR] & BESCR_PME) &&
+(env->spr[SPR_POWER_MMCR0] & MMCR0_PMAO)) {
+env->spr[SPR_BESCR] &= ~BESCR_PME;
+env->spr[SPR_BESCR] |= BESCR_PMEO;
+}
+
+/*
+ * In the case of External Event-Based exceptions, do a
+ * similar logic with BESCR_EE and BESCR_EEO. BESCR_EEO must
+ * also be cleared by software.
+ *
+ * PowerISA 3.1 considers that we'll not have BESCR_PMEO and
+ * BESCR_EEO set at the same time. We can check for BESCR_PMEO
+ * being not set in step above to see if this exception was
+ * trigged by an external event.
+ */
+if (env->spr[SPR_BESCR] & BESCR_EE &&
+!(env->spr[SPR_BESCR] & BESCR_PMEO)) {
+env->spr[SPR_BESCR] &= ~BESCR_EE;
+env->spr[SPR_BESCR] |= BESCR_EEO;
+}
+
+/*
+ * Clear BESCR_GE, save NIP for 'rfebb' and point the
+ * execution to the event handler (SPR_EBBHR) address.
+ */
+env->spr[SPR_BESCR] &= ~BESCR_GE;
+env->spr[SPR_EBBRR] = env->nip;
+nip = env->spr[SPR_EBBHR];
 powerpc_set_excp_state(cpu, nip, env->msr);
 }
 /*
-- 
2.31.1