Re: [PATCH v1 17/55] KVM: PPC: Book3S HV P9: Implement PMU save/restore in C

[Athira Rajeev]

> On 13-Aug-2021, at 9:54 AM, Nicholas Piggin <npig...@gmail.com> wrote:
> 
> Excerpts from Athira Rajeev's message of August 9, 2021 1:03 pm:
>> 
>> 
>>> On 26-Jul-2021, at 9:19 AM, Nicholas Piggin <npig...@gmail.com> wrote:
> 
> 
>>> +static void freeze_pmu(unsigned long mmcr0, unsigned long mmcra)
>>> +{
>>> +   if (!(mmcr0 & MMCR0_FC))
>>> +           goto do_freeze;
>>> +   if (mmcra & MMCRA_SAMPLE_ENABLE)
>>> +           goto do_freeze;
>>> +   if (cpu_has_feature(CPU_FTR_ARCH_31)) {
>>> +           if (!(mmcr0 & MMCR0_PMCCEXT))
>>> +                   goto do_freeze;
>>> +           if (!(mmcra & MMCRA_BHRB_DISABLE))
>>> +                   goto do_freeze;
>>> +   }
>>> +   return;
>>> +
>>> +do_freeze:
>>> +   mmcr0 = MMCR0_FC;
>>> +   mmcra = 0;
>>> +   if (cpu_has_feature(CPU_FTR_ARCH_31)) {
>>> +           mmcr0 |= MMCR0_PMCCEXT;
>>> +           mmcra = MMCRA_BHRB_DISABLE;
>>> +   }
>>> +
>>> +   mtspr(SPRN_MMCR0, mmcr0);
>>> +   mtspr(SPRN_MMCRA, mmcra);
>>> +   isync();
>>> +}
>>> +
>> Hi Nick,
>> 
>> After feezing pmu, do we need to clear “pmcregs_in_use” as well?
> 
> Not until we save the values out of the registers. pmcregs_in_use = 0 
> means our hypervisor is free to clear our PMU register contents.
> 
>> Also can’t we unconditionally do the MMCR0/MMCRA/ freeze settings in here ? 
>> do we need the if conditions for FC/PMCCEXT/BHRB ?
> 
> I think it's possible, but pretty minimal advantage. I would prefer to 
> set them the way perf does for now.

Sure Nick, 

Other changes looks good to me.

Reviewed-by: Athira Rajeev <atraj...@linux.vnet.ibm.com>

Thanks
Athira
> If we can move this code into perf/
> it should become easier for you to tweak things.
> 
> Thanks,
> Nick