-----Original Message----- From: Peter Krempa <pkrempa@redhat.com> Sent: Friday, June 18, 2021 7:17 PM To: Duan, Zhenzhong <zhenzhong.duan@intel.com> Cc: libvir-list@redhat.com; Yamahata, Isaku <isaku.yamahata@intel.com>; Tian, Jun J <jun.j.tian@intel.com>; Qiang, Chenyi <chenyi.qiang@intel.com> Subject: Re: [RFC PATCH 3/7] conf: introduce TrustDomain element in domain
On Fri, Jun 18, 2021 at 16:50:48 +0800, Zhenzhong Duan wrote:
The TrustDomain element can be used to define the security model to use when launching a domain. Only type 'tdx' is supported currently.
When 'tdx' is used, the VM will launched with Intel TDX feature enabled. TDX feature supports running encrypted VM (Trust Domain, TD) under the control of KVM. A TD runs in a CPU model which protects the confidentiality of its memory and its CPU state from other software
There is a child element 'policy' in TrustDomain. In 'policy', bit 0 is used to enable TDX debug, other bits are reserved currently.
For example:
<TrustDomain type='tdx'> <policy>0x0001</policy> </TrustDomain>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- docs/schemas/domaincommon.rng | 16 ++++ src/conf/domain_conf.c | 84 +++++++++++++++++++ src/conf/domain_conf.h | 15 ++++ src/conf/virconftypes.h | 2 + src/qemu/qemu_validate.c | 8 ++ .../genericxml2xmlindata/trust-domain-tdx.xml | 21 +++++ tests/genericxml2xmltest.c | 1 + 7 files changed, 147 insertions(+) create mode 100644 tests/genericxml2xmlindata/trust-domain-tdx.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 5ea14b6dbf..2b39a01e84 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -89,6 +89,9 @@ <optional> <ref name="launchSecurity"/> </optional> + <optional> + <ref name="TrustDomain"/>
All our RNG schema entries ...
+ </optional> <optional> <ref name="bhyvecmdline"/> </optional> @@ -518,6 +521,19 @@ </element> </define>
+ <define name="TrustDomain"> + <element name="TrustDomain"> + <attribute name="type"> + <value>tdx</value> + </attribute> + <interleave> + <element name="policy"> + <ref name="hexuint"/> + </element> + </interleave> + </element> + </define> + <!-- Enable or disable perf events for the domain. For each of the events the following rules apply: diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 139cdfc0a7..a51db088c1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1403,6 +1403,12 @@ VIR_ENUM_IMPL(virDomainLaunchSecurity, "sev", );
+VIR_ENUM_IMPL(virDomainTrustDomain, + VIR_DOMAIN_TRUST_DOMAIN_LAST, + "", + "tdx", +); + static virClass *virDomainObjClass; static virClass *virDomainXMLOptionClass; static void virDomainObjDispose(void *obj);
[...]
@@ -14793,6 +14810,53 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode, return NULL; }
+static virDomainTDXDef * +virDomainTDXDefParseXML(xmlNodePtr tdxNode, + xmlXPathContextPtr ctxt) { + VIR_XPATH_NODE_AUTORESTORE(ctxt) + virDomainTDXDef *def; + unsigned long policy; + g_autofree char *type = NULL; + + def = g_new0(virDomainTDXDef, 1); + + ctxt->node = tdxNode; + + if (!(type = virXMLPropString(tdxNode, "type"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing trust domain type")); + goto error; + } + + def->sectype = virDomainTrustDomainTypeFromString(type); + switch ((virDomainTrustDomain) def->sectype) { + case VIR_DOMAIN_TRUST_DOMAIN_TDX: + break; + case VIR_DOMAIN_TRUST_DOMAIN_NONE: + case VIR_DOMAIN_TRUST_DOMAIN_LAST: + default: + virReportError(VIR_ERR_XML_ERROR, + _("unsupported trust domain type '%s'"), + type); + goto error; + } + + if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("failed to get trust domain policy")); + goto error; + } + + def->policy = policy; + + return def; + + error: + virDomainTDXDefFree(def);
If you register an autoptr cleanup function for this type you'll be able to avoid the whole 'error' label. Good suggestion, will do.
+ return NULL; +}
[...]
@@ -26870,6 +26941,18 @@ virDomainSEVDefFormat(virBuffer *buf, virDomainSEVDef *sev) virBufferAddLit(buf, "</launchSecurity>\n"); }
+static void +virDomainTDXDefFormat(virBuffer *buf, virDomainTDXDef *tdx) { + if (!tdx) + return; + + virBufferAsprintf(buf, "<TrustDomain type='tdx'>\n");
... as well as XML element names don't start with capital letter, so this one shouldn't either.
Will fix.
+ virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<policy>0x%04x</policy>\n", tdx->policy); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</TrustDomain>\n"); }
static void
[...]
virDomainPerfDefFormat(virBuffer *buf, virDomainPerfDef *perf) diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index b21068486e..dbd2eb984c 100644 --- a/src/conf/virconftypes.h +++ b/src/conf/virconftypes.h @@ -202,6 +202,8 @@ typedef struct _virDomainResourceDef virDomainResourceDef;
typedef struct _virDomainSEVDef virDomainSEVDef;
+typedef struct _virDomainTDXDef virDomainTDXDef; +
Plase don't mix generic (conf/) changes ...
Will refactor. Thanks Zhenzhong
typedef struct _virDomainShmemDef virDomainShmemDef;
typedef struct _virDomainSmartcardDef virDomainSmartcardDef; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 382473d03b..2efd011cc0 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1222,6 +1222,14 @@ qemuValidateDomainDef(const virDomainDef *def, return -1; }
+ if (def->tdx && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDX_GUEST)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("TDX trust domain is not supported with " + "this QEMU binary")); + return -1; + }
... with targetted qemu changes like this.
+ if (def->naudios > 1 && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_AUDIODEV)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
[...]