From 0266f7adf7128b62224897358e3d999ebf15aa26 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Tue, 11 Jun 2024 10:44:24 +0200 Subject: [PATCH 01/14] qemu_monitor_json: Report error in error paths in SEV related code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit 66efdfabd9b2c8527008c600bf66f21528d70889 upstream 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 Reviewed-by: Daniel P. Berrangé Signed-off-by: suryasaimadhu Signed-off-by: PrithivishS --- 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 d6dccfeda6..39cd90586d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8121,8 +8121,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); } @@ -8165,8 +8168,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; } -- Gitee From 07754425a58d5a3ea488ce66fd9aeadd605aa60b Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Tue, 11 Jun 2024 12:12:08 +0200 Subject: [PATCH 02/14] conf: Move some members of virDomainSEVDef into virDomainSEVCommonDef MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit d2cad18ca3c5dd88c416b6e4dd9c709f527b1dbe upstream 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. [Backport Changes] 1. In src/conf/domain_conf.h, the struct members user_id,secret_header and secret in _virDomainSEVDef were retained. These members were originally introduced in commit cbc574f26c7fa ("conf: qemu: add libvirt support reuse id for Hygon CSV") and 66ab1f1ce7ae3 ("conf: qemu: support provide inject secret for Hygon CSV") as part of the Hygon hardware support changes. Signed-off-by: Michal Privoznik Reviewed-by: Daniel P. Berrangé Signed-off-by: suryasaimadhu Signed-off-by: PrithivishS --- src/conf/domain_conf.c | 57 +++++++++++++++++++++---------- src/conf/domain_conf.h | 12 ++++--- src/conf/schemas/domaincommon.rng | 24 +++++++------ src/conf/virconftypes.h | 2 ++ src/qemu/qemu_command.c | 8 ++--- src/qemu/qemu_process.c | 18 +++++----- src/qemu/qemu_validate.c | 4 +-- 7 files changed, 78 insertions(+), 47 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e30ec0dca4..7ff980c768 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13533,8 +13533,8 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node, static int -virDomainSEVDefParseXML(virDomainSEVDef *def, - xmlXPathContextPtr ctxt) +virDomainSEVCommonDefParseXML(virDomainSEVCommonDef *def, + xmlXPathContextPtr ctxt) { int rc; @@ -13542,12 +13542,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 */ @@ -13570,6 +13564,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); def->user_id = virXPathString("string(./userid)", ctxt); @@ -18350,7 +18361,7 @@ virDomainMemorytuneDefParseMemory(xmlXPathContextPtr ctxt, if (virXMLPropUIntDefault(node, "min_bandwidth", 10, VIR_XML_PROP_NONE, &min_bandwidth, UINT_MAX) < 0) return -1; - + if (min_bandwidth != UINT_MAX && virResctrlAllocSetMemoryBandwidth(alloc, VIR_MEMORY_TYPE_MIN_BANDWIDTH, id, min_bandwidth) < 0) return -1; @@ -26732,6 +26743,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, "%d\n", def->cbitpos); + + if (def->haveReducedPhysBits) + virBufferAsprintf(childBuf, "%d\n", + def->reduced_phys_bits); +} + + static void virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) { @@ -26748,16 +26777,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)); - - if (sev->haveCbitpos) - virBufferAsprintf(&childBuf, "%d\n", sev->cbitpos); + virDomainSEVCommonDefFormat(&attrBuf, &childBuf, &sev->common); - if (sev->haveReducedPhysBits) - virBufferAsprintf(&childBuf, "%d\n", - sev->reduced_phys_bits); virBufferAsprintf(&childBuf, "0x%04x\n", sev->policy); if (sev->dh_cert) virBufferEscapeString(&childBuf, "%s\n", sev->dh_cert); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cadca33246..6fb75087fe 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2867,15 +2867,19 @@ typedef enum { } virDomainLaunchSecurity; -struct _virDomainSEVDef { - char *dh_cert; - char *session; - unsigned int policy; +struct _virDomainSEVCommonDef { bool haveCbitpos; unsigned int cbitpos; bool haveReducedPhysBits; unsigned int reduced_phys_bits; virTristateBool kernel_hashes; +}; + +struct _virDomainSEVDef { + virDomainSEVCommonDef common; + char *dh_cert; + char *session; + unsigned int policy; char *user_id; char *secret_header; char *secret; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 78773f744b..cd52843e14 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -527,6 +527,19 @@ + + + + + + + + + + + + + sev @@ -537,16 +550,7 @@ - - - - - - - - - - + diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index 9eff2f014e..37f93f1997 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 _virDomainCCADef virDomainCCADef; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index cc8ecfa9fb..b645296fb4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9814,7 +9814,7 @@ qemuBuildSEVCommandLine(virDomainObj *vm, virCommand *cmd, g_autofree char *secretpath = 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->user_id) VIR_DEBUG("user_id=%s", sev->user_id); @@ -9832,13 +9832,13 @@ qemuBuildSEVCommandLine(virDomainObj *vm, virCommand *cmd, secretpath = g_strdup_printf("%s/secret.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:user-id", sev->user_id, "S:dh-cert-file", dhpath, "S:session-file", sessionpath, - "T:kernel-hashes", sev->kernel_hashes, + "T:kernel-hashes", sev->common.kernel_hashes, "S:secret-header-file", secretheaderpath, "S:secret-file", secretpath, NULL) < 0) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4141ea813b..c9ee26df66 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6811,14 +6811,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; @@ -8636,9 +8636,9 @@ qemuProcessKill(virDomainObj *vm, unsigned int flags) } /* Request an extra delay of two seconds per current nhostdevs - * to be safe against stalls by the kernel freeing up the resources - * At the same time, Calculate the extra waiting delay required by the - * VM specifications. The unpin time during device passthrough is + * to be safe against stalls by the kernel freeing up the resources + * At the same time, Calculate the extra waiting delay required by the + * VM specifications. The unpin time during device passthrough is * related to the momory */ extraWaitingTime = vm->def->nhostdevs * 2; if (vm->def->nhostdevs > 0) { diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index e3ed00d9b0..e34e163d0a 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1201,7 +1201,7 @@ qemuValidateDomainDef(const virDomainDef *def, def->os.arch == VIR_ARCH_AARCH64 && (def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_EFI && !virDomainDefHasOldStyleUEFI(def))) { - if (!isCvm || def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_NONE) { + if (!isCvm || def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_NONE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("ACPI requires UEFI on this architecture")); return -1; @@ -1305,7 +1305,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")); -- Gitee From adab5d7021729a84c75ddfbbb00685f19f28deff Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Mon, 17 Jun 2024 11:53:46 +0200 Subject: [PATCH 03/14] conf: Drop needless NULL checks guarding virBufferEscapeString() commit be58733d903fdd4a9a64483f14dbb4af97e526f3 upstream There's no need to guard virBufferEscapeString() with a call to NULL as the very first thing the function does is check all three arguments for NULL. This patch was generated using the following spatch: @@ expression X, Y, E; @@ - if (E) virBufferEscapeString(X, Y, E); Signed-off-by: Michal Privoznik Reviewed-by: Jonathon Jongsma Signed-off-by: suryasaimadhu Signed-off-by: PrithivishS --- src/conf/capabilities.c | 6 ++-- src/conf/domain_conf.c | 57 +++++++++++------------------- src/conf/network_conf.c | 6 ++-- src/conf/node_device_conf.c | 57 ++++++++++++------------------ src/conf/snapshot_conf.c | 5 ++- src/conf/storage_encryption_conf.c | 9 ++--- src/conf/storage_source_conf.c | 3 +- src/conf/virnwfilterbindingdef.c | 3 +- 8 files changed, 55 insertions(+), 91 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 02298e40a3..aca9be7c83 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -688,10 +688,8 @@ virCapabilitiesDomainDataLookupInternal(virCaps *caps, if (domaintype > VIR_DOMAIN_VIRT_NONE) virBufferAsprintf(&buf, "domaintype=%s ", virDomainVirtTypeToString(domaintype)); - if (emulator) - virBufferEscapeString(&buf, "emulator=%s ", emulator); - if (machinetype) - virBufferEscapeString(&buf, "machine=%s ", machinetype); + virBufferEscapeString(&buf, "emulator=%s ", emulator); + virBufferEscapeString(&buf, "machine=%s ", machinetype); if (virBufferCurrentContent(&buf) && !virBufferCurrentContent(&buf)[0]) virBufferAsprintf(&buf, "%s", _("any configuration")); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7ff980c768..86a5e70d4e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5384,8 +5384,7 @@ virDomainDeviceInfoFormat(virBuffer *buf, if (rombar) virBufferAsprintf(buf, " bar='%s'", rombar); } - if (info->romfile) - virBufferEscapeString(buf, " file='%s'", info->romfile); + virBufferEscapeString(buf, " file='%s'", info->romfile); virBufferAddLit(buf, "/>\n"); } @@ -22304,8 +22303,7 @@ virSecurityDeviceLabelDefFormat(virBuffer *buf, virBufferAddLit(buf, "model) - virBufferEscapeString(buf, " model='%s'", def->model); + virBufferEscapeString(buf, " model='%s'", def->model); if (def->labelskip) virBufferAddLit(buf, " labelskip='yes'"); @@ -22500,8 +22498,7 @@ virDomainDiskSourceFormatNetwork(virBuffer *attrBuf, virBufferAsprintf(childBuf, "\n", src->timeout); if (src->protocol == VIR_STORAGE_NET_PROTOCOL_SSH) { - if (src->ssh_known_hosts_file) - virBufferEscapeString(childBuf, "\n", src->ssh_known_hosts_file); + virBufferEscapeString(childBuf, "\n", src->ssh_known_hosts_file); if (src->ssh_keyfile || src->ssh_agent) { virBufferAddLit(childBuf, "idx); - if (model) - virBufferEscapeString(&attrBuf, " model='%s'", model); + virBufferEscapeString(&attrBuf, " model='%s'", model); switch (def->type) { case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: @@ -24689,8 +24685,7 @@ virDomainChrTargetDefFormat(virBuffer *buf, case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: - if (def->target.name) - virBufferEscapeString(buf, " name='%s'", def->target.name); + virBufferEscapeString(buf, " name='%s'", def->target.name); if (def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO && def->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT && @@ -26097,9 +26092,8 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; } - if (def->data.vnc.keymap) - virBufferEscapeString(buf, " keymap='%s'", - def->data.vnc.keymap); + virBufferEscapeString(buf, " keymap='%s'", + def->data.vnc.keymap); if (def->data.vnc.sharePolicy) virBufferAsprintf(buf, " sharePolicy='%s'", @@ -26114,13 +26108,11 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - if (def->data.sdl.display) - virBufferEscapeString(buf, " display='%s'", - def->data.sdl.display); + virBufferEscapeString(buf, " display='%s'", + def->data.sdl.display); - if (def->data.sdl.xauth) - virBufferEscapeString(buf, " xauth='%s'", - def->data.sdl.xauth); + virBufferEscapeString(buf, " xauth='%s'", + def->data.sdl.xauth); if (def->data.sdl.fullscreen) virBufferAddLit(buf, " fullscreen='yes'"); @@ -26159,9 +26151,8 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: - if (def->data.desktop.display) - virBufferEscapeString(buf, " display='%s'", - def->data.desktop.display); + virBufferEscapeString(buf, " display='%s'", + def->data.desktop.display); if (def->data.desktop.fullscreen) virBufferAddLit(buf, " fullscreen='yes'"); @@ -26214,9 +26205,8 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; } - if (def->data.spice.keymap) - virBufferEscapeString(buf, " keymap='%s'", - def->data.spice.keymap); + virBufferEscapeString(buf, " keymap='%s'", + def->data.spice.keymap); if (def->data.spice.defaultMode != VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY) virBufferAsprintf(buf, " defaultMode='%s'", @@ -26593,11 +26583,9 @@ virDomainResourceDefFormat(virBuffer *buf, if (!def) return; - if (def->partition) - virBufferEscapeString(&childBuf, "%s\n", def->partition); + virBufferEscapeString(&childBuf, "%s\n", def->partition); - if (def->appid) - virBufferEscapeString(&childBuf, "\n", def->appid); + virBufferEscapeString(&childBuf, "\n", def->appid); virXMLFormatElement(buf, "resource", NULL, &childBuf); } @@ -26780,11 +26768,9 @@ virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) virDomainSEVCommonDefFormat(&attrBuf, &childBuf, &sev->common); virBufferAsprintf(&childBuf, "0x%04x\n", sev->policy); - if (sev->dh_cert) - virBufferEscapeString(&childBuf, "%s\n", sev->dh_cert); + virBufferEscapeString(&childBuf, "%s\n", sev->dh_cert); - if (sev->session) - virBufferEscapeString(&childBuf, "%s\n", sev->session); + virBufferEscapeString(&childBuf, "%s\n", sev->session); if (sev->user_id) virBufferEscapeString(&childBuf, "%s\n", sev->user_id); @@ -28041,9 +28027,8 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, for (i = 0; def->os.initenv && def->os.initenv[i]; i++) virBufferAsprintf(buf, "%s\n", def->os.initenv[i]->name, def->os.initenv[i]->value); - if (def->os.initdir) - virBufferEscapeString(buf, "%s\n", - def->os.initdir); + virBufferEscapeString(buf, "%s\n", + def->os.initdir); if (def->os.inituser) virBufferAsprintf(buf, "%s\n", def->os.inituser); if (def->os.initgroup) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 0449b6f07c..cc8956af53 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2043,10 +2043,8 @@ virNetworkDNSDefFormat(virBuffer *buf, def->srvs[i].service); virBufferEscapeString(buf, "protocol='%s'", def->srvs[i].protocol); - if (def->srvs[i].domain) - virBufferEscapeString(buf, " domain='%s'", def->srvs[i].domain); - if (def->srvs[i].target) - virBufferEscapeString(buf, " target='%s'", def->srvs[i].target); + virBufferEscapeString(buf, " domain='%s'", def->srvs[i].domain); + virBufferEscapeString(buf, " target='%s'", def->srvs[i].target); if (def->srvs[i].port) virBufferAsprintf(buf, " port='%d'", def->srvs[i].port); if (def->srvs[i].priority) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index f722ab37c6..6c78c9a263 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -176,20 +176,16 @@ virNodeDeviceCapSystemDefFormat(virBuffer *buf, { char uuidstr[VIR_UUID_STRING_BUFLEN]; - if (data->system.product_name) - virBufferEscapeString(buf, "%s\n", - data->system.product_name); + virBufferEscapeString(buf, "%s\n", + data->system.product_name); virBufferAddLit(buf, "\n"); virBufferAdjustIndent(buf, 2); - if (data->system.hardware.vendor_name) - virBufferEscapeString(buf, "%s\n", - data->system.hardware.vendor_name); - if (data->system.hardware.version) - virBufferEscapeString(buf, "%s\n", - data->system.hardware.version); - if (data->system.hardware.serial) - virBufferEscapeString(buf, "%s\n", - data->system.hardware.serial); + virBufferEscapeString(buf, "%s\n", + data->system.hardware.vendor_name); + virBufferEscapeString(buf, "%s\n", + data->system.hardware.version); + virBufferEscapeString(buf, "%s\n", + data->system.hardware.serial); virUUIDFormat(data->system.hardware.uuid, uuidstr); virBufferAsprintf(buf, "%s\n", uuidstr); virBufferAdjustIndent(buf, -2); @@ -197,15 +193,12 @@ virNodeDeviceCapSystemDefFormat(virBuffer *buf, virBufferAddLit(buf, "\n"); virBufferAdjustIndent(buf, 2); - if (data->system.firmware.vendor_name) - virBufferEscapeString(buf, "%s\n", - data->system.firmware.vendor_name); - if (data->system.firmware.version) - virBufferEscapeString(buf, "%s\n", - data->system.firmware.version); - if (data->system.firmware.release_date) - virBufferEscapeString(buf, "%s\n", - data->system.firmware.release_date); + virBufferEscapeString(buf, "%s\n", + data->system.firmware.vendor_name); + virBufferEscapeString(buf, "%s\n", + data->system.firmware.version); + virBufferEscapeString(buf, "%s\n", + data->system.firmware.release_date); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "\n"); } @@ -225,9 +218,8 @@ virNodeDeviceCapMdevTypesFormat(virBuffer *buf, virMediatedDeviceType *type = mdev_types[i]; virBufferEscapeString(buf, "\n", type->id); virBufferAdjustIndent(buf, 2); - if (type->name) - virBufferEscapeString(buf, "%s\n", - type->name); + virBufferEscapeString(buf, "%s\n", + type->name); virBufferEscapeString(buf, "%s\n", type->device_api); virBufferAsprintf(buf, @@ -451,10 +443,9 @@ virNodeDeviceCapUSBInterfaceDefFormat(virBuffer *buf, data->usb_if.subclass); virBufferAsprintf(buf, "%d\n", data->usb_if.protocol); - if (data->usb_if.description) - virBufferEscapeString(buf, - "%s\n", - data->usb_if.description); + virBufferEscapeString(buf, + "%s\n", + data->usb_if.description); } @@ -466,9 +457,8 @@ virNodeDeviceCapNetDefFormat(virBuffer *buf, virBufferEscapeString(buf, "%s\n", data->net.ifname); - if (data->net.address) - virBufferEscapeString(buf, "
%s
\n", - data->net.address); + virBufferEscapeString(buf, "
%s
\n", + data->net.address); virInterfaceLinkFormat(buf, &data->net.lnk); if (data->net.features) { for (i = 0; i < VIR_NET_DEV_FEAT_LAST; i++) { @@ -530,9 +520,8 @@ virNodeDeviceCapSCSIDefFormat(virBuffer *buf, virBufferAsprintf(buf, "%d\n", data->scsi.target); virBufferAsprintf(buf, "%d\n", data->scsi.lun); - if (data->scsi.type) - virBufferEscapeString(buf, "%s\n", - data->scsi.type); + virBufferEscapeString(buf, "%s\n", + data->scsi.type); } diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index d7fcded302..039ed77b84 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -819,9 +819,8 @@ virDomainSnapshotDefFormatInternal(virBuffer *buf, virBufferAdjustIndent(buf, 2); virBufferEscapeString(buf, "%s\n", def->parent.name); - if (def->parent.description) - virBufferEscapeString(buf, "%s\n", - def->parent.description); + virBufferEscapeString(buf, "%s\n", + def->parent.description); if (def->state) virBufferAsprintf(buf, "%s\n", virDomainSnapshotStateTypeToString(def->state)); diff --git a/src/conf/storage_encryption_conf.c b/src/conf/storage_encryption_conf.c index 1849df5c6c..b86001ec50 100644 --- a/src/conf/storage_encryption_conf.c +++ b/src/conf/storage_encryption_conf.c @@ -317,16 +317,13 @@ virStorageEncryptionInfoDefFormat(virBuffer *buf, { virBufferEscapeString(buf, "cipher_name); virBufferAsprintf(buf, " size='%u'", enc->cipher_size); - if (enc->cipher_mode) - virBufferEscapeString(buf, " mode='%s'", enc->cipher_mode); - if (enc->cipher_hash) - virBufferEscapeString(buf, " hash='%s'", enc->cipher_hash); + virBufferEscapeString(buf, " mode='%s'", enc->cipher_mode); + virBufferEscapeString(buf, " hash='%s'", enc->cipher_hash); virBufferAddLit(buf, "/>\n"); if (enc->ivgen_name) { virBufferEscapeString(buf, "ivgen_name); - if (enc->ivgen_hash) - virBufferEscapeString(buf, " hash='%s'", enc->ivgen_hash); + virBufferEscapeString(buf, " hash='%s'", enc->ivgen_hash); virBufferAddLit(buf, "/>\n"); } } diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c index f974a521b1..003fbf031e 100644 --- a/src/conf/storage_source_conf.c +++ b/src/conf/storage_source_conf.c @@ -1347,8 +1347,7 @@ int virStorageSourcePrivateDataFormatRelPath(virStorageSource *src, virBuffer *buf) { - if (src->relPath) - virBufferEscapeString(buf, "%s\n", src->relPath); + virBufferEscapeString(buf, "%s\n", src->relPath); return 0; } diff --git a/src/conf/virnwfilterbindingdef.c b/src/conf/virnwfilterbindingdef.c index 423ed7a392..fe45c84347 100644 --- a/src/conf/virnwfilterbindingdef.c +++ b/src/conf/virnwfilterbindingdef.c @@ -203,8 +203,7 @@ virNWFilterBindingDefFormatBuf(virBuffer *buf, virBufferAddLit(buf, "\n"); virBufferEscapeString(buf, "\n", def->portdevname); - if (def->linkdevname) - virBufferEscapeString(buf, "\n", def->linkdevname); + virBufferEscapeString(buf, "\n", def->linkdevname); virMacAddrFormat(&def->mac, mac); virBufferAsprintf(buf, "\n", mac); -- Gitee From 63b7504d3a4bb145e14424040ba9e03201f825c4 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Tue, 11 Jun 2024 13:00:58 +0200 Subject: [PATCH 04/14] conf: Separate SEV formatting into a function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit faa3548ed5f7d389810669e9f723029bc937fbbf upstream To avoid convolution of switch() inside of virDomainSecDefFormat() even more (as new sectypes are added), move formatting into a separate function. [Backport Changes] 1.In src/conf/domain_conf.c, in function virDomainSecDefFormat(), current commit modifies switch case VIR_DOMAIN_LAUNCH_SECURITY_SEV and migrates its contents into a separate function for improved code clarity and maintainability. As part of this migration, the 'if' logic blocks that verify the existence of 'user_id','secret_header' and 'secret' before calling virBufferEscapeString() were also moved. This refactoring ensures the hygon support introduced in commit 'cbc574f26c7f ("conf: qemu: add libvirt support reuse id for Hygon CSV")' and 'commit 66ab1f1ce7ae3 ("conf: qemu: support provide inject secret for Hygon CSV")' are intact. Signed-off-by: Michal Privoznik Reviewed-by: Daniel P. Berrangé Signed-off-by: suryasaimadhu Signed-off-by: PrithivishS --- src/conf/domain_conf.c | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 86a5e70d4e..00d51ebdd2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26749,6 +26749,27 @@ virDomainSEVCommonDefFormat(virBuffer *attrBuf, } +static void +virDomainSEVDefFormat(virBuffer *attrBuf, + virBuffer *childBuf, + virDomainSEVDef *def) +{ + virDomainSEVCommonDefFormat(attrBuf, childBuf, &def->common); + + virBufferAsprintf(childBuf, "0x%04x\n", def->policy); + virBufferEscapeString(childBuf, "%s\n", def->dh_cert); + virBufferEscapeString(childBuf, "%s\n", def->session); + if (def->user_id) + virBufferEscapeString(childBuf, "%s\n", def->user_id); + if (def->secret_header) + virBufferEscapeString(childBuf, "%s\n", + def->secret_header); + if (def->secret) + virBufferEscapeString(childBuf, "%s\n", def->secret); + +} + + static void virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) { @@ -26762,25 +26783,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, "0x%04x\n", sev->policy); - virBufferEscapeString(&childBuf, "%s\n", sev->dh_cert); - - virBufferEscapeString(&childBuf, "%s\n", sev->session); - - if (sev->user_id) - virBufferEscapeString(&childBuf, "%s\n", sev->user_id); - if (sev->secret_header) - virBufferEscapeString(&childBuf, "%s\n", sev->secret_header); - if (sev->secret) - virBufferEscapeString(&childBuf, "%s\n", sev->secret); - + case VIR_DOMAIN_LAUNCH_SECURITY_SEV: + virDomainSEVDefFormat(&attrBuf, &childBuf, &sec->data.sev); break; - } case VIR_DOMAIN_LAUNCH_SECURITY_PV: case VIR_DOMAIN_LAUNCH_SECURITY_CVM: -- Gitee From 7ebb8d32195f6cd5c8fc41f466a375d8570d2a3a Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Wed, 12 Jun 2024 10:06:57 +0200 Subject: [PATCH 05/14] Drop needless typecast to virDomainLaunchSecurity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit a44a43361feea6cf9809cb9ea5873cdf197975fb upstream 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 Reviewed-by: Daniel P. Berrangé Signed-off-by: suryasaimadhu Signed-off-by: PrithivishS --- 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 00d51ebdd2..a522c28207 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3827,7 +3827,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); @@ -13619,7 +13619,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; @@ -26782,7 +26782,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 b645296fb4..6407ceef5b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7055,7 +7055,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"); @@ -9898,7 +9898,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 85f74b494c..dfe3f4517b 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -1358,7 +1358,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 b6c8b9bb96..2a05b846f6 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 c9ee26df66..775d038c50 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7054,7 +7054,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_CCA: diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index e34e163d0a..cb9c6694f9 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1297,7 +1297,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", -- Gitee From c28f9f34aec4b2b0b399fcc070b08e2e747576d7 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Wed, 12 Jun 2024 09:29:59 +0200 Subject: [PATCH 06/14] src: Convert some _virDomainSecDef::sectype checks to switch() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit 7d16c296e3f0ed61443d3095fd6f8951de57f79d upstream 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(). [Backport Changes] 1.The enumeration value VIR_DOMAIN_LAUNCH_SECURITY_CVM, a member of the virDomainLaunchSecurity enum, was introduced in libvirt as part of CSV support in commit ddf9053ad7d. However,the switch statements in the upstream patch do not account for this new enum member. This backport ensures that VIR_DOMAIN_LAUNCH_SECURITY_CVM is correctly handled in all the newly added switch statements by adding the appropriate case block in all affected files. This change prevents compiler warnings and enhances the completeness of the code. Signed-off-by: Michal Privoznik Reviewed-by: Daniel P. Berrangé Signed-off-by: TauqirAzam Signed-off-by: PrithivishS --- src/qemu/qemu_cgroup.c | 19 +++++++++++++++---- src/qemu/qemu_driver.c | 17 +++++++++++++++-- src/qemu/qemu_process.c | 18 ++++++++++++++---- src/security/security_dac.c | 34 +++++++++++++++++++++++++++------- 4 files changed, 71 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 47402b3750..1280989a01 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -844,10 +844,21 @@ 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_CVM: + 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 df275c403c..1aab51dea8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19105,10 +19105,23 @@ 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_CVM: + 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 775d038c50..bd8e00b455 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6979,11 +6979,21 @@ 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_CVM: + 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 96aebfce5b..4be9174704 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1977,10 +1977,20 @@ 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_CVM: + 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++) { @@ -2201,10 +2211,20 @@ 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_CVM: + 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)) -- Gitee From 59fbfcc0cb980967315b85654aa636fcb4799887 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Mon, 10 Jun 2024 16:17:26 +0200 Subject: [PATCH 07/14] qemu_monitor: Allow querying SEV-SNP state in 'query-sev' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit 914b986275718b404ef5f700a7da58d5e1172c93 upstream 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 Reviewed-by: Daniel P. Berrangé Signed-off-by: TauqirAzam Signed-off-by: PrithivishS --- 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 1aab51dea8..5e295e1df3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19030,10 +19030,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); @@ -19048,14 +19045,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) @@ -19067,21 +19062,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 5cfe285d08..3169d00a83 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4091,14 +4091,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 902f82ca65..3ab9ccd56e 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1354,14 +1354,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 39cd90586d..4394c62374 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8131,6 +8131,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 @@ -8144,13 +8148,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; @@ -8165,16 +8171,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; + } + + 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; + } + break; + + case QEMU_MONITOR_SEV_GUEST_TYPE_SEV_SNP: + if (virJSONValueObjectGetNumberUlong(data, "snp-policy", &info->data.sev_snp.snp_policy) < 0) + goto error; + break; + + case QEMU_MONITOR_SEV_GUEST_TYPE_LAST: + break; } return 0; + + error: + 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 cdf9f5efc4..962bf77374 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -425,12 +425,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; } -- Gitee From f57ce42f7d3f5da5e720a4f0e91f7c2e798dcf60 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Tue, 11 Jun 2024 11:53:43 +0200 Subject: [PATCH 08/14] qemu: Report snp-policy in virDomainGetLaunchSecurityInfo() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit be26d0ebbed6167cef69f892ca0e7618fefa9b91 upstream Signed-off-by: Michal Privoznik Reviewed-by: Daniel P. Berrangé Signed-off-by: TauqirAzam Signed-off-by: PrithivishS --- 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 a2549d001c..2d8ec632c5 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -6330,6 +6330,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 5e295e1df3..e79649e676 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19082,6 +19082,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; } -- Gitee From 1da7d37e557ab373b49e90506240606b9763ddba Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Thu, 23 Feb 2023 15:47:11 +0100 Subject: [PATCH 09/14] qemu: capabilities: Introduce QEMU_CAPS_VIRTIO_BLK_IOTHREAD_MAPPING MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit ee7121ab8ec77703a8554dea5d393cc4d91f2720 upstream The capability represents the support for mapping virtqueues to iothreads for the 'virtio-blk' device. [Backport Changes] Changes in tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml were skipped because Libvirt 9.10 supports XML files only for QEMU 8.2 and earlier versions. Since the backport effort focuses on enabling Euler Libvirt version 9.10 to work with Euler QEMU version 8.2, the XML file changes are limited to QEMU versions 8.2 and below. Signed-off-by: Peter Krempa Reviewed-by: Ján Tomko Signed-off-by: TauqirAzam Signed-off-by: PrithivishS --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 3 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 90fb603adf..4ebe6daf4c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -698,6 +698,7 @@ VIR_ENUM_IMPL(virQEMUCaps, /* 450 */ "run-with.async-teardown", /* QEMU_CAPS_RUN_WITH_ASYNC_TEARDOWN */ "virtio-blk-vhost-vdpa", /* QEMU_CAPS_DEVICE_VIRTIO_BLK_VHOST_VDPA */ + "virtio-blk.iothread-mapping", /* QEMU_CAPS_VIRTIO_BLK_IOTHREAD_MAPPING */ "smp-clusters", /* QEMU_CAPS_SMP_CLUSTERS */ "tmm-guest", /* QEMU_CAPS_VIRTCCA */ "rme-guest", /* QEMU_CAPS_CCA_GUEST */ @@ -1432,6 +1433,7 @@ static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsVirtioBlk[] = { { "scsi", QEMU_CAPS_VIRTIO_BLK_SCSI, virQEMUCapsDevicePropsVirtioBlkSCSIDefault }, { "queue-size", QEMU_CAPS_VIRTIO_BLK_QUEUE_SIZE, NULL }, { "acpi-index", QEMU_CAPS_ACPI_INDEX, NULL }, + { "iothread-vq-mapping", QEMU_CAPS_VIRTIO_BLK_IOTHREAD_MAPPING, NULL }, }; static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsVirtioNet[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index e4a97bc491..6d16b2382d 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -677,6 +677,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 450 */ QEMU_CAPS_RUN_WITH_ASYNC_TEARDOWN, /* asynchronous teardown -run-with async-teardown=on|off */ QEMU_CAPS_DEVICE_VIRTIO_BLK_VHOST_VDPA, /* virtio-blk-vhost-vdpa block driver */ + QEMU_CAPS_VIRTIO_BLK_IOTHREAD_MAPPING, /* virtio-blk supports per-virtqueue iothread mapping */ QEMU_CAPS_SMP_CLUSTERS, /* -smp clusters= */ QEMU_CAPS_VIRTCCA, /* tmm-guest */ QEMU_CAPS_CCA_GUEST, /* -object rme-guest */ -- Gitee From 0bf8b6ebee7393884f870fa2cb328b8a9730cae7 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Wed, 12 Jun 2024 09:04:16 +0200 Subject: [PATCH 10/14] qemu_capabilities: Introduce QEMU_CAPS_SEV_SNP_GUEST MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit 1abcba9d4d8566b298168e1f57bce7507a0790b4 upstream This capability tracks sev-snp-guest object availability. [Backport Changes] 1. In the file src/qemu/qemu_capabilities.c, within enum VIR_ENUM_IMPL, added missing members from index 454 to 459 prior to sev-snp-guest to support sev-snp and to maintain the enum integrity. 2. In the file src/qemu/qemu_capabilities.h, within enum typedef, added missing members from index 454 to 459 prior to sev-snp-guest to support sev-snp and to maintain the enum integrity. 3. Changes in tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml were skipped because Libvirt 9.10 supports XML files only for QEMU 8.2 and earlier versions. Since the backport effort focuses on enabling Euler Libvirt version 9.10 to work with Euler QEMU version 8.2, the XML file changes are limited to QEMU versions 8.2 and below. Signed-off-by: Michal Privoznik Reviewed-by: Daniel P. Berrangé Signed-off-by: suryasaimadhu Signed-off-by: PrithivishS --- src/qemu/qemu_capabilities.c | 19 +++++++++++++++++-- src/qemu/qemu_capabilities.h | 12 ++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4ebe6daf4c..52689842d6 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -699,9 +699,21 @@ VIR_ENUM_IMPL(virQEMUCaps, "run-with.async-teardown", /* QEMU_CAPS_RUN_WITH_ASYNC_TEARDOWN */ "virtio-blk-vhost-vdpa", /* QEMU_CAPS_DEVICE_VIRTIO_BLK_VHOST_VDPA */ "virtio-blk.iothread-mapping", /* QEMU_CAPS_VIRTIO_BLK_IOTHREAD_MAPPING */ - "smp-clusters", /* QEMU_CAPS_SMP_CLUSTERS */ - "tmm-guest", /* QEMU_CAPS_VIRTCCA */ + + "smp-clusters", /* QEMU_CAPS_SMP_CLUSTERS */ + "tmm-guest", /* QEMU_CAPS_VIRTCCA */ + + /* 455 */ + "blockjob.backing-mask-protocol", /* QEMU_CAPS_BLOCKJOB_BACKING_MASK_PROTOCOL */ + "display-reload", /* QEMU_CAPS_DISPLAY_RELOAD */ + "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 */ "rme-guest", /* QEMU_CAPS_CCA_GUEST */ + ); @@ -1395,7 +1407,10 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "cryptodev-backend-lkcf", QEMU_CAPS_OBJECT_CRYPTO_LKCF }, { "pvpanic-pci", QEMU_CAPS_DEVICE_PANIC_PCI }, { "tmm-guest", QEMU_CAPS_VIRTCCA }, + { "sev-snp-guest", QEMU_CAPS_SEV_SNP_GUEST }, { "rme-guest", QEMU_CAPS_CCA_GUEST }, + + }; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 6d16b2382d..3b6f8cce41 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -679,9 +679,21 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_DEVICE_VIRTIO_BLK_VHOST_VDPA, /* virtio-blk-vhost-vdpa block driver */ QEMU_CAPS_VIRTIO_BLK_IOTHREAD_MAPPING, /* virtio-blk supports per-virtqueue iothread mapping */ QEMU_CAPS_SMP_CLUSTERS, /* -smp clusters= */ + QEMU_CAPS_VIRTCCA, /* tmm-guest */ + + /* 455 */ + QEMU_CAPS_BLOCKJOB_BACKING_MASK_PROTOCOL, /* backing-mask-protocol of block-commit/block-stream */ + QEMU_CAPS_DISPLAY_RELOAD, /* 'display-reload' qmp command is supported */ + QEMU_CAPS_DEVICE_USB_MTP, /* -device usb-mtp */ + 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_CCA_GUEST, /* -object rme-guest */ + QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; -- Gitee From db1426598f8dae200a1c3f848cf5cea91730f781 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Tue, 11 Jun 2024 11:58:41 +0200 Subject: [PATCH 11/14] conf: Introduce SEV-SNP support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit c65eba1f57ab1670f5bfc6bd178df1d5c3b09e2a upstream 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 . [Backport Changes] 1. The enumeration value VIR_DOMAIN_LAUNCH_SECURITY_CVM, which is a member of the virDomainLaunchSecurity enum, was introduced in Euler libvirt as part of CSV support in commit ddf9053. However, the switch statements in the current patch do not account for this new enum member. This backport ensures that VIR_DOMAIN_LAUNCH_SECURITY_CVM is correctly handled by adding the appropriate case block in the function virDomainDefLaunchSecurityValidate() in the file src/conf/domain_validate.c. This change prevents compiler warnings and enhances the completeness of the code. 2. The addition of the launch-security-sev-snp test cases(in tests/qemuxml2argvdata/) to the Euler Libvirt 9.10 test framework was skipped because it requires complete backport of the latest test framework from Libvirt 10.5 and above. So,this commit prioritizes enabling SEV-SNP in Libvirt 9.10 and excludes the changes to the files listed below : tests/qemuxml2argvdata/launch-security-sev-snp.x86_64-latest.args tests/qemuxml2argvdata/launch-security-sev-snp.x86_64-latest.xml tests/qemuxml2argvdata/launch-security-sev-snp.xml Signed-off-by: Michal Privoznik Reviewed-by: Daniel P. Berrangé Signed-off-by: PKumarAditya Signed-off-by: PrithivishS --- docs/formatdomain.rst | 97 +++++++++++++++++++++++++++++++ src/conf/domain_conf.c | 71 ++++++++++++++++++++++ src/conf/domain_conf.h | 14 +++++ src/conf/domain_validate.c | 45 ++++++++++++++ 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 + 14 files changed, 302 insertions(+) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 56d94bb1ba..920b6fae64 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -8765,6 +8765,11 @@ these security states. For more information see the Learn the architecture - Arm Confidential Compute Architecture software stack: ``__ +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 ```` should be used. It shares some +attributes and elements with ``type='sev'`` but differs in others. Example configuration: + :: @@ -8773,6 +8778,15 @@ Arm Confidential Compute Architecture software stack: sha256 ... + + 47 + 1 + 0x00030000 + ... + ... + ... + .../hostData> + ... @@ -8796,6 +8810,89 @@ The ```` element accepts the following attributes: The optional ``measurement-log`` element provides a way to create an event log in the format defined by the Trusted Computing Group for TPM2. +``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 ```` 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: +``__ + +The ```` 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 a522c28207..fcc12e0955 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1515,6 +1515,7 @@ VIR_ENUM_IMPL(virDomainLaunchSecurity, VIR_DOMAIN_LAUNCH_SECURITY_LAST, "", "sev", + "sev-snp", "s390-pv", "cvm", "cca", @@ -3835,6 +3836,12 @@ virDomainSecDefFree(virDomainSecDef *def) g_free(def->data.sev.secret_header); g_free(def->data.sev.secret); 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_CCA: g_free(def->data.cca.measurement_algo); g_free(def->data.cca.personalization_value); @@ -13589,6 +13596,34 @@ virDomainSEVDefParseXML(virDomainSEVDef *def, return 0; } +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 int virDomainCCADefParseXML(virDomainCCADef *def, @@ -13624,6 +13659,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: case VIR_DOMAIN_LAUNCH_SECURITY_CVM: break; @@ -26770,6 +26809,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, "0x%08llx\n", def->policy); + + virBufferEscapeString(childBuf, + "%s\n", + def->guest_visible_workarounds); + virBufferEscapeString(childBuf, "%s\n", def->id_block); + virBufferEscapeString(childBuf, "%s\n", def->id_auth); + virBufferEscapeString(childBuf, "%s\n", def->host_data); +} + + static void virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) { @@ -26787,6 +26854,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: case VIR_DOMAIN_LAUNCH_SECURITY_CVM: case VIR_DOMAIN_LAUNCH_SECURITY_CCA: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6fb75087fe..7b2c932a18 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2859,6 +2859,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_CVM, VIR_DOMAIN_LAUNCH_SECURITY_CCA, @@ -2891,10 +2892,23 @@ struct _virDomainCCADef { virTristateBool measurement_log; }; +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; virDomainCCADef cca; } data; }; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index d88ef6b915..9c14864ff3 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1787,6 +1787,48 @@ 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_CVM: + case VIR_DOMAIN_LAUNCH_SECURITY_LAST: + } + + return 0; +} + +#undef CHECK_BASE64_LEN + static int virDomainDefValidateInternal(const virDomainDef *def, virDomainXMLOption *xmlopt) @@ -1842,6 +1884,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 cd52843e14..8376da100d 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -515,6 +515,9 @@ + + + s390-pv @@ -578,6 +581,52 @@ + + + sev-snp + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +