Messages in this thread |  | | From | Huacai Chen <> | | Date | Thu, 27 Nov 2025 14:54:32 +0800 | | Subject | Re: [PATCH v3 0/6] KVM: LoongArch: selftests: Add timer test case |
| |
On Thu, Nov 27, 2025 at 2:48 PM Bibo Mao <maobibo@loongson.cn> wrote: > > > > On 2025/11/27 下午2:42, Huacai Chen wrote: > > On Thu, Nov 27, 2025 at 2:21 PM Bibo Mao <maobibo@loongson.cn> wrote: > >> > >> > >> > >> On 2025/11/27 上午10:51, Huacai Chen wrote: > >>> On Thu, Nov 27, 2025 at 10:48 AM Bibo Mao <maobibo@loongson.cn> wrote: > >>>> > >>>> > >>>> > >>>> On 2025/11/27 上午10:45, Huacai Chen wrote: > >>>>> On Thu, Nov 27, 2025 at 10:37 AM Bibo Mao <maobibo@loongson.cn> wrote: > >>>>>> > >>>>>> > >>>>>> > >>>>>> On 2025/11/27 上午10:09, Huacai Chen wrote: > >>>>>>> On Thu, Nov 27, 2025 at 9:08 AM Bibo Mao <maobibo@loongson.cn> wrote: > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> On 2025/11/26 下午9:43, Huacai Chen wrote: > >>>>>>>>> On Mon, Nov 24, 2025 at 10:17 AM Bibo Mao <maobibo@loongson.cn> wrote: > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> On 2025/11/24 上午10:03, Huacai Chen wrote: > >>>>>>>>>>> On Mon, Nov 24, 2025 at 9:58 AM Bibo Mao <maobibo@loongson.cn> wrote: > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> On 2025/11/21 下午10:08, Huacai Chen wrote: > >>>>>>>>>>>>> Hi, Bibo, > >>>>>>>>>>>>> > >>>>>>>>>>>>> On Thu, Nov 20, 2025 at 2:58 PM Bibo Mao <maobibo@loongson.cn> 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. I said it only "reduce the probability" because the one-shot test case is also observed errors. If we use "retry method" to fix, then one-shot test case is also needed.
Huacai
> > 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 > >>>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>> > >>>> > >>>> > >> > >> >
|  |