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

[PATCH v2 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.

v1 -> v2
- Extended @shape to allow creation of VM without in-kernel APIC support
  (Andrew Jones)
- Changed the test case based on Andrew's comments.
- Few more changes to the test case to pass the address of the flag on
  which guest waits to execute HLT.

Manali Shukla (3):
  KVM: selftests: Add safe_halt() and cli() helpers to common code
  KVM: selftests: Extend @shape to allow creation of VM without
    in-kernel APIC
  KVM: selftests: Add a test case for KVM_X86_DISABLE_EXITS_HLT

 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/include/kvm_util_base.h     |  17 ++-
 .../selftests/kvm/include/x86_64/processor.h  |  17 +++
 tools/testing/selftests/kvm/lib/kvm_util.c    |   1 +
 .../selftests/kvm/lib/x86_64/processor.c      |   4 +-
 .../kvm/x86_64/halt_disable_exit_test.c       | 120 ++++++++++++++++++
 6 files changed, 158 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/x86_64/halt_disable_exit_test.c


base-commit: e9da6f08edb0bd4c621165496778d77a222e1174
-- 
2.34.1

[PATCH v2 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 v2 2/3] KVM: selftests: Extend @shape to allow creation of VM without in-kernel APIC

[Manali Shukla]

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().

Carve out space in the @shape passed to the various VM creation helpers to
allow using the shape to control creation of a VM without in-kernel APIC
support or with in-kernel APIC support.

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

Suggested-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Manali Shukla <manali.shukla@amd.com>
---
 .../selftests/kvm/include/kvm_util_base.h       | 17 ++++++++++++++++-
 tools/testing/selftests/kvm/lib/kvm_util.c      |  1 +
 .../selftests/kvm/lib/x86_64/processor.c        |  4 +++-
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
index 4a40b332115d..c94cfbdf0150 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_base.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
@@ -130,6 +130,7 @@ struct kvm_vm {
 	 * memslot.
 	 */
 	uint32_t memslots[NR_MEM_REGIONS];
+	uint8_t flags;
 };
 
 struct vcpu_reg_sublist {
@@ -197,11 +198,14 @@ enum vm_guest_mode {
 	NUM_VM_MODES,
 };
 
+#define NO_IRQCHIP  0x01
+
 struct vm_shape {
 	uint32_t type;
 	uint8_t  mode;
 	uint8_t  subtype;
-	uint16_t padding;
+	uint8_t  flags;
+	uint8_t padding;
 };
 
 kvm_static_assert(sizeof(struct vm_shape) == sizeof(uint64_t));
@@ -218,6 +222,17 @@ kvm_static_assert(sizeof(struct vm_shape) == sizeof(uint64_t));
 	shape;					\
 })
 
+#define VM_SHAPE_FLAGS(__FLAGS)			\
+({						\
+	struct vm_shape shape = {		\
+		.mode = VM_MODE_DEFAULT,	\
+		.type = VM_TYPE_DEFAULT,	\
+		.flags = __FLAGS		\
+	};					\
+						\
+	shape;					\
+})
+
 #if defined(__aarch64__)
 
 extern enum vm_guest_mode vm_mode_default;
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index adc51b0712ca..86546f603959 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -226,6 +226,7 @@ struct kvm_vm *____vm_create(struct vm_shape shape)
 	vm->mode = shape.mode;
 	vm->type = shape.type;
 	vm->subtype = shape.subtype;
+	vm->flags = shape.flags;
 
 	vm->pa_bits = vm_guest_mode_params[vm->mode].pa_bits;
 	vm->va_bits = vm_guest_mode_params[vm->mode].va_bits;
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index 49288fe10cd3..e5ca92feae67 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -574,7 +574,9 @@ static void vcpu_setup(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
 
 void kvm_arch_vm_post_create(struct kvm_vm *vm)
 {
-	vm_create_irqchip(vm);
+	if (!(vm->flags & NO_IRQCHIP))
+		vm_create_irqchip(vm);
+
 	sync_global_to_guest(vm, host_cpu_is_intel);
 	sync_global_to_guest(vm, host_cpu_is_amd);
 
-- 
2.34.1

[PATCH v2 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       | 119 ++++++++++++++++++
 2 files changed, 120 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..4cc6a09906a2
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86_64/halt_disable_exit_test.c
@@ -0,0 +1,119 @@
+// 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 "processor.h"
+#include "test_util.h"
+
+#define SIG_IPI SIGUSR1
+static pthread_t task_thread, vcpu_thread;
+
+static void guest_code(uint64_t *is_hlt_exec)
+{
+	while (!READ_ONCE(*is_hlt_exec))
+		;
+
+	safe_halt();
+	GUEST_DONE();
+}
+
+static void *task_worker(void *arg)
+{
+	uint64_t *is_hlt_exec = (uint64_t *)arg;
+
+	usleep(100000);
+	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;
+	struct kvm_vcpu *vcpu = (struct kvm_vcpu *) arg;
+	struct kvm_run *run;
+
+	struct kvm_signal_mask *sigmask = alloca(offsetof(struct kvm_signal_mask, sigset)
+						 + sizeof(sigset_t));
+	sigset_t *sigset = (sigset_t *) &sigmask->sigset;
+
+	/*
+	 * 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);
+	return 0;
+}
+
+int main(int argc, char *argv[])
+{
+	vm_vaddr_t hlt_vm_addr;
+
+	struct kvm_vm *vm;
+	struct kvm_vcpu *vcpu;
+	uint64_t *host_hlt_addr;
+	void *retval;
+	sigset_t sigset;
+	int ret;
+
+	TEST_REQUIRE(kvm_has_cap(KVM_CAP_X86_DISABLE_EXITS));
+
+	/* Create a VM without in kernel APIC support */
+	vm = __vm_create(VM_SHAPE_FLAGS(NO_IRQCHIP), 1, 0);
+	vm_enable_cap(vm, KVM_CAP_X86_DISABLE_EXITS, KVM_X86_DISABLE_EXITS_HLT);
+	vcpu = vm_vcpu_add(vm, 0, guest_code);
+
+
+	hlt_vm_addr = vm_vaddr_alloc_page(vm);
+	host_hlt_addr = (uint64_t *)addr_gva2hva(vm, hlt_vm_addr);
+	vcpu_args_set(vcpu, 1, hlt_vm_addr);
+
+	/* 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, vcpu);
+	TEST_ASSERT(ret == 0, "pthread_create vcpu thread failed errno=%d", errno);
+
+	ret = pthread_create(&task_thread, NULL, task_worker, host_hlt_addr);
+	TEST_ASSERT(ret == 0, "pthread_create task thread failed errno=%d", errno);
+
+	ret = pthread_join(vcpu_thread, &retval);
+	TEST_ASSERT(ret == 0, "pthread_join on vcpu thread failed with errno=%d", ret);
+
+	ret = pthread_join(task_thread, &retval);
+	TEST_ASSERT(ret == 0, "pthread_join on task thread failed with errno=%d", ret);
+
+	kvm_vm_free(vm);
+	return 0;
+}
-- 
2.34.1

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

[Dongli Zhang]

On 4/1/24 08:20, 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>
> ---
>  tools/testing/selftests/kvm/Makefile          |   1 +
>  .../kvm/x86_64/halt_disable_exit_test.c       | 119 ++++++++++++++++++
>  2 files changed, 120 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..4cc6a09906a2
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86_64/halt_disable_exit_test.c
> @@ -0,0 +1,119 @@
> +// 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 "processor.h"
> +#include "test_util.h"
> +
> +#define SIG_IPI SIGUSR1
> +static pthread_t task_thread, vcpu_thread;
> +
> +static void guest_code(uint64_t *is_hlt_exec)
> +{
> +	while (!READ_ONCE(*is_hlt_exec))
> +		;
> +
> +	safe_halt();

May I confirm if this selftest works on nested L1 VM as a hypervisor?

Thank you very much!

Dongli Zhang

Re: [PATCH v2 2/3] KVM: selftests: Extend @shape to allow creation of VM without in-kernel APIC

[Sean Christopherson]

On Mon, Apr 01, 2024, Manali Shukla wrote:
> 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().
> 
> Carve out space in the @shape passed to the various VM creation helpers to
> allow using the shape to control creation of a VM without in-kernel APIC
> support or with in-kernel APIC support.
> 
> This is a preparatory patch to create a vm without in-kernel APIC support for
> the KVM_X86_DISABLE_EXITS_HLT test.

Ugh, when I suggested creating a VM without an in-kernel APIC as away to easily
test that HLT doesn't exit, I wasn't thinking about the side effects of creating
a runnable VM without an in-kernel APIC.  The other downside is that practically
no one uses a userspace local APIC these days, i.e. the selftest isn't a great
representation of real world setups.

Given that KVM already provides vcpu->stat.halt_exits, using a stats FD for
verifying exiting behavior is probably a better option.  The other check that
could be added would be to verify that mp_state is always RUNNABLE (which is a
bug/gap in KVM as migrating a vCPU that was halted in the guest won't resume in
a halted state on the target).

Re: [PATCH v2 2/3] KVM: selftests: Extend @shape to allow creation of VM without in-kernel APIC

[Manali Shukla]

Hi Sean,

On 4/1/2024 10:45 PM, Sean Christopherson wrote:
> On Mon, Apr 01, 2024, Manali Shukla wrote:
>> 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().
>>
>> Carve out space in the @shape passed to the various VM creation helpers to
>> allow using the shape to control creation of a VM without in-kernel APIC
>> support or with in-kernel APIC support.
>>
>> This is a preparatory patch to create a vm without in-kernel APIC support for
>> the KVM_X86_DISABLE_EXITS_HLT test.
> 
> Ugh, when I suggested creating a VM without an in-kernel APIC as away to easily
> test that HLT doesn't exit, I wasn't thinking about the side effects of creating
> a runnable VM without an in-kernel APIC.  The other downside is that practically
> no one uses a userspace local APIC these days, i.e. the selftest isn't a great
> representation of real world setups.
> 
> Given that KVM already provides vcpu->stat.halt_exits, using a stats FD for
> verifying exiting behavior is probably a better option.  The other check that
> could be added would be to verify that mp_state is always RUNNABLE (which is a
> bug/gap in KVM as migrating a vCPU that was halted in the guest won't resume in
> a halted state on the target).
 
Sure. I will work on it.

- Manali

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

[Manali Shukla]

On 4/1/2024 10:29 PM, Dongli Zhang wrote:
> 
> 
> On 4/1/24 08:20, 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>
>> ---
>>  tools/testing/selftests/kvm/Makefile          |   1 +
>>  .../kvm/x86_64/halt_disable_exit_test.c       | 119 ++++++++++++++++++
>>  2 files changed, 120 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..4cc6a09906a2
>> --- /dev/null
>> +++ b/tools/testing/selftests/kvm/x86_64/halt_disable_exit_test.c
>> @@ -0,0 +1,119 @@
>> +// 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 "processor.h"
>> +#include "test_util.h"
>> +
>> +#define SIG_IPI SIGUSR1
>> +static pthread_t task_thread, vcpu_thread;
>> +
>> +static void guest_code(uint64_t *is_hlt_exec)
>> +{
>> +	while (!READ_ONCE(*is_hlt_exec))
>> +		;
>> +
>> +	safe_halt();
> 
> May I confirm if this selftest works on nested L1 VM as a hypervisor?

Yes, this selftest works on nested L1 VM.

> 
> Thank you very much!
> 
> Dongli Zhang