Re: [PATCH v5 09/10] target/ppc: PMU Event-Based exception support

[Daniel Henrique Barboza <danielhb413@gmail.com>]

On 11/8/21 16:48, Matheus K. Ferst wrote:
> On 01/11/2021 20:56, Daniel Henrique Barboza wrote:
>> From: Gustavo Romero <gromero@linux.ibm.com>
>>
>> Following up the rfebb implementation, this patch adds the EBB exception
>> support that are triggered by Performance Monitor alerts. This exception
>> occurs when an enabled PMU condition or event happens and both MMCR0_EBE
>> and BESCR_PME are set.
>>
>> The supported PM alerts will consist of counter negative conditions of
>> the PMU counters. This will be achieved by a timer mechanism that will
>> predict when a counter becomes negative. The PMU timer callback will set
>> the appropriate bits in MMCR0 and fire a PMC interrupt. The EBB
>> exception code will then set the appropriate BESCR bits, set the next
>> instruction pointer to the address pointed by the return register
>> (SPR_EBBRR), and redirect execution to the handler (pointed by
>> SPR_EBBHR).
>>
>> CC: Gustavo Romero <gustavo.romero@linaro.org>
>> Signed-off-by: Gustavo Romero <gromero@linux.ibm.com>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>>   target/ppc/cpu.h         |  5 ++++-
>>   target/ppc/excp_helper.c | 28 ++++++++++++++++++++++++++++
>>   target/ppc/power8-pmu.c  | 26 ++++++++++++++++++++++++--
>>   3 files changed, 56 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
>> index 8f545ff482..592031ce54 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_EBB = 103, /* Event-based branch exception                  */
>> +
>>       /* EOL                                                                   */
>> -    POWERPC_EXCP_NB       = 103,
>> +    POWERPC_EXCP_NB       = 104,
>>       /* QEMU exceptions: special cases we want to stop translation            */
>>       POWERPC_EXCP_SYSCALL_USER = 0x203, /* System call in user mode only      */
>>   };
>> @@ -2455,6 +2457,7 @@ enum {
>>       PPC_INTERRUPT_HMI,            /* Hypervisor Maintenance interrupt    */
>>       PPC_INTERRUPT_HDOORBELL,      /* Hypervisor Doorbell interrupt        */
>>       PPC_INTERRUPT_HVIRT,          /* Hypervisor virtualization interrupt  */
>> +    PPC_INTERRUPT_PMC,            /* Hypervisor virtualization interrupt  */
>>   };
>>
>>   /* Processor Compatibility mask (PCR) */
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 7be334e007..88aa0a84f8 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -797,6 +797,22 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>>           cpu_abort(cs, "Non maskable external exception "
>>                     "is not implemented yet !\n");
>>           break;
>> +    case POWERPC_EXCP_EBB:       /* Event-based branch exception             */
>> +        if ((env->spr[SPR_BESCR] & BESCR_GE) &&
>> +            (env->spr[SPR_BESCR] & BESCR_PME)) {
> 
> Do we need to check FSCR[EBB] here?

FSCR[EBB] is being checked in the spr_read_ebb* and spr_write_ebb* callbacks
in translate.c. These are the read/write callbacks of EBB sprs
(register_power8_ebb_sprs() in target/ppc/cpu_init.c).


Thanks,


Daniel


> 
>> +            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 */
>> +            powerpc_set_excp_state(cpu, nip, env->msr);
>> +        }
>> +        /*
>> +         * This interrupt is handled by userspace. No need
>> +         * to proceed.
>> +         */
>> +        return;
>>       default:
>>       excp_invalid:
>>           cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
>> @@ -1044,6 +1060,18 @@ static void ppc_hw_interrupt(CPUPPCState *env)
>>               powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_THERM);
>>               return;
>>           }
>> +        /* PMC -> Event-based branch exception */
>> +        if (env->pending_interrupts & (1 << PPC_INTERRUPT_PMC)) {
>> +            /*
>> +             * Performance Monitor event-based exception can only
>> +             * occur in problem state.
>> +             */
>> +            if (msr_pr == 1) {
>> +                env->pending_interrupts &= ~(1 << PPC_INTERRUPT_PMC);
>> +                powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_EBB);
>> +                return;
>> +            }
>> +        }
>>       }
>>
>>       if (env->resume_as_sreset) {
>> diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c
>> index aa10233b29..ca3954ff0e 100644
>> --- a/target/ppc/power8-pmu.c
>> +++ b/target/ppc/power8-pmu.c
>> @@ -323,8 +323,30 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu)
>>           return;
>>       }
>>
>> -    /* PMC interrupt not implemented yet */
>> -    return;
>> +    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 demands a new hflags compute */
>> +        hreg_compute_hflags(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);
>> +    }
>> +
>> +    pmu_update_cycles(env);
>> +
>> +    if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMAE) {
>> +        env->spr[SPR_POWER_MMCR0] &= ~MMCR0_PMAE;
>> +        env->spr[SPR_POWER_MMCR0] |= MMCR0_PMAO;
>> +    }
>> +
>> +    /* Fire the PMC hardware exception */
>> +    ppc_set_irq(cpu, PPC_INTERRUPT_PMC, 1);
>>   }
>>
>>   /* This helper assumes that the PMC is running. */
>> -- 
>> 2.31.1
>>
> 
>