[libvirt][PATCH v4 1/4] conf: Introduce SGX related element into domain xml

Huang, Haibin haibin.huang at intel.com
Fri Jul 9 00:25:47 UTC 2021



> -----Original Message-----
> From: Tim Wiederhake <twiederh at redhat.com>
> Sent: Monday, July 5, 2021 7:32 PM
> To: Huang, Haibin <haibin.huang at intel.com>
> Cc: libvir-list at redhat.com; Ding, Jian-feng <jian-feng.ding at intel.com>; Yang,
> Lin A <lin.a.yang at intel.com>; Lu, Lianhao <lianhao.lu at intel.com>
> Subject: Re: [libvirt][PATCH v4 1/4] conf: Introduce SGX related element into
> domain xml
> 
> On Thu, 2021-07-01 at 20:10 +0800, Haibin Huang wrote:
> > From: Lin Yang <lin.a.yang at intel.com>
> >
> >   <launchSecurity type='sgx'>
> >     <epc_size unit='KiB'>1024</epc_size>
> >   </launchSecurity>
> 
> Please also update "docs/schemas/domaincommon.rng".
[Haibin] ok, I will do it.
> 
> > ---
> >  src/conf/domain_conf.c  | 106 +++++++++++++++++++++++++++++-------
> --
> > --
> >  src/conf/domain_conf.h  |  10 ++++
> >  src/conf/virconftypes.h |   3 ++
> >  3 files changed, 91 insertions(+), 28 deletions(-)
> >
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index
> > ef67efa1da..4336dafd82 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -1336,6 +1336,7 @@ VIR_ENUM_IMPL(virDomainLaunchSecurity,
> >                VIR_DOMAIN_LAUNCH_SECURITY_LAST,
> >                "",
> >                "sev",
> > +              "sgx",
> >  );
> >
> >  static virClassPtr virDomainObjClass; @@ -3409,6 +3410,16 @@
> > virDomainSEVDefFree(virDomainSEVDefPtr def)
> >  }
> >
> >
> > +static void
> > +virDomainSGXDefFree(virDomainSGXDefPtr def) {
> > +    if (!def)
> > +        return;
> > +
> > +    VIR_FREE(def);
> > +}
> > +
> > +
> >  void virDomainDefFree(virDomainDefPtr def)
> >  {
> >      size_t i;
> > @@ -3597,6 +3608,7 @@ void virDomainDefFree(virDomainDefPtr def)
> >          (def->ns.free)(def->namespaceData);
> >
> >      virDomainSEVDefFree(def->sev);
> > +    virDomainSGXDefFree(def->sgx);
> >
> >      xmlFreeNode(def->metadata);
> >
> > @@ -16700,39 +16712,17 @@
> virDomainMemoryTargetDefParseXML(xmlNodePtr
> > node,
> >      return 0;
> >  }
> >
> > -
> >  static virDomainSEVDefPtr
> > -virDomainSEVDefParseXML(xmlNodePtr sevNode,
> > -                        xmlXPathContextPtr ctxt)
> > +virDomainSEVDefParseXML(xmlXPathContextPtr ctxt)
> >  {
> >      VIR_XPATH_NODE_AUTORESTORE(ctxt);
> >      virDomainSEVDefPtr def;
> >      unsigned long policy;
> > -    g_autofree char *type = NULL;
> >
> >      if (VIR_ALLOC(def) < 0)
> >          return NULL;
> >
> > -    ctxt->node = sevNode;
> > -
> > -    if (!(type = virXMLPropString(sevNode, "type"))) {
> > -        virReportError(VIR_ERR_XML_ERROR, "%s",
> > -                       _("missing launch security type"));
> > -        goto error;
> > -    }
> > -
> > -    def->sectype = virDomainLaunchSecurityTypeFromString(type);
> > -    switch ((virDomainLaunchSecurity) def->sectype) {
> > -    case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
> > -        break;
> > -    case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
> > -    case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
> > -    default:
> > -        virReportError(VIR_ERR_XML_ERROR,
> > -                       _("unsupported launch security type '%s'"),
> > -                       type);
> > -        goto error;
> > -    }
> > +    def->sectype = VIR_DOMAIN_LAUNCH_SECURITY_SEV;
> >
> >      if (virXPathUInt("string(./cbitpos)", ctxt, &def->cbitpos) < 0) {
> >          virReportError(VIR_ERR_XML_ERROR, "%s", @@ -16764,6 +16754,63
> > @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
> >      return NULL;
> >  }
> >
> > +static virDomainSGXDefPtr
> > +virDomainSGXDefParseXML(xmlXPathContextPtr ctxt) {
> > +    VIR_XPATH_NODE_AUTORESTORE(ctxt);
> 
> I do not believe that this is necessary.
[Haibin] ok, I will remove " VIR_XPATH_NODE_AUTORESTORE(ctxt);"
> 
> > +    virDomainSGXDefPtr def;
> > +
> > +    if (VIR_ALLOC(def) < 0)
> > +        return NULL;
> > +
> > +    def->sectype = VIR_DOMAIN_LAUNCH_SECURITY_SGX;
> > +
> > +    if (virDomainParseMemory("./epc_size", "./epc_size/@unit", ctxt,
> > +                             &def->epc_size, false, false) < 0)
> > +        goto error;
> > +
> > +    return def;
> > +
> > + error:
> > +    virDomainSGXDefFree(def);
> > +    return NULL;
> > +}
> > +
> > +static int
> > +virDomainLaunchSecurityDefParseXML(xmlNodePtr launchSecurityNode,
> > +                                   xmlXPathContextPtr ctxt,
> > +                                   virDomainDefPtr def) {
> > +    VIR_XPATH_NODE_AUTORESTORE(ctxt);
> > +    g_autofree char *type = NULL;
> > +
> > +    ctxt->node = launchSecurityNode;
> > +
> > +    if (!(type = virXMLPropString(launchSecurityNode, "type"))) {
> > +        virReportError(VIR_ERR_XML_ERROR, "%s",
> > +                       _("missing launch security type"));
> > +        return -1;
> > +    }
> > +
> > +    switch ((virDomainLaunchSecurity)
> > virDomainLaunchSecurityTypeFromString(type)) {
> > +    case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
> > +        def->sev = virDomainSEVDefParseXML(ctxt);
> 
> I believe this should return "-1" when "def->sev == NULL".
[Haibin] ok, I will add check code for def->sev.
> 
> > +        break;
> > +    case VIR_DOMAIN_LAUNCH_SECURITY_SGX:
> > +        def->sgx = virDomainSGXDefParseXML(ctxt);
> 
> Similar.
[Haibin] ok, I will add check code for def->sgx.
> 
> Regards,
> Tim
> > +        break;
> > +    case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
> > +    case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
> > +    default:
> > +        virReportError(VIR_ERR_XML_ERROR,
> > +                       _("unsupported launch security type '%s'"),
> > +                       type);
> > +        return -1;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> >  static virDomainMemoryDefPtr
> >  virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt,
> >                             xmlNodePtr memdevNode, @@ -22227,12
> > +22274,15 @@ virDomainDefParseXML(xmlDocPtr xml,
> >      ctxt->node = node;
> >      VIR_FREE(nodes);
> >
> > -    /* Check for SEV feature */
> > -    if ((node = virXPathNode("./launchSecurity", ctxt)) != NULL) {
> > -        def->sev = virDomainSEVDefParseXML(node, ctxt);
> > -        if (!def->sev)
> > +    /* analysis of launch security */
> > +    if ((n = virXPathNodeSet("./launchSecurity", ctxt, &nodes)) < 0)
> > +        goto error;
> > +
> > +    for (i = 0; i < n; i++) {
> > +        if (virDomainLaunchSecurityDefParseXML(nodes[i], ctxt, def)
> > != 0)
> >              goto error;
> >      }
> > +    VIR_FREE(nodes);
> >
> >      /* analysis of memory devices */
> >      if ((n = virXPathNodeSet("./devices/memory", ctxt, &nodes)) < 0)
> > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index
> > 011bf66cb4..88adf461df 100644
> > --- a/src/conf/domain_conf.h
> > +++ b/src/conf/domain_conf.h
> > @@ -2447,6 +2447,7 @@ struct _virDomainKeyWrapDef {
> >  typedef enum {
> >      VIR_DOMAIN_LAUNCH_SECURITY_NONE,
> >      VIR_DOMAIN_LAUNCH_SECURITY_SEV,
> > +    VIR_DOMAIN_LAUNCH_SECURITY_SGX,
> >
> >      VIR_DOMAIN_LAUNCH_SECURITY_LAST,
> >  } virDomainLaunchSecurity;
> > @@ -2462,6 +2463,12 @@ struct _virDomainSEVDef {
> >  };
> >
> >
> > +struct _virDomainSGXDef {
> > +    int sectype; /* enum virDomainLaunchSecurity */
> > +    unsigned long long epc_size; /* kibibytes */ };
> > +
> > +
> >  typedef enum {
> >      VIR_DOMAIN_IOMMU_MODEL_INTEL,
> >      VIR_DOMAIN_IOMMU_MODEL_SMMUV3,
> > @@ -2670,6 +2677,9 @@ struct _virDomainDef {
> >      /* SEV-specific domain */
> >      virDomainSEVDefPtr sev;
> >
> > +    /* SGX-specific domain */
> > +    virDomainSGXDefPtr sgx;
> > +
> >      /* Application-specific custom metadata */
> >      xmlNodePtr metadata;
> >
> > diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index
> > 1c62cde251..084bcc7687 100644
> > --- a/src/conf/virconftypes.h
> > +++ b/src/conf/virconftypes.h
> > @@ -291,6 +291,9 @@ typedef virDomainResourceDef
> > *virDomainResourceDefPtr;
> >  typedef struct _virDomainSEVDef virDomainSEVDef;
> >  typedef virDomainSEVDef *virDomainSEVDefPtr;
> >
> > +typedef struct _virDomainSGXDef virDomainSGXDef; typedef
> > +virDomainSGXDef *virDomainSGXDefPtr;
> > +
> >  typedef struct _virDomainShmemDef virDomainShmemDef;
> >  typedef virDomainShmemDef *virDomainShmemDefPtr;
> >





More information about the libvir-list mailing list