[RFC PATCH 3/7] conf: introduce TrustDomain element in domain

Duan, Zhenzhong zhenzhong.duan at intel.com
Mon Jun 21 03:41:49 UTC 2021


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





More information about the libvir-list mailing list