Re: [PATCH v5 52/65] i386/tdx: Wire TDX_REPORT_FATAL_ERROR with GuestPanic facility

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

 



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
@@ -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.

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.

If no objection from others, I will use @error-codes (plural) in the next version.

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:
   #







[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