Message ID | 20211201151734.654994-11-danielhb413@gmail.com |
---|---|
State | Changes Requested |
Headers | show |
Series | PMU-EBB support for PPC64 TCG | expand |
On Wed, Dec 01, 2021 at 12:17:34PM -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. > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Still looks fine, but I'm not seeing a particularly strong reason to keep this split from the previous patch. > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > --- > 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); > } > /*
On 12/8/21 22:52, David Gibson wrote: > On Wed, Dec 01, 2021 at 12:17:34PM -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. >> >> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > > Still looks fine, but I'm not seeing a particularly strong reason to > keep this split from the previous patch. > Fair enough. I'll squash this patch with the previous one. Thanks, Daniel >> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> >> --- >> 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); >> } >> /* >
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); } /*