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

[Daniel Henrique Barboza]

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);
> >           }
> >           /*