Xiaoyao Li <[email protected]> writes:
> On 2/19/2024 8:53 PM, Markus Armbruster wrote:
>> Xiaoyao Li <[email protected]> writes:
>>
>>> Integrate TDX's TDX_REPORT_FATAL_ERROR into QEMU GuestPanic facility
>>>
>>> Originated-from: Isaku Yamahata <[email protected]>
>>> Signed-off-by: Xiaoyao Li <[email protected]>
>>> ---
>>> 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
>>> @@ -485,10 +485,12 @@
>>> #
>>> # @s390: s390 guest panic information type (Since: 2.12)
>>> #
>>> +# @tdx: tdx guest panic information type (Since: 8.2)
>>> +#
>>> # Since: 2.9
>>> ##
>>> { 'enum': 'GuestPanicInformationType',
>>> - 'data': [ 'hyper-v', 's390' ] }
>>> + 'data': [ 'hyper-v', 's390', 'tdx' ] }
>>>
>>> ##
>>> # @GuestPanicInformation:
>>> @@ -503,7 +505,8 @@
>>> 'base': {'type': 'GuestPanicInformationType'},
>>> 'discriminator': 'type',
>>> 'data': {'hyper-v': 'GuestPanicInformationHyperV',
>>> - 's390': 'GuestPanicInformationS390'}}
>>> + 's390': 'GuestPanicInformationS390',
>>> + 'tdx' : 'GuestPanicInformationTdx'}}
>>>
>>> ##
>>> # @GuestPanicInformationHyperV:
>>> @@ -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?
>>> +#
>>> +# @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.
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:
>>> #
>>
>> [...]
>>