This series is posted in context of the discussion at: https://lore.kernel.org/lkml/Ywa9T+jKUpaHLu%2Fl@google.com/
Major changes: 1) Move common startup logic to a single function in kvm_util.c 2) Introduce following APIs: kvm_selftest_arch_init: to perform arch specific common startup. kvm_arch_vm_post_create: to perform arch specific common setup after VM creation.
Changelog ========= v4: * Removed the patch to precompute cpu type, will be introduced as part of a separate series in future. v3: * Original series is split into two and this v3 version contains the improvements to selftest and VM setup. * Planning to upload the second series to execute hypercall instruction according to cpu type separately. * Addressed comments from David and Sean. link to v3: https://lore.kernel.org/lkml/20221013121319.994170-1-vannapurve@google.com/ v2: * Addressed comments from Andrew and David * Common function with constructor attribute used to setup initial state * Changes are split in more logical granules as per feedback link to v2: https://lore.kernel.org/all/20220915000448.1674802-1-vannapurve@google.com/
Vishal Annapurve (3): KVM: selftests: move common startup logic to kvm_util.c KVM: selftests: Add arch specific initialization KVM: selftests: Add arch specific post vm creation hook
.../selftests/kvm/aarch64/arch_timer.c | 3 --- .../selftests/kvm/aarch64/hypercalls.c | 2 -- .../testing/selftests/kvm/aarch64/vgic_irq.c | 3 --- .../selftests/kvm/include/kvm_util_base.h | 9 ++++++++ .../selftests/kvm/lib/aarch64/processor.c | 18 ++++++++-------- tools/testing/selftests/kvm/lib/kvm_util.c | 21 ++++++++++++++++--- .../selftests/kvm/lib/x86_64/processor.c | 6 ++++++ .../testing/selftests/kvm/memslot_perf_test.c | 3 --- tools/testing/selftests/kvm/rseq_test.c | 3 --- tools/testing/selftests/kvm/s390x/memop.c | 2 -- tools/testing/selftests/kvm/s390x/resets.c | 2 -- .../selftests/kvm/s390x/sync_regs_test.c | 3 --- .../selftests/kvm/set_memory_region_test.c | 3 --- .../kvm/x86_64/cr4_cpuid_sync_test.c | 3 --- .../kvm/x86_64/emulator_error_test.c | 3 --- .../selftests/kvm/x86_64/hyperv_cpuid.c | 3 --- .../selftests/kvm/x86_64/platform_info_test.c | 3 --- .../kvm/x86_64/pmu_event_filter_test.c | 3 --- .../selftests/kvm/x86_64/set_sregs_test.c | 3 --- .../kvm/x86_64/svm_nested_soft_inject_test.c | 3 --- .../selftests/kvm/x86_64/sync_regs_test.c | 3 --- .../selftests/kvm/x86_64/userspace_io_test.c | 3 --- .../kvm/x86_64/userspace_msr_exit_test.c | 3 --- 23 files changed, 42 insertions(+), 66 deletions(-)
Consolidate common startup logic in one place by implementing a single setup function with __attribute((constructor)) for all selftests within kvm_util.c.
This allows moving logic like: /* Tell stdout not to buffer its content */ setbuf(stdout, NULL); to a single file for all selftests.
This will also allow any required setup at entry in future to be done in common main function.
More context is discussed at: https://lore.kernel.org/lkml/Ywa9T+jKUpaHLu%2Fl@google.com/
Suggested-by: Sean Christopherson seanjc@google.com Reviewed-by: Andrew Jones andrew.jones@linux.dev Reviewed-by: Peter Gonda pgonda@google.com Signed-off-by: Vishal Annapurve vannapurve@google.com --- tools/testing/selftests/kvm/aarch64/arch_timer.c | 3 --- tools/testing/selftests/kvm/aarch64/hypercalls.c | 2 -- tools/testing/selftests/kvm/aarch64/vgic_irq.c | 3 --- tools/testing/selftests/kvm/lib/kvm_util.c | 6 ++++++ tools/testing/selftests/kvm/memslot_perf_test.c | 3 --- tools/testing/selftests/kvm/rseq_test.c | 3 --- tools/testing/selftests/kvm/s390x/memop.c | 2 -- tools/testing/selftests/kvm/s390x/resets.c | 2 -- tools/testing/selftests/kvm/s390x/sync_regs_test.c | 3 --- tools/testing/selftests/kvm/set_memory_region_test.c | 3 --- tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c | 3 --- tools/testing/selftests/kvm/x86_64/emulator_error_test.c | 3 --- tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c | 3 --- tools/testing/selftests/kvm/x86_64/platform_info_test.c | 3 --- tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c | 3 --- tools/testing/selftests/kvm/x86_64/set_sregs_test.c | 3 --- .../selftests/kvm/x86_64/svm_nested_soft_inject_test.c | 3 --- tools/testing/selftests/kvm/x86_64/sync_regs_test.c | 3 --- tools/testing/selftests/kvm/x86_64/userspace_io_test.c | 3 --- .../testing/selftests/kvm/x86_64/userspace_msr_exit_test.c | 3 --- 20 files changed, 6 insertions(+), 54 deletions(-)
diff --git a/tools/testing/selftests/kvm/aarch64/arch_timer.c b/tools/testing/selftests/kvm/aarch64/arch_timer.c index 574eb73f0e90..07836bd2672b 100644 --- a/tools/testing/selftests/kvm/aarch64/arch_timer.c +++ b/tools/testing/selftests/kvm/aarch64/arch_timer.c @@ -462,9 +462,6 @@ int main(int argc, char *argv[]) { struct kvm_vm *vm;
- /* Tell stdout not to buffer its content */ - setbuf(stdout, NULL); - if (!parse_args(argc, argv)) exit(KSFT_SKIP);
diff --git a/tools/testing/selftests/kvm/aarch64/hypercalls.c b/tools/testing/selftests/kvm/aarch64/hypercalls.c index a39da3fe4952..6463fd118429 100644 --- a/tools/testing/selftests/kvm/aarch64/hypercalls.c +++ b/tools/testing/selftests/kvm/aarch64/hypercalls.c @@ -306,8 +306,6 @@ static void test_run(void)
int main(void) { - setbuf(stdout, NULL); - test_run(); return 0; } diff --git a/tools/testing/selftests/kvm/aarch64/vgic_irq.c b/tools/testing/selftests/kvm/aarch64/vgic_irq.c index 17417220a083..3f204f2e93bf 100644 --- a/tools/testing/selftests/kvm/aarch64/vgic_irq.c +++ b/tools/testing/selftests/kvm/aarch64/vgic_irq.c @@ -818,9 +818,6 @@ int main(int argc, char **argv) int opt; bool eoi_split = false;
- /* Tell stdout not to buffer its content */ - setbuf(stdout, NULL); - while ((opt = getopt(argc, argv, "hn:e:l:")) != -1) { switch (opt) { case 'n': diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index f1cb1627161f..37d7d144c74e 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -2021,3 +2021,9 @@ void __vm_get_stat(struct kvm_vm *vm, const char *stat_name, uint64_t *data, break; } } + +void __attribute((constructor)) kvm_selftest_init(void) +{ + /* Tell stdout not to buffer its content. */ + setbuf(stdout, NULL); +} diff --git a/tools/testing/selftests/kvm/memslot_perf_test.c b/tools/testing/selftests/kvm/memslot_perf_test.c index 44995446d942..f7ba77ff45c9 100644 --- a/tools/testing/selftests/kvm/memslot_perf_test.c +++ b/tools/testing/selftests/kvm/memslot_perf_test.c @@ -1007,9 +1007,6 @@ int main(int argc, char *argv[]) struct test_result rbestslottime; int tctr;
- /* Tell stdout not to buffer its content */ - setbuf(stdout, NULL); - if (!parse_args(argc, argv, &targs)) return -1;
diff --git a/tools/testing/selftests/kvm/rseq_test.c b/tools/testing/selftests/kvm/rseq_test.c index 6f88da7e60be..18297b803159 100644 --- a/tools/testing/selftests/kvm/rseq_test.c +++ b/tools/testing/selftests/kvm/rseq_test.c @@ -205,9 +205,6 @@ int main(int argc, char *argv[]) struct kvm_vcpu *vcpu; u32 cpu, rseq_cpu;
- /* Tell stdout not to buffer its content */ - setbuf(stdout, NULL); - r = sched_getaffinity(0, sizeof(possible_mask), &possible_mask); TEST_ASSERT(!r, "sched_getaffinity failed, errno = %d (%s)", errno, strerror(errno)); diff --git a/tools/testing/selftests/kvm/s390x/memop.c b/tools/testing/selftests/kvm/s390x/memop.c index 9113696d5178..3fd81e58f40c 100644 --- a/tools/testing/selftests/kvm/s390x/memop.c +++ b/tools/testing/selftests/kvm/s390x/memop.c @@ -760,8 +760,6 @@ int main(int argc, char *argv[])
TEST_REQUIRE(kvm_has_cap(KVM_CAP_S390_MEM_OP));
- setbuf(stdout, NULL); /* Tell stdout not to buffer its content */ - ksft_print_header();
ksft_set_plan(ARRAY_SIZE(testlist)); diff --git a/tools/testing/selftests/kvm/s390x/resets.c b/tools/testing/selftests/kvm/s390x/resets.c index 19486084eb30..e41e2cb8ffa9 100644 --- a/tools/testing/selftests/kvm/s390x/resets.c +++ b/tools/testing/selftests/kvm/s390x/resets.c @@ -296,8 +296,6 @@ int main(int argc, char *argv[]) bool has_s390_vcpu_resets = kvm_check_cap(KVM_CAP_S390_VCPU_RESETS); int idx;
- setbuf(stdout, NULL); /* Tell stdout not to buffer its content */ - ksft_print_header(); ksft_set_plan(ARRAY_SIZE(testlist));
diff --git a/tools/testing/selftests/kvm/s390x/sync_regs_test.c b/tools/testing/selftests/kvm/s390x/sync_regs_test.c index 3fdb6e2598eb..2ddde41c44ba 100644 --- a/tools/testing/selftests/kvm/s390x/sync_regs_test.c +++ b/tools/testing/selftests/kvm/s390x/sync_regs_test.c @@ -231,9 +231,6 @@ int main(int argc, char *argv[])
TEST_REQUIRE(kvm_has_cap(KVM_CAP_SYNC_REGS));
- /* Tell stdout not to buffer its content */ - setbuf(stdout, NULL); - ksft_print_header();
ksft_set_plan(ARRAY_SIZE(testlist)); diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c index 0d55f508d595..614141d6e53d 100644 --- a/tools/testing/selftests/kvm/set_memory_region_test.c +++ b/tools/testing/selftests/kvm/set_memory_region_test.c @@ -392,9 +392,6 @@ int main(int argc, char *argv[]) int i, loops; #endif
- /* Tell stdout not to buffer its content */ - setbuf(stdout, NULL); - #ifdef __x86_64__ /* * FIXME: the zero-memslot test fails on aarch64 and s390x because diff --git a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c index 4208487652f8..1027a671c7d3 100644 --- a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c +++ b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c @@ -57,9 +57,6 @@ int main(int argc, char *argv[])
TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_XSAVE));
- /* Tell stdout not to buffer its content */ - setbuf(stdout, NULL); - vm = vm_create_with_one_vcpu(&vcpu, guest_code); run = vcpu->run;
diff --git a/tools/testing/selftests/kvm/x86_64/emulator_error_test.c b/tools/testing/selftests/kvm/x86_64/emulator_error_test.c index 236e11755ba6..3334adcfd591 100644 --- a/tools/testing/selftests/kvm/x86_64/emulator_error_test.c +++ b/tools/testing/selftests/kvm/x86_64/emulator_error_test.c @@ -156,9 +156,6 @@ int main(int argc, char *argv[]) uint64_t *hva; int rc;
- /* Tell stdout not to buffer its content */ - setbuf(stdout, NULL); - TEST_REQUIRE(kvm_has_cap(KVM_CAP_SMALLER_MAXPHYADDR));
vm = vm_create_with_one_vcpu(&vcpu, guest_code); diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c index e804eb08dff9..5c27efbf405e 100644 --- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c +++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c @@ -134,9 +134,6 @@ int main(int argc, char *argv[]) const struct kvm_cpuid2 *hv_cpuid_entries; struct kvm_vcpu *vcpu;
- /* Tell stdout not to buffer its content */ - setbuf(stdout, NULL); - TEST_REQUIRE(kvm_has_cap(KVM_CAP_HYPERV_CPUID));
vm = vm_create_with_one_vcpu(&vcpu, guest_code); diff --git a/tools/testing/selftests/kvm/x86_64/platform_info_test.c b/tools/testing/selftests/kvm/x86_64/platform_info_test.c index 76417c7d687b..310a104d94f0 100644 --- a/tools/testing/selftests/kvm/x86_64/platform_info_test.c +++ b/tools/testing/selftests/kvm/x86_64/platform_info_test.c @@ -72,9 +72,6 @@ int main(int argc, char *argv[]) struct kvm_vm *vm; uint64_t msr_platform_info;
- /* Tell stdout not to buffer its content */ - setbuf(stdout, NULL); - TEST_REQUIRE(kvm_has_cap(KVM_CAP_MSR_PLATFORM_INFO));
vm = vm_create_with_one_vcpu(&vcpu, guest_code); diff --git a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c index ea4e259a1e2e..a6ffa245c897 100644 --- a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c +++ b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c @@ -447,9 +447,6 @@ int main(int argc, char *argv[]) struct kvm_vcpu *vcpu; struct kvm_vm *vm;
- /* Tell stdout not to buffer its content */ - setbuf(stdout, NULL); - TEST_REQUIRE(kvm_has_cap(KVM_CAP_PMU_EVENT_FILTER));
TEST_REQUIRE(use_intel_pmu() || use_amd_pmu()); diff --git a/tools/testing/selftests/kvm/x86_64/set_sregs_test.c b/tools/testing/selftests/kvm/x86_64/set_sregs_test.c index 2bb08bf2125d..a284fcef6ed7 100644 --- a/tools/testing/selftests/kvm/x86_64/set_sregs_test.c +++ b/tools/testing/selftests/kvm/x86_64/set_sregs_test.c @@ -82,9 +82,6 @@ int main(int argc, char *argv[]) uint64_t cr4; int rc;
- /* Tell stdout not to buffer its content */ - setbuf(stdout, NULL); - /* * Create a dummy VM, specifically to avoid doing KVM_SET_CPUID2, and * use it to verify all supported CR4 bits can be set prior to defining diff --git a/tools/testing/selftests/kvm/x86_64/svm_nested_soft_inject_test.c b/tools/testing/selftests/kvm/x86_64/svm_nested_soft_inject_test.c index e637d7736012..e497ace629c1 100644 --- a/tools/testing/selftests/kvm/x86_64/svm_nested_soft_inject_test.c +++ b/tools/testing/selftests/kvm/x86_64/svm_nested_soft_inject_test.c @@ -194,9 +194,6 @@ static void run_test(bool is_nmi)
int main(int argc, char *argv[]) { - /* Tell stdout not to buffer its content */ - setbuf(stdout, NULL); - TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_SVM));
TEST_ASSERT(kvm_cpu_has(X86_FEATURE_NRIPS), diff --git a/tools/testing/selftests/kvm/x86_64/sync_regs_test.c b/tools/testing/selftests/kvm/x86_64/sync_regs_test.c index 9b6db0b0b13e..d2f9b5bdfab2 100644 --- a/tools/testing/selftests/kvm/x86_64/sync_regs_test.c +++ b/tools/testing/selftests/kvm/x86_64/sync_regs_test.c @@ -90,9 +90,6 @@ int main(int argc, char *argv[]) struct kvm_vcpu_events events; int rv, cap;
- /* Tell stdout not to buffer its content */ - setbuf(stdout, NULL); - cap = kvm_check_cap(KVM_CAP_SYNC_REGS); TEST_REQUIRE((cap & TEST_SYNC_FIELDS) == TEST_SYNC_FIELDS); TEST_REQUIRE(!(cap & INVALID_SYNC_FIELD)); diff --git a/tools/testing/selftests/kvm/x86_64/userspace_io_test.c b/tools/testing/selftests/kvm/x86_64/userspace_io_test.c index 7316521428f8..91076c9787b4 100644 --- a/tools/testing/selftests/kvm/x86_64/userspace_io_test.c +++ b/tools/testing/selftests/kvm/x86_64/userspace_io_test.c @@ -56,9 +56,6 @@ int main(int argc, char *argv[]) struct kvm_vm *vm; struct ucall uc;
- /* Tell stdout not to buffer its content */ - setbuf(stdout, NULL); - vm = vm_create_with_one_vcpu(&vcpu, guest_code); run = vcpu->run;
diff --git a/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c b/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c index fae95089e655..25fa55344a10 100644 --- a/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c +++ b/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c @@ -818,9 +818,6 @@ static void test_user_exit_msr_flags(void)
int main(int argc, char *argv[]) { - /* Tell stdout not to buffer its content */ - setbuf(stdout, NULL); - test_msr_filter_allow();
test_msr_filter_deny();
On Tue, Nov 15, 2022, Vishal Annapurve wrote:
Consolidate common startup logic in one place by implementing a single setup function with __attribute((constructor)) for all selftests within kvm_util.c.
This allows moving logic like: /* Tell stdout not to buffer its content */ setbuf(stdout, NULL); to a single file for all selftests.
This will also allow any required setup at entry in future to be done in common main function.
More context is discussed at: https://lore.kernel.org/lkml/Ywa9T+jKUpaHLu%2Fl@google.com/
Nit,
Link: https://lore.kernel.org/lkml/Ywa9T+jKUpaHLu%2Fl@google.com
is the the "standard" way to convey this information.
On Wed, Nov 16, 2022 at 9:36 AM Sean Christopherson seanjc@google.com wrote:
On Tue, Nov 15, 2022, Vishal Annapurve wrote:
Consolidate common startup logic in one place by implementing a single setup function with __attribute((constructor)) for all selftests within kvm_util.c.
This allows moving logic like: /* Tell stdout not to buffer its content */ setbuf(stdout, NULL); to a single file for all selftests.
This will also allow any required setup at entry in future to be done in common main function.
More context is discussed at: https://lore.kernel.org/lkml/Ywa9T+jKUpaHLu%2Fl@google.com/
Nit,
Link: https://lore.kernel.org/lkml/Ywa9T+jKUpaHLu%2Fl@google.com
is the the "standard" way to convey this information.
Noted.
Introduce arch specific API: kvm_selftest_arch_init to allow each arch to handle initialization before running any selftest logic.
Suggested-by: Sean Christopherson seanjc@google.com Reviewed-by: Andrew Jones andrew.jones@linux.dev Reviewed-by: Peter Gonda pgonda@google.com Signed-off-by: Vishal Annapurve vannapurve@google.com --- .../selftests/kvm/include/kvm_util_base.h | 5 +++++ .../selftests/kvm/lib/aarch64/processor.c | 18 +++++++++--------- tools/testing/selftests/kvm/lib/kvm_util.c | 6 ++++++ 3 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h index e42a09cd24a0..eec0e4898efe 100644 --- a/tools/testing/selftests/kvm/include/kvm_util_base.h +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h @@ -838,4 +838,9 @@ static inline int __vm_disable_nx_huge_pages(struct kvm_vm *vm) return __vm_enable_cap(vm, KVM_CAP_VM_DISABLE_NX_HUGE_PAGES, 0); }
+/* + * API to execute architecture specific setup before executing main(). + */ +void kvm_selftest_arch_init(void); + #endif /* SELFTEST_KVM_UTIL_BASE_H */ diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c index 6f5551368944..0de4aabc0c76 100644 --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c @@ -495,15 +495,6 @@ void aarch64_get_supported_page_sizes(uint32_t ipa, close(kvm_fd); }
-/* - * arm64 doesn't have a true default mode, so start by computing the - * available IPA space and page sizes early. - */ -void __attribute__((constructor)) init_guest_modes(void) -{ - guest_modes_append_default(); -} - void smccc_hvc(uint32_t function_id, uint64_t arg0, uint64_t arg1, uint64_t arg2, uint64_t arg3, uint64_t arg4, uint64_t arg5, uint64_t arg6, struct arm_smccc_res *res) @@ -528,3 +519,12 @@ void smccc_hvc(uint32_t function_id, uint64_t arg0, uint64_t arg1, [arg4] "r"(arg4), [arg5] "r"(arg5), [arg6] "r"(arg6) : "x0", "x1", "x2", "x3", "x4", "x5", "x6", "x7"); } + +void kvm_selftest_arch_init(void) +{ + /* + * arm64 doesn't have a true default mode, so start by computing the + * available IPA space and page sizes early. + */ + guest_modes_append_default(); +} diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index 37d7d144c74e..deb4c731b9fa 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -2022,8 +2022,14 @@ void __vm_get_stat(struct kvm_vm *vm, const char *stat_name, uint64_t *data, } }
+__weak void kvm_selftest_arch_init(void) +{ +} + void __attribute((constructor)) kvm_selftest_init(void) { /* Tell stdout not to buffer its content. */ setbuf(stdout, NULL); + + kvm_selftest_arch_init(); }
On Tue, Nov 15, 2022, Vishal Annapurve wrote:
Introduce arch specific API: kvm_selftest_arch_init to allow each arch to handle initialization before running any selftest logic.
Suggested-by: Sean Christopherson seanjc@google.com Reviewed-by: Andrew Jones andrew.jones@linux.dev Reviewed-by: Peter Gonda pgonda@google.com Signed-off-by: Vishal Annapurve vannapurve@google.com
.../selftests/kvm/include/kvm_util_base.h | 5 +++++ .../selftests/kvm/lib/aarch64/processor.c | 18 +++++++++--------- tools/testing/selftests/kvm/lib/kvm_util.c | 6 ++++++ 3 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h index e42a09cd24a0..eec0e4898efe 100644 --- a/tools/testing/selftests/kvm/include/kvm_util_base.h +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h @@ -838,4 +838,9 @@ static inline int __vm_disable_nx_huge_pages(struct kvm_vm *vm) return __vm_enable_cap(vm, KVM_CAP_VM_DISABLE_NX_HUGE_PAGES, 0); } +/*
- API to execute architecture specific setup before executing main().
- */
I find the "API" blurb to be somewhat confusing. When I think of APIs, I think of functions that are provided by a library that are called by users of the library.
An example might also help readers understand what types of setup can/should be done with this hook.
Maybe something like this?
/* * Arch hook that is invoked via a constructor, i.e. before exeucting main(), * to allow for arch-specific setup that is common to all tests, e.g. computing * the default guest "mode". */
On Wed, Nov 16, 2022 at 9:46 AM Sean Christopherson seanjc@google.com wrote:
On Tue, Nov 15, 2022, Vishal Annapurve wrote:
Introduce arch specific API: kvm_selftest_arch_init to allow each arch to handle initialization before running any selftest logic.
Suggested-by: Sean Christopherson seanjc@google.com Reviewed-by: Andrew Jones andrew.jones@linux.dev Reviewed-by: Peter Gonda pgonda@google.com Signed-off-by: Vishal Annapurve vannapurve@google.com
.../selftests/kvm/include/kvm_util_base.h | 5 +++++ .../selftests/kvm/lib/aarch64/processor.c | 18 +++++++++--------- tools/testing/selftests/kvm/lib/kvm_util.c | 6 ++++++ 3 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h index e42a09cd24a0..eec0e4898efe 100644 --- a/tools/testing/selftests/kvm/include/kvm_util_base.h +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h @@ -838,4 +838,9 @@ static inline int __vm_disable_nx_huge_pages(struct kvm_vm *vm) return __vm_enable_cap(vm, KVM_CAP_VM_DISABLE_NX_HUGE_PAGES, 0); }
+/*
- API to execute architecture specific setup before executing main().
- */
I find the "API" blurb to be somewhat confusing. When I think of APIs, I think of functions that are provided by a library that are called by users of the library.
An example might also help readers understand what types of setup can/should be done with this hook.
Maybe something like this?
/*
- Arch hook that is invoked via a constructor, i.e. before exeucting main(),
- to allow for arch-specific setup that is common to all tests, e.g. computing
- the default guest "mode".
*/
Ack, that looks better.
Add arch specific API kvm_arch_vm_post_create to perform any required setup after VM creation.
Suggested-by: Sean Christopherson seanjc@google.com Reviewed-by: Andrew Jones andrew.jones@linux.dev Reviewed-by: Peter Gonda pgonda@google.com Signed-off-by: Vishal Annapurve vannapurve@google.com --- tools/testing/selftests/kvm/include/kvm_util_base.h | 4 ++++ tools/testing/selftests/kvm/lib/kvm_util.c | 9 ++++++--- tools/testing/selftests/kvm/lib/x86_64/processor.c | 6 ++++++ 3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h index eec0e4898efe..1e7d3eae8c91 100644 --- a/tools/testing/selftests/kvm/include/kvm_util_base.h +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h @@ -843,4 +843,8 @@ static inline int __vm_disable_nx_huge_pages(struct kvm_vm *vm) */ void kvm_selftest_arch_init(void);
+/* + * API to execute architecture specific setup after creating the VM. + */ +void kvm_arch_vm_post_create(struct kvm_vm *vm); #endif /* SELFTEST_KVM_UTIL_BASE_H */ diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index deb4c731b9fa..3ed72980c996 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -340,9 +340,8 @@ struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint32_t nr_runnable_vcpus,
kvm_vm_elf_load(vm, program_invocation_name);
-#ifdef __x86_64__ - vm_create_irqchip(vm); -#endif + kvm_arch_vm_post_create(vm); + return vm; }
@@ -2022,6 +2021,10 @@ void __vm_get_stat(struct kvm_vm *vm, const char *stat_name, uint64_t *data, } }
+__weak void kvm_arch_vm_post_create(struct kvm_vm *vm) +{ +} + __weak void kvm_selftest_arch_init(void) { } diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c index 39c4409ef56a..fa65e8142c16 100644 --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c @@ -1327,3 +1327,9 @@ bool vm_is_unrestricted_guest(struct kvm_vm *vm)
return get_kvm_intel_param_bool("unrestricted_guest"); } + + +void kvm_arch_vm_post_create(struct kvm_vm *vm) +{ + vm_create_irqchip(vm); +}
On Tue, Nov 15, 2022, Vishal Annapurve wrote:
Add arch specific API kvm_arch_vm_post_create to perform any required setup after VM creation.
Suggested-by: Sean Christopherson seanjc@google.com Reviewed-by: Andrew Jones andrew.jones@linux.dev Reviewed-by: Peter Gonda pgonda@google.com Signed-off-by: Vishal Annapurve vannapurve@google.com
tools/testing/selftests/kvm/include/kvm_util_base.h | 4 ++++ tools/testing/selftests/kvm/lib/kvm_util.c | 9 ++++++--- tools/testing/selftests/kvm/lib/x86_64/processor.c | 6 ++++++ 3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h index eec0e4898efe..1e7d3eae8c91 100644 --- a/tools/testing/selftests/kvm/include/kvm_util_base.h +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h @@ -843,4 +843,8 @@ static inline int __vm_disable_nx_huge_pages(struct kvm_vm *vm) */ void kvm_selftest_arch_init(void); +/*
- API to execute architecture specific setup after creating the VM.
- */
Meh, I think the function name is self-explanatory.
+void kvm_arch_vm_post_create(struct kvm_vm *vm); #endif /* SELFTEST_KVM_UTIL_BASE_H */ diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index deb4c731b9fa..3ed72980c996 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -340,9 +340,8 @@ struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint32_t nr_runnable_vcpus, kvm_vm_elf_load(vm, program_invocation_name); -#ifdef __x86_64__
- vm_create_irqchip(vm);
-#endif
- kvm_arch_vm_post_create(vm);
- return vm;
} @@ -2022,6 +2021,10 @@ void __vm_get_stat(struct kvm_vm *vm, const char *stat_name, uint64_t *data, } } +__weak void kvm_arch_vm_post_create(struct kvm_vm *vm) +{ +}
__weak void kvm_selftest_arch_init(void) { } diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c index 39c4409ef56a..fa65e8142c16 100644 --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c @@ -1327,3 +1327,9 @@ bool vm_is_unrestricted_guest(struct kvm_vm *vm) return get_kvm_intel_param_bool("unrestricted_guest"); }
Extra newline.
+void kvm_arch_vm_post_create(struct kvm_vm *vm) +{
- vm_create_irqchip(vm);
+}
2.38.1.431.g37b22c650d-goog
On Tue, Nov 15, 2022, Vishal Annapurve wrote:
This series is posted in context of the discussion at: https://lore.kernel.org/lkml/Ywa9T+jKUpaHLu%2Fl@google.com/
Major changes:
- Move common startup logic to a single function in kvm_util.c
- Introduce following APIs: kvm_selftest_arch_init: to perform arch specific common startup. kvm_arch_vm_post_create: to perform arch specific common setup after VM creation.
Looks good! A few uber nits, but nothing that can't be fixed up when applying.
linux-kselftest-mirror@lists.linaro.org
-
Sean Christopherson -
Vishal Annapurve