[PATCH v1 0/3] Add a test case for KVM_X86_DISABLE_EXIT

[PATCH v1 0/3] Add a test case for KVM_X86_DISABLE_EXIT

[Manali Shukla]

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.

By default, vms are created with in-kernel APIC support in KVM selftests.
VM needs to be created without in-kernel APIC support for this test, so
that HLT will exit to userspace. To do so, __vm_create() is modified to not
call KVM_CREATE_IRQCHIP ioctl while creating vm.

Add a test case to test KVM_X86_DISABLE_EXITS_HLT functionality.

Patch 1, 2 -> Preparatory patches to add the KVM_X86_DISABLE_EXITS_HLT test
              case 
Patch 3 -> Adds a test case for KVM_X86_DISABLE_EXITS_HLT

Testing done:
Tested KVM_X86_DISABLE_EXITS_HLT test case on AMD and Intel machines.

Manali Shukla (3):
  KVM: selftests: Add safe_halt() and cli() helpers to common code
  KVM: selftests: Change __vm_create() to create a vm without in-kernel
    APIC
  KVM: selftests: Add a test case for KVM_X86_DISABLE_EXITS_HLT

 tools/testing/selftests/kvm/Makefile          |   1 +
 tools/testing/selftests/kvm/dirty_log_test.c  |   2 +-
 .../selftests/kvm/include/kvm_util_base.h     |   4 +-
 .../selftests/kvm/include/x86_64/processor.h  |  17 +++
 tools/testing/selftests/kvm/lib/kvm_util.c    |  11 +-
 .../kvm/x86_64/halt_disable_exit_test.c       | 113 ++++++++++++++++++
 .../kvm/x86_64/ucna_injection_test.c          |   2 +-
 7 files changed, 143 insertions(+), 7 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/x86_64/halt_disable_exit_test.c


base-commit: e9da6f08edb0bd4c621165496778d77a222e1174
-- 
2.34.1

[PATCH v1 1/3] KVM: selftests: Add safe_halt() and cli() helpers to common code

[Manali Shukla]

Add safe_halt() and cli() helpers to processor.h to make them broadly
available in KVM selftests.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Manali Shukla <manali.shukla@amd.com>
---
 .../selftests/kvm/include/x86_64/processor.h    | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index 20c9e3b33b07..6de37f6b8ddc 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -1217,6 +1217,23 @@ static inline void kvm_hypercall_map_gpa_range(uint64_t gpa, uint64_t size,
 	GUEST_ASSERT(!ret);
 }
 
+/*
+ * Execute HLT in an STI interrupt shadow to ensure that a pending IRQ that's
+ * intended to be a wake event arrives *after* HLT is executed.  Modern CPUs,
+ * except for a few oddballs that KVM is unlikely to run on, block IRQs for one
+ * instruction after STI, *if* RFLAGS.IF=0 before STI.  Note, Intel CPUs may
+ * block other events beyond regular IRQs, e.g. may block NMIs and SMIs too.
+ */
+static inline void safe_halt(void)
+{
+	asm volatile("sti; hlt");
+}
+
+static inline void cli(void)
+{
+	asm volatile ("cli");
+}
+
 void __vm_xsave_require_permission(uint64_t xfeature, const char *name);
 
 #define vm_xsave_require_permission(xfeature)	\
-- 
2.34.1

[PATCH v1 2/3] KVM: selftests: Change __vm_create() to create a vm without in-kernel APIC

[Manali Shukla]

Change __vm_create() to incorporate creation of a vm without in-kernel
APIC.  Currently, all the vms are created with in-kernel APIC support in
KVM selftests because KVM_CREATE_IRQCHIP ioctl is called by default from
kvm_arch_vm_post_create().

Add a flag in __vm_create() for a userspace to decide whether to start a vm
with in-kernel APIC or without in-kernel APIC.

It is a preparatory patch to create a vm without in-kernel APIC support for
the KVM_X86_DISABLE_EXITS_HLT test.

Signed-off-by: Manali Shukla <manali.shukla@amd.com>
---
 tools/testing/selftests/kvm/dirty_log_test.c          |  2 +-
 tools/testing/selftests/kvm/include/kvm_util_base.h   |  4 ++--
 tools/testing/selftests/kvm/lib/kvm_util.c            | 11 ++++++++---
 .../selftests/kvm/x86_64/ucna_injection_test.c        |  2 +-
 4 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
index 6cbecf499767..667a83d67bfe 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -699,7 +699,7 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, struct kvm_vcpu **vcpu,
 
 	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
 
-	vm = __vm_create(VM_SHAPE(mode), 1, extra_mem_pages);
+	vm = __vm_create(VM_SHAPE(mode), 1, extra_mem_pages, true);
 
 	log_mode_create_vm_done(vm);
 	*vcpu = vm_vcpu_add(vm, 0, guest_code);
diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
index 4a40b332115d..00e37c376cf3 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_base.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
@@ -879,7 +879,7 @@ static inline vm_paddr_t vm_phy_pages_alloc(struct kvm_vm *vm, size_t num,
  */
 struct kvm_vm *____vm_create(struct vm_shape shape);
 struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus,
-			   uint64_t nr_extra_pages);
+			   uint64_t nr_extra_pages, bool is_in_kernel_apic);
 
 static inline struct kvm_vm *vm_create_barebones(void)
 {
@@ -900,7 +900,7 @@ static inline struct kvm_vm *vm_create_barebones_protected_vm(void)
 
 static inline struct kvm_vm *vm_create(uint32_t nr_runnable_vcpus)
 {
-	return __vm_create(VM_SHAPE_DEFAULT, nr_runnable_vcpus, 0);
+	return __vm_create(VM_SHAPE_DEFAULT, nr_runnable_vcpus, 0, true);
 }
 
 struct kvm_vm *__vm_create_with_vcpus(struct vm_shape shape, uint32_t nr_vcpus,
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index adc51b0712ca..9c2a9e216d80 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -354,7 +354,7 @@ static uint64_t vm_nr_pages_required(enum vm_guest_mode mode,
 }
 
 struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus,
-			   uint64_t nr_extra_pages)
+			   uint64_t nr_extra_pages, bool is_in_kernel_apic)
 {
 	uint64_t nr_pages = vm_nr_pages_required(shape.mode, nr_runnable_vcpus,
 						 nr_extra_pages);
@@ -382,7 +382,12 @@ struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus,
 	slot0 = memslot2region(vm, 0);
 	ucall_init(vm, slot0->region.guest_phys_addr + slot0->region.memory_size);
 
-	kvm_arch_vm_post_create(vm);
+	if (is_in_kernel_apic) {
+		kvm_arch_vm_post_create(vm);
+	} else {
+		sync_global_to_guest(vm, host_cpu_is_intel);
+		sync_global_to_guest(vm, host_cpu_is_amd);
+	}
 
 	return vm;
 }
@@ -415,7 +420,7 @@ struct kvm_vm *__vm_create_with_vcpus(struct vm_shape shape, uint32_t nr_vcpus,
 
 	TEST_ASSERT(!nr_vcpus || vcpus, "Must provide vCPU array");
 
-	vm = __vm_create(shape, nr_vcpus, extra_mem_pages);
+	vm = __vm_create(shape, nr_vcpus, extra_mem_pages, true);
 
 	for (i = 0; i < nr_vcpus; ++i)
 		vcpus[i] = vm_vcpu_add(vm, i, guest_code);
diff --git a/tools/testing/selftests/kvm/x86_64/ucna_injection_test.c b/tools/testing/selftests/kvm/x86_64/ucna_injection_test.c
index 0ed32ec903d0..095188562709 100644
--- a/tools/testing/selftests/kvm/x86_64/ucna_injection_test.c
+++ b/tools/testing/selftests/kvm/x86_64/ucna_injection_test.c
@@ -271,7 +271,7 @@ int main(int argc, char *argv[])
 
 	kvm_check_cap(KVM_CAP_MCE);
 
-	vm = __vm_create(VM_SHAPE_DEFAULT, 3, 0);
+	vm = __vm_create(VM_SHAPE_DEFAULT, 3, 0, true);
 
 	kvm_ioctl(vm->kvm_fd, KVM_X86_GET_MCE_CAP_SUPPORTED,
 		  &supported_mcg_caps);
-- 
2.34.1

[PATCH v1 3/3] KVM: selftests: Add a test case for KVM_X86_DISABLE_EXITS_HLT

[Manali Shukla]

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

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;
+}
-- 
2.34.1

Re: [PATCH v1 2/3] KVM: selftests: Change __vm_create() to create a vm without in-kernel APIC

[Andrew Jones]

On Wed, Mar 27, 2024 at 05:42:54AM +0000, Manali Shukla wrote:
...
> diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
> index 4a40b332115d..00e37c376cf3 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util_base.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
> @@ -879,7 +879,7 @@ static inline vm_paddr_t vm_phy_pages_alloc(struct kvm_vm *vm, size_t num,
>   */
>  struct kvm_vm *____vm_create(struct vm_shape shape);
>  struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus,
> -			   uint64_t nr_extra_pages);
> +			   uint64_t nr_extra_pages, bool is_in_kernel_apic);

Adding boolean flag parameters to functions, which will 99% of the time be
called with the same value set for them, is not nice.

...
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index adc51b0712ca..9c2a9e216d80 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -354,7 +354,7 @@ static uint64_t vm_nr_pages_required(enum vm_guest_mode mode,
>  }
>  
>  struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus,
> -			   uint64_t nr_extra_pages)
> +			   uint64_t nr_extra_pages, bool is_in_kernel_apic)
>  {
>  	uint64_t nr_pages = vm_nr_pages_required(shape.mode, nr_runnable_vcpus,
>  						 nr_extra_pages);
> @@ -382,7 +382,12 @@ struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus,
>  	slot0 = memslot2region(vm, 0);
>  	ucall_init(vm, slot0->region.guest_phys_addr + slot0->region.memory_size);
>  
> -	kvm_arch_vm_post_create(vm);
> +	if (is_in_kernel_apic) {
> +		kvm_arch_vm_post_create(vm);
> +	} else {
> +		sync_global_to_guest(vm, host_cpu_is_intel);
> +		sync_global_to_guest(vm, host_cpu_is_amd);
> +	}

__vm_create() is shared with other architectures, and duplicating part of
kvm_arch_vm_post_create() here is not a good approach, even if the
framework was only for x86.

I suggest:

 1. Extend vm_shape to include a flags field and create a flag called
    NO_IRQCHIP

 2. Add a flags member to kvm_vm and set it to the value of vm_shape.flags
    in ____vm_create()

 3. Check !(vm.flags & NO_IRQCHIP) in x86's kvm_arch_vm_post_create()
    before calling vm_create_irqchip()
    
Then, in your tests, you'll create your vm shape with the NO_IRQCHIP flag
set.

Thanks,
drew

Re: [PATCH v1 2/3] KVM: selftests: Change __vm_create() to create a vm without in-kernel APIC

[Manali Shukla]

Hi Andrew,
Thank you for reviewing my patches.

On 3/27/2024 4:57 PM, Andrew Jones wrote:
> On Wed, Mar 27, 2024 at 05:42:54AM +0000, Manali Shukla wrote:
> ...
>> diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
>> index 4a40b332115d..00e37c376cf3 100644
>> --- a/tools/testing/selftests/kvm/include/kvm_util_base.h
>> +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
>> @@ -879,7 +879,7 @@ static inline vm_paddr_t vm_phy_pages_alloc(struct kvm_vm *vm, size_t num,
>>   */
>>  struct kvm_vm *____vm_create(struct vm_shape shape);
>>  struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus,
>> -			   uint64_t nr_extra_pages);
>> +			   uint64_t nr_extra_pages, bool is_in_kernel_apic);
> 
> Adding boolean flag parameters to functions, which will 99% of the time be
> called with the same value set for them, is not nice.

Agreed.

> 
> ...
>> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
>> index adc51b0712ca..9c2a9e216d80 100644
>> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
>> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
>> @@ -354,7 +354,7 @@ static uint64_t vm_nr_pages_required(enum vm_guest_mode mode,
>>  }
>>  
>>  struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus,
>> -			   uint64_t nr_extra_pages)
>> +			   uint64_t nr_extra_pages, bool is_in_kernel_apic)
>>  {
>>  	uint64_t nr_pages = vm_nr_pages_required(shape.mode, nr_runnable_vcpus,
>>  						 nr_extra_pages);
>> @@ -382,7 +382,12 @@ struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus,
>>  	slot0 = memslot2region(vm, 0);
>>  	ucall_init(vm, slot0->region.guest_phys_addr + slot0->region.memory_size);
>>  
>> -	kvm_arch_vm_post_create(vm);
>> +	if (is_in_kernel_apic) {
>> +		kvm_arch_vm_post_create(vm);
>> +	} else {
>> +		sync_global_to_guest(vm, host_cpu_is_intel);
>> +		sync_global_to_guest(vm, host_cpu_is_amd);
>> +	}
> 
> __vm_create() is shared with other architectures, and duplicating part of
> kvm_arch_vm_post_create() here is not a good approach, even if the
> framework was only for x86.
> 
> I suggest:
> 
>  1. Extend vm_shape to include a flags field and create a flag called
>     NO_IRQCHIP
> 
>  2. Add a flags member to kvm_vm and set it to the value of vm_shape.flags
>     in ____vm_create()
> 
>  3. Check !(vm.flags & NO_IRQCHIP) in x86's kvm_arch_vm_post_create()
>     before calling vm_create_irqchip()
>     
> Then, in your tests, you'll create your vm shape with the NO_IRQCHIP flag
> set.

Sure. I will incorporate your suggestions in the next version.

> 
> Thanks,
> drew

- Manali

Re: [PATCH v1 3/3] KVM: selftests: Add a test case for KVM_X86_DISABLE_EXITS_HLT

[Muhammad Usama Anjum]

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;
> +}

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH v1 3/3] KVM: selftests: Add a test case for KVM_X86_DISABLE_EXITS_HLT

[Manali Shukla]

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

Re: [PATCH v1 3/3] KVM: selftests: Add a test case for KVM_X86_DISABLE_EXITS_HLT

[Manali Shukla]

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

Re: [PATCH v1 3/3] KVM: selftests: Add a test case for KVM_X86_DISABLE_EXITS_HLT

[Muhammad Usama Anjum]

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
> 

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH v1 3/3] KVM: selftests: Add a test case for KVM_X86_DISABLE_EXITS_HLT

[Manali Shukla]

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