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: Enable the high counter first, isb, enable low counter Disable the low counter first, isb, disable the high counter This was the case in Fc. However this is not written anymore in subsequent revions such as Ia. Anyway, just in case, and because it also makes the code a little bit simpler, introduce 2 helpers to enable/disable chain counters that execute those sequences and replace the existing PMCNTENCLR/ENSET calls (at least this cannot do any harm). 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@xxxxxxxxxx> --- v1 -> v2: - fix the enable_chain_counter()/disable_chain_counter() sequence, ie. swap n + 1 / n as reported by Alexandru. - fix an other comment using the 'low' terminology --- arm/pmu.c | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/arm/pmu.c b/arm/pmu.c index 74dd4c10..74c9f6f9 100644 --- a/arm/pmu.c +++ b/arm/pmu.c @@ -731,6 +731,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 + 1), PMCNTENSET_EL0); /* Enable the high counter first */ + isb(); + write_sysreg_s(BIT(even), PMCNTENSET_EL0); /* Enable the low counter */ + isb(); +} + +static void disable_chain_counter(int even) +{ + write_sysreg_s(BIT(even), PMCNTENCLR_EL0); /* Disable the low counter first*/ + isb(); + write_sysreg_s(BIT(even + 1), PMCNTENCLR_EL0); /* Disable the high counter */ + isb(); +} + static void test_chain_promotion(bool unused) { uint32_t events[] = {MEM_ACCESS, CHAIN}; @@ -769,16 +785,17 @@ static void test_chain_promotion(bool unused) /* 1st COUNT with CHAIN enabled, next COUNT with CHAIN disabled */ report_prefix_push("subtest3"); pmu_reset(); - write_sysreg_s(0x3, PMCNTENSET_EL0); write_regn_el0(pmevcntr, 0, PRE_OVERFLOW2_32); - isb(); + enable_chain_counter(0); PRINT_REGS("init"); mem_access_loop(addr, COUNT, pmu.pmcr_ro | PMU_PMCR_E); PRINT_REGS("After 1st loop"); /* disable the CHAIN event */ - write_sysreg_s(0x2, PMCNTENCLR_EL0); + disable_chain_counter(0); + write_sysreg_s(0x1, PMCNTENSET_EL0); /* Enable the low counter */ + isb(); mem_access_loop(addr, COUNT, pmu.pmcr_ro | PMU_PMCR_E); PRINT_REGS("After 2nd loop"); report(read_sysreg(pmovsclr_el0) == 0x1, @@ -799,9 +816,11 @@ static void test_chain_promotion(bool unused) mem_access_loop(addr, COUNT, pmu.pmcr_ro | PMU_PMCR_E); PRINT_REGS("After 1st loop"); - /* enable the CHAIN event */ - write_sysreg_s(0x3, PMCNTENSET_EL0); + /* Disable the low counter first and enable the chain counter */ + write_sysreg_s(0x1, PMCNTENCLR_EL0); isb(); + enable_chain_counter(0); + mem_access_loop(addr, COUNT, pmu.pmcr_ro | PMU_PMCR_E); PRINT_REGS("After 2nd loop"); @@ -825,10 +844,10 @@ static void test_chain_promotion(bool unused) PRINT_REGS("After 1st loop"); /* 0 becomes CHAINED */ - write_sysreg_s(0x0, PMCNTENSET_EL0); + write_sysreg_s(0x3, PMCNTENCLR_EL0); write_regn_el0(pmevtyper, 1, CHAIN | PMEVTYPER_EXCLUDE_EL0); - write_sysreg_s(0x3, PMCNTENSET_EL0); write_regn_el0(pmevcntr, 1, 0x0); + enable_chain_counter(0); mem_access_loop(addr, COUNT, pmu.pmcr_ro | PMU_PMCR_E); PRINT_REGS("After 2nd loop"); @@ -844,13 +863,13 @@ static void test_chain_promotion(bool unused) write_regn_el0(pmevtyper, 0, MEM_ACCESS | PMEVTYPER_EXCLUDE_EL0); write_regn_el0(pmevtyper, 1, CHAIN | PMEVTYPER_EXCLUDE_EL0); write_regn_el0(pmevcntr, 0, PRE_OVERFLOW2_32); - write_sysreg_s(0x3, PMCNTENSET_EL0); + enable_chain_counter(0); PRINT_REGS("init"); mem_access_loop(addr, COUNT, pmu.pmcr_ro | PMU_PMCR_E); PRINT_REGS("After 1st loop"); - write_sysreg_s(0x0, PMCNTENSET_EL0); + disable_chain_counter(0); write_regn_el0(pmevtyper, 1, CPU_CYCLES | PMEVTYPER_EXCLUDE_EL0); write_sysreg_s(0x3, PMCNTENSET_EL0); -- 2.38.1