Make use of virDomainLaunchSecurity enum and automatic memory freeing. Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; ---

src/conf/domain_conf.c | 123 +++++++++++++++++++++-------------------- src/conf/domain_conf.h | 2 + 2 files changed, 64 insertions(+), 61 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index af2fd03d3c..93ec50ff5d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3490,7 +3490,7 @@ virDomainResctrlDefFree(virDomainResctrlDef *resctrl) } -static void +void virDomainSEVDefFree(virDomainSEVDef *def) { if (!def) @@ -14719,73 +14719,66 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode, xmlXPathContextPtr ctxt) { VIR_XPATH_NODE_AUTORESTORE(ctxt) - virDomainSEVDef *def; unsigned long policy; g_autofree char *type = NULL; int rc = -1; - def = g_new0(virDomainSEVDef, 1); + g_autoptr(virDomainSEVDef) def = g_new0(virDomainSEVDef, 1); ctxt->node = sevNode; if (!(type = virXMLPropString(sevNode, "type"))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing launch security type")); - goto error; + return NULL; } def->sectype = virDomainLaunchSecurityTypeFromString(type); switch ((virDomainLaunchSecurity) def->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: - break; + if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("failed to get launch security policy for " + "launch security type SEV")); + return NULL; + } + + /* the following attributes are platform dependent and if missing, + * we can autofill them from domain capabilities later + */ + rc = virXPathUInt("string(./cbitpos)", ctxt, &def->cbitpos); + if (rc == 0) { + def->haveCbitpos = true; + } else if (rc == -2) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Invalid format for launch security cbitpos")); + return NULL; + } + + rc = virXPathUInt("string(./reducedPhysBits)", ctxt, + &def->reduced_phys_bits); + if (rc == 0) { + def->haveReducedPhysBits = true; + } else if (rc == -2) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Invalid format for launch security " + "reduced-phys-bits")); + return NULL; + } + + def->policy = policy; + def->dh_cert = virXPathString("string(./dhCert)", ctxt); + def->session = virXPathString("string(./session)", ctxt); + + return g_steal_pointer(&def); 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; - } - - if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("failed to get launch security policy for " - "launch security type SEV")); - goto error; - } - - /* the following attributes are platform dependent and if missing, we can - * autofill them from domain capabilities later - */ - rc = virXPathUInt("string(./cbitpos)", ctxt, &def->cbitpos); - if (rc == 0) { - def->haveCbitpos = true; - } else if (rc == -2) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Invalid format for launch security cbitpos")); - goto error; - } - - rc = virXPathUInt("string(./reducedPhysBits)", ctxt, - &def->reduced_phys_bits); - if (rc == 0) { - def->haveReducedPhysBits = true; - } else if (rc == -2) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Invalid format for launch security " - "reduced-phys-bits")); - goto error; + return NULL; } - - def->policy = policy; - def->dh_cert = virXPathString("string(./dhCert)", ctxt); - def->session = virXPathString("string(./session)", ctxt); - - return def; - - error: - virDomainSEVDefFree(def); - return NULL; } @@ -26844,25 +26837,33 @@ virDomainSEVDefFormat(virBuffer *buf, virDomainSEVDef *sev) if (!sev) return; - virBufferAsprintf(buf, "<launchSecurity type='%s'>\n", - virDomainLaunchSecurityTypeToString(sev->sectype)); - virBufferAdjustIndent(buf, 2); + switch ((virDomainLaunchSecurity) sev->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: { + virBufferAsprintf(buf, "<launchSecurity type='%s'>\n", + virDomainLaunchSecurityTypeToString(sev->sectype)); + virBufferAdjustIndent(buf, 2); - if (sev->haveCbitpos) - virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos); + if (sev->haveCbitpos) + virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos); - if (sev->haveReducedPhysBits) - virBufferAsprintf(buf, "<reducedPhysBits>%d</reducedPhysBits>\n", - sev->reduced_phys_bits); - virBufferAsprintf(buf, "<policy>0x%04x</policy>\n", sev->policy); - if (sev->dh_cert) - virBufferEscapeString(buf, "<dhCert>%s</dhCert>\n", sev->dh_cert); + if (sev->haveReducedPhysBits) + virBufferAsprintf(buf, "<reducedPhysBits>%d</reducedPhysBits>\n", + sev->reduced_phys_bits); + virBufferAsprintf(buf, "<policy>0x%04x</policy>\n", sev->policy); + if (sev->dh_cert) + virBufferEscapeString(buf, "<dhCert>%s</dhCert>\n", sev->dh_cert); - if (sev->session) - virBufferEscapeString(buf, "<session>%s</session>\n", sev->session); + if (sev->session) + virBufferEscapeString(buf, "<session>%s</session>\n", sev->session); - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</launchSecurity>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</launchSecurity>\n"); + break; + } + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + break; + } } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f706c498ff..512c7c8bd7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3285,6 +3285,8 @@ void virDomainRedirdevDefFree(virDomainRedirdevDef *def); void virDomainRedirFilterDefFree(virDomainRedirFilterDef *def); void virDomainShmemDefFree(virDomainShmemDef *def); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainShmemDef, virDomainShmemDefFree); +void virDomainSEVDefFree(virDomainSEVDef *def); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainSEVDef, virDomainSEVDefFree); void virDomainDeviceDefFree(virDomainDeviceDef *def); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainDeviceDef, virDomainDeviceDefFree);

On Tue, Jun 22, 2021 at 03:10:45PM +0200, Boris Fiuczynski wrote: > Make use of virDomainLaunchSecurity enum and automatic memory freeing. > > Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; > --- > src/conf/domain_conf.c | 123 +++++++++++++++++++++-------------------- > src/conf/domain_conf.h | 2 + > 2 files changed, 64 insertions(+), 61 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index af2fd03d3c..93ec50ff5d 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -3490,7 +3490,7 @@ virDomainResctrlDefFree(virDomainResctrlDef *resctrl) > } > > > -static void > +void > virDomainSEVDefFree(virDomainSEVDef *def) > { > if (!def) > @@ -14719,73 +14719,66 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode, > xmlXPathContextPtr ctxt) > { > VIR_XPATH_NODE_AUTORESTORE(ctxt) > - virDomainSEVDef *def; > unsigned long policy; > g_autofree char *type = NULL; > int rc = -1; > No need for this empty line. > - def = g_new0(virDomainSEVDef, 1); > + g_autoptr(virDomainSEVDef) def = g_new0(virDomainSEVDef, 1); > > ctxt->node = sevNode; > > if (!(type = virXMLPropString(sevNode, "type"))) { > virReportError(VIR_ERR_XML_ERROR, "%s", > _("missing launch security type")); > - goto error; > + return NULL; > } > > def->sectype = virDomainLaunchSecurityTypeFromString(type); > switch ((virDomainLaunchSecurity) def->sectype) { > case VIR_DOMAIN_LAUNCH_SECURITY_SEV: > - break; > + if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("failed to get launch security policy for " > + "launch security type SEV")); > + return NULL; > + } > + > + /* the following attributes are platform dependent and if missing, > + * we can autofill them from domain capabilities later > + */ > + rc = virXPathUInt("string(./cbitpos)", ctxt, &def->cbitpos); > + if (rc == 0) { > + def->haveCbitpos = true; > + } else if (rc == -2) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("Invalid format for launch security cbitpos")); > + return NULL; > + } > + > + rc = virXPathUInt("string(./reducedPhysBits)", ctxt, > + &def->reduced_phys_bits); > + if (rc == 0) { > + def->haveReducedPhysBits = true; > + } else if (rc == -2) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("Invalid format for launch security " > + "reduced-phys-bits")); > + return NULL; > + } > + > + def->policy = policy; > + def->dh_cert = virXPathString("string(./dhCert)", ctxt); > + def->session = virXPathString("string(./session)", ctxt); > + > + return g_steal_pointer(&def); > 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; > - } > - > - if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) { > - virReportError(VIR_ERR_XML_ERROR, "%s", > - _("failed to get launch security policy for " > - "launch security type SEV")); > - goto error; > - } > - > - /* the following attributes are platform dependent and if missing, we can > - * autofill them from domain capabilities later > - */ > - rc = virXPathUInt("string(./cbitpos)", ctxt, &def->cbitpos); > - if (rc == 0) { > - def->haveCbitpos = true; > - } else if (rc == -2) { > - virReportError(VIR_ERR_XML_ERROR, "%s", > - _("Invalid format for launch security cbitpos")); > - goto error; > - } > - > - rc = virXPathUInt("string(./reducedPhysBits)", ctxt, > - &def->reduced_phys_bits); > - if (rc == 0) { > - def->haveReducedPhysBits = true; > - } else if (rc == -2) { > - virReportError(VIR_ERR_XML_ERROR, "%s", > - _("Invalid format for launch security " > - "reduced-phys-bits")); > - goto error; > + return NULL; > } > - > - def->policy = policy; > - def->dh_cert = virXPathString("string(./dhCert)", ctxt); > - def->session = virXPathString("string(./session)", ctxt); > - > - return def; > - > - error: > - virDomainSEVDefFree(def); > - return NULL; > } > > > @@ -26844,25 +26837,33 @@ virDomainSEVDefFormat(virBuffer *buf, virDomainSEVDef *sev) > if (!sev) > return; > > - virBufferAsprintf(buf, "<launchSecurity type='%s'>\n", > - virDomainLaunchSecurityTypeToString(sev->sectype)); > - virBufferAdjustIndent(buf, 2); > + switch ((virDomainLaunchSecurity) sev->sectype) { > + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: { > + virBufferAsprintf(buf, "<launchSecurity type='%s'>\n", > + virDomainLaunchSecurityTypeToString(sev->sectype)); I would keep this line out out of the switch as it can be reused by other launchSecurity types. In the spirit of modernizing the parse/format code you can user the following pattern witch will make addition of other types with sub-elements easier: g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; virBufferAsprintf(buf, " type='%s'", virDomainLaunchSecurityTypeToString(sev->sectype)); switch ((virDomainLaunchSecurity) sev->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: if (sev->haveCbitpos) virBufferAsprintf(childBuf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos); ... } virXMLFormatElement(buf, "launchSecurity", &attrBuf, &childBuf); With this helper we don't have to duplicate the code formatting launchSecurity type and it will also correctly handle if there are any sub-elements or not. Otherwise looks good. Pavel

> + virBufferAdjustIndent(buf, 2); > > - if (sev->haveCbitpos) > - virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos); > + if (sev->haveCbitpos) > + virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos); > > - if (sev->haveReducedPhysBits) > - virBufferAsprintf(buf, "<reducedPhysBits>%d</reducedPhysBits>\n", > - sev->reduced_phys_bits); > - virBufferAsprintf(buf, "<policy>0x%04x</policy>\n", sev->policy); > - if (sev->dh_cert) > - virBufferEscapeString(buf, "<dhCert>%s</dhCert>\n", sev->dh_cert); > + if (sev->haveReducedPhysBits) > + virBufferAsprintf(buf, "<reducedPhysBits>%d</reducedPhysBits>\n", > + sev->reduced_phys_bits); > + virBufferAsprintf(buf, "<policy>0x%04x</policy>\n", sev->policy); > + if (sev->dh_cert) > + virBufferEscapeString(buf, "<dhCert>%s</dhCert>\n", sev->dh_cert); > > - if (sev->session) > - virBufferEscapeString(buf, "<session>%s</session>\n", sev->session); > + if (sev->session) > + virBufferEscapeString(buf, "<session>%s</session>\n", sev->session); > > - virBufferAdjustIndent(buf, -2); > - virBufferAddLit(buf, "</launchSecurity>\n"); > + virBufferAdjustIndent(buf, -2); > + virBufferAddLit(buf, "</launchSecurity>\n"); > + break; > + } > + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: > + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: > + break; > + } > } > > > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index f706c498ff..512c7c8bd7 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -3285,6 +3285,8 @@ void virDomainRedirdevDefFree(virDomainRedirdevDef *def); > void virDomainRedirFilterDefFree(virDomainRedirFilterDef *def); > void virDomainShmemDefFree(virDomainShmemDef *def); > G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainShmemDef, virDomainShmemDefFree); > +void virDomainSEVDefFree(virDomainSEVDef *def); > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainSEVDef, virDomainSEVDefFree); > void virDomainDeviceDefFree(virDomainDeviceDef *def); > > G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainDeviceDef, virDomainDeviceDefFree); > -- > 2.30.2 >

Adding virDomainSecDef for general launch security data and moving virDomainSEVDef as an element for SEV data. Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; ---

src/conf/domain_conf.c | 127 +++++++++++++++++++++++------------- src/conf/domain_conf.h | 11 +++- src/conf/virconftypes.h | 2 + src/qemu/qemu_cgroup.c | 4 +- src/qemu/qemu_command.c | 44 +++++++++++-- src/qemu/qemu_driver.c | 3 +- src/qemu/qemu_firmware.c | 33 ++++++---- src/qemu/qemu_namespace.c | 20 ++++-- src/qemu/qemu_process.c | 33 ++++++++-- src/qemu/qemu_validate.c | 22 +++++-- src/security/security_dac.c | 6 +- 11 files changed, 218 insertions(+), 87 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 93ec50ff5d..2bd5210a16 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3502,6 +3502,19 @@ virDomainSEVDefFree(virDomainSEVDef *def) g_free(def); } + +void +virDomainSecDefFree(virDomainSecDef *def) +{ + if (!def) + return; + + virDomainSEVDefFree(def->sev); + + g_free(def); +} + + static void virDomainOSDefClear(virDomainOSDef *os) { @@ -3703,7 +3716,7 @@ void virDomainDefFree(virDomainDef *def) if (def->namespaceData && def->ns.free) (def->ns.free)(def->namespaceData); - virDomainSEVDefFree(def->sev); + virDomainSecDefFree(def->sec); xmlFreeNode(def->metadata); @@ -14720,57 +14733,72 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode, { VIR_XPATH_NODE_AUTORESTORE(ctxt) unsigned long policy; - g_autofree char *type = NULL; int rc = -1; g_autoptr(virDomainSEVDef) def = g_new0(virDomainSEVDef, 1); ctxt->node = sevNode; - if (!(type = virXMLPropString(sevNode, "type"))) { + if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing launch security type")); + _("failed to get launch security policy for " + "launch security type SEV")); return NULL; } - def->sectype = virDomainLaunchSecurityTypeFromString(type); - switch ((virDomainLaunchSecurity) def->sectype) { - case VIR_DOMAIN_LAUNCH_SECURITY_SEV: - if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("failed to get launch security policy for " - "launch security type SEV")); - return NULL; - } + /* the following attributes are platform dependent and if missing, + * we can autofill them from domain capabilities later + */ + rc = virXPathUInt("string(./cbitpos)", ctxt, &def->cbitpos); + if (rc == 0) { + def->haveCbitpos = true; + } else if (rc == -2) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Invalid format for launch security cbitpos")); + return NULL; + } - /* the following attributes are platform dependent and if missing, - * we can autofill them from domain capabilities later - */ - rc = virXPathUInt("string(./cbitpos)", ctxt, &def->cbitpos); - if (rc == 0) { - def->haveCbitpos = true; - } else if (rc == -2) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Invalid format for launch security cbitpos")); - return NULL; - } + rc = virXPathUInt("string(./reducedPhysBits)", ctxt, + &def->reduced_phys_bits); + if (rc == 0) { + def->haveReducedPhysBits = true; + } else if (rc == -2) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Invalid format for launch security " + "reduced-phys-bits")); + return NULL; + } - rc = virXPathUInt("string(./reducedPhysBits)", ctxt, - &def->reduced_phys_bits); - if (rc == 0) { - def->haveReducedPhysBits = true; - } else if (rc == -2) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Invalid format for launch security " - "reduced-phys-bits")); - return NULL; - } + def->policy = policy; + def->dh_cert = virXPathString("string(./dhCert)", ctxt); + def->session = virXPathString("string(./session)", ctxt); + + return g_steal_pointer(&def); +} + + +static virDomainSecDef * +virDomainSecDefParseXML(xmlNodePtr lsecNode, + xmlXPathContextPtr ctxt) +{ + g_autoptr(virDomainSecDef) sec = g_new0(virDomainSecDef, 1); + g_autofree char *type = NULL; - def->policy = policy; - def->dh_cert = virXPathString("string(./dhCert)", ctxt); - def->session = virXPathString("string(./session)", ctxt); + ctxt->node = lsecNode; - return g_steal_pointer(&def); + if (!(type = virXMLPropString(lsecNode, "type"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing launch security type")); + return NULL; + } + + sec->sectype = virDomainLaunchSecurityTypeFromString(type); + switch ((virDomainLaunchSecurity) sec->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + sec->sev = virDomainSEVDefParseXML(lsecNode, ctxt); + if (!sec->sev) + return NULL; + break; case VIR_DOMAIN_LAUNCH_SECURITY_NONE: case VIR_DOMAIN_LAUNCH_SECURITY_LAST: default: @@ -14779,6 +14807,8 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode, type); return NULL; } + + return g_steal_pointer(&sec); } @@ -20098,10 +20128,10 @@ virDomainDefParseXML(xmlDocPtr xml, ctxt->node = node; VIR_FREE(nodes); - /* Check for SEV feature */ + /* Check for launch security e.g. SEV feature */ if ((node = virXPathNode("./launchSecurity", ctxt)) != NULL) { - def->sev = virDomainSEVDefParseXML(node, ctxt); - if (!def->sev) + def->sec = virDomainSecDefParseXML(node, ctxt); + if (!def->sec) goto error; } @@ -26832,15 +26862,19 @@ virDomainKeyWrapDefFormat(virBuffer *buf, virDomainKeyWrapDef *keywrap) static void -virDomainSEVDefFormat(virBuffer *buf, virDomainSEVDef *sev) +virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) { - if (!sev) + if (!sec) return; - switch ((virDomainLaunchSecurity) sev->sectype) { + switch ((virDomainLaunchSecurity) sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: { + virDomainSEVDef *sev = sec->sev; + if (!sev) + return; + virBufferAsprintf(buf, "<launchSecurity type='%s'>\n", - virDomainLaunchSecurityTypeToString(sev->sectype)); + virDomainLaunchSecurityTypeToString(sec->sectype)); virBufferAdjustIndent(buf, 2); if (sev->haveCbitpos) @@ -26860,6 +26894,7 @@ virDomainSEVDefFormat(virBuffer *buf, virDomainSEVDef *sev) virBufferAddLit(buf, "</launchSecurity>\n"); break; } + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: case VIR_DOMAIN_LAUNCH_SECURITY_LAST: break; @@ -28272,7 +28307,7 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, if (def->keywrap) virDomainKeyWrapDefFormat(buf, def->keywrap); - virDomainSEVDefFormat(buf, def->sev); + virDomainSecDefFormat(buf, def->sec); 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 512c7c8bd7..fa7ab1895d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2651,7 +2651,6 @@ typedef enum { struct _virDomainSEVDef { - int sectype; /* enum virDomainLaunchSecurity */ char *dh_cert; char *session; unsigned int policy; @@ -2661,6 +2660,10 @@ struct _virDomainSEVDef { unsigned int reduced_phys_bits; }; +struct _virDomainSecDef { + int sectype; /* enum virDomainLaunchSecurity */ + virDomainSEVDef *sev; +}; typedef enum { VIR_DOMAIN_IOMMU_MODEL_INTEL, @@ -2871,8 +2874,8 @@ struct _virDomainDef { virDomainKeyWrapDef *keywrap; - /* SEV-specific domain */ - virDomainSEVDef *sev; + /* launch security e.g. SEV */ + virDomainSecDef *sec; /* Application-specific custom metadata */ xmlNodePtr metadata; @@ -3287,6 +3290,8 @@ void virDomainShmemDefFree(virDomainShmemDef *def); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainShmemDef, virDomainShmemDefFree); void virDomainSEVDefFree(virDomainSEVDef *def); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainSEVDef, virDomainSEVDefFree); +void virDomainSecDefFree(virDomainSecDef *def); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainSecDef, virDomainSecDefFree); void virDomainDeviceDefFree(virDomainDeviceDef *def); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainDeviceDef, virDomainDeviceDefFree); diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index b21068486e..21420ba8ea 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 _virDomainSecDef virDomainSecDef; + typedef struct _virDomainShmemDef virDomainShmemDef; typedef struct _virDomainSmartcardDef virDomainSmartcardDef; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 038d6478b2..f2d99abcfa 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -856,7 +856,9 @@ qemuSetupDevicesCgroup(virDomainObj *vm) return -1; } - if (vm->def->sev && qemuSetupSEVCgroup(vm) < 0) + if (vm->def->sec && + vm->def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV && + qemuSetupSEVCgroup(vm) < 0) return -1; return 0; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ea513693f7..4135a8444a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6966,11 +6966,20 @@ qemuBuildMachineCommandLine(virCommand *cmd, if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_LOADPARM)) qemuAppendLoadparmMachineParm(&buf, def); - if (def->sev) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT)) { - virBufferAddLit(&buf, ",confidential-guest-support=sev0"); - } else { - virBufferAddLit(&buf, ",memory-encryption=sev0"); + if (def->sec) { + switch ((virDomainLaunchSecurity) def->sec->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT)) { + virBufferAddLit(&buf, ",confidential-guest-support=sev0"); + } else { + virBufferAddLit(&buf, ",memory-encryption=sev0"); + } + break; + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: + break; + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + virReportEnumRangeError(virDomainLaunchSecurity, def->sec->sectype); + return -1; } } @@ -9860,6 +9869,29 @@ qemuBuildSEVCommandLine(virDomainObj *vm, virCommand *cmd, return 0; } + +static int +qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd, + virDomainSecDef *sec) +{ + if (!sec) + return 0; + + switch ((virDomainLaunchSecurity) sec->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + return qemuBuildSEVCommandLine(vm, cmd, sec->sev); + break; + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: + break; + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + virReportEnumRangeError(virDomainLaunchSecurity, sec->sectype); + return -1; + } + + return 0; +} + + static int qemuBuildVMCoreInfoCommandLine(virCommand *cmd, const virDomainDef *def) @@ -10559,7 +10591,7 @@ qemuBuildCommandLine(virQEMUDriver *driver, if (qemuBuildVMCoreInfoCommandLine(cmd, def) < 0) return NULL; - if (qemuBuildSEVCommandLine(vm, cmd, def->sev) < 0) + if (qemuBuildSecCommandLine(vm, cmd, def->sec) < 0) return NULL; if (snapshot) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 235f575901..9973875092 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19830,7 +19830,8 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain, if (virDomainGetLaunchSecurityInfoEnsureACL(domain->conn, vm->def) < 0) goto cleanup; - if (vm->def->sev) { + if (vm->def->sec && + vm->def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { if (qemuDomainGetSEVMeasurement(driver, vm, params, nparams, flags) < 0) goto cleanup; } diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index e17b024b06..6d1bab181e 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -1053,19 +1053,28 @@ qemuFirmwareMatchDomain(const virDomainDef *def, return false; } - if (def->sev && - def->sev->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { - if (!supportsSEV) { - VIR_DEBUG("Domain requires SEV, firmware '%s' doesn't support it", - path); - return false; - } + if (def->sec) { + switch ((virDomainLaunchSecurity) def->sec->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + if (!supportsSEV) { + VIR_DEBUG("Domain requires SEV, firmware '%s' doesn't support it", + path); + return false; + } - if (def->sev->policy & VIR_QEMU_FIRMWARE_AMD_SEV_ES_POLICY && - !supportsSEVES) { - VIR_DEBUG("Domain requires SEV-ES, firmware '%s' doesn't support it", - path); - return false; + if (def->sec->sev && + def->sec->sev->policy & VIR_QEMU_FIRMWARE_AMD_SEV_ES_POLICY && + !supportsSEVES) { + VIR_DEBUG("Domain requires SEV-ES, firmware '%s' doesn't support it", + path); + return false; + } + break; + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: + break; + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + virReportEnumRangeError(virDomainLaunchSecurity, def->sec->sectype); + return -1; } } diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c index 98495e8ef8..35c8eb83fd 100644 --- a/src/qemu/qemu_namespace.c +++ b/src/qemu/qemu_namespace.c @@ -594,16 +594,26 @@ static int qemuDomainSetupLaunchSecurity(virDomainObj *vm, GSList **paths) { - virDomainSEVDef *sev = vm->def->sev; + virDomainSecDef *sec = vm->def->sec; - if (!sev || sev->sectype != VIR_DOMAIN_LAUNCH_SECURITY_SEV) + if (!sec) return 0; - VIR_DEBUG("Setting up launch security"); + switch ((virDomainLaunchSecurity) sec->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + VIR_DEBUG("Setting up launch security for SEV"); - *paths = g_slist_prepend(*paths, g_strdup(QEMU_DEV_SEV)); + *paths = g_slist_prepend(*paths, g_strdup(QEMU_DEV_SEV)); + + VIR_DEBUG("Set up launch security for SEV"); + break; + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: + break; + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + virReportEnumRangeError(virDomainLaunchSecurity, sec->sectype); + return -1; + } - VIR_DEBUG("Set up launch security"); return 0; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2b03b0ab98..d9073fb3a3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6480,7 +6480,7 @@ qemuProcessUpdateSEVInfo(virDomainObj *vm) { qemuDomainObjPrivate *priv = vm->privateData; virQEMUCaps *qemuCaps = priv->qemuCaps; - virDomainSEVDef *sev = vm->def->sev; + virDomainSEVDef *sev = vm->def->sec->sev; virSEVCapability *sevCaps = NULL; /* if platform specific info like 'cbitpos' and 'reducedPhysBits' have @@ -6636,7 +6636,8 @@ qemuProcessPrepareDomain(virQEMUDriver *driver, for (i = 0; i < vm->def->nshmems; i++) qemuDomainPrepareShmemChardev(vm->def->shmems[i]); - if (vm->def->sev) { + if (vm->def->sec && + vm->def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { VIR_DEBUG("Updating SEV platform info"); if (qemuProcessUpdateSEVInfo(vm) < 0) return -1; @@ -6674,10 +6675,10 @@ qemuProcessSEVCreateFile(virDomainObj *vm, static int qemuProcessPrepareSEVGuestInput(virDomainObj *vm) { - virDomainSEVDef *sev = vm->def->sev; + virDomainSEVDef *sev = vm->def->sec->sev; if (!sev) - return 0; + return -1; VIR_DEBUG("Preparing SEV guest"); @@ -6695,6 +6696,28 @@ qemuProcessPrepareSEVGuestInput(virDomainObj *vm) } +static int +qemuProcessPrepareLaunchSecurityGuestInput(virDomainObj *vm) +{ + virDomainSecDef *sec = vm->def->sec; + + if (!sec) + return 0; + + switch ((virDomainLaunchSecurity) sec->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + return qemuProcessPrepareSEVGuestInput(vm); + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: + break; + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + virReportEnumRangeError(virDomainLaunchSecurity, sec->sectype); + return -1; + } + + return 0; +} + + static int qemuProcessPrepareHostStorage(virQEMUDriver *driver, virDomainObj *vm, @@ -6874,7 +6897,7 @@ qemuProcessPrepareHost(virQEMUDriver *driver, if (qemuExtDevicesPrepareHost(driver, vm) < 0) return -1; - if (qemuProcessPrepareSEVGuestInput(vm) < 0) + if (qemuProcessPrepareLaunchSecurityGuestInput(vm) < 0) return -1; return 0; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 382473d03b..957dbc906c 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1214,12 +1214,22 @@ qemuValidateDomainDef(const virDomainDef *def, if (qemuValidateDomainDefPanic(def, qemuCaps) < 0) return -1; - if (def->sev && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("SEV launch security is not supported with " - "this QEMU binary")); - return -1; + if (def->sec) { + switch ((virDomainLaunchSecurity) def->sec->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("SEV launch security is not supported with " + "this QEMU binary")); + return -1; + } + break; + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: + break; + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + virReportEnumRangeError(virDomainLaunchSecurity, def->sec->sectype); + return -1; + } } if (def->naudios > 1 && diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 4909107fcc..b874dd4ab6 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1958,7 +1958,8 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr, rc = -1; } - if (def->sev) { + if (def->sec && + def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { if (virSecurityDACRestoreSEVLabel(mgr, def) < 0) rc = -1; } @@ -2165,7 +2166,8 @@ virSecurityDACSetAllLabel(virSecurityManager *mgr, return -1; } - if (def->sev) { + if (def->sec && + def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { if (virSecurityDACSetSEVLabel(mgr, def) < 0) return -1; }

On Tue, Jun 22, 2021 at 03:10:46PM +0200, Boris Fiuczynski wrote: > Adding virDomainSecDef for general launch security data > and moving virDomainSEVDef as an element for SEV data. > > Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; > --- > src/conf/domain_conf.c | 127 +++++++++++++++++++++++------------- > src/conf/domain_conf.h | 11 +++- > src/conf/virconftypes.h | 2 + > src/qemu/qemu_cgroup.c | 4 +- > src/qemu/qemu_command.c | 44 +++++++++++-- > src/qemu/qemu_driver.c | 3 +- > src/qemu/qemu_firmware.c | 33 ++++++---- > src/qemu/qemu_namespace.c | 20 ++++-- > src/qemu/qemu_process.c | 33 ++++++++-- > src/qemu/qemu_validate.c | 22 +++++-- > src/security/security_dac.c | 6 +- > 11 files changed, 218 insertions(+), 87 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 93ec50ff5d..2bd5210a16 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -3502,6 +3502,19 @@ virDomainSEVDefFree(virDomainSEVDef *def) > g_free(def); > } > > + > +void > +virDomainSecDefFree(virDomainSecDef *def) > +{ > + if (!def) > + return; > + > + virDomainSEVDefFree(def->sev); > + > + g_free(def); > +} > + > + > static void > virDomainOSDefClear(virDomainOSDef *os) > { > @@ -3703,7 +3716,7 @@ void virDomainDefFree(virDomainDef *def) > if (def->namespaceData && def->ns.free) > (def->ns.free)(def->namespaceData); > > - virDomainSEVDefFree(def->sev); > + virDomainSecDefFree(def->sec); > > xmlFreeNode(def->metadata); > > @@ -14720,57 +14733,72 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode, > { > VIR_XPATH_NODE_AUTORESTORE(ctxt) > unsigned long policy; > - g_autofree char *type = NULL; > int rc = -1; > > g_autoptr(virDomainSEVDef) def = g_new0(virDomainSEVDef, 1); > > ctxt->node = sevNode; > > - if (!(type = virXMLPropString(sevNode, "type"))) { > + if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) { > virReportError(VIR_ERR_XML_ERROR, "%s", > - _("missing launch security type")); > + _("failed to get launch security policy for " > + "launch security type SEV")); > return NULL; > } > > - def->sectype = virDomainLaunchSecurityTypeFromString(type); > - switch ((virDomainLaunchSecurity) def->sectype) { > - case VIR_DOMAIN_LAUNCH_SECURITY_SEV: > - if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) { > - virReportError(VIR_ERR_XML_ERROR, "%s", > - _("failed to get launch security policy for " > - "launch security type SEV")); > - return NULL; > - } > + /* the following attributes are platform dependent and if missing, > + * we can autofill them from domain capabilities later > + */ > + rc = virXPathUInt("string(./cbitpos)", ctxt, &def->cbitpos); > + if (rc == 0) { > + def->haveCbitpos = true; > + } else if (rc == -2) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("Invalid format for launch security cbitpos")); > + return NULL; > + } > > - /* the following attributes are platform dependent and if missing, > - * we can autofill them from domain capabilities later > - */ > - rc = virXPathUInt("string(./cbitpos)", ctxt, &def->cbitpos); > - if (rc == 0) { > - def->haveCbitpos = true; > - } else if (rc == -2) { > - virReportError(VIR_ERR_XML_ERROR, "%s", > - _("Invalid format for launch security cbitpos")); > - return NULL; > - } > + rc = virXPathUInt("string(./reducedPhysBits)", ctxt, > + &def->reduced_phys_bits); > + if (rc == 0) { > + def->haveReducedPhysBits = true; > + } else if (rc == -2) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("Invalid format for launch security " > + "reduced-phys-bits")); > + return NULL; > + } > > - rc = virXPathUInt("string(./reducedPhysBits)", ctxt, > - &def->reduced_phys_bits); > - if (rc == 0) { > - def->haveReducedPhysBits = true; > - } else if (rc == -2) { > - virReportError(VIR_ERR_XML_ERROR, "%s", > - _("Invalid format for launch security " > - "reduced-phys-bits")); > - return NULL; > - } > + def->policy = policy; > + def->dh_cert = virXPathString("string(./dhCert)", ctxt); > + def->session = virXPathString("string(./session)", ctxt); > + > + return g_steal_pointer(&def); > +} > + > + > +static virDomainSecDef * > +virDomainSecDefParseXML(xmlNodePtr lsecNode, > + xmlXPathContextPtr ctxt) > +{ > + g_autoptr(virDomainSecDef) sec = g_new0(virDomainSecDef, 1); > + g_autofree char *type = NULL; > > - def->policy = policy; > - def->dh_cert = virXPathString("string(./dhCert)", ctxt); > - def->session = virXPathString("string(./session)", ctxt); > + ctxt->node = lsecNode; > > - return g_steal_pointer(&def); > + if (!(type = virXMLPropString(lsecNode, "type"))) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("missing launch security type")); > + return NULL; > + } > + > + sec->sectype = virDomainLaunchSecurityTypeFromString(type); > + switch ((virDomainLaunchSecurity) sec->sectype) { > + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: > + sec->sev = virDomainSEVDefParseXML(lsecNode, ctxt); > + if (!sec->sev) > + return NULL; > + break; > case VIR_DOMAIN_LAUNCH_SECURITY_NONE: > case VIR_DOMAIN_LAUNCH_SECURITY_LAST: > default: > @@ -14779,6 +14807,8 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode, > type); > return NULL; > } > + > + return g_steal_pointer(&sec); > } > > > @@ -20098,10 +20128,10 @@ virDomainDefParseXML(xmlDocPtr xml, > ctxt->node = node; > VIR_FREE(nodes); > > - /* Check for SEV feature */ > + /* Check for launch security e.g. SEV feature */ > if ((node = virXPathNode("./launchSecurity", ctxt)) != NULL) { > - def->sev = virDomainSEVDefParseXML(node, ctxt); > - if (!def->sev) > + def->sec = virDomainSecDefParseXML(node, ctxt); > + if (!def->sec) > goto error; > } > > @@ -26832,15 +26862,19 @@ virDomainKeyWrapDefFormat(virBuffer *buf, virDomainKeyWrapDef *keywrap) > > > static void > -virDomainSEVDefFormat(virBuffer *buf, virDomainSEVDef *sev) > +virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) > { > - if (!sev) > + if (!sec) > return; > > - switch ((virDomainLaunchSecurity) sev->sectype) { > + switch ((virDomainLaunchSecurity) sec->sectype) { > case VIR_DOMAIN_LAUNCH_SECURITY_SEV: { > + virDomainSEVDef *sev = sec->sev; > + if (!sev) > + return; > + > virBufferAsprintf(buf, "<launchSecurity type='%s'>\n", > - virDomainLaunchSecurityTypeToString(sev->sectype)); > + virDomainLaunchSecurityTypeToString(sec->sectype)); > virBufferAdjustIndent(buf, 2); > > if (sev->haveCbitpos) > @@ -26860,6 +26894,7 @@ virDomainSEVDefFormat(virBuffer *buf, virDomainSEVDef *sev) > virBufferAddLit(buf, "</launchSecurity>\n"); > break; > } > + > case VIR_DOMAIN_LAUNCH_SECURITY_NONE: > case VIR_DOMAIN_LAUNCH_SECURITY_LAST: > break; > @@ -28272,7 +28307,7 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, > if (def->keywrap) > virDomainKeyWrapDefFormat(buf, def->keywrap); > > - virDomainSEVDefFormat(buf, def->sev); > + virDomainSecDefFormat(buf, def->sec); > > 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 512c7c8bd7..fa7ab1895d 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -2651,7 +2651,6 @@ typedef enum { > > > struct _virDomainSEVDef { > - int sectype; /* enum virDomainLaunchSecurity */ > char *dh_cert; > char *session; > unsigned int policy; > @@ -2661,6 +2660,10 @@ struct _virDomainSEVDef { > unsigned int reduced_phys_bits; > }; > > +struct _virDomainSecDef { > + int sectype; /* enum virDomainLaunchSecurity */ > + virDomainSEVDef *sev; I would rather use union here like we do in other similar internal structures: struct _virDomainSecDef { int sectype; /* enum virDomainLaunchSecurity */ union data { virDomainSEVDef sev; } } or struct _virDomainSecDef { int sectype; /* enum virDomainLaunchSecurity */ union data { virDomainSEVDef *sev; } } depending if we need to have the specific SEV structure as pointer or not based on its usage. I personally think we can do it without the pointer as it should not happen that sectype will be set to SEV but we will not have any data.

Pavel > +}; > > typedef enum { > VIR_DOMAIN_IOMMU_MODEL_INTEL, > @@ -2871,8 +2874,8 @@ struct _virDomainDef { > > virDomainKeyWrapDef *keywrap; > > - /* SEV-specific domain */ > - virDomainSEVDef *sev; > + /* launch security e.g. SEV */ > + virDomainSecDef *sec; > > /* Application-specific custom metadata */ > xmlNodePtr metadata; > @@ -3287,6 +3290,8 @@ void virDomainShmemDefFree(virDomainShmemDef *def); > G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainShmemDef, virDomainShmemDefFree); > void virDomainSEVDefFree(virDomainSEVDef *def); > G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainSEVDef, virDomainSEVDefFree); > +void virDomainSecDefFree(virDomainSecDef *def); > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainSecDef, virDomainSecDefFree); > void virDomainDeviceDefFree(virDomainDeviceDef *def); > > G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainDeviceDef, virDomainDeviceDefFree); > diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h > index b21068486e..21420ba8ea 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 _virDomainSecDef virDomainSecDef; > + > typedef struct _virDomainShmemDef virDomainShmemDef; > > typedef struct _virDomainSmartcardDef virDomainSmartcardDef; > diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c > index 038d6478b2..f2d99abcfa 100644 > --- a/src/qemu/qemu_cgroup.c > +++ b/src/qemu/qemu_cgroup.c > @@ -856,7 +856,9 @@ qemuSetupDevicesCgroup(virDomainObj *vm) > return -1; > } > > - if (vm->def->sev && qemuSetupSEVCgroup(vm) < 0) > + if (vm->def->sec && > + vm->def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV && > + qemuSetupSEVCgroup(vm) < 0) > return -1; > > return 0; > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index ea513693f7..4135a8444a 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -6966,11 +6966,20 @@ qemuBuildMachineCommandLine(virCommand *cmd, > if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_LOADPARM)) > qemuAppendLoadparmMachineParm(&buf, def); > > - if (def->sev) { > - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT)) { > - virBufferAddLit(&buf, ",confidential-guest-support=sev0"); > - } else { > - virBufferAddLit(&buf, ",memory-encryption=sev0"); > + if (def->sec) { > + switch ((virDomainLaunchSecurity) def->sec->sectype) { > + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: > + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT)) { > + virBufferAddLit(&buf, ",confidential-guest-support=sev0"); > + } else { > + virBufferAddLit(&buf, ",memory-encryption=sev0"); > + } > + break; > + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: > + break; > + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: > + virReportEnumRangeError(virDomainLaunchSecurity, def->sec->sectype); > + return -1; > } > } > > @@ -9860,6 +9869,29 @@ qemuBuildSEVCommandLine(virDomainObj *vm, virCommand *cmd, > return 0; > } > > + > +static int > +qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd, > + virDomainSecDef *sec) > +{ > + if (!sec) > + return 0; > + > + switch ((virDomainLaunchSecurity) sec->sectype) { > + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: > + return qemuBuildSEVCommandLine(vm, cmd, sec->sev); > + break; > + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: > + break; > + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: > + virReportEnumRangeError(virDomainLaunchSecurity, sec->sectype); > + return -1; > + } > + > + return 0; > +} > + > + > static int > qemuBuildVMCoreInfoCommandLine(virCommand *cmd, > const virDomainDef *def) > @@ -10559,7 +10591,7 @@ qemuBuildCommandLine(virQEMUDriver *driver, > if (qemuBuildVMCoreInfoCommandLine(cmd, def) < 0) > return NULL; > > - if (qemuBuildSEVCommandLine(vm, cmd, def->sev) < 0) > + if (qemuBuildSecCommandLine(vm, cmd, def->sec) < 0) > return NULL; > > if (snapshot) > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 235f575901..9973875092 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -19830,7 +19830,8 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain, > if (virDomainGetLaunchSecurityInfoEnsureACL(domain->conn, vm->def) < 0) > goto cleanup; > > - if (vm->def->sev) { > + if (vm->def->sec && > + vm->def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { > if (qemuDomainGetSEVMeasurement(driver, vm, params, nparams, flags) < 0) > goto cleanup; > } > diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c > index e17b024b06..6d1bab181e 100644 > --- a/src/qemu/qemu_firmware.c > +++ b/src/qemu/qemu_firmware.c > @@ -1053,19 +1053,28 @@ qemuFirmwareMatchDomain(const virDomainDef *def, > return false; > } > > - if (def->sev && > - def->sev->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { > - if (!supportsSEV) { > - VIR_DEBUG("Domain requires SEV, firmware '%s' doesn't support it", > - path); > - return false; > - } > + if (def->sec) { > + switch ((virDomainLaunchSecurity) def->sec->sectype) { > + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: > + if (!supportsSEV) { > + VIR_DEBUG("Domain requires SEV, firmware '%s' doesn't support it", > + path); > + return false; > + } > > - if (def->sev->policy & VIR_QEMU_FIRMWARE_AMD_SEV_ES_POLICY && > - !supportsSEVES) { > - VIR_DEBUG("Domain requires SEV-ES, firmware '%s' doesn't support it", > - path); > - return false; > + if (def->sec->sev && > + def->sec->sev->policy & VIR_QEMU_FIRMWARE_AMD_SEV_ES_POLICY && > + !supportsSEVES) { > + VIR_DEBUG("Domain requires SEV-ES, firmware '%s' doesn't support it", > + path); > + return false; > + } > + break; > + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: > + break; > + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: > + virReportEnumRangeError(virDomainLaunchSecurity, def->sec->sectype); > + return -1; > } > } > > diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c > index 98495e8ef8..35c8eb83fd 100644 > --- a/src/qemu/qemu_namespace.c > +++ b/src/qemu/qemu_namespace.c > @@ -594,16 +594,26 @@ static int > qemuDomainSetupLaunchSecurity(virDomainObj *vm, > GSList **paths) > { > - virDomainSEVDef *sev = vm->def->sev; > + virDomainSecDef *sec = vm->def->sec; > > - if (!sev || sev->sectype != VIR_DOMAIN_LAUNCH_SECURITY_SEV) > + if (!sec) > return 0; > > - VIR_DEBUG("Setting up launch security"); > + switch ((virDomainLaunchSecurity) sec->sectype) { > + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: > + VIR_DEBUG("Setting up launch security for SEV"); > > - *paths = g_slist_prepend(*paths, g_strdup(QEMU_DEV_SEV)); > + *paths = g_slist_prepend(*paths, g_strdup(QEMU_DEV_SEV)); > + > + VIR_DEBUG("Set up launch security for SEV"); > + break; > + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: > + break; > + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: > + virReportEnumRangeError(virDomainLaunchSecurity, sec->sectype); > + return -1; > + } > > - VIR_DEBUG("Set up launch security"); > return 0; > } > > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > index 2b03b0ab98..d9073fb3a3 100644 > --- a/src/qemu/qemu_process.c > +++ b/src/qemu/qemu_process.c > @@ -6480,7 +6480,7 @@ qemuProcessUpdateSEVInfo(virDomainObj *vm) > { > qemuDomainObjPrivate *priv = vm->privateData; > virQEMUCaps *qemuCaps = priv->qemuCaps; > - virDomainSEVDef *sev = vm->def->sev; > + virDomainSEVDef *sev = vm->def->sec->sev; > virSEVCapability *sevCaps = NULL; > > /* if platform specific info like 'cbitpos' and 'reducedPhysBits' have > @@ -6636,7 +6636,8 @@ qemuProcessPrepareDomain(virQEMUDriver *driver, > for (i = 0; i < vm->def->nshmems; i++) > qemuDomainPrepareShmemChardev(vm->def->shmems[i]); > > - if (vm->def->sev) { > + if (vm->def->sec && > + vm->def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { > VIR_DEBUG("Updating SEV platform info"); > if (qemuProcessUpdateSEVInfo(vm) < 0) > return -1; > @@ -6674,10 +6675,10 @@ qemuProcessSEVCreateFile(virDomainObj *vm, > static int > qemuProcessPrepareSEVGuestInput(virDomainObj *vm) > { > - virDomainSEVDef *sev = vm->def->sev; > + virDomainSEVDef *sev = vm->def->sec->sev; > > if (!sev) > - return 0; > + return -1; This should not happen as we would abort if allocation of virDomainSEVDef failed. In addition if we go with the union where the data would not be a pointer there is no need for this check at all.

Pavel > VIR_DEBUG("Preparing SEV guest"); > > @@ -6695,6 +6696,28 @@ qemuProcessPrepareSEVGuestInput(virDomainObj *vm) > } > > > +static int > +qemuProcessPrepareLaunchSecurityGuestInput(virDomainObj *vm) > +{ > + virDomainSecDef *sec = vm->def->sec; > + > + if (!sec) > + return 0; > + > + switch ((virDomainLaunchSecurity) sec->sectype) { > + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: > + return qemuProcessPrepareSEVGuestInput(vm); > + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: > + break; > + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: > + virReportEnumRangeError(virDomainLaunchSecurity, sec->sectype); > + return -1; > + } > + > + return 0; > +} > + > + > static int > qemuProcessPrepareHostStorage(virQEMUDriver *driver, > virDomainObj *vm, > @@ -6874,7 +6897,7 @@ qemuProcessPrepareHost(virQEMUDriver *driver, > if (qemuExtDevicesPrepareHost(driver, vm) < 0) > return -1; > > - if (qemuProcessPrepareSEVGuestInput(vm) < 0) > + if (qemuProcessPrepareLaunchSecurityGuestInput(vm) < 0) > return -1; > > return 0; > diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c > index 382473d03b..957dbc906c 100644 > --- a/src/qemu/qemu_validate.c > +++ b/src/qemu/qemu_validate.c > @@ -1214,12 +1214,22 @@ qemuValidateDomainDef(const virDomainDef *def, > if (qemuValidateDomainDefPanic(def, qemuCaps) < 0) > return -1; > > - if (def->sev && > - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST)) { > - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("SEV launch security is not supported with " > - "this QEMU binary")); > - return -1; > + if (def->sec) { > + switch ((virDomainLaunchSecurity) def->sec->sectype) { > + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: > + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST)) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("SEV launch security is not supported with " > + "this QEMU binary")); > + return -1; > + } > + break; > + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: > + break; > + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: > + virReportEnumRangeError(virDomainLaunchSecurity, def->sec->sectype); > + return -1; > + } > } > > if (def->naudios > 1 && > diff --git a/src/security/security_dac.c b/src/security/security_dac.c > index 4909107fcc..b874dd4ab6 100644 > --- a/src/security/security_dac.c > +++ b/src/security/security_dac.c > @@ -1958,7 +1958,8 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr, > rc = -1; > } > > - if (def->sev) { > + if (def->sec && > + def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { > if (virSecurityDACRestoreSEVLabel(mgr, def) < 0) > rc = -1; > } > @@ -2165,7 +2166,8 @@ virSecurityDACSetAllLabel(virSecurityManager *mgr, > return -1; > } > > - if (def->sev) { > + if (def->sec && > + def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { > if (virSecurityDACSetSEVLabel(mgr, def) < 0) > return -1; > } > -- > 2.30.2 >

Add launch security type 's390-pv' as well as some tests. Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; --- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 8 +++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 26 ++++++++++++++ src/qemu/qemu_firmware.c | 1 + src/qemu/qemu_namespace.c | 1 + src/qemu/qemu_process.c | 1 + src/qemu/qemu_validate.c | 9 +++++ .../launch-security-s390-pv-ignore-policy.xml | 24 +++++++++++++ .../launch-security-s390-pv.xml | 18 ++++++++++ .../launch-security-s390-pv-ignore-policy.xml | 1 + tests/genericxml2xmltest.c | 2 ++ ...ty-s390-pv-ignore-policy.s390x-latest.args | 35 +++++++++++++++++++ .../launch-security-s390-pv-ignore-policy.xml | 33 +++++++++++++++++ .../launch-security-s390-pv.s390x-latest.args | 35 +++++++++++++++++++ .../launch-security-s390-pv.xml | 30 ++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ 17 files changed, 229 insertions(+) create mode 100644 tests/genericxml2xmlindata/launch-security-s390-pv-ignore-policy.xml create mode 100644 tests/genericxml2xmlindata/launch-security-s390-pv.xml create mode 120000 tests/genericxml2xmloutdata/launch-security-s390-pv-ignore-policy.xml create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.s390x-latest.args create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.xml create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv.s390x-latest.args create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv.xml diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 8c1b6c3a09..b81c51728d 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -485,6 +485,7 @@ <attribute name="type"> <choice> <value>sev</value> + <value>s390-pv</value> </choice> </attribute> <interleave> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2bd5210a16..a7fc8cd65f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1401,6 +1401,7 @@ VIR_ENUM_IMPL(virDomainLaunchSecurity, VIR_DOMAIN_LAUNCH_SECURITY_LAST, "", "sev", + "s390-pv", ); static virClass *virDomainObjClass; @@ -14799,6 +14800,8 @@ virDomainSecDefParseXML(xmlNodePtr lsecNode, if (!sec->sev) return NULL; break; + case VIR_DOMAIN_LAUNCH_SECURITY_PV: + break; case VIR_DOMAIN_LAUNCH_SECURITY_NONE: case VIR_DOMAIN_LAUNCH_SECURITY_LAST: default: @@ -26895,6 +26898,11 @@ virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) break; } + case VIR_DOMAIN_LAUNCH_SECURITY_PV: + virBufferAsprintf(buf, "<launchSecurity type='%s'/>\n", + virDomainLaunchSecurityTypeToString(sec->sectype)); + break; + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: case VIR_DOMAIN_LAUNCH_SECURITY_LAST: break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index fa7ab1895d..9d9acab50c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2645,6 +2645,7 @@ struct _virDomainKeyWrapDef { typedef enum { VIR_DOMAIN_LAUNCH_SECURITY_NONE, VIR_DOMAIN_LAUNCH_SECURITY_SEV, + VIR_DOMAIN_LAUNCH_SECURITY_PV, VIR_DOMAIN_LAUNCH_SECURITY_LAST, } virDomainLaunchSecurity; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4135a8444a..3ab803f7ce 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6975,6 +6975,9 @@ qemuBuildMachineCommandLine(virCommand *cmd, virBufferAddLit(&buf, ",memory-encryption=sev0"); } break; + case VIR_DOMAIN_LAUNCH_SECURITY_PV: + virBufferAddLit(&buf, ",confidential-guest-support=pv0"); + break;

case VIR_DOMAIN_LAUNCH_SECURITY_NONE: break; case VIR_DOMAIN_LAUNCH_SECURITY_LAST: @@ -9870,6 +9873,26 @@ qemuBuildSEVCommandLine(virDomainObj *vm, virCommand *cmd, } +static int +qemuBuildPVCommandLine(virDomainObj *vm, virCommand *cmd) +{ + g_autoptr(virJSONValue) props = NULL; + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + qemuDomainObjPrivate *priv = vm->privateData; + + if (qemuMonitorCreateObjectProps(&props, "s390-pv-guest", "pv0", + NULL) < 0) + return -1; + + if (qemuBuildObjectCommandlineFromJSON(&buf, props, priv->qemuCaps) < 0) + return -1; + + virCommandAddArg(cmd, "-object"); + virCommandAddArgBuffer(cmd, &buf); + return 0; +} + + static int qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd, virDomainSecDef *sec) @@ -9881,6 +9904,9 @@ qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd, case VIR_DOMAIN_LAUNCH_SECURITY_SEV: return qemuBuildSEVCommandLine(vm, cmd, sec->sev); break; + case VIR_DOMAIN_LAUNCH_SECURITY_PV: + return qemuBuildPVCommandLine(vm, cmd); + break; case VIR_DOMAIN_LAUNCH_SECURITY_NONE: break; case VIR_DOMAIN_LAUNCH_SECURITY_LAST: diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 6d1bab181e..3b408fa7b8 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -1070,6 +1070,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def, return false; } break; + case VIR_DOMAIN_LAUNCH_SECURITY_PV: case VIR_DOMAIN_LAUNCH_SECURITY_NONE: break; case VIR_DOMAIN_LAUNCH_SECURITY_LAST: diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c index 35c8eb83fd..156ee84292 100644 --- a/src/qemu/qemu_namespace.c +++ b/src/qemu/qemu_namespace.c @@ -607,6 +607,7 @@ qemuDomainSetupLaunchSecurity(virDomainObj *vm, VIR_DEBUG("Set up launch security for SEV"); break; + case VIR_DOMAIN_LAUNCH_SECURITY_PV: case VIR_DOMAIN_LAUNCH_SECURITY_NONE: break; case VIR_DOMAIN_LAUNCH_SECURITY_LAST: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d9073fb3a3..9f9904cc6c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6707,6 +6707,7 @@ qemuProcessPrepareLaunchSecurityGuestInput(virDomainObj *vm) switch ((virDomainLaunchSecurity) sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: return qemuProcessPrepareSEVGuestInput(vm); + case VIR_DOMAIN_LAUNCH_SECURITY_PV: case VIR_DOMAIN_LAUNCH_SECURITY_NONE: break; case VIR_DOMAIN_LAUNCH_SECURITY_LAST: diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 957dbc906c..c39dc5136d 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1224,6 +1224,15 @@ qemuValidateDomainDef(const virDomainDef *def, return -1; } break; + case VIR_DOMAIN_LAUNCH_SECURITY_PV: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT) || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_S390_PV_GUEST)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("S390 PV launch security is not supported with " + "this QEMU binary")); + return -1; + } + break; case VIR_DOMAIN_LAUNCH_SECURITY_NONE: break; case VIR_DOMAIN_LAUNCH_SECURITY_LAST: diff --git a/tests/genericxml2xmlindata/launch-security-s390-pv-ignore-policy.xml b/tests/genericxml2xmlindata/launch-security-s390-pv-ignore-policy.xml new file mode 100644 index 0000000000..0c398cced8 --- /dev/null +++ b/tests/genericxml2xmlindata/launch-security-s390-pv-ignore-policy.xml @@ -0,0 +1,24 @@ +<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='s390x' machine='s390-ccw-virtio'>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> + <launchSecurity type='s390-pv'> + <cbitpos>47</cbitpos> + <reducedPhysBits>1</reducedPhysBits> + <policy>0x0001</policy> + <dhCert>AQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAA</dhCert> + <session>IHAVENOIDEABUTJUSTPROVIDINGASTRING</session>

+ </launchSecurity> +</domain> diff --git a/tests/genericxml2xmlindata/launch-security-s390-pv.xml b/tests/genericxml2xmlindata/launch-security-s390-pv.xml new file mode 100644 index 0000000000..29c7fc152d --- /dev/null +++ b/tests/genericxml2xmlindata/launch-security-s390-pv.xml @@ -0,0 +1,18 @@ +<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='s390x' machine='s390-ccw-virtio'>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> + <launchSecurity type='s390-pv'/> +</domain> diff --git a/tests/genericxml2xmloutdata/launch-security-s390-pv-ignore-policy.xml b/tests/genericxml2xmloutdata/launch-security-s390-pv-ignore-policy.xml new file mode 120000 index 0000000000..075c72603d --- /dev/null +++ b/tests/genericxml2xmloutdata/launch-security-s390-pv-ignore-policy.xml @@ -0,0 +1 @@ +../genericxml2xmlindata/launch-security-s390-pv.xml \ No newline at end of file diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index ac89422a32..eb15f66c3c 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -233,6 +233,8 @@ mymain(void) DO_TEST("tseg"); DO_TEST("launch-security-sev"); + DO_TEST("launch-security-s390-pv"); + DO_TEST_DIFFERENT("launch-security-s390-pv-ignore-policy"); DO_TEST_DIFFERENT("cputune"); DO_TEST("device-backenddomain"); diff --git a/tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.s390x-latest.args b/tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.s390x-latest.args new file mode 100644 index 0000000000..c9d9b84dd3 --- /dev/null +++ b/tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.s390x-latest.args @@ -0,0 +1,35 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-s390x \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine s390-ccw-virtio,accel=kvm,usb=off,dump-guest-core=off,confidential-guest-support=pv0,memory-backend=s390.ram \ +-cpu gen15a-base,aen=on,cmmnt=on,vxpdeh=on,aefsi=on,diag318=on,csske=on,mepoch=on,msa9=on,msa8=on,msa7=on,msa6=on,msa5=on,msa4=on,msa3=on,msa2=on,msa1=on,sthyi=on,edat=on,ri=on,deflate=on,edat2=on,etoken=on,vx=on,ipter=on,mepochptff=on,ap=on,vxeh=on,vxpd=on,esop=on,msa9_pckmo=on,vxeh2=on,esort=on,apqi=on,apft=on,els=on,iep=on,apqci=on,cte=on,ais=on,bpb=on,gs=on,ppa15=on,zpci=on,sea_esop2=on,te=on,cmm=on \ +-m 214 \ +-object '{"qom-type":"memory-backend-ram","id":"s390.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \ +-device virtio-blk-ccw,devno=fe.0.0000,drive=libvirt-1-format,id=virtio-disk0,bootindex=1 \ +-audiodev id=audio1,driver=none \ +-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0001 \ +-object '{"qom-type":"s390-pv-guest","id":"pv0"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.xml b/tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.xml new file mode 100644 index 0000000000..052d96dedb --- /dev/null +++ b/tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.xml @@ -0,0 +1,33 @@ +<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='s390x' machine='s390-ccw-virtio'>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> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='virtio'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> + </disk> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/> + </memballoon> + <panic model='s390'/> + </devices> + <launchSecurity type='s390-pv'> + <dhCert>AQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAA</dhCert> + <session>IHAVENOIDEABUTJUSTPROVIDINGASTRING</session>

+ </launchSecurity> +</domain> diff --git a/tests/qemuxml2argvdata/launch-security-s390-pv.s390x-latest.args b/tests/qemuxml2argvdata/launch-security-s390-pv.s390x-latest.args new file mode 100644 index 0000000000..c9d9b84dd3 --- /dev/null +++ b/tests/qemuxml2argvdata/launch-security-s390-pv.s390x-latest.args @@ -0,0 +1,35 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-s390x \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine s390-ccw-virtio,accel=kvm,usb=off,dump-guest-core=off,confidential-guest-support=pv0,memory-backend=s390.ram \ +-cpu gen15a-base,aen=on,cmmnt=on,vxpdeh=on,aefsi=on,diag318=on,csske=on,mepoch=on,msa9=on,msa8=on,msa7=on,msa6=on,msa5=on,msa4=on,msa3=on,msa2=on,msa1=on,sthyi=on,edat=on,ri=on,deflate=on,edat2=on,etoken=on,vx=on,ipter=on,mepochptff=on,ap=on,vxeh=on,vxpd=on,esop=on,msa9_pckmo=on,vxeh2=on,esort=on,apqi=on,apft=on,els=on,iep=on,apqci=on,cte=on,ais=on,bpb=on,gs=on,ppa15=on,zpci=on,sea_esop2=on,te=on,cmm=on \ +-m 214 \ +-object '{"qom-type":"memory-backend-ram","id":"s390.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \ +-device virtio-blk-ccw,devno=fe.0.0000,drive=libvirt-1-format,id=virtio-disk0,bootindex=1 \ +-audiodev id=audio1,driver=none \ +-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0001 \ +-object '{"qom-type":"s390-pv-guest","id":"pv0"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/launch-security-s390-pv.xml b/tests/qemuxml2argvdata/launch-security-s390-pv.xml new file mode 100644 index 0000000000..c40c2b4bf2 --- /dev/null +++ b/tests/qemuxml2argvdata/launch-security-s390-pv.xml @@ -0,0 +1,30 @@ +<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='s390x' machine='s390-ccw-virtio'>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> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='virtio'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> + </disk> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/> + </memballoon> + <panic model='s390'/> + </devices> + <launchSecurity type='s390-pv'/> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ef6afae586..3268e4fe37 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -3461,6 +3461,9 @@ mymain(void) DO_TEST_CAPS_VER("launch-security-sev-missing-platform-info", "2.12.0"); DO_TEST_CAPS_VER_PARSE_ERROR("launch-security-sev-missing-policy", "2.12.0"); + DO_TEST_CAPS_ARCH_LATEST("launch-security-s390-pv", "s390x"); + DO_TEST_CAPS_ARCH_LATEST("launch-security-s390-pv-ignore-policy", "s390x"); + DO_TEST_CAPS_LATEST("vhost-user-fs-fd-memory"); DO_TEST_CAPS_LATEST("vhost-user-fs-hugepages"); DO_TEST_CAPS_LATEST_PARSE_ERROR("vhost-user-fs-readonly"); -- 2.30.2

On 6/25/21 12:00 PM, Pavel Hrdina wrote: > On Tue, Jun 22, 2021 at 03:10:48PM +0200, Boris Fiuczynski wrote: > > Add launch security type 's390-pv' as well as some tests. > > > > Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; > > Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; > > --- > > docs/schemas/domaincommon.rng | 1 + > > src/conf/domain_conf.c | 8 +++++ > > src/conf/domain_conf.h | 1 + > > src/qemu/qemu_command.c | 26 ++++++++++++++ > > src/qemu/qemu_firmware.c | 1 + > > src/qemu/qemu_namespace.c | 1 + > > src/qemu/qemu_process.c | 1 + > > src/qemu/qemu_validate.c | 9 +++++ > > .../launch-security-s390-pv-ignore-policy.xml | 24 +++++++++++++ > > .../launch-security-s390-pv.xml | 18 ++++++++++ > > .../launch-security-s390-pv-ignore-policy.xml | 1 + > > tests/genericxml2xmltest.c | 2 ++ > > ...ty-s390-pv-ignore-policy.s390x-latest.args | 35 +++++++++++++++++++ > > .../launch-security-s390-pv-ignore-policy.xml | 33 +++++++++++++++++ > > .../launch-security-s390-pv.s390x-latest.args | 35 +++++++++++++++++++ > > .../launch-security-s390-pv.xml | 30 ++++++++++++++++ > > tests/qemuxml2argvtest.c | 3 ++ > > 17 files changed, 229 insertions(+) > > create mode 100644 tests/genericxml2xmlindata/launch-security-s390-pv-ignore-policy.xml > > create mode 100644 tests/genericxml2xmlindata/launch-security-s390-pv.xml > > create mode 120000 tests/genericxml2xmloutdata/launch-security-s390-pv-ignore-policy.xml > > create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.s390x-latest.args > > create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.xml > > create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv.s390x-latest.args > > create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv.xml > > <snip> > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > > index 4135a8444a..3ab803f7ce 100644 > > --- a/src/qemu/qemu_command.c > > +++ b/src/qemu/qemu_command.c > > @@ -6975,6 +6975,9 @@ qemuBuildMachineCommandLine(virCommand *cmd, > > virBufferAddLit(&buf, ",memory-encryption=sev0"); > > } > > break; > > + case VIR_DOMAIN_LAUNCH_SECURITY_PV: > > + virBufferAddLit(&buf, ",confidential-guest-support=pv0"); > > + break; > > This could be possible shared for all launchSecurity types as well but > it can be done as followup. That would mean using for example lsec0 > instead of sev0, pv0, somethingelse0 and so on. It's just an id which > can be anything. > I will add an follow-up patch which changes the id to a common id 'lsec0'. > > case VIR_DOMAIN_LAUNCH_SECURITY_NONE: > > break; > > case VIR_DOMAIN_LAUNCH_SECURITY_LAST: <snip/> > > diff --git a/tests/genericxml2xmlindata/launch-security-s390-pv-ignore-policy.xml b/tests/genericxml2xmlindata/launch-security-s390-pv-ignore-policy.xml > > new file mode 100644 > > index 0000000000..0c398cced8 > > --- /dev/null > > +++ b/tests/genericxml2xmlindata/launch-security-s390-pv-ignore-policy.xml > > @@ -0,0 +1,24 @@ > > +<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='s390x' machine='s390-ccw-virtio'>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> > > + <launchSecurity type='s390-pv'> > > + <cbitpos>47</cbitpos> > > + <reducedPhysBits>1</reducedPhysBits> > > + <policy>0x0001</policy> > > + <dhCert>AQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAA</dhCert> > > + <session>IHAVENOIDEABUTJUSTPROVIDINGASTRING</session> > > I thing we should not ignore invalid XML bits and error out instead. This would result in s390-pv checking for the existence of any child elements and if there is a child fail. If other launchSecurity types come along with new or shared child elements checking for SEV and the new types would have to added/changed. Wouldn't that get messy quickly?

> > > + </launchSecurity> > > +</domain> > > diff --git a/tests/genericxml2xmlindata/launch-security-s390-pv.xml b/tests/genericxml2xmlindata/launch-security-s390-pv.xml > > new file mode 100644 > > index 0000000000..29c7fc152d > > --- /dev/null > > +++ b/tests/genericxml2xmlindata/launch-security-s390-pv.xml > > @@ -0,0 +1,18 @@ > > +<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='s390x' machine='s390-ccw-virtio'>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> > > + <launchSecurity type='s390-pv'/> > > +</domain> > > diff --git a/tests/genericxml2xmloutdata/launch-security-s390-pv-ignore-policy.xml b/tests/genericxml2xmloutdata/launch-security-s390-pv-ignore-policy.xml > > new file mode 120000 > > index 0000000000..075c72603d > > --- /dev/null > > +++ b/tests/genericxml2xmloutdata/launch-security-s390-pv-ignore-policy.xml > > @@ -0,0 +1 @@ > > +../genericxml2xmlindata/launch-security-s390-pv.xml > > \ No newline at end of file > > diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c > > index ac89422a32..eb15f66c3c 100644 > > --- a/tests/genericxml2xmltest.c > > +++ b/tests/genericxml2xmltest.c > > @@ -233,6 +233,8 @@ mymain(void) > > DO_TEST("tseg"); > > DO_TEST("launch-security-sev"); > > + DO_TEST("launch-security-s390-pv"); > > + DO_TEST_DIFFERENT("launch-security-s390-pv-ignore-policy"); > > DO_TEST_DIFFERENT("cputune"); > > DO_TEST("device-backenddomain"); > > diff --git a/tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.s390x-latest.args b/tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.s390x-latest.args > > new file mode 100644 > > index 0000000000..c9d9b84dd3 > > --- /dev/null > > +++ b/tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.s390x-latest.args > > @@ -0,0 +1,35 @@ > > +LC_ALL=C \ > > +PATH=/bin \ > > +HOME=/tmp/lib/domain--1-QEMUGuest1 \ > > +USER=test \ > > +LOGNAME=test \ > > +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ > > +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ > > +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ > > +/usr/bin/qemu-system-s390x \ > > +-name guest=QEMUGuest1,debug-threads=on \ > > +-S \ > > +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ > > +-machine s390-ccw-virtio,accel=kvm,usb=off,dump-guest-core=off,confidential-guest-support=pv0,memory-backend=s390.ram \ > > +-cpu gen15a-base,aen=on,cmmnt=on,vxpdeh=on,aefsi=on,diag318=on,csske=on,mepoch=on,msa9=on,msa8=on,msa7=on,msa6=on,msa5=on,msa4=on,msa3=on,msa2=on,msa1=on,sthyi=on,edat=on,ri=on,deflate=on,edat2=on,etoken=on,vx=on,ipter=on,mepochptff=on,ap=on,vxeh=on,vxpd=on,esop=on,msa9_pckmo=on,vxeh2=on,esort=on,apqi=on,apft=on,els=on,iep=on,apqci=on,cte=on,ais=on,bpb=on,gs=on,ppa15=on,zpci=on,sea_esop2=on,te=on,cmm=on \ > > +-m 214 \ > > +-object '{"qom-type":"memory-backend-ram","id":"s390.ram","size":224395264}' \ > > +-overcommit mem-lock=off \ > > +-smp 1,sockets=1,cores=1,threads=1 \ > > +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ > > +-display none \ > > +-no-user-config \ > > +-nodefaults \ > > +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ > > +-mon chardev=charmonitor,id=monitor,mode=control \ > > +-rtc base=utc \ > > +-no-shutdown \ > > +-boot strict=on \ > > +-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ > > +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \ > > +-device virtio-blk-ccw,devno=fe.0.0000,drive=libvirt-1-format,id=virtio-disk0,bootindex=1 \ > > +-audiodev id=audio1,driver=none \ > > +-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0001 \ > > +-object '{"qom-type":"s390-pv-guest","id":"pv0"}' \ > > +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ > > +-msg timestamp=on > > diff --git a/tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.xml b/tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.xml > > new file mode 100644 > > index 0000000000..052d96dedb > > --- /dev/null > > +++ b/tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.xml > > @@ -0,0 +1,33 @@ > > +<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='s390x' machine='s390-ccw-virtio'>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> > > + <emulator>/usr/bin/qemu-system-s390x</emulator> > > + <disk type='block' device='disk'> > > + <driver name='qemu' type='raw'/> > > + <source dev='/dev/HostVG/QEMUGuest1'/> > > + <target dev='hda' bus='virtio'/> > > + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> > > + </disk> > > + <controller type='pci' index='0' model='pci-root'/> > > + <memballoon model='virtio'> > > + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/> > > + </memballoon> > > + <panic model='s390'/> > > + </devices> > > + <launchSecurity type='s390-pv'> > > + <dhCert>AQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAA</dhCert> > > + <session>IHAVENOIDEABUTJUSTPROVIDINGASTRING</session> > > This doesn't look correct, we should not format dhCert or session with > s390-pv because based on the patches they are not used at all. Isn't this the same issue as before about unsued elements being ignored?

Add documentation for launch security type s390-pv. Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; --- docs/formatdomain.rst | 7 ++++ docs/kbase/s390_protected_virt.rst | 55 +++++++++++++++++++++++++----- 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index c6dede053f..a1b028c4ad 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -8078,6 +8078,13 @@ Note: DEA/TDEA is synonymous with DES/TDES. Launch Security --------------- +Specifying ``<launchSecurity type='s390-pv'\>`` in a s390 domain prepares +the guest to run in protected virtualization secure mode, also known as +IBM Secure Execution. For more required host and guest preparation steps, see +`Protected Virtualization on s390 <kbase/s390_protected_virt.html>`__ +:since:`Since 7.5.0` + + The contents of the ``<launchSecurity type='sev'>`` element is used to provide the guest owners input used for creating an encrypted VM using the AMD SEV feature (Secure Encrypted Virtualization). SEV is an extension to the AMD-V diff --git a/docs/kbase/s390_protected_virt.rst b/docs/kbase/s390_protected_virt.rst index 1718a556d4..66203568d9 100644 --- a/docs/kbase/s390_protected_virt.rst +++ b/docs/kbase/s390_protected_virt.rst @@ -127,10 +127,13 @@ Protected virtualization guests support I/O using virtio devices. As the virtio data structures of secure guests are not accessible by the host, it is necessary to use shared memory ('bounce buffers'). -To enable virtio devices to use shared buffers, it is necessary -to configure them with platform_iommu enabled. This can done by adding -``iommu='on'`` to the driver element of a virtio device definition in the -guest's XML, e.g. +Since libvirt 7.5.0 the +`<launchSecurity> <https://libvirt.org/formatdomain.html#launchSecurity>`__ +element with type ``s390-pv`` should be used on protected virtualization guests. +Without ``launchSecurity`` you must enable all virtio devices to use shared +buffers by configuring them with platform_iommu enabled. +This can done by adding ``iommu='on'`` to the driver element of a virtio +device definition in the guest's XML, e.g. :: @@ -140,8 +143,10 @@ guest's XML, e.g. <driver name='vhost' iommu='on'/> </interface> -It is mandatory to define all virtio bus devices in this way to -prevent the host from attempting to access protected memory. +Unless you are using ``launchSecurity`` you must define all virtio bus +devices in this way to prevent the host from attempting to access +protected memory. + Ballooning will not work and is fenced by QEMU. It should be disabled by specifying @@ -158,8 +163,42 @@ allocated 2K entries. A commonly used value for swiotlb is 262144. Example guest definition ======================== -Minimal domain XML for a protected virtualization guest, essentially -it's mostly about the ``iommu`` property +Minimal domain XML for a protected virtualization guest with +the ``launchSecurity`` element of type ``s390-pv`` + +:: + + <domain type='kvm'> + <name>protected</name> + <memory unit='KiB'>2048000</memory> + <currentMemory unit='KiB'>2048000</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='s390x'>hvm</type> + </os> + <cpu mode='host-model'/> + <devices> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='none' io='native'> + <source file='/var/lib/libvirt/images/protected.qcow2'/> + <target dev='vda' bus='virtio'/> + </disk> + <interface type='network'> + <source network='default'/> + <model type='virtio'/> + </interface> + <console type='pty'/> + <memballoon model='none'/> + </devices> + <launchSecurity type='s390-pv'/> + </domain> + + +Example guest definition without launchSecurity +=============================================== + +Minimal domain XML for a protected virtualization guest using the +``iommu='on'`` setting for each virtio device.

:: -- 2.30.2