Xiaoyao Li <xiaoyao.li@xxxxxxxxx> writes:
> On 2/29/2024 4:51 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 v5:
>>> - mention additional error information in gpa when it presents;
>>> - refine the documentation; (Markus)
>>>
>>> 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 | 31 +++++++++++++++++++++--
>>> system/runstate.c | 58 +++++++++++++++++++++++++++++++++++++++++++
>>> target/i386/kvm/tdx.c | 24 +++++++++++++++++-
>>> 3 files changed, 110 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/qapi/run-state.json b/qapi/run-state.json
>>> index dd0770b379e5..b71dd1884eb6 100644
>>> --- a/qapi/run-state.json
>>> +++ b/qapi/run-state.json
>>> @@ -483,10 +483,12 @@
>>> #
>>> # @s390: s390 guest panic information type (Since: 2.12)
>>> #
>>> +# @tdx: tdx guest panic information type (Since: 9.0)
>>> +#
>>> # Since: 2.9
>>> ##
>>> { 'enum': 'GuestPanicInformationType',
>>> - 'data': [ 'hyper-v', 's390' ] }
>>> + 'data': [ 'hyper-v', 's390', 'tdx' ] }
>>> ##
>>> # @GuestPanicInformation:
>>> @@ -501,7 +503,8 @@
>>> 'base': {'type': 'GuestPanicInformationType'},
>>> 'discriminator': 'type',
>>> 'data': {'hyper-v': 'GuestPanicInformationHyperV',
>>> - 's390': 'GuestPanicInformationS390'}}
>>> + 's390': 'GuestPanicInformationS390',
>>> + 'tdx' : 'GuestPanicInformationTdx'}}
>>> ##
>>> # @GuestPanicInformationHyperV:
>>> @@ -564,6 +567,30 @@
>>> 'psw-addr': 'uint64',
>>> 'reason': 'S390CrashReason'}}
>>> +##
>>> +# @GuestPanicInformationTdx:
>>> +#
>>> +# TDX Guest panic information specific to TDX, as specified in the
>>> +# "Guest-Hypervisor Communication Interface (GHCI) Specification",
>>> +# section TDG.VP.VMCALL<ReportFatalError>.
>>> +#
>>> +# @error-code: TD-specific error code
>>> +#
>>> +# @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. Present when the
>>> +# "GPA valid" bit (bit 63) is set in @error-code.
>>
>> Uh, peeking at GHCI Spec section 3.4 TDG.VP.VMCALL<ReportFatalError>, I
>> see operand R12 consists of
>>
>> bits name description
>> 31:0 TD-specific error code TD-specific error code
>> Panic – 0x0.
>> Values – 0x1 to 0xFFFFFFFF
>> reserved.
>> 62:32 TD-specific extended TD-specific extended error code.
>> error code TD software defined.
>> 63 GPA Valid Set if the TD specified additional
>> information in the GPA parameter
>> (R13).
>> Is @error-code all of R12, or just bits 31:0?
>> If it's all of R12, description of @error-code as "TD-specific error
>> code" is misleading.
>
> We pass all of R12 to @error_code.
>
> Here it wants to use "error_code" as generic as the whole R12. Do you have any better description of it ?
Sadly, the spec is of no help: it doesn't name the entire thing, only
the three sub-fields TD-specific error code, TD-specific extended error
code, GPA valid.
We could take the hint, and provide the sub-fields instead:
* @error-code contains the TD-specific error code (bits 31:0)
* @extended-error-code contains the TD-specific extended error code
(bits 62:32)
* we don't need @gpa-valid, because it's the same as "@gpa is present"
If we decide to keep the single member, we do need another name for it.
@error-codes (plural) doesn't exactly feel wonderful, but it gives at
least a subtle hint that it's not just *the* error code.
>> If it's just bits 31:0, then 'Present when the "GPA valid" bit (bit 63)
>> is set in @error-code' is wrong. Could go with 'Only present when the
>> guest provides this information'.
>>
>>> +#
>>> +#
>>
>> Drop one of these two lines, please.
>>
>>> +# Since: 9.0
>>> +##
>>> +{'struct': 'GuestPanicInformationTdx',
>>> + 'data': {'error-code': 'uint64',
>>> + 'message': 'str',
>>> + '*gpa': 'uint64'}}
>>> +
>>> ##
>>> # @MEMORY_FAILURE:
>>> #
>>
