2022-06-06 06:17:19

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

Overhaul KVM's selftest APIs to get selftests to a state where adding new
features and writing tests is less painful/disgusting.

Patches 1 fixes a goof in kvm/queue and should be squashed.

I would really, really, really like to get this queued up sooner than
later, or maybe just thrown into a separate selftests-specific branch that
folks can develop against. Rebasing is tedious, frustrating, and time
consuming. And spoiler alert, there's another 42 x86-centric patches
inbound that builds on this series to clean up CPUID related crud...

The primary theme is to stop treating tests like second class citizens.
Stop hiding vcpu, kvm_vm, etc... There's no sensitive data/constructs, and
the encapsulation has led to really, really bad and difficult to maintain
code. E.g. having to pass around the VM just to call a vCPU ioctl(),
arbitrary non-zero vCPU IDs, tests having to care about the vCPU ID in the
first place, etc...

The other theme in the rework is to deduplicate code and try to set us
up for success in the future. E.g. provide macros/helpers instead of
spamming CTRL-C => CTRL-V (see the -1k LoC), structure the VM creation
APIs to build on one another, etc...

The absurd patch count (as opposed to just ridiculous) is due to converting
each test away from using hardcoded vCPU IDs in a separate patch. The vast
majority of those patches probably aren't worth reviewing in depth, the
changes are mostly mechanical in nature.

However, _running_ non-x86 tests (or tests that have unique non-x86
behavior) would be extremely valuable. All patches have been compile tested
on x86, arm, risc-v, and s390, but I've only run the tests on x86. Based on
my track record for the x86+common tests, I will be very, very surprised if
I didn't break any of the non-x86 tests, e.g. pthread_create()'s 'void *'
param tripped me up multiple times.

I have not run x86's amx_test due to lack of hardware. I also haven't run
sev_migration; something is wonky in either the upstream support for INIT_EX
or in our test machines and I can't get SEV to initialize.

v2:
- Drop the forced -Werror patch. [Vitaly]
- Add TEST_REQUIRE to reduce KSFT_SKIP boilerplate.
- Rebase to kvm/queue, commit 55371f1d0c01.
- Clean up even more bad copy+paste code (x86 was hiding a lot of crud).
- Assert that the input to an ioctl() is (likely) the correct struct.

v1: https://lore.kernel.org/all/[email protected]

Sean Christopherson (144):
KVM: Fix references to non-existent KVM_CAP_TRIPLE_FAULT_EVENT
KVM: selftests: Fix buggy-but-benign check in
test_v3_new_redist_regions()
KVM: selftests: Fix typo in vgic_init test
KVM: selftests: Drop stale declarations from kvm_util_base.h
KVM: selftests: Always open VM file descriptors with O_RDWR
KVM: selftests: Add another underscore to inner ioctl() helpers
KVM: selftests: Make vcpu_ioctl() a wrapper to pretty print ioctl name
KVM: selftests: Drop @mode from common vm_create() helper
KVM: selftests: Split vcpu_set_nested_state() into two helpers
KVM: sefltests: Use vcpu_ioctl() and __vcpu_ioctl() helpers
KVM: selftests: Add __vcpu_run() helper
KVM: selftests: Use vcpu_access_device_attr() in arm64 code
KVM: selftests: Remove vcpu_get_fd()
KVM: selftests: Add vcpu_get() to retrieve and assert on vCPU
existence
KVM: selftests: Make vm_ioctl() a wrapper to pretty print ioctl name
KVM: sefltests: Use vm_ioctl() and __vm_ioctl() helpers
KVM: selftests: Make kvm_ioctl() a wrapper to pretty print ioctl name
KVM: selftests: Use kvm_ioctl() helpers
KVM: selftests: Use __KVM_SYSCALL_ERROR() to handle non-KVM syscall
errors
KVM: selftests: Make x86-64's register dump helpers static
KVM: selftests: Get rid of kvm_util_internal.h
KVM: selftests: Use KVM_IOCTL_ERROR() for one-off arm64 ioctls
KVM: selftests: Drop @test param from kvm_create_device()
KVM: selftests: Move KVM_CREATE_DEVICE_TEST code to separate helper
KVM: selftests: Multiplex return code and fd in __kvm_create_device()
KVM: selftests: Rename KVM_HAS_DEVICE_ATTR helpers for consistency
KVM: selftests: Drop 'int' return from asserting *_has_device_attr()
KVM: selftests: Split get/set device_attr helpers
KVM: selftests: Add a VM backpointer to 'struct vcpu'
KVM: selftests: Consolidate KVM_ENABLE_CAP usage
KVM: selftests: Simplify KVM_ENABLE_CAP helper APIs
KVM: selftests: Cache list of MSRs to save/restore
KVM: selftests: Harden and comment XSS / KVM_SET_MSRS interaction
KVM: selftests: Dedup MSR index list helpers, simplify dedicated test
KVM: selftests: Rename MP_STATE and GUEST_DEBUG helpers for
consistency
KVM: selftest: Add proper helpers for x86-specific save/restore ioctls
KVM: selftests: Add vm_create_*() variants to expose/return 'struct
vcpu'
KVM: selftests: Push vm_adjust_num_guest_pages() into "w/o vCPUs"
helper
KVM: selftests: Use vm_create_without_vcpus() in set_boot_cpu_id
KVM: selftests: Use vm_create_without_vcpus() in dirty_log_test
KVM: selftests: Use vm_create_without_vcpus() in hardware_disable_test
KVM: selftests: Use vm_create_without_vcpus() in psci_test
KVM: selftests: Rename vm_create() => vm_create_barebones(), drop
param
KVM: selftests: Rename vm_create_without_vcpus() => vm_create()
KVM: selftests: Make vm_create() a wrapper that specifies
VM_MODE_DEFAULT
KVM: selftests: Rename xAPIC state test's vcpu struct
KVM: selftests: Rename vcpu.state => vcpu.run
KVM: selftests: Rename 'struct vcpu' to 'struct kvm_vcpu'
KVM: selftests: Return the created vCPU from vm_vcpu_add()
KVM: selftests: Convert memslot_perf_test away from VCPU_ID
KVM: selftests: Convert rseq_test away from VCPU_ID
KVM: selftests: Convert xss_msr_test away from VCPU_ID
KVM: selftests: Convert vmx_preemption_timer_test away from VCPU_ID
KVM: selftests: Convert vmx_pmu_msrs_test away from VCPU_ID
KVM: selftests: Convert vmx_set_nested_state_test away from VCPU_ID
KVM: selftests: Convert vmx_tsc_adjust_test away from VCPU_ID
KVM: selftests: Convert mmu_role_test away from VCPU_ID
KVM: selftests: Convert pmu_event_filter_test away from VCPU_ID
KVM: selftests: Convert smm_test away from VCPU_ID
KVM: selftests: Convert state_test away from VCPU_ID
KVM: selftests: Convert svm_int_ctl_test away from VCPU_ID
KVM: selftests: Convert svm_vmcall_test away from VCPU_ID
KVM: selftests: Convert sync_regs_test away from VCPU_ID
KVM: selftests: Convert hyperv_cpuid away from VCPU_ID
KVM: selftests: Convert kvm_pv_test away from VCPU_ID
KVM: selftests: Convert platform_info_test away from VCPU_ID
KVM: selftests: Convert vmx_nested_tsc_scaling_test away from VCPU_ID
KVM: selftests: Convert set_sregs_test away from VCPU_ID
KVM: selftests: Convert vmx_dirty_log_test away from VCPU_ID
KVM: selftests: Convert vmx_close_while_nested_test away from VCPU_ID
KVM: selftests: Convert vmx_apic_access_test away from VCPU_ID
KVM: selftests: Convert userspace_msr_exit_test away from VCPU_ID
KVM: selftests: Convert vmx_exception_with_invalid_guest_state away
from VCPU_ID
KVM: selftests: Convert tsc_msrs_test away from VCPU_ID
KVM: selftests: Convert kvm_clock_test away from VCPU_ID
KVM: selftests: Convert hyperv_svm_test away from VCPU_ID
KVM: selftests: Convert hyperv_features away from VCPU_ID
KVM: selftests: Convert hyperv_clock away from VCPU_ID
KVM: selftests: Convert evmcs_test away from VCPU_ID
KVM: selftests: Convert emulator_error_test away from VCPU_ID
KVM: selftests: Convert debug_regs away from VCPU_ID
KVM: selftests: Add proper helper for advancing RIP in debug_regs
KVM: selftests: Convert amx_test away from VCPU_ID
KVM: selftests: Convert cr4_cpuid_sync_test away from VCPU_ID
KVM: selftests: Convert cpuid_test away from VCPU_ID
KVM: selftests: Convert userspace_io_test away from VCPU_ID
KVM: selftests: Convert vmx_invalid_nested_guest_state away from
VCPU_ID
KVM: selftests: Convert xen_vmcall_test away from VCPU_ID
KVM: selftests: Convert xen_shinfo_test away from VCPU_ID
KVM: selftests: Convert dirty_log_test away from VCPU_ID
KVM: selftests: Convert set_memory_region_test away from VCPU_ID
KVM: selftests: Convert system_counter_offset_test away from VCPU_ID
KVM: selftests: Track kvm_vcpu object in tsc_scaling_sync
KVM: selftests: Convert xapic_state_test away from hardcoded vCPU ID
KVM: selftests: Convert debug-exceptions away from VCPU_ID
KVM: selftests: Convert fix_hypercall_test away from VCPU_ID
KVM: selftests: Convert vgic_irq away from VCPU_ID
KVM: selftests: Make arm64's guest_get_vcpuid() declaration arm64-only
KVM: selftests: Move vm_is_unrestricted_guest() to x86-64
KVM: selftests: Add "arch" to common utils that have arch
implementations
KVM: selftests: Return created vcpu from vm_vcpu_add_default()
KVM: selftests: Rename vm_vcpu_add* helpers to better show
relationships
KVM: selftests: Convert set_boot_cpu_id away from global VCPU_IDs
KVM: selftests: Convert psci_test away from VCPU_ID
KVM: selftests: Convert hardware_disable_test to pass around vCPU
objects
KVM: selftests: Add VM creation helper that "returns" vCPUs
KVM: selftests: Convert steal_time away from VCPU_ID
KVM: selftests: Convert arch_timer away from VCPU_ID
KVM: selftests: Convert svm_nested_soft_inject_test away from VCPU_ID
KVM: selftests: Convert triple_fault_event_test away from VCPU_ID
KVM: selftests: Convert vgic_init away from
vm_create_default_with_vcpus()
KVM: selftests: Consolidate KVM_{G,S}ET_ONE_REG helpers
KVM: selftests: Sync stage before VM is freed in hypercalls test
KVM: selftests: Convert hypercalls test away from vm_create_default()
KVM: selftests: Convert xapic_ipi_test away from *_VCPU_ID
KVM: selftests: Convert sync_regs_test away from VCPU_ID
KVM: selftests: Convert s390's "resets" test away from VCPU_ID
KVM: selftests: Convert memop away from VCPU_ID
KVM: selftests: Convert s390x/diag318_test_handler away from VCPU_ID
KVM: selftests: Convert tprot away from VCPU_ID
KVM: selftests: Use vm_create() in tsc_scaling_sync
KVM: selftests: Use vm_create_with_vcpus() in max_guest_memory_test
KVM: selftests: Drop vm_create_default* helpers
KVM: selftests: Drop @vcpuids param from VM creators
KVM: selftests: Convert kvm_page_table_test away from reliance on
vcpu_id
KVM: selftests: Convert kvm_binary_stats_test away from vCPU IDs
KVM: selftests: Convert get-reg-list away from its "VCPU_ID"
KVM: selftests: Stop hardcoding vCPU IDs in vcpu_width_config
KVM: selftests: Stop conflating vCPU index and ID in perf tests
KVM: selftests: Remove vcpu_get() usage from dirty_log_test
KVM: selftests: Require vCPU output array when creating VM with vCPUs
KVM: selftests: Purge vm+vcpu_id == vcpu silliness
KVM: selftests: Drop vcpu_get(), rename vcpu_find() => vcpu_exists()
KVM: selftests: Remove vcpu_state() helper
KVM: selftests: Open code and drop 'struct kvm_vm' accessors
KVM: selftests: Drop @slot0_mem_pages from __vm_create_with_vcpus()
KVM: selftests: Drop @num_percpu_pages from __vm_create_with_vcpus()
KVM: selftests: Move per-VM/per-vCPU nr pages calculation to
__vm_create()
KVM: selftests: Trust that MAXPHYADDR > memslot0 in
vmx_apic_access_test
KVM: selftests: Drop DEFAULT_GUEST_PHY_PAGES, open code the magic
number
KVM: selftests: Return an 'unsigned int' from kvm_check_cap()
KVM: selftests: Add kvm_has_cap() to provide syntactic sugar
KVM: selftests: Add TEST_REQUIRE macros to reduce skipping copy+paste
KVM: selftests: Sanity check input to ioctls() at build time

Documentation/virt/kvm/api.rst | 4 +-
.../selftests/kvm/aarch64/arch_timer.c | 79 +-
.../selftests/kvm/aarch64/debug-exceptions.c | 22 +-
.../selftests/kvm/aarch64/get-reg-list.c | 29 +-
.../selftests/kvm/aarch64/hypercalls.c | 90 +-
.../testing/selftests/kvm/aarch64/psci_test.c | 69 +-
.../selftests/kvm/aarch64/vcpu_width_config.c | 71 +-
.../testing/selftests/kvm/aarch64/vgic_init.c | 379 +++---
.../testing/selftests/kvm/aarch64/vgic_irq.c | 40 +-
.../selftests/kvm/access_tracking_perf_test.c | 92 +-
.../selftests/kvm/demand_paging_test.c | 49 +-
.../selftests/kvm/dirty_log_perf_test.c | 51 +-
tools/testing/selftests/kvm/dirty_log_test.c | 95 +-
.../selftests/kvm/hardware_disable_test.c | 29 +-
.../selftests/kvm/include/aarch64/processor.h | 28 +-
.../selftests/kvm/include/aarch64/vgic.h | 6 +-
.../selftests/kvm/include/kvm_util_base.h | 743 ++++++++---
.../selftests/kvm/include/perf_test_util.h | 5 +-
.../selftests/kvm/include/riscv/processor.h | 20 -
.../testing/selftests/kvm/include/test_util.h | 9 +
.../selftests/kvm/include/ucall_common.h | 2 +-
.../selftests/kvm/include/x86_64/evmcs.h | 2 +-
.../selftests/kvm/include/x86_64/processor.h | 109 +-
.../selftests/kvm/kvm_binary_stats_test.c | 31 +-
.../selftests/kvm/kvm_create_max_vcpus.c | 10 +-
.../selftests/kvm/kvm_page_table_test.c | 66 +-
.../selftests/kvm/lib/aarch64/processor.c | 81 +-
.../testing/selftests/kvm/lib/aarch64/ucall.c | 9 +-
.../testing/selftests/kvm/lib/aarch64/vgic.c | 54 +-
tools/testing/selftests/kvm/lib/elf.c | 1 -
tools/testing/selftests/kvm/lib/guest_modes.c | 6 +-
tools/testing/selftests/kvm/lib/kvm_util.c | 1104 +++--------------
.../selftests/kvm/lib/kvm_util_internal.h | 128 --
.../selftests/kvm/lib/perf_test_util.c | 84 +-
.../selftests/kvm/lib/riscv/processor.c | 111 +-
tools/testing/selftests/kvm/lib/riscv/ucall.c | 14 +-
.../kvm/lib/s390x/diag318_test_handler.c | 11 +-
.../selftests/kvm/lib/s390x/processor.c | 44 +-
tools/testing/selftests/kvm/lib/s390x/ucall.c | 8 +-
.../selftests/kvm/lib/x86_64/processor.c | 533 +++-----
tools/testing/selftests/kvm/lib/x86_64/svm.c | 6 +-
.../testing/selftests/kvm/lib/x86_64/ucall.c | 10 +-
tools/testing/selftests/kvm/lib/x86_64/vmx.c | 16 +-
.../selftests/kvm/max_guest_memory_test.c | 53 +-
.../kvm/memslot_modification_stress_test.c | 13 +-
.../testing/selftests/kvm/memslot_perf_test.c | 28 +-
tools/testing/selftests/kvm/rseq_test.c | 22 +-
tools/testing/selftests/kvm/s390x/memop.c | 93 +-
tools/testing/selftests/kvm/s390x/resets.c | 140 ++-
.../selftests/kvm/s390x/sync_regs_test.c | 45 +-
tools/testing/selftests/kvm/s390x/tprot.c | 25 +-
.../selftests/kvm/set_memory_region_test.c | 43 +-
tools/testing/selftests/kvm/steal_time.c | 120 +-
.../kvm/system_counter_offset_test.c | 35 +-
tools/testing/selftests/kvm/x86_64/amx_test.c | 56 +-
.../testing/selftests/kvm/x86_64/cpuid_test.c | 29 +-
.../kvm/x86_64/cr4_cpuid_sync_test.c | 22 +-
.../testing/selftests/kvm/x86_64/debug_regs.c | 77 +-
.../kvm/x86_64/emulator_error_test.c | 74 +-
.../testing/selftests/kvm/x86_64/evmcs_test.c | 61 +-
.../selftests/kvm/x86_64/fix_hypercall_test.c | 45 +-
.../kvm/x86_64/get_msr_index_features.c | 117 +-
.../selftests/kvm/x86_64/hyperv_clock.c | 25 +-
.../selftests/kvm/x86_64/hyperv_cpuid.c | 34 +-
.../selftests/kvm/x86_64/hyperv_features.c | 61 +-
.../selftests/kvm/x86_64/hyperv_svm_test.c | 20 +-
.../selftests/kvm/x86_64/kvm_clock_test.c | 29 +-
.../selftests/kvm/x86_64/kvm_pv_test.c | 33 +-
.../kvm/x86_64/max_vcpuid_cap_test.c | 28 +-
.../selftests/kvm/x86_64/mmio_warning_test.c | 16 +-
.../selftests/kvm/x86_64/mmu_role_test.c | 30 +-
.../selftests/kvm/x86_64/platform_info_test.c | 51 +-
.../kvm/x86_64/pmu_event_filter_test.c | 97 +-
.../selftests/kvm/x86_64/set_boot_cpu_id.c | 91 +-
.../selftests/kvm/x86_64/set_sregs_test.c | 47 +-
.../selftests/kvm/x86_64/sev_migrate_tests.c | 120 +-
tools/testing/selftests/kvm/x86_64/smm_test.c | 37 +-
.../testing/selftests/kvm/x86_64/state_test.c | 29 +-
.../selftests/kvm/x86_64/svm_int_ctl_test.c | 21 +-
.../kvm/x86_64/svm_nested_soft_inject_test.c | 17 +-
.../selftests/kvm/x86_64/svm_vmcall_test.c | 16 +-
.../selftests/kvm/x86_64/sync_regs_test.c | 62 +-
.../kvm/x86_64/triple_fault_event_test.c | 39 +-
.../selftests/kvm/x86_64/tsc_msrs_test.c | 35 +-
.../selftests/kvm/x86_64/tsc_scaling_sync.c | 25 +-
.../selftests/kvm/x86_64/userspace_io_test.c | 18 +-
.../kvm/x86_64/userspace_msr_exit_test.c | 187 ++-
.../kvm/x86_64/vmx_apic_access_test.c | 27 +-
.../kvm/x86_64/vmx_close_while_nested_test.c | 17 +-
.../selftests/kvm/x86_64/vmx_dirty_log_test.c | 13 +-
.../vmx_exception_with_invalid_guest_state.c | 68 +-
.../x86_64/vmx_invalid_nested_guest_state.c | 18 +-
.../kvm/x86_64/vmx_nested_tsc_scaling_test.c | 29 +-
.../selftests/kvm/x86_64/vmx_pmu_caps_test.c | 48 +-
.../kvm/x86_64/vmx_preemption_timer_test.c | 35 +-
.../kvm/x86_64/vmx_set_nested_state_test.c | 91 +-
.../kvm/x86_64/vmx_tsc_adjust_test.c | 13 +-
.../selftests/kvm/x86_64/xapic_ipi_test.c | 48 +-
.../selftests/kvm/x86_64/xapic_state_test.c | 60 +-
.../selftests/kvm/x86_64/xen_shinfo_test.c | 73 +-
.../selftests/kvm/x86_64/xen_vmcall_test.c | 25 +-
.../selftests/kvm/x86_64/xss_msr_test.c | 56 +-
102 files changed, 3059 insertions(+), 4178 deletions(-)
delete mode 100644 tools/testing/selftests/kvm/lib/kvm_util_internal.h


base-commit: 55371f1d0c01357f29da613f7525c3f252320bbf
--
2.36.1.255.ge46751e96f-goog


2022-06-06 06:19:31

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH v2 015/144] KVM: selftests: Make vm_ioctl() a wrapper to pretty print ioctl name

Make vm_ioctl() a macro wrapper and print the _name_ of the ioctl on
failure instead of the number.

Deliberately do not use __stringify(), as that will expand the ioctl all
the way down to its numerical sequence. Again the intent is to print the
name of the macro.

Signed-off-by: Sean Christopherson <[email protected]>
---
.../selftests/kvm/include/kvm_util_base.h | 38 ++++++++++---------
tools/testing/selftests/kvm/lib/kvm_util.c | 28 ++++----------
2 files changed, 28 insertions(+), 38 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
index c2dfc4341b31..39e1971e5d65 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_base.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
@@ -105,6 +105,27 @@ int open_kvm_dev_path_or_exit(void);
int kvm_check_cap(long cap);
int vm_check_cap(struct kvm_vm *vm, long cap);
int vm_enable_cap(struct kvm_vm *vm, struct kvm_enable_cap *cap);
+
+#define __KVM_SYSCALL_ERROR(_name, _ret) \
+ "%s failed, rc: %i errno: %i (%s)", (_name), (_ret), errno, strerror(errno)
+
+#define __KVM_IOCTL_ERROR(_name, _ret) __KVM_SYSCALL_ERROR(_name, _ret)
+#define KVM_IOCTL_ERROR(_ioctl, _ret) __KVM_IOCTL_ERROR(#_ioctl, _ret)
+
+int __kvm_ioctl(struct kvm_vm *vm, unsigned long cmd, void *arg);
+void kvm_ioctl(struct kvm_vm *vm, unsigned long cmd, void *arg);
+
+int __vm_ioctl(struct kvm_vm *vm, unsigned long cmd, void *arg);
+void _vm_ioctl(struct kvm_vm *vm, unsigned long cmd, const char *name, void *arg);
+#define vm_ioctl(vm, cmd, arg) _vm_ioctl(vm, cmd, #cmd, arg)
+
+int __vcpu_ioctl(struct kvm_vm *vm, uint32_t vcpuid, unsigned long cmd,
+ void *arg);
+void _vcpu_ioctl(struct kvm_vm *vm, uint32_t vcpuid, unsigned long cmd,
+ const char *name, void *arg);
+#define vcpu_ioctl(vm, vcpuid, cmd, arg) \
+ _vcpu_ioctl(vm, vcpuid, cmd, #cmd, arg)
+
void vm_enable_dirty_ring(struct kvm_vm *vm, uint32_t ring_size);
const char *vm_guest_mode_string(uint32_t i);

@@ -156,23 +177,6 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
uint64_t guest_paddr, uint32_t slot, uint64_t npages,
uint32_t flags);

-#define __KVM_SYSCALL_ERROR(_name, _ret) \
- "%s failed, rc: %i errno: %i (%s)", (_name), (_ret), errno, strerror(errno)
-
-#define __KVM_IOCTL_ERROR(_name, _ret) __KVM_SYSCALL_ERROR(_name, _ret)
-#define KVM_IOCTL_ERROR(_ioctl, _ret) __KVM_IOCTL_ERROR(#_ioctl, _ret)
-
-void _vcpu_ioctl(struct kvm_vm *vm, uint32_t vcpuid, unsigned long ioctl,
- const char *name, void *arg);
-int __vcpu_ioctl(struct kvm_vm *vm, uint32_t vcpuid, unsigned long ioctl,
- void *arg);
-#define vcpu_ioctl(vm, vcpuid, ioctl, arg) \
- _vcpu_ioctl(vm, vcpuid, ioctl, #ioctl, arg)
-
-void vm_ioctl(struct kvm_vm *vm, unsigned long ioctl, void *arg);
-int __vm_ioctl(struct kvm_vm *vm, unsigned long cmd, void *arg);
-void kvm_ioctl(struct kvm_vm *vm, unsigned long ioctl, void *arg);
-int __kvm_ioctl(struct kvm_vm *vm, unsigned long ioctl, void *arg);
void vm_mem_region_set_flags(struct kvm_vm *vm, uint32_t slot, uint32_t flags);
void vm_mem_region_move(struct kvm_vm *vm, uint32_t slot, uint64_t new_gpa);
void vm_mem_region_delete(struct kvm_vm *vm, uint32_t slot);
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 940decfaa633..7eedd9ff20fa 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -1690,32 +1690,18 @@ void *vcpu_map_dirty_ring(struct kvm_vm *vm, uint32_t vcpuid)
return vcpu->dirty_gfns;
}

-/*
- * VM Ioctl
- *
- * Input Args:
- * vm - Virtual Machine
- * cmd - Ioctl number
- * arg - Argument to pass to the ioctl
- *
- * Return: None
- *
- * Issues an arbitrary ioctl on a VM fd.
- */
-void vm_ioctl(struct kvm_vm *vm, unsigned long cmd, void *arg)
-{
- int ret;
-
- ret = __vm_ioctl(vm, cmd, arg);
- TEST_ASSERT(ret == 0, "vm ioctl %lu failed, rc: %i errno: %i (%s)",
- cmd, ret, errno, strerror(errno));
-}
-
int __vm_ioctl(struct kvm_vm *vm, unsigned long cmd, void *arg)
{
return ioctl(vm->fd, cmd, arg);
}

+void _vm_ioctl(struct kvm_vm *vm, unsigned long cmd, const char *name, void *arg)
+{
+ int ret = __vm_ioctl(vm, cmd, arg);
+
+ TEST_ASSERT(!ret, __KVM_IOCTL_ERROR(name, ret));
+}
+
/*
* KVM system ioctl
*
--
2.36.1.255.ge46751e96f-goog

2022-06-06 06:19:33

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH v2 137/144] KVM: selftests: Drop @num_percpu_pages from __vm_create_with_vcpus()

Drop @num_percpu_pages from __vm_create_with_vcpus(), all callers pass
'0' and there's unlikely to be a test that allocates just enough memory
that it needs a per-CPU allocation, but not so much that it won't just do
its own memory management.

Signed-off-by: Sean Christopherson <[email protected]>
---
tools/testing/selftests/kvm/include/kvm_util_base.h | 4 ++--
tools/testing/selftests/kvm/kvm_page_table_test.c | 2 +-
tools/testing/selftests/kvm/lib/kvm_util.c | 7 +++----
tools/testing/selftests/kvm/lib/perf_test_util.c | 2 +-
4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
index f84e01612c52..6143d45a02a7 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_base.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
@@ -562,14 +562,14 @@ static inline struct kvm_vm *vm_create(uint64_t nr_pages)
}

struct kvm_vm *__vm_create_with_vcpus(enum vm_guest_mode mode, uint32_t nr_vcpus,
- uint64_t extra_mem_pages, uint32_t num_percpu_pages,
+ uint64_t extra_mem_pages,
void *guest_code, struct kvm_vcpu *vcpus[]);

static inline struct kvm_vm *vm_create_with_vcpus(uint32_t nr_vcpus,
void *guest_code,
struct kvm_vcpu *vcpus[])
{
- return __vm_create_with_vcpus(VM_MODE_DEFAULT, nr_vcpus, 0, 0,
+ return __vm_create_with_vcpus(VM_MODE_DEFAULT, nr_vcpus, 0,
guest_code, vcpus);
}

diff --git a/tools/testing/selftests/kvm/kvm_page_table_test.c b/tools/testing/selftests/kvm/kvm_page_table_test.c
index a68c57572ab4..f42c6ac6d71d 100644
--- a/tools/testing/selftests/kvm/kvm_page_table_test.c
+++ b/tools/testing/selftests/kvm/kvm_page_table_test.c
@@ -254,7 +254,7 @@ static struct kvm_vm *pre_init_before_test(enum vm_guest_mode mode, void *arg)

/* Create a VM with enough guest pages */
guest_num_pages = test_mem_size / guest_page_size;
- vm = __vm_create_with_vcpus(mode, nr_vcpus, guest_num_pages, 0,
+ vm = __vm_create_with_vcpus(mode, nr_vcpus, guest_num_pages,
guest_code, test_args.vcpus);

/* Align down GPA of the testing memslot */
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index f68234a2ee83..508a5eafe15b 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -281,7 +281,6 @@ struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint64_t nr_pages)
* mode - VM Mode (e.g. VM_MODE_P52V48_4K)
* nr_vcpus - VCPU count
* extra_mem_pages - Non-slot0 physical memory total size
- * num_percpu_pages - Per-cpu physical memory pages
* guest_code - Guest entry point
* vcpuids - VCPU IDs
*
@@ -295,7 +294,7 @@ struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint64_t nr_pages)
* no real memory allocation for non-slot0 memory in this function.
*/
struct kvm_vm *__vm_create_with_vcpus(enum vm_guest_mode mode, uint32_t nr_vcpus,
- uint64_t extra_mem_pages, uint32_t num_percpu_pages,
+ uint64_t extra_mem_pages,
void *guest_code, struct kvm_vcpu *vcpus[])
{
uint64_t vcpu_pages, extra_pg_pages, pages;
@@ -310,7 +309,7 @@ struct kvm_vm *__vm_create_with_vcpus(enum vm_guest_mode mode, uint32_t nr_vcpus
* N pages) will be: N/x+N/x^2+N/x^3+... which is definitely smaller
* than N/x*2.
*/
- vcpu_pages = (DEFAULT_STACK_PGS + num_percpu_pages) * nr_vcpus;
+ vcpu_pages = nr_vcpus * DEFAULT_STACK_PGS;
extra_pg_pages = (DEFAULT_GUEST_PHY_PAGES + extra_mem_pages + vcpu_pages) / PTES_PER_MIN_PAGE * 2;
pages = DEFAULT_GUEST_PHY_PAGES + vcpu_pages + extra_pg_pages;

@@ -333,7 +332,7 @@ struct kvm_vm *__vm_create_with_one_vcpu(struct kvm_vcpu **vcpu,
struct kvm_vcpu *vcpus[1];
struct kvm_vm *vm;

- vm = __vm_create_with_vcpus(VM_MODE_DEFAULT, 1, extra_mem_pages, 0,
+ vm = __vm_create_with_vcpus(VM_MODE_DEFAULT, 1, extra_mem_pages,
guest_code, vcpus);

*vcpu = vcpus[0];
diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
index 656f309584aa..1f25ed69ca98 100644
--- a/tools/testing/selftests/kvm/lib/perf_test_util.c
+++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
@@ -144,7 +144,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int nr_vcpus,
* The memory is also added to memslot 0, but that's a benign side
* effect as KVM allows aliasing HVAs in meslots.
*/
- vm = __vm_create_with_vcpus(mode, nr_vcpus, guest_num_pages, 0,
+ vm = __vm_create_with_vcpus(mode, nr_vcpus, guest_num_pages,
guest_code, vcpus);

pta->vm = vm;
--
2.36.1.255.ge46751e96f-goog

2022-06-08 03:16:02

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

Marc, Christian, Anup, can you please give this a go?

Paolo

On 6/3/22 02:41, Sean Christopherson wrote:
> Overhaul KVM's selftest APIs to get selftests to a state where adding new
> features and writing tests is less painful/disgusting.
>
> Patches 1 fixes a goof in kvm/queue and should be squashed.
>
> I would really, really, really like to get this queued up sooner than
> later, or maybe just thrown into a separate selftests-specific branch that
> folks can develop against. Rebasing is tedious, frustrating, and time
> consuming. And spoiler alert, there's another 42 x86-centric patches
> inbound that builds on this series to clean up CPUID related crud...
>
> The primary theme is to stop treating tests like second class citizens.
> Stop hiding vcpu, kvm_vm, etc... There's no sensitive data/constructs, and
> the encapsulation has led to really, really bad and difficult to maintain
> code. E.g. having to pass around the VM just to call a vCPU ioctl(),
> arbitrary non-zero vCPU IDs, tests having to care about the vCPU ID in the
> first place, etc...
>
> The other theme in the rework is to deduplicate code and try to set us
> up for success in the future. E.g. provide macros/helpers instead of
> spamming CTRL-C => CTRL-V (see the -1k LoC), structure the VM creation
> APIs to build on one another, etc...
>
> The absurd patch count (as opposed to just ridiculous) is due to converting
> each test away from using hardcoded vCPU IDs in a separate patch. The vast
> majority of those patches probably aren't worth reviewing in depth, the
> changes are mostly mechanical in nature.
>
> However, _running_ non-x86 tests (or tests that have unique non-x86
> behavior) would be extremely valuable. All patches have been compile tested
> on x86, arm, risc-v, and s390, but I've only run the tests on x86. Based on
> my track record for the x86+common tests, I will be very, very surprised if
> I didn't break any of the non-x86 tests, e.g. pthread_create()'s 'void *'
> param tripped me up multiple times.
>
> I have not run x86's amx_test due to lack of hardware. I also haven't run
> sev_migration; something is wonky in either the upstream support for INIT_EX
> or in our test machines and I can't get SEV to initialize.
>
> v2:
> - Drop the forced -Werror patch. [Vitaly]
> - Add TEST_REQUIRE to reduce KSFT_SKIP boilerplate.
> - Rebase to kvm/queue, commit 55371f1d0c01.
> - Clean up even more bad copy+paste code (x86 was hiding a lot of crud).
> - Assert that the input to an ioctl() is (likely) the correct struct.
>
> v1: https://lore.kernel.org/all/[email protected]
>
> Sean Christopherson (144):
> KVM: Fix references to non-existent KVM_CAP_TRIPLE_FAULT_EVENT
> KVM: selftests: Fix buggy-but-benign check in
> test_v3_new_redist_regions()
> KVM: selftests: Fix typo in vgic_init test
> KVM: selftests: Drop stale declarations from kvm_util_base.h
> KVM: selftests: Always open VM file descriptors with O_RDWR
> KVM: selftests: Add another underscore to inner ioctl() helpers
> KVM: selftests: Make vcpu_ioctl() a wrapper to pretty print ioctl name
> KVM: selftests: Drop @mode from common vm_create() helper
> KVM: selftests: Split vcpu_set_nested_state() into two helpers
> KVM: sefltests: Use vcpu_ioctl() and __vcpu_ioctl() helpers
> KVM: selftests: Add __vcpu_run() helper
> KVM: selftests: Use vcpu_access_device_attr() in arm64 code
> KVM: selftests: Remove vcpu_get_fd()
> KVM: selftests: Add vcpu_get() to retrieve and assert on vCPU
> existence
> KVM: selftests: Make vm_ioctl() a wrapper to pretty print ioctl name
> KVM: sefltests: Use vm_ioctl() and __vm_ioctl() helpers
> KVM: selftests: Make kvm_ioctl() a wrapper to pretty print ioctl name
> KVM: selftests: Use kvm_ioctl() helpers
> KVM: selftests: Use __KVM_SYSCALL_ERROR() to handle non-KVM syscall
> errors
> KVM: selftests: Make x86-64's register dump helpers static
> KVM: selftests: Get rid of kvm_util_internal.h
> KVM: selftests: Use KVM_IOCTL_ERROR() for one-off arm64 ioctls
> KVM: selftests: Drop @test param from kvm_create_device()
> KVM: selftests: Move KVM_CREATE_DEVICE_TEST code to separate helper
> KVM: selftests: Multiplex return code and fd in __kvm_create_device()
> KVM: selftests: Rename KVM_HAS_DEVICE_ATTR helpers for consistency
> KVM: selftests: Drop 'int' return from asserting *_has_device_attr()
> KVM: selftests: Split get/set device_attr helpers
> KVM: selftests: Add a VM backpointer to 'struct vcpu'
> KVM: selftests: Consolidate KVM_ENABLE_CAP usage
> KVM: selftests: Simplify KVM_ENABLE_CAP helper APIs
> KVM: selftests: Cache list of MSRs to save/restore
> KVM: selftests: Harden and comment XSS / KVM_SET_MSRS interaction
> KVM: selftests: Dedup MSR index list helpers, simplify dedicated test
> KVM: selftests: Rename MP_STATE and GUEST_DEBUG helpers for
> consistency
> KVM: selftest: Add proper helpers for x86-specific save/restore ioctls
> KVM: selftests: Add vm_create_*() variants to expose/return 'struct
> vcpu'
> KVM: selftests: Push vm_adjust_num_guest_pages() into "w/o vCPUs"
> helper
> KVM: selftests: Use vm_create_without_vcpus() in set_boot_cpu_id
> KVM: selftests: Use vm_create_without_vcpus() in dirty_log_test
> KVM: selftests: Use vm_create_without_vcpus() in hardware_disable_test
> KVM: selftests: Use vm_create_without_vcpus() in psci_test
> KVM: selftests: Rename vm_create() => vm_create_barebones(), drop
> param
> KVM: selftests: Rename vm_create_without_vcpus() => vm_create()
> KVM: selftests: Make vm_create() a wrapper that specifies
> VM_MODE_DEFAULT
> KVM: selftests: Rename xAPIC state test's vcpu struct
> KVM: selftests: Rename vcpu.state => vcpu.run
> KVM: selftests: Rename 'struct vcpu' to 'struct kvm_vcpu'
> KVM: selftests: Return the created vCPU from vm_vcpu_add()
> KVM: selftests: Convert memslot_perf_test away from VCPU_ID
> KVM: selftests: Convert rseq_test away from VCPU_ID
> KVM: selftests: Convert xss_msr_test away from VCPU_ID
> KVM: selftests: Convert vmx_preemption_timer_test away from VCPU_ID
> KVM: selftests: Convert vmx_pmu_msrs_test away from VCPU_ID
> KVM: selftests: Convert vmx_set_nested_state_test away from VCPU_ID
> KVM: selftests: Convert vmx_tsc_adjust_test away from VCPU_ID
> KVM: selftests: Convert mmu_role_test away from VCPU_ID
> KVM: selftests: Convert pmu_event_filter_test away from VCPU_ID
> KVM: selftests: Convert smm_test away from VCPU_ID
> KVM: selftests: Convert state_test away from VCPU_ID
> KVM: selftests: Convert svm_int_ctl_test away from VCPU_ID
> KVM: selftests: Convert svm_vmcall_test away from VCPU_ID
> KVM: selftests: Convert sync_regs_test away from VCPU_ID
> KVM: selftests: Convert hyperv_cpuid away from VCPU_ID
> KVM: selftests: Convert kvm_pv_test away from VCPU_ID
> KVM: selftests: Convert platform_info_test away from VCPU_ID
> KVM: selftests: Convert vmx_nested_tsc_scaling_test away from VCPU_ID
> KVM: selftests: Convert set_sregs_test away from VCPU_ID
> KVM: selftests: Convert vmx_dirty_log_test away from VCPU_ID
> KVM: selftests: Convert vmx_close_while_nested_test away from VCPU_ID
> KVM: selftests: Convert vmx_apic_access_test away from VCPU_ID
> KVM: selftests: Convert userspace_msr_exit_test away from VCPU_ID
> KVM: selftests: Convert vmx_exception_with_invalid_guest_state away
> from VCPU_ID
> KVM: selftests: Convert tsc_msrs_test away from VCPU_ID
> KVM: selftests: Convert kvm_clock_test away from VCPU_ID
> KVM: selftests: Convert hyperv_svm_test away from VCPU_ID
> KVM: selftests: Convert hyperv_features away from VCPU_ID
> KVM: selftests: Convert hyperv_clock away from VCPU_ID
> KVM: selftests: Convert evmcs_test away from VCPU_ID
> KVM: selftests: Convert emulator_error_test away from VCPU_ID
> KVM: selftests: Convert debug_regs away from VCPU_ID
> KVM: selftests: Add proper helper for advancing RIP in debug_regs
> KVM: selftests: Convert amx_test away from VCPU_ID
> KVM: selftests: Convert cr4_cpuid_sync_test away from VCPU_ID
> KVM: selftests: Convert cpuid_test away from VCPU_ID
> KVM: selftests: Convert userspace_io_test away from VCPU_ID
> KVM: selftests: Convert vmx_invalid_nested_guest_state away from
> VCPU_ID
> KVM: selftests: Convert xen_vmcall_test away from VCPU_ID
> KVM: selftests: Convert xen_shinfo_test away from VCPU_ID
> KVM: selftests: Convert dirty_log_test away from VCPU_ID
> KVM: selftests: Convert set_memory_region_test away from VCPU_ID
> KVM: selftests: Convert system_counter_offset_test away from VCPU_ID
> KVM: selftests: Track kvm_vcpu object in tsc_scaling_sync
> KVM: selftests: Convert xapic_state_test away from hardcoded vCPU ID
> KVM: selftests: Convert debug-exceptions away from VCPU_ID
> KVM: selftests: Convert fix_hypercall_test away from VCPU_ID
> KVM: selftests: Convert vgic_irq away from VCPU_ID
> KVM: selftests: Make arm64's guest_get_vcpuid() declaration arm64-only
> KVM: selftests: Move vm_is_unrestricted_guest() to x86-64
> KVM: selftests: Add "arch" to common utils that have arch
> implementations
> KVM: selftests: Return created vcpu from vm_vcpu_add_default()
> KVM: selftests: Rename vm_vcpu_add* helpers to better show
> relationships
> KVM: selftests: Convert set_boot_cpu_id away from global VCPU_IDs
> KVM: selftests: Convert psci_test away from VCPU_ID
> KVM: selftests: Convert hardware_disable_test to pass around vCPU
> objects
> KVM: selftests: Add VM creation helper that "returns" vCPUs
> KVM: selftests: Convert steal_time away from VCPU_ID
> KVM: selftests: Convert arch_timer away from VCPU_ID
> KVM: selftests: Convert svm_nested_soft_inject_test away from VCPU_ID
> KVM: selftests: Convert triple_fault_event_test away from VCPU_ID
> KVM: selftests: Convert vgic_init away from
> vm_create_default_with_vcpus()
> KVM: selftests: Consolidate KVM_{G,S}ET_ONE_REG helpers
> KVM: selftests: Sync stage before VM is freed in hypercalls test
> KVM: selftests: Convert hypercalls test away from vm_create_default()
> KVM: selftests: Convert xapic_ipi_test away from *_VCPU_ID
> KVM: selftests: Convert sync_regs_test away from VCPU_ID
> KVM: selftests: Convert s390's "resets" test away from VCPU_ID
> KVM: selftests: Convert memop away from VCPU_ID
> KVM: selftests: Convert s390x/diag318_test_handler away from VCPU_ID
> KVM: selftests: Convert tprot away from VCPU_ID
> KVM: selftests: Use vm_create() in tsc_scaling_sync
> KVM: selftests: Use vm_create_with_vcpus() in max_guest_memory_test
> KVM: selftests: Drop vm_create_default* helpers
> KVM: selftests: Drop @vcpuids param from VM creators
> KVM: selftests: Convert kvm_page_table_test away from reliance on
> vcpu_id
> KVM: selftests: Convert kvm_binary_stats_test away from vCPU IDs
> KVM: selftests: Convert get-reg-list away from its "VCPU_ID"
> KVM: selftests: Stop hardcoding vCPU IDs in vcpu_width_config
> KVM: selftests: Stop conflating vCPU index and ID in perf tests
> KVM: selftests: Remove vcpu_get() usage from dirty_log_test
> KVM: selftests: Require vCPU output array when creating VM with vCPUs
> KVM: selftests: Purge vm+vcpu_id == vcpu silliness
> KVM: selftests: Drop vcpu_get(), rename vcpu_find() => vcpu_exists()
> KVM: selftests: Remove vcpu_state() helper
> KVM: selftests: Open code and drop 'struct kvm_vm' accessors
> KVM: selftests: Drop @slot0_mem_pages from __vm_create_with_vcpus()
> KVM: selftests: Drop @num_percpu_pages from __vm_create_with_vcpus()
> KVM: selftests: Move per-VM/per-vCPU nr pages calculation to
> __vm_create()
> KVM: selftests: Trust that MAXPHYADDR > memslot0 in
> vmx_apic_access_test
> KVM: selftests: Drop DEFAULT_GUEST_PHY_PAGES, open code the magic
> number
> KVM: selftests: Return an 'unsigned int' from kvm_check_cap()
> KVM: selftests: Add kvm_has_cap() to provide syntactic sugar
> KVM: selftests: Add TEST_REQUIRE macros to reduce skipping copy+paste
> KVM: selftests: Sanity check input to ioctls() at build time
>
> Documentation/virt/kvm/api.rst | 4 +-
> .../selftests/kvm/aarch64/arch_timer.c | 79 +-
> .../selftests/kvm/aarch64/debug-exceptions.c | 22 +-
> .../selftests/kvm/aarch64/get-reg-list.c | 29 +-
> .../selftests/kvm/aarch64/hypercalls.c | 90 +-
> .../testing/selftests/kvm/aarch64/psci_test.c | 69 +-
> .../selftests/kvm/aarch64/vcpu_width_config.c | 71 +-
> .../testing/selftests/kvm/aarch64/vgic_init.c | 379 +++---
> .../testing/selftests/kvm/aarch64/vgic_irq.c | 40 +-
> .../selftests/kvm/access_tracking_perf_test.c | 92 +-
> .../selftests/kvm/demand_paging_test.c | 49 +-
> .../selftests/kvm/dirty_log_perf_test.c | 51 +-
> tools/testing/selftests/kvm/dirty_log_test.c | 95 +-
> .../selftests/kvm/hardware_disable_test.c | 29 +-
> .../selftests/kvm/include/aarch64/processor.h | 28 +-
> .../selftests/kvm/include/aarch64/vgic.h | 6 +-
> .../selftests/kvm/include/kvm_util_base.h | 743 ++++++++---
> .../selftests/kvm/include/perf_test_util.h | 5 +-
> .../selftests/kvm/include/riscv/processor.h | 20 -
> .../testing/selftests/kvm/include/test_util.h | 9 +
> .../selftests/kvm/include/ucall_common.h | 2 +-
> .../selftests/kvm/include/x86_64/evmcs.h | 2 +-
> .../selftests/kvm/include/x86_64/processor.h | 109 +-
> .../selftests/kvm/kvm_binary_stats_test.c | 31 +-
> .../selftests/kvm/kvm_create_max_vcpus.c | 10 +-
> .../selftests/kvm/kvm_page_table_test.c | 66 +-
> .../selftests/kvm/lib/aarch64/processor.c | 81 +-
> .../testing/selftests/kvm/lib/aarch64/ucall.c | 9 +-
> .../testing/selftests/kvm/lib/aarch64/vgic.c | 54 +-
> tools/testing/selftests/kvm/lib/elf.c | 1 -
> tools/testing/selftests/kvm/lib/guest_modes.c | 6 +-
> tools/testing/selftests/kvm/lib/kvm_util.c | 1104 +++--------------
> .../selftests/kvm/lib/kvm_util_internal.h | 128 --
> .../selftests/kvm/lib/perf_test_util.c | 84 +-
> .../selftests/kvm/lib/riscv/processor.c | 111 +-
> tools/testing/selftests/kvm/lib/riscv/ucall.c | 14 +-
> .../kvm/lib/s390x/diag318_test_handler.c | 11 +-
> .../selftests/kvm/lib/s390x/processor.c | 44 +-
> tools/testing/selftests/kvm/lib/s390x/ucall.c | 8 +-
> .../selftests/kvm/lib/x86_64/processor.c | 533 +++-----
> tools/testing/selftests/kvm/lib/x86_64/svm.c | 6 +-
> .../testing/selftests/kvm/lib/x86_64/ucall.c | 10 +-
> tools/testing/selftests/kvm/lib/x86_64/vmx.c | 16 +-
> .../selftests/kvm/max_guest_memory_test.c | 53 +-
> .../kvm/memslot_modification_stress_test.c | 13 +-
> .../testing/selftests/kvm/memslot_perf_test.c | 28 +-
> tools/testing/selftests/kvm/rseq_test.c | 22 +-
> tools/testing/selftests/kvm/s390x/memop.c | 93 +-
> tools/testing/selftests/kvm/s390x/resets.c | 140 ++-
> .../selftests/kvm/s390x/sync_regs_test.c | 45 +-
> tools/testing/selftests/kvm/s390x/tprot.c | 25 +-
> .../selftests/kvm/set_memory_region_test.c | 43 +-
> tools/testing/selftests/kvm/steal_time.c | 120 +-
> .../kvm/system_counter_offset_test.c | 35 +-
> tools/testing/selftests/kvm/x86_64/amx_test.c | 56 +-
> .../testing/selftests/kvm/x86_64/cpuid_test.c | 29 +-
> .../kvm/x86_64/cr4_cpuid_sync_test.c | 22 +-
> .../testing/selftests/kvm/x86_64/debug_regs.c | 77 +-
> .../kvm/x86_64/emulator_error_test.c | 74 +-
> .../testing/selftests/kvm/x86_64/evmcs_test.c | 61 +-
> .../selftests/kvm/x86_64/fix_hypercall_test.c | 45 +-
> .../kvm/x86_64/get_msr_index_features.c | 117 +-
> .../selftests/kvm/x86_64/hyperv_clock.c | 25 +-
> .../selftests/kvm/x86_64/hyperv_cpuid.c | 34 +-
> .../selftests/kvm/x86_64/hyperv_features.c | 61 +-
> .../selftests/kvm/x86_64/hyperv_svm_test.c | 20 +-
> .../selftests/kvm/x86_64/kvm_clock_test.c | 29 +-
> .../selftests/kvm/x86_64/kvm_pv_test.c | 33 +-
> .../kvm/x86_64/max_vcpuid_cap_test.c | 28 +-
> .../selftests/kvm/x86_64/mmio_warning_test.c | 16 +-
> .../selftests/kvm/x86_64/mmu_role_test.c | 30 +-
> .../selftests/kvm/x86_64/platform_info_test.c | 51 +-
> .../kvm/x86_64/pmu_event_filter_test.c | 97 +-
> .../selftests/kvm/x86_64/set_boot_cpu_id.c | 91 +-
> .../selftests/kvm/x86_64/set_sregs_test.c | 47 +-
> .../selftests/kvm/x86_64/sev_migrate_tests.c | 120 +-
> tools/testing/selftests/kvm/x86_64/smm_test.c | 37 +-
> .../testing/selftests/kvm/x86_64/state_test.c | 29 +-
> .../selftests/kvm/x86_64/svm_int_ctl_test.c | 21 +-
> .../kvm/x86_64/svm_nested_soft_inject_test.c | 17 +-
> .../selftests/kvm/x86_64/svm_vmcall_test.c | 16 +-
> .../selftests/kvm/x86_64/sync_regs_test.c | 62 +-
> .../kvm/x86_64/triple_fault_event_test.c | 39 +-
> .../selftests/kvm/x86_64/tsc_msrs_test.c | 35 +-
> .../selftests/kvm/x86_64/tsc_scaling_sync.c | 25 +-
> .../selftests/kvm/x86_64/userspace_io_test.c | 18 +-
> .../kvm/x86_64/userspace_msr_exit_test.c | 187 ++-
> .../kvm/x86_64/vmx_apic_access_test.c | 27 +-
> .../kvm/x86_64/vmx_close_while_nested_test.c | 17 +-
> .../selftests/kvm/x86_64/vmx_dirty_log_test.c | 13 +-
> .../vmx_exception_with_invalid_guest_state.c | 68 +-
> .../x86_64/vmx_invalid_nested_guest_state.c | 18 +-
> .../kvm/x86_64/vmx_nested_tsc_scaling_test.c | 29 +-
> .../selftests/kvm/x86_64/vmx_pmu_caps_test.c | 48 +-
> .../kvm/x86_64/vmx_preemption_timer_test.c | 35 +-
> .../kvm/x86_64/vmx_set_nested_state_test.c | 91 +-
> .../kvm/x86_64/vmx_tsc_adjust_test.c | 13 +-
> .../selftests/kvm/x86_64/xapic_ipi_test.c | 48 +-
> .../selftests/kvm/x86_64/xapic_state_test.c | 60 +-
> .../selftests/kvm/x86_64/xen_shinfo_test.c | 73 +-
> .../selftests/kvm/x86_64/xen_vmcall_test.c | 25 +-
> .../selftests/kvm/x86_64/xss_msr_test.c | 56 +-
> 102 files changed, 3059 insertions(+), 4178 deletions(-)
> delete mode 100644 tools/testing/selftests/kvm/lib/kvm_util_internal.h
>
>
> base-commit: 55371f1d0c01357f29da613f7525c3f252320bbf

2022-06-08 06:19:27

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

On Tue, Jun 07, 2022, Sean Christopherson wrote:
> +Raghu
>
> On Tue, Jun 07, 2022, Paolo Bonzini wrote:
> > Marc, Christian, Anup, can you please give this a go?
>
> Raghu is going to run on arm64, I'll work with him to iron out any bugs (I should
> have done this before posting). I.e. Marc is mostly off the hook unless there's
> tests we can't run.

arm64 is quite broken, the only tests that pass are those that don't actually
enter the guest. Common tests, e.g. rseq and memslots tests, fail with the same
signature, so presumably I botched something in lib/aarch64, but I haven't been
able to find anything via inspection.

Raghu is bisecting...

2022-06-08 06:25:03

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

+Raghu

On Tue, Jun 07, 2022, Paolo Bonzini wrote:
> Marc, Christian, Anup, can you please give this a go?

Raghu is going to run on arm64, I'll work with him to iron out any bugs (I should
have done this before posting). I.e. Marc is mostly off the hook unless there's
tests we can't run.


>
> Paolo
>
> On 6/3/22 02:41, Sean Christopherson wrote:
> > Overhaul KVM's selftest APIs to get selftests to a state where adding new
> > features and writing tests is less painful/disgusting.
> >
> > Patches 1 fixes a goof in kvm/queue and should be squashed.
> >
> > I would really, really, really like to get this queued up sooner than
> > later, or maybe just thrown into a separate selftests-specific branch that
> > folks can develop against. Rebasing is tedious, frustrating, and time
> > consuming. And spoiler alert, there's another 42 x86-centric patches
> > inbound that builds on this series to clean up CPUID related crud...
> >
> > The primary theme is to stop treating tests like second class citizens.
> > Stop hiding vcpu, kvm_vm, etc... There's no sensitive data/constructs, and
> > the encapsulation has led to really, really bad and difficult to maintain
> > code. E.g. having to pass around the VM just to call a vCPU ioctl(),
> > arbitrary non-zero vCPU IDs, tests having to care about the vCPU ID in the
> > first place, etc...
> >
> > The other theme in the rework is to deduplicate code and try to set us
> > up for success in the future. E.g. provide macros/helpers instead of
> > spamming CTRL-C => CTRL-V (see the -1k LoC), structure the VM creation
> > APIs to build on one another, etc...
> >
> > The absurd patch count (as opposed to just ridiculous) is due to converting
> > each test away from using hardcoded vCPU IDs in a separate patch. The vast
> > majority of those patches probably aren't worth reviewing in depth, the
> > changes are mostly mechanical in nature.
> >
> > However, _running_ non-x86 tests (or tests that have unique non-x86
> > behavior) would be extremely valuable. All patches have been compile tested
> > on x86, arm, risc-v, and s390, but I've only run the tests on x86. Based on
> > my track record for the x86+common tests, I will be very, very surprised if
> > I didn't break any of the non-x86 tests, e.g. pthread_create()'s 'void *'
> > param tripped me up multiple times.
> >
> > I have not run x86's amx_test due to lack of hardware. I also haven't run
> > sev_migration; something is wonky in either the upstream support for INIT_EX
> > or in our test machines and I can't get SEV to initialize.
> >
> > v2:
> > - Drop the forced -Werror patch. [Vitaly]
> > - Add TEST_REQUIRE to reduce KSFT_SKIP boilerplate.
> > - Rebase to kvm/queue, commit 55371f1d0c01.
> > - Clean up even more bad copy+paste code (x86 was hiding a lot of crud).
> > - Assert that the input to an ioctl() is (likely) the correct struct.
> >
> > v1: https://lore.kernel.org/all/[email protected]
> >
> > Sean Christopherson (144):
> > KVM: Fix references to non-existent KVM_CAP_TRIPLE_FAULT_EVENT
> > KVM: selftests: Fix buggy-but-benign check in
> > test_v3_new_redist_regions()
> > KVM: selftests: Fix typo in vgic_init test
> > KVM: selftests: Drop stale declarations from kvm_util_base.h
> > KVM: selftests: Always open VM file descriptors with O_RDWR
> > KVM: selftests: Add another underscore to inner ioctl() helpers
> > KVM: selftests: Make vcpu_ioctl() a wrapper to pretty print ioctl name
> > KVM: selftests: Drop @mode from common vm_create() helper
> > KVM: selftests: Split vcpu_set_nested_state() into two helpers
> > KVM: sefltests: Use vcpu_ioctl() and __vcpu_ioctl() helpers
> > KVM: selftests: Add __vcpu_run() helper
> > KVM: selftests: Use vcpu_access_device_attr() in arm64 code
> > KVM: selftests: Remove vcpu_get_fd()
> > KVM: selftests: Add vcpu_get() to retrieve and assert on vCPU
> > existence
> > KVM: selftests: Make vm_ioctl() a wrapper to pretty print ioctl name
> > KVM: sefltests: Use vm_ioctl() and __vm_ioctl() helpers
> > KVM: selftests: Make kvm_ioctl() a wrapper to pretty print ioctl name
> > KVM: selftests: Use kvm_ioctl() helpers
> > KVM: selftests: Use __KVM_SYSCALL_ERROR() to handle non-KVM syscall
> > errors
> > KVM: selftests: Make x86-64's register dump helpers static
> > KVM: selftests: Get rid of kvm_util_internal.h
> > KVM: selftests: Use KVM_IOCTL_ERROR() for one-off arm64 ioctls
> > KVM: selftests: Drop @test param from kvm_create_device()
> > KVM: selftests: Move KVM_CREATE_DEVICE_TEST code to separate helper
> > KVM: selftests: Multiplex return code and fd in __kvm_create_device()
> > KVM: selftests: Rename KVM_HAS_DEVICE_ATTR helpers for consistency
> > KVM: selftests: Drop 'int' return from asserting *_has_device_attr()
> > KVM: selftests: Split get/set device_attr helpers
> > KVM: selftests: Add a VM backpointer to 'struct vcpu'
> > KVM: selftests: Consolidate KVM_ENABLE_CAP usage
> > KVM: selftests: Simplify KVM_ENABLE_CAP helper APIs
> > KVM: selftests: Cache list of MSRs to save/restore
> > KVM: selftests: Harden and comment XSS / KVM_SET_MSRS interaction
> > KVM: selftests: Dedup MSR index list helpers, simplify dedicated test
> > KVM: selftests: Rename MP_STATE and GUEST_DEBUG helpers for
> > consistency
> > KVM: selftest: Add proper helpers for x86-specific save/restore ioctls
> > KVM: selftests: Add vm_create_*() variants to expose/return 'struct
> > vcpu'
> > KVM: selftests: Push vm_adjust_num_guest_pages() into "w/o vCPUs"
> > helper
> > KVM: selftests: Use vm_create_without_vcpus() in set_boot_cpu_id
> > KVM: selftests: Use vm_create_without_vcpus() in dirty_log_test
> > KVM: selftests: Use vm_create_without_vcpus() in hardware_disable_test
> > KVM: selftests: Use vm_create_without_vcpus() in psci_test
> > KVM: selftests: Rename vm_create() => vm_create_barebones(), drop
> > param
> > KVM: selftests: Rename vm_create_without_vcpus() => vm_create()
> > KVM: selftests: Make vm_create() a wrapper that specifies
> > VM_MODE_DEFAULT
> > KVM: selftests: Rename xAPIC state test's vcpu struct
> > KVM: selftests: Rename vcpu.state => vcpu.run
> > KVM: selftests: Rename 'struct vcpu' to 'struct kvm_vcpu'
> > KVM: selftests: Return the created vCPU from vm_vcpu_add()
> > KVM: selftests: Convert memslot_perf_test away from VCPU_ID
> > KVM: selftests: Convert rseq_test away from VCPU_ID
> > KVM: selftests: Convert xss_msr_test away from VCPU_ID
> > KVM: selftests: Convert vmx_preemption_timer_test away from VCPU_ID
> > KVM: selftests: Convert vmx_pmu_msrs_test away from VCPU_ID
> > KVM: selftests: Convert vmx_set_nested_state_test away from VCPU_ID
> > KVM: selftests: Convert vmx_tsc_adjust_test away from VCPU_ID
> > KVM: selftests: Convert mmu_role_test away from VCPU_ID
> > KVM: selftests: Convert pmu_event_filter_test away from VCPU_ID
> > KVM: selftests: Convert smm_test away from VCPU_ID
> > KVM: selftests: Convert state_test away from VCPU_ID
> > KVM: selftests: Convert svm_int_ctl_test away from VCPU_ID
> > KVM: selftests: Convert svm_vmcall_test away from VCPU_ID
> > KVM: selftests: Convert sync_regs_test away from VCPU_ID
> > KVM: selftests: Convert hyperv_cpuid away from VCPU_ID
> > KVM: selftests: Convert kvm_pv_test away from VCPU_ID
> > KVM: selftests: Convert platform_info_test away from VCPU_ID
> > KVM: selftests: Convert vmx_nested_tsc_scaling_test away from VCPU_ID
> > KVM: selftests: Convert set_sregs_test away from VCPU_ID
> > KVM: selftests: Convert vmx_dirty_log_test away from VCPU_ID
> > KVM: selftests: Convert vmx_close_while_nested_test away from VCPU_ID
> > KVM: selftests: Convert vmx_apic_access_test away from VCPU_ID
> > KVM: selftests: Convert userspace_msr_exit_test away from VCPU_ID
> > KVM: selftests: Convert vmx_exception_with_invalid_guest_state away
> > from VCPU_ID
> > KVM: selftests: Convert tsc_msrs_test away from VCPU_ID
> > KVM: selftests: Convert kvm_clock_test away from VCPU_ID
> > KVM: selftests: Convert hyperv_svm_test away from VCPU_ID
> > KVM: selftests: Convert hyperv_features away from VCPU_ID
> > KVM: selftests: Convert hyperv_clock away from VCPU_ID
> > KVM: selftests: Convert evmcs_test away from VCPU_ID
> > KVM: selftests: Convert emulator_error_test away from VCPU_ID
> > KVM: selftests: Convert debug_regs away from VCPU_ID
> > KVM: selftests: Add proper helper for advancing RIP in debug_regs
> > KVM: selftests: Convert amx_test away from VCPU_ID
> > KVM: selftests: Convert cr4_cpuid_sync_test away from VCPU_ID
> > KVM: selftests: Convert cpuid_test away from VCPU_ID
> > KVM: selftests: Convert userspace_io_test away from VCPU_ID
> > KVM: selftests: Convert vmx_invalid_nested_guest_state away from
> > VCPU_ID
> > KVM: selftests: Convert xen_vmcall_test away from VCPU_ID
> > KVM: selftests: Convert xen_shinfo_test away from VCPU_ID
> > KVM: selftests: Convert dirty_log_test away from VCPU_ID
> > KVM: selftests: Convert set_memory_region_test away from VCPU_ID
> > KVM: selftests: Convert system_counter_offset_test away from VCPU_ID
> > KVM: selftests: Track kvm_vcpu object in tsc_scaling_sync
> > KVM: selftests: Convert xapic_state_test away from hardcoded vCPU ID
> > KVM: selftests: Convert debug-exceptions away from VCPU_ID
> > KVM: selftests: Convert fix_hypercall_test away from VCPU_ID
> > KVM: selftests: Convert vgic_irq away from VCPU_ID
> > KVM: selftests: Make arm64's guest_get_vcpuid() declaration arm64-only
> > KVM: selftests: Move vm_is_unrestricted_guest() to x86-64
> > KVM: selftests: Add "arch" to common utils that have arch
> > implementations
> > KVM: selftests: Return created vcpu from vm_vcpu_add_default()
> > KVM: selftests: Rename vm_vcpu_add* helpers to better show
> > relationships
> > KVM: selftests: Convert set_boot_cpu_id away from global VCPU_IDs
> > KVM: selftests: Convert psci_test away from VCPU_ID
> > KVM: selftests: Convert hardware_disable_test to pass around vCPU
> > objects
> > KVM: selftests: Add VM creation helper that "returns" vCPUs
> > KVM: selftests: Convert steal_time away from VCPU_ID
> > KVM: selftests: Convert arch_timer away from VCPU_ID
> > KVM: selftests: Convert svm_nested_soft_inject_test away from VCPU_ID
> > KVM: selftests: Convert triple_fault_event_test away from VCPU_ID
> > KVM: selftests: Convert vgic_init away from
> > vm_create_default_with_vcpus()
> > KVM: selftests: Consolidate KVM_{G,S}ET_ONE_REG helpers
> > KVM: selftests: Sync stage before VM is freed in hypercalls test
> > KVM: selftests: Convert hypercalls test away from vm_create_default()
> > KVM: selftests: Convert xapic_ipi_test away from *_VCPU_ID
> > KVM: selftests: Convert sync_regs_test away from VCPU_ID
> > KVM: selftests: Convert s390's "resets" test away from VCPU_ID
> > KVM: selftests: Convert memop away from VCPU_ID
> > KVM: selftests: Convert s390x/diag318_test_handler away from VCPU_ID
> > KVM: selftests: Convert tprot away from VCPU_ID
> > KVM: selftests: Use vm_create() in tsc_scaling_sync
> > KVM: selftests: Use vm_create_with_vcpus() in max_guest_memory_test
> > KVM: selftests: Drop vm_create_default* helpers
> > KVM: selftests: Drop @vcpuids param from VM creators
> > KVM: selftests: Convert kvm_page_table_test away from reliance on
> > vcpu_id
> > KVM: selftests: Convert kvm_binary_stats_test away from vCPU IDs
> > KVM: selftests: Convert get-reg-list away from its "VCPU_ID"
> > KVM: selftests: Stop hardcoding vCPU IDs in vcpu_width_config
> > KVM: selftests: Stop conflating vCPU index and ID in perf tests
> > KVM: selftests: Remove vcpu_get() usage from dirty_log_test
> > KVM: selftests: Require vCPU output array when creating VM with vCPUs
> > KVM: selftests: Purge vm+vcpu_id == vcpu silliness
> > KVM: selftests: Drop vcpu_get(), rename vcpu_find() => vcpu_exists()
> > KVM: selftests: Remove vcpu_state() helper
> > KVM: selftests: Open code and drop 'struct kvm_vm' accessors
> > KVM: selftests: Drop @slot0_mem_pages from __vm_create_with_vcpus()
> > KVM: selftests: Drop @num_percpu_pages from __vm_create_with_vcpus()
> > KVM: selftests: Move per-VM/per-vCPU nr pages calculation to
> > __vm_create()
> > KVM: selftests: Trust that MAXPHYADDR > memslot0 in
> > vmx_apic_access_test
> > KVM: selftests: Drop DEFAULT_GUEST_PHY_PAGES, open code the magic
> > number
> > KVM: selftests: Return an 'unsigned int' from kvm_check_cap()
> > KVM: selftests: Add kvm_has_cap() to provide syntactic sugar
> > KVM: selftests: Add TEST_REQUIRE macros to reduce skipping copy+paste
> > KVM: selftests: Sanity check input to ioctls() at build time
> >
> > Documentation/virt/kvm/api.rst | 4 +-
> > .../selftests/kvm/aarch64/arch_timer.c | 79 +-
> > .../selftests/kvm/aarch64/debug-exceptions.c | 22 +-
> > .../selftests/kvm/aarch64/get-reg-list.c | 29 +-
> > .../selftests/kvm/aarch64/hypercalls.c | 90 +-
> > .../testing/selftests/kvm/aarch64/psci_test.c | 69 +-
> > .../selftests/kvm/aarch64/vcpu_width_config.c | 71 +-
> > .../testing/selftests/kvm/aarch64/vgic_init.c | 379 +++---
> > .../testing/selftests/kvm/aarch64/vgic_irq.c | 40 +-
> > .../selftests/kvm/access_tracking_perf_test.c | 92 +-
> > .../selftests/kvm/demand_paging_test.c | 49 +-
> > .../selftests/kvm/dirty_log_perf_test.c | 51 +-
> > tools/testing/selftests/kvm/dirty_log_test.c | 95 +-
> > .../selftests/kvm/hardware_disable_test.c | 29 +-
> > .../selftests/kvm/include/aarch64/processor.h | 28 +-
> > .../selftests/kvm/include/aarch64/vgic.h | 6 +-
> > .../selftests/kvm/include/kvm_util_base.h | 743 ++++++++---
> > .../selftests/kvm/include/perf_test_util.h | 5 +-
> > .../selftests/kvm/include/riscv/processor.h | 20 -
> > .../testing/selftests/kvm/include/test_util.h | 9 +
> > .../selftests/kvm/include/ucall_common.h | 2 +-
> > .../selftests/kvm/include/x86_64/evmcs.h | 2 +-
> > .../selftests/kvm/include/x86_64/processor.h | 109 +-
> > .../selftests/kvm/kvm_binary_stats_test.c | 31 +-
> > .../selftests/kvm/kvm_create_max_vcpus.c | 10 +-
> > .../selftests/kvm/kvm_page_table_test.c | 66 +-
> > .../selftests/kvm/lib/aarch64/processor.c | 81 +-
> > .../testing/selftests/kvm/lib/aarch64/ucall.c | 9 +-
> > .../testing/selftests/kvm/lib/aarch64/vgic.c | 54 +-
> > tools/testing/selftests/kvm/lib/elf.c | 1 -
> > tools/testing/selftests/kvm/lib/guest_modes.c | 6 +-
> > tools/testing/selftests/kvm/lib/kvm_util.c | 1104 +++--------------
> > .../selftests/kvm/lib/kvm_util_internal.h | 128 --
> > .../selftests/kvm/lib/perf_test_util.c | 84 +-
> > .../selftests/kvm/lib/riscv/processor.c | 111 +-
> > tools/testing/selftests/kvm/lib/riscv/ucall.c | 14 +-
> > .../kvm/lib/s390x/diag318_test_handler.c | 11 +-
> > .../selftests/kvm/lib/s390x/processor.c | 44 +-
> > tools/testing/selftests/kvm/lib/s390x/ucall.c | 8 +-
> > .../selftests/kvm/lib/x86_64/processor.c | 533 +++-----
> > tools/testing/selftests/kvm/lib/x86_64/svm.c | 6 +-
> > .../testing/selftests/kvm/lib/x86_64/ucall.c | 10 +-
> > tools/testing/selftests/kvm/lib/x86_64/vmx.c | 16 +-
> > .../selftests/kvm/max_guest_memory_test.c | 53 +-
> > .../kvm/memslot_modification_stress_test.c | 13 +-
> > .../testing/selftests/kvm/memslot_perf_test.c | 28 +-
> > tools/testing/selftests/kvm/rseq_test.c | 22 +-
> > tools/testing/selftests/kvm/s390x/memop.c | 93 +-
> > tools/testing/selftests/kvm/s390x/resets.c | 140 ++-
> > .../selftests/kvm/s390x/sync_regs_test.c | 45 +-
> > tools/testing/selftests/kvm/s390x/tprot.c | 25 +-
> > .../selftests/kvm/set_memory_region_test.c | 43 +-
> > tools/testing/selftests/kvm/steal_time.c | 120 +-
> > .../kvm/system_counter_offset_test.c | 35 +-
> > tools/testing/selftests/kvm/x86_64/amx_test.c | 56 +-
> > .../testing/selftests/kvm/x86_64/cpuid_test.c | 29 +-
> > .../kvm/x86_64/cr4_cpuid_sync_test.c | 22 +-
> > .../testing/selftests/kvm/x86_64/debug_regs.c | 77 +-
> > .../kvm/x86_64/emulator_error_test.c | 74 +-
> > .../testing/selftests/kvm/x86_64/evmcs_test.c | 61 +-
> > .../selftests/kvm/x86_64/fix_hypercall_test.c | 45 +-
> > .../kvm/x86_64/get_msr_index_features.c | 117 +-
> > .../selftests/kvm/x86_64/hyperv_clock.c | 25 +-
> > .../selftests/kvm/x86_64/hyperv_cpuid.c | 34 +-
> > .../selftests/kvm/x86_64/hyperv_features.c | 61 +-
> > .../selftests/kvm/x86_64/hyperv_svm_test.c | 20 +-
> > .../selftests/kvm/x86_64/kvm_clock_test.c | 29 +-
> > .../selftests/kvm/x86_64/kvm_pv_test.c | 33 +-
> > .../kvm/x86_64/max_vcpuid_cap_test.c | 28 +-
> > .../selftests/kvm/x86_64/mmio_warning_test.c | 16 +-
> > .../selftests/kvm/x86_64/mmu_role_test.c | 30 +-
> > .../selftests/kvm/x86_64/platform_info_test.c | 51 +-
> > .../kvm/x86_64/pmu_event_filter_test.c | 97 +-
> > .../selftests/kvm/x86_64/set_boot_cpu_id.c | 91 +-
> > .../selftests/kvm/x86_64/set_sregs_test.c | 47 +-
> > .../selftests/kvm/x86_64/sev_migrate_tests.c | 120 +-
> > tools/testing/selftests/kvm/x86_64/smm_test.c | 37 +-
> > .../testing/selftests/kvm/x86_64/state_test.c | 29 +-
> > .../selftests/kvm/x86_64/svm_int_ctl_test.c | 21 +-
> > .../kvm/x86_64/svm_nested_soft_inject_test.c | 17 +-
> > .../selftests/kvm/x86_64/svm_vmcall_test.c | 16 +-
> > .../selftests/kvm/x86_64/sync_regs_test.c | 62 +-
> > .../kvm/x86_64/triple_fault_event_test.c | 39 +-
> > .../selftests/kvm/x86_64/tsc_msrs_test.c | 35 +-
> > .../selftests/kvm/x86_64/tsc_scaling_sync.c | 25 +-
> > .../selftests/kvm/x86_64/userspace_io_test.c | 18 +-
> > .../kvm/x86_64/userspace_msr_exit_test.c | 187 ++-
> > .../kvm/x86_64/vmx_apic_access_test.c | 27 +-
> > .../kvm/x86_64/vmx_close_while_nested_test.c | 17 +-
> > .../selftests/kvm/x86_64/vmx_dirty_log_test.c | 13 +-
> > .../vmx_exception_with_invalid_guest_state.c | 68 +-
> > .../x86_64/vmx_invalid_nested_guest_state.c | 18 +-
> > .../kvm/x86_64/vmx_nested_tsc_scaling_test.c | 29 +-
> > .../selftests/kvm/x86_64/vmx_pmu_caps_test.c | 48 +-
> > .../kvm/x86_64/vmx_preemption_timer_test.c | 35 +-
> > .../kvm/x86_64/vmx_set_nested_state_test.c | 91 +-
> > .../kvm/x86_64/vmx_tsc_adjust_test.c | 13 +-
> > .../selftests/kvm/x86_64/xapic_ipi_test.c | 48 +-
> > .../selftests/kvm/x86_64/xapic_state_test.c | 60 +-
> > .../selftests/kvm/x86_64/xen_shinfo_test.c | 73 +-
> > .../selftests/kvm/x86_64/xen_vmcall_test.c | 25 +-
> > .../selftests/kvm/x86_64/xss_msr_test.c | 56 +-
> > 102 files changed, 3059 insertions(+), 4178 deletions(-)
> > delete mode 100644 tools/testing/selftests/kvm/lib/kvm_util_internal.h
> >
> >
> > base-commit: 55371f1d0c01357f29da613f7525c3f252320bbf
>

2022-06-08 07:57:41

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

On Tue, Jun 07, 2022, Sean Christopherson wrote:
> On Tue, Jun 07, 2022, Sean Christopherson wrote:
> > +Raghu
> >
> > On Tue, Jun 07, 2022, Paolo Bonzini wrote:
> > > Marc, Christian, Anup, can you please give this a go?
> >
> > Raghu is going to run on arm64, I'll work with him to iron out any bugs (I should
> > have done this before posting). I.e. Marc is mostly off the hook unless there's
> > tests we can't run.
>
> arm64 is quite broken, the only tests that pass are those that don't actually
> enter the guest. Common tests, e.g. rseq and memslots tests, fail with the same
> signature, so presumably I botched something in lib/aarch64, but I haven't been
> able to find anything via inspection.
>
> Raghu is bisecting...

Ha! Looks like it's an issue with running upstream selftest using one of our many
internal framework things. Running a few of the tests manually works. We should
have full results tomorrow.

I did find one bug during my inspection, in case someone gets ambitious and wants
to run tests too :-)

diff --git a/tools/testing/selftests/kvm/lib/aarch64/vgic.c b/tools/testing/selftests/kvm/lib/aarch64/vgic.c
index 0de9b0686498..b5f28d21a947 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/vgic.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/vgic.c
@@ -55,7 +55,7 @@ int vgic_v3_setup(struct kvm_vm *vm, unsigned int nr_vcpus, uint32_t nr_irqs,
if (gic_fd < 0)
return gic_fd;

- kvm_device_attr_get(gic_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS, 0, &nr_irqs);
+ kvm_device_attr_set(gic_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS, 0, &nr_irqs);

kvm_device_attr_set(gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
KVM_DEV_ARM_VGIC_CTRL_INIT, NULL);

2022-06-08 15:21:55

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

On 2022-06-07 16:27, Paolo Bonzini wrote:
> Marc, Christian, Anup, can you please give this a go?

Can you please, pretty please, once and for all, kill that alias you
seem to have for me and email me on an address I actually can read?

I can't remember how many times you emailed me on my ex @arm.com address
over the past 2+years...

The same thing probably applies to Sean, btw.

Thanks,

M.
--
Jazz is not dead. It just smells funny...

2022-06-08 16:24:07

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

On Tue, Jun 7, 2022 at 8:57 PM Paolo Bonzini <[email protected]> wrote:
>
> Marc, Christian, Anup, can you please give this a go?

Sure, I will try this series.

Regards,
Anup

>
> Paolo
>
> On 6/3/22 02:41, Sean Christopherson wrote:
> > Overhaul KVM's selftest APIs to get selftests to a state where adding new
> > features and writing tests is less painful/disgusting.
> >
> > Patches 1 fixes a goof in kvm/queue and should be squashed.
> >
> > I would really, really, really like to get this queued up sooner than
> > later, or maybe just thrown into a separate selftests-specific branch that
> > folks can develop against. Rebasing is tedious, frustrating, and time
> > consuming. And spoiler alert, there's another 42 x86-centric patches
> > inbound that builds on this series to clean up CPUID related crud...
> >
> > The primary theme is to stop treating tests like second class citizens.
> > Stop hiding vcpu, kvm_vm, etc... There's no sensitive data/constructs, and
> > the encapsulation has led to really, really bad and difficult to maintain
> > code. E.g. having to pass around the VM just to call a vCPU ioctl(),
> > arbitrary non-zero vCPU IDs, tests having to care about the vCPU ID in the
> > first place, etc...
> >
> > The other theme in the rework is to deduplicate code and try to set us
> > up for success in the future. E.g. provide macros/helpers instead of
> > spamming CTRL-C => CTRL-V (see the -1k LoC), structure the VM creation
> > APIs to build on one another, etc...
> >
> > The absurd patch count (as opposed to just ridiculous) is due to converting
> > each test away from using hardcoded vCPU IDs in a separate patch. The vast
> > majority of those patches probably aren't worth reviewing in depth, the
> > changes are mostly mechanical in nature.
> >
> > However, _running_ non-x86 tests (or tests that have unique non-x86
> > behavior) would be extremely valuable. All patches have been compile tested
> > on x86, arm, risc-v, and s390, but I've only run the tests on x86. Based on
> > my track record for the x86+common tests, I will be very, very surprised if
> > I didn't break any of the non-x86 tests, e.g. pthread_create()'s 'void *'
> > param tripped me up multiple times.
> >
> > I have not run x86's amx_test due to lack of hardware. I also haven't run
> > sev_migration; something is wonky in either the upstream support for INIT_EX
> > or in our test machines and I can't get SEV to initialize.
> >
> > v2:
> > - Drop the forced -Werror patch. [Vitaly]
> > - Add TEST_REQUIRE to reduce KSFT_SKIP boilerplate.
> > - Rebase to kvm/queue, commit 55371f1d0c01.
> > - Clean up even more bad copy+paste code (x86 was hiding a lot of crud).
> > - Assert that the input to an ioctl() is (likely) the correct struct.
> >
> > v1: https://lore.kernel.org/all/[email protected]
> >
> > Sean Christopherson (144):
> > KVM: Fix references to non-existent KVM_CAP_TRIPLE_FAULT_EVENT
> > KVM: selftests: Fix buggy-but-benign check in
> > test_v3_new_redist_regions()
> > KVM: selftests: Fix typo in vgic_init test
> > KVM: selftests: Drop stale declarations from kvm_util_base.h
> > KVM: selftests: Always open VM file descriptors with O_RDWR
> > KVM: selftests: Add another underscore to inner ioctl() helpers
> > KVM: selftests: Make vcpu_ioctl() a wrapper to pretty print ioctl name
> > KVM: selftests: Drop @mode from common vm_create() helper
> > KVM: selftests: Split vcpu_set_nested_state() into two helpers
> > KVM: sefltests: Use vcpu_ioctl() and __vcpu_ioctl() helpers
> > KVM: selftests: Add __vcpu_run() helper
> > KVM: selftests: Use vcpu_access_device_attr() in arm64 code
> > KVM: selftests: Remove vcpu_get_fd()
> > KVM: selftests: Add vcpu_get() to retrieve and assert on vCPU
> > existence
> > KVM: selftests: Make vm_ioctl() a wrapper to pretty print ioctl name
> > KVM: sefltests: Use vm_ioctl() and __vm_ioctl() helpers
> > KVM: selftests: Make kvm_ioctl() a wrapper to pretty print ioctl name
> > KVM: selftests: Use kvm_ioctl() helpers
> > KVM: selftests: Use __KVM_SYSCALL_ERROR() to handle non-KVM syscall
> > errors
> > KVM: selftests: Make x86-64's register dump helpers static
> > KVM: selftests: Get rid of kvm_util_internal.h
> > KVM: selftests: Use KVM_IOCTL_ERROR() for one-off arm64 ioctls
> > KVM: selftests: Drop @test param from kvm_create_device()
> > KVM: selftests: Move KVM_CREATE_DEVICE_TEST code to separate helper
> > KVM: selftests: Multiplex return code and fd in __kvm_create_device()
> > KVM: selftests: Rename KVM_HAS_DEVICE_ATTR helpers for consistency
> > KVM: selftests: Drop 'int' return from asserting *_has_device_attr()
> > KVM: selftests: Split get/set device_attr helpers
> > KVM: selftests: Add a VM backpointer to 'struct vcpu'
> > KVM: selftests: Consolidate KVM_ENABLE_CAP usage
> > KVM: selftests: Simplify KVM_ENABLE_CAP helper APIs
> > KVM: selftests: Cache list of MSRs to save/restore
> > KVM: selftests: Harden and comment XSS / KVM_SET_MSRS interaction
> > KVM: selftests: Dedup MSR index list helpers, simplify dedicated test
> > KVM: selftests: Rename MP_STATE and GUEST_DEBUG helpers for
> > consistency
> > KVM: selftest: Add proper helpers for x86-specific save/restore ioctls
> > KVM: selftests: Add vm_create_*() variants to expose/return 'struct
> > vcpu'
> > KVM: selftests: Push vm_adjust_num_guest_pages() into "w/o vCPUs"
> > helper
> > KVM: selftests: Use vm_create_without_vcpus() in set_boot_cpu_id
> > KVM: selftests: Use vm_create_without_vcpus() in dirty_log_test
> > KVM: selftests: Use vm_create_without_vcpus() in hardware_disable_test
> > KVM: selftests: Use vm_create_without_vcpus() in psci_test
> > KVM: selftests: Rename vm_create() => vm_create_barebones(), drop
> > param
> > KVM: selftests: Rename vm_create_without_vcpus() => vm_create()
> > KVM: selftests: Make vm_create() a wrapper that specifies
> > VM_MODE_DEFAULT
> > KVM: selftests: Rename xAPIC state test's vcpu struct
> > KVM: selftests: Rename vcpu.state => vcpu.run
> > KVM: selftests: Rename 'struct vcpu' to 'struct kvm_vcpu'
> > KVM: selftests: Return the created vCPU from vm_vcpu_add()
> > KVM: selftests: Convert memslot_perf_test away from VCPU_ID
> > KVM: selftests: Convert rseq_test away from VCPU_ID
> > KVM: selftests: Convert xss_msr_test away from VCPU_ID
> > KVM: selftests: Convert vmx_preemption_timer_test away from VCPU_ID
> > KVM: selftests: Convert vmx_pmu_msrs_test away from VCPU_ID
> > KVM: selftests: Convert vmx_set_nested_state_test away from VCPU_ID
> > KVM: selftests: Convert vmx_tsc_adjust_test away from VCPU_ID
> > KVM: selftests: Convert mmu_role_test away from VCPU_ID
> > KVM: selftests: Convert pmu_event_filter_test away from VCPU_ID
> > KVM: selftests: Convert smm_test away from VCPU_ID
> > KVM: selftests: Convert state_test away from VCPU_ID
> > KVM: selftests: Convert svm_int_ctl_test away from VCPU_ID
> > KVM: selftests: Convert svm_vmcall_test away from VCPU_ID
> > KVM: selftests: Convert sync_regs_test away from VCPU_ID
> > KVM: selftests: Convert hyperv_cpuid away from VCPU_ID
> > KVM: selftests: Convert kvm_pv_test away from VCPU_ID
> > KVM: selftests: Convert platform_info_test away from VCPU_ID
> > KVM: selftests: Convert vmx_nested_tsc_scaling_test away from VCPU_ID
> > KVM: selftests: Convert set_sregs_test away from VCPU_ID
> > KVM: selftests: Convert vmx_dirty_log_test away from VCPU_ID
> > KVM: selftests: Convert vmx_close_while_nested_test away from VCPU_ID
> > KVM: selftests: Convert vmx_apic_access_test away from VCPU_ID
> > KVM: selftests: Convert userspace_msr_exit_test away from VCPU_ID
> > KVM: selftests: Convert vmx_exception_with_invalid_guest_state away
> > from VCPU_ID
> > KVM: selftests: Convert tsc_msrs_test away from VCPU_ID
> > KVM: selftests: Convert kvm_clock_test away from VCPU_ID
> > KVM: selftests: Convert hyperv_svm_test away from VCPU_ID
> > KVM: selftests: Convert hyperv_features away from VCPU_ID
> > KVM: selftests: Convert hyperv_clock away from VCPU_ID
> > KVM: selftests: Convert evmcs_test away from VCPU_ID
> > KVM: selftests: Convert emulator_error_test away from VCPU_ID
> > KVM: selftests: Convert debug_regs away from VCPU_ID
> > KVM: selftests: Add proper helper for advancing RIP in debug_regs
> > KVM: selftests: Convert amx_test away from VCPU_ID
> > KVM: selftests: Convert cr4_cpuid_sync_test away from VCPU_ID
> > KVM: selftests: Convert cpuid_test away from VCPU_ID
> > KVM: selftests: Convert userspace_io_test away from VCPU_ID
> > KVM: selftests: Convert vmx_invalid_nested_guest_state away from
> > VCPU_ID
> > KVM: selftests: Convert xen_vmcall_test away from VCPU_ID
> > KVM: selftests: Convert xen_shinfo_test away from VCPU_ID
> > KVM: selftests: Convert dirty_log_test away from VCPU_ID
> > KVM: selftests: Convert set_memory_region_test away from VCPU_ID
> > KVM: selftests: Convert system_counter_offset_test away from VCPU_ID
> > KVM: selftests: Track kvm_vcpu object in tsc_scaling_sync
> > KVM: selftests: Convert xapic_state_test away from hardcoded vCPU ID
> > KVM: selftests: Convert debug-exceptions away from VCPU_ID
> > KVM: selftests: Convert fix_hypercall_test away from VCPU_ID
> > KVM: selftests: Convert vgic_irq away from VCPU_ID
> > KVM: selftests: Make arm64's guest_get_vcpuid() declaration arm64-only
> > KVM: selftests: Move vm_is_unrestricted_guest() to x86-64
> > KVM: selftests: Add "arch" to common utils that have arch
> > implementations
> > KVM: selftests: Return created vcpu from vm_vcpu_add_default()
> > KVM: selftests: Rename vm_vcpu_add* helpers to better show
> > relationships
> > KVM: selftests: Convert set_boot_cpu_id away from global VCPU_IDs
> > KVM: selftests: Convert psci_test away from VCPU_ID
> > KVM: selftests: Convert hardware_disable_test to pass around vCPU
> > objects
> > KVM: selftests: Add VM creation helper that "returns" vCPUs
> > KVM: selftests: Convert steal_time away from VCPU_ID
> > KVM: selftests: Convert arch_timer away from VCPU_ID
> > KVM: selftests: Convert svm_nested_soft_inject_test away from VCPU_ID
> > KVM: selftests: Convert triple_fault_event_test away from VCPU_ID
> > KVM: selftests: Convert vgic_init away from
> > vm_create_default_with_vcpus()
> > KVM: selftests: Consolidate KVM_{G,S}ET_ONE_REG helpers
> > KVM: selftests: Sync stage before VM is freed in hypercalls test
> > KVM: selftests: Convert hypercalls test away from vm_create_default()
> > KVM: selftests: Convert xapic_ipi_test away from *_VCPU_ID
> > KVM: selftests: Convert sync_regs_test away from VCPU_ID
> > KVM: selftests: Convert s390's "resets" test away from VCPU_ID
> > KVM: selftests: Convert memop away from VCPU_ID
> > KVM: selftests: Convert s390x/diag318_test_handler away from VCPU_ID
> > KVM: selftests: Convert tprot away from VCPU_ID
> > KVM: selftests: Use vm_create() in tsc_scaling_sync
> > KVM: selftests: Use vm_create_with_vcpus() in max_guest_memory_test
> > KVM: selftests: Drop vm_create_default* helpers
> > KVM: selftests: Drop @vcpuids param from VM creators
> > KVM: selftests: Convert kvm_page_table_test away from reliance on
> > vcpu_id
> > KVM: selftests: Convert kvm_binary_stats_test away from vCPU IDs
> > KVM: selftests: Convert get-reg-list away from its "VCPU_ID"
> > KVM: selftests: Stop hardcoding vCPU IDs in vcpu_width_config
> > KVM: selftests: Stop conflating vCPU index and ID in perf tests
> > KVM: selftests: Remove vcpu_get() usage from dirty_log_test
> > KVM: selftests: Require vCPU output array when creating VM with vCPUs
> > KVM: selftests: Purge vm+vcpu_id == vcpu silliness
> > KVM: selftests: Drop vcpu_get(), rename vcpu_find() => vcpu_exists()
> > KVM: selftests: Remove vcpu_state() helper
> > KVM: selftests: Open code and drop 'struct kvm_vm' accessors
> > KVM: selftests: Drop @slot0_mem_pages from __vm_create_with_vcpus()
> > KVM: selftests: Drop @num_percpu_pages from __vm_create_with_vcpus()
> > KVM: selftests: Move per-VM/per-vCPU nr pages calculation to
> > __vm_create()
> > KVM: selftests: Trust that MAXPHYADDR > memslot0 in
> > vmx_apic_access_test
> > KVM: selftests: Drop DEFAULT_GUEST_PHY_PAGES, open code the magic
> > number
> > KVM: selftests: Return an 'unsigned int' from kvm_check_cap()
> > KVM: selftests: Add kvm_has_cap() to provide syntactic sugar
> > KVM: selftests: Add TEST_REQUIRE macros to reduce skipping copy+paste
> > KVM: selftests: Sanity check input to ioctls() at build time
> >
> > Documentation/virt/kvm/api.rst | 4 +-
> > .../selftests/kvm/aarch64/arch_timer.c | 79 +-
> > .../selftests/kvm/aarch64/debug-exceptions.c | 22 +-
> > .../selftests/kvm/aarch64/get-reg-list.c | 29 +-
> > .../selftests/kvm/aarch64/hypercalls.c | 90 +-
> > .../testing/selftests/kvm/aarch64/psci_test.c | 69 +-
> > .../selftests/kvm/aarch64/vcpu_width_config.c | 71 +-
> > .../testing/selftests/kvm/aarch64/vgic_init.c | 379 +++---
> > .../testing/selftests/kvm/aarch64/vgic_irq.c | 40 +-
> > .../selftests/kvm/access_tracking_perf_test.c | 92 +-
> > .../selftests/kvm/demand_paging_test.c | 49 +-
> > .../selftests/kvm/dirty_log_perf_test.c | 51 +-
> > tools/testing/selftests/kvm/dirty_log_test.c | 95 +-
> > .../selftests/kvm/hardware_disable_test.c | 29 +-
> > .../selftests/kvm/include/aarch64/processor.h | 28 +-
> > .../selftests/kvm/include/aarch64/vgic.h | 6 +-
> > .../selftests/kvm/include/kvm_util_base.h | 743 ++++++++---
> > .../selftests/kvm/include/perf_test_util.h | 5 +-
> > .../selftests/kvm/include/riscv/processor.h | 20 -
> > .../testing/selftests/kvm/include/test_util.h | 9 +
> > .../selftests/kvm/include/ucall_common.h | 2 +-
> > .../selftests/kvm/include/x86_64/evmcs.h | 2 +-
> > .../selftests/kvm/include/x86_64/processor.h | 109 +-
> > .../selftests/kvm/kvm_binary_stats_test.c | 31 +-
> > .../selftests/kvm/kvm_create_max_vcpus.c | 10 +-
> > .../selftests/kvm/kvm_page_table_test.c | 66 +-
> > .../selftests/kvm/lib/aarch64/processor.c | 81 +-
> > .../testing/selftests/kvm/lib/aarch64/ucall.c | 9 +-
> > .../testing/selftests/kvm/lib/aarch64/vgic.c | 54 +-
> > tools/testing/selftests/kvm/lib/elf.c | 1 -
> > tools/testing/selftests/kvm/lib/guest_modes.c | 6 +-
> > tools/testing/selftests/kvm/lib/kvm_util.c | 1104 +++--------------
> > .../selftests/kvm/lib/kvm_util_internal.h | 128 --
> > .../selftests/kvm/lib/perf_test_util.c | 84 +-
> > .../selftests/kvm/lib/riscv/processor.c | 111 +-
> > tools/testing/selftests/kvm/lib/riscv/ucall.c | 14 +-
> > .../kvm/lib/s390x/diag318_test_handler.c | 11 +-
> > .../selftests/kvm/lib/s390x/processor.c | 44 +-
> > tools/testing/selftests/kvm/lib/s390x/ucall.c | 8 +-
> > .../selftests/kvm/lib/x86_64/processor.c | 533 +++-----
> > tools/testing/selftests/kvm/lib/x86_64/svm.c | 6 +-
> > .../testing/selftests/kvm/lib/x86_64/ucall.c | 10 +-
> > tools/testing/selftests/kvm/lib/x86_64/vmx.c | 16 +-
> > .../selftests/kvm/max_guest_memory_test.c | 53 +-
> > .../kvm/memslot_modification_stress_test.c | 13 +-
> > .../testing/selftests/kvm/memslot_perf_test.c | 28 +-
> > tools/testing/selftests/kvm/rseq_test.c | 22 +-
> > tools/testing/selftests/kvm/s390x/memop.c | 93 +-
> > tools/testing/selftests/kvm/s390x/resets.c | 140 ++-
> > .../selftests/kvm/s390x/sync_regs_test.c | 45 +-
> > tools/testing/selftests/kvm/s390x/tprot.c | 25 +-
> > .../selftests/kvm/set_memory_region_test.c | 43 +-
> > tools/testing/selftests/kvm/steal_time.c | 120 +-
> > .../kvm/system_counter_offset_test.c | 35 +-
> > tools/testing/selftests/kvm/x86_64/amx_test.c | 56 +-
> > .../testing/selftests/kvm/x86_64/cpuid_test.c | 29 +-
> > .../kvm/x86_64/cr4_cpuid_sync_test.c | 22 +-
> > .../testing/selftests/kvm/x86_64/debug_regs.c | 77 +-
> > .../kvm/x86_64/emulator_error_test.c | 74 +-
> > .../testing/selftests/kvm/x86_64/evmcs_test.c | 61 +-
> > .../selftests/kvm/x86_64/fix_hypercall_test.c | 45 +-
> > .../kvm/x86_64/get_msr_index_features.c | 117 +-
> > .../selftests/kvm/x86_64/hyperv_clock.c | 25 +-
> > .../selftests/kvm/x86_64/hyperv_cpuid.c | 34 +-
> > .../selftests/kvm/x86_64/hyperv_features.c | 61 +-
> > .../selftests/kvm/x86_64/hyperv_svm_test.c | 20 +-
> > .../selftests/kvm/x86_64/kvm_clock_test.c | 29 +-
> > .../selftests/kvm/x86_64/kvm_pv_test.c | 33 +-
> > .../kvm/x86_64/max_vcpuid_cap_test.c | 28 +-
> > .../selftests/kvm/x86_64/mmio_warning_test.c | 16 +-
> > .../selftests/kvm/x86_64/mmu_role_test.c | 30 +-
> > .../selftests/kvm/x86_64/platform_info_test.c | 51 +-
> > .../kvm/x86_64/pmu_event_filter_test.c | 97 +-
> > .../selftests/kvm/x86_64/set_boot_cpu_id.c | 91 +-
> > .../selftests/kvm/x86_64/set_sregs_test.c | 47 +-
> > .../selftests/kvm/x86_64/sev_migrate_tests.c | 120 +-
> > tools/testing/selftests/kvm/x86_64/smm_test.c | 37 +-
> > .../testing/selftests/kvm/x86_64/state_test.c | 29 +-
> > .../selftests/kvm/x86_64/svm_int_ctl_test.c | 21 +-
> > .../kvm/x86_64/svm_nested_soft_inject_test.c | 17 +-
> > .../selftests/kvm/x86_64/svm_vmcall_test.c | 16 +-
> > .../selftests/kvm/x86_64/sync_regs_test.c | 62 +-
> > .../kvm/x86_64/triple_fault_event_test.c | 39 +-
> > .../selftests/kvm/x86_64/tsc_msrs_test.c | 35 +-
> > .../selftests/kvm/x86_64/tsc_scaling_sync.c | 25 +-
> > .../selftests/kvm/x86_64/userspace_io_test.c | 18 +-
> > .../kvm/x86_64/userspace_msr_exit_test.c | 187 ++-
> > .../kvm/x86_64/vmx_apic_access_test.c | 27 +-
> > .../kvm/x86_64/vmx_close_while_nested_test.c | 17 +-
> > .../selftests/kvm/x86_64/vmx_dirty_log_test.c | 13 +-
> > .../vmx_exception_with_invalid_guest_state.c | 68 +-
> > .../x86_64/vmx_invalid_nested_guest_state.c | 18 +-
> > .../kvm/x86_64/vmx_nested_tsc_scaling_test.c | 29 +-
> > .../selftests/kvm/x86_64/vmx_pmu_caps_test.c | 48 +-
> > .../kvm/x86_64/vmx_preemption_timer_test.c | 35 +-
> > .../kvm/x86_64/vmx_set_nested_state_test.c | 91 +-
> > .../kvm/x86_64/vmx_tsc_adjust_test.c | 13 +-
> > .../selftests/kvm/x86_64/xapic_ipi_test.c | 48 +-
> > .../selftests/kvm/x86_64/xapic_state_test.c | 60 +-
> > .../selftests/kvm/x86_64/xen_shinfo_test.c | 73 +-
> > .../selftests/kvm/x86_64/xen_vmcall_test.c | 25 +-
> > .../selftests/kvm/x86_64/xss_msr_test.c | 56 +-
> > 102 files changed, 3059 insertions(+), 4178 deletions(-)
> > delete mode 100644 tools/testing/selftests/kvm/lib/kvm_util_internal.h
> >
> >
> > base-commit: 55371f1d0c01357f29da613f7525c3f252320bbf
>

2022-06-08 23:53:22

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

On Wed, Jun 08, 2022, Marc Zyngier wrote:
> On 2022-06-07 16:27, Paolo Bonzini wrote:
> > Marc, Christian, Anup, can you please give this a go?
>
> Can you please, pretty please, once and for all, kill that alias you
> seem to have for me and email me on an address I actually can read?
>
> I can't remember how many times you emailed me on my ex @arm.com address
> over the past 2+years...
>
> The same thing probably applies to Sean, btw.

Ha! I was wondering how my old @intel address snuck in...

On the aarch64 side, with the following tweaks, courtesy of Raghu, all tests
pass. I'll work these into the next version, and hopefully also learn how to
run on aarch64 myself...

Note, the i => 0 "fix" in test_v3_typer_accesses() is a direct revert of patch 3,
"KVM: selftests: Fix typo in vgic_init test". I'll just drop that patch unless
someone figures out why doing the right thing causes the test to fail.

diff --git a/tools/testing/selftests/kvm/aarch64/vgic_init.c b/tools/testing/selftests/kvm/aarch64/vgic_init.c
index b91ea02a8a80..66b7e9c76370 100644
--- a/tools/testing/selftests/kvm/aarch64/vgic_init.c
+++ b/tools/testing/selftests/kvm/aarch64/vgic_init.c
@@ -317,7 +317,7 @@ static void test_vgic_then_vcpus(uint32_t gic_dev_type)

/* Add the rest of the VCPUs */
for (i = 1; i < NR_VCPUS; ++i)
- vm_vcpu_add(v.vm, i, guest_code);
+ vcpus[i] = vm_vcpu_add(v.vm, i, guest_code);

ret = run_vcpu(vcpus[3]);
TEST_ASSERT(ret == -EINVAL, "dist/rdist overlap detected on 1st vcpu run");
@@ -424,7 +424,7 @@ static void test_v3_typer_accesses(void)
KVM_DEV_ARM_VGIC_CTRL_INIT, NULL);

for (i = 0; i < NR_VCPUS ; i++) {
- ret = v3_redist_reg_get(v.gic_fd, i, GICR_TYPER, &val);
+ ret = v3_redist_reg_get(v.gic_fd, 0, GICR_TYPER, &val);
TEST_ASSERT(!ret && !val, "read GICR_TYPER before rdist region setting");
}

@@ -654,11 +654,12 @@ static void test_v3_its_region(void)
*/
int test_kvm_device(uint32_t gic_dev_type)
{
+ struct kvm_vcpu *vcpus[NR_VCPUS];
struct vm_gic v;
uint32_t other;
int ret;

- v.vm = vm_create_with_vcpus(NR_VCPUS, guest_code, NULL);
+ v.vm = vm_create_with_vcpus(NR_VCPUS, guest_code, vcpus);

/* try to create a non existing KVM device */
ret = __kvm_test_create_device(v.vm, 0);
diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
index b3116c151d1c..17f7ef975d5c 100644
--- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
+++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
@@ -419,7 +419,7 @@ static void run_test(struct vcpu_config *c)

check_supported(c);

- vm = vm_create_barebones();
+ vm = vm_create(1);
prepare_vcpu_init(c, &init);
vcpu = aarch64_vcpu_add(vm, 0, &init, NULL);
finalize_vcpu(vcpu, c);

2022-06-09 06:50:54

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

On Wed, Jun 8, 2022 at 9:26 PM Anup Patel <[email protected]> wrote:
>
> On Tue, Jun 7, 2022 at 8:57 PM Paolo Bonzini <[email protected]> wrote:
> >
> > Marc, Christian, Anup, can you please give this a go?
>
> Sure, I will try this series.

I tried to apply this series on top of kvm/next and kvm/queue but
I always get conflicts. It seems this series is dependent on other
in-flight patches.

Is there a branch somewhere in a public repo ?

Regards,
Anup

>
> Regards,
> Anup
>
> >
> > Paolo
> >
> > On 6/3/22 02:41, Sean Christopherson wrote:
> > > Overhaul KVM's selftest APIs to get selftests to a state where adding new
> > > features and writing tests is less painful/disgusting.
> > >
> > > Patches 1 fixes a goof in kvm/queue and should be squashed.
> > >
> > > I would really, really, really like to get this queued up sooner than
> > > later, or maybe just thrown into a separate selftests-specific branch that
> > > folks can develop against. Rebasing is tedious, frustrating, and time
> > > consuming. And spoiler alert, there's another 42 x86-centric patches
> > > inbound that builds on this series to clean up CPUID related crud...
> > >
> > > The primary theme is to stop treating tests like second class citizens.
> > > Stop hiding vcpu, kvm_vm, etc... There's no sensitive data/constructs, and
> > > the encapsulation has led to really, really bad and difficult to maintain
> > > code. E.g. having to pass around the VM just to call a vCPU ioctl(),
> > > arbitrary non-zero vCPU IDs, tests having to care about the vCPU ID in the
> > > first place, etc...
> > >
> > > The other theme in the rework is to deduplicate code and try to set us
> > > up for success in the future. E.g. provide macros/helpers instead of
> > > spamming CTRL-C => CTRL-V (see the -1k LoC), structure the VM creation
> > > APIs to build on one another, etc...
> > >
> > > The absurd patch count (as opposed to just ridiculous) is due to converting
> > > each test away from using hardcoded vCPU IDs in a separate patch. The vast
> > > majority of those patches probably aren't worth reviewing in depth, the
> > > changes are mostly mechanical in nature.
> > >
> > > However, _running_ non-x86 tests (or tests that have unique non-x86
> > > behavior) would be extremely valuable. All patches have been compile tested
> > > on x86, arm, risc-v, and s390, but I've only run the tests on x86. Based on
> > > my track record for the x86+common tests, I will be very, very surprised if
> > > I didn't break any of the non-x86 tests, e.g. pthread_create()'s 'void *'
> > > param tripped me up multiple times.
> > >
> > > I have not run x86's amx_test due to lack of hardware. I also haven't run
> > > sev_migration; something is wonky in either the upstream support for INIT_EX
> > > or in our test machines and I can't get SEV to initialize.
> > >
> > > v2:
> > > - Drop the forced -Werror patch. [Vitaly]
> > > - Add TEST_REQUIRE to reduce KSFT_SKIP boilerplate.
> > > - Rebase to kvm/queue, commit 55371f1d0c01.
> > > - Clean up even more bad copy+paste code (x86 was hiding a lot of crud).
> > > - Assert that the input to an ioctl() is (likely) the correct struct.
> > >
> > > v1: https://lore.kernel.org/all/[email protected]
> > >
> > > Sean Christopherson (144):
> > > KVM: Fix references to non-existent KVM_CAP_TRIPLE_FAULT_EVENT
> > > KVM: selftests: Fix buggy-but-benign check in
> > > test_v3_new_redist_regions()
> > > KVM: selftests: Fix typo in vgic_init test
> > > KVM: selftests: Drop stale declarations from kvm_util_base.h
> > > KVM: selftests: Always open VM file descriptors with O_RDWR
> > > KVM: selftests: Add another underscore to inner ioctl() helpers
> > > KVM: selftests: Make vcpu_ioctl() a wrapper to pretty print ioctl name
> > > KVM: selftests: Drop @mode from common vm_create() helper
> > > KVM: selftests: Split vcpu_set_nested_state() into two helpers
> > > KVM: sefltests: Use vcpu_ioctl() and __vcpu_ioctl() helpers
> > > KVM: selftests: Add __vcpu_run() helper
> > > KVM: selftests: Use vcpu_access_device_attr() in arm64 code
> > > KVM: selftests: Remove vcpu_get_fd()
> > > KVM: selftests: Add vcpu_get() to retrieve and assert on vCPU
> > > existence
> > > KVM: selftests: Make vm_ioctl() a wrapper to pretty print ioctl name
> > > KVM: sefltests: Use vm_ioctl() and __vm_ioctl() helpers
> > > KVM: selftests: Make kvm_ioctl() a wrapper to pretty print ioctl name
> > > KVM: selftests: Use kvm_ioctl() helpers
> > > KVM: selftests: Use __KVM_SYSCALL_ERROR() to handle non-KVM syscall
> > > errors
> > > KVM: selftests: Make x86-64's register dump helpers static
> > > KVM: selftests: Get rid of kvm_util_internal.h
> > > KVM: selftests: Use KVM_IOCTL_ERROR() for one-off arm64 ioctls
> > > KVM: selftests: Drop @test param from kvm_create_device()
> > > KVM: selftests: Move KVM_CREATE_DEVICE_TEST code to separate helper
> > > KVM: selftests: Multiplex return code and fd in __kvm_create_device()
> > > KVM: selftests: Rename KVM_HAS_DEVICE_ATTR helpers for consistency
> > > KVM: selftests: Drop 'int' return from asserting *_has_device_attr()
> > > KVM: selftests: Split get/set device_attr helpers
> > > KVM: selftests: Add a VM backpointer to 'struct vcpu'
> > > KVM: selftests: Consolidate KVM_ENABLE_CAP usage
> > > KVM: selftests: Simplify KVM_ENABLE_CAP helper APIs
> > > KVM: selftests: Cache list of MSRs to save/restore
> > > KVM: selftests: Harden and comment XSS / KVM_SET_MSRS interaction
> > > KVM: selftests: Dedup MSR index list helpers, simplify dedicated test
> > > KVM: selftests: Rename MP_STATE and GUEST_DEBUG helpers for
> > > consistency
> > > KVM: selftest: Add proper helpers for x86-specific save/restore ioctls
> > > KVM: selftests: Add vm_create_*() variants to expose/return 'struct
> > > vcpu'
> > > KVM: selftests: Push vm_adjust_num_guest_pages() into "w/o vCPUs"
> > > helper
> > > KVM: selftests: Use vm_create_without_vcpus() in set_boot_cpu_id
> > > KVM: selftests: Use vm_create_without_vcpus() in dirty_log_test
> > > KVM: selftests: Use vm_create_without_vcpus() in hardware_disable_test
> > > KVM: selftests: Use vm_create_without_vcpus() in psci_test
> > > KVM: selftests: Rename vm_create() => vm_create_barebones(), drop
> > > param
> > > KVM: selftests: Rename vm_create_without_vcpus() => vm_create()
> > > KVM: selftests: Make vm_create() a wrapper that specifies
> > > VM_MODE_DEFAULT
> > > KVM: selftests: Rename xAPIC state test's vcpu struct
> > > KVM: selftests: Rename vcpu.state => vcpu.run
> > > KVM: selftests: Rename 'struct vcpu' to 'struct kvm_vcpu'
> > > KVM: selftests: Return the created vCPU from vm_vcpu_add()
> > > KVM: selftests: Convert memslot_perf_test away from VCPU_ID
> > > KVM: selftests: Convert rseq_test away from VCPU_ID
> > > KVM: selftests: Convert xss_msr_test away from VCPU_ID
> > > KVM: selftests: Convert vmx_preemption_timer_test away from VCPU_ID
> > > KVM: selftests: Convert vmx_pmu_msrs_test away from VCPU_ID
> > > KVM: selftests: Convert vmx_set_nested_state_test away from VCPU_ID
> > > KVM: selftests: Convert vmx_tsc_adjust_test away from VCPU_ID
> > > KVM: selftests: Convert mmu_role_test away from VCPU_ID
> > > KVM: selftests: Convert pmu_event_filter_test away from VCPU_ID
> > > KVM: selftests: Convert smm_test away from VCPU_ID
> > > KVM: selftests: Convert state_test away from VCPU_ID
> > > KVM: selftests: Convert svm_int_ctl_test away from VCPU_ID
> > > KVM: selftests: Convert svm_vmcall_test away from VCPU_ID
> > > KVM: selftests: Convert sync_regs_test away from VCPU_ID
> > > KVM: selftests: Convert hyperv_cpuid away from VCPU_ID
> > > KVM: selftests: Convert kvm_pv_test away from VCPU_ID
> > > KVM: selftests: Convert platform_info_test away from VCPU_ID
> > > KVM: selftests: Convert vmx_nested_tsc_scaling_test away from VCPU_ID
> > > KVM: selftests: Convert set_sregs_test away from VCPU_ID
> > > KVM: selftests: Convert vmx_dirty_log_test away from VCPU_ID
> > > KVM: selftests: Convert vmx_close_while_nested_test away from VCPU_ID
> > > KVM: selftests: Convert vmx_apic_access_test away from VCPU_ID
> > > KVM: selftests: Convert userspace_msr_exit_test away from VCPU_ID
> > > KVM: selftests: Convert vmx_exception_with_invalid_guest_state away
> > > from VCPU_ID
> > > KVM: selftests: Convert tsc_msrs_test away from VCPU_ID
> > > KVM: selftests: Convert kvm_clock_test away from VCPU_ID
> > > KVM: selftests: Convert hyperv_svm_test away from VCPU_ID
> > > KVM: selftests: Convert hyperv_features away from VCPU_ID
> > > KVM: selftests: Convert hyperv_clock away from VCPU_ID
> > > KVM: selftests: Convert evmcs_test away from VCPU_ID
> > > KVM: selftests: Convert emulator_error_test away from VCPU_ID
> > > KVM: selftests: Convert debug_regs away from VCPU_ID
> > > KVM: selftests: Add proper helper for advancing RIP in debug_regs
> > > KVM: selftests: Convert amx_test away from VCPU_ID
> > > KVM: selftests: Convert cr4_cpuid_sync_test away from VCPU_ID
> > > KVM: selftests: Convert cpuid_test away from VCPU_ID
> > > KVM: selftests: Convert userspace_io_test away from VCPU_ID
> > > KVM: selftests: Convert vmx_invalid_nested_guest_state away from
> > > VCPU_ID
> > > KVM: selftests: Convert xen_vmcall_test away from VCPU_ID
> > > KVM: selftests: Convert xen_shinfo_test away from VCPU_ID
> > > KVM: selftests: Convert dirty_log_test away from VCPU_ID
> > > KVM: selftests: Convert set_memory_region_test away from VCPU_ID
> > > KVM: selftests: Convert system_counter_offset_test away from VCPU_ID
> > > KVM: selftests: Track kvm_vcpu object in tsc_scaling_sync
> > > KVM: selftests: Convert xapic_state_test away from hardcoded vCPU ID
> > > KVM: selftests: Convert debug-exceptions away from VCPU_ID
> > > KVM: selftests: Convert fix_hypercall_test away from VCPU_ID
> > > KVM: selftests: Convert vgic_irq away from VCPU_ID
> > > KVM: selftests: Make arm64's guest_get_vcpuid() declaration arm64-only
> > > KVM: selftests: Move vm_is_unrestricted_guest() to x86-64
> > > KVM: selftests: Add "arch" to common utils that have arch
> > > implementations
> > > KVM: selftests: Return created vcpu from vm_vcpu_add_default()
> > > KVM: selftests: Rename vm_vcpu_add* helpers to better show
> > > relationships
> > > KVM: selftests: Convert set_boot_cpu_id away from global VCPU_IDs
> > > KVM: selftests: Convert psci_test away from VCPU_ID
> > > KVM: selftests: Convert hardware_disable_test to pass around vCPU
> > > objects
> > > KVM: selftests: Add VM creation helper that "returns" vCPUs
> > > KVM: selftests: Convert steal_time away from VCPU_ID
> > > KVM: selftests: Convert arch_timer away from VCPU_ID
> > > KVM: selftests: Convert svm_nested_soft_inject_test away from VCPU_ID
> > > KVM: selftests: Convert triple_fault_event_test away from VCPU_ID
> > > KVM: selftests: Convert vgic_init away from
> > > vm_create_default_with_vcpus()
> > > KVM: selftests: Consolidate KVM_{G,S}ET_ONE_REG helpers
> > > KVM: selftests: Sync stage before VM is freed in hypercalls test
> > > KVM: selftests: Convert hypercalls test away from vm_create_default()
> > > KVM: selftests: Convert xapic_ipi_test away from *_VCPU_ID
> > > KVM: selftests: Convert sync_regs_test away from VCPU_ID
> > > KVM: selftests: Convert s390's "resets" test away from VCPU_ID
> > > KVM: selftests: Convert memop away from VCPU_ID
> > > KVM: selftests: Convert s390x/diag318_test_handler away from VCPU_ID
> > > KVM: selftests: Convert tprot away from VCPU_ID
> > > KVM: selftests: Use vm_create() in tsc_scaling_sync
> > > KVM: selftests: Use vm_create_with_vcpus() in max_guest_memory_test
> > > KVM: selftests: Drop vm_create_default* helpers
> > > KVM: selftests: Drop @vcpuids param from VM creators
> > > KVM: selftests: Convert kvm_page_table_test away from reliance on
> > > vcpu_id
> > > KVM: selftests: Convert kvm_binary_stats_test away from vCPU IDs
> > > KVM: selftests: Convert get-reg-list away from its "VCPU_ID"
> > > KVM: selftests: Stop hardcoding vCPU IDs in vcpu_width_config
> > > KVM: selftests: Stop conflating vCPU index and ID in perf tests
> > > KVM: selftests: Remove vcpu_get() usage from dirty_log_test
> > > KVM: selftests: Require vCPU output array when creating VM with vCPUs
> > > KVM: selftests: Purge vm+vcpu_id == vcpu silliness
> > > KVM: selftests: Drop vcpu_get(), rename vcpu_find() => vcpu_exists()
> > > KVM: selftests: Remove vcpu_state() helper
> > > KVM: selftests: Open code and drop 'struct kvm_vm' accessors
> > > KVM: selftests: Drop @slot0_mem_pages from __vm_create_with_vcpus()
> > > KVM: selftests: Drop @num_percpu_pages from __vm_create_with_vcpus()
> > > KVM: selftests: Move per-VM/per-vCPU nr pages calculation to
> > > __vm_create()
> > > KVM: selftests: Trust that MAXPHYADDR > memslot0 in
> > > vmx_apic_access_test
> > > KVM: selftests: Drop DEFAULT_GUEST_PHY_PAGES, open code the magic
> > > number
> > > KVM: selftests: Return an 'unsigned int' from kvm_check_cap()
> > > KVM: selftests: Add kvm_has_cap() to provide syntactic sugar
> > > KVM: selftests: Add TEST_REQUIRE macros to reduce skipping copy+paste
> > > KVM: selftests: Sanity check input to ioctls() at build time
> > >
> > > Documentation/virt/kvm/api.rst | 4 +-
> > > .../selftests/kvm/aarch64/arch_timer.c | 79 +-
> > > .../selftests/kvm/aarch64/debug-exceptions.c | 22 +-
> > > .../selftests/kvm/aarch64/get-reg-list.c | 29 +-
> > > .../selftests/kvm/aarch64/hypercalls.c | 90 +-
> > > .../testing/selftests/kvm/aarch64/psci_test.c | 69 +-
> > > .../selftests/kvm/aarch64/vcpu_width_config.c | 71 +-
> > > .../testing/selftests/kvm/aarch64/vgic_init.c | 379 +++---
> > > .../testing/selftests/kvm/aarch64/vgic_irq.c | 40 +-
> > > .../selftests/kvm/access_tracking_perf_test.c | 92 +-
> > > .../selftests/kvm/demand_paging_test.c | 49 +-
> > > .../selftests/kvm/dirty_log_perf_test.c | 51 +-
> > > tools/testing/selftests/kvm/dirty_log_test.c | 95 +-
> > > .../selftests/kvm/hardware_disable_test.c | 29 +-
> > > .../selftests/kvm/include/aarch64/processor.h | 28 +-
> > > .../selftests/kvm/include/aarch64/vgic.h | 6 +-
> > > .../selftests/kvm/include/kvm_util_base.h | 743 ++++++++---
> > > .../selftests/kvm/include/perf_test_util.h | 5 +-
> > > .../selftests/kvm/include/riscv/processor.h | 20 -
> > > .../testing/selftests/kvm/include/test_util.h | 9 +
> > > .../selftests/kvm/include/ucall_common.h | 2 +-
> > > .../selftests/kvm/include/x86_64/evmcs.h | 2 +-
> > > .../selftests/kvm/include/x86_64/processor.h | 109 +-
> > > .../selftests/kvm/kvm_binary_stats_test.c | 31 +-
> > > .../selftests/kvm/kvm_create_max_vcpus.c | 10 +-
> > > .../selftests/kvm/kvm_page_table_test.c | 66 +-
> > > .../selftests/kvm/lib/aarch64/processor.c | 81 +-
> > > .../testing/selftests/kvm/lib/aarch64/ucall.c | 9 +-
> > > .../testing/selftests/kvm/lib/aarch64/vgic.c | 54 +-
> > > tools/testing/selftests/kvm/lib/elf.c | 1 -
> > > tools/testing/selftests/kvm/lib/guest_modes.c | 6 +-
> > > tools/testing/selftests/kvm/lib/kvm_util.c | 1104 +++--------------
> > > .../selftests/kvm/lib/kvm_util_internal.h | 128 --
> > > .../selftests/kvm/lib/perf_test_util.c | 84 +-
> > > .../selftests/kvm/lib/riscv/processor.c | 111 +-
> > > tools/testing/selftests/kvm/lib/riscv/ucall.c | 14 +-
> > > .../kvm/lib/s390x/diag318_test_handler.c | 11 +-
> > > .../selftests/kvm/lib/s390x/processor.c | 44 +-
> > > tools/testing/selftests/kvm/lib/s390x/ucall.c | 8 +-
> > > .../selftests/kvm/lib/x86_64/processor.c | 533 +++-----
> > > tools/testing/selftests/kvm/lib/x86_64/svm.c | 6 +-
> > > .../testing/selftests/kvm/lib/x86_64/ucall.c | 10 +-
> > > tools/testing/selftests/kvm/lib/x86_64/vmx.c | 16 +-
> > > .../selftests/kvm/max_guest_memory_test.c | 53 +-
> > > .../kvm/memslot_modification_stress_test.c | 13 +-
> > > .../testing/selftests/kvm/memslot_perf_test.c | 28 +-
> > > tools/testing/selftests/kvm/rseq_test.c | 22 +-
> > > tools/testing/selftests/kvm/s390x/memop.c | 93 +-
> > > tools/testing/selftests/kvm/s390x/resets.c | 140 ++-
> > > .../selftests/kvm/s390x/sync_regs_test.c | 45 +-
> > > tools/testing/selftests/kvm/s390x/tprot.c | 25 +-
> > > .../selftests/kvm/set_memory_region_test.c | 43 +-
> > > tools/testing/selftests/kvm/steal_time.c | 120 +-
> > > .../kvm/system_counter_offset_test.c | 35 +-
> > > tools/testing/selftests/kvm/x86_64/amx_test.c | 56 +-
> > > .../testing/selftests/kvm/x86_64/cpuid_test.c | 29 +-
> > > .../kvm/x86_64/cr4_cpuid_sync_test.c | 22 +-
> > > .../testing/selftests/kvm/x86_64/debug_regs.c | 77 +-
> > > .../kvm/x86_64/emulator_error_test.c | 74 +-
> > > .../testing/selftests/kvm/x86_64/evmcs_test.c | 61 +-
> > > .../selftests/kvm/x86_64/fix_hypercall_test.c | 45 +-
> > > .../kvm/x86_64/get_msr_index_features.c | 117 +-
> > > .../selftests/kvm/x86_64/hyperv_clock.c | 25 +-
> > > .../selftests/kvm/x86_64/hyperv_cpuid.c | 34 +-
> > > .../selftests/kvm/x86_64/hyperv_features.c | 61 +-
> > > .../selftests/kvm/x86_64/hyperv_svm_test.c | 20 +-
> > > .../selftests/kvm/x86_64/kvm_clock_test.c | 29 +-
> > > .../selftests/kvm/x86_64/kvm_pv_test.c | 33 +-
> > > .../kvm/x86_64/max_vcpuid_cap_test.c | 28 +-
> > > .../selftests/kvm/x86_64/mmio_warning_test.c | 16 +-
> > > .../selftests/kvm/x86_64/mmu_role_test.c | 30 +-
> > > .../selftests/kvm/x86_64/platform_info_test.c | 51 +-
> > > .../kvm/x86_64/pmu_event_filter_test.c | 97 +-
> > > .../selftests/kvm/x86_64/set_boot_cpu_id.c | 91 +-
> > > .../selftests/kvm/x86_64/set_sregs_test.c | 47 +-
> > > .../selftests/kvm/x86_64/sev_migrate_tests.c | 120 +-
> > > tools/testing/selftests/kvm/x86_64/smm_test.c | 37 +-
> > > .../testing/selftests/kvm/x86_64/state_test.c | 29 +-
> > > .../selftests/kvm/x86_64/svm_int_ctl_test.c | 21 +-
> > > .../kvm/x86_64/svm_nested_soft_inject_test.c | 17 +-
> > > .../selftests/kvm/x86_64/svm_vmcall_test.c | 16 +-
> > > .../selftests/kvm/x86_64/sync_regs_test.c | 62 +-
> > > .../kvm/x86_64/triple_fault_event_test.c | 39 +-
> > > .../selftests/kvm/x86_64/tsc_msrs_test.c | 35 +-
> > > .../selftests/kvm/x86_64/tsc_scaling_sync.c | 25 +-
> > > .../selftests/kvm/x86_64/userspace_io_test.c | 18 +-
> > > .../kvm/x86_64/userspace_msr_exit_test.c | 187 ++-
> > > .../kvm/x86_64/vmx_apic_access_test.c | 27 +-
> > > .../kvm/x86_64/vmx_close_while_nested_test.c | 17 +-
> > > .../selftests/kvm/x86_64/vmx_dirty_log_test.c | 13 +-
> > > .../vmx_exception_with_invalid_guest_state.c | 68 +-
> > > .../x86_64/vmx_invalid_nested_guest_state.c | 18 +-
> > > .../kvm/x86_64/vmx_nested_tsc_scaling_test.c | 29 +-
> > > .../selftests/kvm/x86_64/vmx_pmu_caps_test.c | 48 +-
> > > .../kvm/x86_64/vmx_preemption_timer_test.c | 35 +-
> > > .../kvm/x86_64/vmx_set_nested_state_test.c | 91 +-
> > > .../kvm/x86_64/vmx_tsc_adjust_test.c | 13 +-
> > > .../selftests/kvm/x86_64/xapic_ipi_test.c | 48 +-
> > > .../selftests/kvm/x86_64/xapic_state_test.c | 60 +-
> > > .../selftests/kvm/x86_64/xen_shinfo_test.c | 73 +-
> > > .../selftests/kvm/x86_64/xen_vmcall_test.c | 25 +-
> > > .../selftests/kvm/x86_64/xss_msr_test.c | 56 +-
> > > 102 files changed, 3059 insertions(+), 4178 deletions(-)
> > > delete mode 100644 tools/testing/selftests/kvm/lib/kvm_util_internal.h
> > >
> > >
> > > base-commit: 55371f1d0c01357f29da613f7525c3f252320bbf
> >

2022-06-09 08:01:41

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

On Wed, Jun 08, 2022 at 11:20:06PM +0000, Sean Christopherson wrote:
> On Wed, Jun 08, 2022, Marc Zyngier wrote:
> > On 2022-06-07 16:27, Paolo Bonzini wrote:
> > > Marc, Christian, Anup, can you please give this a go?
> >
> > Can you please, pretty please, once and for all, kill that alias you
> > seem to have for me and email me on an address I actually can read?
> >
> > I can't remember how many times you emailed me on my ex @arm.com address
> > over the past 2+years...
> >
> > The same thing probably applies to Sean, btw.
>
> Ha! I was wondering how my old @intel address snuck in...
>
> On the aarch64 side, with the following tweaks, courtesy of Raghu, all tests
> pass. I'll work these into the next version, and hopefully also learn how to
> run on aarch64 myself...
>
> Note, the i => 0 "fix" in test_v3_typer_accesses() is a direct revert of patch 3,
> "KVM: selftests: Fix typo in vgic_init test". I'll just drop that patch unless
> someone figures out why doing the right thing causes the test to fail.

CCing Eric for that one.

>
> diff --git a/tools/testing/selftests/kvm/aarch64/vgic_init.c b/tools/testing/selftests/kvm/aarch64/vgic_init.c
> index b91ea02a8a80..66b7e9c76370 100644
> --- a/tools/testing/selftests/kvm/aarch64/vgic_init.c
> +++ b/tools/testing/selftests/kvm/aarch64/vgic_init.c
> @@ -317,7 +317,7 @@ static void test_vgic_then_vcpus(uint32_t gic_dev_type)
>
> /* Add the rest of the VCPUs */
> for (i = 1; i < NR_VCPUS; ++i)
> - vm_vcpu_add(v.vm, i, guest_code);
> + vcpus[i] = vm_vcpu_add(v.vm, i, guest_code);
>
> ret = run_vcpu(vcpus[3]);
> TEST_ASSERT(ret == -EINVAL, "dist/rdist overlap detected on 1st vcpu run");
> @@ -424,7 +424,7 @@ static void test_v3_typer_accesses(void)
> KVM_DEV_ARM_VGIC_CTRL_INIT, NULL);
>
> for (i = 0; i < NR_VCPUS ; i++) {
> - ret = v3_redist_reg_get(v.gic_fd, i, GICR_TYPER, &val);
> + ret = v3_redist_reg_get(v.gic_fd, 0, GICR_TYPER, &val);
> TEST_ASSERT(!ret && !val, "read GICR_TYPER before rdist region setting");
> }
>
> @@ -654,11 +654,12 @@ static void test_v3_its_region(void)
> */
> int test_kvm_device(uint32_t gic_dev_type)
> {
> + struct kvm_vcpu *vcpus[NR_VCPUS];
> struct vm_gic v;
> uint32_t other;
> int ret;
>
> - v.vm = vm_create_with_vcpus(NR_VCPUS, guest_code, NULL);
> + v.vm = vm_create_with_vcpus(NR_VCPUS, guest_code, vcpus);
>
> /* try to create a non existing KVM device */
> ret = __kvm_test_create_device(v.vm, 0);
> diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> index b3116c151d1c..17f7ef975d5c 100644
> --- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> +++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> @@ -419,7 +419,7 @@ static void run_test(struct vcpu_config *c)
>
> check_supported(c);
>
> - vm = vm_create_barebones();
> + vm = vm_create(1);

Hmm, looks like something, somewhere for AArch64 needs improving to avoid
strangeness like this. I'll look into it after we get this series merged.

> prepare_vcpu_init(c, &init);
> vcpu = aarch64_vcpu_add(vm, 0, &init, NULL);
> finalize_vcpu(vcpu, c);
>

Thanks,
drew

2022-06-09 15:14:52

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

On Thu, Jun 09, 2022, Anup Patel wrote:
> On Wed, Jun 8, 2022 at 9:26 PM Anup Patel <[email protected]> wrote:
> >
> > On Tue, Jun 7, 2022 at 8:57 PM Paolo Bonzini <[email protected]> wrote:
> > >
> > > Marc, Christian, Anup, can you please give this a go?
> >
> > Sure, I will try this series.
>
> I tried to apply this series on top of kvm/next and kvm/queue but
> I always get conflicts. It seems this series is dependent on other
> in-flight patches.

Hrm, that's odd, it's based directly on kvm/queue, commit 55371f1d0c01 ("KVM: ...).

> Is there a branch somewhere in a public repo ?

https://github.com/sean-jc/linux/tree/x86/selftests_overhaul

2022-06-09 15:42:52

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

On Thu, Jun 09, 2022, Andrew Jones wrote:
> On Wed, Jun 08, 2022 at 11:20:06PM +0000, Sean Christopherson wrote:
> > diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> > index b3116c151d1c..17f7ef975d5c 100644
> > --- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> > +++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> > @@ -419,7 +419,7 @@ static void run_test(struct vcpu_config *c)
> >
> > check_supported(c);
> >
> > - vm = vm_create_barebones();
> > + vm = vm_create(1);
>
> Hmm, looks like something, somewhere for AArch64 needs improving to avoid
> strangeness like this. I'll look into it after we get this series merged.

Huh, you're right, that is odd. Ah, duh, aarch64_vcpu_add() allocates a stack
for the vCPU, and that will fail if there's no memslot from which to allocate
guest memory.

So, this is my goof in

KVM: selftests: Rename vm_create() => vm_create_barebones(), drop param

get-reg-list should first be converted to vm_create_without_vcpus(). I'll also
add a comment explaining that vm_create_barebones() can be used with __vm_vcpu_add(),
but not the "full" vm_vcpu_add() or vm_arch_vcpu_add() variants.

> > prepare_vcpu_init(c, &init);
> > vcpu = aarch64_vcpu_add(vm, 0, &init, NULL);
> > finalize_vcpu(vcpu, c);
> >
>
> Thanks,
> drew
>

2022-06-09 18:12:52

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

On Thu, Jun 09, 2022, Sean Christopherson wrote:
> On Thu, Jun 09, 2022, Andrew Jones wrote:
> > On Wed, Jun 08, 2022 at 11:20:06PM +0000, Sean Christopherson wrote:
> > > diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> > > index b3116c151d1c..17f7ef975d5c 100644
> > > --- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> > > +++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> > > @@ -419,7 +419,7 @@ static void run_test(struct vcpu_config *c)
> > >
> > > check_supported(c);
> > >
> > > - vm = vm_create_barebones();
> > > + vm = vm_create(1);
> >
> > Hmm, looks like something, somewhere for AArch64 needs improving to avoid
> > strangeness like this. I'll look into it after we get this series merged.
>
> Huh, you're right, that is odd. Ah, duh, aarch64_vcpu_add() allocates a stack
> for the vCPU, and that will fail if there's no memslot from which to allocate
> guest memory.
>
> So, this is my goof in
>
> KVM: selftests: Rename vm_create() => vm_create_barebones(), drop param
>
> get-reg-list should first be converted to vm_create_without_vcpus(). I'll also
> add a comment explaining that vm_create_barebones() can be used with __vm_vcpu_add(),
> but not the "full" vm_vcpu_add() or vm_arch_vcpu_add() variants.

Actually, I agree with your assessment. A better solution is to open code the
calls to add and setup the vCPU. It's a small amount of code duplication, but I
actually like the end result because it better documents the test's dependencies.

Assuming it actually works, i.e. the stack setup is truly unnecessary, I'll add a
patch like so before the barebones change.

diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
index ecfb773ec41e..7bba365b1522 100644
--- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
+++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
@@ -418,7 +418,8 @@ static void run_test(struct vcpu_config *c)

vm = vm_create(DEFAULT_GUEST_PHY_PAGES);
prepare_vcpu_init(c, &init);
- aarch64_vcpu_add_default(vm, 0, &init, NULL);
+ vm_vcpu_add(vm, vcpuid);
+ aarch64_vcpu_setup(vm, 0, &init);
finalize_vcpu(vm, 0, c);

reg_list = vcpu_get_reg_list(vm, 0);

2022-06-09 21:08:31

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

On Thu, Jun 09, 2022, Andrew Jones wrote:
> On Wed, Jun 08, 2022 at 11:20:06PM +0000, Sean Christopherson wrote:
> > On Wed, Jun 08, 2022, Marc Zyngier wrote:
> > > On 2022-06-07 16:27, Paolo Bonzini wrote:
> > > > Marc, Christian, Anup, can you please give this a go?
> > >
> > > Can you please, pretty please, once and for all, kill that alias you
> > > seem to have for me and email me on an address I actually can read?
> > >
> > > I can't remember how many times you emailed me on my ex @arm.com address
> > > over the past 2+years...
> > >
> > > The same thing probably applies to Sean, btw.
> >
> > Ha! I was wondering how my old @intel address snuck in...
> >
> > On the aarch64 side, with the following tweaks, courtesy of Raghu, all tests
> > pass. I'll work these into the next version, and hopefully also learn how to
> > run on aarch64 myself...
> >
> > Note, the i => 0 "fix" in test_v3_typer_accesses() is a direct revert of patch 3,
> > "KVM: selftests: Fix typo in vgic_init test". I'll just drop that patch unless
> > someone figures out why doing the right thing causes the test to fail.
>
> CCing Eric for that one.

> > @@ -424,7 +424,7 @@ static void test_v3_typer_accesses(void)
> > KVM_DEV_ARM_VGIC_CTRL_INIT, NULL);
> >
> > for (i = 0; i < NR_VCPUS ; i++) {
> > - ret = v3_redist_reg_get(v.gic_fd, i, GICR_TYPER, &val);
> > + ret = v3_redist_reg_get(v.gic_fd, 0, GICR_TYPER, &val);
> > TEST_ASSERT(!ret && !val, "read GICR_TYPER before rdist region setting");

Figured it out, "val" should be "i * 0x100", not "0". The asserts in this test
are awful and don't print the actual "val". test_assert() shares part of the blame
for printing a stale errno, but holy moly this test makes it painful to debug
trivial issues.

2022-06-10 00:45:06

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

On Thu, Jun 09, 2022, Sean Christopherson wrote:
> On Thu, Jun 09, 2022, Anup Patel wrote:
> > On Wed, Jun 8, 2022 at 9:26 PM Anup Patel <[email protected]> wrote:
> > >
> > > On Tue, Jun 7, 2022 at 8:57 PM Paolo Bonzini <[email protected]> wrote:
> > > >
> > > > Marc, Christian, Anup, can you please give this a go?
> > >
> > > Sure, I will try this series.
> >
> > I tried to apply this series on top of kvm/next and kvm/queue but
> > I always get conflicts. It seems this series is dependent on other
> > in-flight patches.
>
> Hrm, that's odd, it's based directly on kvm/queue, commit 55371f1d0c01 ("KVM: ...).

Duh, Paolo updated kvm/queue. Where's Captain Obvious when you need him...

> > Is there a branch somewhere in a public repo ?
>
> https://github.com/sean-jc/linux/tree/x86/selftests_overhaul

I pushed a new version that's based on the current kvm/queue, commit 5e9402ac128b.
arm and x86 look good (though I've yet to test on AMD).

Thomas,
If you get a chance, could you rerun the s390 tests? The recent refactorings to
use TAP generated some fun conflicts.

Speaking of TAP, I added a patch to convert __TEST_REQUIRE to use ksft_exit_skip()
instead of KVM's custom print_skip(). The s390 tests are being converted to use
TAP output, I couldn't see any advantage of KVM's arbitrary "skipping test" over
TAP-friendly output, and converting everything is far easier than special casing s390.

2022-06-10 01:55:23

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

+s390 folks...

On Fri, Jun 10, 2022, Sean Christopherson wrote:
> On Thu, Jun 09, 2022, Sean Christopherson wrote:
> > On Thu, Jun 09, 2022, Anup Patel wrote:
> > > On Wed, Jun 8, 2022 at 9:26 PM Anup Patel <[email protected]> wrote:
> > > >
> > > > On Tue, Jun 7, 2022 at 8:57 PM Paolo Bonzini <[email protected]> wrote:
> > > > >
> > > > > Marc, Christian, Anup, can you please give this a go?
> > > >
> > > > Sure, I will try this series.
> > >
> > > I tried to apply this series on top of kvm/next and kvm/queue but
> > > I always get conflicts. It seems this series is dependent on other
> > > in-flight patches.
> >
> > Hrm, that's odd, it's based directly on kvm/queue, commit 55371f1d0c01 ("KVM: ...).
>
> Duh, Paolo updated kvm/queue. Where's Captain Obvious when you need him...
>
> > > Is there a branch somewhere in a public repo ?
> >
> > https://github.com/sean-jc/linux/tree/x86/selftests_overhaul
>
> I pushed a new version that's based on the current kvm/queue, commit 5e9402ac128b.
> arm and x86 look good (though I've yet to test on AMD).
>
> Thomas,
> If you get a chance, could you rerun the s390 tests? The recent refactorings to
> use TAP generated some fun conflicts.
>
> Speaking of TAP, I added a patch to convert __TEST_REQUIRE to use ksft_exit_skip()
> instead of KVM's custom print_skip(). The s390 tests are being converted to use
> TAP output, I couldn't see any advantage of KVM's arbitrary "skipping test" over
> TAP-friendly output, and converting everything is far easier than special casing s390.

2022-06-10 10:11:57

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

On Thu, Jun 09, 2022 at 05:26:19PM +0000, Sean Christopherson wrote:
> On Thu, Jun 09, 2022, Sean Christopherson wrote:
> > On Thu, Jun 09, 2022, Andrew Jones wrote:
> > > On Wed, Jun 08, 2022 at 11:20:06PM +0000, Sean Christopherson wrote:
> > > > diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> > > > index b3116c151d1c..17f7ef975d5c 100644
> > > > --- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> > > > +++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> > > > @@ -419,7 +419,7 @@ static void run_test(struct vcpu_config *c)
> > > >
> > > > check_supported(c);
> > > >
> > > > - vm = vm_create_barebones();
> > > > + vm = vm_create(1);
> > >
> > > Hmm, looks like something, somewhere for AArch64 needs improving to avoid
> > > strangeness like this. I'll look into it after we get this series merged.
> >
> > Huh, you're right, that is odd. Ah, duh, aarch64_vcpu_add() allocates a stack
> > for the vCPU, and that will fail if there's no memslot from which to allocate
> > guest memory.
> >
> > So, this is my goof in
> >
> > KVM: selftests: Rename vm_create() => vm_create_barebones(), drop param
> >
> > get-reg-list should first be converted to vm_create_without_vcpus(). I'll also
> > add a comment explaining that vm_create_barebones() can be used with __vm_vcpu_add(),
> > but not the "full" vm_vcpu_add() or vm_arch_vcpu_add() variants.
>
> Actually, I agree with your assessment. A better solution is to open code the
> calls to add and setup the vCPU. It's a small amount of code duplication, but I
> actually like the end result because it better documents the test's dependencies.
>
> Assuming it actually works, i.e. the stack setup is truly unnecessary, I'll add a
> patch like so before the barebones change.
>
> diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> index ecfb773ec41e..7bba365b1522 100644
> --- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> +++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> @@ -418,7 +418,8 @@ static void run_test(struct vcpu_config *c)
>
> vm = vm_create(DEFAULT_GUEST_PHY_PAGES);
> prepare_vcpu_init(c, &init);
> - aarch64_vcpu_add_default(vm, 0, &init, NULL);
> + vm_vcpu_add(vm, vcpuid);
> + aarch64_vcpu_setup(vm, 0, &init);
> finalize_vcpu(vm, 0, c);
>
> reg_list = vcpu_get_reg_list(vm, 0);
>

LGTM, Thanks

2022-06-10 12:03:48

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

On Fri, Jun 10, 2022 at 6:04 AM Sean Christopherson <[email protected]> wrote:
>
> On Thu, Jun 09, 2022, Sean Christopherson wrote:
> > On Thu, Jun 09, 2022, Anup Patel wrote:
> > > On Wed, Jun 8, 2022 at 9:26 PM Anup Patel <[email protected]> wrote:
> > > >
> > > > On Tue, Jun 7, 2022 at 8:57 PM Paolo Bonzini <[email protected]> wrote:
> > > > >
> > > > > Marc, Christian, Anup, can you please give this a go?
> > > >
> > > > Sure, I will try this series.
> > >
> > > I tried to apply this series on top of kvm/next and kvm/queue but
> > > I always get conflicts. It seems this series is dependent on other
> > > in-flight patches.
> >
> > Hrm, that's odd, it's based directly on kvm/queue, commit 55371f1d0c01 ("KVM: ...).
>
> Duh, Paolo updated kvm/queue. Where's Captain Obvious when you need him...
>
> > > Is there a branch somewhere in a public repo ?
> >
> > https://github.com/sean-jc/linux/tree/x86/selftests_overhaul
>
> I pushed a new version that's based on the current kvm/queue, commit 5e9402ac128b.
> arm and x86 look good (though I've yet to test on AMD).
>

I have tested this for KVM RISC-V and it works fine.

Tested-by: Anup Patel <[email protected]>

Regards,
Anup

2022-06-10 19:18:28

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

On Fri, Jun 03, 2022 at 12:41:07AM +0000, Sean Christopherson wrote:
> Overhaul KVM's selftest APIs to get selftests to a state where adding new
> features and writing tests is less painful/disgusting.
>
> Patches 1 fixes a goof in kvm/queue and should be squashed.
>
> I would really, really, really like to get this queued up sooner than
> later, or maybe just thrown into a separate selftests-specific branch that
> folks can develop against. Rebasing is tedious, frustrating, and time
> consuming. And spoiler alert, there's another 42 x86-centric patches
> inbound that builds on this series to clean up CPUID related crud...
>
> The primary theme is to stop treating tests like second class citizens.
> Stop hiding vcpu, kvm_vm, etc... There's no sensitive data/constructs, and
> the encapsulation has led to really, really bad and difficult to maintain
> code. E.g. having to pass around the VM just to call a vCPU ioctl(),
> arbitrary non-zero vCPU IDs, tests having to care about the vCPU ID in the
> first place, etc...
>
> The other theme in the rework is to deduplicate code and try to set us
> up for success in the future. E.g. provide macros/helpers instead of
> spamming CTRL-C => CTRL-V (see the -1k LoC), structure the VM creation
> APIs to build on one another, etc...
>
> The absurd patch count (as opposed to just ridiculous) is due to converting
> each test away from using hardcoded vCPU IDs in a separate patch. The vast
> majority of those patches probably aren't worth reviewing in depth, the
> changes are mostly mechanical in nature.
>
> However, _running_ non-x86 tests (or tests that have unique non-x86
> behavior) would be extremely valuable. All patches have been compile tested
> on x86, arm, risc-v, and s390, but I've only run the tests on x86. Based on
> my track record for the x86+common tests, I will be very, very surprised if
> I didn't break any of the non-x86 tests, e.g. pthread_create()'s 'void *'
> param tripped me up multiple times.
>
> I have not run x86's amx_test due to lack of hardware. I also haven't run
> sev_migration; something is wonky in either the upstream support for INIT_EX
> or in our test machines and I can't get SEV to initialize.
>
> v2:
> - Drop the forced -Werror patch. [Vitaly]
> - Add TEST_REQUIRE to reduce KSFT_SKIP boilerplate.
> - Rebase to kvm/queue, commit 55371f1d0c01.
> - Clean up even more bad copy+paste code (x86 was hiding a lot of crud).
> - Assert that the input to an ioctl() is (likely) the correct struct.
>
> v1: https://lore.kernel.org/all/[email protected]
>

Hi Sean,

I've completed a thorough skim / review and it looks great to me. Besides
the final patch where I'm wondering about the loss of the type checking
on our ioctl wrappers, I don't think there are any patches where I
wouldn't be happy to add an r-b. So, for the series, except the last patch

Reviewed-by: Andrew Jones <[email protected]>

Thanks,
drew

2022-06-11 16:16:09

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

On 6/10/22 02:34, Sean Christopherson wrote:
> I pushed a new version that's based on the current kvm/queue, commit 5e9402ac128b.
> arm and x86 look good (though I've yet to test on AMD).
>
> Thomas,
> If you get a chance, could you rerun the s390 tests? The recent refactorings to
> use TAP generated some fun conflicts.

I did so, rebased over David's nested dirty_log_perf_test patches and
pushed to kvm/queue.

Paolo

2022-06-13 08:15:30

by Thomas Huth

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

On 10/06/2022 02.57, Sean Christopherson wrote:
> +s390 folks...
>
> On Fri, Jun 10, 2022, Sean Christopherson wrote:
>> On Thu, Jun 09, 2022, Sean Christopherson wrote:
>>> On Thu, Jun 09, 2022, Anup Patel wrote:
>>>> On Wed, Jun 8, 2022 at 9:26 PM Anup Patel <[email protected]> wrote:
>>>>>
>>>>> On Tue, Jun 7, 2022 at 8:57 PM Paolo Bonzini <[email protected]> wrote:
>>>>>>
>>>>>> Marc, Christian, Anup, can you please give this a go?
>>>>>
>>>>> Sure, I will try this series.
>>>>
>>>> I tried to apply this series on top of kvm/next and kvm/queue but
>>>> I always get conflicts. It seems this series is dependent on other
>>>> in-flight patches.
>>>
>>> Hrm, that's odd, it's based directly on kvm/queue, commit 55371f1d0c01 ("KVM: ...).
>>
>> Duh, Paolo updated kvm/queue. Where's Captain Obvious when you need him...
>>
>>>> Is there a branch somewhere in a public repo ?
>>>
>>> https://github.com/sean-jc/linux/tree/x86/selftests_overhaul
>>
>> I pushed a new version that's based on the current kvm/queue, commit 5e9402ac128b.
>> arm and x86 look good (though I've yet to test on AMD).
>>
>> Thomas,
>> If you get a chance, could you rerun the s390 tests? The recent refactorings to
>> use TAP generated some fun conflicts.

Still works fine!
Tested-by: Thomas Huth <[email protected]>

>> Speaking of TAP, I added a patch to convert __TEST_REQUIRE to use ksft_exit_skip()
>> instead of KVM's custom print_skip(). The s390 tests are being converted to use
>> TAP output, I couldn't see any advantage of KVM's arbitrary "skipping test" over
>> TAP-friendly output, and converting everything is far easier than special casing s390.

Sounds like a good idea to me. I already considered starting to convert some
x86 tests, too
(https://lore.kernel.org/linux-kselftest/[email protected]
), but didn't get much feedback there yet, but anyway, we'll be better
prepared with your change for that now.

Thomas

2022-06-13 19:11:25

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH v2 000/144] KVM: selftests: Overhaul APIs, purge VCPU_ID

On Sat, Jun 11, 2022, Paolo Bonzini wrote:
> On 6/10/22 02:34, Sean Christopherson wrote:
> > I pushed a new version that's based on the current kvm/queue, commit 5e9402ac128b.
> > arm and x86 look good (though I've yet to test on AMD).
> >
> > Thomas,
> > If you get a chance, could you rerun the s390 tests? The recent refactorings to
> > use TAP generated some fun conflicts.
>
> I did so, rebased over David's nested dirty_log_perf_test patches and pushed
> to kvm/queue.

Wahoo! Thanks!

And a huge thanks to Drew for the reviews!