[kvm-unit-tests PATCH] riscv: sbi: Fix timer delay measurement race

[kvm-unit-tests PATCH] riscv: sbi: Fix timer delay measurement race

[Qingwei Hu]

The timer test samples the counter before checking timer_works in the
wait loop. If the timer interrupt arrives between these operations, the
handler sets timer_works and the loop exits with a timestamp taken before
the interrupt. That timestamp can precede the deadline, causing a false
"timer delay honored" failure in both the normal and masked IRQ tests.

Record the counter at the start of timer_irq_handler() and use it to
calculate the elapsed time. Keep the loop timestamp for timeout detection.

Fixes: 14fa12b1351c ("riscv: sbi: Add test for timer extension")
Signed-off-by: Qingwei Hu <qingwei.hu@bytedance.com>
---
 riscv/sbi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/riscv/sbi.c b/riscv/sbi.c
index 3b8aadce..a298228b 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -245,6 +245,7 @@ struct timer_info {
 	bool timer_irq_set;
 	bool timer_irq_cleared;
 	unsigned long timer_irq_count;
+	unsigned long irq_cycles;
 };
 
 static struct timer_info timer_info;
@@ -256,6 +257,7 @@ static bool timer_irq_pending(void)
 
 static void timer_irq_handler(struct pt_regs *regs)
 {
+	timer_info.irq_cycles = timer_get_cycles();
 	timer_info.timer_works = true;
 
 	if (timer_info.timer_irq_count < ULONG_MAX)
@@ -304,7 +306,7 @@ static void timer_check_set_timer(bool mask_timer_irq)
 	}
 
 	if (timer_info.timer_works) {
-		duration = end - begin;
+		duration = timer_info.irq_cycles - begin;
 		report(duration >= d && duration <= (d + margin), "timer delay honored%s", mask_test_str);
 	}
 
-- 
2.39.5

Re: [kvm-unit-tests PATCH] riscv: sbi: Fix timer delay measurement race

[Andrew Jones]

On Fri, Oct 23, 2026 at 11:04:42AM +0800, Qingwei Hu wrote:
> The timer test samples the counter before checking timer_works in the
> wait loop. If the timer interrupt arrives between these operations, the
> handler sets timer_works and the loop exits with a timestamp taken before
> the interrupt. That timestamp can precede the deadline, causing a false
> "timer delay honored" failure in both the normal and masked IRQ tests.
> 
> Record the counter at the start of timer_irq_handler() and use it to
> calculate the elapsed time. Keep the loop timestamp for timeout detection.
> 
> Fixes: 14fa12b1351c ("riscv: sbi: Add test for timer extension")
> Signed-off-by: Qingwei Hu <qingwei.hu@bytedance.com>
> ---
>  riscv/sbi.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/riscv/sbi.c b/riscv/sbi.c
> index 3b8aadce..a298228b 100644
> --- a/riscv/sbi.c
> +++ b/riscv/sbi.c
> @@ -245,6 +245,7 @@ struct timer_info {
>  	bool timer_irq_set;
>  	bool timer_irq_cleared;
>  	unsigned long timer_irq_count;
> +	unsigned long irq_cycles;
>  };
>  
>  static struct timer_info timer_info;
> @@ -256,6 +257,7 @@ static bool timer_irq_pending(void)
>  
>  static void timer_irq_handler(struct pt_regs *regs)
>  {
> +	timer_info.irq_cycles = timer_get_cycles();
>  	timer_info.timer_works = true;
>  
>  	if (timer_info.timer_irq_count < ULONG_MAX)
> @@ -304,7 +306,7 @@ static void timer_check_set_timer(bool mask_timer_irq)
>  	}
>  
>  	if (timer_info.timer_works) {
> -		duration = end - begin;
> +		duration = timer_info.irq_cycles - begin;
>  		report(duration >= d && duration <= (d + margin), "timer delay honored%s", mask_test_str);
>  	}
>  
> -- 
> 2.39.5
> 

Hi Qingwei,

Your patch is fine, but I think we could also just change the loop a bit

    while (!timer_info.timer_works && timer_get_cycles() <= begin + d + margin)
        cpu_relax();
    end = timer_get_cycles();

Thanks,
drew

Re: [External] [kvm-unit-tests PATCH] riscv: sbi: Fix timer delay measurement race

[Qingwei Hu]

> On Oct 23, 2026, at 18:13, Andrew Jones <andrew.jones@linux.dev> wrote:
> 
> On Fri, Oct 23, 2026 at 11:04:42AM +0800, Qingwei Hu wrote:
>> The timer test samples the counter before checking timer_works in the
>> wait loop. If the timer interrupt arrives between these operations, the
>> handler sets timer_works and the loop exits with a timestamp taken before
>> the interrupt. That timestamp can precede the deadline, causing a false
>> "timer delay honored" failure in both the normal and masked IRQ tests.
>> 
>> Record the counter at the start of timer_irq_handler() and use it to
>> calculate the elapsed time. Keep the loop timestamp for timeout detection.
>> 
>> Fixes: 14fa12b1351c ("riscv: sbi: Add test for timer extension")
>> Signed-off-by: Qingwei Hu <qingwei.hu@bytedance.com>
>> ---
>> riscv/sbi.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/riscv/sbi.c b/riscv/sbi.c
>> index 3b8aadce..a298228b 100644
>> --- a/riscv/sbi.c
>> +++ b/riscv/sbi.c
>> @@ -245,6 +245,7 @@ struct timer_info {
>> bool timer_irq_set;
>> bool timer_irq_cleared;
>> unsigned long timer_irq_count;
>> + unsigned long irq_cycles;
>> };
>> 
>> static struct timer_info timer_info;
>> @@ -256,6 +257,7 @@ static bool timer_irq_pending(void)
>> 
>> static void timer_irq_handler(struct pt_regs *regs)
>> {
>> + timer_info.irq_cycles = timer_get_cycles();
>> timer_info.timer_works = true;
>> 
>> if (timer_info.timer_irq_count < ULONG_MAX)
>> @@ -304,7 +306,7 @@ static void timer_check_set_timer(bool mask_timer_irq)
>> }
>> 
>> if (timer_info.timer_works) {
>> - duration = end - begin;
>> + duration = timer_info.irq_cycles - begin;
>> report(duration >= d && duration <= (d + margin), "timer delay honored%s", mask_test_str);
>> }
>> 
>> -- 
>> 2.39.5
>> 
> 
> Hi Qingwei,
> 
> Your patch is fine, but I think we could also just change the loop a bit
> 
>    while (!timer_info.timer_works && timer_get_cycles() <= begin + d + margin)
>        cpu_relax();
>    end = timer_get_cycles();
> 
> Thanks,
> drew

Hi Drew,

Thanks for this tip. I have tested that the recommended code is useful.

I'll send a v2 with your suggested change.

Thanks,
Qingwei