[PATCH rfcv4 05/13] conf: add tdx as launch security type

Zhenzhong Duan posted 13 patches 6 months, 2 weeks ago
There is a newer version of this series
[PATCH rfcv4 05/13] conf: add tdx as launch security type
Posted by Zhenzhong Duan 6 months, 2 weeks ago
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' and three optional element for tdx type.
In 'policy', bit 0 is set to enable TDX debug, bit 28 set to enable
sept-ve-disable, other bits are reserved currently. mrConfigId, mrOwner
and mrOwnerConfig are base64 encoded SHA384 digest.

For example:

 <launchSecurity type='tdx'>
   <policy>0x10000001</policy>
   <mrConfigId>xxx</mrConfigId>
   <mrOwner>xxx</mrOwner>
   <mrOwnerConfig>xxx</mrOwnerConfig>
 </launchSecurity>

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 src/conf/domain_conf.c            | 42 +++++++++++++++++++++++++++++++
 src/conf/domain_conf.h            |  9 +++++++
 src/conf/schemas/domaincommon.rng | 29 +++++++++++++++++++++
 src/conf/virconftypes.h           |  2 ++
 src/qemu/qemu_command.c           |  2 ++
 src/qemu/qemu_firmware.c          |  1 +
 src/qemu/qemu_namespace.c         |  1 +
 src/qemu/qemu_process.c           |  1 +
 src/qemu/qemu_validate.c          |  1 +
 9 files changed, 88 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a0912062ff..c557da0c65 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1508,6 +1508,7 @@ VIR_ENUM_IMPL(virDomainLaunchSecurity,
               "",
               "sev",
               "s390-pv",
+              "tdx",
 );
 
 typedef enum {
@@ -3832,6 +3833,10 @@ virDomainSecDefFree(virDomainSecDef *def)
         g_free(def->data.sev.dh_cert);
         g_free(def->data.sev.session);
         break;
+    case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
+        g_free(def->data.tdx.mrconfigid);
+        g_free(def->data.tdx.mrowner);
+        g_free(def->data.tdx.mrownerconfig);
     case VIR_DOMAIN_LAUNCH_SECURITY_PV:
     case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
     case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
@@ -13649,6 +13654,24 @@ virDomainSEVDefParseXML(virDomainSEVDef *def,
 }
 
 
+static int
+virDomainTDXDefParseXML(virDomainTDXDef *def,
+                        xmlXPathContextPtr ctxt)
+{
+    if (virXPathULongLongBase("string(./policy)", ctxt, 16, &def->policy) < 0) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("failed to get launch security policy for launch security type TDX"));
+        return -1;
+    }
+
+    def->mrconfigid = virXPathString("string(./mrConfigId)", ctxt);
+    def->mrowner = virXPathString("string(./mrOwner)", ctxt);
+    def->mrownerconfig = virXPathString("string(./mrOwnerConfig)", ctxt);
+
+    return 0;
+}
+
+
 static virDomainSecDef *
 virDomainSecDefParseXML(xmlNodePtr lsecNode,
                         xmlXPathContextPtr ctxt)
@@ -13668,6 +13691,10 @@ virDomainSecDefParseXML(xmlNodePtr lsecNode,
         if (virDomainSEVDefParseXML(&sec->data.sev, ctxt) < 0)
             return NULL;
         break;
+    case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
+        if (virDomainTDXDefParseXML(&sec->data.tdx, ctxt) < 0)
+            return NULL;
+        break;
     case VIR_DOMAIN_LAUNCH_SECURITY_PV:
         break;
     case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
@@ -26661,6 +26688,21 @@ virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec)
         break;
     }
 
+    case VIR_DOMAIN_LAUNCH_SECURITY_TDX: {
+        virDomainTDXDef *tdx = &sec->data.tdx;
+
+        virBufferAsprintf(&childBuf, "<policy>0x%llx</policy>\n", tdx->policy);
+
+        if (tdx->mrconfigid)
+            virBufferEscapeString(&childBuf, "<mrConfigId>%s</mrConfigId>\n", tdx->mrconfigid);
+        if (tdx->mrowner)
+            virBufferEscapeString(&childBuf, "<mrOwner>%s</mrOwner>\n", tdx->mrowner);
+        if (tdx->mrownerconfig)
+            virBufferEscapeString(&childBuf, "<mrOwnerConfig>%s</mrOwnerConfig>\n", tdx->mrownerconfig);
+
+        break;
+    }
+
     case VIR_DOMAIN_LAUNCH_SECURITY_PV:
         break;
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 356c25405b..7882b7a75d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2856,6 +2856,7 @@ typedef enum {
     VIR_DOMAIN_LAUNCH_SECURITY_NONE,
     VIR_DOMAIN_LAUNCH_SECURITY_SEV,
     VIR_DOMAIN_LAUNCH_SECURITY_PV,
+    VIR_DOMAIN_LAUNCH_SECURITY_TDX,
 
     VIR_DOMAIN_LAUNCH_SECURITY_LAST,
 } virDomainLaunchSecurity;
@@ -2872,10 +2873,18 @@ struct _virDomainSEVDef {
     virTristateBool kernel_hashes;
 };
 
+struct _virDomainTDXDef {
+    unsigned long long policy;
+    char *mrconfigid;
+    char *mrowner;
+    char *mrownerconfig;
+};
+
 struct _virDomainSecDef {
     virDomainLaunchSecurity sectype;
     union {
         virDomainSEVDef sev;
+        virDomainTDXDef tdx;
     } data;
 };
 
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index d84e030255..f6e1782b33 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -520,6 +520,9 @@
             <value>s390-pv</value>
           </attribute>
         </group>
+        <group>
+          <ref name="launchSecurityTDX"/>
+        </group>
       </choice>
     </element>
   </define>
@@ -565,6 +568,32 @@
     </interleave>
   </define>
 
+  <define name="launchSecurityTDX">
+    <attribute name="type">
+      <value>tdx</value>
+    </attribute>
+    <interleave>
+      <element name="policy">
+        <ref name="hexuint"/>
+      </element>
+      <optional>
+        <element name="mrConfigId">
+          <data type="string"/>
+        </element>
+      </optional>
+      <optional>
+        <element name="mrOwner">
+          <data type="string"/>
+        </element>
+      </optional>
+      <optional>
+        <element name="mrOwnerConfig">
+          <data type="string"/>
+        </element>
+      </optional>
+    </interleave>
+  </define>
+
   <!--
       Enable or disable perf events for the domain. For each
       of the events the following rules apply:
diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h
index 0779bc224b..e2e0bba012 100644
--- a/src/conf/virconftypes.h
+++ b/src/conf/virconftypes.h
@@ -212,6 +212,8 @@ typedef struct _virDomainResourceDef virDomainResourceDef;
 
 typedef struct _virDomainSEVDef virDomainSEVDef;
 
+typedef struct _virDomainTDXDef virDomainTDXDef;
+
 typedef struct _virDomainSecDef virDomainSecDef;
 
 typedef struct _virDomainShmemDef virDomainShmemDef;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8d4442c360..dde2d5fa01 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7037,6 +7037,7 @@ qemuBuildMachineCommandLine(virCommand *cmd,
             }
             break;
         case VIR_DOMAIN_LAUNCH_SECURITY_PV:
+        case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
             virBufferAddLit(&buf, ",confidential-guest-support=lsec0");
             break;
         case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
@@ -9758,6 +9759,7 @@ qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd,
     case VIR_DOMAIN_LAUNCH_SECURITY_PV:
         return qemuBuildPVCommandLine(vm, cmd);
         break;
+    case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
     case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
     case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
         virReportEnumRangeError(virDomainLaunchSecurity, sec->sectype);
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 78844e3066..7ab10a9286 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1339,6 +1339,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
             }
             break;
         case VIR_DOMAIN_LAUNCH_SECURITY_PV:
+        case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
             break;
         case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
         case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
index 915d44310f..ac8a4b5c07 100644
--- a/src/qemu/qemu_namespace.c
+++ b/src/qemu/qemu_namespace.c
@@ -660,6 +660,7 @@ qemuDomainSetupLaunchSecurity(virDomainObj *vm,
         VIR_DEBUG("Set up launch security for SEV");
         break;
     case VIR_DOMAIN_LAUNCH_SECURITY_PV:
+    case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
         break;
     case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
     case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index da2b024f92..bd8624e3f6 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6781,6 +6781,7 @@ qemuProcessPrepareLaunchSecurityGuestInput(virDomainObj *vm)
     case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
         return qemuProcessPrepareSEVGuestInput(vm);
     case VIR_DOMAIN_LAUNCH_SECURITY_PV:
+    case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
         return 0;
     case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
     case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index fd7b90e47d..ee25fd70d9 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1322,6 +1322,7 @@ qemuValidateDomainDef(const virDomainDef *def,
                 return -1;
             }
             break;
+        case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
         case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
         case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
             virReportEnumRangeError(virDomainLaunchSecurity, def->sec->sectype);
-- 
2.34.1
Re: [PATCH rfcv4 05/13] conf: add tdx as launch security type
Posted by Daniel P. Berrangé 6 months ago
On Fri, May 24, 2024 at 02:21:20PM +0800, Zhenzhong Duan wrote:
> 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' and three optional element for tdx type.
> In 'policy', bit 0 is set to enable TDX debug, bit 28 set to enable
> sept-ve-disable, other bits are reserved currently. mrConfigId, mrOwner
> and mrOwnerConfig are base64 encoded SHA384 digest.
> 
> For example:
> 
>  <launchSecurity type='tdx'>
>    <policy>0x10000001</policy>
>    <mrConfigId>xxx</mrConfigId>
>    <mrOwner>xxx</mrOwner>
>    <mrOwnerConfig>xxx</mrOwnerConfig>
>  </launchSecurity>
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  src/conf/domain_conf.c            | 42 +++++++++++++++++++++++++++++++
>  src/conf/domain_conf.h            |  9 +++++++
>  src/conf/schemas/domaincommon.rng | 29 +++++++++++++++++++++
>  src/conf/virconftypes.h           |  2 ++
>  src/qemu/qemu_command.c           |  2 ++
>  src/qemu/qemu_firmware.c          |  1 +
>  src/qemu/qemu_namespace.c         |  1 +
>  src/qemu/qemu_process.c           |  1 +
>  src/qemu/qemu_validate.c          |  1 +
>  9 files changed, 88 insertions(+)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a0912062ff..c557da0c65 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1508,6 +1508,7 @@ VIR_ENUM_IMPL(virDomainLaunchSecurity,
>                "",
>                "sev",
>                "s390-pv",
> +              "tdx",
>  );
>  
>  typedef enum {
> @@ -3832,6 +3833,10 @@ virDomainSecDefFree(virDomainSecDef *def)
>          g_free(def->data.sev.dh_cert);
>          g_free(def->data.sev.session);
>          break;
> +    case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
> +        g_free(def->data.tdx.mrconfigid);
> +        g_free(def->data.tdx.mrowner);
> +        g_free(def->data.tdx.mrownerconfig);

Missing 'break' here. I'm surprised the compiler didn't complain,
as we have warning flags set to require explicit marking of case
fallthroughs.

>      case VIR_DOMAIN_LAUNCH_SECURITY_PV:
>      case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
>      case VIR_DOMAIN_LAUNCH_SECURITY_LAST:




With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
RE: [PATCH rfcv4 05/13] conf: add tdx as launch security type
Posted by Duan, Zhenzhong 6 months ago

>-----Original Message-----
>From: Daniel P. Berrangé <berrange@redhat.com>
>Subject: Re: [PATCH rfcv4 05/13] conf: add tdx as launch security type
>
>On Fri, May 24, 2024 at 02:21:20PM +0800, Zhenzhong Duan wrote:
>> 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' and three optional element for tdx type.
>> In 'policy', bit 0 is set to enable TDX debug, bit 28 set to enable
>> sept-ve-disable, other bits are reserved currently. mrConfigId, mrOwner
>> and mrOwnerConfig are base64 encoded SHA384 digest.
>>
>> For example:
>>
>>  <launchSecurity type='tdx'>
>>    <policy>0x10000001</policy>
>>    <mrConfigId>xxx</mrConfigId>
>>    <mrOwner>xxx</mrOwner>
>>    <mrOwnerConfig>xxx</mrOwnerConfig>
>>  </launchSecurity>
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>  src/conf/domain_conf.c            | 42
>+++++++++++++++++++++++++++++++
>>  src/conf/domain_conf.h            |  9 +++++++
>>  src/conf/schemas/domaincommon.rng | 29 +++++++++++++++++++++
>>  src/conf/virconftypes.h           |  2 ++
>>  src/qemu/qemu_command.c           |  2 ++
>>  src/qemu/qemu_firmware.c          |  1 +
>>  src/qemu/qemu_namespace.c         |  1 +
>>  src/qemu/qemu_process.c           |  1 +
>>  src/qemu/qemu_validate.c          |  1 +
>>  9 files changed, 88 insertions(+)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index a0912062ff..c557da0c65 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -1508,6 +1508,7 @@ VIR_ENUM_IMPL(virDomainLaunchSecurity,
>>                "",
>>                "sev",
>>                "s390-pv",
>> +              "tdx",
>>  );
>>
>>  typedef enum {
>> @@ -3832,6 +3833,10 @@ virDomainSecDefFree(virDomainSecDef *def)
>>          g_free(def->data.sev.dh_cert);
>>          g_free(def->data.sev.session);
>>          break;
>> +    case VIR_DOMAIN_LAUNCH_SECURITY_TDX:
>> +        g_free(def->data.tdx.mrconfigid);
>> +        g_free(def->data.tdx.mrowner);
>> +        g_free(def->data.tdx.mrownerconfig);
>
>Missing 'break' here. I'm surprised the compiler didn't complain,
>as we have warning flags set to require explicit marking of case
>fallthroughs.

Will do.

Thanks
Zhenzhong

>
>>      case VIR_DOMAIN_LAUNCH_SECURITY_PV:
>>      case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
>>      case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
>
>
>
>
>With regards,
>Daniel
>--
>|: https://berrange.com      -o-
>https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>|: https://entangle-photo.org    -o-
>https://www.instagram.com/dberrange :|