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@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?
> 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
>>
>
