While working on qemuMonitorJSONGetSEVMeasurement() and qemuMonitorJSONGetSEVInfo() I've noticed that if these functions fail, they do so without appropriate error set. Fill in error reporting. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_monitor_json.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index c5e758e7f8..8f8f3c95f0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -7961,8 +7961,11 @@ qemuMonitorJSONGetSEVMeasurement(qemuMonitor *mon) if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT))) return NULL; - if (!(tmp = virJSONValueObjectGetString(data, "data"))) + if (!(tmp = virJSONValueObjectGetString(data, "data"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-sev-launch-measure reply was missing 'data'")); return NULL; + } return g_strdup(tmp); } @@ -8005,8 +8008,11 @@ qemuMonitorJSONGetSEVInfo(qemuMonitor *mon, if (virJSONValueObjectGetNumberUint(data, "api-major", apiMajor) < 0 || virJSONValueObjectGetNumberUint(data, "api-minor", apiMinor) < 0 || virJSONValueObjectGetNumberUint(data, "build-id", buildID) < 0 || - virJSONValueObjectGetNumberUint(data, "policy", policy) < 0) + virJSONValueObjectGetNumberUint(data, "policy", policy) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-sev reply was missing some data")); return -1; + } return 0; } -- 2.44.2

Some parts of SEV are to be shared with SEV SNP. In order to reuse XML parsing / formatting code cleanly, let's move those common bits into a new struct (virDomainSEVCommonDef) and adjust rest of the code. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 55 +++++++++++++++++++++---------- src/conf/domain_conf.h | 13 +++++--- src/conf/schemas/domaincommon.rng | 24 ++++++++------ src/conf/virconftypes.h | 2 ++ src/qemu/qemu_command.c | 8 ++--- src/qemu/qemu_process.c | 12 +++---- src/qemu/qemu_validate.c | 2 +- 7 files changed, 74 insertions(+), 42 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2f1e99865b..9179cc18bb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13621,8 +13621,8 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node, static int -virDomainSEVDefParseXML(virDomainSEVDef *def, - xmlXPathContextPtr ctxt) +virDomainSEVCommonDefParseXML(virDomainSEVCommonDef *def, + xmlXPathContextPtr ctxt) { int rc; @@ -13630,12 +13630,6 @@ virDomainSEVDefParseXML(virDomainSEVDef *def, &def->kernel_hashes) < 0) return -1; - if (virXPathUIntBase("string(./policy)", ctxt, 16, &def->policy) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("failed to get launch security policy")); - return -1; - } - /* the following attributes are platform dependent and if missing, we can * autofill them from domain capabilities later */ @@ -13658,6 +13652,23 @@ virDomainSEVDefParseXML(virDomainSEVDef *def, return -1; } + return 0; +} + + +static int +virDomainSEVDefParseXML(virDomainSEVDef *def, + xmlXPathContextPtr ctxt) +{ + if (virDomainSEVCommonDefParseXML(&def->common, ctxt) < 0) + return -1; + + if (virXPathUIntBase("string(./policy)", ctxt, 16, &def->policy) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("failed to get launch security policy")); + return -1; + } + def->dh_cert = virXPathString("string(./dhCert)", ctxt); def->session = virXPathString("string(./session)", ctxt); @@ -26641,6 +26652,24 @@ virDomainKeyWrapDefFormat(virBuffer *buf, virDomainKeyWrapDef *keywrap) } +static void +virDomainSEVCommonDefFormat(virBuffer *attrBuf, + virBuffer *childBuf, + virDomainSEVCommonDef *def) +{ + if (def->kernel_hashes != VIR_TRISTATE_BOOL_ABSENT) + virBufferAsprintf(attrBuf, " kernelHashes='%s'", + virTristateBoolTypeToString(def->kernel_hashes)); + + if (def->haveCbitpos) + virBufferAsprintf(childBuf, "<cbitpos>%d</cbitpos>\n", def->cbitpos); + + if (def->haveReducedPhysBits) + virBufferAsprintf(childBuf, "<reducedPhysBits>%d</reducedPhysBits>\n", + def->reduced_phys_bits); +} + + static void virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) { @@ -26657,16 +26686,8 @@ virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) case VIR_DOMAIN_LAUNCH_SECURITY_SEV: { virDomainSEVDef *sev = &sec->data.sev; - if (sev->kernel_hashes != VIR_TRISTATE_BOOL_ABSENT) - virBufferAsprintf(&attrBuf, " kernelHashes='%s'", - virTristateBoolTypeToString(sev->kernel_hashes)); + virDomainSEVCommonDefFormat(&attrBuf, &childBuf, &sev->common); - if (sev->haveCbitpos) - virBufferAsprintf(&childBuf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos); - - if (sev->haveReducedPhysBits) - virBufferAsprintf(&childBuf, "<reducedPhysBits>%d</reducedPhysBits>\n", - sev->reduced_phys_bits); virBufferAsprintf(&childBuf, "<policy>0x%04x</policy>\n", sev->policy); virBufferEscapeString(&childBuf, "<dhCert>%s</dhCert>\n", sev->dh_cert); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cdab6ef2da..c6c3c2e2a5 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2866,10 +2866,7 @@ typedef enum { } virDomainLaunchSecurity; -struct _virDomainSEVDef { - char *dh_cert; - char *session; - unsigned int policy; +struct _virDomainSEVCommonDef { bool haveCbitpos; unsigned int cbitpos; bool haveReducedPhysBits; @@ -2877,6 +2874,14 @@ struct _virDomainSEVDef { virTristateBool kernel_hashes; }; + +struct _virDomainSEVDef { + virDomainSEVCommonDef common; + char *dh_cert; + char *session; + unsigned int policy; +}; + struct _virDomainSecDef { virDomainLaunchSecurity sectype; union { diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index a46a824f88..9a7649df1c 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -524,6 +524,19 @@ </element> </define> + <define name="launchSecuritySEVCommon"> + <optional> + <element name="cbitpos"> + <data type="unsignedInt"/> + </element> + </optional> + <optional> + <element name="reducedPhysBits"> + <data type="unsignedInt"/> + </element> + </optional> + </define> + <define name="launchSecuritySEV"> <attribute name="type"> <value>sev</value> @@ -534,16 +547,7 @@ </attribute> </optional> <interleave> - <optional> - <element name="cbitpos"> - <data type="unsignedInt"/> - </element> - </optional> - <optional> - <element name="reducedPhysBits"> - <data type="unsignedInt"/> - </element> - </optional> + <ref name="launchSecuritySEVCommon"/> <element name="policy"> <ref name="hexuint"/> </element> diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index 0779bc224b..34bb1e262f 100644 --- a/src/conf/virconftypes.h +++ b/src/conf/virconftypes.h @@ -210,6 +210,8 @@ typedef struct _virDomainResctrlMonDef virDomainResctrlMonDef; typedef struct _virDomainResourceDef virDomainResourceDef; +typedef struct _virDomainSEVCommonDef virDomainSEVCommonDef; + typedef struct _virDomainSEVDef virDomainSEVDef; typedef struct _virDomainSecDef virDomainSecDef; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2d0eddc79e..a32cb8f8e9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9728,7 +9728,7 @@ qemuBuildSEVCommandLine(virDomainObj *vm, virCommand *cmd, g_autofree char *sessionpath = NULL; VIR_DEBUG("policy=0x%x cbitpos=%d reduced_phys_bits=%d", - sev->policy, sev->cbitpos, sev->reduced_phys_bits); + sev->policy, sev->common.cbitpos, sev->common.reduced_phys_bits); if (sev->dh_cert) dhpath = g_strdup_printf("%s/dh_cert.base64", priv->libDir); @@ -9737,12 +9737,12 @@ qemuBuildSEVCommandLine(virDomainObj *vm, virCommand *cmd, sessionpath = g_strdup_printf("%s/session.base64", priv->libDir); if (qemuMonitorCreateObjectProps(&props, "sev-guest", "lsec0", - "u:cbitpos", sev->cbitpos, - "u:reduced-phys-bits", sev->reduced_phys_bits, + "u:cbitpos", sev->common.cbitpos, + "u:reduced-phys-bits", sev->common.reduced_phys_bits, "u:policy", sev->policy, "S:dh-cert-file", dhpath, "S:session-file", sessionpath, - "T:kernel-hashes", sev->kernel_hashes, + "T:kernel-hashes", sev->common.kernel_hashes, NULL) < 0) return -1; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ae6594e10e..9886a11245 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6569,14 +6569,14 @@ qemuProcessUpdateSEVInfo(virDomainObj *vm) * mandatory on QEMU cmdline */ sevCaps = virQEMUCapsGetSEVCapabilities(qemuCaps); - if (!sev->haveCbitpos) { - sev->cbitpos = sevCaps->cbitpos; - sev->haveCbitpos = true; + if (!sev->common.haveCbitpos) { + sev->common.cbitpos = sevCaps->cbitpos; + sev->common.haveCbitpos = true; } - if (!sev->haveReducedPhysBits) { - sev->reduced_phys_bits = sevCaps->reduced_phys_bits; - sev->haveReducedPhysBits = true; + if (!sev->common.haveReducedPhysBits) { + sev->common.reduced_phys_bits = sevCaps->reduced_phys_bits; + sev->common.haveReducedPhysBits = true; } return 0; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index b82d937a0d..a00ec8e940 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1318,7 +1318,7 @@ qemuValidateDomainDef(const virDomainDef *def, return -1; } - if (def->sec->data.sev.kernel_hashes != VIR_TRISTATE_BOOL_ABSENT && + if (def->sec->data.sev.common.kernel_hashes != VIR_TRISTATE_BOOL_ABSENT && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST_KERNEL_HASHES)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("SEV measured direct kernel boot is not supported with this QEMU binary")); -- 2.44.2

To avoid convolution of switch() inside of virDomainSecDefFormat() even more (as new sectypes are added), move formatting into a separate function. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9179cc18bb..264f182d05 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26670,6 +26670,19 @@ virDomainSEVCommonDefFormat(virBuffer *attrBuf, } +static void +virDomainSEVDefFormat(virBuffer *attrBuf, + virBuffer *childBuf, + virDomainSEVDef *def) +{ + virDomainSEVCommonDefFormat(attrBuf, childBuf, &def->common); + + virBufferAsprintf(childBuf, "<policy>0x%04x</policy>\n", def->policy); + virBufferEscapeString(childBuf, "<dhCert>%s</dhCert>\n", def->dh_cert); + virBufferEscapeString(childBuf, "<session>%s</session>\n", def->session); +} + + static void virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) { @@ -26683,18 +26696,9 @@ virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) virDomainLaunchSecurityTypeToString(sec->sectype)); switch ((virDomainLaunchSecurity) sec->sectype) { - case VIR_DOMAIN_LAUNCH_SECURITY_SEV: { - virDomainSEVDef *sev = &sec->data.sev; - - virDomainSEVCommonDefFormat(&attrBuf, &childBuf, &sev->common); - - virBufferAsprintf(&childBuf, "<policy>0x%04x</policy>\n", sev->policy); - virBufferEscapeString(&childBuf, "<dhCert>%s</dhCert>\n", sev->dh_cert); - - virBufferEscapeString(&childBuf, "<session>%s</session>\n", sev->session); - + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + virDomainSEVDefFormat(&attrBuf, &childBuf, &sec->data.sev); break; - } case VIR_DOMAIN_LAUNCH_SECURITY_PV: break; -- 2.44.2

The sectype member of _virDomainSecDef struct is already declared as of virDomainLaunchSecurity type. There's no need to typecast it to the very same type when passing it to switch(). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 6 +++--- src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_firmware.c | 2 +- src/qemu/qemu_namespace.c | 2 +- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_validate.c | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 264f182d05..102a011be8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3830,7 +3830,7 @@ virDomainSecDefFree(virDomainSecDef *def) if (!def) return; - switch ((virDomainLaunchSecurity) def->sectype) { + switch (def->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: g_free(def->data.sev.dh_cert); g_free(def->data.sev.session); @@ -13690,7 +13690,7 @@ virDomainSecDefParseXML(xmlNodePtr lsecNode, &sec->sectype) < 0) return NULL; - switch ((virDomainLaunchSecurity) sec->sectype) { + switch (sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: if (virDomainSEVDefParseXML(&sec->data.sev, ctxt) < 0) return NULL; @@ -26695,7 +26695,7 @@ virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) virBufferAsprintf(&attrBuf, " type='%s'", virDomainLaunchSecurityTypeToString(sec->sectype)); - switch ((virDomainLaunchSecurity) sec->sectype) { + switch (sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: virDomainSEVDefFormat(&attrBuf, &childBuf, &sec->data.sev); break; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a32cb8f8e9..1879fa608c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7054,7 +7054,7 @@ qemuBuildMachineCommandLine(virCommand *cmd, qemuAppendLoadparmMachineParm(&buf, def); if (def->sec) { - switch ((virDomainLaunchSecurity) def->sec->sectype) { + switch (def->sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT)) { virBufferAddLit(&buf, ",confidential-guest-support=lsec0"); @@ -9777,7 +9777,7 @@ qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd, if (!sec) return 0; - switch ((virDomainLaunchSecurity) sec->sectype) { + switch (sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: return qemuBuildSEVCommandLine(vm, cmd, &sec->data.sev); break; diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 78844e3066..8946dc1aa5 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -1323,7 +1323,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def, } if (def->sec) { - switch ((virDomainLaunchSecurity) def->sec->sectype) { + switch (def->sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: if (!supportsSEV) { VIR_DEBUG("Domain requires SEV, firmware '%s' doesn't support it", diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c index 915d44310f..fb7c9329dd 100644 --- a/src/qemu/qemu_namespace.c +++ b/src/qemu/qemu_namespace.c @@ -651,7 +651,7 @@ qemuDomainSetupLaunchSecurity(virDomainObj *vm, if (!sec) return 0; - switch ((virDomainLaunchSecurity) sec->sectype) { + switch (sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: VIR_DEBUG("Setting up launch security for SEV"); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 9886a11245..f730cd1828 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6806,7 +6806,7 @@ qemuProcessPrepareLaunchSecurityGuestInput(virDomainObj *vm) if (!sec) return 0; - switch ((virDomainLaunchSecurity) sec->sectype) { + switch (sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: return qemuProcessPrepareSEVGuestInput(vm); case VIR_DOMAIN_LAUNCH_SECURITY_PV: diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index a00ec8e940..05ee477456 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1310,7 +1310,7 @@ qemuValidateDomainDef(const virDomainDef *def, return -1; if (def->sec) { - switch ((virDomainLaunchSecurity) def->sec->sectype) { + switch (def->sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", -- 2.44.2

In a few instances there is a plain if() check for _virDomainSecDef::sectype. While this works perfectly for now, soon there'll be another type and we can utilize compiler to identify all the places that need adaptation. Switch those if() statements to switch(). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_cgroup.c | 18 ++++++++++++++---- src/qemu/qemu_driver.c | 16 ++++++++++++++-- src/qemu/qemu_process.c | 17 +++++++++++++---- src/security/security_dac.c | 32 +++++++++++++++++++++++++------- 4 files changed, 66 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 5a5ba763a0..9e559c7c4f 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -845,10 +845,20 @@ qemuSetupDevicesCgroup(virDomainObj *vm) return -1; } - if (vm->def->sec && - vm->def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV && - qemuSetupSEVCgroup(vm) < 0) - return -1; + if (vm->def->sec) { + switch (vm->def->sec->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + if (qemuSetupSEVCgroup(vm) < 0) + return -1; + break; + case VIR_DOMAIN_LAUNCH_SECURITY_PV: + break; + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + virReportEnumRangeError(virDomainLaunchSecurity, vm->def->sec->sectype); + return -1; + } + } return 0; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3fd401fd3b..eb1612b9d7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19111,10 +19111,22 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain, if (virDomainGetLaunchSecurityInfoEnsureACL(domain->conn, vm->def) < 0) goto cleanup; - if (vm->def->sec && - vm->def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { + if (!vm->def->sec) { + ret = 0; + goto cleanup; + } + + switch (vm->def->sec->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: if (qemuDomainGetSEVInfo(vm, params, nparams, flags) < 0) goto cleanup; + break; + case VIR_DOMAIN_LAUNCH_SECURITY_PV: + break; + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + virReportEnumRangeError(virDomainLaunchSecurity, vm->def->sec->sectype); + return -1; } ret = 0; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f730cd1828..d14975180a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6741,11 +6741,20 @@ qemuProcessPrepareDomain(virQEMUDriver *driver, for (i = 0; i < vm->def->nshmems; i++) qemuDomainPrepareShmemChardev(vm->def->shmems[i]); - if (vm->def->sec && - vm->def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { - VIR_DEBUG("Updating SEV platform info"); - if (qemuProcessUpdateSEVInfo(vm) < 0) + if (vm->def->sec) { + switch (vm->def->sec->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + VIR_DEBUG("Updating SEV platform info"); + if (qemuProcessUpdateSEVInfo(vm) < 0) + return -1; + break; + case VIR_DOMAIN_LAUNCH_SECURITY_PV: + break; + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + virReportEnumRangeError(virDomainLaunchSecurity, vm->def->sec->sectype); return -1; + } } return 0; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 669b90125c..d0864313c1 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1951,10 +1951,19 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr, rc = -1; } - if (def->sec && - def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { - if (virSecurityDACRestoreSEVLabel(mgr, def) < 0) - rc = -1; + if (def->sec) { + switch (def->sec->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + if (virSecurityDACRestoreSEVLabel(mgr, def) < 0) + rc = -1; + break; + case VIR_DOMAIN_LAUNCH_SECURITY_PV: + break; + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + virReportEnumRangeError(virDomainLaunchSecurity, def->sec->sectype); + return -1; + } } for (i = 0; i < def->nsysinfo; i++) { @@ -2175,10 +2184,19 @@ virSecurityDACSetAllLabel(virSecurityManager *mgr, return -1; } - if (def->sec && - def->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { - if (virSecurityDACSetSEVLabel(mgr, def) < 0) + if (def->sec) { + switch (def->sec->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + if (virSecurityDACSetSEVLabel(mgr, def) < 0) + return -1; + break; + case VIR_DOMAIN_LAUNCH_SECURITY_PV: + break; + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + virReportEnumRangeError(virDomainLaunchSecurity, def->sec->sectype); return -1; + } } if (virSecurityDACGetImageIds(secdef, priv, &user, &group)) -- 2.44.2

In QEMU commit v9.0.0-1155-g59d3740cb4 the return type of 'query-sev' monitor command changed to accommodate SEV-SNP. Even though we currently support launching plain SNP guests, this will soon change. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 32 ++++++++++-------- src/qemu/qemu_monitor.c | 7 ++-- src/qemu/qemu_monitor.h | 41 +++++++++++++++++++---- src/qemu/qemu_monitor_json.c | 63 ++++++++++++++++++++++++++++------ src/qemu/qemu_monitor_json.h | 8 ++--- tests/qemumonitorjsontest.c | 65 ++++++++++++++++++++++++++++++++---- 6 files changed, 167 insertions(+), 49 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index eb1612b9d7..068c721d9f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19036,10 +19036,7 @@ qemuDomainGetSEVInfo(virDomainObj *vm, int ret = -1; int rv; g_autofree char *tmp = NULL; - unsigned int apiMajor = 0; - unsigned int apiMinor = 0; - unsigned int buildID = 0; - unsigned int policy = 0; + qemuMonitorSEVInfo info = { }; int maxpar = 0; virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); @@ -19054,14 +19051,12 @@ qemuDomainGetSEVInfo(virDomainObj *vm, qemuDomainObjEnterMonitor(vm); tmp = qemuMonitorGetSEVMeasurement(QEMU_DOMAIN_PRIVATE(vm)->mon); - if (!tmp) { qemuDomainObjExitMonitor(vm); goto endjob; } - rv = qemuMonitorGetSEVInfo(QEMU_DOMAIN_PRIVATE(vm)->mon, - &apiMajor, &apiMinor, &buildID, &policy); + rv = qemuMonitorGetSEVInfo(QEMU_DOMAIN_PRIVATE(vm)->mon, &info); qemuDomainObjExitMonitor(vm); if (rv < 0) @@ -19073,21 +19068,30 @@ qemuDomainGetSEVInfo(virDomainObj *vm, goto endjob; if (virTypedParamsAddUInt(params, nparams, &maxpar, VIR_DOMAIN_LAUNCH_SECURITY_SEV_API_MAJOR, - apiMajor) < 0) + info.apiMajor) < 0) goto endjob; if (virTypedParamsAddUInt(params, nparams, &maxpar, VIR_DOMAIN_LAUNCH_SECURITY_SEV_API_MINOR, - apiMinor) < 0) + info.apiMinor) < 0) goto endjob; if (virTypedParamsAddUInt(params, nparams, &maxpar, VIR_DOMAIN_LAUNCH_SECURITY_SEV_BUILD_ID, - buildID) < 0) - goto endjob; - if (virTypedParamsAddUInt(params, nparams, &maxpar, - VIR_DOMAIN_LAUNCH_SECURITY_SEV_POLICY, - policy) < 0) + info.buildID) < 0) goto endjob; + switch (info.type) { + case QEMU_MONITOR_SEV_GUEST_TYPE_SEV: + if (virTypedParamsAddUInt(params, nparams, &maxpar, + VIR_DOMAIN_LAUNCH_SECURITY_SEV_POLICY, + info.data.sev.policy) < 0) + goto endjob; + break; + + case QEMU_MONITOR_SEV_GUEST_TYPE_SEV_SNP: + case QEMU_MONITOR_SEV_GUEST_TYPE_LAST: + break; + } + ret = 0; endjob: diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 34e2ccab97..b1c0c6a064 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4043,14 +4043,11 @@ qemuMonitorGetSEVMeasurement(qemuMonitor *mon) int qemuMonitorGetSEVInfo(qemuMonitor *mon, - unsigned int *apiMajor, - unsigned int *apiMinor, - unsigned int *buildID, - unsigned int *policy) + qemuMonitorSEVInfo *info) { QEMU_CHECK_MONITOR(mon); - return qemuMonitorJSONGetSEVInfo(mon, apiMajor, apiMinor, buildID, policy); + return qemuMonitorJSONGetSEVInfo(mon, info); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index b78f539c85..8dde3f9fff 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1334,14 +1334,43 @@ int qemuMonitorBlockdevMediumInsert(qemuMonitor *mon, char * qemuMonitorGetSEVMeasurement(qemuMonitor *mon); +typedef struct _qemuMonitorSEVGuestInfo qemuMonitorSEVGuestInfo; +struct _qemuMonitorSEVGuestInfo { + unsigned int policy; + unsigned int handle; +}; + +typedef struct _qemuMonitorSEVSNPGuestInfo qemuMonitorSEVSNPGuestInfo; +struct _qemuMonitorSEVSNPGuestInfo { + unsigned long long snp_policy; +}; + + +typedef enum { + QEMU_MONITOR_SEV_GUEST_TYPE_SEV, + QEMU_MONITOR_SEV_GUEST_TYPE_SEV_SNP, + + QEMU_MONITOR_SEV_GUEST_TYPE_LAST +} qemuMonitorSEVGuestType; + +VIR_ENUM_DECL(qemuMonitorSEVGuest); + +typedef struct _qemuMonitorSEVInfo qemuMonitorSEVInfo; +struct _qemuMonitorSEVInfo { + unsigned int apiMajor; + unsigned int apiMinor; + unsigned int buildID; + qemuMonitorSEVGuestType type; + union { + qemuMonitorSEVGuestInfo sev; + qemuMonitorSEVSNPGuestInfo sev_snp; + } data; +}; + int qemuMonitorGetSEVInfo(qemuMonitor *mon, - unsigned int *apiMajor, - unsigned int *apiMinor, - unsigned int *buildID, - unsigned int *policy) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) - ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5); + qemuMonitorSEVInfo *info) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuMonitorSetLaunchSecurityState(qemuMonitor *mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 8f8f3c95f0..5a6af90ac8 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -7971,6 +7971,10 @@ qemuMonitorJSONGetSEVMeasurement(qemuMonitor *mon) } +VIR_ENUM_IMPL(qemuMonitorSEVGuest, + QEMU_MONITOR_SEV_GUEST_TYPE_LAST, + "sev", "sev-snp"); + /** * Retrieve info about the SEV setup, returning those fields that * are required to do a launch attestation, as per @@ -7984,13 +7988,15 @@ qemuMonitorJSONGetSEVMeasurement(qemuMonitor *mon) * { "return": { "enabled": true, "api-major" : 0, "api-minor" : 0, * "build-id" : 0, "policy" : 0, "state" : "running", * "handle" : 1 } } + * + * Or newer (as of QEMU v9.0.0-1155-g59d3740cb4): + * + * {"return": {"enabled": true, "api-minor": 55, "handle": 1, "state": "launch-secret", + * "api-major": 1, "sev-type": "sev", "build-id": 21, "policy": 1}} */ int qemuMonitorJSONGetSEVInfo(qemuMonitor *mon, - unsigned int *apiMajor, - unsigned int *apiMinor, - unsigned int *buildID, - unsigned int *policy) + qemuMonitorSEVInfo *info) { g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; @@ -8005,16 +8011,51 @@ qemuMonitorJSONGetSEVInfo(qemuMonitor *mon, if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT))) return -1; - if (virJSONValueObjectGetNumberUint(data, "api-major", apiMajor) < 0 || - virJSONValueObjectGetNumberUint(data, "api-minor", apiMinor) < 0 || - virJSONValueObjectGetNumberUint(data, "build-id", buildID) < 0 || - virJSONValueObjectGetNumberUint(data, "policy", policy) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-sev reply was missing some data")); - return -1; + if (virJSONValueObjectGetNumberUint(data, "api-major", &info->apiMajor) < 0 || + virJSONValueObjectGetNumberUint(data, "api-minor", &info->apiMinor) < 0 || + virJSONValueObjectGetNumberUint(data, "build-id", &info->buildID) < 0) { + goto error_report; + } + + if (virJSONValueObjectHasKey(data, "sev-type")) { + const char *sevTypeStr = virJSONValueObjectGetString(data, "sev-type"); + int sevType; + + if ((sevType = qemuMonitorSEVGuestTypeFromString(sevTypeStr)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown SEV type '%1$s'"), + sevTypeStr); + return -1; + } + + info->type = sevType; + } else { + info->type = QEMU_MONITOR_SEV_GUEST_TYPE_SEV; + } + + switch (info->type) { + case QEMU_MONITOR_SEV_GUEST_TYPE_SEV: + if (virJSONValueObjectGetNumberUint(data, "policy", &info->data.sev.policy) < 0 || + virJSONValueObjectGetNumberUint(data, "handle", &info->data.sev.handle) < 0) { + goto error_report; + } + break; + + case QEMU_MONITOR_SEV_GUEST_TYPE_SEV_SNP: + if (virJSONValueObjectGetNumberUlong(data, "snp-policy", &info->data.sev_snp.snp_policy) < 0) + goto error_report; + break; + + case QEMU_MONITOR_SEV_GUEST_TYPE_LAST: + break; } return 0; + + error_report: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-sev reply was missing some data")); + return -1; } diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 9684660d86..921dd34ed2 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -417,12 +417,8 @@ qemuMonitorJSONGetSEVMeasurement(qemuMonitor *mon); int qemuMonitorJSONGetSEVInfo(qemuMonitor *mon, - unsigned int *apiMajor, - unsigned int *apiMinor, - unsigned int *buildID, - unsigned int *policy) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) - ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5); + qemuMonitorSEVInfo *info) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuMonitorJSONGetVersion(qemuMonitor *mon, diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 45cee23798..66d0c127ca 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2730,10 +2730,7 @@ testQemuMonitorJSONGetSEVInfo(const void *opaque) const testGenericData *data = opaque; virDomainXMLOption *xmlopt = data->xmlopt; g_autoptr(qemuMonitorTest) test = NULL; - unsigned int apiMajor = 0; - unsigned int apiMinor = 0; - unsigned int buildID = 0; - unsigned int policy = 0; + qemuMonitorSEVInfo info = { }; if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema))) return -1; @@ -2753,16 +2750,70 @@ testQemuMonitorJSONGetSEVInfo(const void *opaque) "}") < 0) return -1; - if (qemuMonitorGetSEVInfo(qemuMonitorTestGetMonitor(test), - &apiMajor, &apiMinor, &buildID, &policy) < 0) + if (qemuMonitorGetSEVInfo(qemuMonitorTestGetMonitor(test), &info) < 0) return -1; - if (apiMajor != 1 || apiMinor != 8 || buildID != 834 || policy != 3) { + if (info.apiMajor != 1 || info.apiMinor != 8 || info.buildID != 834 || + info.type != QEMU_MONITOR_SEV_GUEST_TYPE_SEV || + info.data.sev.policy != 3 || info.data.sev.handle != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Unexpected SEV info values"); return -1; } + if (qemuMonitorTestAddItem(test, "query-sev", + "{" + " \"return\": {" + " \"enabled\": true," + " \"api-minor\": 55," + " \"handle\": 1," + " \"state\": \"running\"," + " \"api-major\": 1," + " \"sev-type\": \"sev\"," + " \"build-id\": 21," + " \"policy\": 1" + " }," + " \"id\": \"libvirt-16\"" + "}") < 0) + return -1; + + if (qemuMonitorGetSEVInfo(qemuMonitorTestGetMonitor(test), &info) < 0) + return -1; + + if (info.apiMajor != 1 || info.apiMinor != 55 || info.buildID != 21 || + info.type != QEMU_MONITOR_SEV_GUEST_TYPE_SEV || + info.data.sev.policy != 1 || info.data.sev.handle != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "Unexpected SEV info values"); + return -1; + } + + if (qemuMonitorTestAddItem(test, "query-sev", + "{" + " \"return\": {" + " \"enabled\": true," + " \"api-minor\": 55," + " \"state\": \"running\"," + " \"api-major\": 1," + " \"sev-type\": \"sev-snp\"," + " \"build-id\": 21," + " \"snp-policy\": 196608" + " }," + " \"id\": \"libvirt-16\"" + "}") < 0) + return -1; + + if (qemuMonitorGetSEVInfo(qemuMonitorTestGetMonitor(test), &info) < 0) + return -1; + + if (info.apiMajor != 1 || info.apiMinor != 55 || info.buildID != 21 || + info.type != QEMU_MONITOR_SEV_GUEST_TYPE_SEV_SNP || + info.data.sev_snp.snp_policy != 0x30000) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "Unexpected SEV SNP info values"); + return -1; + } + return 0; } -- 2.44.2

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt-domain.h | 10 ++++++++++ src/qemu/qemu_driver.c | 6 ++++++ 2 files changed, 16 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 2f5b01bbfe..8f00e9e959 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -6312,6 +6312,16 @@ int virDomainSetLifecycleAction(virDomainPtr domain, */ # define VIR_DOMAIN_LAUNCH_SECURITY_SEV_POLICY "sev-policy" +/** + * VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP_POLICY: + * + * Macro represents the policy of the SEV-SNP guest, + * as VIR_TYPED_PARAM_ULLONG. + * + * Since: 10.5.0 + */ +# define VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP_POLICY "sev-snp-policy" + /** * VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET_HEADER: * diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 068c721d9f..1a71857147 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19088,6 +19088,12 @@ qemuDomainGetSEVInfo(virDomainObj *vm, break; case QEMU_MONITOR_SEV_GUEST_TYPE_SEV_SNP: + if (virTypedParamsAddULLong(params, nparams, &maxpar, + VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP_POLICY, + info.data.sev_snp.snp_policy) < 0) + goto endjob; + break; + case QEMU_MONITOR_SEV_GUEST_TYPE_LAST: break; } -- 2.44.2

This capability tracks sev-snp-guest object availability. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_capabilities.h | 3 +++ tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml | 1 + 3 files changed, 8 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 28c20a9555..fe704d16dd 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -708,6 +708,9 @@ VIR_ENUM_IMPL(virQEMUCaps, "usb-mtp", /* QEMU_CAPS_DEVICE_USB_MTP */ "machine.virt.ras", /* QEMU_CAPS_MACHINE_VIRT_RAS */ "virtio-sound", /* QEMU_CAPS_DEVICE_VIRTIO_SOUND */ + + /* 460 */ + "sev-snp-guest", /* QEMU_CAPS_SEV_SNP_GUEST */ ); @@ -1393,6 +1396,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "usb-mtp", QEMU_CAPS_DEVICE_USB_MTP }, { "virtio-sound-pci", QEMU_CAPS_DEVICE_VIRTIO_SOUND }, { "virtio-sound-device", QEMU_CAPS_DEVICE_VIRTIO_SOUND }, + { "sev-snp-guest", QEMU_CAPS_SEV_SNP_GUEST }, }; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 00b4066e9a..a98da8c2eb 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -688,6 +688,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_MACHINE_VIRT_RAS, /* -machine virt,ras= */ QEMU_CAPS_DEVICE_VIRTIO_SOUND, /* -device virtio-sound-* */ + /* 460 */ + QEMU_CAPS_SEV_SNP_GUEST, /* -object sev-snp-guest */ + QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml index e0332ce1e8..a9973a0913 100644 --- a/tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml @@ -203,6 +203,7 @@ <flag name='display-reload'/> <flag name='usb-mtp'/> <flag name='virtio-sound'/> + <flag name='sev-snp-guest'/> <version>9000050</version> <microcodeVersion>43100246</microcodeVersion> <package>v9.0.0-1388-g80e8f06021-dirty</package> -- 2.44.2

SEV-SNP is an enhancement of SEV/SEV-ES and thus it shares some fields with it. Nevertheless, on XML level, it's yet another type of <launchSecurity/>. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/formatdomain.rst | 108 ++++++++++++++++++ src/conf/domain_conf.c | 73 ++++++++++++ src/conf/domain_conf.h | 15 +++ src/conf/domain_validate.c | 44 +++++++ src/conf/schemas/domaincommon.rng | 49 ++++++++ src/conf/virconftypes.h | 2 + src/qemu/qemu_cgroup.c | 1 + src/qemu/qemu_command.c | 4 + src/qemu/qemu_driver.c | 1 + src/qemu/qemu_firmware.c | 3 + src/qemu/qemu_namespace.c | 1 + src/qemu/qemu_process.c | 3 + src/qemu/qemu_validate.c | 9 ++ src/security/security_dac.c | 2 + ...launch-security-sev-snp.x86_64-latest.args | 34 ++++++ .../launch-security-sev-snp.x86_64-latest.xml | 1 + .../launch-security-sev-snp.xml | 47 ++++++++ tests/qemuxmlconftest.c | 2 + 18 files changed, 399 insertions(+) create mode 100644 tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.args create mode 120000 tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/launch-security-sev-snp.xml diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 00f861e385..5c09b87d2b 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -8867,6 +8867,114 @@ spec <https://support.amd.com/TechDocs/55766_SEV-KM_API_Specification.pdf>`__ session blob defined in the SEV API spec. See SEV spec LAUNCH_START section for the session blob format. + +Some modern AMD processors support Secure Encrypted Virtualization with Secure +Nested Paging enhancement, also known as SEV-SNP. :since:`Since 10.5.0` To +enable it ``<launchSecurity type='sev-snp'>`` should be used. It shares some +attributes and elements with ``type='sev'`` but differs in others. Example configuration: + +:: + + <domain> + ... + <launchSecurity type='sev-snp' authorKey='yes' vcek='no'> + <cbitpos>47</cbitpos> + <reducedPhysBits>1</reducedPhysBits> + <policy>0x00030000</policy> + <guestVisibleWorkarounds>...</guestVisibleWorkarounds> + <idBlock>...</idBlock> + <idAuth>...</idAuth> + <hostData>.../hostData> + </launchSecurity> + ... + </domain> + +The ``<launchSecurity/>`` element accepts the following attributes: + +``kernelHashes`` + The optional ``kernelHashes`` attribute indicates whether the + hashes of the kernel, ramdisk and command line should be included + in the measurement done by the firmware. This is only valid if + using direct kernel boot. + +``authorKey`` + The optional ``authorKey`` attribute indicates whether ``<idAuth/>`` element + contains the 'AUTHOR_KEY' field defined SEV-SNP firmware ABI. + +``vcek`` + The optional ``vcek`` attribute indicates whether the guest is allowed to + chose between VLEK (Versioned Loaded Endorsement Key) or VCEK (Versioned + Chip Endorsement Key) when requesting attestation reports from firmware. + Set this to ``no`` to disable the use of VCEK. + +Aforementioned SEV-SNP firmware ABI can be found here: +`<https://www.amd.com/system/files/TechDocs/56860.pdf>`__ + +The ``<launchSecurity/>`` element then accepts the following child elements: + +``cbitpos`` + The required ``cbitpos`` element provides the C-bit (aka encryption bit) + location in guest page table entry. The value of ``cbitpos`` is hypervisor + dependent and can be obtained through the ``sev`` element from the domain + capabilities. +``reducedPhysBits`` + The required ``reducedPhysBits`` element provides the physical address bit + reduction. Similar to ``cbitpos`` the value of ``reduced-phys-bit`` is + hypervisor dependent and can be obtained through the ``sev`` element from the + domain capabilities. +``policy`` + The required ``policy`` element provides the guest policy which must be + maintained by the SEV-SNP firmware. This policy is enforced by the firmware + and restricts what configuration and operational commands can be performed + on this guest by the hypervisor. The guest policy provided during guest + launch is bound to the guest and cannot be changed throughout the lifetime + of the guest. The policy is also transmitted during snapshot and migration + flows and enforced on the destination platform. The guest policy is a 64bit + unsigned number with the fields shown in table (See section `4.3 Guest + Policy` in aforementioned firmware ABI specification): + + ====== ========================================================================================= + Bit(s) Description + ====== ========================================================================================= + 63:25 Reserved. Must be zero. + 24 Ciphertext hiding must be enabled when set, otherwise may be enabled or disabled. + 23 Running Average Power Limit (RAPL) must be disabled when set. + 22 Require AES 256 XTS for memory encryption when set, otherwise AES 128 XEX may be allowed. + 21 CXL can be populated with devices or memory when set. + 20 Guest can be activated only on one socket when set. + 19 Debugging is allowed when set. + 18 Association with a migration agent is allowed when set. + 17 Reserved. Must be set. + 16 SMT is allowed. + 15:8 The minimum ABI major version required for this guest to run. + 7:0 The minimum ABI minor version required for this guest to run. + ====== ========================================================================================= + + The default value is hypervisor dependant and QEMU defaults to value 0x30000 + meaning no minimum ABI major/minor version is required and SMT is allowed. + +``guestVisibleWorkarounds`` + The optional ``guestVisibleWorkarounds`` element is a 16-byte, + base64-encoded blob to report hypervisor-defined workarounds, corresponding + to the 'GOSVW' parameter of the SNP_LAUNCH_START command defined in the + SEV-SNP firmware ABI. + +``idBlock`` + The optional ``idBlock`` element is a 96-byte, base64-encoded blob to + provide the 'ID Block' structure for the SNP_LAUNCH_FINISH command defined + in the SEV-SNP firmware ABI. + +``idAuth`` + The optional ``idAuth`` element is a 4096-byte, base64-encoded blob to + provide the 'ID Authentication Information Structure' for the + SNP_LAUNCH_FINISH command defined in the SEV-SNP firmware ABI. + +``hostData`` + The optional ``hostData`` element is a 32-byte, base64-encoded, user-defined + blob to provide to the guest, as documented for the 'HOST_DATA' parameter of + the SNP_LAUNCH_FINISH command in the SEV-SNP firmware ABI. + + Example configs =============== diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 102a011be8..cb1154b23f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1509,6 +1509,7 @@ VIR_ENUM_IMPL(virDomainLaunchSecurity, VIR_DOMAIN_LAUNCH_SECURITY_LAST, "", "sev", + "sev-snp", "s390-pv", ); @@ -3835,6 +3836,12 @@ virDomainSecDefFree(virDomainSecDef *def) g_free(def->data.sev.dh_cert); g_free(def->data.sev.session); break; + case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: + g_free(def->data.sev_snp.guest_visible_workarounds); + g_free(def->data.sev_snp.id_block); + g_free(def->data.sev_snp.id_auth); + g_free(def->data.sev_snp.host_data); + break; case VIR_DOMAIN_LAUNCH_SECURITY_PV: case VIR_DOMAIN_LAUNCH_SECURITY_NONE: case VIR_DOMAIN_LAUNCH_SECURITY_LAST: @@ -13676,6 +13683,36 @@ virDomainSEVDefParseXML(virDomainSEVDef *def, } +static int +virDomainSEVSNPDefParseXML(virDomainSEVSNPDef *def, + xmlXPathContextPtr ctxt) +{ + if (virDomainSEVCommonDefParseXML(&def->common, ctxt) < 0) + return -1; + + if (virXMLPropTristateBool(ctxt->node, "authorKey", VIR_XML_PROP_NONE, + &def->author_key) < 0) + return -1; + + if (virXMLPropTristateBool(ctxt->node, "vcek", VIR_XML_PROP_NONE, + &def->vcek) < 0) + return -1; + + if (virXPathULongLongBase("string(./policy)", ctxt, 16, &def->policy) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("failed to get launch security policy")); + return -1; + } + + def->guest_visible_workarounds = virXPathString("string(./guestVisibleWorkarounds)", ctxt); + def->id_block = virXPathString("string(./idBlock)", ctxt); + def->id_auth = virXPathString("string(./idAuth)", ctxt); + def->host_data = virXPathString("string(./hostData)", ctxt); + + return 0; +} + + static virDomainSecDef * virDomainSecDefParseXML(xmlNodePtr lsecNode, xmlXPathContextPtr ctxt) @@ -13695,6 +13732,10 @@ virDomainSecDefParseXML(xmlNodePtr lsecNode, if (virDomainSEVDefParseXML(&sec->data.sev, ctxt) < 0) return NULL; break; + case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: + if (virDomainSEVSNPDefParseXML(&sec->data.sev_snp, ctxt) < 0) + return NULL; + break; case VIR_DOMAIN_LAUNCH_SECURITY_PV: break; case VIR_DOMAIN_LAUNCH_SECURITY_NONE: @@ -26683,6 +26724,34 @@ virDomainSEVDefFormat(virBuffer *attrBuf, } +static void +virDomainSEVSNPDefFormat(virBuffer *attrBuf, + virBuffer *childBuf, + virDomainSEVSNPDef *def) +{ + virDomainSEVCommonDefFormat(attrBuf, childBuf, &def->common); + + if (def->author_key != VIR_TRISTATE_BOOL_ABSENT) { + virBufferAsprintf(attrBuf, " authorKey='%s'", + virTristateBoolTypeToString(def->author_key)); + } + + if (def->vcek != VIR_TRISTATE_BOOL_ABSENT) { + virBufferAsprintf(attrBuf, " vcek='%s'", + virTristateBoolTypeToString(def->vcek)); + } + + virBufferAsprintf(childBuf, "<policy>0x%08llx</policy>\n", def->policy); + + virBufferEscapeString(childBuf, + "<guestVisibleWorkarounds>%s</guestVisibleWorkarounds>\n", + def->guest_visible_workarounds); + virBufferEscapeString(childBuf, "<idBlock>%s</idBlock>\n", def->id_block); + virBufferEscapeString(childBuf, "<idAuth>%s</idAuth>\n", def->id_auth); + virBufferEscapeString(childBuf, "<hostData>%s</hostData>\n", def->host_data); +} + + static void virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) { @@ -26700,6 +26769,10 @@ virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) virDomainSEVDefFormat(&attrBuf, &childBuf, &sec->data.sev); break; + case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: + virDomainSEVSNPDefFormat(&attrBuf, &childBuf, &sec->data.sev_snp); + break; + case VIR_DOMAIN_LAUNCH_SECURITY_PV: break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c6c3c2e2a5..2818a9f1f5 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2860,6 +2860,7 @@ struct _virDomainKeyWrapDef { typedef enum { VIR_DOMAIN_LAUNCH_SECURITY_NONE, VIR_DOMAIN_LAUNCH_SECURITY_SEV, + VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP, VIR_DOMAIN_LAUNCH_SECURITY_PV, VIR_DOMAIN_LAUNCH_SECURITY_LAST, @@ -2882,10 +2883,24 @@ struct _virDomainSEVDef { unsigned int policy; }; + +struct _virDomainSEVSNPDef { + virDomainSEVCommonDef common; + unsigned long long policy; + char *guest_visible_workarounds; + char *id_block; + char *id_auth; + char *host_data; + virTristateBool author_key; + virTristateBool vcek; +}; + + struct _virDomainSecDef { virDomainLaunchSecurity sectype; union { virDomainSEVDef sev; + virDomainSEVSNPDef sev_snp; } data; }; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 395e036e8f..0661caef68 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1800,6 +1800,47 @@ virDomainDefValidateIOThreads(const virDomainDef *def) } +#define CHECK_BASE64_LEN(val, elemName, exp_len) \ +{ \ + size_t len; \ + g_autofree unsigned char *tmp = NULL; \ + if (val && (tmp = g_base64_decode(val, &len)) && len != exp_len) { \ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \ + _("Unexpected length of '%1$s', expected %2$u got %3$zu"), \ + elemName, exp_len, len); \ + return -1; \ + } \ +} + +static int +virDomainDefLaunchSecurityValidate(const virDomainDef *def) +{ + virDomainSEVSNPDef *sev_snp; + + if (!def->sec) + return 0; + + switch (def->sec->sectype) { + case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: + sev_snp = &def->sec->data.sev_snp; + + CHECK_BASE64_LEN(sev_snp->guest_visible_workarounds, "guestVisibleWorkarounds", 16); + CHECK_BASE64_LEN(sev_snp->id_block, "idBlock", 96); + CHECK_BASE64_LEN(sev_snp->id_auth, "idAuth", 4096); + CHECK_BASE64_LEN(sev_snp->host_data, "hostData", 32); + break; + + case VIR_DOMAIN_LAUNCH_SECURITY_NONE: + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + case VIR_DOMAIN_LAUNCH_SECURITY_PV: + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + } + + return 0; +} + +#undef CHECK_BASE64_LEN + static int virDomainDefValidateInternal(const virDomainDef *def, virDomainXMLOption *xmlopt) @@ -1855,6 +1896,9 @@ virDomainDefValidateInternal(const virDomainDef *def, if (virDomainDefValidateIOThreads(def) < 0) return -1; + if (virDomainDefLaunchSecurityValidate(def) < 0) + return -1; + return 0; } diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 9a7649df1c..844a931deb 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -515,6 +515,9 @@ <group> <ref name="launchSecuritySEV"/> </group> + <group> + <ref name="launchSecuritySEVSNP"/> + </group> <group> <attribute name="type"> <value>s390-pv</value> @@ -569,6 +572,52 @@ </interleave> </define> + <define name="launchSecuritySEVSNP"> + <attribute name="type"> + <value>sev-snp</value> + </attribute> + <optional> + <attribute name="kernelHashes"> + <ref name="virYesNo"/> + </attribute> + </optional> + <optional> + <attribute name="authorKey"> + <ref name="virYesNo"/> + </attribute> + </optional> + <optional> + <attribute name="vcek"> + <ref name="virYesNo"/> + </attribute> + </optional> + <interleave> + <ref name="launchSecuritySEVCommon"/> + <element name="policy"> + <ref name="hexuint"/> + </element> + <optional> + <element name="guestVisibleWorkarounds"> + <data type="string"/> + </element> + </optional> + <optional> + <element name="idBlock"> + <data type="string"/> + </element> + </optional> + <optional> + <element name="idAuth"> + <data type="string"/> + </element> + </optional> + <optional> + <element name="hostData"> + <data type="string"/> + </element> + </optional> + </interleave> + </define> <!-- Enable or disable perf events for the domain. For each of the events the following rules apply: diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index 34bb1e262f..d8e7c5278c 100644 --- a/src/conf/virconftypes.h +++ b/src/conf/virconftypes.h @@ -214,6 +214,8 @@ typedef struct _virDomainSEVCommonDef virDomainSEVCommonDef; typedef struct _virDomainSEVDef virDomainSEVDef; +typedef struct _virDomainSEVSNPDef virDomainSEVSNPDef; + typedef struct _virDomainSecDef virDomainSecDef; typedef struct _virDomainShmemDef virDomainShmemDef; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 9e559c7c4f..23b7e6b4e8 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -848,6 +848,7 @@ qemuSetupDevicesCgroup(virDomainObj *vm) if (vm->def->sec) { switch (vm->def->sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: if (qemuSetupSEVCgroup(vm) < 0) return -1; break; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1879fa608c..8d83417bb6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7062,6 +7062,8 @@ qemuBuildMachineCommandLine(virCommand *cmd, virBufferAddLit(&buf, ",memory-encryption=lsec0"); } break; + case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: + break; case VIR_DOMAIN_LAUNCH_SECURITY_PV: virBufferAddLit(&buf, ",confidential-guest-support=lsec0"); break; @@ -9781,6 +9783,8 @@ qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd, case VIR_DOMAIN_LAUNCH_SECURITY_SEV: return qemuBuildSEVCommandLine(vm, cmd, &sec->data.sev); break; + case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: + break; case VIR_DOMAIN_LAUNCH_SECURITY_PV: return qemuBuildPVCommandLine(vm, cmd); break; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1a71857147..fc1704f4fc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19128,6 +19128,7 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain, switch (vm->def->sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: if (qemuDomainGetSEVInfo(vm, params, nparams, flags) < 0) goto cleanup; break; diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 8946dc1aa5..262eeecc5c 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -1338,6 +1338,9 @@ qemuFirmwareMatchDomain(const virDomainDef *def, return false; } break; + + case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: + break; case VIR_DOMAIN_LAUNCH_SECURITY_PV: break; case VIR_DOMAIN_LAUNCH_SECURITY_NONE: diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c index fb7c9329dd..bbe3d5a1f7 100644 --- a/src/qemu/qemu_namespace.c +++ b/src/qemu/qemu_namespace.c @@ -653,6 +653,7 @@ qemuDomainSetupLaunchSecurity(virDomainObj *vm, switch (sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: VIR_DEBUG("Setting up launch security for SEV"); *paths = g_slist_prepend(*paths, g_strdup(QEMU_DEV_SEV)); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d14975180a..b9b6ccf1de 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6744,6 +6744,7 @@ qemuProcessPrepareDomain(virQEMUDriver *driver, if (vm->def->sec) { switch (vm->def->sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: VIR_DEBUG("Updating SEV platform info"); if (qemuProcessUpdateSEVInfo(vm) < 0) return -1; @@ -6818,6 +6819,8 @@ qemuProcessPrepareLaunchSecurityGuestInput(virDomainObj *vm) switch (sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: return qemuProcessPrepareSEVGuestInput(vm); + case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: + break; case VIR_DOMAIN_LAUNCH_SECURITY_PV: return 0; case VIR_DOMAIN_LAUNCH_SECURITY_NONE: diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 05ee477456..3cfcceafc9 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1325,6 +1325,15 @@ qemuValidateDomainDef(const virDomainDef *def, return -1; } break; + + case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_SNP_GUEST)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("SEV SNP launch security is not supported with this QEMU binary")); + 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)) { diff --git a/src/security/security_dac.c b/src/security/security_dac.c index d0864313c1..1a3b51a298 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1954,6 +1954,7 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr, if (def->sec) { switch (def->sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: if (virSecurityDACRestoreSEVLabel(mgr, def) < 0) rc = -1; break; @@ -2187,6 +2188,7 @@ virSecurityDACSetAllLabel(virSecurityManager *mgr, if (def->sec) { switch (def->sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: if (virSecurityDACSetSEVLabel(mgr, def) < 0) return -1; break; diff --git a/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.args b/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.args new file mode 100644 index 0000000000..37ac58edb8 --- /dev/null +++ b/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.args @@ -0,0 +1,34 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \ +-accel kvm \ +-cpu qemu64 \ +-m size=219136k \ +-object '{"qom-type":"memory-backend-ram","id":"pc.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 \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","read-only":false}' \ +-device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-storage","id":"ide0-0-0","bootindex":1}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.xml b/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.xml new file mode 120000 index 0000000000..0159cc057b --- /dev/null +++ b/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.xml @@ -0,0 +1 @@ +launch-security-sev-snp.xml \ No newline at end of file diff --git a/tests/qemuxmlconfdata/launch-security-sev-snp.xml b/tests/qemuxmlconfdata/launch-security-sev-snp.xml new file mode 100644 index 0000000000..b277d7de1b --- /dev/null +++ b/tests/qemuxmlconfdata/launch-security-sev-snp.xml @@ -0,0 +1,47 @@ +<domain type='kvm'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='none'/> + </devices> + <launchSecurity type='sev-snp' authorKey='yes' vcek='no'> + <cbitpos>47</cbitpos> + <reducedPhysBits>1</reducedPhysBits> + <policy>0x00030000</policy> + <guestVisibleWorkarounds>bGlidmlydGxpYnZpcnRsaQ==</guestVisibleWorkarounds> + <idBlock>bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZp</idBlock> + <idAuth>bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bA==</idAuth> + <hostData>bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnY=</hostData> + </launchSecurity> +</domain> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 5700ea314f..6733e73479 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -2849,6 +2849,8 @@ mymain(void) QEMU_CAPS_SEV_GUEST, QEMU_CAPS_LAST); + DO_TEST_CAPS_ARCH_LATEST("launch-security-sev-snp", "x86_64"); + DO_TEST_CAPS_ARCH_LATEST("launch-security-s390-pv", "s390x"); DO_TEST_CAPS_LATEST("vhost-user-fs-fd-memory"); -- 2.44.2

Pretty straightforward as qemu has 'sev-snp-guest' object which attributes maps pretty much 1:1 to our XML model. Except for @vcek where QEMU has 'vcek-disabled`, an inverted boolean, while we model it as virTristateBool. But that's easy to map too. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 44 ++++++++++++++++++- ...launch-security-sev-snp.x86_64-latest.args | 3 +- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8d83417bb6..39f6cf61c5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7056,14 +7056,13 @@ qemuBuildMachineCommandLine(virCommand *cmd, if (def->sec) { switch (def->sec->sectype) { case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT)) { virBufferAddLit(&buf, ",confidential-guest-support=lsec0"); } else { virBufferAddLit(&buf, ",memory-encryption=lsec0"); } break; - case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: - break; case VIR_DOMAIN_LAUNCH_SECURITY_PV: virBufferAddLit(&buf, ",confidential-guest-support=lsec0"); break; @@ -9755,6 +9754,46 @@ qemuBuildSEVCommandLine(virDomainObj *vm, virCommand *cmd, } +static int +qemuBuildSEVSNPCommandLine(virDomainObj *vm, + virCommand *cmd, + virDomainSEVSNPDef *def) +{ + g_autoptr(virJSONValue) props = NULL; + qemuDomainObjPrivate *priv = vm->privateData; + virTristateBool vcek_disabled = VIR_TRISTATE_BOOL_ABSENT; + + VIR_DEBUG("policy=0x%llx cbitpos=%d reduced_phys_bits=%d", + def->policy, def->common.cbitpos, def->common.reduced_phys_bits); + + /* On QEMU cmd line, there's vcek-disabled which is an inverted boolean. */ + if (def->vcek == VIR_TRISTATE_BOOL_YES) { + vcek_disabled = VIR_TRISTATE_BOOL_NO; + } else if (def->vcek == VIR_TRISTATE_BOOL_NO) { + vcek_disabled = VIR_TRISTATE_BOOL_YES; + } + + if (qemuMonitorCreateObjectProps(&props, "sev-snp-guest", "lsec0", + "u:cbitpos", def->common.cbitpos, + "u:reduced-phys-bits", def->common.reduced_phys_bits, + "T:kernel-hashes", def->common.kernel_hashes, + "U:policy", def->policy, + "S:guest-visible-workarounds", def->guest_visible_workarounds, + "S:id-block", def->id_block, + "S:id-auth", def->id_auth, + "S:host-data", def->host_data, + "T:author-key-enabled", def->author_key, + "T:vcek-disabled", vcek_disabled, + NULL) < 0) + return -1; + + if (qemuBuildObjectCommandlineFromJSON(cmd, props, priv->qemuCaps) < 0) + return -1; + + return 0; +} + + static int qemuBuildPVCommandLine(virDomainObj *vm, virCommand *cmd) { @@ -9784,6 +9823,7 @@ qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd, return qemuBuildSEVCommandLine(vm, cmd, &sec->data.sev); break; case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: + return qemuBuildSEVSNPCommandLine(vm, cmd, &sec->data.sev_snp); break; case VIR_DOMAIN_LAUNCH_SECURITY_PV: return qemuBuildPVCommandLine(vm, cmd); diff --git a/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.args b/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.args index 37ac58edb8..e74335f09d 100644 --- a/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.args +++ b/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.args @@ -10,7 +10,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -name guest=QEMUGuest1,debug-threads=on \ -S \ -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \ --machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \ +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,confidential-guest-support=lsec0,acpi=off \ -accel kvm \ -cpu qemu64 \ -m size=219136k \ @@ -30,5 +30,6 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","read-only":false}' \ -device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-storage","id":"ide0-0-0","bootindex":1}' \ -audiodev '{"id":"audio1","driver":"none"}' \ +-object '{"qom-type":"sev-snp-guest","id":"lsec0","cbitpos":47,"reduced-phys-bits":1,"policy":196608,"guest-visible-workarounds":"bGlidmlydGxpYnZpcnRsaQ==","id-block":"bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZp","id-auth":"bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnZpcnRsaWJ2aXJ0bA==","host-data":"bGlidmlydGxpYnZpcnRsaWJ2aXJ0bGlidmlydGxpYnY=","author-key-enabled":true,"vcek-disabled":true}' \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ -msg timestamp=on -- 2.44.2

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fc1704f4fc..3a76df8ddb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19185,9 +19185,10 @@ qemuDomainSetLaunchSecurityState(virDomainPtr domain, /* Currently only SEV is supported */ if (!vm->def->sec || - vm->def->sec->sectype != VIR_DOMAIN_LAUNCH_SECURITY_SEV) { + (vm->def->sec->sectype != VIR_DOMAIN_LAUNCH_SECURITY_SEV && + vm->def->sec->sectype != VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("setting a launch secret is only supported in SEV-enabled domains")); + _("setting a launch secret is only supported in SEV/SEV-SNP enabled domains")); goto cleanup; } -- 2.44.2

The firmware descriptors have 'amd-sev-snp` feature which describes whether firmware is suitable for SEV-SNP guests. Provide necessary implementation to detect the feature and pick the right firmware if guest is SEV-SNP enabled. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_firmware.c | 15 +++++++++++++++ .../qemu/firmware/60-edk2-ovmf-x64-amdsev.json | 1 + 2 files changed, 16 insertions(+) diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 262eeecc5c..424b0b3217 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -148,6 +148,7 @@ typedef enum { QEMU_FIRMWARE_FEATURE_ACPI_S4, QEMU_FIRMWARE_FEATURE_AMD_SEV, QEMU_FIRMWARE_FEATURE_AMD_SEV_ES, + QEMU_FIRMWARE_FEATURE_AMD_SEV_SNP, QEMU_FIRMWARE_FEATURE_ENROLLED_KEYS, QEMU_FIRMWARE_FEATURE_REQUIRES_SMM, QEMU_FIRMWARE_FEATURE_SECURE_BOOT, @@ -165,6 +166,7 @@ VIR_ENUM_IMPL(qemuFirmwareFeature, "acpi-s4", "amd-sev", "amd-sev-es", + "amd-sev-snp", "enrolled-keys", "requires-smm", "secure-boot", @@ -1148,6 +1150,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def, bool requiresSMM = false; bool supportsSEV = false; bool supportsSEVES = false; + bool supportsSEVSNP = false; bool supportsSecureBoot = false; bool hasEnrolledKeys = false; int reqSecureBoot; @@ -1195,6 +1198,10 @@ qemuFirmwareMatchDomain(const virDomainDef *def, supportsSEVES = true; break; + case QEMU_FIRMWARE_FEATURE_AMD_SEV_SNP: + supportsSEVSNP = true; + break; + case QEMU_FIRMWARE_FEATURE_REQUIRES_SMM: requiresSMM = true; break; @@ -1340,6 +1347,11 @@ qemuFirmwareMatchDomain(const virDomainDef *def, break; case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP: + if (!supportsSEVSNP) { + VIR_DEBUG("Domain requires SEV-SNP firmware '%s' doesn't support it", + path); + return false; + } break; case VIR_DOMAIN_LAUNCH_SECURITY_PV: break; @@ -1451,6 +1463,7 @@ qemuFirmwareEnableFeaturesModern(virDomainDef *def, case QEMU_FIRMWARE_FEATURE_ACPI_S4: case QEMU_FIRMWARE_FEATURE_AMD_SEV: case QEMU_FIRMWARE_FEATURE_AMD_SEV_ES: + case QEMU_FIRMWARE_FEATURE_AMD_SEV_SNP: case QEMU_FIRMWARE_FEATURE_VERBOSE_DYNAMIC: case QEMU_FIRMWARE_FEATURE_VERBOSE_STATIC: case QEMU_FIRMWARE_FEATURE_NONE: @@ -1501,6 +1514,7 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw, case QEMU_FIRMWARE_FEATURE_ACPI_S4: case QEMU_FIRMWARE_FEATURE_AMD_SEV: case QEMU_FIRMWARE_FEATURE_AMD_SEV_ES: + case QEMU_FIRMWARE_FEATURE_AMD_SEV_SNP: case QEMU_FIRMWARE_FEATURE_VERBOSE_DYNAMIC: case QEMU_FIRMWARE_FEATURE_VERBOSE_STATIC: case QEMU_FIRMWARE_FEATURE_LAST: @@ -1935,6 +1949,7 @@ qemuFirmwareGetSupported(const char *machine, case QEMU_FIRMWARE_FEATURE_ACPI_S4: case QEMU_FIRMWARE_FEATURE_AMD_SEV: case QEMU_FIRMWARE_FEATURE_AMD_SEV_ES: + case QEMU_FIRMWARE_FEATURE_AMD_SEV_SNP: case QEMU_FIRMWARE_FEATURE_ENROLLED_KEYS: case QEMU_FIRMWARE_FEATURE_SECURE_BOOT: case QEMU_FIRMWARE_FEATURE_VERBOSE_DYNAMIC: diff --git a/tests/qemufirmwaredata/out/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsev.json b/tests/qemufirmwaredata/out/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsev.json index 2d3b821acb..d83d394ba7 100644 --- a/tests/qemufirmwaredata/out/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsev.json +++ b/tests/qemufirmwaredata/out/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsev.json @@ -21,6 +21,7 @@ "features": [ "amd-sev", "amd-sev-es", + "amd-sev-snp", "verbose-dynamic" ] } -- 2.44.2