Re: [kvm-unit-tests PATCH 4/6] arm: pmu: Fix chain counter enable/disable sequences

[Marc Zyngier <maz@kernel.org>]

On 2023-04-21 11:52, Alexandru Elisei wrote:
> Hi,
> 
> On Wed, Mar 15, 2023 at 12:07:23PM +0100, Eric Auger wrote:
>> In some ARM ARM ddi0487 revisions it is said that
>> disabling/enabling a pair of counters that are paired
>> by a CHAIN event should follow a given sequence:
>> 
>> Disable the low counter first, isb, disable the high counter
>> Enable the high counter first, isb, enable low counter
>> 
>> This was the case in Fc. However this is not written anymore
>> in Ia revision.
>> 
>> Introduce 2 helpers to execute those sequences and replace
>> the existing PMCNTENCLR/ENSET calls.
>> 
>> Also fix 2 write_sysreg_s(0x0, PMCNTENSET_EL0) in subtest 5 & 6
>> and replace them by PMCNTENCLR writes since writing 0 in
>> PMCNTENSET_EL0 has no effect.
>> 
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>>  arm/pmu.c | 37 ++++++++++++++++++++++++++++---------
>>  1 file changed, 28 insertions(+), 9 deletions(-)
>> 
>> diff --git a/arm/pmu.c b/arm/pmu.c
>> index dde399e2..af679667 100644
>> --- a/arm/pmu.c
>> +++ b/arm/pmu.c
>> @@ -730,6 +730,22 @@ static void test_chained_sw_incr(bool unused)
>>  		    read_regn_el0(pmevcntr, 0), \
>>  		    read_sysreg(pmovsclr_el0))
>> 
>> +static void enable_chain_counter(int even)
>> +{
>> +	write_sysreg_s(BIT(even), PMCNTENSET_EL0); /* Enable the high 
>> counter first */
>> +	isb();
>> +	write_sysreg_s(BIT(even + 1), PMCNTENSET_EL0); /* Enable the low 
>> counter */
>> +	isb();
>> +}
> 
> In ARM DDI 0487F.b, at the bottom of page D7-2727:
> 
> "When enabling a pair of counters that are paired by a CHAIN event,
> software must:
> 
> 1. Enable the high counter, by setting PMCNTENCLR_EL0[n+1] to 0 and, if
> necessary, setting PMCR_EL0.E to 1.
> 2. Execute an ISB instruction, or perform another Context 
> synchronization
> event.
> 3. Enable the low counter by setting PMCNTENCLR_EL0[n] to 0."

This particular text seems to have been removed from the H.a and I.a
revisions of the ARM ARM, and I cannot spot any equivalent requirement.

         M.
-- 
Jazz is not dead. It just smells funny...