diff mbox series

[kvm-unit-tests,v2,13/18] arm64: timer: Test behavior when timer disabled or masked

Message ID 20191128180418.6938-14-alexandru.elisei@arm.com (mailing list archive)
State New, archived
Headers show
Series arm/arm64: Various fixes | expand

Commit Message

Alexandru Elisei Nov. 28, 2019, 6:04 p.m. UTC
When the timer is disabled (the *_CTL_EL0.ENABLE bit is clear) or the
timer interrupt is masked at the timer level (the *_CTL_EL0.IMASK bit is
set), timer interrupts must not be pending or asserted by the VGIC.
However, only when the timer interrupt is masked, we can still check
that the timer condition is met by reading the *_CTL_EL0.ISTATUS bit.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---

This test was used to discover a bug and test the fix introduced by KVM
commit 16e604a437c8 ("KVM: arm/arm64: vgic: Reevaluate level sensitive
interrupts on enable").

 arm/timer.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Andrew Jones Dec. 13, 2019, 6:28 p.m. UTC | #1
On Thu, Nov 28, 2019 at 06:04:13PM +0000, Alexandru Elisei wrote:
> When the timer is disabled (the *_CTL_EL0.ENABLE bit is clear) or the
> timer interrupt is masked at the timer level (the *_CTL_EL0.IMASK bit is
> set), timer interrupts must not be pending or asserted by the VGIC.
> However, only when the timer interrupt is masked, we can still check
> that the timer condition is met by reading the *_CTL_EL0.ISTATUS bit.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
> 
> This test was used to discover a bug and test the fix introduced by KVM
> commit 16e604a437c8 ("KVM: arm/arm64: vgic: Reevaluate level sensitive
> interrupts on enable").

This kind of information can/should go above the ---, IMO.

Thanks,
drew

> 
>  arm/timer.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arm/timer.c b/arm/timer.c
> index d2cd5dc7a58b..09d527bb09a8 100644
> --- a/arm/timer.c
> +++ b/arm/timer.c
> @@ -230,9 +230,17 @@ static void test_timer(struct timer_info *info)
>  
>  	/* Disable the timer again and prepare to take interrupts */
>  	info->write_ctl(0);
> +	isb();
> +	info->irq_received = false;
>  	set_timer_irq_enabled(info, true);
> +	report("no interrupt when timer is disabled", !info->irq_received);
>  	report("interrupt signal no longer pending", !gic_timer_pending(info));
>  
> +	info->write_ctl(ARCH_TIMER_CTL_ENABLE | ARCH_TIMER_CTL_IMASK);
> +	isb();
> +	report("interrupt signal not pending", !gic_timer_pending(info));
> +	report("timer condition met", info->read_ctl() & ARCH_TIMER_CTL_ISTATUS);
> +
>  	report("latency within 10 ms", test_cval_10msec(info));
>  	report("interrupt received", info->irq_received);
>  
> -- 
> 2.20.1
>
Alexandru Elisei Dec. 30, 2019, 9:21 a.m. UTC | #2
Hi,

On 12/13/19 6:28 PM, Andrew Jones wrote:
> On Thu, Nov 28, 2019 at 06:04:13PM +0000, Alexandru Elisei wrote:
>> When the timer is disabled (the *_CTL_EL0.ENABLE bit is clear) or the
>> timer interrupt is masked at the timer level (the *_CTL_EL0.IMASK bit is
>> set), timer interrupts must not be pending or asserted by the VGIC.
>> However, only when the timer interrupt is masked, we can still check
>> that the timer condition is met by reading the *_CTL_EL0.ISTATUS bit.
>>
>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>> ---
>>
>> This test was used to discover a bug and test the fix introduced by KVM
>> commit 16e604a437c8 ("KVM: arm/arm64: vgic: Reevaluate level sensitive
>> interrupts on enable").
> This kind of information can/should go above the ---, IMO.

Sure, I'll do that.

Thanks,
Alex
> Thanks,
> drew
>
>>  arm/timer.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/arm/timer.c b/arm/timer.c
>> index d2cd5dc7a58b..09d527bb09a8 100644
>> --- a/arm/timer.c
>> +++ b/arm/timer.c
>> @@ -230,9 +230,17 @@ static void test_timer(struct timer_info *info)
>>  
>>  	/* Disable the timer again and prepare to take interrupts */
>>  	info->write_ctl(0);
>> +	isb();
>> +	info->irq_received = false;
>>  	set_timer_irq_enabled(info, true);
>> +	report("no interrupt when timer is disabled", !info->irq_received);
>>  	report("interrupt signal no longer pending", !gic_timer_pending(info));
>>  
>> +	info->write_ctl(ARCH_TIMER_CTL_ENABLE | ARCH_TIMER_CTL_IMASK);
>> +	isb();
>> +	report("interrupt signal not pending", !gic_timer_pending(info));
>> +	report("timer condition met", info->read_ctl() & ARCH_TIMER_CTL_ISTATUS);
>> +
>>  	report("latency within 10 ms", test_cval_10msec(info));
>>  	report("interrupt received", info->irq_received);
>>  
>> -- 
>> 2.20.1
>>
diff mbox series

Patch

diff --git a/arm/timer.c b/arm/timer.c
index d2cd5dc7a58b..09d527bb09a8 100644
--- a/arm/timer.c
+++ b/arm/timer.c
@@ -230,9 +230,17 @@  static void test_timer(struct timer_info *info)
 
 	/* Disable the timer again and prepare to take interrupts */
 	info->write_ctl(0);
+	isb();
+	info->irq_received = false;
 	set_timer_irq_enabled(info, true);
+	report("no interrupt when timer is disabled", !info->irq_received);
 	report("interrupt signal no longer pending", !gic_timer_pending(info));
 
+	info->write_ctl(ARCH_TIMER_CTL_ENABLE | ARCH_TIMER_CTL_IMASK);
+	isb();
+	report("interrupt signal not pending", !gic_timer_pending(info));
+	report("timer condition met", info->read_ctl() & ARCH_TIMER_CTL_ISTATUS);
+
 	report("latency within 10 ms", test_cval_10msec(info));
 	report("interrupt received", info->irq_received);