| Message ID | 20231212204647.2170650-11-sagis@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | TDX KVM selftests | expand |
On 12/13/2023 4:46 AM, Sagi Shahar wrote: > From: Erdem Aktas <erdemaktas@google.com> > > Verifies TDVMCALL<INSTRUCTION.IO> READ and WRITE operations. > > 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> > --- > .../kvm/include/x86_64/tdx/test_util.h | 34 ++++++++ > .../selftests/kvm/x86_64/tdx_vm_tests.c | 82 +++++++++++++++++++ > 2 files changed, 116 insertions(+) One nit comment below. Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com> > > diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h b/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h > index 6d69921136bd..95a5d5be7f0b 100644 > --- a/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h > +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h > @@ -9,6 +9,40 @@ > #define TDX_TEST_SUCCESS_PORT 0x30 > #define TDX_TEST_SUCCESS_SIZE 4 > > +/** > + * Assert that some IO operation involving tdg_vp_vmcall_instruction_io() was > + * called in the guest. > + */ > +#define TDX_TEST_ASSERT_IO(VCPU, PORT, SIZE, DIR) \ > + do { \ > + 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) && \ > + ((VCPU)->run->io.port == (PORT)) && \ > + ((VCPU)->run->io.size == (SIZE)) && \ > + ((VCPU)->run->io.direction == (DIR)), \ > + "Got unexpected IO exit values: %u (%s) %d %d %d\n", \ > + (VCPU)->run->exit_reason, \ > + exit_reason_str((VCPU)->run->exit_reason), \ > + (VCPU)->run->io.port, (VCPU)->run->io.size, \ > + (VCPU)->run->io.direction); \ > + } while (0) > + > +/** > + * Check and report if there was some failure in the guest, either an exception > + * like a triple fault, or if a tdx_test_fatal() was hit. > + */ > +#define TDX_TEST_CHECK_GUEST_FAILURE(VCPU) \ > + do { \ > + 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[1], \ > + (VCPU)->run->system_event.data[1]); \ > + } while (0) > + > /** > * Assert that tdx_test_success() was called in the guest. > */ > 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 8638c7bbedaa..75467c407ca7 100644 > --- a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c > +++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c > @@ -2,6 +2,7 @@ > > #include <signal.h> > #include "kvm_util_base.h" > +#include "tdx/tdcall.h" > #include "tdx/tdx.h" > #include "tdx/tdx_util.h" > #include "tdx/test_util.h" > @@ -74,6 +75,86 @@ void verify_report_fatal_error(void) > 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 > + */ > +void guest_ioexit(void) > +{ > + uint64_t data_out, data_in, delta; > + uint64_t ret; > + > + data_out = 0xAB; > + ret = tdg_vp_vmcall_instruction_io(TDX_IOEXIT_TEST_PORT, 1, > + TDG_VP_VMCALL_INSTRUCTION_IO_WRITE, > + &data_out); > + if (ret) > + tdx_test_fatal(ret); > + > + ret = tdg_vp_vmcall_instruction_io(TDX_IOEXIT_TEST_PORT, 1, > + TDG_VP_VMCALL_INSTRUCTION_IO_READ, > + &data_in); > + if (ret) > + tdx_test_fatal(ret); > + > + delta = data_in - data_out; > + if (delta != 1) Nit: Is it more direct to compare data_in with 0xAC? > + tdx_test_fatal(ret); > + > + tdx_test_success(); > +} > + > +void verify_td_ioexit(void) > +{ > + struct kvm_vm *vm; > + struct kvm_vcpu *vcpu; > + > + uint32_t port_data; > + > + 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 */ > + td_vcpu_run(vcpu); > + TDX_TEST_CHECK_GUEST_FAILURE(vcpu); > + TDX_TEST_ASSERT_IO(vcpu, TDX_IOEXIT_TEST_PORT, 1, > + TDG_VP_VMCALL_INSTRUCTION_IO_WRITE); > + port_data = *(uint8_t *)((void *)vcpu->run + vcpu->run->io.data_offset); > + > + printf("\t ... IO WRITE: OK\n"); > + > + /* > + * Wait for the guest to do a IO read. Provide the previous written data > + * + 1 back to the guest > + */ > + td_vcpu_run(vcpu); > + TDX_TEST_CHECK_GUEST_FAILURE(vcpu); > + TDX_TEST_ASSERT_IO(vcpu, TDX_IOEXIT_TEST_PORT, 1, > + TDG_VP_VMCALL_INSTRUCTION_IO_READ); > + *(uint8_t *)((void *)vcpu->run + vcpu->run->io.data_offset) = port_data + 1; > + > + printf("\t ... IO READ: OK\n"); > + > + /* > + * Wait for the guest to complete execution successfully. The read > + * value is checked within the guest. > + */ > + td_vcpu_run(vcpu); > + TDX_TEST_CHECK_GUEST_FAILURE(vcpu); > + TDX_TEST_ASSERT_SUCCESS(vcpu); > + > + printf("\t ... IO verify read/write values: OK\n"); > + kvm_vm_free(vm); > + printf("\t ... PASSED\n"); > +} > + > int main(int argc, char **argv) > { > setbuf(stdout, NULL); > @@ -85,6 +166,7 @@ int main(int argc, char **argv) > > run_in_new_process(&verify_td_lifecycle); > run_in_new_process(&verify_report_fatal_error); > + run_in_new_process(&verify_td_ioexit); > > return 0; > }
On Tue, Dec 12, 2023 at 12:46:25PM -0800, Sagi Shahar wrote: > From: Erdem Aktas <erdemaktas@google.com> > > Verifies TDVMCALL<INSTRUCTION.IO> READ and WRITE operations. > > 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> > --- > .../kvm/include/x86_64/tdx/test_util.h | 34 ++++++++ > .../selftests/kvm/x86_64/tdx_vm_tests.c | 82 +++++++++++++++++++ > 2 files changed, 116 insertions(+) > > diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h b/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h > index 6d69921136bd..95a5d5be7f0b 100644 > --- a/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h > +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h > @@ -9,6 +9,40 @@ > #define TDX_TEST_SUCCESS_PORT 0x30 > #define TDX_TEST_SUCCESS_SIZE 4 > > +/** > + * Assert that some IO operation involving tdg_vp_vmcall_instruction_io() was > + * called in the guest. > + */ > +#define TDX_TEST_ASSERT_IO(VCPU, PORT, SIZE, DIR) \ > + do { \ > + 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) && \ > + ((VCPU)->run->io.port == (PORT)) && \ > + ((VCPU)->run->io.size == (SIZE)) && \ > + ((VCPU)->run->io.direction == (DIR)), \ > + "Got unexpected IO exit values: %u (%s) %d %d %d\n", \ > + (VCPU)->run->exit_reason, \ > + exit_reason_str((VCPU)->run->exit_reason), \ > + (VCPU)->run->io.port, (VCPU)->run->io.size, \ > + (VCPU)->run->io.direction); \ > + } while (0) > + > +/** > + * Check and report if there was some failure in the guest, either an exception > + * like a triple fault, or if a tdx_test_fatal() was hit. > + */ > +#define TDX_TEST_CHECK_GUEST_FAILURE(VCPU) \ > + do { \ > + 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[1], \ > + (VCPU)->run->system_event.data[1]); \ > + } while (0) > + > /** > * Assert that tdx_test_success() was called in the guest. > */ > 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 8638c7bbedaa..75467c407ca7 100644 > --- a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c > +++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c > @@ -2,6 +2,7 @@ > > #include <signal.h> > #include "kvm_util_base.h" > +#include "tdx/tdcall.h" > #include "tdx/tdx.h" > #include "tdx/tdx_util.h" > #include "tdx/test_util.h" > @@ -74,6 +75,86 @@ void verify_report_fatal_error(void) > 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 > + */ > +void guest_ioexit(void) > +{ > + uint64_t data_out, data_in, delta; > + uint64_t ret; > + > + data_out = 0xAB; > + ret = tdg_vp_vmcall_instruction_io(TDX_IOEXIT_TEST_PORT, 1, > + TDG_VP_VMCALL_INSTRUCTION_IO_WRITE, > + &data_out); > + if (ret) > + tdx_test_fatal(ret); > + > + ret = tdg_vp_vmcall_instruction_io(TDX_IOEXIT_TEST_PORT, 1, > + TDG_VP_VMCALL_INSTRUCTION_IO_READ, > + &data_in); > + if (ret) > + tdx_test_fatal(ret); > + > + delta = data_in - data_out; > + if (delta != 1) > + tdx_test_fatal(ret); > + > + tdx_test_success(); > +} > + > +void verify_td_ioexit(void) > +{ > + struct kvm_vm *vm; > + struct kvm_vcpu *vcpu; > + > + uint32_t port_data; > + > + 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 */ > + td_vcpu_run(vcpu); > + TDX_TEST_CHECK_GUEST_FAILURE(vcpu); This check is a vain, because the first VMExit from vcpu run is always KVM_EXIT_IO caused by tdg_vp_vmcall_instruction_io(). > + TDX_TEST_ASSERT_IO(vcpu, TDX_IOEXIT_TEST_PORT, 1, > + TDG_VP_VMCALL_INSTRUCTION_IO_WRITE); > + port_data = *(uint8_t *)((void *)vcpu->run + vcpu->run->io.data_offset); > + > + printf("\t ... IO WRITE: OK\n"); So, even if there's an error in emulating writing of TDX_IOEXIT_TEST_PORT, and guest would then find a failure and trigger tdx_test_fatal(), this line will still print "IO WRITE: OK", which is not right. > + > + /* > + * Wait for the guest to do a IO read. Provide the previous written data > + * + 1 back to the guest > + */ > + td_vcpu_run(vcpu); > + TDX_TEST_CHECK_GUEST_FAILURE(vcpu); This check is a vain, too, as in write case. > + TDX_TEST_ASSERT_IO(vcpu, TDX_IOEXIT_TEST_PORT, 1, > + TDG_VP_VMCALL_INSTRUCTION_IO_READ); > + *(uint8_t *)((void *)vcpu->run + vcpu->run->io.data_offset) = port_data + 1; > + > + printf("\t ... IO READ: OK\n"); Same as in write case, this line should not be printed until after guest finishing checking return code. > + > + /* > + * Wait for the guest to complete execution successfully. The read > + * value is checked within the guest. > + */ > + td_vcpu_run(vcpu); > + TDX_TEST_CHECK_GUEST_FAILURE(vcpu); > + TDX_TEST_ASSERT_SUCCESS(vcpu); > + > + printf("\t ... IO verify read/write values: OK\n"); > + kvm_vm_free(vm); > + printf("\t ... PASSED\n"); > +} > + > int main(int argc, char **argv) > { > setbuf(stdout, NULL); > @@ -85,6 +166,7 @@ int main(int argc, char **argv) > > run_in_new_process(&verify_td_lifecycle); > run_in_new_process(&verify_report_fatal_error); > + run_in_new_process(&verify_td_ioexit); > > return 0; > } > -- > 2.43.0.472.g3155946c3a-goog > >
On 3/4/2024 10:19 AM, Yan Zhao wrote: > On Tue, Dec 12, 2023 at 12:46:25PM -0800, Sagi Shahar wrote: >> From: Erdem Aktas <erdemaktas@google.com> >> >> Verifies TDVMCALL<INSTRUCTION.IO> READ and WRITE operations. >> >> 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> >> --- >> .../kvm/include/x86_64/tdx/test_util.h | 34 ++++++++ >> .../selftests/kvm/x86_64/tdx_vm_tests.c | 82 +++++++++++++++++++ >> 2 files changed, 116 insertions(+) >> >> diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h b/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h >> index 6d69921136bd..95a5d5be7f0b 100644 >> --- a/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h >> +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h >> @@ -9,6 +9,40 @@ >> #define TDX_TEST_SUCCESS_PORT 0x30 >> #define TDX_TEST_SUCCESS_SIZE 4 >> >> +/** >> + * Assert that some IO operation involving tdg_vp_vmcall_instruction_io() was >> + * called in the guest. >> + */ >> +#define TDX_TEST_ASSERT_IO(VCPU, PORT, SIZE, DIR) \ >> + do { \ >> + 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) && \ >> + ((VCPU)->run->io.port == (PORT)) && \ >> + ((VCPU)->run->io.size == (SIZE)) && \ >> + ((VCPU)->run->io.direction == (DIR)), \ >> + "Got unexpected IO exit values: %u (%s) %d %d %d\n", \ >> + (VCPU)->run->exit_reason, \ >> + exit_reason_str((VCPU)->run->exit_reason), \ >> + (VCPU)->run->io.port, (VCPU)->run->io.size, \ >> + (VCPU)->run->io.direction); \ >> + } while (0) >> + >> +/** >> + * Check and report if there was some failure in the guest, either an exception >> + * like a triple fault, or if a tdx_test_fatal() was hit. >> + */ >> +#define TDX_TEST_CHECK_GUEST_FAILURE(VCPU) \ >> + do { \ >> + 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[1], \ >> + (VCPU)->run->system_event.data[1]); \ >> + } while (0) >> + >> /** >> * Assert that tdx_test_success() was called in the guest. >> */ >> 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 8638c7bbedaa..75467c407ca7 100644 >> --- a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c >> +++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c >> @@ -2,6 +2,7 @@ >> >> #include <signal.h> >> #include "kvm_util_base.h" >> +#include "tdx/tdcall.h" >> #include "tdx/tdx.h" >> #include "tdx/tdx_util.h" >> #include "tdx/test_util.h" >> @@ -74,6 +75,86 @@ void verify_report_fatal_error(void) >> 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 >> + */ >> +void guest_ioexit(void) >> +{ >> + uint64_t data_out, data_in, delta; >> + uint64_t ret; >> + >> + data_out = 0xAB; >> + ret = tdg_vp_vmcall_instruction_io(TDX_IOEXIT_TEST_PORT, 1, >> + TDG_VP_VMCALL_INSTRUCTION_IO_WRITE, >> + &data_out); >> + if (ret) >> + tdx_test_fatal(ret); >> + >> + ret = tdg_vp_vmcall_instruction_io(TDX_IOEXIT_TEST_PORT, 1, >> + TDG_VP_VMCALL_INSTRUCTION_IO_READ, >> + &data_in); >> + if (ret) >> + tdx_test_fatal(ret); >> + >> + delta = data_in - data_out; >> + if (delta != 1) >> + tdx_test_fatal(ret); >> + >> + tdx_test_success(); >> +} >> + >> +void verify_td_ioexit(void) >> +{ >> + struct kvm_vm *vm; >> + struct kvm_vcpu *vcpu; >> + >> + uint32_t port_data; >> + >> + 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 */ >> + td_vcpu_run(vcpu); >> + TDX_TEST_CHECK_GUEST_FAILURE(vcpu); > This check is a vain, because the first VMExit from vcpu run is always > KVM_EXIT_IO caused by tdg_vp_vmcall_instruction_io(). I think tdg_vp_vmcall_instruction_io() could fail if RCX (GPR select) doesn't meet the requirement (some bits must be 0). Although RCX is set by guest code (in selftest, it set in __tdx_hypercall()) and it will not trigger the error, it still can be used as a guard to make sure guest doesn't pass a invalid RCX. > > >> + TDX_TEST_ASSERT_IO(vcpu, TDX_IOEXIT_TEST_PORT, 1, >> + TDG_VP_VMCALL_INSTRUCTION_IO_WRITE); >> + port_data = *(uint8_t *)((void *)vcpu->run + vcpu->run->io.data_offset); >> + >> + printf("\t ... IO WRITE: OK\n"); > So, even if there's an error in emulating writing of TDX_IOEXIT_TEST_PORT, > and guest would then find a failure and trigger tdx_test_fatal(), this line > will still print "IO WRITE: OK", which is not right. > >> + >> + /* >> + * Wait for the guest to do a IO read. Provide the previous written data >> + * + 1 back to the guest >> + */ >> + td_vcpu_run(vcpu); >> + TDX_TEST_CHECK_GUEST_FAILURE(vcpu); > This check is a vain, too, as in write case. > >> + TDX_TEST_ASSERT_IO(vcpu, TDX_IOEXIT_TEST_PORT, 1, >> + TDG_VP_VMCALL_INSTRUCTION_IO_READ); >> + *(uint8_t *)((void *)vcpu->run + vcpu->run->io.data_offset) = port_data + 1; >> + >> + printf("\t ... IO READ: OK\n"); > Same as in write case, this line should not be printed until after guest > finishing checking return code. > >> + >> + /* >> + * Wait for the guest to complete execution successfully. The read >> + * value is checked within the guest. >> + */ >> + td_vcpu_run(vcpu); >> + TDX_TEST_CHECK_GUEST_FAILURE(vcpu); >> + TDX_TEST_ASSERT_SUCCESS(vcpu); >> + >> + printf("\t ... IO verify read/write values: OK\n"); >> + kvm_vm_free(vm); >> + printf("\t ... PASSED\n"); >> +} >> + >> int main(int argc, char **argv) >> { >> setbuf(stdout, NULL); >> @@ -85,6 +166,7 @@ int main(int argc, char **argv) >> >> run_in_new_process(&verify_td_lifecycle); >> run_in_new_process(&verify_report_fatal_error); >> + run_in_new_process(&verify_td_ioexit); >> >> return 0; >> } >> -- >> 2.43.0.472.g3155946c3a-goog >> >>
On Mon, Mar 04, 2024 at 05:16:53PM +0800, Binbin Wu wrote: > > > On 3/4/2024 10:19 AM, Yan Zhao wrote: > > On Tue, Dec 12, 2023 at 12:46:25PM -0800, Sagi Shahar wrote: > > > From: Erdem Aktas <erdemaktas@google.com> > > > > > > Verifies TDVMCALL<INSTRUCTION.IO> READ and WRITE operations. > > > > > > 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> > > > --- > > > .../kvm/include/x86_64/tdx/test_util.h | 34 ++++++++ > > > .../selftests/kvm/x86_64/tdx_vm_tests.c | 82 +++++++++++++++++++ > > > 2 files changed, 116 insertions(+) > > > > > > diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h b/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h > > > index 6d69921136bd..95a5d5be7f0b 100644 > > > --- a/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h > > > +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h > > > @@ -9,6 +9,40 @@ > > > #define TDX_TEST_SUCCESS_PORT 0x30 > > > #define TDX_TEST_SUCCESS_SIZE 4 > > > +/** > > > + * Assert that some IO operation involving tdg_vp_vmcall_instruction_io() was > > > + * called in the guest. > > > + */ > > > +#define TDX_TEST_ASSERT_IO(VCPU, PORT, SIZE, DIR) \ > > > + do { \ > > > + 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) && \ > > > + ((VCPU)->run->io.port == (PORT)) && \ > > > + ((VCPU)->run->io.size == (SIZE)) && \ > > > + ((VCPU)->run->io.direction == (DIR)), \ > > > + "Got unexpected IO exit values: %u (%s) %d %d %d\n", \ > > > + (VCPU)->run->exit_reason, \ > > > + exit_reason_str((VCPU)->run->exit_reason), \ > > > + (VCPU)->run->io.port, (VCPU)->run->io.size, \ > > > + (VCPU)->run->io.direction); \ > > > + } while (0) > > > + > > > +/** > > > + * Check and report if there was some failure in the guest, either an exception > > > + * like a triple fault, or if a tdx_test_fatal() was hit. > > > + */ > > > +#define TDX_TEST_CHECK_GUEST_FAILURE(VCPU) \ > > > + do { \ > > > + 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[1], \ > > > + (VCPU)->run->system_event.data[1]); \ > > > + } while (0) > > > + > > > /** > > > * Assert that tdx_test_success() was called in the guest. > > > */ > > > 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 8638c7bbedaa..75467c407ca7 100644 > > > --- a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c > > > +++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c > > > @@ -2,6 +2,7 @@ > > > #include <signal.h> > > > #include "kvm_util_base.h" > > > +#include "tdx/tdcall.h" > > > #include "tdx/tdx.h" > > > #include "tdx/tdx_util.h" > > > #include "tdx/test_util.h" > > > @@ -74,6 +75,86 @@ void verify_report_fatal_error(void) > > > 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 > > > + */ > > > +void guest_ioexit(void) > > > +{ > > > + uint64_t data_out, data_in, delta; > > > + uint64_t ret; > > > + > > > + data_out = 0xAB; > > > + ret = tdg_vp_vmcall_instruction_io(TDX_IOEXIT_TEST_PORT, 1, > > > + TDG_VP_VMCALL_INSTRUCTION_IO_WRITE, > > > + &data_out); > > > + if (ret) > > > + tdx_test_fatal(ret); > > > + > > > + ret = tdg_vp_vmcall_instruction_io(TDX_IOEXIT_TEST_PORT, 1, > > > + TDG_VP_VMCALL_INSTRUCTION_IO_READ, > > > + &data_in); > > > + if (ret) > > > + tdx_test_fatal(ret); > > > + > > > + delta = data_in - data_out; > > > + if (delta != 1) > > > + tdx_test_fatal(ret); > > > + > > > + tdx_test_success(); > > > +} > > > + > > > +void verify_td_ioexit(void) > > > +{ > > > + struct kvm_vm *vm; > > > + struct kvm_vcpu *vcpu; > > > + > > > + uint32_t port_data; > > > + > > > + 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 */ > > > + td_vcpu_run(vcpu); > > > + TDX_TEST_CHECK_GUEST_FAILURE(vcpu); > > This check is a vain, because the first VMExit from vcpu run is always > > KVM_EXIT_IO caused by tdg_vp_vmcall_instruction_io(). > > I think tdg_vp_vmcall_instruction_io() could fail if RCX (GPR select) > doesn't > meet the requirement (some bits must be 0). > Although RCX is set by guest code (in selftest, it set in __tdx_hypercall()) > and it will not trigger the error, it still can be used as a guard to make > sure guest doesn't pass a invalid RCX. > Right. This check can be kept in case an failure is delivered to TD in host kernel directly, though it cannot guard if the failure will trigger an exit to user space (e.g. if kernel TDX code has a bug). > > > > > > > > + TDX_TEST_ASSERT_IO(vcpu, TDX_IOEXIT_TEST_PORT, 1, > > > + TDG_VP_VMCALL_INSTRUCTION_IO_WRITE); > > > + port_data = *(uint8_t *)((void *)vcpu->run + vcpu->run->io.data_offset); > > > + > > > + printf("\t ... IO WRITE: OK\n"); > > So, even if there's an error in emulating writing of TDX_IOEXIT_TEST_PORT, > > and guest would then find a failure and trigger tdx_test_fatal(), this line > > will still print "IO WRITE: OK", which is not right. > > > > > + > > > + /* > > > + * Wait for the guest to do a IO read. Provide the previous written data > > > + * + 1 back to the guest > > > + */ > > > + td_vcpu_run(vcpu); > > > + TDX_TEST_CHECK_GUEST_FAILURE(vcpu); > > This check is a vain, too, as in write case. > > > > > + TDX_TEST_ASSERT_IO(vcpu, TDX_IOEXIT_TEST_PORT, 1, > > > + TDG_VP_VMCALL_INSTRUCTION_IO_READ); > > > + *(uint8_t *)((void *)vcpu->run + vcpu->run->io.data_offset) = port_data + 1; > > > + > > > + printf("\t ... IO READ: OK\n"); > > Same as in write case, this line should not be printed until after guest > > finishing checking return code. > > > > > + > > > + /* > > > + * Wait for the guest to complete execution successfully. The read > > > + * value is checked within the guest. > > > + */ > > > + td_vcpu_run(vcpu); > > > + TDX_TEST_CHECK_GUEST_FAILURE(vcpu); > > > + TDX_TEST_ASSERT_SUCCESS(vcpu); > > > + > > > + printf("\t ... IO verify read/write values: OK\n"); > > > + kvm_vm_free(vm); > > > + printf("\t ... PASSED\n"); > > > +} > > > + > > > int main(int argc, char **argv) > > > { > > > setbuf(stdout, NULL); > > > @@ -85,6 +166,7 @@ int main(int argc, char **argv) > > > run_in_new_process(&verify_td_lifecycle); > > > run_in_new_process(&verify_report_fatal_error); > > > + run_in_new_process(&verify_td_ioexit); > > > return 0; > > > } > > > -- > > > 2.43.0.472.g3155946c3a-goog > > > > > > >
On Sun, Mar 3, 2024 at 8:49 PM Yan Zhao <yan.y.zhao@intel.com> wrote: > > On Tue, Dec 12, 2023 at 12:46:25PM -0800, Sagi Shahar wrote: > > From: Erdem Aktas <erdemaktas@google.com> > > > > Verifies TDVMCALL<INSTRUCTION.IO> READ and WRITE operations. > > > > 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> > > --- > > .../kvm/include/x86_64/tdx/test_util.h | 34 ++++++++ > > .../selftests/kvm/x86_64/tdx_vm_tests.c | 82 +++++++++++++++++++ > > 2 files changed, 116 insertions(+) > > > > diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h b/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h > > index 6d69921136bd..95a5d5be7f0b 100644 > > --- a/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h > > +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h > > @@ -9,6 +9,40 @@ > > #define TDX_TEST_SUCCESS_PORT 0x30 > > #define TDX_TEST_SUCCESS_SIZE 4 > > > > +/** > > + * Assert that some IO operation involving tdg_vp_vmcall_instruction_io() was > > + * called in the guest. > > + */ > > +#define TDX_TEST_ASSERT_IO(VCPU, PORT, SIZE, DIR) \ > > + do { \ > > + 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) && \ > > + ((VCPU)->run->io.port == (PORT)) && \ > > + ((VCPU)->run->io.size == (SIZE)) && \ > > + ((VCPU)->run->io.direction == (DIR)), \ > > + "Got unexpected IO exit values: %u (%s) %d %d %d\n", \ > > + (VCPU)->run->exit_reason, \ > > + exit_reason_str((VCPU)->run->exit_reason), \ > > + (VCPU)->run->io.port, (VCPU)->run->io.size, \ > > + (VCPU)->run->io.direction); \ > > + } while (0) > > + > > +/** > > + * Check and report if there was some failure in the guest, either an exception > > + * like a triple fault, or if a tdx_test_fatal() was hit. > > + */ > > +#define TDX_TEST_CHECK_GUEST_FAILURE(VCPU) \ > > + do { \ > > + 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[1], \ > > + (VCPU)->run->system_event.data[1]); \ > > + } while (0) > > + > > /** > > * Assert that tdx_test_success() was called in the guest. > > */ > > 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 8638c7bbedaa..75467c407ca7 100644 > > --- a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c > > +++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c > > @@ -2,6 +2,7 @@ > > > > #include <signal.h> > > #include "kvm_util_base.h" > > +#include "tdx/tdcall.h" > > #include "tdx/tdx.h" > > #include "tdx/tdx_util.h" > > #include "tdx/test_util.h" > > @@ -74,6 +75,86 @@ void verify_report_fatal_error(void) > > 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 > > + */ > > +void guest_ioexit(void) > > +{ > > + uint64_t data_out, data_in, delta; > > + uint64_t ret; > > + > > + data_out = 0xAB; > > + ret = tdg_vp_vmcall_instruction_io(TDX_IOEXIT_TEST_PORT, 1, > > + TDG_VP_VMCALL_INSTRUCTION_IO_WRITE, > > + &data_out); > > + if (ret) > > + tdx_test_fatal(ret); > > + > > + ret = tdg_vp_vmcall_instruction_io(TDX_IOEXIT_TEST_PORT, 1, > > + TDG_VP_VMCALL_INSTRUCTION_IO_READ, > > + &data_in); > > + if (ret) > > + tdx_test_fatal(ret); > > + > > + delta = data_in - data_out; > > + if (delta != 1) > > + tdx_test_fatal(ret); > > + > > + tdx_test_success(); > > +} > > + > > +void verify_td_ioexit(void) > > +{ > > + struct kvm_vm *vm; > > + struct kvm_vcpu *vcpu; > > + > > + uint32_t port_data; > > + > > + 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 */ > > + td_vcpu_run(vcpu); > > + TDX_TEST_CHECK_GUEST_FAILURE(vcpu); > This check is a vain, because the first VMExit from vcpu run is always > KVM_EXIT_IO caused by tdg_vp_vmcall_instruction_io(). > > > > + TDX_TEST_ASSERT_IO(vcpu, TDX_IOEXIT_TEST_PORT, 1, > > + TDG_VP_VMCALL_INSTRUCTION_IO_WRITE); > > + port_data = *(uint8_t *)((void *)vcpu->run + vcpu->run->io.data_offset); > > + > > + printf("\t ... IO WRITE: OK\n"); > So, even if there's an error in emulating writing of TDX_IOEXIT_TEST_PORT, > and guest would then find a failure and trigger tdx_test_fatal(), this line > will still print "IO WRITE: OK", which is not right. Changed this to "IO WRITE: DONE". This is a useful for understanding errors if they happen so I don't want to remove this log entirely. > > > + > > + /* > > + * Wait for the guest to do a IO read. Provide the previous written data > > + * + 1 back to the guest > > + */ > > + td_vcpu_run(vcpu); > > + TDX_TEST_CHECK_GUEST_FAILURE(vcpu); > This check is a vain, too, as in write case. There's no harm in adding this check and it helps debugging issues if something unexpected happens. Since this check always come together with td_vcpu_run I'm going to simplify the code and introduce a TDX_RUN macro that simply runs the CPU and checks for reported guest failures which is going to replace the td_vcpu_run/TDX_TEST_CHECK_GUEST_FAILURE combo. > > > + TDX_TEST_ASSERT_IO(vcpu, TDX_IOEXIT_TEST_PORT, 1, > > + TDG_VP_VMCALL_INSTRUCTION_IO_READ); > > + *(uint8_t *)((void *)vcpu->run + vcpu->run->io.data_offset) = port_data + 1; > > + > > + printf("\t ... IO READ: OK\n"); > Same as in write case, this line should not be printed until after guest > finishing checking return code. > > > + > > + /* > > + * Wait for the guest to complete execution successfully. The read > > + * value is checked within the guest. > > + */ > > + td_vcpu_run(vcpu); > > + TDX_TEST_CHECK_GUEST_FAILURE(vcpu); > > + TDX_TEST_ASSERT_SUCCESS(vcpu); > > + > > + printf("\t ... IO verify read/write values: OK\n"); > > + kvm_vm_free(vm); > > + printf("\t ... PASSED\n"); > > +} > > + > > int main(int argc, char **argv) > > { > > setbuf(stdout, NULL); > > @@ -85,6 +166,7 @@ int main(int argc, char **argv) > > > > run_in_new_process(&verify_td_lifecycle); > > run_in_new_process(&verify_report_fatal_error); > > + run_in_new_process(&verify_td_ioexit); > > > > return 0; > > } > > -- > > 2.43.0.472.g3155946c3a-goog > > > > >
diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h b/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h index 6d69921136bd..95a5d5be7f0b 100644 --- a/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h @@ -9,6 +9,40 @@ #define TDX_TEST_SUCCESS_PORT 0x30 #define TDX_TEST_SUCCESS_SIZE 4 +/** + * Assert that some IO operation involving tdg_vp_vmcall_instruction_io() was + * called in the guest. + */ +#define TDX_TEST_ASSERT_IO(VCPU, PORT, SIZE, DIR) \ + do { \ + 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) && \ + ((VCPU)->run->io.port == (PORT)) && \ + ((VCPU)->run->io.size == (SIZE)) && \ + ((VCPU)->run->io.direction == (DIR)), \ + "Got unexpected IO exit values: %u (%s) %d %d %d\n", \ + (VCPU)->run->exit_reason, \ + exit_reason_str((VCPU)->run->exit_reason), \ + (VCPU)->run->io.port, (VCPU)->run->io.size, \ + (VCPU)->run->io.direction); \ + } while (0) + +/** + * Check and report if there was some failure in the guest, either an exception + * like a triple fault, or if a tdx_test_fatal() was hit. + */ +#define TDX_TEST_CHECK_GUEST_FAILURE(VCPU) \ + do { \ + 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[1], \ + (VCPU)->run->system_event.data[1]); \ + } while (0) + /** * Assert that tdx_test_success() was called in the guest. */ 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 8638c7bbedaa..75467c407ca7 100644 --- a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c +++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c @@ -2,6 +2,7 @@ #include <signal.h> #include "kvm_util_base.h" +#include "tdx/tdcall.h" #include "tdx/tdx.h" #include "tdx/tdx_util.h" #include "tdx/test_util.h" @@ -74,6 +75,86 @@ void verify_report_fatal_error(void) 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 + */ +void guest_ioexit(void) +{ + uint64_t data_out, data_in, delta; + uint64_t ret; + + data_out = 0xAB; + ret = tdg_vp_vmcall_instruction_io(TDX_IOEXIT_TEST_PORT, 1, + TDG_VP_VMCALL_INSTRUCTION_IO_WRITE, + &data_out); + if (ret) + tdx_test_fatal(ret); + + ret = tdg_vp_vmcall_instruction_io(TDX_IOEXIT_TEST_PORT, 1, + TDG_VP_VMCALL_INSTRUCTION_IO_READ, + &data_in); + if (ret) + tdx_test_fatal(ret); + + delta = data_in - data_out; + if (delta != 1) + tdx_test_fatal(ret); + + tdx_test_success(); +} + +void verify_td_ioexit(void) +{ + struct kvm_vm *vm; + struct kvm_vcpu *vcpu; + + uint32_t port_data; + + 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 */ + td_vcpu_run(vcpu); + TDX_TEST_CHECK_GUEST_FAILURE(vcpu); + TDX_TEST_ASSERT_IO(vcpu, TDX_IOEXIT_TEST_PORT, 1, + TDG_VP_VMCALL_INSTRUCTION_IO_WRITE); + port_data = *(uint8_t *)((void *)vcpu->run + vcpu->run->io.data_offset); + + printf("\t ... IO WRITE: OK\n"); + + /* + * Wait for the guest to do a IO read. Provide the previous written data + * + 1 back to the guest + */ + td_vcpu_run(vcpu); + TDX_TEST_CHECK_GUEST_FAILURE(vcpu); + TDX_TEST_ASSERT_IO(vcpu, TDX_IOEXIT_TEST_PORT, 1, + TDG_VP_VMCALL_INSTRUCTION_IO_READ); + *(uint8_t *)((void *)vcpu->run + vcpu->run->io.data_offset) = port_data + 1; + + printf("\t ... IO READ: OK\n"); + + /* + * Wait for the guest to complete execution successfully. The read + * value is checked within the guest. + */ + td_vcpu_run(vcpu); + TDX_TEST_CHECK_GUEST_FAILURE(vcpu); + TDX_TEST_ASSERT_SUCCESS(vcpu); + + printf("\t ... IO verify read/write values: OK\n"); + kvm_vm_free(vm); + printf("\t ... PASSED\n"); +} + int main(int argc, char **argv) { setbuf(stdout, NULL); @@ -85,6 +166,7 @@ int main(int argc, char **argv) run_in_new_process(&verify_td_lifecycle); run_in_new_process(&verify_report_fatal_error); + run_in_new_process(&verify_td_ioexit); return 0; }