Hi Muhammad Usama Anjum, Thank you for reviewing my patch. On 3/30/2024 1:43 AM, Muhammad Usama Anjum wrote: > On 3/27/24 10:42 AM, Manali Shukla wrote: >> By default, HLT instruction executed by guest is intercepted by hypervisor. >> However, KVM_CAP_X86_DISABLE_EXITS capability can be used to not intercept >> HLT by setting KVM_X86_DISABLE_EXITS_HLT. >> >> Add a test case to test KVM_X86_DISABLE_EXITS_HLT functionality. >> >> Suggested-by: Sean Christopherson <seanjc@xxxxxxxxxx> >> Signed-off-by: Manali Shukla <manali.shukla@xxxxxxx> > Thank you for the new test patch. We have been trying to ensure TAP > conformance for tests which cannot be achieved if new tests aren't using > TAP already. Please make your test TAP compliant. As per my understanding about TAP interface, kvm_test_harness.h file includes a MACRO, which is used to create VM with one vcpu using vm_create_with_one_vcpu(), but halt_disable_exit_test creates a customized VM with KVM_CAP_X86_DISABLE_EXITS capability set and different vm_shape parameters to start a VM without in-kernel APIC support. AFAIU, I won't be able to use KVM_ONE_VCPU_TEST_SUITE MACRO as is. How do you suggest to proceed with this issue? > >> --- >> tools/testing/selftests/kvm/Makefile | 1 + >> .../kvm/x86_64/halt_disable_exit_test.c | 113 ++++++++++++++++++ > Add generated object to .gitignore file. Sure. I will do it. > >> 2 files changed, 114 insertions(+) >> create mode 100644 tools/testing/selftests/kvm/x86_64/halt_disable_exit_test.c >> >> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile >> index c75251d5c97c..9f72abb95d2e 100644 >> --- a/tools/testing/selftests/kvm/Makefile >> +++ b/tools/testing/selftests/kvm/Makefile >> @@ -89,6 +89,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/set_sregs_test >> TEST_GEN_PROGS_x86_64 += x86_64/smaller_maxphyaddr_emulation_test >> TEST_GEN_PROGS_x86_64 += x86_64/smm_test >> TEST_GEN_PROGS_x86_64 += x86_64/state_test >> +TEST_GEN_PROGS_x86_64 += x86_64/halt_disable_exit_test >> TEST_GEN_PROGS_x86_64 += x86_64/vmx_preemption_timer_test >> TEST_GEN_PROGS_x86_64 += x86_64/svm_vmcall_test >> TEST_GEN_PROGS_x86_64 += x86_64/svm_int_ctl_test >> diff --git a/tools/testing/selftests/kvm/x86_64/halt_disable_exit_test.c b/tools/testing/selftests/kvm/x86_64/halt_disable_exit_test.c >> new file mode 100644 >> index 000000000000..b7279dd0eaff >> --- /dev/null >> +++ b/tools/testing/selftests/kvm/x86_64/halt_disable_exit_test.c >> @@ -0,0 +1,113 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * KVM disable halt exit test >> + * >> + * Copyright (C) 2024 Advanced Micro Devices, Inc. >> + */ >> +#include <pthread.h> >> +#include <signal.h> >> +#include "kvm_util.h" >> +#include "svm_util.h" >> +#include "processor.h" >> +#include "test_util.h" >> + >> +pthread_t task_thread, vcpu_thread; >> +#define SIG_IPI SIGUSR1 >> + >> +static void guest_code(uint8_t is_hlt_exec) >> +{ >> + while (!READ_ONCE(is_hlt_exec)) >> + ; >> + >> + safe_halt(); >> + GUEST_DONE(); >> +} >> + >> +static void *task_worker(void *arg) >> +{ >> + uint8_t *is_hlt_exec = (uint8_t *)arg; >> + >> + usleep(1000); >> + WRITE_ONCE(*is_hlt_exec, 1); >> + pthread_kill(vcpu_thread, SIG_IPI); >> + return 0; >> +} >> + >> +static void *vcpu_worker(void *arg) >> +{ >> + int ret; >> + int sig = -1; >> + uint8_t *is_hlt_exec = (uint8_t *)arg; >> + struct kvm_vm *vm; >> + struct kvm_run *run; >> + struct kvm_vcpu *vcpu; >> + struct kvm_signal_mask *sigmask = alloca(offsetof(struct kvm_signal_mask, sigset) >> + + sizeof(sigset_t)); >> + sigset_t *sigset = (sigset_t *) &sigmask->sigset; >> + >> + /* Create a VM without in kernel APIC support */ >> + vm = __vm_create(VM_SHAPE_DEFAULT, 1, 0, false); >> + vm_enable_cap(vm, KVM_CAP_X86_DISABLE_EXITS, KVM_X86_DISABLE_EXITS_HLT); >> + vcpu = vm_vcpu_add(vm, 0, guest_code); >> + vcpu_args_set(vcpu, 1, *is_hlt_exec); >> + >> + /* >> + * SIG_IPI is unblocked atomically while in KVM_RUN. It causes the >> + * ioctl to return with -EINTR, but it is still pending and we need >> + * to accept it with the sigwait. >> + */ >> + sigmask->len = 8; >> + pthread_sigmask(0, NULL, sigset); >> + sigdelset(sigset, SIG_IPI); >> + vcpu_ioctl(vcpu, KVM_SET_SIGNAL_MASK, sigmask); >> + sigemptyset(sigset); >> + sigaddset(sigset, SIG_IPI); >> + run = vcpu->run; >> + >> +again: >> + ret = __vcpu_run(vcpu); >> + TEST_ASSERT_EQ(errno, EINTR); >> + >> + if (ret == -1 && errno == EINTR) { >> + sigwait(sigset, &sig); >> + assert(sig == SIG_IPI); >> + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_INTR); >> + goto again; >> + } >> + >> + if (run->exit_reason == KVM_EXIT_HLT) >> + TEST_FAIL("Expected KVM_EXIT_INTR, got KVM_EXIT_HLT"); >> + >> + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); >> + kvm_vm_free(vm); >> + return 0; >> +} >> + >> +int main(int argc, char *argv[]) >> +{ >> + int ret; >> + void *retval; >> + uint8_t is_halt_exec; >> + sigset_t sigset; >> + >> + TEST_REQUIRE(kvm_has_cap(KVM_CAP_X86_DISABLE_EXITS)); >> + >> + /* Ensure that vCPU threads start with SIG_IPI blocked. */ >> + sigemptyset(&sigset); >> + sigaddset(&sigset, SIG_IPI); >> + pthread_sigmask(SIG_BLOCK, &sigset, NULL); >> + >> + ret = pthread_create(&vcpu_thread, NULL, vcpu_worker, &is_halt_exec); >> + TEST_ASSERT(ret == 0, "pthread_create vcpu thread failed errno=%d", errno); >> + >> + ret = pthread_create(&task_thread, NULL, task_worker, &is_halt_exec); >> + TEST_ASSERT(ret == 0, "pthread_create task thread failed errno=%d", errno); >> + >> + pthread_join(vcpu_thread, &retval); >> + TEST_ASSERT(ret == 0, "pthread_join on vcpu thread failed with errno=%d", ret); >> + >> + pthread_join(task_thread, &retval); >> + TEST_ASSERT(ret == 0, "pthread_join on task thread failed with errno=%d", ret); >> + >> + return 0; >> +} > - Manali