| Message ID | 20221115213845.3348210-1-vannapurve@google.com |
|---|---|
| Headers | show |
| Series | Minor improvements to the selftest setup logic | expand |
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 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: > 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. Looks good! A few uber nits, but nothing that can't be fixed up when applying.
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.
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(-)