[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6aa15d1d-b67a-4c94-b6f0-674531ba7a63@linux.intel.com>
Date: Thu, 14 Aug 2025 11:24:02 +0800
From: Binbin Wu <binbin.wu@...ux.intel.com>
To: Sagi Shahar <sagis@...gle.com>
Cc: linux-kselftest@...r.kernel.org, Paolo Bonzini <pbonzini@...hat.com>,
Shuah Khan <shuah@...nel.org>, Sean Christopherson <seanjc@...gle.com>,
Ackerley Tng <ackerleytng@...gle.com>, Ryan Afranji <afranji@...gle.com>,
Andrew Jones <ajones@...tanamicro.com>,
Isaku Yamahata <isaku.yamahata@...el.com>,
Erdem Aktas <erdemaktas@...gle.com>,
Rick Edgecombe <rick.p.edgecombe@...el.com>,
Roger Wang <runanwang@...gle.com>, Oliver Upton <oliver.upton@...ux.dev>,
"Pratik R. Sampat" <pratikrajesh.sampat@....com>,
Reinette Chatre <reinette.chatre@...el.com>, Ira Weiny
<ira.weiny@...el.com>, linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH v8 11/30] KVM: selftests: TDX: Adding test case for TDX
port IO
On 8/8/2025 4:16 AM, Sagi Shahar wrote:
> Verifies TDVMCALL<INSTRUCTION.IO> READ and WRITE operations.
>
> Signed-off-by: Sagi Shahar <sagis@...gle.com>
> ---
> .../selftests/kvm/include/x86/tdx/test_util.h | 20 +++++
> .../selftests/kvm/lib/x86/tdx/test_util.c | 35 +++++++++
> tools/testing/selftests/kvm/x86/tdx_vm_test.c | 78 ++++++++++++++++++-
> 3 files changed, 130 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/x86/tdx/test_util.h b/tools/testing/selftests/kvm/include/x86/tdx/test_util.h
> index dafeee9af1dc..cf11955d56d6 100644
> --- a/tools/testing/selftests/kvm/include/x86/tdx/test_util.h
> +++ b/tools/testing/selftests/kvm/include/x86/tdx/test_util.h
> @@ -13,6 +13,19 @@
> #define PORT_READ 0
> #define PORT_WRITE 1
>
> +/*
> + * Assert that some IO operation involving tdg_vp_vmcall_instruction_io() was
> + * called in the guest.
> + */
> +void tdx_test_assert_io(struct kvm_vcpu *vcpu, uint16_t port, uint8_t size,
> + uint8_t direction);
> +
> +/*
> + * Run the tdx vcpu and check if there was some failure in the guest, either
> + * an exception like a triple fault,
Only "KVM_EXIT_SYSTEM_EVENT" is checked in the implementation.
> or if a tdx_test_fatal() was hit.
> + */
> +void tdx_run(struct kvm_vcpu *vcpu);
> +
> /*
> * Run a test in a new process.
> *
> @@ -57,4 +70,11 @@ void tdx_test_fatal(uint64_t error_code);
> */
> void tdx_test_fatal_with_data(uint64_t error_code, uint64_t data_gpa);
>
> +/*
> + * Assert on @error and report the @error to userspace.
> + * Return value from tdg_vp_vmcall_report_fatal_error() is ignored since execution
> + * is not expected to continue beyond this point.
> + */
> +void tdx_assert_error(uint64_t error);
> +
> #endif // SELFTEST_TDX_TEST_UTIL_H
> diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/test_util.c b/tools/testing/selftests/kvm/lib/x86/tdx/test_util.c
> index 6c82a0c3bd37..4ccc5298ba25 100644
> --- a/tools/testing/selftests/kvm/lib/x86/tdx/test_util.c
> +++ b/tools/testing/selftests/kvm/lib/x86/tdx/test_util.c
> @@ -8,8 +8,37 @@
>
> #include "kvm_util.h"
> #include "tdx/tdx.h"
> +#include "tdx/tdx_util.h"
> #include "tdx/test_util.h"
>
> +void tdx_test_assert_io(struct kvm_vcpu *vcpu, uint16_t port, uint8_t size,
> + uint8_t direction)
> +{
> + TEST_ASSERT(vcpu->run->exit_reason == KVM_EXIT_IO,
> + "Got exit_reason other than KVM_EXIT_IO: %u (%s)\n",
> + vcpu->run->exit_reason,
> + exit_reason_str(vcpu->run->exit_reason));
> +
> + TEST_ASSERT(vcpu->run->exit_reason == KVM_EXIT_IO &&
KVM_EXIT_IO has been checked above already.
> + vcpu->run->io.port == port &&
> + vcpu->run->io.size == size &&
> + vcpu->run->io.direction == direction,
> + "Got unexpected IO exit values: %u (%s) %u %u %u\n",
> + vcpu->run->exit_reason,
> + exit_reason_str(vcpu->run->exit_reason),
the exit reason info could be removed since it is redundant
> + vcpu->run->io.port, vcpu->run->io.size,
> + vcpu->run->io.direction);
> +}
> +
> +void tdx_run(struct kvm_vcpu *vcpu)
> +{
> + td_vcpu_run(vcpu);
> + if (vcpu->run->exit_reason == KVM_EXIT_SYSTEM_EVENT)
> + TEST_FAIL("Guest reported error. error code: %lld (0x%llx)\n",
> + vcpu->run->system_event.data[12],
> + vcpu->run->system_event.data[13]);
> +}
> +
> int run_in_new_process(void (*func)(void))
> {
> int wstatus;
> @@ -69,3 +98,9 @@ void tdx_test_fatal(uint64_t error_code)
> {
> tdx_test_fatal_with_data(error_code, 0);
> }
> +
> +void tdx_assert_error(uint64_t error)
> +{
> + if (error)
> + tdx_test_fatal(error);
> +}
> diff --git a/tools/testing/selftests/kvm/x86/tdx_vm_test.c b/tools/testing/selftests/kvm/x86/tdx_vm_test.c
> index 7d6d71602761..97330e28f236 100644
> --- a/tools/testing/selftests/kvm/x86/tdx_vm_test.c
> +++ b/tools/testing/selftests/kvm/x86/tdx_vm_test.c
> @@ -3,6 +3,7 @@
> #include <signal.h>
>
> #include "kvm_util.h"
> +#include "tdx/tdcall.h"
> #include "tdx/tdx.h"
> #include "tdx/tdx_util.h"
> #include "tdx/test_util.h"
> @@ -25,7 +26,7 @@ static void verify_td_lifecycle(void)
>
> printf("Verifying TD lifecycle:\n");
>
> - td_vcpu_run(vcpu);
> + tdx_run(vcpu);
> tdx_test_assert_success(vcpu);
>
> kvm_vm_free(vm);
> @@ -69,9 +70,78 @@ void verify_report_fatal_error(void)
> TEST_ASSERT_EQ(vcpu->run->system_event.data[12], 0x0BAAAAAD00000000);
> TEST_ASSERT_EQ(vcpu->run->system_event.data[13], 0);
>
> - td_vcpu_run(vcpu);
> + tdx_run(vcpu);
> + tdx_test_assert_success(vcpu);
> +
> + kvm_vm_free(vm);
> + printf("\t ... PASSED\n");
> +}
> +
> +#define TDX_IOEXIT_TEST_PORT 0x50
> +
> +/*
> + * Verifies IO functionality by writing a |value| to a predefined port.
> + * Verifies that the read value is |value| + 1 from the same port.
> + * If all the tests are passed then write a value to port TDX_TEST_PORT
TDX_TEST_PORT should be TDX_TEST_SUCCESS_PORT?
> + */
> +void guest_ioexit(void)
> +{
> + uint64_t data_out, data_in;
> + uint64_t ret;
> +
> + data_out = 0xAB;
> + ret = tdg_vp_vmcall_instruction_io(TDX_IOEXIT_TEST_PORT, 1,
> + PORT_WRITE, &data_out);
> + tdx_assert_error(ret);
Is it better to move the tdx_assert_error() inside
tdg_vp_vmcall_instruction_io()? So that the callers can skip the check for each
call to tdg_vp_vmcall_instruction_io().
> +
> + ret = tdg_vp_vmcall_instruction_io(TDX_IOEXIT_TEST_PORT, 1,
> + PORT_READ, &data_in);
> + tdx_assert_error(ret);
> +
> + if (data_in != 0xAC)
> + tdx_test_fatal(data_in);
> +
> + tdx_test_success();
> +}
> +
> +void verify_td_ioexit(void)
> +{
> + struct kvm_vcpu *vcpu;
> + uint32_t port_data;
> + struct kvm_vm *vm;
> +
> + vm = td_create();
> + td_initialize(vm, VM_MEM_SRC_ANONYMOUS, 0);
> + vcpu = td_vcpu_add(vm, 0, guest_ioexit);
> + td_finalize(vm);
> +
> + printf("Verifying TD IO Exit:\n");
> +
> + /* Wait for guest to do a IO write */
> + tdx_run(vcpu);
> + tdx_test_assert_io(vcpu, TDX_IOEXIT_TEST_PORT, 1, PORT_WRITE);
> + port_data = *(uint8_t *)((void *)vcpu->run + vcpu->run->io.data_offset);
> +
> + printf("\t ... IO WRITE: DONE\n");
> +
> + /*
> + * Wait for the guest to do a IO read. Provide the previous written data
> + * + 1 back to the guest
> + */
> + tdx_run(vcpu);
> + tdx_test_assert_io(vcpu, TDX_IOEXIT_TEST_PORT, 1, PORT_READ);
> + *(uint8_t *)((void *)vcpu->run + vcpu->run->io.data_offset) = port_data + 1;
> +
> + printf("\t ... IO READ: DONE\n");
> +
> + /*
> + * Wait for the guest to complete execution successfully. The read
> + * value is checked within the guest.
> + */
> + tdx_run(vcpu);
> tdx_test_assert_success(vcpu);
>
> + printf("\t ... IO verify read/write values: OK\n");
> kvm_vm_free(vm);
> printf("\t ... PASSED\n");
> }
> @@ -83,11 +153,13 @@ int main(int argc, char **argv)
> if (!is_tdx_enabled())
> ksft_exit_skip("TDX is not supported by the KVM. Exiting.\n");
>
> - ksft_set_plan(2);
> + ksft_set_plan(3);
> ksft_test_result(!run_in_new_process(&verify_td_lifecycle),
> "verify_td_lifecycle\n");
> ksft_test_result(!run_in_new_process(&verify_report_fatal_error),
> "verify_report_fatal_error\n");
> + ksft_test_result(!run_in_new_process(&verify_td_ioexit),
> + "verify_td_ioexit\n");
>
> ksft_finished();
> return 0;
Powered by blists - more mailing lists