Re: [kvm-unit-tests PATCH v2 2/2] arm: pmu-overflow-interrupt: Increase count values

[Shaoqin Huang <shahuang@redhat.com>]

On 11/14/23 01:42, Eric Auger wrote:
> On some hardware, some pmu-overflow-interrupt failures can be observed.
> Although the even counter overflows, the interrupt is not seen as
> expected. This happens in the subtest after "promote to 64-b" comment.
> After analysis, the PMU overflow interrupt actually hits, ie.
> kvm_pmu_perf_overflow() gets called and KVM_REQ_IRQ_PENDING is set,
> as expected. However the PMCR.E is reset by the handle_exit path, at
> kvm_pmu_handle_pmcr() before the next guest entry and
> kvm_pmu_flush_hwstate/kvm_pmu_update_state subsequent call.
> There, since the enable bit has been reset, kvm_pmu_update_state() does
> not inject the interrupt into the guest.
> 
> This does not seem to be a KVM bug but rather an unfortunate
> scenario where the test disables the PMCR.E too closely to the
> advent of the overflow interrupt.
> 
> Since it looks like a benign and inlikely case, let's resize the number
> of iterations to prevent the PMCR enable bit from being resetted
> immediately at the same time as the actual overflow event.
> 
> COUNT_INT is introduced, arbitrarily set to 1000 iterations and is
> used in this test.
> 
> An alternative would be to let the PMU enabled and wait for the
> interrupt but those extra executions might disturb the counting.
> 
> Reported-by: Jan Richter <jarichte@redhat.com>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Shaoqin Huang <shahuang@redhat.com>
> 
> ---
> 
> v1 -> v2:
> - Only increase mem_access_loop iterations
> ---
>   arm/pmu.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/arm/pmu.c b/arm/pmu.c
> index 86199577..4b388899 100644
> --- a/arm/pmu.c
> +++ b/arm/pmu.c
> @@ -66,6 +66,7 @@
>   #define PRE_OVERFLOW_64		0xFFFFFFFFFFFFFFF0ULL
>   #define COUNT 250
>   #define MARGIN 100
> +#define COUNT_INT 1000
>   /*
>    * PRE_OVERFLOW2 is set so that 1st @COUNT iterations do not
>    * produce 32b overflow and 2nd @COUNT iterations do. To accommodate
> @@ -978,7 +979,7 @@ static void test_overflow_interrupt(bool overflow_at_64bits)
>   
>   	/* interrupts are disabled (PMINTENSET_EL1 == 0) */
>   
> -	mem_access_loop(addr, 200, pmu.pmcr_ro | PMU_PMCR_E | pmcr_lp);
> +	mem_access_loop(addr, COUNT_INT, pmu.pmcr_ro | PMU_PMCR_E | pmcr_lp);
>   	report(expect_interrupts(0), "no overflow interrupt after preset");
>   
>   	set_pmcr(pmu.pmcr_ro | PMU_PMCR_E | pmcr_lp);
> @@ -1002,7 +1003,7 @@ static void test_overflow_interrupt(bool overflow_at_64bits)
>   	write_sysreg(ALL_SET_32, pmintenset_el1);
>   	isb();
>   
> -	mem_access_loop(addr, 200, pmu.pmcr_ro | PMU_PMCR_E | pmcr_lp);
> +	mem_access_loop(addr, COUNT_INT, pmu.pmcr_ro | PMU_PMCR_E | pmcr_lp);
>   
>   	set_pmcr(pmu.pmcr_ro | PMU_PMCR_E | pmcr_lp);
>   	isb();
> @@ -1010,7 +1011,7 @@ static void test_overflow_interrupt(bool overflow_at_64bits)
>   	for (i = 0; i < 100; i++)
>   		write_sysreg(0x3, pmswinc_el0);
>   
> -	mem_access_loop(addr, 200, pmu.pmcr_ro);
> +	mem_access_loop(addr, COUNT_INT, pmu.pmcr_ro);
>   	report_info("overflow=0x%lx", read_sysreg(pmovsclr_el0));
>   	report(expect_interrupts(0x3),
>   		"overflow interrupts expected on #0 and #1");
> @@ -1029,7 +1030,7 @@ static void test_overflow_interrupt(bool overflow_at_64bits)
>   	write_regn_el0(pmevtyper, 1, CHAIN | PMEVTYPER_EXCLUDE_EL0);
>   	write_regn_el0(pmevcntr, 0, pre_overflow);
>   	isb();
> -	mem_access_loop(addr, 200, pmu.pmcr_ro | PMU_PMCR_E | pmcr_lp);
> +	mem_access_loop(addr, COUNT_INT, pmu.pmcr_ro | PMU_PMCR_E | pmcr_lp);
>   	report(expect_interrupts(0x1), "expect overflow interrupt");
>   
>   	/* overflow on odd counter */
> @@ -1037,7 +1038,7 @@ static void test_overflow_interrupt(bool overflow_at_64bits)
>   	write_regn_el0(pmevcntr, 0, pre_overflow);
>   	write_regn_el0(pmevcntr, 1, all_set);
>   	isb();
> -	mem_access_loop(addr, 400, pmu.pmcr_ro | PMU_PMCR_E | pmcr_lp);
> +	mem_access_loop(addr, COUNT_INT, pmu.pmcr_ro | PMU_PMCR_E | pmcr_lp);
>   	if (overflow_at_64bits) {
>   		report(expect_interrupts(0x1),
>   		       "expect overflow interrupt on even counter");

-- 
Shaoqin