| Message ID | 20240327054255.24626-4-manali.shukla@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add a test case for KVM_X86_DISABLE_EXIT | expand |
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@google.com> > Signed-off-by: Manali Shukla <manali.shukla@amd.com> 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. > --- > tools/testing/selftests/kvm/Makefile | 1 + > .../kvm/x86_64/halt_disable_exit_test.c | 113 ++++++++++++++++++ Add generated object to .gitignore file. > 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; > +}
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@google.com> >> Signed-off-by: Manali Shukla <manali.shukla@amd.com> > 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
On 4/1/2024 10:58 AM, Manali Shukla wrote: > 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@google.com> >>> Signed-off-by: Manali Shukla <manali.shukla@amd.com> >> 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. I think adding of generated object to .gitignore is not needed because /tools/testing/selftests/kvm/.gitignore uses pattern matching to exclude everything except .c, .h, .S, and .sh files from Git, which was committed via below commit. commit 43e96957e8b87bad8e4ba666750ff0cda9e03ffb KVM: selftests: Use pattern matching in .gitignore >>>>> 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
On 4/1/24 10:28 AM, Manali Shukla wrote: > 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@google.com> >>> Signed-off-by: Manali Shukla <manali.shukla@amd.com> >> 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? TAP interface is just a way to print logs which are machine readable for CIs. So log messages, test pass or fail should be marked by tools/testing/selftests/kselftest.h or tools/testing/selftests/kselftest_harness.h. It depends on the design of your test that which would be suitable. It seems that most tests in KVM suite aren't TAP compliant. In this case, I'm okay with non-TAP compliant test as the whole suite is far from compliance. > >> >>> --- >>> 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 >
On 4/1/2024 3:11 PM, Muhammad Usama Anjum wrote: > On 4/1/24 10:28 AM, Manali Shukla wrote: >> 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@google.com> >>>> Signed-off-by: Manali Shukla <manali.shukla@amd.com> >>> 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? > TAP interface is just a way to print logs which are machine readable for > CIs. So log messages, test pass or fail should be marked by > tools/testing/selftests/kselftest.h or > tools/testing/selftests/kselftest_harness.h. It depends on the design of > your test that which would be suitable. > > It seems that most tests in KVM suite aren't TAP compliant. In this case, > I'm okay with non-TAP compliant test as the whole suite is far from compliance. > Sure. I can keep it non-TAP compliant for now. >> >>> >>>> --- >>>> 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 >> >
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; +}
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@google.com> Signed-off-by: Manali Shukla <manali.shukla@amd.com> --- tools/testing/selftests/kvm/Makefile | 1 + .../kvm/x86_64/halt_disable_exit_test.c | 113 ++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 tools/testing/selftests/kvm/x86_64/halt_disable_exit_test.c