Re: [PATCH 18/19] target/ppc/pmu_book3s_helper.c: add PM_CMPLU_STALL mock events

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

On 8/10/21 1:17 AM, David Gibson wrote:
> On Mon, Aug 09, 2021 at 10:10:56AM -0300, Daniel Henrique Barboza wrote:
>> EBB powerpc kernel test 'multi_counter_test' uses PM_CMPLU_STALL events
>> that we do not support. These events are related to CPU stalled/wasted
>> cycles while waiting for resources, cache misses and so on.
>>
>> Unlike the 0xFA event added previously, there's no available equivalent
>> for us to use, and at this moment we can't sample those events as well.
>> What we can do is mock those events as if we were calculating them.
>>
>> This patch implements PM_CMPLU_STALL, PM_CMPLU_STALL_FXU,
>> PM_CMPLU_STALL_OTHER_CMPL and PM_CMPLU_STALL_THRD mock events by giving
>> them a fixed amount of the total elapsed cycles.
>>
>> The chosen sample values for these events (25% of total cycles for
>> PM_CMPLU_STALL and 5% for the other three) were chosen at random and has
>> no intention of being truthful with what a real PowerPC hardware would
>> give us. Our intention here is to make 'multi_counter_test' EBB test
>> pass.
> 
> Hmm.  I guess these mock values make sense for getting the kernel
> tests to pass, but I'm not sure if it's a good idea in general.  Would
> we be better off just reporting 0 always - that would be a strong hint
> to someone attempting to analyze results that something is fishy (in
> this case that they don't actually have a real CPU).

We can drop this patch and I'll add a note in the docs that the EBB kernel
test 'multi_counter_test' fails because the PMU does not implement STALL
events.


Thanks,


Daniel

> 
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>>   target/ppc/pmu_book3s_helper.c | 81 +++++++++++++++++++++++++++++++++-
>>   1 file changed, 79 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/ppc/pmu_book3s_helper.c b/target/ppc/pmu_book3s_helper.c
>> index ae7050cd62..32cf76b77f 100644
>> --- a/target/ppc/pmu_book3s_helper.c
>> +++ b/target/ppc/pmu_book3s_helper.c
>> @@ -92,16 +92,54 @@ static void update_PMC_PM_CYC(CPUPPCState *env, int sprn,
>>       env->spr[sprn] += get_cycles(icount_delta);
>>   }
>>   
>> +static int get_stall_ratio(uint8_t stall_event)
>> +{
>> +    int stall_ratio = 0;
>> +
>> +    switch (stall_event) {
>> +    case 0xA:
>> +        stall_ratio = 25;
>> +        break;
>> +    case 0x6:
>> +    case 0x16:
>> +    case 0x1C:
>> +        stall_ratio = 5;
>> +        break;
>> +    default:
>> +        break;
>> +    }
>> +
>> +    return stall_ratio;
>> +}
>> +
>> +static void update_PMC_PM_STALL(CPUPPCState *env, int sprn,
>> +                                uint64_t icount_delta,
>> +                                uint8_t stall_event)
>> +{
>> +    int stall_ratio = get_stall_ratio(stall_event);
>> +    uint64_t cycles = muldiv64(get_cycles(icount_delta), stall_ratio, 100);
>> +
>> +    env->spr[sprn] += cycles;
>> +}
>> +
>>   static void update_programmable_PMC_reg(CPUPPCState *env, int sprn,
>>                                           uint64_t icount_delta)
>>   {
>> -    switch (get_PMC_event(env, sprn)) {
>> +    uint8_t event = get_PMC_event(env, sprn);
>> +
>> +    switch (event) {
>>       case 0x2:
>>           update_PMC_PM_INST_CMPL(env, sprn, icount_delta);
>>           break;
>>       case 0x1E:
>>           update_PMC_PM_CYC(env, sprn, icount_delta);
>>           break;
>> +    case 0xA:
>> +    case 0x6:
>> +    case 0x16:
>> +    case 0x1C:
>> +        update_PMC_PM_STALL(env, sprn, icount_delta, event);
>> +        break;
>>       default:
>>           return;
>>       }
>> @@ -163,6 +201,34 @@ static int64_t get_CYC_timeout(CPUPPCState *env, int sprn)
>>       return muldiv64(remaining_cyc, NANOSECONDS_PER_SECOND, PPC_CPU_FREQ);
>>   }
>>   
>> +static int64_t get_stall_timeout(CPUPPCState *env, int sprn,
>> +                                 uint8_t stall_event)
>> +{
>> +    uint64_t remaining_cyc;
>> +    int stall_multiplier;
>> +
>> +    if (env->spr[sprn] == 0) {
>> +        return icount_to_ns(COUNTER_NEGATIVE_VAL);
>> +    }
>> +
>> +    if (env->spr[sprn] >= COUNTER_NEGATIVE_VAL) {
>> +        return 0;
>> +    }
>> +
>> +    remaining_cyc = COUNTER_NEGATIVE_VAL - env->spr[sprn];
>> +
>> +    /*
>> +     * Consider that for this stall event we'll advance the counter
>> +     * in a lower rate, thus requiring more cycles to overflow.
>> +     * E.g. for PM_CMPLU_STALL (0xA), ratio 25, it'll require
>> +     * 100/25 = 4 times the same amount of cycles to overflow.
>> +     */
>> +    stall_multiplier = 100 / get_stall_ratio(stall_event);
>> +    remaining_cyc *= stall_multiplier;
>> +
>> +    return muldiv64(remaining_cyc, NANOSECONDS_PER_SECOND, PPC_CPU_FREQ);
>> +}
>> +
>>   static bool pmc_counter_negative_enabled(CPUPPCState *env, int sprn)
>>   {
>>       bool PMC14_running = !(env->spr[SPR_POWER_MMCR0] & MMCR0_FC14);
>> @@ -191,6 +257,7 @@ static bool pmc_counter_negative_enabled(CPUPPCState *env, int sprn)
>>   static int64_t get_counter_neg_timeout(CPUPPCState *env, int sprn)
>>   {
>>       int64_t timeout = -1;
>> +    uint8_t event;
>>   
>>       if (!pmc_counter_negative_enabled(env, sprn)) {
>>           return -1;
>> @@ -205,13 +272,23 @@ static int64_t get_counter_neg_timeout(CPUPPCState *env, int sprn)
>>       case SPR_POWER_PMC2:
>>       case SPR_POWER_PMC3:
>>       case SPR_POWER_PMC4:
>> -        switch (get_PMC_event(env, sprn)) {
>> +        event = get_PMC_event(env, sprn);
>> +
>> +        switch (event) {
>>           case 0x2:
>>               timeout = get_INST_CMPL_timeout(env, sprn);
>>               break;
>>           case 0x1E:
>>               timeout = get_CYC_timeout(env, sprn);
>>               break;
>> +        case 0xA:
>> +        case 0x6:
>> +        case 0x16:
>> +        case 0x1c:
>> +            timeout = get_stall_timeout(env, sprn, event);
>> +            break;
>> +        default:
>> +            break;
>>           }
>>   
>>           break;
>