> -----Original Message-----
> From: Peter Krempa <pkrempa@xxxxxxxxxx>
> Sent: Friday, June 18, 2021 7:17 PM
> To: Duan, Zhenzhong <zhenzhong.duan@xxxxxxxxx>
> Cc: libvir-list@xxxxxxxxxx; Yamahata, Isaku <isaku.yamahata@xxxxxxxxx>;
> Tian, Jun J <jun.j.tian@xxxxxxxxx>; Qiang, Chenyi <chenyi.qiang@xxxxxxxxx>
> 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@xxxxxxxxx>
> > ---
> > 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",
>
> [...]
