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

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

 



On Fri, Jun 18, 2021 at 04:50:48PM +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>

Any reason why you are adding a new element that basically copies
exactly what <launchSecurity> is doing?

In libvirt it will essentially use `confidential-guest-support` which is
used for launchSecurity so there is no need to duplicate the code and
the XML element.

It could look like this:

  <launchSecurity type='tdx'>
    <policy>0x0001</policy>
  </launchSecurity>

We would have to reorganize the <launchSecurity> documentation a little
bit but otherwise there is nothing blocking us to have single element to
specify different type of encrypted/confidential/secure/... VMs.

We already have patches for similar feature for s390 which will also
reuse this element and in the future any other CPU architecture can
reuse it.

Pavel

> 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"/>
> +        </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);
> @@ -3502,6 +3508,16 @@ virDomainSEVDefFree(virDomainSEVDef *def)
>      g_free(def);
>  }
>  
> +
> +static void
> +virDomainTDXDefFree(virDomainTDXDef *def)
> +{
> +    if (!def)
> +        return;
> +
> +    g_free(def);
> +}
> +
>  static void
>  virDomainOSDefClear(virDomainOSDef *os)
>  {
> @@ -3704,6 +3720,7 @@ void virDomainDefFree(virDomainDef *def)
>          (def->ns.free)(def->namespaceData);
>  
>      virDomainSEVDefFree(def->sev);
> +    virDomainTDXDefFree(def->tdx);
>  
>      xmlFreeNode(def->metadata);
>  
> @@ -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);
> +    return NULL;
> +}
> +
>  
>  static virDomainMemoryDef *
>  virDomainMemoryDefParseXML(virDomainXMLOption *xmlopt,
> @@ -20117,6 +20181,13 @@ virDomainDefParseXML(xmlDocPtr xml,
>              goto error;
>      }
>  
> +    /* Check for TDX feature */
> +    if ((node = virXPathNode("./TrustDomain", ctxt)) != NULL) {
> +        def->tdx = virDomainTDXDefParseXML(node, ctxt);
> +        if (!def->tdx)
> +            goto error;
> +    }
> +
>      /* analysis of memory devices */
>      if ((n = virXPathNodeSet("./devices/memory", ctxt, &nodes)) < 0)
>          goto error;
> @@ -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");
> +    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)
> @@ -28277,6 +28360,7 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def,
>          virDomainKeyWrapDefFormat(buf, def->keywrap);
>  
>      virDomainSEVDefFormat(buf, def->sev);
> +    virDomainTDXDefFormat(buf, def->tdx);
>  
>      if (def->namespaceData && def->ns.format) {
>          if ((def->ns.format)(buf, def->namespaceData) < 0)
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index f706c498ff..7cb5061c8c 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2661,6 +2661,17 @@ struct _virDomainSEVDef {
>      unsigned int reduced_phys_bits;
>  };
>  
> +typedef enum {
> +    VIR_DOMAIN_TRUST_DOMAIN_NONE,
> +    VIR_DOMAIN_TRUST_DOMAIN_TDX,
> +
> +    VIR_DOMAIN_TRUST_DOMAIN_LAST,
> +} virDomainTrustDomain;
> +
> +struct _virDomainTDXDef {
> +    int sectype; /* enum virDomainTrustDomain */
> +    unsigned int policy; /* bit 0 set hint debug enabled, other bit reserved */
> +};
>  
>  typedef enum {
>      VIR_DOMAIN_IOMMU_MODEL_INTEL,
> @@ -2874,6 +2885,9 @@ struct _virDomainDef {
>      /* SEV-specific domain */
>      virDomainSEVDef *sev;
>  
> +    /* TDX-specific domain */
> +    virDomainTDXDef *tdx;
> +
>      /* Application-specific custom metadata */
>      xmlNodePtr metadata;
>  
> @@ -3888,6 +3902,7 @@ VIR_ENUM_DECL(virDomainVsockModel);
>  VIR_ENUM_DECL(virDomainShmemModel);
>  VIR_ENUM_DECL(virDomainShmemRole);
>  VIR_ENUM_DECL(virDomainLaunchSecurity);
> +VIR_ENUM_DECL(virDomainTrustDomain);
>  /* from libvirt.h */
>  VIR_ENUM_DECL(virDomainState);
>  VIR_ENUM_DECL(virDomainNostateReason);
> 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;
> +
>  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;
> +    }
> +
>      if (def->naudios > 1 &&
>          !virQEMUCapsGet(qemuCaps, QEMU_CAPS_AUDIODEV)) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> diff --git a/tests/genericxml2xmlindata/trust-domain-tdx.xml b/tests/genericxml2xmlindata/trust-domain-tdx.xml
> new file mode 100644
> index 0000000000..7a56cf0e92
> --- /dev/null
> +++ b/tests/genericxml2xmlindata/trust-domain-tdx.xml
> @@ -0,0 +1,21 @@
> +<domain type='kvm'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219100</memory>
> +  <currentMemory unit='KiB'>219100</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='x86_64' machine='pc-1.0'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +  </devices>
> +  <TrustDomain type='tdx'>
> +    <policy>0x0001</policy>
> +  </TrustDomain>
> +</domain>
> +
> diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c
> index ac89422a32..0bd7717953 100644
> --- a/tests/genericxml2xmltest.c
> +++ b/tests/genericxml2xmltest.c
> @@ -233,6 +233,7 @@ mymain(void)
>      DO_TEST("tseg");
>  
>      DO_TEST("launch-security-sev");
> +    DO_TEST("trust-domain-tdx");
>  
>      DO_TEST_DIFFERENT("cputune");
>      DO_TEST("device-backenddomain");
> -- 
> 2.25.1
> 

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux