Xiaoyao Li <xiaoyao.li@xxxxxxxxx> writes: > On 3/7/2024 9:51 PM, Markus Armbruster wrote: >> 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 [...] >>>>> @@ -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. > > The reason we only defined one single member, is that the > extended-error-code is not used now, and I believe it won't be used in > the near future. Aha! Then I recommend * @error-code contains the TD-specific error code (bits 31:0) * Omit bits 62:32 from the reply; if we later find an actual use for them, we can add a suitable member * Omit bit 63, because it's the same as "@gpa is present" > If no objection from others, I will use @error-codes (plural) in the > next version. I recommend to keep the @error-code name, but narrow its value to the actual error code, i.e. bits 31:0. >>>> 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'. [...]
