On Thu, Aug 14, 2025 at 2:05 AM Binbin Wu <binbin...@linux.intel.com> wrote: > > > > On 8/13/2025 6:58 PM, Binbin Wu wrote: > > > > > > On 8/8/2025 4:16 AM, Sagi Shahar wrote: > >> The test checks report_fatal_error functionality. > >> > >> TD guest can use TDG.VP.VMCALL<ReportFatalError> to report the fatal error > >> it has experienced. TD guest is requesting a termination with the error > >> information that include 16 general-purpose registers. > > > > I think it's worth to mention that KVM converts > > TDG.VP.VMCALL<ReportFatalError> > > to KVM_EXIT_SYSTEM_EVENT with the type KVM_SYSTEM_EVENT_TDX_FATAL. > > > >> > >> Co-developed-by: Binbin Wu <binbin...@linux.intel.com> > >> Signed-off-by: Binbin Wu <binbin...@linux.intel.com> > >> Signed-off-by: Sagi Shahar <sa...@google.com> > >> --- > >> .../selftests/kvm/include/x86/tdx/tdx.h | 6 ++- > >> .../selftests/kvm/include/x86/tdx/tdx_util.h | 1 + > >> .../selftests/kvm/include/x86/tdx/test_util.h | 19 +++++++ > >> tools/testing/selftests/kvm/lib/x86/tdx/tdx.c | 18 +++++++ > >> .../selftests/kvm/lib/x86/tdx/tdx_util.c | 6 +++ > >> .../selftests/kvm/lib/x86/tdx/test_util.c | 10 ++++ > >> tools/testing/selftests/kvm/x86/tdx_vm_test.c | 51 ++++++++++++++++++- > >> 7 files changed, 108 insertions(+), 3 deletions(-) > >> > >> diff --git a/tools/testing/selftests/kvm/include/x86/tdx/tdx.h > >> b/tools/testing/selftests/kvm/include/x86/tdx/tdx.h > >> index a7161efe4ee2..2acccc9dccf9 100644 > >> --- a/tools/testing/selftests/kvm/include/x86/tdx/tdx.h > >> +++ b/tools/testing/selftests/kvm/include/x86/tdx/tdx.h > >> @@ -4,9 +4,13 @@ > >> #include <stdint.h> > >> +#include "kvm_util.h" > >> + > >> +#define TDG_VP_VMCALL_REPORT_FATAL_ERROR 0x10003 > >> + > >> #define TDG_VP_VMCALL_INSTRUCTION_IO 30 > >> uint64_t tdg_vp_vmcall_instruction_io(uint64_t port, uint64_t size, > >> uint64_t write, uint64_t *data); > >> - > >> +void tdg_vp_vmcall_report_fatal_error(uint64_t error_code, uint64_t > >> data_gpa); > >> #endif // SELFTEST_TDX_TDX_H > >> diff --git a/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h > >> b/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h > >> index 57a2f5893ffe..d66cf17f03ea 100644 > >> --- a/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h > >> +++ b/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h > >> @@ -15,5 +15,6 @@ struct kvm_vm *td_create(void); > >> void td_initialize(struct kvm_vm *vm, enum vm_mem_backing_src_type > >> src_type, > >> uint64_t attributes); > >> void td_finalize(struct kvm_vm *vm); > >> +void td_vcpu_run(struct kvm_vcpu *vcpu); > >> #endif // SELFTESTS_TDX_KVM_UTIL_H > >> 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 07d63bf1ffe1..dafeee9af1dc 100644 > >> --- a/tools/testing/selftests/kvm/include/x86/tdx/test_util.h > >> +++ b/tools/testing/selftests/kvm/include/x86/tdx/test_util.h > >> @@ -38,4 +38,23 @@ bool is_tdx_enabled(void); > >> void tdx_test_success(void); > >> void tdx_test_assert_success(struct kvm_vcpu *vcpu); > >> +/* > >> + * Report an error with @error_code 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_test_fatal(uint64_t error_code); > > Another thing to mention is that tdx_test_fatal() and > tdx_test_fatal_with_data() > use R12 to pass the input error_code, which is functionally workable, since > both > guest and userspace code are in KVM selftest test code. > > But TDX GHCI spec has its own format for R12: > - 31:0 > TD-specific error code > * Panic – 0x0. > * Values – 0x1 to 0xFFFFFFFF reserved. > - 62:32 > TD-specific extended error code. > TD software defined. > - 63 > Set if the TD specified additional information in the GPA parameter (R13). > So, this patch series doesn't follow the format. > > Also, tdx_test_fatal_with_data() set bit 63 of R12, so, the value reported to > userspace will be different in R12 from the original parameter passed by the > guest, and setting bit 63 could collide with the error code defined by guest. > > IMHO, it's better to follow the GHCI spec. > But if TDX KVM selftest code doesn't want to follow it, then it should not set > bit 63 for tdx_test_fatal_with_data() in R12. >
Other than the fact that the address pointed to by data_gpa is a fake address, the rest of the implementation should follow the spec. And even on a normal system, TDX doesn't guarantee that the address provided by the guest is valid, it will simply send it to userspace as is. I can replace the setting of bit 63 with a GUEST_ASSERT to verify it is set if data_gpa is provided by the guest. > >> > >> + > >> +/* > >> + * Report an error with @error_code to userspace. > >> + * > >> + * @data_gpa may point to an optional shared guest memory holding the > >> error > >> + * string. > > > > A according to the GHCI spec, this is the optional GPA pointing to a shared > > guest memory, but in these TDX KVM selftest cases, it may not used that > > way. It may need some clarification about it. And based on the usage in > > this patch series, the name data_gpa may be misleading. > > > > > >> + * > >> + * Return value from tdg_vp_vmcall_report_fatal_error() is ignored since > >> + * execution is not expected to continue beyond this point. > >> + */ > >> +void tdx_test_fatal_with_data(uint64_t error_code, uint64_t data_gpa); > >> + > [...]
