Re: [PATCH v3 0/6] KVM: LoongArch: selftests: Add timer test case

[Bibo Mao]

On 2025/11/27 下午2:42, Huacai Chen wrote:
> On Thu, Nov 27, 2025 at 2:21 PM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
> > On 2025/11/27 上午10:51, Huacai Chen wrote:
> > > On Thu, Nov 27, 2025 at 10:48 AM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
> > > > On 2025/11/27 上午10:45, Huacai Chen wrote:
> > > > > On Thu, Nov 27, 2025 at 10:37 AM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
> > > > > > On 2025/11/27 上午10:09, Huacai Chen wrote:
> > > > > > > On Thu, Nov 27, 2025 at 9:08 AM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
> > > > > > > > On 2025/11/26 下午9:43, Huacai Chen wrote:
> > > > > > > > > On Mon, Nov 24, 2025 at 10:17 AM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
> > > > > > > > > > On 2025/11/24 上午10:03, Huacai Chen wrote:
> > > > > > > > > > > On Mon, Nov 24, 2025 at 9:58 AM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
> > > > > > > > > > > > On 2025/11/21 下午10:08, Huacai Chen wrote:
> > > > > > > > > > > > > Hi, Bibo,
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Thu, Nov 20, 2025 at 2:58 PM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
> > > > > > > > > > > > > > This patchset adds timer test case for LoongArch system, it is based
> > > > > > > > > > > > > > on common arch_timer test case. And it includes one-shot and period mode
> > > > > > > > > > > > > > timer interrupt test, software emulated timer function and time counter
> > > > > > > > > > > > > > test.
> > > > > > > > > > > > > I test this series on top of 6.18-rc6 with Loongson-3A5000, sometimes
> > > > > > > > > > > > > it passes, sometimes I get:
> > > > > > > > > > > > > [root@fedora kvm]# ./arch_timer
> > > > > > > > > > > > > Random seed: 0x6b8b4567
> > > > > > > > > > > > > Guest assert failed,  vcpu 2; stage; 0; iter: 1
> > > > > > > > > > > > > ==== Test Assertion Failure ====
> > > > > > > > > > > > >          loongarch/arch_timer.c:79: irq_iter == 0
> > > > > > > > > > > > >          pid=60138 tid=60142 errno=4 - Interrupted system call
> > > > > > > > > > > > >             1  0x00000001200037cf: test_vcpu_run 于 arch_timer.c:70
> > > > > > > > > > > > >             2  0x00007ffff2449f27: ?? ??:0
> > > > > > > > > > > > >             3  0x00007ffff24c0633: ?? ??:0
> > > > > > > > > > > > >          irq_iter = 0x1.
> > > > > > > > > > > > >          Guest period timer interrupt was not triggered within the specified
> > > > > > > > > > > > >          interval, try to increase the error margin by [-e] option.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Is this as expected, or something is wrong?
> > > > > > > > > > > > There is problem with that. In generic the vCPU task is rescheduled on
> > > > > > > > > > > > other CPUs or preempted, so period timer interrupt is not handled in
> > > > > > > > > > > > specified time.
> > > > > > > > > > > Then this series need to be updated, or problem comes from other places?
> > > > > > > > > > I think this series need be updated, test success criteria with period
> > > > > > > > > > timer need consider this situation. Let me check how to handle this.
> > > > > > > > > Any updates available?
> > > > > > > > It can be solved by modifying udelay() method with get_cycles() or using
> > > > > > > > cpu loop calculation method.
> > > > > > > >
> > > > > > > > --- a/tools/testing/selftests/kvm/include/loongarch/arch_timer.h
> > > > > > > > +++ b/tools/testing/selftests/kvm/include/loongarch/arch_timer.hYes, no common part for it, but it can be a common problem. If other
> > > > architectures have problems they should also modify their own
> > > > __delay(), right?
> > > > > > > > @@ -71,10 +71,17 @@ static inline void timer_irq_disable(void)
> > > > > > > >
> > > > > > > >       static inline void __delay(uint64_t cycles)
> > > > > > > >       {
> > > > > > > > -       uint64_t start = timer_get_cycles();
> > > > > > > > -
> > > > > > > > -       while ((timer_get_cycles() - start) < cycles)
> > > > > > > > -               cpu_relax();
> > > > > > > > +       uint64_t start, next, loops = 0;
> > > > > > > > +
> > > > > > > > +       start = timer_get_cycles();
> > > > > > > > +       while (loops < cycles) {
> > > > > > > > +               next = timer_get_cycles();
> > > > > > > > +               /* only count one cycle if VM is preempted */
> > > > > > > > +               if (next > start) {
> > > > > > > > +                       loops++;
> > > > > > > > +                       start = next;
> > > > > > > > +               }
> > > > > > > > +       }
> > > > > > > >       }
> > > > > > > Looks good. But ARM64 and RISC-V also use a simple implementation of
> > > > > > there is no period test on them.
> > > > > I think the one-shot test can also have this problem if the CPU is
> > > > > preempted for a very long time.
> > > > >
> > > > > > > __delay(). So should this problem be thought of as a common problem?
> > > > > > > If yes, maybe we can keep __delay() as is and wait for the common
> > > > > > > parts to be fixed.
> > > > > > Also there is no common udelay() API, it is arch specific. Someone may
> > > > > > argue that skipping stolen cycles is not generic for __delay(), other
> > > > > > test cases want accurate cycles rather than skipping stolen cycles. It
> > > > > > is timer test case specific.
> > > > > >
> > > > > > Or adding another api __delay_loops() or keep it as is and wait for
> > > > > > other architectures, there should be no common part for it.
> > > > > Yes, no common part for it, but it can be a common problem. If other
> > > > > architectures have problems they should also modify their own
> > > > > __delay(), right?
> > > > yes, what to do then?
> > > Merge window is coming, let's keep it as is. And this problem only
> > > exist when the background load is high (so preemption happens easily),
> > > I think this is not the usual case.
> > Another method is to modify period timer test case, calling udelay with
> > loop times rather than one time, something like this:
> > --- a/tools/testing/selftests/kvm/loongarch/arch_timer.c
> > +++ b/tools/testing/selftests/kvm/loongarch/arch_timer.c
> > @@ -75,7 +75,7 @@ static void guest_test_oneshot_timer(uint32_t cpu)
> >
> >    static void guest_test_period_timer(uint32_t cpu)
> >    {
> > -       uint32_t irq_iter;
> > +       uint32_t irq_iter, config_iter;
> >           uint64_t us;
> >           struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[cpu];
> >
> > @@ -84,7 +84,8 @@ static void guest_test_period_timer(uint32_t cpu)
> >           us = msecs_to_usecs(test_args.timer_period_ms) +
> > test_args.timer_err_margin_us;
> >           timer_set_next_cmp_ms(test_args.timer_period_ms, true);
> >           /* Setup a timeout for the interrupt to arrive */
> > -       udelay(us * test_args.nr_iter);
> > +       for (config_iter = 0; config_iter < test_args.nr_iter;
> > config_iter++)
> > +               udelay(us);
> This can reduce the probability but still cause problem if the
> background CPU load is very high. So I suggest keep it as is.
If merge window is close, one fix can be post in later.

Even if the background CPU load is high, timer interrupt will happen 
every time udelay() is called. So total times of timer interrupt 
triggered will meet the test case requirements.

Regards
Bibo Mao
> Huacai
>
> > Regards
> > Bibo Mao
> > > Huacai
> > >
> > > > > Huacai
> > > > >
> > > > > > Regards
> > > > > > Bibo Mao
> > > > > > > Huacai
> > > > > > >
> > > > > > > > Regards
> > > > > > > > Bibo Mao
> > > > > > > > > Huacai
> > > > > > > > >
> > > > > > > > > > Regards
> > > > > > > > > > Bibo Mao
> > > > > > > > > > > Huacai
> > > > > > > > > > >
> > > > > > > > > > > > Regards
> > > > > > > > > > > > Bibo Mao
> > > > > > > > > > > > > Hucai
> > > > > > > > > > > > >
> > > > > > > > > > > > > > ---
> > > > > > > > > > > > > > v2 ... v3:
> > > > > > > > > > > > > >          1. Adjust order about patch 2 and patch 3
> > > > > > > > > > > > > >          2. Add test case with alphabetical order
> > > > > > > > > > > > > >          3. Merge one-shot and period timer interrupt test case into one
> > > > > > > > > > > > > >          4. Only add LoongArch specific modification with common file
> > > > > > > > > > > > > >             Makefile.kvm
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > v1 ... v2:
> > > > > > > > > > > > > >          1. Restore PC and PRMD after exception handler
> > > > > > > > > > > > > >          2. Split patch 4 into two small patches with period timer test and
> > > > > > > > > > > > > >             time counter test
> > > > > > > > > > > > > >          3. With time counter test, set time count with 0 when create VM. And
> > > > > > > > > > > > > >             verify time count starts from 0 in guest code
> > > > > > > > > > > > > > ---
> > > > > > > > > > > > > > Bibo Mao (6):
> > > > > > > > > > > > > >          KVM: LoongArch: selftests: Add system registers save and restore on
> > > > > > > > > > > > > >            exception
> > > > > > > > > > > > > >          KVM: LoongArch: selftests: Add basic interfaces
> > > > > > > > > > > > > >          KVM: LoongArch: selftests: Add exception handler register interface
> > > > > > > > > > > > > >          KVM: LoongArch: selftests: Add timer interrupt test case
> > > > > > > > > > > > > >          KVM: LoongArch: selftests: Add SW emulated timer test
> > > > > > > > > > > > > >          KVM: LoongArch: selftests: Add time counter test
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >         tools/testing/selftests/kvm/Makefile.kvm      |   1 +
> > > > > > > > > > > > > >         .../kvm/include/loongarch/arch_timer.h        |  84 ++++++++
> > > > > > > > > > > > > >         .../kvm/include/loongarch/processor.h         |  81 +++++++-
> > > > > > > > > > > > > >         .../selftests/kvm/lib/loongarch/exception.S   |   6 +
> > > > > > > > > > > > > >         .../selftests/kvm/lib/loongarch/processor.c   |  47 ++++-
> > > > > > > > > > > > > >         .../selftests/kvm/loongarch/arch_timer.c      | 194 ++++++++++++++++++
> > > > > > > > > > > > > >         6 files changed, 410 insertions(+), 3 deletions(-)
> > > > > > > > > > > > > >         create mode 100644 tools/testing/selftests/kvm/include/loongarch/arch_timer.h
> > > > > > > > > > > > > >         create mode 100644 tools/testing/selftests/kvm/loongarch/arch_timer.c
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > base-commit: 23cb64fb76257309e396ea4cec8396d4a1dbae68
> > > > > > > > > > > > > > --
> > > > > > > > > > > > > > 2.39.3