Xiaoyao Li <xiaoyao.li@xxxxxxxxx> writes: > On 2/27/2024 7:51 PM, Markus Armbruster wrote: >> Xiaoyao Li <xiaoyao.li@xxxxxxxxx> writes: >> >>> On 2/19/2024 8:53 PM, Markus Armbruster wrote: >>>> Xiaoyao Li <xiaoyao.li@xxxxxxxxx> writes: >>>> >>>>> Integrate TDX's TDX_REPORT_FATAL_ERROR into QEMU GuestPanic facility >>>>> >>>>> Originated-from: Isaku Yamahata <isaku.yamahata@xxxxxxxxx> >>>>> Signed-off-by: Xiaoyao Li <xiaoyao.li@xxxxxxxxx> >>>>> --- >>>>> Changes in v4: >>>>> - refine the documentation; (Markus) >>>>> >>>>> Changes in v3: >>>>> - Add docmentation of new type and struct; (Daniel) >>>>> - refine the error message handling; (Daniel) >>>>> --- >>>>> qapi/run-state.json | 28 ++++++++++++++++++++-- >>>>> system/runstate.c | 54 +++++++++++++++++++++++++++++++++++++++++++ >>>>> target/i386/kvm/tdx.c | 24 ++++++++++++++++++- >>>>> 3 files changed, 103 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/qapi/run-state.json b/qapi/run-state.json >>>>> index 08bc99cb8561..5429116679e3 100644 >>>>> --- a/qapi/run-state.json >>>>> +++ b/qapi/run-state.json [...] >>>>> @@ -566,6 +569,27 @@ >>>>> 'psw-addr': 'uint64', >>>>> 'reason': 'S390CrashReason'}} >>>>> >>>>> +## >>>>> +# @GuestPanicInformationTdx: >>>>> +# >>>>> +# TDX Guest panic information specific to TDX GCHI >>>>> +# TDG.VP.VMCALL<ReportFatalError>. >>>>> +# >>>>> +# @error-code: TD-specific error code >>>> >>>> Where could a user find information on these error codes? >>> >>> TDX GHCI (Guset-host-communication-Interface)spec. It defines all the >>> TDVMCALL leaves. >>> >>> 0: panic; >>> 0x1 - 0xffffffff: reserved. >> >> Would it make sense to add a reference? > > https://cdrdv2.intel.com/v1/dl/getContent/726792 URLs have this annoying tendency to rot. What about # @error-code: Error code as defined in "Guest-Hypervisor Communication # Interface (GHCI) Specification for Intel TDX 1.5" >>>>> +# >>>>> +# @gpa: guest-physical address of a page that contains additional >>>>> +# error data, in forms of zero-terminated string. >>>> >>>> "in the form of a zero-terminated string" >>> >>> fixed. >>> >>>>> +# >>>>> +# @message: Human-readable error message provided by the guest. Not >>>>> +# to be trusted. >>>> >>>> How is this message related to the one pointed to by @gpa? >>> >>> In general, @message contains a brief message of the error. While @gpa >>> (when valid) contains a verbose message. >>> >>> The reason why we need both is because sometime when TD guest hits a >>> fatal error, its memory may get corrupted so we cannot pass information >>> via @gpa. Information in @message is passed through GPRs. >> >> Well, we do pass information via @gpa, always. I guess it page's >> contents can be corrupted. > > No. It's not always. the bit 63 of the error code is "GPA valid" bit. > @gpa is valid only when bit 63 of error code is 1. > > And current Linux TD guest implementation doesn't use @gpa at all. > https://github.com/torvalds/linux/blob/45ec2f5f6ed3ec3a79ba1329ad585497cdcbe663/arch/x86/coco/tdx/tdx.c#L131 Aha! Why would we want to include @gpa when the "GPA valid" bit is off? If we do want it, then # @gpa: guest-physical address of a page that contains more verbose # error information, as zero-terminated string. Valid when the # "GPA valid" bit is set in @error-code. If we don't, then make @gpa optional, present when valid, and document it like this: # @gpa: guest-physical address of a page that contains more verbose # error information, as zero-terminated string. Present when the # "GPA valid" bit is set in @error-code. >> Perhaps something like >> >> # @message: Human-readable error message provided by the guest. Not >> # to be trusted. >> # >> # @gpa: guest-physical address of a page that contains more verbose >> # error information, as zero-terminated string. Note that guest >> # memory corruption can corrupt the page's contents. >> >>>>> +# >>>>> +# Since: 9.0 >>>>> +## >>>>> +{'struct': 'GuestPanicInformationTdx', >>>>> + 'data': {'error-code': 'uint64', >>>>> + 'gpa': 'uint64', >>>>> + 'message': 'str'}} >> >> Note that my proposed doc string has the members in a different order. >> Recommend to use the same order here. >> >>>>> + >>>>> ## >>>>> # @MEMORY_FAILURE: >>>>> # >>>> >>>> [...] >>>> >>