Re: [PATCH v4 53/66] i386/tdx: Wire TDX_REPORT_FATAL_ERROR with GuestPanic facility

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
>>> @@ -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:
>>>   #
>> 
>> [...]
>> 





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux