Re: [PATCH v8 10/30] KVM: selftests: TDX: Add report_fatal_error test

[Sagi Shahar <sagis@google.com>]

On Wed, Aug 13, 2025 at 5:58 AM Binbin Wu <binbin.wu@linux.intel.com> 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.
>

Done. Will get updated in the next version.

> >
> > Co-developed-by: Binbin Wu <binbin.wu@linux.intel.com>
> > Signed-off-by: Binbin Wu <binbin.wu@linux.intel.com>
> > Signed-off-by: Sagi Shahar <sagis@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);
> > +
> > +/*
> > + * 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.
>

I can add a comment at the call site saying that the data_gpa is a
fake one for the sake of testing. Whether the data_gpa points to a
valid shared memory or not doesn't make a lot of difference from a
unit test perspective since we are testing the ReportFatalError
functionality itself.

Making the data_gpa valid for the test requires setting up additional
shared memory with no significant benefit for the test.

>
> > + *
> > + * 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);
> > +
> >   #endif // SELFTEST_TDX_TEST_UTIL_H
> > diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdx.c b/tools/testing/selftests/kvm/lib/x86/tdx/tdx.c
> > index f417ee75bee2..ba088bfc1e62 100644
> > --- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx.c
> > +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx.c
> > @@ -1,5 +1,7 @@
> >   // SPDX-License-Identifier: GPL-2.0-only
> >
> > +#include <string.h>
> > +
> >   #include "tdx/tdcall.h"
> >   #include "tdx/tdx.h"
> >
> > @@ -25,3 +27,19 @@ uint64_t tdg_vp_vmcall_instruction_io(uint64_t port, uint64_t size,
> >
> >       return ret;
> >   }
> > +
> > +void tdg_vp_vmcall_report_fatal_error(uint64_t error_code, uint64_t data_gpa)
> > +{
> > +     struct tdx_hypercall_args args;
> > +
> > +     memset(&args, 0, sizeof(struct tdx_hypercall_args));
> > +
> > +     if (data_gpa)
> > +             error_code |= 0x8000000000000000;
> > +
> > +     args.r11 = TDG_VP_VMCALL_REPORT_FATAL_ERROR;
> > +     args.r12 = error_code;
> > +     args.r13 = data_gpa;
> > +
> > +     __tdx_hypercall(&args, 0);
> > +}
> > diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> > index e2bf9766dc03..5e4455be828a 100644
> > --- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> > +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> > @@ -9,6 +9,7 @@
> >   #include "kvm_util.h"
> >   #include "processor.h"
> >   #include "tdx/td_boot.h"
> > +#include "tdx/tdx.h"
> >   #include "test_util.h"
> >
> >   uint64_t tdx_s_bit;
> > @@ -603,3 +604,8 @@ void td_finalize(struct kvm_vm *vm)
> >
> >       tdx_td_finalize_mr(vm);
> >   }
> > +
> > +void td_vcpu_run(struct kvm_vcpu *vcpu)
> > +{
> > +     vcpu_run(vcpu);
> > +}
> > 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 7355b213c344..6c82a0c3bd37 100644
> > --- a/tools/testing/selftests/kvm/lib/x86/tdx/test_util.c
> > +++ b/tools/testing/selftests/kvm/lib/x86/tdx/test_util.c
> > @@ -59,3 +59,13 @@ void tdx_test_assert_success(struct kvm_vcpu *vcpu)
> >                   vcpu->run->io.port, vcpu->run->io.size,
> >                   vcpu->run->io.direction);
> >   }
> > +
> > +void tdx_test_fatal_with_data(uint64_t error_code, uint64_t data_gpa)
> > +{
> > +     tdg_vp_vmcall_report_fatal_error(error_code, data_gpa);
> > +}
> > +
> > +void tdx_test_fatal(uint64_t error_code)
> > +{
> > +     tdx_test_fatal_with_data(error_code, 0);
> > +}
> > diff --git a/tools/testing/selftests/kvm/x86/tdx_vm_test.c b/tools/testing/selftests/kvm/x86/tdx_vm_test.c
> > index fdb7c40065a6..7d6d71602761 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/tdx.h"
> >   #include "tdx/tdx_util.h"
> >   #include "tdx/test_util.h"
> >   #include "test_util.h"
> > @@ -24,7 +25,51 @@ static void verify_td_lifecycle(void)
> >
> >       printf("Verifying TD lifecycle:\n");
> >
> > -     vcpu_run(vcpu);
> > +     td_vcpu_run(vcpu);
> > +     tdx_test_assert_success(vcpu);
> > +
> > +     kvm_vm_free(vm);
> > +     printf("\t ... PASSED\n");
> > +}
> > +
> > +void guest_code_report_fatal_error(void)
> > +{
> > +     uint64_t err;
> > +
> > +     /*
> > +      * Note: err should follow the GHCI spec definition:
> > +      * bits 31:0 should be set to 0.
> > +      * bits 62:32 are used for TD-specific extended error code.
> > +      * bit 63 is used to mark additional information in shared memory.
> > +      */
> > +     err = 0x0BAAAAAD00000000;
> > +     tdx_test_fatal(err);
> > +
> > +     tdx_test_success();
> > +}
> > +
> > +void verify_report_fatal_error(void)
> > +{
> > +     struct kvm_vcpu *vcpu;
> > +     struct kvm_vm *vm;
> > +
> > +     vm = td_create();
> > +     td_initialize(vm, VM_MEM_SRC_ANONYMOUS, 0);
> > +     vcpu = td_vcpu_add(vm, 0, guest_code_report_fatal_error);
> > +     td_finalize(vm);
> > +
> > +     printf("Verifying report_fatal_error:\n");
> > +
> > +     td_vcpu_run(vcpu);
> > +
> > +     TEST_ASSERT_EQ(vcpu->run->exit_reason, KVM_EXIT_SYSTEM_EVENT);
> > +     TEST_ASSERT_EQ(vcpu->run->system_event.type, KVM_SYSTEM_EVENT_TDX_FATAL);
> > +     TEST_ASSERT_EQ(vcpu->run->system_event.ndata, 16);
> > +
> > +     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_test_assert_success(vcpu);
> >
> >       kvm_vm_free(vm);
> > @@ -38,9 +83,11 @@ 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(1);
> > +     ksft_set_plan(2);
> >       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_finished();
> >       return 0;
>