On 8/21/2023 5:58 PM, Daniel P. Berrangé wrote:
On Fri, Aug 18, 2023 at 05:50:30AM -0400, Xiaoyao Li wrote:
Originated-from: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
Signed-off-by: Xiaoyao Li <xiaoyao.li@xxxxxxxxx>
---
qapi/run-state.json | 17 +++++++++++++--
softmmu/runstate.c | 49 +++++++++++++++++++++++++++++++++++++++++++
target/i386/kvm/tdx.c | 24 ++++++++++++++++++++-
3 files changed, 87 insertions(+), 3 deletions(-)
diff --git a/qapi/run-state.json b/qapi/run-state.json
index f216ba54ec4c..506bbe31541f 100644
--- a/qapi/run-state.json
+++ b/qapi/run-state.json
@@ -499,7 +499,7 @@
# Since: 2.9
##
{ 'enum': 'GuestPanicInformationType',
- 'data': [ 'hyper-v', 's390' ] }
+ 'data': [ 'hyper-v', 's390', 'tdx' ] }
Missing documentation for the 'tdx' value
##
# @GuestPanicInformation:
@@ -514,7 +514,8 @@
'base': {'type': 'GuestPanicInformationType'},
'discriminator': 'type',
'data': {'hyper-v': 'GuestPanicInformationHyperV',
- 's390': 'GuestPanicInformationS390'}}
+ 's390': 'GuestPanicInformationS390',
+ 'tdx' : 'GuestPanicInformationTdx'}}
##
# @GuestPanicInformationHyperV:
@@ -577,6 +578,18 @@
'psw-addr': 'uint64',
'reason': 'S390CrashReason'}}
+##
+# @GuestPanicInformationTdx:
+#
+# TDX GHCI TDG.VP.VMCALL<ReportFatalError> specific guest panic information
Not documented any of the struct members. Especially please include
the warning that 'message' comes from the guest and so must not be
trusted, not assumed to be well formed.
Will do it in next version.
thanks!
+#
+# Since: 8.2
+##
+{'struct': 'GuestPanicInformationTdx',
+ 'data': {'error-code': 'uint64',
+ 'gpa': 'uint64',
+ 'message': 'str'}}
+
##
# @MEMORY_FAILURE:
#
diff --git a/softmmu/runstate.c b/softmmu/runstate.c
index f3bd86281813..cab11484ed7e 100644
--- a/softmmu/runstate.c
+++ b/softmmu/runstate.c
@@ -518,7 +518,56 @@ void qemu_system_guest_panicked(GuestPanicInformation *info)
S390CrashReason_str(info->u.s390.reason),
info->u.s390.psw_mask,
info->u.s390.psw_addr);
+ } else if (info->type == GUEST_PANIC_INFORMATION_TYPE_TDX) {
+ char *buf = NULL;
+ bool printable = false;
+
+ /*
+ * Although message is defined as a json string, we shouldn't
+ * unconditionally treat it as is because the guest generated it and
+ * it's not necessarily trustable.
+ */
+ if (info->u.tdx.message) {
+ /* The caller guarantees the NUL-terminated string. */
+ int len = strlen(info->u.tdx.message);
+ int i;
+
+ printable = len > 0;
+ for (i = 0; i < len; i++) {
+ if (!(0x20 <= info->u.tdx.message[i] &&
+ info->u.tdx.message[i] <= 0x7e)) {
+ printable = false;
+ break;
+ }
+ }
+
+ /* 3 = length of "%02x " */
+ buf = g_malloc(len * 3);
+ for (i = 0; i < len; i++) {
+ if (info->u.tdx.message[i] == '\0') {
+ break;
+ } else {
+ sprintf(buf + 3 * i, "%02x ", info->u.tdx.message[i]);
+ }
+ }
+ if (i > 0)
+ /* replace the last ' '(space) to NUL */
+ buf[i * 3 - 1] = '\0';
+ else
+ buf[0] = '\0';
You're building this escaped buffer but...
+ }
+
+ qemu_log_mask(LOG_GUEST_ERROR,
+ //" TDX report fatal error:\"%s\" %s",
+ " TDX report fatal error:\"%s\""
+ "error: 0x%016" PRIx64 " gpa page: 0x%016" PRIx64 "\n",
+ printable ? info->u.tdx.message : "",
+ //buf ? buf : "",
...then not actually using it
Either delete the 'buf' code, or use it.
Sorry for posting some internal testing version.
Does below look good to you?
@@ -518,7 +518,56 @@ void
qemu_system_guest_panicked(GuestPanicInformation *info)
S390CrashReason_str(info->u.s390.reason),
info->u.s390.psw_mask,
info->u.s390.psw_addr);
+ } else if (info->type == GUEST_PANIC_INFORMATION_TYPE_TDX) {
+ bool printable = false;
+ char *buf = NULL;
+ int len = 0, i;
+
+ /*
+ * Although message is defined as a json string, we shouldn't
+ * unconditionally treat it as is because the guest
generated it and
+ * it's not necessarily trustable.
+ */
+ if (info->u.tdx.message) {
+ /* The caller guarantees the NUL-terminated string. */
+ len = strlen(info->u.tdx.message);
+
+ printable = len > 0;
+ for (i = 0; i < len; i++) {
+ if (!(0x20 <= info->u.tdx.message[i] &&
+ info->u.tdx.message[i] <= 0x7e)) {
+ printable = false;
+ break;
+ }
+ }
+ }
+
+ if (!printable && len) {
+ /* 3 = length of "%02x " */
+ buf = g_malloc(len * 3);
+ for (i = 0; i < len; i++) {
+ if (info->u.tdx.message[i] == '\0') {
+ break;
+ } else {
+ sprintf(buf + 3 * i, "%02x ",
info->u.tdx.message[i]);
+ }
+ }
+ if (i > 0)
+ /* replace the last ' '(space) to NUL */
+ buf[i * 3 - 1] = '\0';
+ else
+ buf[0] = '\0';
+ }
+
+ qemu_log_mask(LOG_GUEST_ERROR,
+ " TDX guest reports fatal error:\"%s\""
+ " error code: 0x%016" PRIx64 " gpa page:
0x%016" PRIx64 "\n",
+ printable ? info->u.tdx.message : buf,
+ info->u.tdx.error_code,
+ info->u.tdx.gpa);
+ g_free(buf);
}
+ info->u.tdx.error_code,
+ info->u.tdx.gpa);
+ g_free(buf);
}
+
qapi_free_GuestPanicInformation(info);
}
}
diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
index f111b46dac92..7efaa13f59e2 100644
--- a/target/i386/kvm/tdx.c
+++ b/target/i386/kvm/tdx.c
@@ -18,6 +18,7 @@
#include "qom/object_interfaces.h"
#include "standard-headers/asm-x86/kvm_para.h"
#include "sysemu/kvm.h"
+#include "sysemu/runstate.h"
#include "sysemu/sysemu.h"
#include "exec/address-spaces.h"
#include "exec/ramblock.h"
@@ -1408,11 +1409,26 @@ static void tdx_handle_get_quote(X86CPU *cpu, struct kvm_tdx_vmcall *vmcall)
vmcall->status_code = TDG_VP_VMCALL_SUCCESS;
}
+static void tdx_panicked_on_fatal_error(X86CPU *cpu, uint64_t error_code,
+ uint64_t gpa, char *message)
+{
+ GuestPanicInformation *panic_info;
+
+ panic_info = g_new0(GuestPanicInformation, 1);
+ panic_info->type = GUEST_PANIC_INFORMATION_TYPE_TDX;
+ panic_info->u.tdx.error_code = error_code;
+ panic_info->u.tdx.gpa = gpa;
+ panic_info->u.tdx.message = (char *)message;
+
+ qemu_system_guest_panicked(panic_info);
+}
+
static void tdx_handle_report_fatal_error(X86CPU *cpu,
struct kvm_tdx_vmcall *vmcall)
{
uint64_t error_code = vmcall->in_r12;
char *message = NULL;
+ uint64_t gpa = -1ull;
if (error_code & 0xffff) {
error_report("invalid error code of TDG.VP.VMCALL<REPORT_FATAL_ERROR>\n");
@@ -1441,7 +1457,13 @@ static void tdx_handle_report_fatal_error(X86CPU *cpu,
}
error_report("TD guest reports fatal error. %s\n", message ? : "");
In tdx_panicked_on_fatal_error you're avoiding printing 'message' if it
contains non-printable characters, but here you're printing it regardless.
I guess you meant qemu_system_guest_panicked().
Do we still need this error_report call at all ?
yes. It can and should be dropped before I sent to maillist. I keep it
internally for testing purpose because qemu_log_mask() doesn't get
printed by default.
- exit(1);
+
+#define TDX_REPORT_FATAL_ERROR_GPA_VALID BIT_ULL(63)
+ if (error_code & TDX_REPORT_FATAL_ERROR_GPA_VALID) {
+ gpa = vmcall->in_r13;
Bad indent
Fixed.
+ }
+
+ tdx_panicked_on_fatal_error(cpu, error_code, gpa, message);
}
static void tdx_handle_setup_event_notify_interrupt(X86CPU *cpu,
--
2.34.1
With regards,
Daniel
