On Fri, Sep 8, 2023 at 3:01 AM Andrew Jones ajones@ventanamicro.com wrote:
On Thu, Sep 07, 2023 at 12:20:29PM +0800, Haibo Xu wrote:
On Mon, Sep 4, 2023 at 10:58 PM Andrew Jones ajones@ventanamicro.com wrote:
On Sat, Sep 02, 2023 at 08:59:30PM +0800, Haibo Xu wrote:
Add a KVM selftest to validate the Sstc timer functionality. The test was ported from arm64 arch timer test.
Signed-off-by: Haibo Xu haibo1.xu@intel.com
diff --git a/tools/testing/selftests/kvm/riscv/arch_timer.c b/tools/testing/selftests/kvm/riscv/arch_timer.c new file mode 100644 index 000000000000..c50a33c1e4f9 --- /dev/null +++ b/tools/testing/selftests/kvm/riscv/arch_timer.c @@ -0,0 +1,130 @@ +// SPDX-License-Identifier: GPL-2.0-only +/*
- arch_timer.c - Tests the riscv64 sstc timer IRQ functionality
- The test validates the sstc timer IRQs using vstimecmp registers.
- It's ported from the aarch64 arch_timer test.
guest_run[_stage]() can be shared with aarch64, we just have a single stage=0 for riscv.
Yes, we can. But if we share the guest_run[_stage]() by moving it to kvm/arch_timer.c or kvm/include/timer_test.h, we need to declare extra sub-functions somewhere in a header file(etc. guest_configure_timer_action()).
OK, whatever balances the reduction of duplicate code and avoidance of exporting helper functions. BTW, riscv may not need/want all the same helper functions as aarch64. Anyway, I guess I'll see how the next version turns out.
+static void guest_code(void) +{
uint32_t cpu = guest_get_vcpuid();struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[cpu];local_irq_disable();timer_irq_disable();local_irq_enable();I don't think we need to disable all interrupts when disabling the timer interrupt.
There is no local_irq_disable() protection during the initial debug phase, but the test always fail with below error messages:
Guest assert failed, vcpu 0; stage; 0; iter: 0 ==== Test Assertion Failure ==== riscv/arch_timer.c:78: config_iter + 1 == irq_iter pid=585 tid=586 errno=4 - Interrupted system call (stack trace empty) 0x1 != 0x0 (config_iter + 1 != irq_iter)
To be frank, I am not quite sure why the local_irq_disable/enable() matters. One possible reason may be some timer irq was triggered before we set up the timecmp register.
We should ensure we know the exact, expected state of the vcpu before, during, and after the test. If a state doesn't match expectations, then the test should assert and we should go investigate the test code to see if setup/checking is correct. If it is, then we've found a bug in KVM that we need to go investigate.
For Sstc, a pending timer interrupt completely depends on stimecmp, so we need to watch that closely. Take a look at the attached simple timer test I pulled together to illustrate how stimecmp, timer interrupt enable, and all interrupt enable interact. You may want to use it to help port the arch_timer.
Thanks for sharing the test codes. Will have an investigation on it.
Thanks, drew