RISCV: Add kvm Sstc timer selftest

Rename the vm_init_descriptor_tables() and vcpu_init_vector_tables()

prototypes to vm_init_vector_tables() and vcpu_init_vector_tables() respectively, so that we can use common names for the architectures (x86/aarch64/riscv) and then put them in a common header.

By the way, vm_install_exception_handler() prototype were also moved to the common header since they are commonly used across the architectures.

The patch is a preparation to share the guest exception handling codes in riscv.

Suggested-by Andrew Jones <[email protected]>

Suggested-by Sean Christopherson <[email protected]>

Signed-off-by Haibo Xu <[email protected]>

---
 tools/testing/selftests/kvm/aarch64/arch_timer.c          | 4 ++--
 tools/testing/selftests/kvm/aarch64/debug-exceptions.c    | 4 ++--
 tools/testing/selftests/kvm/aarch64/page_fault_test.c     | 4 ++--
 tools/testing/selftests/kvm/aarch64/vgic_irq.c            | 4 ++--
 tools/testing/selftests/kvm/include/aarch64/processor.h   | 8 +-------
 tools/testing/selftests/kvm/include/kvm_util_base.h       | 7 +++++++
 tools/testing/selftests/kvm/include/x86_64/processor.h    | 5 -----
 tools/testing/selftests/kvm/lib/aarch64/processor.c       | 6 +++---
 tools/testing/selftests/kvm/lib/x86_64/processor.c        | 4 ++--
 tools/testing/selftests/kvm/x86_64/amx_test.c             | 4 ++--
 tools/testing/selftests/kvm/x86_64/fix_hypercall_test.c   | 4 ++--
 tools/testing/selftests/kvm/x86_64/hyperv_evmcs.c         | 4 ++--
 tools/testing/selftests/kvm/x86_64/hyperv_features.c      | 8 ++++----
 tools/testing/selftests/kvm/x86_64/hyperv_ipi.c           | 6 +++---
 tools/testing/selftests/kvm/x86_64/kvm_pv_test.c          | 4 ++--
 tools/testing/selftests/kvm/x86_64/monitor_mwait_test.c   | 4 ++--
 .../testing/selftests/kvm/x86_64/pmu_event_filter_test.c  | 8 ++++----
 .../kvm/x86_64/smaller_maxphyaddr_emulation_test.c        | 4 ++--
 tools/testing/selftests/kvm/x86_64/svm_int_ctl_test.c     | 4 ++--
 .../selftests/kvm/x86_64/svm_nested_shutdown_test.c       | 4 ++--
 .../selftests/kvm/x86_64/svm_nested_soft_inject_test.c    | 4 ++--

 tools/testing/selftests/kvm/x86_64/ucna_injection_test.c  | 8 ++++----
 .../selftests/kvm/x86_64/userspace_msr_exit_test.c        | 4 ++--
 .../kvm/x86_64/vmx_exception_with_invalid_guest_state.c   | 4 ++--
 tools/testing/selftests/kvm/x86_64/vmx_pmu_caps_test.c    | 4 ++--
 tools/testing/selftests/kvm/x86_64/xapic_ipi_test.c       | 4 ++--
 tools/testing/selftests/kvm/x86_64/xcr0_cpuid_test.c      | 4 ++--
 tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c      | 4 ++--
 28 files changed, 66 insertions(+), 70 deletions(-)

Quoted 1 line · Haibo Xu

Rename the vm_init_descriptor_tables() and vcpu_init_vector_tables()

^ vcpu_init_descriptor_tables()

Quoted 35 lines

prototypes to vm_init_vector_tables() and vcpu_init_vector_tables() respectively, so that we can use common names for the architectures (x86/aarch64/riscv) and then put them in a common header.

By the way, vm_install_exception_handler() prototype were also moved to the common header since they are commonly used across the architectures.

The patch is a preparation to share the guest exception handling codes in riscv.

Suggested-by Andrew Jones <[email protected]>

Suggested-by Sean Christopherson <[email protected]>

Signed-off-by Haibo Xu <[email protected]>

---
 tools/testing/selftests/kvm/aarch64/arch_timer.c          | 4 ++--
 tools/testing/selftests/kvm/aarch64/debug-exceptions.c    | 4 ++--
 tools/testing/selftests/kvm/aarch64/page_fault_test.c     | 4 ++--
 tools/testing/selftests/kvm/aarch64/vgic_irq.c            | 4 ++--
 tools/testing/selftests/kvm/include/aarch64/processor.h   | 8 +-------
 tools/testing/selftests/kvm/include/kvm_util_base.h       | 7 +++++++
 tools/testing/selftests/kvm/include/x86_64/processor.h    | 5 -----
 tools/testing/selftests/kvm/lib/aarch64/processor.c       | 6 +++---
 tools/testing/selftests/kvm/lib/x86_64/processor.c        | 4 ++--
 tools/testing/selftests/kvm/x86_64/amx_test.c             | 4 ++--
 tools/testing/selftests/kvm/x86_64/fix_hypercall_test.c   | 4 ++--
 tools/testing/selftests/kvm/x86_64/hyperv_evmcs.c         | 4 ++--
 tools/testing/selftests/kvm/x86_64/hyperv_features.c      | 8 ++++----
 tools/testing/selftests/kvm/x86_64/hyperv_ipi.c           | 6 +++---
 tools/testing/selftests/kvm/x86_64/kvm_pv_test.c          | 4 ++--
 tools/testing/selftests/kvm/x86_64/monitor_mwait_test.c   | 4 ++--
 .../testing/selftests/kvm/x86_64/pmu_event_filter_test.c  | 8 ++++----
 .../kvm/x86_64/smaller_maxphyaddr_emulation_test.c        | 4 ++--
 tools/testing/selftests/kvm/x86_64/svm_int_ctl_test.c     | 4 ++--
 .../selftests/kvm/x86_64/svm_nested_shutdown_test.c       | 4 ++--
 .../selftests/kvm/x86_64/svm_nested_soft_inject_test.c    | 4 ++--

Quoted 9 lines

 tools/testing/selftests/kvm/x86_64/ucna_injection_test.c  | 8 ++++----
 .../selftests/kvm/x86_64/userspace_msr_exit_test.c        | 4 ++--
 .../kvm/x86_64/vmx_exception_with_invalid_guest_state.c   | 4 ++--
 tools/testing/selftests/kvm/x86_64/vmx_pmu_caps_test.c    | 4 ++--
 tools/testing/selftests/kvm/x86_64/xapic_ipi_test.c       | 4 ++--
 tools/testing/selftests/kvm/x86_64/xcr0_cpuid_test.c      | 4 ++--
 tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c      | 4 ++--
 28 files changed, 66 insertions(+), 70 deletions(-)

Reviewed-by Andrew Jones <[email protected]>

Split the arch-neutral test code out of aarch64/arch_timer.c and put them into a common arch_timer.c. This is a preparation to share timer test codes in riscv.

Suggested-by Andrew Jones <[email protected]>

Signed-off-by Haibo Xu <[email protected]>

---
 tools/testing/selftests/kvm/Makefile          |   9 +-
 .../selftests/kvm/aarch64/arch_timer.c        | 288 +-----------------
 tools/testing/selftests/kvm/arch_timer.c      | 252 +++++++++++++++
 .../selftests/kvm/include/timer_test.h        |  52 ++++
 4 files changed, 317 insertions(+), 284 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/arch_timer.c
 create mode 100644 tools/testing/selftests/kvm/include/timer_test.h

tools/testing/selftests/kvm/Makefile +3 -5

index 0b9c42fbce8c..fb8904e2c06a 100644
@@ -140,7 +140,6 @@ TEST_GEN_PROGS_x86_64 += system_counter_offset_test
 TEST_GEN_PROGS_EXTENDED_x86_64 += x86_64/nx_huge_pages_test
 
 TEST_GEN_PROGS_aarch64 += aarch64/aarch32_id_regs
-TEST_GEN_PROGS_aarch64 += aarch64/arch_timer
 TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
 TEST_GEN_PROGS_aarch64 += aarch64/hypercalls
 TEST_GEN_PROGS_aarch64 += aarch64/page_fault_test

@@ -150,6 +149,7 @@ TEST_GEN_PROGS_aarch64 += aarch64/vcpu_width_config
 TEST_GEN_PROGS_aarch64 += aarch64/vgic_init
 TEST_GEN_PROGS_aarch64 += aarch64/vgic_irq
 TEST_GEN_PROGS_aarch64 += access_tracking_perf_test
+TEST_GEN_PROGS_aarch64 += arch_timer
 TEST_GEN_PROGS_aarch64 += demand_paging_test
 TEST_GEN_PROGS_aarch64 += dirty_log_test
 TEST_GEN_PROGS_aarch64 += dirty_log_perf_test

@@ -188,6 +188,7 @@ TEST_GEN_PROGS_riscv += set_memory_region_test
 TEST_GEN_PROGS_riscv += kvm_binary_stats_test
 
 SPLIT_TESTS += get-reg-list
+SPLIT_TESTS += arch_timer
 
 TEST_PROGS += $(TEST_PROGS_$(ARCH_DIR))
 TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(ARCH_DIR))

@@ -248,13 +249,10 @@ TEST_DEP_FILES += $(patsubst %.o, %.d, $(SPLIT_TESTS_OBJS))
 -include $(TEST_DEP_FILES)
 
 $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED): %: %.o
-	$(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $(TARGET_ARCH) $< $(LIBKVM_OBJS) $(LDLIBS) -o $@
+	$(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $(TARGET_ARCH) $^ $(LDLIBS) -o $@
 $(TEST_GEN_OBJ): $(OUTPUT)/%.o: %.c
 	$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
 
-$(SPLIT_TESTS_TARGETS): %: %.o $(SPLIT_TESTS_OBJS)
-	$(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $(TARGET_ARCH) $^ $(LDLIBS) -o $@
-
 EXTRA_CLEAN += $(LIBKVM_OBJS) $(TEST_DEP_FILES) $(TEST_GEN_OBJ) $(SPLIT_TESTS_OBJS) cscope.*
 
 x := $(shell mkdir -p $(sort $(dir $(LIBKVM_C_OBJ) $(LIBKVM_S_OBJ))))

tools/testing/selftests/kvm/Makefile +0 -0

 cscope: include_paths = $(LINUX_TOOL_INCLUDE) $(LINUX_HDR_PATH) include lib ..
 cscope:

tools/testing/selftests/kvm/aarch64/arch_timer.c +9 -279

index b63859829a96..ceb649548751 100644
@@ -1,91 +1,25 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * arch_timer.c - Tests the aarch64 timer IRQ functionality
- *
  * The test validates both the virtual and physical timer IRQs using
- * CVAL and TVAL registers. This consitutes the four stages in the test.
- * The guest's main thread configures the timer interrupt for a stage
- * and waits for it to fire, with a timeout equal to the timer period.
- * It asserts that the timeout doesn't exceed the timer period.
- *
- * On the other hand, upon receipt of an interrupt, the guest's interrupt
- * handler validates the interrupt by checking if the architectural state
- * is in compliance with the specifications.
- *
- * The test provides command-line options to configure the timer's
- * period (-p), number of vCPUs (-n), and iterations per stage (-i).
- * To stress-test the timer stack even more, an option to migrate the
- * vCPUs across pCPUs (-m), at a particular rate, is also provided.
+ * CVAL and TVAL registers.
  *
  * Copyright (c) 2021, Google LLC.
  */
 #define _GNU_SOURCE
 
-#include <stdlib.h>
-#include <pthread.h>
-#include <linux/kvm.h>
-#include <linux/sizes.h>
-#include <linux/bitmap.h>
-#include <sys/sysinfo.h>
-
-#include "kvm_util.h"
-#include "processor.h"
-#include "delay.h"
 #include "arch_timer.h"
+#include "delay.h"
 #include "gic.h"
+#include "processor.h"
+#include "timer_test.h"
 #include "vgic.h"
 
-#define NR_VCPUS_DEF			4
-#define NR_TEST_ITERS_DEF		5
-#define TIMER_TEST_PERIOD_MS_DEF	10
-#define TIMER_TEST_ERR_MARGIN_US	100
-#define TIMER_TEST_MIGRATION_FREQ_MS	2
-
-struct test_args {
-	int nr_vcpus;
-	int nr_iter;
-	int timer_period_ms;
-	int migration_freq_ms;
-	struct kvm_arm_counter_offset offset;
-};
-
-static struct test_args test_args = {
-	.nr_vcpus = NR_VCPUS_DEF,
-	.nr_iter = NR_TEST_ITERS_DEF,
-	.timer_period_ms = TIMER_TEST_PERIOD_MS_DEF,
-	.migration_freq_ms = TIMER_TEST_MIGRATION_FREQ_MS,
-	.offset = { .reserved = 1 },
-};
-
-#define msecs_to_usecs(msec)		((msec) * 1000LL)
-
-#define GICD_BASE_GPA			0x8000000ULL
-#define GICR_BASE_GPA			0x80A0000ULL
-
-enum guest_stage {
-	GUEST_STAGE_VTIMER_CVAL = 1,
-	GUEST_STAGE_VTIMER_TVAL,
-	GUEST_STAGE_PTIMER_CVAL,
-	GUEST_STAGE_PTIMER_TVAL,
-	GUEST_STAGE_MAX,
-};
-
-/* Shared variables between host and guest */
-struct test_vcpu_shared_data {
-	int nr_iter;
-	enum guest_stage guest_stage;
-	uint64_t xcnt;
-};
-
-static struct kvm_vcpu *vcpus[KVM_MAX_VCPUS];
-static pthread_t pt_vcpu_run[KVM_MAX_VCPUS];
-static struct test_vcpu_shared_data vcpu_shared_data[KVM_MAX_VCPUS];
+extern struct test_args test_args;
+extern struct kvm_vcpu *vcpus[];
+extern struct test_vcpu_shared_data vcpu_shared_data[];
 
 static int vtimer_irq, ptimer_irq;
 
-static unsigned long *vcpu_done_map;
-static pthread_mutex_t vcpu_done_map_lock;
-
 static void
 guest_configure_timer_action(struct test_vcpu_shared_data *shared_data)
 {

@@ -222,137 +156,6 @@ static void guest_code(void)
 	GUEST_DONE();
 }
 
-static void *test_vcpu_run(void *arg)
-{
-	unsigned int vcpu_idx = (unsigned long)arg;
-	struct ucall uc;
-	struct kvm_vcpu *vcpu = vcpus[vcpu_idx];
-	struct kvm_vm *vm = vcpu->vm;
-	struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[vcpu_idx];
-
-	vcpu_run(vcpu);
-
-	/* Currently, any exit from guest is an indication of completion */
-	pthread_mutex_lock(&vcpu_done_map_lock);
-	__set_bit(vcpu_idx, vcpu_done_map);
-	pthread_mutex_unlock(&vcpu_done_map_lock);
-
-	switch (get_ucall(vcpu, &uc)) {
-	case UCALL_SYNC:
-	case UCALL_DONE:
-		break;
-	case UCALL_ABORT:
-		sync_global_from_guest(vm, *shared_data);
-		fprintf(stderr, "Guest assert failed,  vcpu %u; stage; %u; iter: %u\n",
-			vcpu_idx, shared_data->guest_stage, shared_data->nr_iter);
-		REPORT_GUEST_ASSERT(uc);
-		break;
-	default:
-		TEST_FAIL("Unexpected guest exit\n");
-	}
-
-	return NULL;
-}
-
-static uint32_t test_get_pcpu(void)
-{
-	uint32_t pcpu;
-	unsigned int nproc_conf;
-	cpu_set_t online_cpuset;
-
-	nproc_conf = get_nprocs_conf();
-	sched_getaffinity(0, sizeof(cpu_set_t), &online_cpuset);
-
-	/* Randomly find an available pCPU to place a vCPU on */
-	do {
-		pcpu = rand() % nproc_conf;
-	} while (!CPU_ISSET(pcpu, &online_cpuset));
-
-	return pcpu;
-}
-
-static int test_migrate_vcpu(unsigned int vcpu_idx)
-{
-	int ret;
-	cpu_set_t cpuset;
-	uint32_t new_pcpu = test_get_pcpu();
-
-	CPU_ZERO(&cpuset);
-	CPU_SET(new_pcpu, &cpuset);
-
-	pr_debug("Migrating vCPU: %u to pCPU: %u\n", vcpu_idx, new_pcpu);
-
-	ret = pthread_setaffinity_np(pt_vcpu_run[vcpu_idx],
-				     sizeof(cpuset), &cpuset);
-
-	/* Allow the error where the vCPU thread is already finished */
-	TEST_ASSERT(ret == 0 || ret == ESRCH,
-		    "Failed to migrate the vCPU:%u to pCPU: %u; ret: %d\n",
-		    vcpu_idx, new_pcpu, ret);
-
-	return ret;
-}
-
-static void *test_vcpu_migration(void *arg)
-{
-	unsigned int i, n_done;
-	bool vcpu_done;
-
-	do {
-		usleep(msecs_to_usecs(test_args.migration_freq_ms));
-
-		for (n_done = 0, i = 0; i < test_args.nr_vcpus; i++) {
-			pthread_mutex_lock(&vcpu_done_map_lock);
-			vcpu_done = test_bit(i, vcpu_done_map);
-			pthread_mutex_unlock(&vcpu_done_map_lock);
-
-			if (vcpu_done) {
-				n_done++;
-				continue;
-			}
-
-			test_migrate_vcpu(i);
-		}
-	} while (test_args.nr_vcpus != n_done);
-
-	return NULL;
-}
-
-static void test_run(struct kvm_vm *vm)
-{
-	pthread_t pt_vcpu_migration;
-	unsigned int i;
-	int ret;
-
-	pthread_mutex_init(&vcpu_done_map_lock, NULL);
-	vcpu_done_map = bitmap_zalloc(test_args.nr_vcpus);
-	TEST_ASSERT(vcpu_done_map, "Failed to allocate vcpu done bitmap\n");
-
-	for (i = 0; i < (unsigned long)test_args.nr_vcpus; i++) {
-		ret = pthread_create(&pt_vcpu_run[i], NULL, test_vcpu_run,
-				     (void *)(unsigned long)i);
-		TEST_ASSERT(!ret, "Failed to create vCPU-%d pthread\n", i);
-	}
-
-	/* Spawn a thread to control the vCPU migrations */
-	if (test_args.migration_freq_ms) {
-		srand(time(NULL));
-
-		ret = pthread_create(&pt_vcpu_migration, NULL,
-					test_vcpu_migration, NULL);
-		TEST_ASSERT(!ret, "Failed to create the migration pthread\n");
-	}
-
-
-	for (i = 0; i < test_args.nr_vcpus; i++)
-		pthread_join(pt_vcpu_run[i], NULL);
-
-	if (test_args.migration_freq_ms)
-		pthread_join(pt_vcpu_migration, NULL);
-
-	bitmap_free(vcpu_done_map);
-}
-
 static void test_init_timer_irq(struct kvm_vm *vm)
 {
 	/* Timer initid should be same for all the vCPUs, so query only vCPU-0 */

@@ -369,7 +172,7 @@ static void test_init_timer_irq(struct kvm_vm *vm)
 
 static int gic_fd;
 
-static struct kvm_vm *test_vm_create(void)
+struct kvm_vm *test_vm_create(void)
 {
 	struct kvm_vm *vm;
 	unsigned int i;

@@ -400,81 +203,8 @@ static struct kvm_vm *test_vm_create(void)
 	return vm;
 }
 
-static void test_vm_cleanup(struct kvm_vm *vm)
+void test_vm_cleanup(struct kvm_vm *vm)
 {
 	close(gic_fd);
 	kvm_vm_free(vm);
 }
-
-static void test_print_help(char *name)
-{
-	pr_info("Usage: %s [-h] [-n nr_vcpus] [-i iterations] [-p timer_period_ms]\n",
-		name);
-	pr_info("\t-n: Number of vCPUs to configure (default: %u; max: %u)\n",
-		NR_VCPUS_DEF, KVM_MAX_VCPUS);
-	pr_info("\t-i: Number of iterations per stage (default: %u)\n",
-		NR_TEST_ITERS_DEF);
-	pr_info("\t-p: Periodicity (in ms) of the guest timer (default: %u)\n",
-		TIMER_TEST_PERIOD_MS_DEF);
-	pr_info("\t-m: Frequency (in ms) of vCPUs to migrate to different pCPU. 0 to turn off (default: %u)\n",
-		TIMER_TEST_MIGRATION_FREQ_MS);
-	pr_info("\t-o: Counter offset (in counter cycles, default: 0)\n");
-	pr_info("\t-h: print this help screen\n");
-}
-
-static bool parse_args(int argc, char *argv[])
-{
-	int opt;
-
-	while ((opt = getopt(argc, argv, "hn:i:p:m:o:")) != -1) {
-		switch (opt) {
-		case 'n':
-			test_args.nr_vcpus = atoi_positive("Number of vCPUs", optarg);
-			if (test_args.nr_vcpus > KVM_MAX_VCPUS) {
-				pr_info("Max allowed vCPUs: %u\n",
-					KVM_MAX_VCPUS);
-				goto err;
-			}
-			break;
-		case 'i':
-			test_args.nr_iter = atoi_positive("Number of iterations", optarg);
-			break;
-		case 'p':
-			test_args.timer_period_ms = atoi_positive("Periodicity", optarg);
-			break;
-		case 'm':
-			test_args.migration_freq_ms = atoi_non_negative("Frequency", optarg);
-			break;
-		case 'o':
-			test_args.offset.counter_offset = strtol(optarg, NULL, 0);
-			test_args.offset.reserved = 0;
-			break;
-		case 'h':
-		default:
-			goto err;
-		}
-	}
-
-	return true;
-
-err:
-	test_print_help(argv[0]);
-	return false;
-}
-
-int main(int argc, char *argv[])
-{
-	struct kvm_vm *vm;
-
-	if (!parse_args(argc, argv))
-		exit(KSFT_SKIP);
-
-	__TEST_REQUIRE(!test_args.migration_freq_ms || get_nprocs() >= 2,
-		       "At least two physical CPUs needed for vCPU migration");
-
-	vm = test_vm_create();
-	test_run(vm);
-	test_vm_cleanup(vm);
-
-	return 0;
-}

tools/testing/selftests/kvm/arch_timer.c new file +0 -0

new file mode 100644
index 000000000000..529024f58c98

tools/testing/selftests/kvm/arch_timer.c new file +23 -0

+ * for it to fire, with a timeout equal to the timer period.
+ * It asserts that the timeout doesn't exceed the timer period.
+ *
+ * On the other hand, upon receipt of an interrupt, the guest's interrupt
+ * handler validates the interrupt by checking if the architectural state
+ * is in compliance with the specifications.
+ *
+ * The test provides command-line options to configure the timer's
+ * period (-p), number of vCPUs (-n), and iterations per stage (-i).
+ * To stress-test the timer stack even more, an option to migrate the
+ * vCPUs across pCPUs (-m), at a particular rate, is also provided.
+ *
+ * Copyright (c) 2021, Google LLC.
+ */
+
+#define _GNU_SOURCE
+
+#include <stdlib.h>
+#include <pthread.h>
+#include <linux/sizes.h>
+#include <linux/bitmap.h>
+#include <sys/sysinfo.h>
+

tools/testing/selftests/kvm/arch_timer.c new file +29 -0

+};
+
+struct kvm_vcpu *vcpus[KVM_MAX_VCPUS];
+struct test_vcpu_shared_data vcpu_shared_data[KVM_MAX_VCPUS];
+
+static pthread_t pt_vcpu_run[KVM_MAX_VCPUS];
+static unsigned long *vcpu_done_map;
+static pthread_mutex_t vcpu_done_map_lock;
+
+static void *test_vcpu_run(void *arg)
+{
+	unsigned int vcpu_idx = (unsigned long)arg;
+	struct ucall uc;
+	struct kvm_vcpu *vcpu = vcpus[vcpu_idx];
+	struct kvm_vm *vm = vcpu->vm;
+	struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[vcpu_idx];
+
+	vcpu_run(vcpu);
+
+	/* Currently, any exit from guest is an indication of completion */
+	pthread_mutex_lock(&vcpu_done_map_lock);
+	__set_bit(vcpu_idx, vcpu_done_map);
+	pthread_mutex_unlock(&vcpu_done_map_lock);
+
+	switch (get_ucall(vcpu, &uc)) {
+	case UCALL_SYNC:
+	case UCALL_DONE:
+		break;
+	case UCALL_ABORT:

tools/testing/selftests/kvm/arch_timer.c new file +175 -0

+
+	return NULL;
+}
+
+static uint32_t test_get_pcpu(void)
+{
+	uint32_t pcpu;
+	unsigned int nproc_conf;
+	cpu_set_t online_cpuset;
+
+	nproc_conf = get_nprocs_conf();
+	sched_getaffinity(0, sizeof(cpu_set_t), &online_cpuset);
+
+	/* Randomly find an available pCPU to place a vCPU on */
+	do {
+		pcpu = rand() % nproc_conf;
+	} while (!CPU_ISSET(pcpu, &online_cpuset));
+
+	return pcpu;
+}
+
+static int test_migrate_vcpu(unsigned int vcpu_idx)
+{
+	int ret;
+	cpu_set_t cpuset;
+	uint32_t new_pcpu = test_get_pcpu();
+
+	CPU_ZERO(&cpuset);
+	CPU_SET(new_pcpu, &cpuset);
+
+	pr_debug("Migrating vCPU: %u to pCPU: %u\n", vcpu_idx, new_pcpu);
+
+	ret = pthread_setaffinity_np(pt_vcpu_run[vcpu_idx],
+				     sizeof(cpuset), &cpuset);
+
+	/* Allow the error where the vCPU thread is already finished */
+	TEST_ASSERT(ret == 0 || ret == ESRCH,
+		    "Failed to migrate the vCPU:%u to pCPU: %u; ret: %d\n",
+		    vcpu_idx, new_pcpu, ret);
+
+	return ret;
+}
+
+static void *test_vcpu_migration(void *arg)
+{
+	unsigned int i, n_done;
+	bool vcpu_done;
+
+	do {
+		usleep(msecs_to_usecs(test_args.migration_freq_ms));
+
+		for (n_done = 0, i = 0; i < test_args.nr_vcpus; i++) {
+			pthread_mutex_lock(&vcpu_done_map_lock);
+			vcpu_done = test_bit(i, vcpu_done_map);
+			pthread_mutex_unlock(&vcpu_done_map_lock);
+
+			if (vcpu_done) {
+				n_done++;
+				continue;
+			}
+
+			test_migrate_vcpu(i);
+		}
+	} while (test_args.nr_vcpus != n_done);
+
+	return NULL;
+}
+
+static void test_run(struct kvm_vm *vm)
+{
+	pthread_t pt_vcpu_migration;
+	unsigned int i;
+	int ret;
+
+	pthread_mutex_init(&vcpu_done_map_lock, NULL);
+	vcpu_done_map = bitmap_zalloc(test_args.nr_vcpus);
+	TEST_ASSERT(vcpu_done_map, "Failed to allocate vcpu done bitmap\n");
+
+	for (i = 0; i < (unsigned long)test_args.nr_vcpus; i++) {
+		ret = pthread_create(&pt_vcpu_run[i], NULL, test_vcpu_run,
+				     (void *)(unsigned long)i);
+		TEST_ASSERT(!ret, "Failed to create vCPU-%d pthread\n", i);
+	}
+
+	/* Spawn a thread to control the vCPU migrations */
+	if (test_args.migration_freq_ms) {
+		srand(time(NULL));
+
+		ret = pthread_create(&pt_vcpu_migration, NULL,
+					test_vcpu_migration, NULL);
+		TEST_ASSERT(!ret, "Failed to create the migration pthread\n");
+	}
+
+
+	for (i = 0; i < test_args.nr_vcpus; i++)
+		pthread_join(pt_vcpu_run[i], NULL);
+
+	if (test_args.migration_freq_ms)
+		pthread_join(pt_vcpu_migration, NULL);
+
+	bitmap_free(vcpu_done_map);
+}
+
+static void test_print_help(char *name)
+{
+	pr_info("Usage: %s [-h] [-n nr_vcpus] [-i iterations] [-p timer_period_ms]\n",
+		name);
+	pr_info("\t-n: Number of vCPUs to configure (default: %u; max: %u)\n",
+		NR_VCPUS_DEF, KVM_MAX_VCPUS);
+	pr_info("\t-i: Number of iterations per stage (default: %u)\n",
+		NR_TEST_ITERS_DEF);
+	pr_info("\t-p: Periodicity (in ms) of the guest timer (default: %u)\n",
+		TIMER_TEST_PERIOD_MS_DEF);
+	pr_info("\t-m: Frequency (in ms) of vCPUs to migrate to different pCPU. 0 to turn off (default: %u)\n",
+		TIMER_TEST_MIGRATION_FREQ_MS);
+	pr_info("\t-o: Counter offset (in counter cycles, default: 0)\n");
+	pr_info("\t-h: print this help screen\n");
+}
+
+static bool parse_args(int argc, char *argv[])
+{
+	int opt;
+
+	while ((opt = getopt(argc, argv, "hn:i:p:m:o:")) != -1) {
+		switch (opt) {
+		case 'n':
+			test_args.nr_vcpus = atoi_positive("Number of vCPUs", optarg);
+			if (test_args.nr_vcpus > KVM_MAX_VCPUS) {
+				pr_info("Max allowed vCPUs: %u\n",
+					KVM_MAX_VCPUS);
+				goto err;
+			}
+			break;
+		case 'i':
+			test_args.nr_iter = atoi_positive("Number of iterations", optarg);
+			break;
+		case 'p':
+			test_args.timer_period_ms = atoi_positive("Periodicity", optarg);
+			break;
+		case 'm':
+			test_args.migration_freq_ms = atoi_non_negative("Frequency", optarg);
+			break;
+		case 'o':
+			test_args.offset.counter_offset = strtol(optarg, NULL, 0);
+			test_args.offset.reserved = 0;
+			break;
+		case 'h':
+		default:
+			goto err;
+		}
+	}
+
+	return true;
+
+err:
+	test_print_help(argv[0]);
+	return false;
+}
+
+int main(int argc, char *argv[])
+{
+	struct kvm_vm *vm;
+
+	if (!parse_args(argc, argv))
+		exit(KSFT_SKIP);
+
+	__TEST_REQUIRE(!test_args.migration_freq_ms || get_nprocs() >= 2,
+		       "At least two physical CPUs needed for vCPU migration");
+
+	vm = test_vm_create();
+	test_run(vm);
+	test_vm_cleanup(vm);
+
+	return 0;
+}

tools/testing/selftests/kvm/include/timer_test.h new file +9 -0

new file mode 100644
index 000000000000..109e4d635627
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * tools/testing/selftests/kvm/include/timer_test.h
+ *
+ * Copyright (C) 2018, Google LLC
+ */
+
+#ifndef SELFTEST_KVM_TIMER_TEST_H
+#define SELFTEST_KVM_TIMER_TEST_H

tools/testing/selftests/kvm/include/timer_test.h new file +9 -0

+
+/* Timer test cmdline parameters */
+struct test_args
+{
+	int nr_vcpus;
+	int nr_iter;
+	int timer_period_ms;
+	int migration_freq_ms;
+	struct kvm_arm_counter_offset offset;

tools/testing/selftests/kvm/include/timer_test.h new file +3 -0

+void test_vm_cleanup(struct kvm_vm *vm);
+
+#endif /* SELFTEST_KVM_TIMER_TEST_H */

Signature · 1 line

2.34.1

Quoted 57 lines · Haibo Xu

Split the arch-neutral test code out of aarch64/arch_timer.c and put them into a common arch_timer.c. This is a preparation to share timer test codes in riscv.

Suggested-by Andrew Jones <[email protected]>

Signed-off-by Haibo Xu <[email protected]>

---
 tools/testing/selftests/kvm/Makefile          |   9 +-
 .../selftests/kvm/aarch64/arch_timer.c        | 288 +-----------------
 tools/testing/selftests/kvm/arch_timer.c      | 252 +++++++++++++++
 .../selftests/kvm/include/timer_test.h        |  52 ++++
 4 files changed, 317 insertions(+), 284 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/arch_timer.c
 create mode 100644 tools/testing/selftests/kvm/include/timer_test.h

tools/testing/selftests/kvm/Makefile +3 -5

index 0b9c42fbce8c..fb8904e2c06a 100644
@@ -140,7 +140,6 @@ TEST_GEN_PROGS_x86_64 += system_counter_offset_test
 TEST_GEN_PROGS_EXTENDED_x86_64 += x86_64/nx_huge_pages_test
 
 TEST_GEN_PROGS_aarch64 += aarch64/aarch32_id_regs
-TEST_GEN_PROGS_aarch64 += aarch64/arch_timer
 TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
 TEST_GEN_PROGS_aarch64 += aarch64/hypercalls
 TEST_GEN_PROGS_aarch64 += aarch64/page_fault_test

@@ -150,6 +149,7 @@ TEST_GEN_PROGS_aarch64 += aarch64/vcpu_width_config
 TEST_GEN_PROGS_aarch64 += aarch64/vgic_init
 TEST_GEN_PROGS_aarch64 += aarch64/vgic_irq
 TEST_GEN_PROGS_aarch64 += access_tracking_perf_test
+TEST_GEN_PROGS_aarch64 += arch_timer
 TEST_GEN_PROGS_aarch64 += demand_paging_test
 TEST_GEN_PROGS_aarch64 += dirty_log_test
 TEST_GEN_PROGS_aarch64 += dirty_log_perf_test

@@ -188,6 +188,7 @@ TEST_GEN_PROGS_riscv += set_memory_region_test
 TEST_GEN_PROGS_riscv += kvm_binary_stats_test
 
 SPLIT_TESTS += get-reg-list
+SPLIT_TESTS += arch_timer
 
 TEST_PROGS += $(TEST_PROGS_$(ARCH_DIR))
 TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(ARCH_DIR))

@@ -248,13 +249,10 @@ TEST_DEP_FILES += $(patsubst %.o, %.d, $(SPLIT_TESTS_OBJS))
 -include $(TEST_DEP_FILES)
 
 $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED): %: %.o
-     $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $(TARGET_ARCH) $< $(LIBKVM_OBJS) $(LDLIBS) -o $@
+     $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $(TARGET_ARCH) $^ $(LDLIBS) -o $@
 $(TEST_GEN_OBJ): $(OUTPUT)/%.o: %.c
      $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
 
-$(SPLIT_TESTS_TARGETS): %: %.o $(SPLIT_TESTS_OBJS)
-     $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $(TARGET_ARCH) $^ $(LDLIBS) -o $@
-
 EXTRA_CLEAN += $(LIBKVM_OBJS) $(TEST_DEP_FILES) $(TEST_GEN_OBJ) $(SPLIT_TESTS_OBJS) cscope.*
 

tools/testing/selftests/kvm/Makefile $(LIBKVM_STRING_OBJ): $(OUTPUT)/%.o: %.c · line 274

Quoted 6 lines

tools/testing/selftests/kvm/Makefile +1 -0

 x := $(shell mkdir -p $(sort $(dir $(LIBKVM_C_OBJ) $(LIBKVM_S_OBJ))))

@@ -273,6 +271,7 @@ $(LIBKVM_STRING_OBJ): $(OUTPUT)/%.o: %.c
 x := $(shell mkdir -p $(sort $(dir $(TEST_GEN_PROGS))))
 $(TEST_GEN_PROGS): $(LIBKVM_OBJS)
 $(TEST_GEN_PROGS_EXTENDED): $(LIBKVM_OBJS)
+$(SPLIT_TESTS_TARGETS): $(OUTPUT)/%: $(ARCH_DIR)/%.o

The improvements to the Makefile to avoid SPLIT_TESTS_TARGETS needing its own $(CC) line should preferably be done in a separate, preliminary patch.

Quoted 342 lines

tools/testing/selftests/kvm/Makefile +0 -0

 cscope: include_paths = $(LINUX_TOOL_INCLUDE) $(LINUX_HDR_PATH) include lib ..
 cscope:

tools/testing/selftests/kvm/aarch64/arch_timer.c +9 -279

index b63859829a96..ceb649548751 100644
@@ -1,91 +1,25 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * arch_timer.c - Tests the aarch64 timer IRQ functionality
- *
  * The test validates both the virtual and physical timer IRQs using
- * CVAL and TVAL registers. This consitutes the four stages in the test.
- * The guest's main thread configures the timer interrupt for a stage
- * and waits for it to fire, with a timeout equal to the timer period.
- * It asserts that the timeout doesn't exceed the timer period.
- *
- * On the other hand, upon receipt of an interrupt, the guest's interrupt
- * handler validates the interrupt by checking if the architectural state
- * is in compliance with the specifications.
- *
- * The test provides command-line options to configure the timer's
- * period (-p), number of vCPUs (-n), and iterations per stage (-i).
- * To stress-test the timer stack even more, an option to migrate the
- * vCPUs across pCPUs (-m), at a particular rate, is also provided.
+ * CVAL and TVAL registers.
  *
  * Copyright (c) 2021, Google LLC.
  */
 #define _GNU_SOURCE
 
-#include <stdlib.h>
-#include <pthread.h>
-#include <linux/kvm.h>
-#include <linux/sizes.h>
-#include <linux/bitmap.h>
-#include <sys/sysinfo.h>
-
-#include "kvm_util.h"
-#include "processor.h"
-#include "delay.h"
 #include "arch_timer.h"
+#include "delay.h"
 #include "gic.h"
+#include "processor.h"
+#include "timer_test.h"
 #include "vgic.h"
 
-#define NR_VCPUS_DEF                 4
-#define NR_TEST_ITERS_DEF            5
-#define TIMER_TEST_PERIOD_MS_DEF     10
-#define TIMER_TEST_ERR_MARGIN_US     100
-#define TIMER_TEST_MIGRATION_FREQ_MS 2
-
-struct test_args {
-     int nr_vcpus;
-     int nr_iter;
-     int timer_period_ms;
-     int migration_freq_ms;
-     struct kvm_arm_counter_offset offset;
-};
-
-static struct test_args test_args = {
-     .nr_vcpus = NR_VCPUS_DEF,
-     .nr_iter = NR_TEST_ITERS_DEF,
-     .timer_period_ms = TIMER_TEST_PERIOD_MS_DEF,
-     .migration_freq_ms = TIMER_TEST_MIGRATION_FREQ_MS,
-     .offset = { .reserved = 1 },
-};
-
-#define msecs_to_usecs(msec)         ((msec) * 1000LL)
-
-#define GICD_BASE_GPA                        0x8000000ULL
-#define GICR_BASE_GPA                        0x80A0000ULL
-
-enum guest_stage {
-     GUEST_STAGE_VTIMER_CVAL = 1,
-     GUEST_STAGE_VTIMER_TVAL,
-     GUEST_STAGE_PTIMER_CVAL,
-     GUEST_STAGE_PTIMER_TVAL,
-     GUEST_STAGE_MAX,
-};
-
-/* Shared variables between host and guest */
-struct test_vcpu_shared_data {
-     int nr_iter;
-     enum guest_stage guest_stage;
-     uint64_t xcnt;
-};
-
-static struct kvm_vcpu *vcpus[KVM_MAX_VCPUS];
-static pthread_t pt_vcpu_run[KVM_MAX_VCPUS];
-static struct test_vcpu_shared_data vcpu_shared_data[KVM_MAX_VCPUS];
+extern struct test_args test_args;
+extern struct kvm_vcpu *vcpus[];
+extern struct test_vcpu_shared_data vcpu_shared_data[];
 
 static int vtimer_irq, ptimer_irq;
 
-static unsigned long *vcpu_done_map;
-static pthread_mutex_t vcpu_done_map_lock;
-
 static void
 guest_configure_timer_action(struct test_vcpu_shared_data *shared_data)
 {

@@ -222,137 +156,6 @@ static void guest_code(void)
      GUEST_DONE();
 }
 
-static void *test_vcpu_run(void *arg)
-{
-     unsigned int vcpu_idx = (unsigned long)arg;
-     struct ucall uc;
-     struct kvm_vcpu *vcpu = vcpus[vcpu_idx];
-     struct kvm_vm *vm = vcpu->vm;
-     struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[vcpu_idx];
-
-     vcpu_run(vcpu);
-
-     /* Currently, any exit from guest is an indication of completion */
-     pthread_mutex_lock(&vcpu_done_map_lock);
-     __set_bit(vcpu_idx, vcpu_done_map);
-     pthread_mutex_unlock(&vcpu_done_map_lock);
-
-     switch (get_ucall(vcpu, &uc)) {
-     case UCALL_SYNC:
-     case UCALL_DONE:
-             break;
-     case UCALL_ABORT:
-             sync_global_from_guest(vm, *shared_data);
-             fprintf(stderr, "Guest assert failed,  vcpu %u; stage; %u; iter: %u\n",
-                     vcpu_idx, shared_data->guest_stage, shared_data->nr_iter);
-             REPORT_GUEST_ASSERT(uc);
-             break;
-     default:
-             TEST_FAIL("Unexpected guest exit\n");
-     }
-
-     return NULL;
-}
-
-static uint32_t test_get_pcpu(void)
-{
-     uint32_t pcpu;
-     unsigned int nproc_conf;
-     cpu_set_t online_cpuset;
-
-     nproc_conf = get_nprocs_conf();
-     sched_getaffinity(0, sizeof(cpu_set_t), &online_cpuset);
-
-     /* Randomly find an available pCPU to place a vCPU on */
-     do {
-             pcpu = rand() % nproc_conf;
-     } while (!CPU_ISSET(pcpu, &online_cpuset));
-
-     return pcpu;
-}
-
-static int test_migrate_vcpu(unsigned int vcpu_idx)
-{
-     int ret;
-     cpu_set_t cpuset;
-     uint32_t new_pcpu = test_get_pcpu();
-
-     CPU_ZERO(&cpuset);
-     CPU_SET(new_pcpu, &cpuset);
-
-     pr_debug("Migrating vCPU: %u to pCPU: %u\n", vcpu_idx, new_pcpu);
-
-     ret = pthread_setaffinity_np(pt_vcpu_run[vcpu_idx],
-                                  sizeof(cpuset), &cpuset);
-
-     /* Allow the error where the vCPU thread is already finished */
-     TEST_ASSERT(ret == 0 || ret == ESRCH,
-                 "Failed to migrate the vCPU:%u to pCPU: %u; ret: %d\n",
-                 vcpu_idx, new_pcpu, ret);
-
-     return ret;
-}
-
-static void *test_vcpu_migration(void *arg)
-{
-     unsigned int i, n_done;
-     bool vcpu_done;
-
-     do {
-             usleep(msecs_to_usecs(test_args.migration_freq_ms));
-
-             for (n_done = 0, i = 0; i < test_args.nr_vcpus; i++) {
-                     pthread_mutex_lock(&vcpu_done_map_lock);
-                     vcpu_done = test_bit(i, vcpu_done_map);
-                     pthread_mutex_unlock(&vcpu_done_map_lock);
-
-                     if (vcpu_done) {
-                             n_done++;
-                             continue;
-                     }
-
-                     test_migrate_vcpu(i);
-             }
-     } while (test_args.nr_vcpus != n_done);
-
-     return NULL;
-}
-
-static void test_run(struct kvm_vm *vm)
-{
-     pthread_t pt_vcpu_migration;
-     unsigned int i;
-     int ret;
-
-     pthread_mutex_init(&vcpu_done_map_lock, NULL);
-     vcpu_done_map = bitmap_zalloc(test_args.nr_vcpus);
-     TEST_ASSERT(vcpu_done_map, "Failed to allocate vcpu done bitmap\n");
-
-     for (i = 0; i < (unsigned long)test_args.nr_vcpus; i++) {
-             ret = pthread_create(&pt_vcpu_run[i], NULL, test_vcpu_run,
-                                  (void *)(unsigned long)i);
-             TEST_ASSERT(!ret, "Failed to create vCPU-%d pthread\n", i);
-     }
-
-     /* Spawn a thread to control the vCPU migrations */
-     if (test_args.migration_freq_ms) {
-             srand(time(NULL));
-
-             ret = pthread_create(&pt_vcpu_migration, NULL,
-                                     test_vcpu_migration, NULL);
-             TEST_ASSERT(!ret, "Failed to create the migration pthread\n");
-     }
-
-
-     for (i = 0; i < test_args.nr_vcpus; i++)
-             pthread_join(pt_vcpu_run[i], NULL);
-
-     if (test_args.migration_freq_ms)
-             pthread_join(pt_vcpu_migration, NULL);
-
-     bitmap_free(vcpu_done_map);
-}
-
 static void test_init_timer_irq(struct kvm_vm *vm)
 {
      /* Timer initid should be same for all the vCPUs, so query only vCPU-0 */

@@ -369,7 +172,7 @@ static void test_init_timer_irq(struct kvm_vm *vm)
 
 static int gic_fd;
 
-static struct kvm_vm *test_vm_create(void)
+struct kvm_vm *test_vm_create(void)
 {
      struct kvm_vm *vm;
      unsigned int i;

@@ -400,81 +203,8 @@ static struct kvm_vm *test_vm_create(void)
      return vm;
 }
 
-static void test_vm_cleanup(struct kvm_vm *vm)
+void test_vm_cleanup(struct kvm_vm *vm)
 {
      close(gic_fd);
      kvm_vm_free(vm);
 }
-
-static void test_print_help(char *name)
-{
-     pr_info("Usage: %s [-h] [-n nr_vcpus] [-i iterations] [-p timer_period_ms]\n",
-             name);
-     pr_info("\t-n: Number of vCPUs to configure (default: %u; max: %u)\n",
-             NR_VCPUS_DEF, KVM_MAX_VCPUS);
-     pr_info("\t-i: Number of iterations per stage (default: %u)\n",
-             NR_TEST_ITERS_DEF);
-     pr_info("\t-p: Periodicity (in ms) of the guest timer (default: %u)\n",
-             TIMER_TEST_PERIOD_MS_DEF);
-     pr_info("\t-m: Frequency (in ms) of vCPUs to migrate to different pCPU. 0 to turn off (default: %u)\n",
-             TIMER_TEST_MIGRATION_FREQ_MS);
-     pr_info("\t-o: Counter offset (in counter cycles, default: 0)\n");
-     pr_info("\t-h: print this help screen\n");
-}
-
-static bool parse_args(int argc, char *argv[])
-{
-     int opt;
-
-     while ((opt = getopt(argc, argv, "hn:i:p:m:o:")) != -1) {
-             switch (opt) {
-             case 'n':
-                     test_args.nr_vcpus = atoi_positive("Number of vCPUs", optarg);
-                     if (test_args.nr_vcpus > KVM_MAX_VCPUS) {
-                             pr_info("Max allowed vCPUs: %u\n",
-                                     KVM_MAX_VCPUS);
-                             goto err;
-                     }
-                     break;
-             case 'i':
-                     test_args.nr_iter = atoi_positive("Number of iterations", optarg);
-                     break;
-             case 'p':
-                     test_args.timer_period_ms = atoi_positive("Periodicity", optarg);
-                     break;
-             case 'm':
-                     test_args.migration_freq_ms = atoi_non_negative("Frequency", optarg);
-                     break;
-             case 'o':
-                     test_args.offset.counter_offset = strtol(optarg, NULL, 0);
-                     test_args.offset.reserved = 0;
-                     break;
-             case 'h':
-             default:
-                     goto err;
-             }
-     }
-
-     return true;
-
-err:
-     test_print_help(argv[0]);
-     return false;
-}
-
-int main(int argc, char *argv[])
-{
-     struct kvm_vm *vm;
-
-     if (!parse_args(argc, argv))
-             exit(KSFT_SKIP);
-
-     __TEST_REQUIRE(!test_args.migration_freq_ms || get_nprocs() >= 2,
-                    "At least two physical CPUs needed for vCPU migration");
-
-     vm = test_vm_create();
-     test_run(vm);
-     test_vm_cleanup(vm);
-
-     return 0;
-}

tools/testing/selftests/kvm/arch_timer.c new file +0 -0

new file mode 100644
index 000000000000..529024f58c98
@@ -0,0 +1,252 @@

tools/testing/selftests/kvm/arch_timer.c line 5

Quoted 5 lines

tools/testing/selftests/kvm/arch_timer.c new file +5 -0

+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * arch_timer.c - Tests the arch timer IRQ functionality
+ *
+ * The guest's main thread configures the timer interrupt for and waits

It looks like the text was edited to remove the 'stage' references, which is fine by me, but the 'for' should have also been removed.

Quoted 27 lines

tools/testing/selftests/kvm/arch_timer.c new file +27 -0

+ * for it to fire, with a timeout equal to the timer period.
+ * It asserts that the timeout doesn't exceed the timer period.
+ *
+ * On the other hand, upon receipt of an interrupt, the guest's interrupt
+ * handler validates the interrupt by checking if the architectural state
+ * is in compliance with the specifications.
+ *
+ * The test provides command-line options to configure the timer's
+ * period (-p), number of vCPUs (-n), and iterations per stage (-i).
+ * To stress-test the timer stack even more, an option to migrate the
+ * vCPUs across pCPUs (-m), at a particular rate, is also provided.
+ *
+ * Copyright (c) 2021, Google LLC.
+ */
+
+#define _GNU_SOURCE
+
+#include <stdlib.h>
+#include <pthread.h>
+#include <linux/sizes.h>
+#include <linux/bitmap.h>
+#include <sys/sysinfo.h>
+
+#include "timer_test.h"
+
+struct test_args test_args = {
+    .nr_vcpus = NR_VCPUS_DEF,

tools/testing/selftests/kvm/arch_timer.c line 38

Quoted 6 lines

tools/testing/selftests/kvm/arch_timer.c new file +6 -0

+    .nr_iter = NR_TEST_ITERS_DEF,
+    .timer_period_ms = TIMER_TEST_PERIOD_MS_DEF,
+    .migration_freq_ms = TIMER_TEST_MIGRATION_FREQ_MS,
+#ifdef __aarch64__
+    .offset = { .reserved = 1 },
+#endif

Please run checkpatch, there are spaces instead of tabs in the struct.

Quoted 34 lines

tools/testing/selftests/kvm/arch_timer.c new file +34 -0

+};
+
+struct kvm_vcpu *vcpus[KVM_MAX_VCPUS];
+struct test_vcpu_shared_data vcpu_shared_data[KVM_MAX_VCPUS];
+
+static pthread_t pt_vcpu_run[KVM_MAX_VCPUS];
+static unsigned long *vcpu_done_map;
+static pthread_mutex_t vcpu_done_map_lock;
+
+static void *test_vcpu_run(void *arg)
+{
+     unsigned int vcpu_idx = (unsigned long)arg;
+     struct ucall uc;
+     struct kvm_vcpu *vcpu = vcpus[vcpu_idx];
+     struct kvm_vm *vm = vcpu->vm;
+     struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[vcpu_idx];
+
+     vcpu_run(vcpu);
+
+     /* Currently, any exit from guest is an indication of completion */
+     pthread_mutex_lock(&vcpu_done_map_lock);
+     __set_bit(vcpu_idx, vcpu_done_map);
+     pthread_mutex_unlock(&vcpu_done_map_lock);
+
+     switch (get_ucall(vcpu, &uc)) {
+     case UCALL_SYNC:
+     case UCALL_DONE:
+             break;
+     case UCALL_ABORT:
+             sync_global_from_guest(vm, *shared_data);
+             fprintf(stderr, "Guest assert failed,  vcpu %u; stage; %u; iter: %u\n",
+                     vcpu_idx, shared_data->guest_stage, shared_data->nr_iter);
+             REPORT_GUEST_ASSERT(uc);
+             break;

tools/testing/selftests/kvm/arch_timer.c line 77

Quoted 5 lines

tools/testing/selftests/kvm/arch_timer.c new file +5 -0

+     default:
+             TEST_FAIL("Unexpected guest exit\n");
+     }
+
+     pr_info("PASS(vCPU-%d).\n", vcpu_idx);

This is new. I can live with it, but generally we don't want to modify functions while moving them.

Quoted 194 lines

tools/testing/selftests/kvm/arch_timer.c new file +175 -0

+
+     return NULL;
+}
+
+static uint32_t test_get_pcpu(void)
+{
+     uint32_t pcpu;
+     unsigned int nproc_conf;
+     cpu_set_t online_cpuset;
+
+     nproc_conf = get_nprocs_conf();
+     sched_getaffinity(0, sizeof(cpu_set_t), &online_cpuset);
+
+     /* Randomly find an available pCPU to place a vCPU on */
+     do {
+             pcpu = rand() % nproc_conf;
+     } while (!CPU_ISSET(pcpu, &online_cpuset));
+
+     return pcpu;
+}
+
+static int test_migrate_vcpu(unsigned int vcpu_idx)
+{
+     int ret;
+     cpu_set_t cpuset;
+     uint32_t new_pcpu = test_get_pcpu();
+
+     CPU_ZERO(&cpuset);
+     CPU_SET(new_pcpu, &cpuset);
+
+     pr_debug("Migrating vCPU: %u to pCPU: %u\n", vcpu_idx, new_pcpu);
+
+     ret = pthread_setaffinity_np(pt_vcpu_run[vcpu_idx],
+                                  sizeof(cpuset), &cpuset);
+
+     /* Allow the error where the vCPU thread is already finished */
+     TEST_ASSERT(ret == 0 || ret == ESRCH,
+                 "Failed to migrate the vCPU:%u to pCPU: %u; ret: %d\n",
+                 vcpu_idx, new_pcpu, ret);
+
+     return ret;
+}
+
+static void *test_vcpu_migration(void *arg)
+{
+     unsigned int i, n_done;
+     bool vcpu_done;
+
+     do {
+             usleep(msecs_to_usecs(test_args.migration_freq_ms));
+
+             for (n_done = 0, i = 0; i < test_args.nr_vcpus; i++) {
+                     pthread_mutex_lock(&vcpu_done_map_lock);
+                     vcpu_done = test_bit(i, vcpu_done_map);
+                     pthread_mutex_unlock(&vcpu_done_map_lock);
+
+                     if (vcpu_done) {
+                             n_done++;
+                             continue;
+                     }
+
+                     test_migrate_vcpu(i);
+             }
+     } while (test_args.nr_vcpus != n_done);
+
+     return NULL;
+}
+
+static void test_run(struct kvm_vm *vm)
+{
+     pthread_t pt_vcpu_migration;
+     unsigned int i;
+     int ret;
+
+     pthread_mutex_init(&vcpu_done_map_lock, NULL);
+     vcpu_done_map = bitmap_zalloc(test_args.nr_vcpus);
+     TEST_ASSERT(vcpu_done_map, "Failed to allocate vcpu done bitmap\n");
+
+     for (i = 0; i < (unsigned long)test_args.nr_vcpus; i++) {
+             ret = pthread_create(&pt_vcpu_run[i], NULL, test_vcpu_run,
+                                  (void *)(unsigned long)i);
+             TEST_ASSERT(!ret, "Failed to create vCPU-%d pthread\n", i);
+     }
+
+     /* Spawn a thread to control the vCPU migrations */
+     if (test_args.migration_freq_ms) {
+             srand(time(NULL));
+
+             ret = pthread_create(&pt_vcpu_migration, NULL,
+                                     test_vcpu_migration, NULL);
+             TEST_ASSERT(!ret, "Failed to create the migration pthread\n");
+     }
+
+
+     for (i = 0; i < test_args.nr_vcpus; i++)
+             pthread_join(pt_vcpu_run[i], NULL);
+
+     if (test_args.migration_freq_ms)
+             pthread_join(pt_vcpu_migration, NULL);
+
+     bitmap_free(vcpu_done_map);
+}
+
+static void test_print_help(char *name)
+{
+     pr_info("Usage: %s [-h] [-n nr_vcpus] [-i iterations] [-p timer_period_ms]\n",
+             name);
+     pr_info("\t-n: Number of vCPUs to configure (default: %u; max: %u)\n",
+             NR_VCPUS_DEF, KVM_MAX_VCPUS);
+     pr_info("\t-i: Number of iterations per stage (default: %u)\n",
+             NR_TEST_ITERS_DEF);
+     pr_info("\t-p: Periodicity (in ms) of the guest timer (default: %u)\n",
+             TIMER_TEST_PERIOD_MS_DEF);
+     pr_info("\t-m: Frequency (in ms) of vCPUs to migrate to different pCPU. 0 to turn off (default: %u)\n",
+             TIMER_TEST_MIGRATION_FREQ_MS);
+     pr_info("\t-o: Counter offset (in counter cycles, default: 0)\n");
+     pr_info("\t-h: print this help screen\n");
+}
+
+static bool parse_args(int argc, char *argv[])
+{
+     int opt;
+
+     while ((opt = getopt(argc, argv, "hn:i:p:m:o:")) != -1) {
+             switch (opt) {
+             case 'n':
+                     test_args.nr_vcpus = atoi_positive("Number of vCPUs", optarg);
+                     if (test_args.nr_vcpus > KVM_MAX_VCPUS) {
+                             pr_info("Max allowed vCPUs: %u\n",
+                                     KVM_MAX_VCPUS);
+                             goto err;
+                     }
+                     break;
+             case 'i':
+                     test_args.nr_iter = atoi_positive("Number of iterations", optarg);
+                     break;
+             case 'p':
+                     test_args.timer_period_ms = atoi_positive("Periodicity", optarg);
+                     break;
+             case 'm':
+                     test_args.migration_freq_ms = atoi_non_negative("Frequency", optarg);
+                     break;
+             case 'o':
+                     test_args.offset.counter_offset = strtol(optarg, NULL, 0);
+                     test_args.offset.reserved = 0;
+                     break;
+             case 'h':
+             default:
+                     goto err;
+             }
+     }
+
+     return true;
+
+err:
+     test_print_help(argv[0]);
+     return false;
+}
+
+int main(int argc, char *argv[])
+{
+     struct kvm_vm *vm;
+
+     if (!parse_args(argc, argv))
+             exit(KSFT_SKIP);
+
+     __TEST_REQUIRE(!test_args.migration_freq_ms || get_nprocs() >= 2,
+                    "At least two physical CPUs needed for vCPU migration");
+
+     vm = test_vm_create();
+     test_run(vm);
+     test_vm_cleanup(vm);
+
+     return 0;
+}

tools/testing/selftests/kvm/include/timer_test.h new file +13 -0

new file mode 100644
index 000000000000..109e4d635627
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * tools/testing/selftests/kvm/include/timer_test.h
+ *
+ * Copyright (C) 2018, Google LLC
+ */
+
+#ifndef SELFTEST_KVM_TIMER_TEST_H
+#define SELFTEST_KVM_TIMER_TEST_H
+
+#include "kvm_util.h"
+
+#define NR_VCPUS_DEF            4

tools/testing/selftests/kvm/include/timer_test.h line 19

Quoted 6 lines

tools/testing/selftests/kvm/include/timer_test.h new file +6 -0

+#define NR_TEST_ITERS_DEF       5
+#define TIMER_TEST_PERIOD_MS_DEF    10
+#define TIMER_TEST_ERR_MARGIN_US    100
+#define TIMER_TEST_MIGRATION_FREQ_MS    2
+
+#define msecs_to_usecs(msec)    ((msec) * 1000LL)

I'd move the above to include/test_util.h

tools/testing/selftests/kvm/include/timer_test.h line 22

Quoted 3 lines

tools/testing/selftests/kvm/include/timer_test.h new file +3 -0

+
+#define GICD_BASE_GPA    0x8000000ULL
+#define GICR_BASE_GPA    0x80A0000ULL

These defines belong in aarch64/arch_timer.c

tools/testing/selftests/kvm/include/timer_test.h line 30

Quoted 8 lines

tools/testing/selftests/kvm/include/timer_test.h new file +8 -0

+
+enum guest_stage {
+     GUEST_STAGE_VTIMER_CVAL=1,
+     GUEST_STAGE_VTIMER_TVAL,
+     GUEST_STAGE_PTIMER_CVAL,
+     GUEST_STAGE_PTIMER_TVAL,
+     GUEST_STAGE_MAX,
+};

This enum also belongs in aarch64/arch_timer.c

Quoted 13 lines

tools/testing/selftests/kvm/include/timer_test.h new file +13 -0

+
+/* Timer test cmdline parameters */
+struct test_args
+{
+     int nr_vcpus;
+     int nr_iter;
+     int timer_period_ms;
+     int migration_freq_ms;
+     struct kvm_arm_counter_offset offset;
+};
+
+/* Shared variables between host and guest */
+struct test_vcpu_shared_data {

tools/testing/selftests/kvm/include/timer_test.h line 49

Quoted 6 lines

tools/testing/selftests/kvm/include/timer_test.h new file +6 -0

+     int nr_iter;
+     enum guest_stage guest_stage;
+     uint64_t xcnt;
+};
+
+struct kvm_vm* test_vm_create(void);

Move * next to function name.

Quoted 6 lines

tools/testing/selftests/kvm/include/timer_test.h new file +3 -1

+void test_vm_cleanup(struct kvm_vm *vm);
+
+#endif /* SELFTEST_KVM_TIMER_TEST_H */
--

2.34.1

Thanks,
drew

Quoted 59 lines · On Mon, Sep 4, 2023 at 9:24?PM Andrew Jones <[email protected]> wrote:

Quoted 57 lines · Haibo Xu

Split the arch-neutral test code out of aarch64/arch_timer.c and put them into a common arch_timer.c. This is a preparation to share timer test codes in riscv.

Suggested-by Andrew Jones <[email protected]>

Signed-off-by Haibo Xu <[email protected]>

---
 tools/testing/selftests/kvm/Makefile          |   9 +-
 .../selftests/kvm/aarch64/arch_timer.c        | 288 +-----------------
 tools/testing/selftests/kvm/arch_timer.c      | 252 +++++++++++++++
 .../selftests/kvm/include/timer_test.h        |  52 ++++
 4 files changed, 317 insertions(+), 284 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/arch_timer.c
 create mode 100644 tools/testing/selftests/kvm/include/timer_test.h

tools/testing/selftests/kvm/Makefile +3 -5

index 0b9c42fbce8c..fb8904e2c06a 100644
@@ -140,7 +140,6 @@ TEST_GEN_PROGS_x86_64 += system_counter_offset_test
 TEST_GEN_PROGS_EXTENDED_x86_64 += x86_64/nx_huge_pages_test
 
 TEST_GEN_PROGS_aarch64 += aarch64/aarch32_id_regs
-TEST_GEN_PROGS_aarch64 += aarch64/arch_timer
 TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
 TEST_GEN_PROGS_aarch64 += aarch64/hypercalls
 TEST_GEN_PROGS_aarch64 += aarch64/page_fault_test

@@ -150,6 +149,7 @@ TEST_GEN_PROGS_aarch64 += aarch64/vcpu_width_config
 TEST_GEN_PROGS_aarch64 += aarch64/vgic_init
 TEST_GEN_PROGS_aarch64 += aarch64/vgic_irq
 TEST_GEN_PROGS_aarch64 += access_tracking_perf_test
+TEST_GEN_PROGS_aarch64 += arch_timer
 TEST_GEN_PROGS_aarch64 += demand_paging_test
 TEST_GEN_PROGS_aarch64 += dirty_log_test
 TEST_GEN_PROGS_aarch64 += dirty_log_perf_test

@@ -188,6 +188,7 @@ TEST_GEN_PROGS_riscv += set_memory_region_test
 TEST_GEN_PROGS_riscv += kvm_binary_stats_test
 
 SPLIT_TESTS += get-reg-list
+SPLIT_TESTS += arch_timer
 
 TEST_PROGS += $(TEST_PROGS_$(ARCH_DIR))
 TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(ARCH_DIR))

@@ -248,13 +249,10 @@ TEST_DEP_FILES += $(patsubst %.o, %.d, $(SPLIT_TESTS_OBJS))
 -include $(TEST_DEP_FILES)
 
 $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED): %: %.o
-     $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $(TARGET_ARCH) $< $(LIBKVM_OBJS) $(LDLIBS) -o $@
+     $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $(TARGET_ARCH) $^ $(LDLIBS) -o $@
 $(TEST_GEN_OBJ): $(OUTPUT)/%.o: %.c
      $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
 
-$(SPLIT_TESTS_TARGETS): %: %.o $(SPLIT_TESTS_OBJS)
-     $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $(TARGET_ARCH) $^ $(LDLIBS) -o $@
-
 EXTRA_CLEAN += $(LIBKVM_OBJS) $(TEST_DEP_FILES) $(TEST_GEN_OBJ) $(SPLIT_TESTS_OBJS) cscope.*
 

Quoted 10 lines

tools/testing/selftests/kvm/Makefile $(LIBKVM_STRING_OBJ): $(OUTPUT)/%.o: %.c · line 274

Quoted 6 lines

tools/testing/selftests/kvm/Makefile +1 -0

 x := $(shell mkdir -p $(sort $(dir $(LIBKVM_C_OBJ) $(LIBKVM_S_OBJ))))

@@ -273,6 +271,7 @@ $(LIBKVM_STRING_OBJ): $(OUTPUT)/%.o: %.c
 x := $(shell mkdir -p $(sort $(dir $(TEST_GEN_PROGS))))
 $(TEST_GEN_PROGS): $(LIBKVM_OBJS)
 $(TEST_GEN_PROGS_EXTENDED): $(LIBKVM_OBJS)
+$(SPLIT_TESTS_TARGETS): $(OUTPUT)/%: $(ARCH_DIR)/%.o

The improvements to the Makefile to avoid SPLIT_TESTS_TARGETS needing its own $(CC) line should preferably be done in a separate, preliminary patch.

Yes, Will move the change to a separate patch.

Quoted 342 lines

Quoted 342 lines

tools/testing/selftests/kvm/Makefile +0 -0

 cscope: include_paths = $(LINUX_TOOL_INCLUDE) $(LINUX_HDR_PATH) include lib ..
 cscope:

tools/testing/selftests/kvm/aarch64/arch_timer.c +9 -279

index b63859829a96..ceb649548751 100644
@@ -1,91 +1,25 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * arch_timer.c - Tests the aarch64 timer IRQ functionality
- *
  * The test validates both the virtual and physical timer IRQs using
- * CVAL and TVAL registers. This consitutes the four stages in the test.
- * The guest's main thread configures the timer interrupt for a stage
- * and waits for it to fire, with a timeout equal to the timer period.
- * It asserts that the timeout doesn't exceed the timer period.
- *
- * On the other hand, upon receipt of an interrupt, the guest's interrupt
- * handler validates the interrupt by checking if the architectural state
- * is in compliance with the specifications.
- *
- * The test provides command-line options to configure the timer's
- * period (-p), number of vCPUs (-n), and iterations per stage (-i).
- * To stress-test the timer stack even more, an option to migrate the
- * vCPUs across pCPUs (-m), at a particular rate, is also provided.
+ * CVAL and TVAL registers.
  *
  * Copyright (c) 2021, Google LLC.
  */
 #define _GNU_SOURCE
 
-#include <stdlib.h>
-#include <pthread.h>
-#include <linux/kvm.h>
-#include <linux/sizes.h>
-#include <linux/bitmap.h>
-#include <sys/sysinfo.h>
-
-#include "kvm_util.h"
-#include "processor.h"
-#include "delay.h"
 #include "arch_timer.h"
+#include "delay.h"
 #include "gic.h"
+#include "processor.h"
+#include "timer_test.h"
 #include "vgic.h"
 
-#define NR_VCPUS_DEF                 4
-#define NR_TEST_ITERS_DEF            5
-#define TIMER_TEST_PERIOD_MS_DEF     10
-#define TIMER_TEST_ERR_MARGIN_US     100
-#define TIMER_TEST_MIGRATION_FREQ_MS 2
-
-struct test_args {
-     int nr_vcpus;
-     int nr_iter;
-     int timer_period_ms;
-     int migration_freq_ms;
-     struct kvm_arm_counter_offset offset;
-};
-
-static struct test_args test_args = {
-     .nr_vcpus = NR_VCPUS_DEF,
-     .nr_iter = NR_TEST_ITERS_DEF,
-     .timer_period_ms = TIMER_TEST_PERIOD_MS_DEF,
-     .migration_freq_ms = TIMER_TEST_MIGRATION_FREQ_MS,
-     .offset = { .reserved = 1 },
-};
-
-#define msecs_to_usecs(msec)         ((msec) * 1000LL)
-
-#define GICD_BASE_GPA                        0x8000000ULL
-#define GICR_BASE_GPA                        0x80A0000ULL
-
-enum guest_stage {
-     GUEST_STAGE_VTIMER_CVAL = 1,
-     GUEST_STAGE_VTIMER_TVAL,
-     GUEST_STAGE_PTIMER_CVAL,
-     GUEST_STAGE_PTIMER_TVAL,
-     GUEST_STAGE_MAX,
-};
-
-/* Shared variables between host and guest */
-struct test_vcpu_shared_data {
-     int nr_iter;
-     enum guest_stage guest_stage;
-     uint64_t xcnt;
-};
-
-static struct kvm_vcpu *vcpus[KVM_MAX_VCPUS];
-static pthread_t pt_vcpu_run[KVM_MAX_VCPUS];
-static struct test_vcpu_shared_data vcpu_shared_data[KVM_MAX_VCPUS];
+extern struct test_args test_args;
+extern struct kvm_vcpu *vcpus[];
+extern struct test_vcpu_shared_data vcpu_shared_data[];
 
 static int vtimer_irq, ptimer_irq;
 
-static unsigned long *vcpu_done_map;
-static pthread_mutex_t vcpu_done_map_lock;
-
 static void
 guest_configure_timer_action(struct test_vcpu_shared_data *shared_data)
 {

@@ -222,137 +156,6 @@ static void guest_code(void)
      GUEST_DONE();
 }
 
-static void *test_vcpu_run(void *arg)
-{
-     unsigned int vcpu_idx = (unsigned long)arg;
-     struct ucall uc;
-     struct kvm_vcpu *vcpu = vcpus[vcpu_idx];
-     struct kvm_vm *vm = vcpu->vm;
-     struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[vcpu_idx];
-
-     vcpu_run(vcpu);
-
-     /* Currently, any exit from guest is an indication of completion */
-     pthread_mutex_lock(&vcpu_done_map_lock);
-     __set_bit(vcpu_idx, vcpu_done_map);
-     pthread_mutex_unlock(&vcpu_done_map_lock);
-
-     switch (get_ucall(vcpu, &uc)) {
-     case UCALL_SYNC:
-     case UCALL_DONE:
-             break;
-     case UCALL_ABORT:
-             sync_global_from_guest(vm, *shared_data);
-             fprintf(stderr, "Guest assert failed,  vcpu %u; stage; %u; iter: %u\n",
-                     vcpu_idx, shared_data->guest_stage, shared_data->nr_iter);
-             REPORT_GUEST_ASSERT(uc);
-             break;
-     default:
-             TEST_FAIL("Unexpected guest exit\n");
-     }
-
-     return NULL;
-}
-
-static uint32_t test_get_pcpu(void)
-{
-     uint32_t pcpu;
-     unsigned int nproc_conf;
-     cpu_set_t online_cpuset;
-
-     nproc_conf = get_nprocs_conf();
-     sched_getaffinity(0, sizeof(cpu_set_t), &online_cpuset);
-
-     /* Randomly find an available pCPU to place a vCPU on */
-     do {
-             pcpu = rand() % nproc_conf;
-     } while (!CPU_ISSET(pcpu, &online_cpuset));
-
-     return pcpu;
-}
-
-static int test_migrate_vcpu(unsigned int vcpu_idx)
-{
-     int ret;
-     cpu_set_t cpuset;
-     uint32_t new_pcpu = test_get_pcpu();
-
-     CPU_ZERO(&cpuset);
-     CPU_SET(new_pcpu, &cpuset);
-
-     pr_debug("Migrating vCPU: %u to pCPU: %u\n", vcpu_idx, new_pcpu);
-
-     ret = pthread_setaffinity_np(pt_vcpu_run[vcpu_idx],
-                                  sizeof(cpuset), &cpuset);
-
-     /* Allow the error where the vCPU thread is already finished */
-     TEST_ASSERT(ret == 0 || ret == ESRCH,
-                 "Failed to migrate the vCPU:%u to pCPU: %u; ret: %d\n",
-                 vcpu_idx, new_pcpu, ret);
-
-     return ret;
-}
-
-static void *test_vcpu_migration(void *arg)
-{
-     unsigned int i, n_done;
-     bool vcpu_done;
-
-     do {
-             usleep(msecs_to_usecs(test_args.migration_freq_ms));
-
-             for (n_done = 0, i = 0; i < test_args.nr_vcpus; i++) {
-                     pthread_mutex_lock(&vcpu_done_map_lock);
-                     vcpu_done = test_bit(i, vcpu_done_map);
-                     pthread_mutex_unlock(&vcpu_done_map_lock);
-
-                     if (vcpu_done) {
-                             n_done++;
-                             continue;
-                     }
-
-                     test_migrate_vcpu(i);
-             }
-     } while (test_args.nr_vcpus != n_done);
-
-     return NULL;
-}
-
-static void test_run(struct kvm_vm *vm)
-{
-     pthread_t pt_vcpu_migration;
-     unsigned int i;
-     int ret;
-
-     pthread_mutex_init(&vcpu_done_map_lock, NULL);
-     vcpu_done_map = bitmap_zalloc(test_args.nr_vcpus);
-     TEST_ASSERT(vcpu_done_map, "Failed to allocate vcpu done bitmap\n");
-
-     for (i = 0; i < (unsigned long)test_args.nr_vcpus; i++) {
-             ret = pthread_create(&pt_vcpu_run[i], NULL, test_vcpu_run,
-                                  (void *)(unsigned long)i);
-             TEST_ASSERT(!ret, "Failed to create vCPU-%d pthread\n", i);
-     }
-
-     /* Spawn a thread to control the vCPU migrations */
-     if (test_args.migration_freq_ms) {
-             srand(time(NULL));
-
-             ret = pthread_create(&pt_vcpu_migration, NULL,
-                                     test_vcpu_migration, NULL);
-             TEST_ASSERT(!ret, "Failed to create the migration pthread\n");
-     }
-
-
-     for (i = 0; i < test_args.nr_vcpus; i++)
-             pthread_join(pt_vcpu_run[i], NULL);
-
-     if (test_args.migration_freq_ms)
-             pthread_join(pt_vcpu_migration, NULL);
-
-     bitmap_free(vcpu_done_map);
-}
-
 static void test_init_timer_irq(struct kvm_vm *vm)
 {
      /* Timer initid should be same for all the vCPUs, so query only vCPU-0 */

@@ -369,7 +172,7 @@ static void test_init_timer_irq(struct kvm_vm *vm)
 
 static int gic_fd;
 
-static struct kvm_vm *test_vm_create(void)
+struct kvm_vm *test_vm_create(void)
 {
      struct kvm_vm *vm;
      unsigned int i;

@@ -400,81 +203,8 @@ static struct kvm_vm *test_vm_create(void)
      return vm;
 }
 
-static void test_vm_cleanup(struct kvm_vm *vm)
+void test_vm_cleanup(struct kvm_vm *vm)
 {
      close(gic_fd);
      kvm_vm_free(vm);
 }
-
-static void test_print_help(char *name)
-{
-     pr_info("Usage: %s [-h] [-n nr_vcpus] [-i iterations] [-p timer_period_ms]\n",
-             name);
-     pr_info("\t-n: Number of vCPUs to configure (default: %u; max: %u)\n",
-             NR_VCPUS_DEF, KVM_MAX_VCPUS);
-     pr_info("\t-i: Number of iterations per stage (default: %u)\n",
-             NR_TEST_ITERS_DEF);
-     pr_info("\t-p: Periodicity (in ms) of the guest timer (default: %u)\n",
-             TIMER_TEST_PERIOD_MS_DEF);
-     pr_info("\t-m: Frequency (in ms) of vCPUs to migrate to different pCPU. 0 to turn off (default: %u)\n",
-             TIMER_TEST_MIGRATION_FREQ_MS);
-     pr_info("\t-o: Counter offset (in counter cycles, default: 0)\n");
-     pr_info("\t-h: print this help screen\n");
-}
-
-static bool parse_args(int argc, char *argv[])
-{
-     int opt;
-
-     while ((opt = getopt(argc, argv, "hn:i:p:m:o:")) != -1) {
-             switch (opt) {
-             case 'n':
-                     test_args.nr_vcpus = atoi_positive("Number of vCPUs", optarg);
-                     if (test_args.nr_vcpus > KVM_MAX_VCPUS) {
-                             pr_info("Max allowed vCPUs: %u\n",
-                                     KVM_MAX_VCPUS);
-                             goto err;
-                     }
-                     break;
-             case 'i':
-                     test_args.nr_iter = atoi_positive("Number of iterations", optarg);
-                     break;
-             case 'p':
-                     test_args.timer_period_ms = atoi_positive("Periodicity", optarg);
-                     break;
-             case 'm':
-                     test_args.migration_freq_ms = atoi_non_negative("Frequency", optarg);
-                     break;
-             case 'o':
-                     test_args.offset.counter_offset = strtol(optarg, NULL, 0);
-                     test_args.offset.reserved = 0;
-                     break;
-             case 'h':
-             default:
-                     goto err;
-             }
-     }
-
-     return true;
-
-err:
-     test_print_help(argv[0]);
-     return false;
-}
-
-int main(int argc, char *argv[])
-{
-     struct kvm_vm *vm;
-
-     if (!parse_args(argc, argv))
-             exit(KSFT_SKIP);
-
-     __TEST_REQUIRE(!test_args.migration_freq_ms || get_nprocs() >= 2,
-                    "At least two physical CPUs needed for vCPU migration");
-
-     vm = test_vm_create();
-     test_run(vm);
-     test_vm_cleanup(vm);
-
-     return 0;
-}

tools/testing/selftests/kvm/arch_timer.c new file +0 -0

new file mode 100644
index 000000000000..529024f58c98
@@ -0,0 +1,252 @@

Quoted 10 lines

tools/testing/selftests/kvm/arch_timer.c line 5

Quoted 5 lines

tools/testing/selftests/kvm/arch_timer.c new file +5 -0

+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * arch_timer.c - Tests the arch timer IRQ functionality
+ *
+ * The guest's main thread configures the timer interrupt for and waits

It looks like the text was edited to remove the 'stage' references, which is fine by me, but the 'for' should have also been removed.

Sure!

Quoted 27 lines

Quoted 27 lines

tools/testing/selftests/kvm/arch_timer.c new file +27 -0

+ * for it to fire, with a timeout equal to the timer period.
+ * It asserts that the timeout doesn't exceed the timer period.
+ *
+ * On the other hand, upon receipt of an interrupt, the guest's interrupt
+ * handler validates the interrupt by checking if the architectural state
+ * is in compliance with the specifications.
+ *
+ * The test provides command-line options to configure the timer's
+ * period (-p), number of vCPUs (-n), and iterations per stage (-i).
+ * To stress-test the timer stack even more, an option to migrate the
+ * vCPUs across pCPUs (-m), at a particular rate, is also provided.
+ *
+ * Copyright (c) 2021, Google LLC.
+ */
+
+#define _GNU_SOURCE
+
+#include <stdlib.h>
+#include <pthread.h>
+#include <linux/sizes.h>
+#include <linux/bitmap.h>
+#include <sys/sysinfo.h>
+
+#include "timer_test.h"
+
+struct test_args test_args = {
+    .nr_vcpus = NR_VCPUS_DEF,

Quoted 9 lines

tools/testing/selftests/kvm/arch_timer.c line 38

Quoted 6 lines

tools/testing/selftests/kvm/arch_timer.c new file +6 -0

+    .nr_iter = NR_TEST_ITERS_DEF,
+    .timer_period_ms = TIMER_TEST_PERIOD_MS_DEF,
+    .migration_freq_ms = TIMER_TEST_MIGRATION_FREQ_MS,
+#ifdef __aarch64__
+    .offset = { .reserved = 1 },
+#endif

Please run checkpatch, there are spaces instead of tabs in the struct.

Yes, the tabs were changed to spaces while copying. Will change it and run checkpatch to catch this kind of error in next version.

Quoted 34 lines

Quoted 34 lines

tools/testing/selftests/kvm/arch_timer.c new file +34 -0

+};
+
+struct kvm_vcpu *vcpus[KVM_MAX_VCPUS];
+struct test_vcpu_shared_data vcpu_shared_data[KVM_MAX_VCPUS];
+
+static pthread_t pt_vcpu_run[KVM_MAX_VCPUS];
+static unsigned long *vcpu_done_map;
+static pthread_mutex_t vcpu_done_map_lock;
+
+static void *test_vcpu_run(void *arg)
+{
+     unsigned int vcpu_idx = (unsigned long)arg;
+     struct ucall uc;
+     struct kvm_vcpu *vcpu = vcpus[vcpu_idx];
+     struct kvm_vm *vm = vcpu->vm;
+     struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[vcpu_idx];
+
+     vcpu_run(vcpu);
+
+     /* Currently, any exit from guest is an indication of completion */
+     pthread_mutex_lock(&vcpu_done_map_lock);
+     __set_bit(vcpu_idx, vcpu_done_map);
+     pthread_mutex_unlock(&vcpu_done_map_lock);
+
+     switch (get_ucall(vcpu, &uc)) {
+     case UCALL_SYNC:
+     case UCALL_DONE:
+             break;
+     case UCALL_ABORT:
+             sync_global_from_guest(vm, *shared_data);
+             fprintf(stderr, "Guest assert failed,  vcpu %u; stage; %u; iter: %u\n",
+                     vcpu_idx, shared_data->guest_stage, shared_data->nr_iter);
+             REPORT_GUEST_ASSERT(uc);
+             break;

Quoted 9 lines

tools/testing/selftests/kvm/arch_timer.c line 77

Quoted 5 lines

tools/testing/selftests/kvm/arch_timer.c new file +5 -0

+     default:
+             TEST_FAIL("Unexpected guest exit\n");
+     }
+
+     pr_info("PASS(vCPU-%d).\n", vcpu_idx);

This is new. I can live with it, but generally we don't want to modify functions while moving them.

Yes, this change was supposed to go with patch 8/8.

Quoted 194 lines

Quoted 194 lines

tools/testing/selftests/kvm/arch_timer.c new file +175 -0

+
+     return NULL;
+}
+
+static uint32_t test_get_pcpu(void)
+{
+     uint32_t pcpu;
+     unsigned int nproc_conf;
+     cpu_set_t online_cpuset;
+
+     nproc_conf = get_nprocs_conf();
+     sched_getaffinity(0, sizeof(cpu_set_t), &online_cpuset);
+
+     /* Randomly find an available pCPU to place a vCPU on */
+     do {
+             pcpu = rand() % nproc_conf;
+     } while (!CPU_ISSET(pcpu, &online_cpuset));
+
+     return pcpu;
+}
+
+static int test_migrate_vcpu(unsigned int vcpu_idx)
+{
+     int ret;
+     cpu_set_t cpuset;
+     uint32_t new_pcpu = test_get_pcpu();
+
+     CPU_ZERO(&cpuset);
+     CPU_SET(new_pcpu, &cpuset);
+
+     pr_debug("Migrating vCPU: %u to pCPU: %u\n", vcpu_idx, new_pcpu);
+
+     ret = pthread_setaffinity_np(pt_vcpu_run[vcpu_idx],
+                                  sizeof(cpuset), &cpuset);
+
+     /* Allow the error where the vCPU thread is already finished */
+     TEST_ASSERT(ret == 0 || ret == ESRCH,
+                 "Failed to migrate the vCPU:%u to pCPU: %u; ret: %d\n",
+                 vcpu_idx, new_pcpu, ret);
+
+     return ret;
+}
+
+static void *test_vcpu_migration(void *arg)
+{
+     unsigned int i, n_done;
+     bool vcpu_done;
+
+     do {
+             usleep(msecs_to_usecs(test_args.migration_freq_ms));
+
+             for (n_done = 0, i = 0; i < test_args.nr_vcpus; i++) {
+                     pthread_mutex_lock(&vcpu_done_map_lock);
+                     vcpu_done = test_bit(i, vcpu_done_map);
+                     pthread_mutex_unlock(&vcpu_done_map_lock);
+
+                     if (vcpu_done) {
+                             n_done++;
+                             continue;
+                     }
+
+                     test_migrate_vcpu(i);
+             }
+     } while (test_args.nr_vcpus != n_done);
+
+     return NULL;
+}
+
+static void test_run(struct kvm_vm *vm)
+{
+     pthread_t pt_vcpu_migration;
+     unsigned int i;
+     int ret;
+
+     pthread_mutex_init(&vcpu_done_map_lock, NULL);
+     vcpu_done_map = bitmap_zalloc(test_args.nr_vcpus);
+     TEST_ASSERT(vcpu_done_map, "Failed to allocate vcpu done bitmap\n");
+
+     for (i = 0; i < (unsigned long)test_args.nr_vcpus; i++) {
+             ret = pthread_create(&pt_vcpu_run[i], NULL, test_vcpu_run,
+                                  (void *)(unsigned long)i);
+             TEST_ASSERT(!ret, "Failed to create vCPU-%d pthread\n", i);
+     }
+
+     /* Spawn a thread to control the vCPU migrations */
+     if (test_args.migration_freq_ms) {
+             srand(time(NULL));
+
+             ret = pthread_create(&pt_vcpu_migration, NULL,
+                                     test_vcpu_migration, NULL);
+             TEST_ASSERT(!ret, "Failed to create the migration pthread\n");
+     }
+
+
+     for (i = 0; i < test_args.nr_vcpus; i++)
+             pthread_join(pt_vcpu_run[i], NULL);
+
+     if (test_args.migration_freq_ms)
+             pthread_join(pt_vcpu_migration, NULL);
+
+     bitmap_free(vcpu_done_map);
+}
+
+static void test_print_help(char *name)
+{
+     pr_info("Usage: %s [-h] [-n nr_vcpus] [-i iterations] [-p timer_period_ms]\n",
+             name);
+     pr_info("\t-n: Number of vCPUs to configure (default: %u; max: %u)\n",
+             NR_VCPUS_DEF, KVM_MAX_VCPUS);
+     pr_info("\t-i: Number of iterations per stage (default: %u)\n",
+             NR_TEST_ITERS_DEF);
+     pr_info("\t-p: Periodicity (in ms) of the guest timer (default: %u)\n",
+             TIMER_TEST_PERIOD_MS_DEF);
+     pr_info("\t-m: Frequency (in ms) of vCPUs to migrate to different pCPU. 0 to turn off (default: %u)\n",
+             TIMER_TEST_MIGRATION_FREQ_MS);
+     pr_info("\t-o: Counter offset (in counter cycles, default: 0)\n");
+     pr_info("\t-h: print this help screen\n");
+}
+
+static bool parse_args(int argc, char *argv[])
+{
+     int opt;
+
+     while ((opt = getopt(argc, argv, "hn:i:p:m:o:")) != -1) {
+             switch (opt) {
+             case 'n':
+                     test_args.nr_vcpus = atoi_positive("Number of vCPUs", optarg);
+                     if (test_args.nr_vcpus > KVM_MAX_VCPUS) {
+                             pr_info("Max allowed vCPUs: %u\n",
+                                     KVM_MAX_VCPUS);
+                             goto err;
+                     }
+                     break;
+             case 'i':
+                     test_args.nr_iter = atoi_positive("Number of iterations", optarg);
+                     break;
+             case 'p':
+                     test_args.timer_period_ms = atoi_positive("Periodicity", optarg);
+                     break;
+             case 'm':
+                     test_args.migration_freq_ms = atoi_non_negative("Frequency", optarg);
+                     break;
+             case 'o':
+                     test_args.offset.counter_offset = strtol(optarg, NULL, 0);
+                     test_args.offset.reserved = 0;
+                     break;
+             case 'h':
+             default:
+                     goto err;
+             }
+     }
+
+     return true;
+
+err:
+     test_print_help(argv[0]);
+     return false;
+}
+
+int main(int argc, char *argv[])
+{
+     struct kvm_vm *vm;
+
+     if (!parse_args(argc, argv))
+             exit(KSFT_SKIP);
+
+     __TEST_REQUIRE(!test_args.migration_freq_ms || get_nprocs() >= 2,
+                    "At least two physical CPUs needed for vCPU migration");
+
+     vm = test_vm_create();
+     test_run(vm);
+     test_vm_cleanup(vm);
+
+     return 0;
+}

tools/testing/selftests/kvm/include/timer_test.h new file +13 -0

new file mode 100644
index 000000000000..109e4d635627
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * tools/testing/selftests/kvm/include/timer_test.h
+ *
+ * Copyright (C) 2018, Google LLC
+ */
+
+#ifndef SELFTEST_KVM_TIMER_TEST_H
+#define SELFTEST_KVM_TIMER_TEST_H
+
+#include "kvm_util.h"
+
+#define NR_VCPUS_DEF            4

Quoted 9 lines

tools/testing/selftests/kvm/include/timer_test.h line 19

Quoted 6 lines

tools/testing/selftests/kvm/include/timer_test.h new file +6 -0

+#define NR_TEST_ITERS_DEF       5
+#define TIMER_TEST_PERIOD_MS_DEF    10
+#define TIMER_TEST_ERR_MARGIN_US    100
+#define TIMER_TEST_MIGRATION_FREQ_MS    2
+
+#define msecs_to_usecs(msec)    ((msec) * 1000LL)

I'd move the above to include/test_util.h

Yes, msecs_to_usecs() macro should be a common API for all the tests.

Quoted 6 lines

tools/testing/selftests/kvm/include/timer_test.h line 22

Quoted 3 lines

tools/testing/selftests/kvm/include/timer_test.h new file +3 -0

+
+#define GICD_BASE_GPA    0x8000000ULL
+#define GICR_BASE_GPA    0x80A0000ULL

These defines belong in aarch64/arch_timer.c

These 2 defines were also defined in other test cases, shall we move them to an aarch64 specific header file? Maybe tools/testing/selftests/kvm/include/aarch64/gic.h?

Quoted 11 lines

tools/testing/selftests/kvm/include/timer_test.h line 30

Quoted 8 lines

tools/testing/selftests/kvm/include/timer_test.h new file +8 -0

+
+enum guest_stage {
+     GUEST_STAGE_VTIMER_CVAL=1,
+     GUEST_STAGE_VTIMER_TVAL,
+     GUEST_STAGE_PTIMER_CVAL,
+     GUEST_STAGE_PTIMER_TVAL,
+     GUEST_STAGE_MAX,
+};

This enum also belongs in aarch64/arch_timer.c

Yes, it should be in aarch64/arch_timer.c

Quoted 13 lines

Quoted 13 lines

tools/testing/selftests/kvm/include/timer_test.h new file +13 -0

+
+/* Timer test cmdline parameters */
+struct test_args
+{
+     int nr_vcpus;
+     int nr_iter;
+     int timer_period_ms;
+     int migration_freq_ms;
+     struct kvm_arm_counter_offset offset;
+};
+
+/* Shared variables between host and guest */
+struct test_vcpu_shared_data {

Quoted 9 lines

tools/testing/selftests/kvm/include/timer_test.h line 49

Quoted 6 lines

tools/testing/selftests/kvm/include/timer_test.h new file +6 -0

+     int nr_iter;
+     enum guest_stage guest_stage;
+     uint64_t xcnt;
+};
+
+struct kvm_vm* test_vm_create(void);

Move * next to function name.

Sure, thanks!

Quoted 9 lines

Quoted 6 lines

tools/testing/selftests/kvm/include/timer_test.h new file +3 -1

+void test_vm_cleanup(struct kvm_vm *vm);
+
+#endif /* SELFTEST_KVM_TIMER_TEST_H */
--

2.34.1

Thanks,
drew

Quoted 2 lines · On Wed, Sep 6, 2023 at 10:14?AM Haibo Xu <[email protected]> wrote:

On Mon, Sep 4, 2023 at 9:24?PM Andrew Jones <[email protected]> wrote:

Quoted 12 lines

Quoted 11 lines

tools/testing/selftests/kvm/include/timer_test.h line 30

Quoted 8 lines

tools/testing/selftests/kvm/include/timer_test.h new file +8 -0

+
+enum guest_stage {
+     GUEST_STAGE_VTIMER_CVAL=1,
+     GUEST_STAGE_VTIMER_TVAL,
+     GUEST_STAGE_PTIMER_CVAL,
+     GUEST_STAGE_PTIMER_TVAL,
+     GUEST_STAGE_MAX,
+};

This enum also belongs in aarch64/arch_timer.c

Quoted 2 lines

Yes, it should be in aarch64/arch_timer.c

After moving the above enum definition to aarch64/arch_timer.c, the below errors was reported
while compiling kvm/arch_timer.o

include/timer_test.h:37:26: error: field ?guest_stage? has incomplete type
37 | enum guest_stage guest_stage;
| ^~~~~~~~~~~

Since kvm/arch_timer.c was independent of kvm/aarch64/arch_timer.c during OBJ compiling,
I think it may be not possible to move the enum definition to aarch64/arch_timer.c

If we keep the definition in this header file, we can enclose it with #ifdef __aarch64__ for aarch64 only.

Quoted 3 lines · Haibo Xu

Quoted 2 lines · On Wed, Sep 6, 2023 at 10:14?AM Haibo Xu <[email protected]> wrote:

On Mon, Sep 4, 2023 at 9:24?PM Andrew Jones <[email protected]> wrote:

...

Quoted 23 lines

Quoted 12 lines

Quoted 11 lines

tools/testing/selftests/kvm/include/timer_test.h line 30

Quoted 8 lines

tools/testing/selftests/kvm/include/timer_test.h new file +8 -0

+
+enum guest_stage {
+     GUEST_STAGE_VTIMER_CVAL=1,
+     GUEST_STAGE_VTIMER_TVAL,
+     GUEST_STAGE_PTIMER_CVAL,
+     GUEST_STAGE_PTIMER_TVAL,
+     GUEST_STAGE_MAX,
+};

This enum also belongs in aarch64/arch_timer.c

Quoted 2 lines

Yes, it should be in aarch64/arch_timer.c

After moving the above enum definition to aarch64/arch_timer.c, the below errors was reported
while compiling kvm/arch_timer.o

include/timer_test.h:37:26: error: field ?guest_stage? has incomplete type
37 | enum guest_stage guest_stage;
| ^~~~~~~~~~~

Quoted 8 lines

Since kvm/arch_timer.c was independent of kvm/aarch64/arch_timer.c during OBJ compiling,
I think it may be not possible to move the enum definition to aarch64/arch_timer.c

If we keep the definition in this header file, we can enclose it with #ifdef __aarch64__ for aarch64 only.

Let's change struct test_vcpu_shared_data to

 struct test_vcpu_shared_data {
        int nr_iter;
        int guest_stage;
        uint64_t xcnt;

};

and then let the aarch64 code treat guest_stage as an enum and the riscv code can completely ignore it (no need to create an unused enum).

Thanks,
drew

Quoted 2 lines · On Mon, Sep 4, 2023 at 9:24?PM Andrew Jones <[email protected]> wrote:

On Sat, Sep 02, 2023 at 08:59:24PM +0800, Haibo Xu wrote:

Quoted 4 lines

Quoted 4 lines

tools/testing/selftests/kvm/include/timer_test.h new file +4 -0

+
+#include "kvm_util.h"
+
+#define NR_VCPUS_DEF            4

Quoted 9 lines

tools/testing/selftests/kvm/include/timer_test.h line 19

Quoted 6 lines

tools/testing/selftests/kvm/include/timer_test.h new file +6 -0

+#define NR_TEST_ITERS_DEF       5
+#define TIMER_TEST_PERIOD_MS_DEF    10
+#define TIMER_TEST_ERR_MARGIN_US    100
+#define TIMER_TEST_MIGRATION_FREQ_MS    2
+
+#define msecs_to_usecs(msec)    ((msec) * 1000LL)

I'd move the above to include/test_util.h

Yes, msecs_to_usecs() macro should be a common API for all the tests.

Quoted 6 lines

tools/testing/selftests/kvm/include/timer_test.h line 22

Quoted 3 lines

tools/testing/selftests/kvm/include/timer_test.h new file +3 -0

+
+#define GICD_BASE_GPA    0x8000000ULL
+#define GICR_BASE_GPA    0x80A0000ULL

These defines belong in aarch64/arch_timer.c

These 2 defines were also defined in other test cases, shall we move them to an aarch64 specific header file? Maybe tools/testing/selftests/kvm/include/aarch64/gic.h?

Quoted 3 lines · Haibo Xu

Quoted 2 lines · On Mon, Sep 4, 2023 at 9:24?PM Andrew Jones <[email protected]> wrote:

On Sat, Sep 02, 2023 at 08:59:24PM +0800, Haibo Xu wrote:

...

Quoted 23 lines

Quoted 4 lines

Quoted 4 lines

tools/testing/selftests/kvm/include/timer_test.h new file +4 -0

+
+#include "kvm_util.h"
+
+#define NR_VCPUS_DEF            4

Quoted 9 lines

tools/testing/selftests/kvm/include/timer_test.h line 19

Quoted 6 lines

tools/testing/selftests/kvm/include/timer_test.h new file +6 -0

+#define NR_TEST_ITERS_DEF       5
+#define TIMER_TEST_PERIOD_MS_DEF    10
+#define TIMER_TEST_ERR_MARGIN_US    100
+#define TIMER_TEST_MIGRATION_FREQ_MS    2
+
+#define msecs_to_usecs(msec)    ((msec) * 1000LL)

I'd move the above to include/test_util.h

Yes, msecs_to_usecs() macro should be a common API for all the tests.

Quoted 6 lines

tools/testing/selftests/kvm/include/timer_test.h line 22

Quoted 3 lines

tools/testing/selftests/kvm/include/timer_test.h new file +3 -0

+
+#define GICD_BASE_GPA    0x8000000ULL
+#define GICR_BASE_GPA    0x80A0000ULL

These defines belong in aarch64/arch_timer.c

Quoted 3 lines

These 2 defines were also defined in other test cases, shall we move them to an aarch64 specific header file? Maybe tools/testing/selftests/kvm/include/aarch64/gic.h?

Even though currently all the aarch64 tests that use the gic are using these defines for the base addresses, each test is free to choose whatever base addresses it likes. I'd just move them back to aarch64/arch_timer.c for now. Consolidating them into shared, default base addresses, if done at all, is work for another series.

Thanks,
drew