[RFC,v5,16/29] KVM: selftests: TDX: Add TDX HLT exit test

Message ID	20231212204647.2170650-17-sagis@google.com (mailing list archive)
State	New
Headers	show Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="ZMnmrFlK" Date: Tue, 12 Dec 2023 12:46:31 -0800 In-Reply-To: <20231212204647.2170650-1-sagis@google.com> Precedence: bulk Mime-Version: 1.0 References: <20231212204647.2170650-1-sagis@google.com> Message-ID: <20231212204647.2170650-17-sagis@google.com> Subject: [RFC PATCH v5 16/29] KVM: selftests: TDX: Add TDX HLT exit test From: Sagi Shahar <sagis@google.com> To: linux-kselftest@vger.kernel.org, Ackerley Tng <ackerleytng@google.com>, Ryan Afranji <afranji@google.com>, Erdem Aktas <erdemaktas@google.com>, Sagi Shahar <sagis@google.com>, Isaku Yamahata <isaku.yamahata@intel.com> Cc: Sean Christopherson <seanjc@google.com>, Paolo Bonzini <pbonzini@redhat.com>, Shuah Khan <shuah@kernel.org>, Peter Gonda <pgonda@google.com>, Haibo Xu <haibo1.xu@intel.com>, Chao Peng <chao.p.peng@linux.intel.com>, Vishal Annapurve <vannapurve@google.com>, Roger Wang <runanwang@google.com>, Vipin Sharma <vipinsh@google.com>, jmattson@google.com, dmatlack@google.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8"
Series	TDX KVM selftests \| expand [RFC,v5,00/29] TDX KVM selftests [RFC,v5,01/29] KVM: selftests: Add function to allow one-to-one GVA to GPA mappings [RFC,v5,02/29] KVM: selftests: Expose function that sets up sregs based on VM's mode [RFC,v5,03/29] KVM: selftests: Store initial stack address in struct kvm_vcpu [RFC,v5,04/29] KVM: selftests: Refactor steps in vCPU descriptor table initialization [RFC,v5,05/29] KVM: selftests: Add helper functions to create TDX VMs [RFC,v5,06/29] KVM: selftests: TDX: Use KVM_TDX_CAPABILITIES to validate TDs' attribute configurati… [RFC,v5,07/29] KVM: selftests: TDX: Update load_td_memory_region for VM memory backed by guest memfd [RFC,v5,08/29] KVM: selftests: TDX: Add TDX lifecycle test [RFC,v5,09/29] KVM: selftests: TDX: Add report_fatal_error test [RFC,v5,10/29] KVM: selftests: TDX: Adding test case for TDX port IO [RFC,v5,11/29] KVM: selftests: TDX: Add basic TDX CPUID test [RFC,v5,12/29] KVM: selftests: TDX: Add basic get_td_vmcall_info test [RFC,v5,13/29] KVM: selftests: TDX: Add TDX IO writes test [RFC,v5,14/29] KVM: selftests: TDX: Add TDX IO reads test [RFC,v5,15/29] KVM: selftests: TDX: Add TDX MSR read/write tests [RFC,v5,16/29] KVM: selftests: TDX: Add TDX HLT exit test [RFC,v5,17/29] KVM: selftests: TDX: Add TDX MMIO reads test [RFC,v5,18/29] KVM: selftests: TDX: Add TDX MMIO writes test [RFC,v5,19/29] KVM: selftests: TDX: Add TDX CPUID TDVMCALL test [RFC,v5,20/29] KVM: selftests: TDX: Verify the behavior when host consumes a TD private memory [RFC,v5,21/29] KVM: selftests: TDX: Add TDG.VP.INFO test [RFC,v5,22/29] KVM: selftests: Add functions to allow mapping as shared [RFC,v5,23/29] KVM: selftests: TDX: Add shared memory test [RFC,v5,24/29] KVM: selftests: Expose _vm_vaddr_alloc [RFC,v5,25/29] KVM: selftests: TDX: Add support for TDG.MEM.PAGE.ACCEPT [RFC,v5,26/29] KVM: selftests: TDX: Add support for TDG.VP.VEINFO.GET [RFC,v5,27/29] KVM: selftests: Propagate KVM_EXIT_MEMORY_FAULT to userspace [RFC,v5,28/29] KVM: selftests: TDX: Add TDX UPM selftest [RFC,v5,29/29] KVM: selftests: TDX: Add TDX UPM selftests for implicit conversion

Message ID

20231212204647.2170650-17-sagis@google.com (mailing list archive)

State

New

Headers

Date: Tue, 12 Dec 2023 12:46:31 -0800
In-Reply-To: <20231212204647.2170650-1-sagis@google.com>
Precedence: bulk
Mime-Version: 1.0
References: <20231212204647.2170650-1-sagis@google.com>
Message-ID: <20231212204647.2170650-17-sagis@google.com>
Subject: [RFC PATCH v5 16/29] KVM: selftests: TDX: Add TDX HLT exit test
From: Sagi Shahar <sagis@google.com>
To: linux-kselftest@vger.kernel.org, Ackerley Tng <ackerleytng@google.com>,
	Ryan Afranji <afranji@google.com>, Erdem Aktas <erdemaktas@google.com>,
	Sagi Shahar <sagis@google.com>, Isaku Yamahata <isaku.yamahata@intel.com>
Cc: Sean Christopherson <seanjc@google.com>,
 Paolo Bonzini <pbonzini@redhat.com>,
	Shuah Khan <shuah@kernel.org>, Peter Gonda <pgonda@google.com>,
 Haibo Xu <haibo1.xu@intel.com>,
	Chao Peng <chao.p.peng@linux.intel.com>,
 Vishal Annapurve <vannapurve@google.com>,
	Roger Wang <runanwang@google.com>, Vipin Sharma <vipinsh@google.com>,
 jmattson@google.com,
	dmatlack@google.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	linux-mm@kvack.org
Content-Type: text/plain; charset="UTF-8"

Series

TDX KVM selftests | expand

Commit Message

Sagi Shahar Dec. 12, 2023, 8:46 p.m. UTC

The test verifies that the guest runs TDVMCALL<INSTRUCTION.HLT> and the
guest vCPU enters to the halted state.

Signed-off-by: Erdem Aktas <erdemaktas@google.com>
Signed-off-by: Sagi Shahar <sagis@google.com>
Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Ryan Afranji <afranji@google.com>
---
 .../selftests/kvm/include/x86_64/tdx/tdx.h    |  2 +
 .../selftests/kvm/lib/x86_64/tdx/tdx.c        | 10 +++
 .../selftests/kvm/x86_64/tdx_vm_tests.c       | 78 +++++++++++++++++++
 3 files changed, 90 insertions(+)

Comments

Binbin Wu March 2, 2024, 7:31 a.m. UTC | #1

On 12/13/2023 4:46 AM, Sagi Shahar wrote:
> The test verifies that the guest runs TDVMCALL<INSTRUCTION.HLT> and the
> guest vCPU enters to the halted state.
>
> Signed-off-by: Erdem Aktas <erdemaktas@google.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Ryan Afranji <afranji@google.com>
> ---
>   .../selftests/kvm/include/x86_64/tdx/tdx.h    |  2 +
>   .../selftests/kvm/lib/x86_64/tdx/tdx.c        | 10 +++
>   .../selftests/kvm/x86_64/tdx_vm_tests.c       | 78 +++++++++++++++++++
>   3 files changed, 90 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> index 85ba6aab79a7..b18e39d20498 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> @@ -8,6 +8,7 @@
>   #define TDG_VP_VMCALL_GET_TD_VM_CALL_INFO 0x10000
>   #define TDG_VP_VMCALL_REPORT_FATAL_ERROR 0x10003
>   
> +#define TDG_VP_VMCALL_INSTRUCTION_HLT 12
>   #define TDG_VP_VMCALL_INSTRUCTION_IO 30
>   #define TDG_VP_VMCALL_INSTRUCTION_RDMSR 31
>   #define TDG_VP_VMCALL_INSTRUCTION_WRMSR 32
> @@ -20,5 +21,6 @@ uint64_t tdg_vp_vmcall_get_td_vmcall_info(uint64_t *r11, uint64_t *r12,
>   					uint64_t *r13, uint64_t *r14);
>   uint64_t tdg_vp_vmcall_instruction_rdmsr(uint64_t index, uint64_t *ret_value);
>   uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index, uint64_t value);
> +uint64_t tdg_vp_vmcall_instruction_hlt(uint64_t interrupt_blocked_flag);
>   
>   #endif // SELFTEST_TDX_TDX_H
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> index 88ea6f2a6469..9485bafedc38 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> @@ -114,3 +114,13 @@ uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index, uint64_t value)
>   
>   	return __tdx_hypercall(&args, 0);
>   }
> +
> +uint64_t tdg_vp_vmcall_instruction_hlt(uint64_t interrupt_blocked_flag)
> +{
> +	struct tdx_hypercall_args args = {
> +		.r11 = TDG_VP_VMCALL_INSTRUCTION_HLT,
> +		.r12 = interrupt_blocked_flag,
> +	};
> +
> +	return __tdx_hypercall(&args, 0);
> +}
> diff --git a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> index 5db3701cc6d9..5fae4c6e5f95 100644
> --- a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> +++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> @@ -721,6 +721,83 @@ void verify_guest_msr_writes(void)
>   	printf("\t ... PASSED\n");
>   }
>   
> +/*
> + * Verifies HLT functionality.
> + */
> +void guest_hlt(void)
> +{
> +	uint64_t ret;
> +	uint64_t interrupt_blocked_flag;
> +
> +	interrupt_blocked_flag = 0;
> +	ret = tdg_vp_vmcall_instruction_hlt(interrupt_blocked_flag);
> +	if (ret)
> +		tdx_test_fatal(ret);
> +
> +	tdx_test_success();
> +}
> +
> +void _verify_guest_hlt(int signum);
> +
> +void wake_me(int interval)
> +{
> +	struct sigaction action;
> +
> +	action.sa_handler = _verify_guest_hlt;
> +	sigemptyset(&action.sa_mask);
> +	action.sa_flags = 0;
> +
> +	TEST_ASSERT(sigaction(SIGALRM, &action, NULL) == 0,
> +		    "Could not set the alarm handler!");
> +
> +	alarm(interval);
> +}
> +
> +void _verify_guest_hlt(int signum)
> +{
> +	struct kvm_vm *vm;
> +	static struct kvm_vcpu *vcpu;
> +
> +	/*
> +	 * This function will also be called by SIGALRM handler to check the
> +	 * vCPU MP State. If vm has been initialized, then we are in the signal
> +	 * handler. Check the MP state and let the guest run again.
> +	 */
> +	if (vcpu != NULL) {

What if the following case if there is a bug in KVM so that:

In guest, execution of tdg_vp_vmcall_instruction_hlt() return 0, but the
vcpu is not actually halted. Then guest will call tdx_test_success().

And the first call of _verify_guest_hlt() will call kvm_vm_free(vm) to free
the vm, which also frees the vcpu, and 1 second later, in this path vcpu 
will
be accessed after free.

> +		struct kvm_mp_state mp_state;
> +
> +		vcpu_mp_state_get(vcpu, &mp_state);
> +		TEST_ASSERT_EQ(mp_state.mp_state, KVM_MP_STATE_HALTED);
> +
> +		/* Let the guest to run and finish the test.*/
> +		mp_state.mp_state = KVM_MP_STATE_RUNNABLE;
> +		vcpu_mp_state_set(vcpu, &mp_state);
> +		return;
> +	}
> +
> +	vm = td_create();
> +	td_initialize(vm, VM_MEM_SRC_ANONYMOUS, 0);
> +	vcpu = td_vcpu_add(vm, 0, guest_hlt);
> +	td_finalize(vm);
> +
> +	printf("Verifying HLT:\n");
> +
> +	printf("\t ... Running guest\n");
> +
> +	/* Wait 1 second for guest to execute HLT */
> +	wake_me(1);
> +	td_vcpu_run(vcpu);
> +
> +	TDX_TEST_ASSERT_SUCCESS(vcpu);
> +
> +	kvm_vm_free(vm);
> +	printf("\t ... PASSED\n");
> +}
> +
> +void verify_guest_hlt(void)
> +{
> +	_verify_guest_hlt(0);
> +}
>   
>   int main(int argc, char **argv)
>   {
> @@ -740,6 +817,7 @@ int main(int argc, char **argv)
>   	run_in_new_process(&verify_guest_reads);
>   	run_in_new_process(&verify_guest_msr_writes);
>   	run_in_new_process(&verify_guest_msr_reads);
> +	run_in_new_process(&verify_guest_hlt);
>   
>   	return 0;
>   }

Yan Zhao March 5, 2024, 5:40 a.m. UTC | #2

On Sat, Mar 02, 2024 at 03:31:07PM +0800, Binbin Wu wrote:
> On 12/13/2023 4:46 AM, Sagi Shahar wrote:
> > The test verifies that the guest runs TDVMCALL<INSTRUCTION.HLT> and the
> > guest vCPU enters to the halted state.
> > 
> > Signed-off-by: Erdem Aktas <erdemaktas@google.com>
> > Signed-off-by: Sagi Shahar <sagis@google.com>
> > Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> > Signed-off-by: Ryan Afranji <afranji@google.com>
> > ---
> >   .../selftests/kvm/include/x86_64/tdx/tdx.h    |  2 +
> >   .../selftests/kvm/lib/x86_64/tdx/tdx.c        | 10 +++
> >   .../selftests/kvm/x86_64/tdx_vm_tests.c       | 78 +++++++++++++++++++
> >   3 files changed, 90 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> > index 85ba6aab79a7..b18e39d20498 100644
> > --- a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> > +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> > @@ -8,6 +8,7 @@
> >   #define TDG_VP_VMCALL_GET_TD_VM_CALL_INFO 0x10000
> >   #define TDG_VP_VMCALL_REPORT_FATAL_ERROR 0x10003
> > +#define TDG_VP_VMCALL_INSTRUCTION_HLT 12
> >   #define TDG_VP_VMCALL_INSTRUCTION_IO 30
> >   #define TDG_VP_VMCALL_INSTRUCTION_RDMSR 31
> >   #define TDG_VP_VMCALL_INSTRUCTION_WRMSR 32
> > @@ -20,5 +21,6 @@ uint64_t tdg_vp_vmcall_get_td_vmcall_info(uint64_t *r11, uint64_t *r12,
> >   					uint64_t *r13, uint64_t *r14);
> >   uint64_t tdg_vp_vmcall_instruction_rdmsr(uint64_t index, uint64_t *ret_value);
> >   uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index, uint64_t value);
> > +uint64_t tdg_vp_vmcall_instruction_hlt(uint64_t interrupt_blocked_flag);
> >   #endif // SELFTEST_TDX_TDX_H
> > diff --git a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> > index 88ea6f2a6469..9485bafedc38 100644
> > --- a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> > +++ b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> > @@ -114,3 +114,13 @@ uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index, uint64_t value)
> >   	return __tdx_hypercall(&args, 0);
> >   }
> > +
> > +uint64_t tdg_vp_vmcall_instruction_hlt(uint64_t interrupt_blocked_flag)
> > +{
> > +	struct tdx_hypercall_args args = {
> > +		.r11 = TDG_VP_VMCALL_INSTRUCTION_HLT,
> > +		.r12 = interrupt_blocked_flag,
> > +	};
> > +
> > +	return __tdx_hypercall(&args, 0);
> > +}
> > diff --git a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> > index 5db3701cc6d9..5fae4c6e5f95 100644
> > --- a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> > +++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> > @@ -721,6 +721,83 @@ void verify_guest_msr_writes(void)
> >   	printf("\t ... PASSED\n");
> >   }
> > +/*
> > + * Verifies HLT functionality.
> > + */
> > +void guest_hlt(void)
> > +{
> > +	uint64_t ret;
> > +	uint64_t interrupt_blocked_flag;
> > +
> > +	interrupt_blocked_flag = 0;
> > +	ret = tdg_vp_vmcall_instruction_hlt(interrupt_blocked_flag);
> > +	if (ret)
> > +		tdx_test_fatal(ret);
> > +
> > +	tdx_test_success();
> > +}
> > +
> > +void _verify_guest_hlt(int signum);
> > +
> > +void wake_me(int interval)
> > +{
> > +	struct sigaction action;
> > +
> > +	action.sa_handler = _verify_guest_hlt;
> > +	sigemptyset(&action.sa_mask);
> > +	action.sa_flags = 0;
> > +
> > +	TEST_ASSERT(sigaction(SIGALRM, &action, NULL) == 0,
> > +		    "Could not set the alarm handler!");
> > +
> > +	alarm(interval);
> > +}
> > +
> > +void _verify_guest_hlt(int signum)
> > +{
> > +	struct kvm_vm *vm;
> > +	static struct kvm_vcpu *vcpu;
> > +
> > +	/*
> > +	 * This function will also be called by SIGALRM handler to check the
> > +	 * vCPU MP State. If vm has been initialized, then we are in the signal
> > +	 * handler. Check the MP state and let the guest run again.
> > +	 */
> > +	if (vcpu != NULL) {
> 
> What if the following case if there is a bug in KVM so that:
> 
> In guest, execution of tdg_vp_vmcall_instruction_hlt() return 0, but the
> vcpu is not actually halted. Then guest will call tdx_test_success().
> 
> And the first call of _verify_guest_hlt() will call kvm_vm_free(vm) to free
> the vm, which also frees the vcpu, and 1 second later, in this path vcpu
> will
> be accessed after free.
>
Right. Another possibility is that if buggy KVM returns success to guest
without putting guest to halted state, the selftest will still print
"PASSED" because the second _verify_guest_hlt() (after waiting for 1s)
has no chance to get executed before the process exits.

> > +		struct kvm_mp_state mp_state;
> > +
> > +		vcpu_mp_state_get(vcpu, &mp_state);
> > +		TEST_ASSERT_EQ(mp_state.mp_state, KVM_MP_STATE_HALTED);
> > +
> > +		/* Let the guest to run and finish the test.*/
> > +		mp_state.mp_state = KVM_MP_STATE_RUNNABLE;
> > +		vcpu_mp_state_set(vcpu, &mp_state);
> > +		return;
> > +	}
> > +
> > +	vm = td_create();
> > +	td_initialize(vm, VM_MEM_SRC_ANONYMOUS, 0);
> > +	vcpu = td_vcpu_add(vm, 0, guest_hlt);
> > +	td_finalize(vm);
> > +
> > +	printf("Verifying HLT:\n");
> > +
> > +	printf("\t ... Running guest\n");
> > +
> > +	/* Wait 1 second for guest to execute HLT */
> > +	wake_me(1);
> > +	td_vcpu_run(vcpu);
> > +
> > +	TDX_TEST_ASSERT_SUCCESS(vcpu);
> > +
> > +	kvm_vm_free(vm);
> > +	printf("\t ... PASSED\n");
> > +}
> > +
> > +void verify_guest_hlt(void)
> > +{
> > +	_verify_guest_hlt(0);
> > +}
> >   int main(int argc, char **argv)
> >   {
> > @@ -740,6 +817,7 @@ int main(int argc, char **argv)
> >   	run_in_new_process(&verify_guest_reads);
> >   	run_in_new_process(&verify_guest_msr_writes);
> >   	run_in_new_process(&verify_guest_msr_reads);
> > +	run_in_new_process(&verify_guest_hlt);
> >   	return 0;
> >   }
> 
>

Sagi Shahar July 27, 2024, 11:23 p.m. UTC | #3

On Tue, Mar 5, 2024 at 12:10 AM Yan Zhao <yan.y.zhao@intel.com> wrote:
>
> On Sat, Mar 02, 2024 at 03:31:07PM +0800, Binbin Wu wrote:
> > On 12/13/2023 4:46 AM, Sagi Shahar wrote:
> > > The test verifies that the guest runs TDVMCALL<INSTRUCTION.HLT> and the
> > > guest vCPU enters to the halted state.
> > >
> > > Signed-off-by: Erdem Aktas <erdemaktas@google.com>
> > > Signed-off-by: Sagi Shahar <sagis@google.com>
> > > Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> > > Signed-off-by: Ryan Afranji <afranji@google.com>
> > > ---
> > >   .../selftests/kvm/include/x86_64/tdx/tdx.h    |  2 +
> > >   .../selftests/kvm/lib/x86_64/tdx/tdx.c        | 10 +++
> > >   .../selftests/kvm/x86_64/tdx_vm_tests.c       | 78 +++++++++++++++++++
> > >   3 files changed, 90 insertions(+)
> > >
> > > diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> > > index 85ba6aab79a7..b18e39d20498 100644
> > > --- a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> > > +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> > > @@ -8,6 +8,7 @@
> > >   #define TDG_VP_VMCALL_GET_TD_VM_CALL_INFO 0x10000
> > >   #define TDG_VP_VMCALL_REPORT_FATAL_ERROR 0x10003
> > > +#define TDG_VP_VMCALL_INSTRUCTION_HLT 12
> > >   #define TDG_VP_VMCALL_INSTRUCTION_IO 30
> > >   #define TDG_VP_VMCALL_INSTRUCTION_RDMSR 31
> > >   #define TDG_VP_VMCALL_INSTRUCTION_WRMSR 32
> > > @@ -20,5 +21,6 @@ uint64_t tdg_vp_vmcall_get_td_vmcall_info(uint64_t *r11, uint64_t *r12,
> > >                                     uint64_t *r13, uint64_t *r14);
> > >   uint64_t tdg_vp_vmcall_instruction_rdmsr(uint64_t index, uint64_t *ret_value);
> > >   uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index, uint64_t value);
> > > +uint64_t tdg_vp_vmcall_instruction_hlt(uint64_t interrupt_blocked_flag);
> > >   #endif // SELFTEST_TDX_TDX_H
> > > diff --git a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> > > index 88ea6f2a6469..9485bafedc38 100644
> > > --- a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> > > +++ b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> > > @@ -114,3 +114,13 @@ uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index, uint64_t value)
> > >     return __tdx_hypercall(&args, 0);
> > >   }
> > > +
> > > +uint64_t tdg_vp_vmcall_instruction_hlt(uint64_t interrupt_blocked_flag)
> > > +{
> > > +   struct tdx_hypercall_args args = {
> > > +           .r11 = TDG_VP_VMCALL_INSTRUCTION_HLT,
> > > +           .r12 = interrupt_blocked_flag,
> > > +   };
> > > +
> > > +   return __tdx_hypercall(&args, 0);
> > > +}
> > > diff --git a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> > > index 5db3701cc6d9..5fae4c6e5f95 100644
> > > --- a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> > > +++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> > > @@ -721,6 +721,83 @@ void verify_guest_msr_writes(void)
> > >     printf("\t ... PASSED\n");
> > >   }
> > > +/*
> > > + * Verifies HLT functionality.
> > > + */
> > > +void guest_hlt(void)
> > > +{
> > > +   uint64_t ret;
> > > +   uint64_t interrupt_blocked_flag;
> > > +
> > > +   interrupt_blocked_flag = 0;
> > > +   ret = tdg_vp_vmcall_instruction_hlt(interrupt_blocked_flag);
> > > +   if (ret)
> > > +           tdx_test_fatal(ret);
> > > +
> > > +   tdx_test_success();
> > > +}
> > > +
> > > +void _verify_guest_hlt(int signum);
> > > +
> > > +void wake_me(int interval)
> > > +{
> > > +   struct sigaction action;
> > > +
> > > +   action.sa_handler = _verify_guest_hlt;
> > > +   sigemptyset(&action.sa_mask);
> > > +   action.sa_flags = 0;
> > > +
> > > +   TEST_ASSERT(sigaction(SIGALRM, &action, NULL) == 0,
> > > +               "Could not set the alarm handler!");
> > > +
> > > +   alarm(interval);
> > > +}
> > > +
> > > +void _verify_guest_hlt(int signum)
> > > +{
> > > +   struct kvm_vm *vm;
> > > +   static struct kvm_vcpu *vcpu;
> > > +
> > > +   /*
> > > +    * This function will also be called by SIGALRM handler to check the
> > > +    * vCPU MP State. If vm has been initialized, then we are in the signal
> > > +    * handler. Check the MP state and let the guest run again.
> > > +    */
> > > +   if (vcpu != NULL) {
> >
> > What if the following case if there is a bug in KVM so that:
> >
> > In guest, execution of tdg_vp_vmcall_instruction_hlt() return 0, but the
> > vcpu is not actually halted. Then guest will call tdx_test_success().
> >
> > And the first call of _verify_guest_hlt() will call kvm_vm_free(vm) to free
> > the vm, which also frees the vcpu, and 1 second later, in this path vcpu
> > will
> > be accessed after free.
> >
> Right. Another possibility is that if buggy KVM returns success to guest
> without putting guest to halted state, the selftest will still print
> "PASSED" because the second _verify_guest_hlt() (after waiting for 1s)
> has no chance to get executed before the process exits.
>
It sounds like in both cases we're going to hit an assert at some
point. If the VM was already freed then vcpu_mp_state_get will fail
the ioctl and assert internally or crash the process. If the guest
never halts and vcpu is still valid then the mp state assert will
fire. Either way it would be pretty obvious that something is wrong.


> > > +           struct kvm_mp_state mp_state;
> > > +
> > > +           vcpu_mp_state_get(vcpu, &mp_state);
> > > +           TEST_ASSERT_EQ(mp_state.mp_state, KVM_MP_STATE_HALTED);
> > > +
> > > +           /* Let the guest to run and finish the test.*/
> > > +           mp_state.mp_state = KVM_MP_STATE_RUNNABLE;
> > > +           vcpu_mp_state_set(vcpu, &mp_state);
> > > +           return;
> > > +   }
> > > +
> > > +   vm = td_create();
> > > +   td_initialize(vm, VM_MEM_SRC_ANONYMOUS, 0);
> > > +   vcpu = td_vcpu_add(vm, 0, guest_hlt);
> > > +   td_finalize(vm);
> > > +
> > > +   printf("Verifying HLT:\n");
> > > +
> > > +   printf("\t ... Running guest\n");
> > > +
> > > +   /* Wait 1 second for guest to execute HLT */
> > > +   wake_me(1);
> > > +   td_vcpu_run(vcpu);
> > > +
> > > +   TDX_TEST_ASSERT_SUCCESS(vcpu);
> > > +
> > > +   kvm_vm_free(vm);
> > > +   printf("\t ... PASSED\n");
> > > +}
> > > +
> > > +void verify_guest_hlt(void)
> > > +{
> > > +   _verify_guest_hlt(0);
> > > +}
> > >   int main(int argc, char **argv)
> > >   {
> > > @@ -740,6 +817,7 @@ int main(int argc, char **argv)
> > >     run_in_new_process(&verify_guest_reads);
> > >     run_in_new_process(&verify_guest_msr_writes);
> > >     run_in_new_process(&verify_guest_msr_reads);
> > > +   run_in_new_process(&verify_guest_hlt);
> > >     return 0;
> > >   }
> >
> >

diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
index 85ba6aab79a7..b18e39d20498 100644
--- a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
+++ b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
@@ -8,6 +8,7 @@ 
 #define TDG_VP_VMCALL_GET_TD_VM_CALL_INFO 0x10000
 #define TDG_VP_VMCALL_REPORT_FATAL_ERROR 0x10003
 
+#define TDG_VP_VMCALL_INSTRUCTION_HLT 12
 #define TDG_VP_VMCALL_INSTRUCTION_IO 30
 #define TDG_VP_VMCALL_INSTRUCTION_RDMSR 31
 #define TDG_VP_VMCALL_INSTRUCTION_WRMSR 32
@@ -20,5 +21,6 @@  uint64_t tdg_vp_vmcall_get_td_vmcall_info(uint64_t *r11, uint64_t *r12,
 					uint64_t *r13, uint64_t *r14);
 uint64_t tdg_vp_vmcall_instruction_rdmsr(uint64_t index, uint64_t *ret_value);
 uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index, uint64_t value);
+uint64_t tdg_vp_vmcall_instruction_hlt(uint64_t interrupt_blocked_flag);
 
 #endif // SELFTEST_TDX_TDX_H
diff --git a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
index 88ea6f2a6469..9485bafedc38 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
@@ -114,3 +114,13 @@  uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index, uint64_t value)
 
 	return __tdx_hypercall(&args, 0);
 }
+
+uint64_t tdg_vp_vmcall_instruction_hlt(uint64_t interrupt_blocked_flag)
+{
+	struct tdx_hypercall_args args = {
+		.r11 = TDG_VP_VMCALL_INSTRUCTION_HLT,
+		.r12 = interrupt_blocked_flag,
+	};
+
+	return __tdx_hypercall(&args, 0);
+}
diff --git a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
index 5db3701cc6d9..5fae4c6e5f95 100644
--- a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
+++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
@@ -721,6 +721,83 @@  void verify_guest_msr_writes(void)
 	printf("\t ... PASSED\n");
 }
 
+/*
+ * Verifies HLT functionality.
+ */
+void guest_hlt(void)
+{
+	uint64_t ret;
+	uint64_t interrupt_blocked_flag;
+
+	interrupt_blocked_flag = 0;
+	ret = tdg_vp_vmcall_instruction_hlt(interrupt_blocked_flag);
+	if (ret)
+		tdx_test_fatal(ret);
+
+	tdx_test_success();
+}
+
+void _verify_guest_hlt(int signum);
+
+void wake_me(int interval)
+{
+	struct sigaction action;
+
+	action.sa_handler = _verify_guest_hlt;
+	sigemptyset(&action.sa_mask);
+	action.sa_flags = 0;
+
+	TEST_ASSERT(sigaction(SIGALRM, &action, NULL) == 0,
+		    "Could not set the alarm handler!");
+
+	alarm(interval);
+}
+
+void _verify_guest_hlt(int signum)
+{
+	struct kvm_vm *vm;
+	static struct kvm_vcpu *vcpu;
+
+	/*
+	 * This function will also be called by SIGALRM handler to check the
+	 * vCPU MP State. If vm has been initialized, then we are in the signal
+	 * handler. Check the MP state and let the guest run again.
+	 */
+	if (vcpu != NULL) {
+		struct kvm_mp_state mp_state;
+
+		vcpu_mp_state_get(vcpu, &mp_state);
+		TEST_ASSERT_EQ(mp_state.mp_state, KVM_MP_STATE_HALTED);
+
+		/* Let the guest to run and finish the test.*/
+		mp_state.mp_state = KVM_MP_STATE_RUNNABLE;
+		vcpu_mp_state_set(vcpu, &mp_state);
+		return;
+	}
+
+	vm = td_create();
+	td_initialize(vm, VM_MEM_SRC_ANONYMOUS, 0);
+	vcpu = td_vcpu_add(vm, 0, guest_hlt);
+	td_finalize(vm);
+
+	printf("Verifying HLT:\n");
+
+	printf("\t ... Running guest\n");
+
+	/* Wait 1 second for guest to execute HLT */
+	wake_me(1);
+	td_vcpu_run(vcpu);
+
+	TDX_TEST_ASSERT_SUCCESS(vcpu);
+
+	kvm_vm_free(vm);
+	printf("\t ... PASSED\n");
+}
+
+void verify_guest_hlt(void)
+{
+	_verify_guest_hlt(0);
+}
 
 int main(int argc, char **argv)
 {
@@ -740,6 +817,7 @@  int main(int argc, char **argv)
 	run_in_new_process(&verify_guest_reads);
 	run_in_new_process(&verify_guest_msr_writes);
 	run_in_new_process(&verify_guest_msr_reads);
+	run_in_new_process(&verify_guest_hlt);
 
 	return 0;
 }

[RFC,v5,16/29] KVM: selftests: TDX: Add TDX HLT exit test

Commit Message

Comments

Patch