From mboxrd@z Thu Jan  1 00:00:00 1970
From: Nicholas Piggin <npiggin@gmail.com>
Date: Fri, 13 Aug 2021 04:24:19 +0000
Subject: Re: [PATCH v1 17/55] KVM: PPC: Book3S HV P9: Implement PMU save/restore in C
Message-Id: <1628827731.ai2zz7xxwa.astroid@bobo.none>
List-Id: <kvm-ppc.vger.kernel.org>
References: <20210726035036.739609-1-npiggin@gmail.com>
 <20210726035036.739609-18-npiggin@gmail.com>
 <1A47BBEF-FC8C-4C4D-8393-9DE66B7FF96C@linux.vnet.ibm.com>
In-Reply-To: <1A47BBEF-FC8C-4C4D-8393-9DE66B7FF96C@linux.vnet.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
To: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org, kvm-ppc@vger.kernel.org

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

Thanks,
Nick