> On 13-Aug-2021, at 9:54 AM, Nicholas Piggin <npiggin@xxxxxxxxx> wrote: > > Excerpts from Athira Rajeev's message of August 9, 2021 1:03 pm: >> >> >>> On 26-Jul-2021, at 9:19 AM, Nicholas Piggin <npiggin@xxxxxxxxx> 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 <atrajeev@xxxxxxxxxxxxxxxxxx> Thanks Athira > If we can move this code into perf/ > it should become easier for you to tweak things. > > Thanks, > Nick