[PATCH rfcv4 08/13] Add Intel TDX Quote Generation Service(QGS) support

Zhenzhong Duan posted 13 patches 1 year, 8 months ago
There is a newer version of this series
[PATCH rfcv4 08/13] Add Intel TDX Quote Generation Service(QGS) support
Posted by Zhenzhong Duan 1 year, 8 months ago
Add element "quoteGenerationService" to tdx launch security type.
Currently it contains only one sub-element "SocketAddress".

"SocketAddress" is modelized according to QEMU QAPI, supporting
inet, unix, vsock and fd type and variant attributes depending
on type.

XML example:

  <launchSecurity type='tdx'>
    <policy>0x0</policy>
    <mrConfigId>xxx</mrConfigId>
    <mrOwner>xxx</mrOwner>
    <mrOwnerConfig>xxx</mrOwnerConfig>
    <quoteGenerationService>
      <SocketAddress type='vsock' cid='xxx' port='xxx'/>
    </quoteGenerationService>
  </launchSecurity>

QEMU command line example:
  qemu-system-x86_64 \
    -object '{"qom-type":"tdx-guest","id":"lsec0","sept-ve-disable":false,"mrconfigid":"xxx","mrowner":"xxx","mrownerconfig":"xxx","quote-generation-socket":{"type":"vsock","cid":"xxx","port":"xxx"}}' \
    -machine pc-q35-6.0,confidential-guest-support=lsec0

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 src/conf/domain_conf.c            | 272 +++++++++++++++++++++++++++++-
 src/conf/domain_conf.h            |  61 +++++++
 src/conf/schemas/domaincommon.rng | 106 ++++++++++++
 src/qemu/qemu_command.c           | 106 ++++++++++++
 4 files changed, 544 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c557da0c65..e1ae8295e5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1511,6 +1511,15 @@ VIR_ENUM_IMPL(virDomainLaunchSecurity,
               "tdx",
 );
 
+VIR_ENUM_IMPL(virDomainSocketAddress,
+              VIR_DOMAIN_SOCKET_ADDRESS_LAST,
+              "",
+              "inet",
+              "unix",
+              "vsock",
+              "fd",
+);
+
 typedef enum {
     VIR_DOMAIN_NET_VHOSTUSER_MODE_NONE,
     VIR_DOMAIN_NET_VHOSTUSER_MODE_CLIENT,
@@ -13654,6 +13663,190 @@ virDomainSEVDefParseXML(virDomainSEVDef *def,
 }
 
 
+static int
+virDomainInetSocketAddressDefParseXML(InetSocketAddress *inet,
+                                      xmlNodePtr node)
+{
+    int ret;
+
+    if (!(inet->host = virXMLPropString(node, "host"))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("missing host for inet socket address"));
+        return -1;
+    }
+
+    if (!(inet->port = virXMLPropString(node, "port"))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("missing port for inet socket address"));
+        return -1;
+    }
+
+    if (virXMLPropTristateBool(node, "numeric", VIR_XML_PROP_NONE,
+                               &inet->numeric) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("wrong attribute numeric for inet socket address"));
+        return -1;
+    }
+
+    if ((ret = virXMLPropUInt(node, "to", 10, VIR_XML_PROP_NONE,
+                             &inet->to)) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("wrong attribute to for inet socket address"));
+        return -1;
+    }
+    if (!ret)
+        inet->has_to = true;
+
+    if (virXMLPropTristateBool(node, "ipv4", VIR_XML_PROP_NONE,
+                               &inet->ipv4) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("wrong attribute ipv4 for inet socket address"));
+        return -1;
+    }
+
+    if (virXMLPropTristateBool(node, "ipv6", VIR_XML_PROP_NONE,
+                               &inet->ipv6) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("wrong attribute ipv6 for inet socket address"));
+        return -1;
+    }
+
+    if (virXMLPropTristateBool(node, "keep_alive", VIR_XML_PROP_NONE,
+                               &inet->keep_alive) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("wrong attribute keep_alive for inet socket address"));
+        return -1;
+    }
+
+    if (virXMLPropTristateBool(node, "mptcp", VIR_XML_PROP_NONE,
+                               &inet->mptcp) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("wrong attribute mptcp for inet socket address"));
+        return -1;
+    }
+
+    return 0;
+}
+
+
+static int
+virDomainUnixSocketAddressDefParseXML(UnixSocketAddress *Unix,
+                                      xmlNodePtr node)
+{
+    if (!(Unix->path = virXMLPropString(node, "path"))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("missing path for unix socket address"));
+        return -1;
+    }
+
+    if (virXMLPropTristateBool(node, "abstract", VIR_XML_PROP_NONE,
+                               &Unix->abstract) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("wrong attribute abstract for unix socket address"));
+        return -1;
+    }
+
+    if (virXMLPropTristateBool(node, "tight", VIR_XML_PROP_NONE,
+                               &Unix->tight) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("wrong attribute tight for unix socket address"));
+        return -1;
+    }
+
+    return 0;
+}
+
+
+static int
+virDomainVsockSocketAddressDefParseXML(VsockSocketAddress *vsock,
+                                       xmlNodePtr node)
+{
+    if (!(vsock->cid = virXMLPropString(node, "cid"))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("missing cid for vsock socket address"));
+        return -1;
+    }
+
+    if (!(vsock->port = virXMLPropString(node, "port"))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("missing port for vsock socket address"));
+        return -1;
+    }
+
+    return 0;
+}
+
+
+static int
+virDomainFdSocketAddressDefParseXML(FdSocketAddress *fd,
+                                    xmlNodePtr node)
+{
+    if (!(fd->str = virXMLPropString(node, "str"))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("missing fd for fd socket address"));
+        return -1;
+    }
+
+    return 0;
+}
+
+
+static int
+virDomainTDXQGSDefParseXML(virDomainTDXDef *def, xmlXPathContextPtr ctxt)
+{
+    g_autofree xmlNodePtr *nodes = NULL;
+    SocketAddress *sa = &def->qgs_sa;
+    xmlNodePtr node;
+    int n;
+
+    if ((n = virXPathNodeSet("./quoteGenerationService/SocketAddress",
+                             ctxt, &nodes)) < 0)
+        return -1;
+
+    if (!n)
+        return 0;
+
+    if (n > 1) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("only a single QGS element is supported"));
+        return -1;
+    }
+    node = nodes[0];
+
+    if (virXMLPropEnum(node, "type", virDomainSocketAddressTypeFromString,
+                       VIR_XML_PROP_REQUIRED, &sa->type) < 0)
+        return -1;
+
+    switch ((virDomainSocketAddress) def->qgs_sa.type) {
+    case VIR_DOMAIN_SOCKET_ADDRESS_INET:
+        if (virDomainInetSocketAddressDefParseXML(&sa->u.inet, node) < 0)
+            return -1;
+        break;
+    case VIR_DOMAIN_SOCKET_ADDRESS_UNIX:
+        if (virDomainUnixSocketAddressDefParseXML(&sa->u.Unix, node) < 0)
+            return -1;
+        break;
+    case VIR_DOMAIN_SOCKET_ADDRESS_VSOCK:
+        if (virDomainVsockSocketAddressDefParseXML(&sa->u.vsock, node) < 0)
+            return -1;
+        break;
+    case VIR_DOMAIN_SOCKET_ADDRESS_FD:
+        if (virDomainFdSocketAddressDefParseXML(&sa->u.fd, node) < 0)
+            return -1;
+        break;
+    case VIR_DOMAIN_SOCKET_ADDRESS_NONE:
+    case VIR_DOMAIN_SOCKET_ADDRESS_LAST:
+    default:
+        virReportError(VIR_ERR_XML_ERROR,
+                       _("unsupported socket address type '%1$s'"),
+                       virDomainSocketAddressTypeToString(def->qgs_sa.type));
+        return -1;
+    }
+
+    return 0;
+}
+
+
 static int
 virDomainTDXDefParseXML(virDomainTDXDef *def,
                         xmlXPathContextPtr ctxt)
@@ -13668,7 +13861,7 @@ virDomainTDXDefParseXML(virDomainTDXDef *def,
     def->mrowner = virXPathString("string(./mrOwner)", ctxt);
     def->mrownerconfig = virXPathString("string(./mrOwnerConfig)", ctxt);
 
-    return 0;
+    return virDomainTDXQGSDefParseXML(def, ctxt);
 }
 
 
@@ -26652,6 +26845,82 @@ virDomainKeyWrapDefFormat(virBuffer *buf, virDomainKeyWrapDef *keywrap)
 }
 
 
+static void
+virDomainTDXQGSDefFormat(virBuffer *buf, virDomainTDXDef *tdx)
+{
+    SocketAddress *sa = &tdx->qgs_sa;
+
+    if (sa->type == VIR_DOMAIN_SOCKET_ADDRESS_NONE)
+        return;
+
+    virBufferAddLit(buf, "<quoteGenerationService>\n");
+    virBufferAdjustIndent(buf, 2);
+    virBufferAsprintf(buf, "<SocketAddress type='%s'",
+                      virDomainSocketAddressTypeToString(sa->type));
+
+    switch ((virDomainSocketAddress) sa->type) {
+    case VIR_DOMAIN_SOCKET_ADDRESS_INET:
+    {
+        InetSocketAddress *inet = &sa->u.inet;
+
+        virBufferAsprintf(buf, " host='%s' port='%s'", inet->host, inet->port);
+        if (inet->numeric != VIR_TRISTATE_BOOL_ABSENT)
+            virBufferAsprintf(buf, " numeric='%s'",
+                              virTristateBoolTypeToString(inet->numeric));
+        if (inet->to)
+            virBufferAsprintf(buf, " to='%d'", inet->to);
+        if (inet->ipv4 != VIR_TRISTATE_BOOL_ABSENT)
+            virBufferAsprintf(buf, " ipv4='%s'",
+                              virTristateBoolTypeToString(inet->ipv4));
+        if (inet->ipv6 != VIR_TRISTATE_BOOL_ABSENT)
+            virBufferAsprintf(buf, " ipv6='%s'",
+                              virTristateBoolTypeToString(inet->ipv6));
+        if (inet->keep_alive != VIR_TRISTATE_BOOL_ABSENT)
+            virBufferAsprintf(buf, " keep_alive='%s'",
+                              virTristateBoolTypeToString(inet->keep_alive));
+        if (inet->mptcp != VIR_TRISTATE_BOOL_ABSENT)
+            virBufferAsprintf(buf, " mptcp='%s'",
+                              virTristateBoolTypeToString(inet->mptcp));
+         break;
+    }
+    case VIR_DOMAIN_SOCKET_ADDRESS_UNIX:
+    {
+        UnixSocketAddress *Unix = &sa->u.Unix;
+
+        virBufferAsprintf(buf, " path='%s'", Unix->path);
+        if (Unix->abstract != VIR_TRISTATE_BOOL_ABSENT)
+            virBufferAsprintf(buf, " abstract='%s'",
+                              virTristateBoolTypeToString(Unix->abstract));
+        if (Unix->tight != VIR_TRISTATE_BOOL_ABSENT)
+            virBufferAsprintf(buf, " tight='%s'",
+                              virTristateBoolTypeToString(Unix->tight));
+        break;
+    }
+    case VIR_DOMAIN_SOCKET_ADDRESS_VSOCK:
+    {
+        VsockSocketAddress *vsock = &sa->u.vsock;
+
+        virBufferAsprintf(buf, " cid='%s' port='%s'", vsock->cid, vsock->port);
+        break;
+    }
+    case VIR_DOMAIN_SOCKET_ADDRESS_FD:
+    {
+        FdSocketAddress *fd = &sa->u.fd;
+
+        virBufferAsprintf(buf, " str='%s'", fd->str);
+        break;
+    }
+    case VIR_DOMAIN_SOCKET_ADDRESS_NONE:
+    case VIR_DOMAIN_SOCKET_ADDRESS_LAST:
+    default:
+        break;
+    }
+    virBufferAddLit(buf, "/>\n");
+    virBufferAdjustIndent(buf, -2);
+    virBufferAddLit(buf, "</quoteGenerationService>\n");
+}
+
+
 static void
 virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec)
 {
@@ -26699,6 +26968,7 @@ virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec)
             virBufferEscapeString(&childBuf, "<mrOwner>%s</mrOwner>\n", tdx->mrowner);
         if (tdx->mrownerconfig)
             virBufferEscapeString(&childBuf, "<mrOwnerConfig>%s</mrOwnerConfig>\n", tdx->mrownerconfig);
+        virDomainTDXQGSDefFormat(&childBuf, tdx);
 
         break;
     }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index bb4973fce8..15cdb3e0e6 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2852,6 +2852,55 @@ struct _virDomainKeyWrapDef {
     virTristateSwitch dea;
 };
 
+typedef enum {
+    VIR_DOMAIN_SOCKET_ADDRESS_NONE,
+    VIR_DOMAIN_SOCKET_ADDRESS_INET,
+    VIR_DOMAIN_SOCKET_ADDRESS_UNIX,
+    VIR_DOMAIN_SOCKET_ADDRESS_VSOCK,
+    VIR_DOMAIN_SOCKET_ADDRESS_FD,
+
+    VIR_DOMAIN_SOCKET_ADDRESS_LAST
+} virDomainSocketAddress;
+
+typedef struct _InetSocketAddress InetSocketAddress;
+typedef struct _UnixSocketAddress UnixSocketAddress;
+typedef struct _VsockSocketAddress VsockSocketAddress;
+typedef struct _FdSocketAddress FdSocketAddress;
+
+struct _InetSocketAddress {
+    char *host;
+    char *port;
+    bool has_numeric;
+    virTristateBool numeric;
+    bool has_to;
+    unsigned int to;
+    bool has_ipv4;
+    virTristateBool ipv4;
+    bool has_ipv6;
+    virTristateBool ipv6;
+    bool has_keep_alive;
+    virTristateBool keep_alive;
+    bool has_mptcp;
+    virTristateBool mptcp;
+};
+
+struct _UnixSocketAddress {
+    char *path;
+    bool has_abstract;
+    virTristateBool abstract;
+    bool has_tight;
+    virTristateBool tight;
+};
+
+struct _VsockSocketAddress {
+    char *cid;
+    char *port;
+};
+
+struct _FdSocketAddress {
+    char *str;
+};
+
 typedef enum {
     VIR_DOMAIN_LAUNCH_SECURITY_NONE,
     VIR_DOMAIN_LAUNCH_SECURITY_SEV,
@@ -2873,11 +2922,22 @@ struct _virDomainSEVDef {
     virTristateBool kernel_hashes;
 };
 
+typedef struct SocketAddress {
+    virDomainSocketAddress type;
+    union {
+        InetSocketAddress inet;
+        UnixSocketAddress Unix;
+        VsockSocketAddress vsock;
+        FdSocketAddress fd;
+    } u;
+} SocketAddress;
+
 struct _virDomainTDXDef {
     unsigned long long policy;
     char *mrconfigid;
     char *mrowner;
     char *mrownerconfig;
+    SocketAddress qgs_sa;
 };
 
 #define VIR_DOMAIN_TDX_POLICY_DEBUG              0x1
@@ -4258,6 +4318,7 @@ VIR_ENUM_DECL(virDomainCryptoBackend);
 VIR_ENUM_DECL(virDomainShmemModel);
 VIR_ENUM_DECL(virDomainShmemRole);
 VIR_ENUM_DECL(virDomainLaunchSecurity);
+VIR_ENUM_DECL(virDomainSocketAddress);
 /* from libvirt.h */
 VIR_ENUM_DECL(virDomainState);
 VIR_ENUM_DECL(virDomainNostateReason);
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index f6e1782b33..e8cc78992a 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -591,9 +591,115 @@
           <data type="string"/>
         </element>
       </optional>
+      <optional>
+        <element name="quoteGenerationService">
+          <ref name="SocketAddress"/>
+        </element>
+      </optional>
     </interleave>
   </define>
 
+  <define name="SocketAddress">
+    <element name="SocketAddress">
+      <choice>
+        <group>
+          <ref name="InetSocketAddress"/>
+        </group>
+        <group>
+          <ref name="UnixSocketAddress"/>
+        </group>
+        <group>
+          <ref name="VsockSocketAddress"/>
+        </group>
+        <group>
+          <ref name="FdSocketAddress"/>
+        </group>
+      </choice>
+    </element>
+  </define>
+
+  <define name="InetSocketAddress">
+    <attribute name="type">
+      <value>inet</value>
+    </attribute>
+    <attribute name="host">
+      <data type="string"/>
+    </attribute>
+    <attribute name="port">
+      <data type="string"/>
+    </attribute>
+    <optional>
+      <attribute name="numeric">
+        <ref name="virYesNo"/>
+      </attribute>
+    </optional>
+    <optional>
+      <attribute name="to">
+        <ref name="uint16"/>
+      </attribute>
+    </optional>
+    <optional>
+      <attribute name="ipv4">
+        <ref name="virYesNo"/>
+      </attribute>
+    </optional>
+    <optional>
+      <attribute name="ipv6">
+        <ref name="virYesNo"/>
+      </attribute>
+    </optional>
+    <optional>
+      <attribute name="keep_alive">
+        <ref name="virYesNo"/>
+      </attribute>
+    </optional>
+    <optional>
+      <attribute name="mptcp">
+        <ref name="virYesNo"/>
+      </attribute>
+    </optional>
+  </define>
+
+  <define name="UnixSocketAddress">
+    <attribute name="type">
+      <value>unix</value>
+    </attribute>
+    <attribute name="path">
+      <data type="string"/>
+    </attribute>
+    <optional>
+      <attribute name="abstract">
+        <ref name="virYesNo"/>
+      </attribute>
+    </optional>
+    <optional>
+      <attribute name="tight">
+        <ref name="virYesNo"/>
+      </attribute>
+    </optional>
+  </define>
+
+  <define name="VsockSocketAddress">
+    <attribute name="type">
+      <value>vsock</value>
+    </attribute>
+    <attribute name="cid">
+      <data type="string"/>
+    </attribute>
+    <attribute name="port">
+      <data type="string"/>
+    </attribute>
+  </define>
+
+  <define name="FdSocketAddress">
+    <attribute name="type">
+      <value>fd</value>
+    </attribute>
+    <attribute name="str">
+      <data type="string"/>
+    </attribute>
+  </define>
+
   <!--
       Enable or disable perf events for the domain. For each
       of the events the following rules apply:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d212d80038..05fec05cd5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9745,21 +9745,127 @@ qemuBuildPVCommandLine(virDomainObj *vm, virCommand *cmd)
 }
 
 
+static virJSONValue *
+qemuJSONBuildInetSocketAddress(const InetSocketAddress *inet)
+{
+    g_autoptr(virJSONValue) addr = NULL;
+    g_autoptr(virJSONValue) data = NULL;
+
+    if (virJSONValueObjectAdd(&addr,
+                              "s:type", "inet",
+                              "s:host", inet->host,
+                              "s:port", inet->port,
+                              "T:numeric", inet->numeric,
+                              "p:to", inet->to,
+                              "T:ipv4", inet->ipv4,
+                              "T:ipv6", inet->ipv6,
+                              "T:keep-alive", inet->keep_alive,
+                              "T:mptcp", inet->mptcp,
+                              NULL) < 0)
+        return NULL;
+
+    return g_steal_pointer(&addr);
+}
+
+
+static virJSONValue *
+qemuJSONBuildUnixSocketAddress(const UnixSocketAddress *Unix)
+{
+    g_autoptr(virJSONValue) addr = NULL;
+    g_autoptr(virJSONValue) data = NULL;
+
+    if (virJSONValueObjectAdd(&addr,
+                              "s:type", "unix",
+                              "s:path", Unix->path,
+                              "T:abstract", Unix->abstract,
+                              "T:tight", Unix->tight,
+                              NULL) < 0)
+        return NULL;
+
+    return g_steal_pointer(&addr);
+}
+
+
+static virJSONValue *
+qemuJSONBuildVsockSocketAddress(const VsockSocketAddress *vsock)
+{
+    g_autoptr(virJSONValue) addr = NULL;
+    g_autoptr(virJSONValue) data = NULL;
+
+    if (virJSONValueObjectAdd(&addr,
+                              "s:type", "vsock",
+                              "s:cid", vsock->cid,
+                              "s:port", vsock->port,
+                              NULL) < 0)
+        return NULL;
+
+    return g_steal_pointer(&addr);
+}
+
+
+static virJSONValue *
+qemuJSONBuildFdSocketAddress(const FdSocketAddress *fd)
+{
+    g_autoptr(virJSONValue) addr = NULL;
+    g_autoptr(virJSONValue) data = NULL;
+
+    if (virJSONValueObjectAdd(&addr,
+                              "s:type", "fd",
+                              "s:str", fd->str,
+                              NULL) < 0)
+        return NULL;
+
+    return g_steal_pointer(&addr);
+}
+
+
+static virJSONValue *
+qemuBuildTDXQGSCommandLine(virDomainTDXDef *tdx)
+{
+    g_autoptr(virJSONValue) addr = NULL;
+
+    switch ((virDomainSocketAddress) tdx->qgs_sa.type) {
+    case VIR_DOMAIN_SOCKET_ADDRESS_INET:
+        addr = qemuJSONBuildInetSocketAddress(&tdx->qgs_sa.u.inet);
+        break;
+    case VIR_DOMAIN_SOCKET_ADDRESS_UNIX:
+        addr = qemuJSONBuildUnixSocketAddress(&tdx->qgs_sa.u.Unix);
+        break;
+    case VIR_DOMAIN_SOCKET_ADDRESS_VSOCK:
+        addr = qemuJSONBuildVsockSocketAddress(&tdx->qgs_sa.u.vsock);
+        break;
+    case VIR_DOMAIN_SOCKET_ADDRESS_FD:
+        addr = qemuJSONBuildFdSocketAddress(&tdx->qgs_sa.u.fd);
+        break;
+    case VIR_DOMAIN_SOCKET_ADDRESS_NONE:
+    case VIR_DOMAIN_SOCKET_ADDRESS_LAST:
+    default:
+        return NULL;
+    }
+
+    return g_steal_pointer(&addr);
+}
+
+
 static int
 qemuBuildTDXCommandLine(virDomainObj *vm, virCommand *cmd,
                         virDomainTDXDef *tdx)
 {
+    g_autoptr(virJSONValue) addr = NULL;
     g_autoptr(virJSONValue) props = NULL;
     qemuDomainObjPrivate *priv = vm->privateData;
     bool sept_ve_disable = tdx->policy & VIR_DOMAIN_TDX_POLICY_SEPT_VE_DISABLE;
 
     VIR_DEBUG("policy=0x%llx", tdx->policy);
 
+    addr = qemuBuildTDXQGSCommandLine(tdx);
+
     if (qemuMonitorCreateObjectProps(&props, "tdx-guest", "lsec0",
                                      "B:debug", !!(tdx->policy & VIR_DOMAIN_TDX_POLICY_DEBUG),
                                      "S:mrconfigid", tdx->mrconfigid,
                                      "S:mrowner", tdx->mrowner,
                                      "S:mrownerconfig", tdx->mrownerconfig,
+                                     "A:quote-generation-socket", &addr,
                                      NULL) < 0)
         return -1;
 
-- 
2.34.1
Re: [PATCH rfcv4 08/13] Add Intel TDX Quote Generation Service(QGS) support
Posted by Daniel P. Berrangé via Devel 10 months, 3 weeks ago
On Fri, May 24, 2024 at 02:21:23PM +0800, Zhenzhong Duan wrote:
> Add element "quoteGenerationService" to tdx launch security type.
> Currently it contains only one sub-element "SocketAddress".
> 
> "SocketAddress" is modelized according to QEMU QAPI, supporting
> inet, unix, vsock and fd type and variant attributes depending
> on type.
> 
> XML example:
> 
>   <launchSecurity type='tdx'>
>     <policy>0x0</policy>
>     <mrConfigId>xxx</mrConfigId>
>     <mrOwner>xxx</mrOwner>
>     <mrOwnerConfig>xxx</mrOwnerConfig>
>     <quoteGenerationService>
>       <SocketAddress type='vsock' cid='xxx' port='xxx'/>
>     </quoteGenerationService>
>   </launchSecurity>
> 
> QEMU command line example:
>   qemu-system-x86_64 \
>     -object '{"qom-type":"tdx-guest","id":"lsec0","sept-ve-disable":false,"mrconfigid":"xxx","mrowner":"xxx","mrownerconfig":"xxx","quote-generation-socket":{"type":"vsock","cid":"xxx","port":"xxx"}}' \
>     -machine pc-q35-6.0,confidential-guest-support=lsec0
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  src/conf/domain_conf.c            | 272 +++++++++++++++++++++++++++++-
>  src/conf/domain_conf.h            |  61 +++++++
>  src/conf/schemas/domaincommon.rng | 106 ++++++++++++
>  src/qemu/qemu_command.c           | 106 ++++++++++++
>  4 files changed, 544 insertions(+), 1 deletion(-)

> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index bb4973fce8..15cdb3e0e6 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2852,6 +2852,55 @@ struct _virDomainKeyWrapDef {
>      virTristateSwitch dea;
>  };
>  
> +typedef enum {
> +    VIR_DOMAIN_SOCKET_ADDRESS_NONE,
> +    VIR_DOMAIN_SOCKET_ADDRESS_INET,
> +    VIR_DOMAIN_SOCKET_ADDRESS_UNIX,
> +    VIR_DOMAIN_SOCKET_ADDRESS_VSOCK,
> +    VIR_DOMAIN_SOCKET_ADDRESS_FD,
> +
> +    VIR_DOMAIN_SOCKET_ADDRESS_LAST
> +} virDomainSocketAddress;
> +
> +typedef struct _InetSocketAddress InetSocketAddress;
> +typedef struct _UnixSocketAddress UnixSocketAddress;
> +typedef struct _VsockSocketAddress VsockSocketAddress;
> +typedef struct _FdSocketAddress FdSocketAddress;
> +
> +struct _InetSocketAddress {
> +    char *host;
> +    char *port;
> +    bool has_numeric;
> +    virTristateBool numeric;
> +    bool has_to;
> +    unsigned int to;
> +    bool has_ipv4;
> +    virTristateBool ipv4;
> +    bool has_ipv6;
> +    virTristateBool ipv6;
> +    bool has_keep_alive;
> +    virTristateBool keep_alive;
> +    bool has_mptcp;
> +    virTristateBool mptcp;
> +};
> +
> +struct _UnixSocketAddress {
> +    char *path;
> +    bool has_abstract;
> +    virTristateBool abstract;
> +    bool has_tight;
> +    virTristateBool tight;
> +};
> +
> +struct _VsockSocketAddress {
> +    char *cid;
> +    char *port;
> +};
> +
> +struct _FdSocketAddress {
> +    char *str;
> +};
> +
>  typedef enum {
>      VIR_DOMAIN_LAUNCH_SECURITY_NONE,
>      VIR_DOMAIN_LAUNCH_SECURITY_SEV,
> @@ -2873,11 +2922,22 @@ struct _virDomainSEVDef {
>      virTristateBool kernel_hashes;
>  };
>  
> +typedef struct SocketAddress {
> +    virDomainSocketAddress type;
> +    union {
> +        InetSocketAddress inet;
> +        UnixSocketAddress Unix;
> +        VsockSocketAddress vsock;
> +        FdSocketAddress fd;

The 'fd' socket type does not make sense to expose
in libvirt XML. FD passing is something handled
privately between libvirt & QEMU, not a end user
choice.

Going further I don't think InetSocketAddress
makes sense to expose, as QGS has no ability
to listen on IP sockets. It can only do UNIX
sockets or VSock AFAICT. Even vsock looks like
a remnant of the old way of doing attestation
before it was integrated into Linux via sysfs
with the kernel making a call into QEMU.

IOW, AFAICT, for QGS all we actually need is
the ability to set a UNIX socket path in libvirt.
eg

     <quoteGenerationService path="/var/run/tdx-qgs/qgs.socket"/>

and probably libvirt should allow 'path' to be optional
so an app can just do

     <quoteGenerationService/>

and libvirt would fill in the default path which QGS listens
on out of the box....


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 08/13] Add Intel TDX Quote Generation Service(QGS) support
Posted by Duan, Zhenzhong 10 months, 2 weeks ago

>-----Original Message-----
>From: Daniel P. Berrangé <berrange@redhat.com>
>Subject: Re: [PATCH rfcv4 08/13] Add Intel TDX Quote Generation Service(QGS)
>support
>
>On Fri, May 24, 2024 at 02:21:23PM +0800, Zhenzhong Duan wrote:
>> Add element "quoteGenerationService" to tdx launch security type.
>> Currently it contains only one sub-element "SocketAddress".
>>
>> "SocketAddress" is modelized according to QEMU QAPI, supporting
>> inet, unix, vsock and fd type and variant attributes depending
>> on type.
>>
>> XML example:
>>
>>   <launchSecurity type='tdx'>
>>     <policy>0x0</policy>
>>     <mrConfigId>xxx</mrConfigId>
>>     <mrOwner>xxx</mrOwner>
>>     <mrOwnerConfig>xxx</mrOwnerConfig>
>>     <quoteGenerationService>
>>       <SocketAddress type='vsock' cid='xxx' port='xxx'/>
>>     </quoteGenerationService>
>>   </launchSecurity>
>>
>> QEMU command line example:
>>   qemu-system-x86_64 \
>>     -object '{"qom-type":"tdx-guest","id":"lsec0","sept-ve-
>disable":false,"mrconfigid":"xxx","mrowner":"xxx","mrownerconfig":"xxx","quot
>e-generation-socket":{"type":"vsock","cid":"xxx","port":"xxx"}}' \
>>     -machine pc-q35-6.0,confidential-guest-support=lsec0
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>  src/conf/domain_conf.c            | 272 +++++++++++++++++++++++++++++-
>>  src/conf/domain_conf.h            |  61 +++++++
>>  src/conf/schemas/domaincommon.rng | 106 ++++++++++++
>>  src/qemu/qemu_command.c           | 106 ++++++++++++
>>  4 files changed, 544 insertions(+), 1 deletion(-)
>
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index bb4973fce8..15cdb3e0e6 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -2852,6 +2852,55 @@ struct _virDomainKeyWrapDef {
>>      virTristateSwitch dea;
>>  };
>>
>> +typedef enum {
>> +    VIR_DOMAIN_SOCKET_ADDRESS_NONE,
>> +    VIR_DOMAIN_SOCKET_ADDRESS_INET,
>> +    VIR_DOMAIN_SOCKET_ADDRESS_UNIX,
>> +    VIR_DOMAIN_SOCKET_ADDRESS_VSOCK,
>> +    VIR_DOMAIN_SOCKET_ADDRESS_FD,
>> +
>> +    VIR_DOMAIN_SOCKET_ADDRESS_LAST
>> +} virDomainSocketAddress;
>> +
>> +typedef struct _InetSocketAddress InetSocketAddress;
>> +typedef struct _UnixSocketAddress UnixSocketAddress;
>> +typedef struct _VsockSocketAddress VsockSocketAddress;
>> +typedef struct _FdSocketAddress FdSocketAddress;
>> +
>> +struct _InetSocketAddress {
>> +    char *host;
>> +    char *port;
>> +    bool has_numeric;
>> +    virTristateBool numeric;
>> +    bool has_to;
>> +    unsigned int to;
>> +    bool has_ipv4;
>> +    virTristateBool ipv4;
>> +    bool has_ipv6;
>> +    virTristateBool ipv6;
>> +    bool has_keep_alive;
>> +    virTristateBool keep_alive;
>> +    bool has_mptcp;
>> +    virTristateBool mptcp;
>> +};
>> +
>> +struct _UnixSocketAddress {
>> +    char *path;
>> +    bool has_abstract;
>> +    virTristateBool abstract;
>> +    bool has_tight;
>> +    virTristateBool tight;
>> +};
>> +
>> +struct _VsockSocketAddress {
>> +    char *cid;
>> +    char *port;
>> +};
>> +
>> +struct _FdSocketAddress {
>> +    char *str;
>> +};
>> +
>>  typedef enum {
>>      VIR_DOMAIN_LAUNCH_SECURITY_NONE,
>>      VIR_DOMAIN_LAUNCH_SECURITY_SEV,
>> @@ -2873,11 +2922,22 @@ struct _virDomainSEVDef {
>>      virTristateBool kernel_hashes;
>>  };
>>
>> +typedef struct SocketAddress {
>> +    virDomainSocketAddress type;
>> +    union {
>> +        InetSocketAddress inet;
>> +        UnixSocketAddress Unix;
>> +        VsockSocketAddress vsock;
>> +        FdSocketAddress fd;
>
>The 'fd' socket type does not make sense to expose
>in libvirt XML. FD passing is something handled
>privately between libvirt & QEMU, not a end user
>choice.

Yes, I can remove ' FdSocketAddress fd'.

>
>Going further I don't think InetSocketAddress
>makes sense to expose, as QGS has no ability
>to listen on IP sockets. It can only do UNIX
>sockets or VSock AFAICT.

Why can't qemu connect to QGS on a remote host?
Even if connect to QGS on localhost, 127.0.0.1 can be used.

> Even vsock looks like
>a remnant of the old way of doing attestation
>before it was integrated into Linux via sysfs
>with the kernel making a call into QEMU.

Not get, qemu does support vsock, see https://lore.kernel.org/qemu-devel/20240229063726.610065-50-xiaoyao.li@intel.com/

>
>IOW, AFAICT, for QGS all we actually need is
>the ability to set a UNIX socket path in libvirt.
>eg
>
>     <quoteGenerationService path="/var/run/tdx-qgs/qgs.socket"/>

Hmm, then we go back to opaque string, do you change your mind?
See https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/XCE4F4FCRFIP7IVZCFPB6RDWLEWXXT2G/

>
>and probably libvirt should allow 'path' to be optional
>so an app can just do
>
>     <quoteGenerationService/>
>
>and libvirt would fill in the default path which QGS listens
>on out of the box....

Yes if we have such default QGS path, do we?

Thanks
Zhenzhong
Re: [PATCH rfcv4 08/13] Add Intel TDX Quote Generation Service(QGS) support
Posted by Daniel P. Berrangé via Devel 10 months, 2 weeks ago
On Wed, Mar 26, 2025 at 03:29:04AM +0000, Duan, Zhenzhong wrote:
> 
> 
> >-----Original Message-----
> >From: Daniel P. Berrangé <berrange@redhat.com>
> >Subject: Re: [PATCH rfcv4 08/13] Add Intel TDX Quote Generation Service(QGS)
> >support
> >
> >On Fri, May 24, 2024 at 02:21:23PM +0800, Zhenzhong Duan wrote:
> >> Add element "quoteGenerationService" to tdx launch security type.
> >> Currently it contains only one sub-element "SocketAddress".
> >>
> >> "SocketAddress" is modelized according to QEMU QAPI, supporting
> >> inet, unix, vsock and fd type and variant attributes depending
> >> on type.

> >
> >The 'fd' socket type does not make sense to expose
> >in libvirt XML. FD passing is something handled
> >privately between libvirt & QEMU, not a end user
> >choice.
> 
> Yes, I can remove ' FdSocketAddress fd'.
> 
> >
> >Going further I don't think InetSocketAddress
> >makes sense to expose, as QGS has no ability
> >to listen on IP sockets. It can only do UNIX
> >sockets or VSock AFAICT.
> 
> Why can't qemu connect to QGS on a remote hoste?

QGS has no IP support, and the work that QGS
performs to create a signed attestation report
has to be done on the same host that the VM
runs on.

> Even if connect to QGS on localhost, 127.0.0.1 can be used.

Again QGS has no IP support, and 127.0.0.1 offers
no benefits over UNIX sockets, while having worse
security.

> > Even vsock looks like
> >a remnant of the old way of doing attestation
> >before it was integrated into Linux via sysfs
> >with the kernel making a call into QEMU.
> 
> Not get, qemu does support vsock, see https://lore.kernel.org/qemu-devel/20240229063726.610065-50-xiaoyao.li@intel.com/

That's simply an artifact of the QEMU patches using QEMU's
generic socket APIs. The QGS vsock support dates from the
earlier days of TDX development, where the guest VM would
be directly connecting to QGS.  With QEMU & the kernel
mediating access to QGS, enabling VSOCK is not required.
VSOCK decreases security of the host - it allows any
running guest (whether confidential or not) to directly
attack QGS via any of its protocol messages.  With QEMU
mediating QGS access, the malicious guest would need to
compromise the kernel, and QEMU before it can access
QGS.

I'm not seeing a compelling functional reason to enable
QEMU to connect to QGS over VSOCK in libvirt, when UNIX
sockets offer the required functionality with greater
security.

> >IOW, AFAICT, for QGS all we actually need is
> >the ability to set a UNIX socket path in libvirt.
> >eg
> >
> >     <quoteGenerationService path="/var/run/tdx-qgs/qgs.socket"/>
> 
> Hmm, then we go back to opaque string, do you change your mind?
> See https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/XCE4F4FCRFIP7IVZCFPB6RDWLEWXXT2G/

The original patch was an opaque string, because it was encoding
various different socket address types in one string.

What I'm proposing is different. Do NOT support different socket
address types initially, only support UNIX sockets, and the only
attribute is thus the bare UNIX socket path.

If we start with only supporting:

    <quoteGenerationService path="/var/run/tdx-qgs/qgs.socket"/>

then in the unlikely even we need more socket types we can extend
this, by adding a "type" attribute defaulting to "unix" if omitted,
so we can then allow a choice

    <quoteGenerationService type="unix" path="/var/run/tdx-qgs/qgs.socket"/>

or

    <quoteGenerationService type="vsock" cid="...." port="..."/>

but hopefully we'll never need this, as UNIX sockets should suffice.

> >and probably libvirt should allow 'path' to be optional
> >so an app can just do
> >
> >     <quoteGenerationService/>
> >
> >and libvirt would fill in the default path which QGS listens
> >on out of the box....
> 
> Yes if we have such default QGS path, do we?

Yes, it is hardcoded in QGS source code:

$ git grep '#define QGS_UNIX_SOCKET_FILE'
server_main.cpp:#define QGS_UNIX_SOCKET_FILE "/var/run/tdx-qgs/qgs.socket"

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 08/13] Add Intel TDX Quote Generation Service(QGS) support
Posted by Duan, Zhenzhong 10 months, 2 weeks ago

>-----Original Message-----
>From: Daniel P. Berrangé <berrange@redhat.com>
>Subject: Re: [PATCH rfcv4 08/13] Add Intel TDX Quote Generation Service(QGS)
>support
>
>On Wed, Mar 26, 2025 at 03:29:04AM +0000, Duan, Zhenzhong wrote:
>>
>>
>> >-----Original Message-----
>> >From: Daniel P. Berrangé <berrange@redhat.com>
>> >Subject: Re: [PATCH rfcv4 08/13] Add Intel TDX Quote Generation Service(QGS)
>> >support
>> >
>> >On Fri, May 24, 2024 at 02:21:23PM +0800, Zhenzhong Duan wrote:
>> >> Add element "quoteGenerationService" to tdx launch security type.
>> >> Currently it contains only one sub-element "SocketAddress".
>> >>
>> >> "SocketAddress" is modelized according to QEMU QAPI, supporting
>> >> inet, unix, vsock and fd type and variant attributes depending
>> >> on type.
>
>> >
>> >The 'fd' socket type does not make sense to expose
>> >in libvirt XML. FD passing is something handled
>> >privately between libvirt & QEMU, not a end user
>> >choice.
>>
>> Yes, I can remove ' FdSocketAddress fd'.
>>
>> >
>> >Going further I don't think InetSocketAddress
>> >makes sense to expose, as QGS has no ability
>> >to listen on IP sockets. It can only do UNIX
>> >sockets or VSock AFAICT.
>>
>> Why can't qemu connect to QGS on a remote hoste?
>
>QGS has no IP support, and the work that QGS
>performs to create a signed attestation report
>has to be done on the same host that the VM
>runs on.
>
>> Even if connect to QGS on localhost, 127.0.0.1 can be used.
>
>Again QGS has no IP support, and 127.0.0.1 offers
>no benefits over UNIX sockets, while having worse
>security.

Clear.

>
>> > Even vsock looks like
>> >a remnant of the old way of doing attestation
>> >before it was integrated into Linux via sysfs
>> >with the kernel making a call into QEMU.
>>
>> Not get, qemu does support vsock, see https://lore.kernel.org/qemu-
>devel/20240229063726.610065-50-xiaoyao.li@intel.com/
>
>That's simply an artifact of the QEMU patches using QEMU's
>generic socket APIs. The QGS vsock support dates from the
>earlier days of TDX development, where the guest VM would
>be directly connecting to QGS.  With QEMU & the kernel
>mediating access to QGS, enabling VSOCK is not required.
>VSOCK decreases security of the host - it allows any
>running guest (whether confidential or not) to directly
>attack QGS via any of its protocol messages.  With QEMU
>mediating QGS access, the malicious guest would need to
>compromise the kernel, and QEMU before it can access
>QGS.

Understood.

>
>I'm not seeing a compelling functional reason to enable
>QEMU to connect to QGS over VSOCK in libvirt, when UNIX
>sockets offer the required functionality with greater

OK.

>security.
>
>> >IOW, AFAICT, for QGS all we actually need is
>> >the ability to set a UNIX socket path in libvirt.
>> >eg
>> >
>> >     <quoteGenerationService path="/var/run/tdx-qgs/qgs.socket"/>
>>
>> Hmm, then we go back to opaque string, do you change your mind?
>> See
>https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/XCE4F4FCRFI
>P7IVZCFPB6RDWLEWXXT2G/
>
>The original patch was an opaque string, because it was encoding
>various different socket address types in one string.
>
>What I'm proposing is different. Do NOT support different socket
>address types initially, only support UNIX sockets, and the only
>attribute is thus the bare UNIX socket path.
>
>If we start with only supporting:
>
>    <quoteGenerationService path="/var/run/tdx-qgs/qgs.socket"/>
>
>then in the unlikely even we need more socket types we can extend
>this, by adding a "type" attribute defaulting to "unix" if omitted,
>so we can then allow a choice
>
>    <quoteGenerationService type="unix" path="/var/run/tdx-qgs/qgs.socket"/>
>
>or
>
>    <quoteGenerationService type="vsock" cid="...." port="..."/>
>
>but hopefully we'll never need this, as UNIX sockets should suffice.

Sounds good, will do this way.

>
>> >and probably libvirt should allow 'path' to be optional
>> >so an app can just do
>> >
>> >     <quoteGenerationService/>
>> >
>> >and libvirt would fill in the default path which QGS listens
>> >on out of the box....
>>
>> Yes if we have such default QGS path, do we?
>
>Yes, it is hardcoded in QGS source code:
>
>$ git grep '#define QGS_UNIX_SOCKET_FILE'
>server_main.cpp:#define QGS_UNIX_SOCKET_FILE "/var/run/tdx-qgs/qgs.socket"

OK, will do.

Thanks
Zhenzhong
Re: [PATCH rfcv4 08/13] Add Intel TDX Quote Generation Service(QGS) support
Posted by Daniel P. Berrangé 1 year, 8 months ago
On Fri, May 24, 2024 at 02:21:23PM +0800, Zhenzhong Duan wrote:
> Add element "quoteGenerationService" to tdx launch security type.
> Currently it contains only one sub-element "SocketAddress".
> 
> "SocketAddress" is modelized according to QEMU QAPI, supporting
> inet, unix, vsock and fd type and variant attributes depending
> on type.
> 
> XML example:
> 
>   <launchSecurity type='tdx'>
>     <policy>0x0</policy>
>     <mrConfigId>xxx</mrConfigId>
>     <mrOwner>xxx</mrOwner>
>     <mrOwnerConfig>xxx</mrOwnerConfig>
>     <quoteGenerationService>
>       <SocketAddress type='vsock' cid='xxx' port='xxx'/>

Libvirt doesn't usually have initial capitals in any XML elements/attrs.
I think everything from <SocketAddress> could be put on the
<quoteGenerationService> element directly.

>     </quoteGenerationService>
>   </launchSecurity>
> 
> QEMU command line example:
>   qemu-system-x86_64 \
>     -object '{"qom-type":"tdx-guest","id":"lsec0","sept-ve-disable":false,"mrconfigid":"xxx","mrowner":"xxx","mrownerconfig":"xxx","quote-generation-socket":{"type":"vsock","cid":"xxx","port":"xxx"}}' \
>     -machine pc-q35-6.0,confidential-guest-support=lsec0
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  src/conf/domain_conf.c            | 272 +++++++++++++++++++++++++++++-
>  src/conf/domain_conf.h            |  61 +++++++
>  src/conf/schemas/domaincommon.rng | 106 ++++++++++++
>  src/qemu/qemu_command.c           | 106 ++++++++++++
>  4 files changed, 544 insertions(+), 1 deletion(-)


> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index bb4973fce8..15cdb3e0e6 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2852,6 +2852,55 @@ struct _virDomainKeyWrapDef {
>      virTristateSwitch dea;
>  };
>  
> +typedef enum {
> +    VIR_DOMAIN_SOCKET_ADDRESS_NONE,
> +    VIR_DOMAIN_SOCKET_ADDRESS_INET,
> +    VIR_DOMAIN_SOCKET_ADDRESS_UNIX,
> +    VIR_DOMAIN_SOCKET_ADDRESS_VSOCK,
> +    VIR_DOMAIN_SOCKET_ADDRESS_FD,
> +
> +    VIR_DOMAIN_SOCKET_ADDRESS_LAST
> +} virDomainSocketAddress;
> +
> +typedef struct _InetSocketAddress InetSocketAddress;
> +typedef struct _UnixSocketAddress UnixSocketAddress;
> +typedef struct _VsockSocketAddress VsockSocketAddress;
> +typedef struct _FdSocketAddress FdSocketAddress;
> +
> +struct _InetSocketAddress {
> +    char *host;
> +    char *port;
> +    bool has_numeric;
> +    virTristateBool numeric;
> +    bool has_to;
> +    unsigned int to;
> +    bool has_ipv4;
> +    virTristateBool ipv4;
> +    bool has_ipv6;
> +    virTristateBool ipv6;
> +    bool has_keep_alive;
> +    virTristateBool keep_alive;
> +    bool has_mptcp;
> +    virTristateBool mptcp;
> +};
> +
> +struct _UnixSocketAddress {
> +    char *path;
> +    bool has_abstract;
> +    virTristateBool abstract;
> +    bool has_tight;
> +    virTristateBool tight;
> +};

All of these "has_XXX" fields are redundant. Only 'has_to'
is ever set, and it is never read after that, so that's
a dead store.

> +
> +struct _VsockSocketAddress {
> +    char *cid;
> +    char *port;
> +};
> +
> +struct _FdSocketAddress {
> +    char *str;
> +};
> +
>  typedef enum {
>      VIR_DOMAIN_LAUNCH_SECURITY_NONE,
>      VIR_DOMAIN_LAUNCH_SECURITY_SEV,
> @@ -2873,11 +2922,22 @@ struct _virDomainSEVDef {
>      virTristateBool kernel_hashes;
>  };
>  
> +typedef struct SocketAddress {
> +    virDomainSocketAddress type;
> +    union {
> +        InetSocketAddress inet;
> +        UnixSocketAddress Unix;
> +        VsockSocketAddress vsock;
> +        FdSocketAddress fd;
> +    } u;
> +} SocketAddress;
> +
>  struct _virDomainTDXDef {
>      unsigned long long policy;
>      char *mrconfigid;
>      char *mrowner;
>      char *mrownerconfig;
> +    SocketAddress qgs_sa;
>  };
>  
>  #define VIR_DOMAIN_TDX_POLICY_DEBUG              0x1


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 08/13] Add Intel TDX Quote Generation Service(QGS) support
Posted by Duan, Zhenzhong 1 year, 8 months ago

>-----Original Message-----
>From: Daniel P. Berrangé <berrange@redhat.com>
>Subject: Re: [PATCH rfcv4 08/13] Add Intel TDX Quote Generation
>Service(QGS) support
>
>On Fri, May 24, 2024 at 02:21:23PM +0800, Zhenzhong Duan wrote:
>> Add element "quoteGenerationService" to tdx launch security type.
>> Currently it contains only one sub-element "SocketAddress".
>>
>> "SocketAddress" is modelized according to QEMU QAPI, supporting
>> inet, unix, vsock and fd type and variant attributes depending
>> on type.
>>
>> XML example:
>>
>>   <launchSecurity type='tdx'>
>>     <policy>0x0</policy>
>>     <mrConfigId>xxx</mrConfigId>
>>     <mrOwner>xxx</mrOwner>
>>     <mrOwnerConfig>xxx</mrOwnerConfig>
>>     <quoteGenerationService>
>>       <SocketAddress type='vsock' cid='xxx' port='xxx'/>
>
>Libvirt doesn't usually have initial capitals in any XML elements/attrs.
>I think everything from <SocketAddress> could be put on the
><quoteGenerationService> element directly.

Got it, will do.

>
>>     </quoteGenerationService>
>>   </launchSecurity>
>>
>> QEMU command line example:
>>   qemu-system-x86_64 \
>>     -object '{"qom-type":"tdx-guest","id":"lsec0","sept-ve-
>disable":false,"mrconfigid":"xxx","mrowner":"xxx","mrownerconfig":"xxx","q
>uote-generation-socket":{"type":"vsock","cid":"xxx","port":"xxx"}}' \
>>     -machine pc-q35-6.0,confidential-guest-support=lsec0
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>  src/conf/domain_conf.c            | 272
>+++++++++++++++++++++++++++++-
>>  src/conf/domain_conf.h            |  61 +++++++
>>  src/conf/schemas/domaincommon.rng | 106 ++++++++++++
>>  src/qemu/qemu_command.c           | 106 ++++++++++++
>>  4 files changed, 544 insertions(+), 1 deletion(-)
>
>
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index bb4973fce8..15cdb3e0e6 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -2852,6 +2852,55 @@ struct _virDomainKeyWrapDef {
>>      virTristateSwitch dea;
>>  };
>>
>> +typedef enum {
>> +    VIR_DOMAIN_SOCKET_ADDRESS_NONE,
>> +    VIR_DOMAIN_SOCKET_ADDRESS_INET,
>> +    VIR_DOMAIN_SOCKET_ADDRESS_UNIX,
>> +    VIR_DOMAIN_SOCKET_ADDRESS_VSOCK,
>> +    VIR_DOMAIN_SOCKET_ADDRESS_FD,
>> +
>> +    VIR_DOMAIN_SOCKET_ADDRESS_LAST
>> +} virDomainSocketAddress;
>> +
>> +typedef struct _InetSocketAddress InetSocketAddress;
>> +typedef struct _UnixSocketAddress UnixSocketAddress;
>> +typedef struct _VsockSocketAddress VsockSocketAddress;
>> +typedef struct _FdSocketAddress FdSocketAddress;
>> +
>> +struct _InetSocketAddress {
>> +    char *host;
>> +    char *port;
>> +    bool has_numeric;
>> +    virTristateBool numeric;
>> +    bool has_to;
>> +    unsigned int to;
>> +    bool has_ipv4;
>> +    virTristateBool ipv4;
>> +    bool has_ipv6;
>> +    virTristateBool ipv6;
>> +    bool has_keep_alive;
>> +    virTristateBool keep_alive;
>> +    bool has_mptcp;
>> +    virTristateBool mptcp;
>> +};
>> +
>> +struct _UnixSocketAddress {
>> +    char *path;
>> +    bool has_abstract;
>> +    virTristateBool abstract;
>> +    bool has_tight;
>> +    virTristateBool tight;
>> +};
>
>All of these "has_XXX" fields are redundant. Only 'has_to'
>is ever set, and it is never read after that, so that's
>a dead store.

Good catch, I copied from qemu QAPI but forgot to cleanup.
I'll remove them all except 'has_to'.

Thanks
Zhenzhong