qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH v5 30/65] i386/tdx: Support user configurable mrconfigid/mrow


From: Xiaoyao Li
Subject: Re: [PATCH v5 30/65] i386/tdx: Support user configurable mrconfigid/mrowner/mrownerconfig
Date: Thu, 29 Feb 2024 18:50:04 +0800
User-agent: Mozilla Thunderbird

On 2/29/2024 4:37 PM, Markus Armbruster wrote:
Xiaoyao Li <xiaoyao.li@intel.com> writes:

From: Isaku Yamahata <isaku.yamahata@intel.com>

Three sha384 hash values, mrconfigid, mrowner and mrownerconfig, of a TD
can be provided for TDX attestation. Detailed meaning of them can be
found: 
https://lore.kernel.org/qemu-devel/31d6dbc1-f453-4cef-ab08-4813f4e0ff92@intel.com/

Allow user to specify those values via property mrconfigid, mrowner and
mrownerconfig. They are all in base64 format.

example
-object tdx-guest, \
   mrconfigid=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v,\
   mrowner=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v,\
   
mrownerconfig=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v

Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Co-developed-by: Xiaoyao Li <xiaoyao.li@intel.com>
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>

---
Changes in v5:
  - refine the description of QAPI properties and add description of
    default value when not specified;

Changes in v4:
  - describe more of there fields in qom.json
  - free the old value before set new value to avoid memory leak in
    _setter(); (Daniel)

Changes in v3:
  - use base64 encoding instread of hex-string;
---
  qapi/qom.json         | 17 ++++++++-
  target/i386/kvm/tdx.c | 87 +++++++++++++++++++++++++++++++++++++++++++
  target/i386/kvm/tdx.h |  3 ++
  3 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/qapi/qom.json b/qapi/qom.json
index 89ed89b9b46e..cac875349a3a 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -905,10 +905,25 @@
  #     pages.  Some guest OS (e.g., Linux TD guest) may require this to
  #     be set, otherwise they refuse to boot.
  #
+# @mrconfigid: ID for non-owner-defined configuration of the guest TD,
+#     e.g., run-time or OS configuration (base64 encoded SHA384 digest).
+#     (A default value 0 of SHA384 is used when absent).

Suggest to drop the parenthesis in the last sentence.

@mrconfigid is a string, so the default value can't be 0.  Actually,
it's not just any string, but a base64 encoded SHA384 digest, which
means it must be exactly 96 hex digits.  So it can't be "0", either.  It
could be
"000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000".

I thought value 0 of SHA384 just means it.

That's my fault and my poor english.

More on this below.

+#
+# @mrowner: ID for the guest TD’s owner (base64 encoded SHA384 digest).
+#     (A default value 0 of SHA384 is used when absent).
+#
+# @mrownerconfig: ID for owner-defined configuration of the guest TD,
+#     e.g., specific to the workload rather than the run-time or OS
+#     (base64 encoded SHA384 digest). (A default value 0 of SHA384 is
+#     used when absent).
+#
  # Since: 9.0
  ##
  { 'struct': 'TdxGuestProperties',
-  'data': { '*sept-ve-disable': 'bool' } }
+  'data': { '*sept-ve-disable': 'bool',
+            '*mrconfigid': 'str',
+            '*mrowner': 'str',
+            '*mrownerconfig': 'str' } }
##
  # @ThreadContextProperties:
diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
index d0ad4f57b5d0..4ce2f1d082ce 100644
--- a/target/i386/kvm/tdx.c
+++ b/target/i386/kvm/tdx.c
@@ -13,6 +13,7 @@
#include "qemu/osdep.h"
  #include "qemu/error-report.h"
+#include "qemu/base64.h"
  #include "qapi/error.h"
  #include "qom/object_interfaces.h"
  #include "standard-headers/asm-x86/kvm_para.h"
@@ -516,6 +517,7 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
      X86CPU *x86cpu = X86_CPU(cpu);
      CPUX86State *env = &x86cpu->env;
      g_autofree struct kvm_tdx_init_vm *init_vm = NULL;
+    size_t data_len;
      int r = 0;
object_property_set_bool(OBJECT(cpu), "pmu", false, &error_abort);
@@ -528,6 +530,38 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
      init_vm = g_malloc0(sizeof(struct kvm_tdx_init_vm) +
                          sizeof(struct kvm_cpuid_entry2) * 
KVM_MAX_CPUID_ENTRIES);
+#define SHA384_DIGEST_SIZE 48
+
+    if (tdx_guest->mrconfigid) {
+        g_autofree uint8_t *data = qbase64_decode(tdx_guest->mrconfigid,
+                              strlen(tdx_guest->mrconfigid), &data_len, errp);
+        if (!data || data_len != SHA384_DIGEST_SIZE) {
+            error_setg(errp, "TDX: failed to decode mrconfigid");
+            return -1;
+        }
+        memcpy(init_vm->mrconfigid, data, data_len);
+    }

When @mrconfigid is absent, the property remains null, and this
conditional is not executed.  init_vm->mrconfigid[], an array of 6
__u64, remains all zero.  How does the kernel treat that?

A all-zero SHA384 value is still a valid value, isn't it?

KVM treats it with no difference.

+
+    if (tdx_guest->mrowner) {
+        g_autofree uint8_t *data = qbase64_decode(tdx_guest->mrowner,
+                              strlen(tdx_guest->mrowner), &data_len, errp);
+        if (!data || data_len != SHA384_DIGEST_SIZE) {
+            error_setg(errp, "TDX: failed to decode mrowner");
+            return -1;
+        }
+        memcpy(init_vm->mrowner, data, data_len);
+    }
+
+    if (tdx_guest->mrownerconfig) {
+        g_autofree uint8_t *data = qbase64_decode(tdx_guest->mrownerconfig,
+                              strlen(tdx_guest->mrownerconfig), &data_len, 
errp);
+        if (!data || data_len != SHA384_DIGEST_SIZE) {
+            error_setg(errp, "TDX: failed to decode mrownerconfig");
+            return -1;
+        }
+        memcpy(init_vm->mrownerconfig, data, data_len);
+    }
+
      r = kvm_vm_enable_cap(kvm_state, KVM_CAP_MAX_VCPUS, 0, ms->smp.cpus);
      if (r < 0) {
          error_setg(errp, "Unable to set MAX VCPUS to %d", ms->smp.cpus);

[...]





reply via email to

[Prev in Thread] Current Thread [Next in Thread]