[libvirt] [PATCH v3 0/9] x86: Secure Encrypted Virtualization (AMD)
The patch series is test with QEMU recent pull which includes SEV support: https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg03826.html This patch series provides support for launching an encrypted guest using AMD's new Secure Encrypted Virtualization (SEV) feature. SEV is an extension to the AMD-V architecture which supports running multiple VMs under the control of a hypervisor. When enabled, SEV feature allows the memory contents of a virtual machine (VM) to be transparently encrypted with a key unique to the guest VM. At very high level the flow looks this: 1. mgmt tool calls virConnectGetDomainCapabilities. This returns an XML document that includes the following <feature> ... <sev supported='yes'> <cbitpos> </cbitpos> <reduced-phys-bits> </reduced-phys-bits> <pdh> </pdh> <cert-chain> </cert-chain> </feature> If <sev> is provided then we indicate that hypervisor is capable of launching SEV guest. 2. (optional) mgmt tool can provide the PDH and Cert-chain to guest owner in case if guest owner wish to establish a secure connection with SEV firmware to negotiate a key used for validating the measurement. 3. mgmt tool requests to start a guest calling virCreateXML(), passing VIR_DOMAIN_START_PAUSED. The xml would include <launch-security type='sev'> <cbitpos> </cbitpos> /* the value is same as what is obtained via virConnectGetDomainCapabilities() <reduced-phys-bits> </reduced-phys-bits> /* the value is same as what is obtained via virConnectGetDomainCapabilities() <dh-cert> .. </dh> /* guest owners diffie-hellman key */ (optional) <session> ..</session> /* guest owners session blob */ (optional) <policy> ..</policy> /* guest policy */ (optional) 4. Libvirt generate the QEMU cli arg to enable the SEV feature, a typical args looks like this: # $QEMU .. -machine memory-encryption=sev0 \ -object sev-guest,id=sev0,dh-cert-file=<file>.... 5. Libvirt generates lifecycle VIR_DOMAIN_EVENT_SUSPENDED_PAUSED event 6. mgmt tool gets the VIR_DOMAIN_EVENT_SUSPENDED_PAUSED and calls virDomainGetLaunchSecretInfo() to retrieve the measurement of encrypted memory. 7. (optional) mgmt tool can provide the measurement value to guest owner, which can validate the measurement and gives GO/NO-GO answer. If mgmt tool gets GO then it resumes the guest otherwise it calls destroy() to kill the guest. 8. mgmt tool resumes the guest TODO: * SEV guest require to use DMA apis for the virtio devices. In order to use the DMA apis the virtio devices must have this tag <driver iommu=on ats=on> It is a bit unclear to me where these changes need to go. Do we need to modify the libvirt to automatically add these when SEV is enabled or we ask mgmt tool to make sure that it creates XML with right tag to enable the DMA APIs for virtio devices. I am looking for some suggestions. Using these patches we have succesfully booted and tested a guest both with and without SEV enabled. SEV Firmware API spec is available at: https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf Changes since v2: * make cbitpos, policy and reduced-phys-bits as unsigned int * update virDomainGetLaunchSecurityInfo to accept virTypedParameterPtr *params instead of virTypedParameterPtr params. Changes since v1: * rename <sev> -> <launch-security> for domain * add more information about policy and other fields in domaincaps.html * split the domain_conf support in two patches * add virDomainGetLaunchInfo() to retrieve the SEV measurement * extend virsh command to show the domain's launch security information * add test cases to validate newly added <launch-security> element * fix issues reported with 'make check' and 'make syntax-check' The complete git tree is available at: https://github.com/codomania/libvirt/tree/v3 Brijesh Singh (8): qemu: provide support to query the SEV capability qemu: introduce SEV feature in hypervisor capabilities conf: introduce launch-security element in domain qemu: add support to launch SEV guest libvirt: add new public API to get launch security info remote: implement the remote protocol for launch security qemu_driver: add support to launch security info virsh: implement new command for launch security Xiaogang Chen (1): tests: extend tests to include sev specific tag parsing docs/formatdomain.html.in | 120 ++++++++++++++++++++++++++++++++++++ docs/formatdomaincaps.html.in | 40 ++++++++++++ docs/schemas/domaincaps.rng | 20 ++++++ docs/schemas/domaincommon.rng | 39 ++++++++++++ include/libvirt/libvirt-domain.h | 17 +++++ src/conf/domain_capabilities.c | 20 ++++++ src/conf/domain_capabilities.h | 14 +++++ src/conf/domain_conf.c | 110 +++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 26 ++++++++ src/driver-hypervisor.h | 7 +++ src/libvirt-domain.c | 48 +++++++++++++++ src/libvirt_public.syms | 5 ++ src/qemu/qemu_capabilities.c | 40 ++++++++++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_capspriv.h | 4 ++ src/qemu/qemu_command.c | 35 +++++++++++ src/qemu/qemu_driver.c | 66 ++++++++++++++++++++ src/qemu/qemu_monitor.c | 17 +++++ src/qemu/qemu_monitor.h | 6 ++ src/qemu/qemu_monitor_json.c | 105 +++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 5 ++ src/qemu/qemu_process.c | 58 +++++++++++++++++ src/remote/remote_daemon_dispatch.c | 47 ++++++++++++++ src/remote/remote_driver.c | 42 ++++++++++++- src/remote/remote_protocol.x | 20 +++++- src/remote_protocol-structs | 11 ++++ tests/genericxml2xmlindata/sev.xml | 20 ++++++ tests/genericxml2xmloutdata/sev.xml | 22 +++++++ tests/genericxml2xmltest.c | 2 + tests/qemuxml2argvdata/sev.args | 24 ++++++++ tests/qemuxml2argvdata/sev.xml | 35 +++++++++++ tests/qemuxml2argvtest.c | 2 + tests/qemuxml2xmloutdata/sev.xml | 39 ++++++++++++ tests/qemuxml2xmltest.c | 2 + tools/virsh-domain.c | 84 +++++++++++++++++++++++++ 35 files changed, 1151 insertions(+), 2 deletions(-) create mode 100644 tests/genericxml2xmlindata/sev.xml create mode 100644 tests/genericxml2xmloutdata/sev.xml create mode 100644 tests/qemuxml2argvdata/sev.args create mode 100644 tests/qemuxml2argvdata/sev.xml create mode 100644 tests/qemuxml2xmloutdata/sev.xml -- 2.14.3
QEMU version >= 2.12 provides support for launching an encrypted VMs on AMD x86 platform using Secure Encrypted Virtualization (SEV) feature. This patch adds support to query the SEV capability from the qemu. Reviewed-by: "Daniel P. Berrangé" <berrange@redhat.com> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- src/conf/domain_capabilities.h | 13 ++++++++ src/qemu/qemu_capabilities.c | 38 ++++++++++++++++++++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_capspriv.h | 4 +++ src/qemu/qemu_monitor.c | 9 ++++++ src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 73 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 ++ 8 files changed, 144 insertions(+) diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h index fa4c1e442f57..72e9daf9120f 100644 --- a/src/conf/domain_capabilities.h +++ b/src/conf/domain_capabilities.h @@ -137,6 +137,19 @@ struct _virDomainCapsCPU { virDomainCapsCPUModelsPtr custom; }; +/* + * SEV capabilities + */ +typedef struct _virSEVCapability virSEVCapability; +typedef virSEVCapability *virSEVCapabilityPtr; +struct _virSEVCapability { + char *pdh; + char *cert_chain; + unsigned int cbitpos; + unsigned int reduced_phys_bits; +}; + + struct _virDomainCaps { virObjectLockable parent; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3eb5ed6d1a60..6da7cf7477c7 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -459,6 +459,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "pl011", "machine.pseries.max-cpu-compat", "dump-completed", + "sev", ); @@ -525,6 +526,8 @@ struct _virQEMUCaps { size_t ngicCapabilities; virGICCapability *gicCapabilities; + virSEVCapability *sevCapabilities; + virQEMUCapsHostCPUData kvmCPU; virQEMUCapsHostCPUData tcgCPU; }; @@ -1694,6 +1697,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "sclplmconsole", QEMU_CAPS_DEVICE_SCLPLMCONSOLE }, { "isa-serial", QEMU_CAPS_DEVICE_ISA_SERIAL }, { "pl011", QEMU_CAPS_DEVICE_PL011 }, + { "sev-guest", QEMU_CAPS_SEV }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { @@ -2770,6 +2774,21 @@ virQEMUCapsSetGICCapabilities(virQEMUCapsPtr qemuCaps, qemuCaps->ngicCapabilities = ncapabilities; } +void +virQEMUCapsSetSEVCapabilities(virQEMUCapsPtr qemuCaps, + virSEVCapability *capabilities) +{ + virSEVCapability *cap = qemuCaps->sevCapabilities; + + if (cap) { + VIR_FREE(cap->pdh); + VIR_FREE(cap->cert_chain); + } + + VIR_FREE(qemuCaps->sevCapabilities); + + qemuCaps->sevCapabilities = capabilities; +} static int virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps, @@ -3273,6 +3292,19 @@ virQEMUCapsProbeQMPGICCapabilities(virQEMUCapsPtr qemuCaps, return 0; } +static int +virQEMUCapsProbeQMPSEVCapabilities(virQEMUCapsPtr qemuCaps, + qemuMonitorPtr mon) +{ + virSEVCapability *caps = NULL; + + if (qemuMonitorGetSEVCapabilities(mon, &caps) < 0) + return -1; + + virQEMUCapsSetSEVCapabilities(qemuCaps, caps); + + return 0; +} bool virQEMUCapsCPUFilterFeatures(const char *name, @@ -4906,6 +4938,12 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_CACHE); + /* Probe for SEV capabilities */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV)) { + if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0) + virQEMUCapsClear(qemuCaps, QEMU_CAPS_SEV); + } + ret = 0; cleanup: return ret; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c2ec2be19311..02acae491ab5 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -444,6 +444,7 @@ typedef enum { QEMU_CAPS_DEVICE_PL011, /* -device pl011 (not user-instantiable) */ QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT, /* -machine pseries,max-cpu-compat= */ QEMU_CAPS_DUMP_COMPLETED, /* DUMP_COMPLETED event */ + QEMU_CAPS_SEV, /* -object sev-guest,... */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h index 222f3368e3b6..1fa85cc14f07 100644 --- a/src/qemu/qemu_capspriv.h +++ b/src/qemu/qemu_capspriv.h @@ -86,6 +86,10 @@ virQEMUCapsSetGICCapabilities(virQEMUCapsPtr qemuCaps, virGICCapability *capabilities, size_t ncapabilities); +void +virQEMUCapsSetSEVCapabilities(virQEMUCapsPtr qemuCaps, + virSEVCapability *capabilities); + int virQEMUCapsParseHelpStr(const char *qemu, const char *str, diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 1d67a97789e7..2820714b5c55 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4007,6 +4007,15 @@ qemuMonitorGetGICCapabilities(qemuMonitorPtr mon, return qemuMonitorJSONGetGICCapabilities(mon, capabilities); } +int +qemuMonitorGetSEVCapabilities(qemuMonitorPtr mon, + virSEVCapability **capabilities) +{ + QEMU_CHECK_MONITOR_JSON(mon); + + return qemuMonitorJSONGetSEVCapabilities(mon, capabilities); +} + int qemuMonitorNBDServerStart(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index adfa87aba91b..aaa14f66fdfb 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -767,6 +767,9 @@ int qemuMonitorSetMigrationCapability(qemuMonitorPtr mon, int qemuMonitorGetGICCapabilities(qemuMonitorPtr mon, virGICCapability **capabilities); +int qemuMonitorGetSEVCapabilities(qemuMonitorPtr mon, + virSEVCapability **capabilities); + typedef enum { QEMU_MONITOR_MIGRATE_BACKGROUND = 1 << 0, QEMU_MONITOR_MIGRATE_NON_SHARED_DISK = 1 << 1, /* migration with non-shared storage with full disk copy */ diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 08dfffdf6435..c51b98d2bda7 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6398,6 +6398,79 @@ qemuMonitorJSONGetGICCapabilities(qemuMonitorPtr mon, return ret; } +int +qemuMonitorJSONGetSEVCapabilities(qemuMonitorPtr mon, + virSEVCapability **capabilities) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr caps; + virSEVCapability *capability = NULL; + const char *pdh = NULL, *cert_chain = NULL; + int cbitpos, reduced_phys_bits; + + *capabilities = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-sev-capabilities", + NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + caps = virJSONValueObjectGetObject(reply, "return"); + + if (virJSONValueObjectGetNumberInt(caps, "cbitpos", &cbitpos) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'cbitpos' field is missing")); + goto cleanup; + } + + if (virJSONValueObjectGetNumberInt(caps, "reduced-phys-bits", + &reduced_phys_bits) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'reduced-phys-bits' field is missing")); + goto cleanup; + } + + if (!(pdh = virJSONValueObjectGetString(caps, "pdh"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'pdh' field is missing")); + goto cleanup; + } + + if (!(cert_chain = virJSONValueObjectGetString(caps, "cert-chain"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'cert-chain' field is missing")); + goto cleanup; + } + + if (VIR_ALLOC(capability) < 0) + goto cleanup; + + if (VIR_STRDUP(capability->pdh, pdh) < 0) + goto cleanup; + + if (VIR_STRDUP(capability->cert_chain, cert_chain) < 0) + goto cleanup; + + capability->cbitpos = cbitpos; + capability->reduced_phys_bits = reduced_phys_bits; + *capabilities = capability; + ret = 0; + + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + + return ret; +} + static virJSONValuePtr qemuMonitorJSONBuildInetSocketAddress(const char *host, const char *port) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index ec243becc4ae..305f789902e9 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -152,6 +152,9 @@ int qemuMonitorJSONSetMigrationCapability(qemuMonitorPtr mon, int qemuMonitorJSONGetGICCapabilities(qemuMonitorPtr mon, virGICCapability **capabilities); +int qemuMonitorJSONGetSEVCapabilities(qemuMonitorPtr mon, + virSEVCapability **capabilities); + int qemuMonitorJSONMigrate(qemuMonitorPtr mon, unsigned int flags, const char *uri); -- 2.14.3
On 03/14/2018 11:44 AM, Brijesh Singh wrote:
QEMU version >= 2.12 provides support for launching an encrypted VMs on AMD x86 platform using Secure Encrypted Virtualization (SEV) feature. This patch adds support to query the SEV capability from the qemu.
Reviewed-by: "Daniel P. Berrangé" <berrange@redhat.com> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- src/conf/domain_capabilities.h | 13 ++++++++ src/qemu/qemu_capabilities.c | 38 ++++++++++++++++++++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_capspriv.h | 4 +++ src/qemu/qemu_monitor.c | 9 ++++++ src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 73 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 ++ 8 files changed, 144 insertions(+)
Now that the 2.12 capabilities were added, I started looking through each of the upstream patch series that would make use of it - this is just "next" on my list. I tried applying just this patch, but kept getting: 29) caps_2.12.0(x86_64) ... libvirt: QEMU Driver error : internal error: query-cpu-definitions reply data was not an array a small bit of debugging found that qemuMonitorJSONGetCPUDefinitions was returning NULL for @data after/when the "query-sev-capabilities". I narrowed it down into the virQEMUCapsInitQMPMonitor when run during qemucapabilitiestest (see [1])
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h index fa4c1e442f57..72e9daf9120f 100644 --- a/src/conf/domain_capabilities.h +++ b/src/conf/domain_capabilities.h @@ -137,6 +137,19 @@ struct _virDomainCapsCPU { virDomainCapsCPUModelsPtr custom; };
+/* + * SEV capabilities + */ +typedef struct _virSEVCapability virSEVCapability; +typedef virSEVCapability *virSEVCapabilityPtr; +struct _virSEVCapability { + char *pdh; + char *cert_chain; + unsigned int cbitpos; + unsigned int reduced_phys_bits; +}; + + struct _virDomainCaps { virObjectLockable parent;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3eb5ed6d1a60..6da7cf7477c7 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -459,6 +459,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "pl011", "machine.pseries.max-cpu-compat", "dump-completed", + "sev",
This should also be "sev-guest" to be consistent with other virQEMUCapsObjectTypes entries naming. BTW: You can/should pull out this entry, the one in virQEMUCapsObjectTypes and the qemu_capabilities.h change to add the entry to virQEMUCapsFlags, then build and run: VIR_TEST_REGENERATE_OUTPUT=1 tests/qemucapabilitiestest in order to get/see the changed tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml file.
);
@@ -525,6 +526,8 @@ struct _virQEMUCaps { size_t ngicCapabilities; virGICCapability *gicCapabilities;
+ virSEVCapability *sevCapabilities; + virQEMUCapsHostCPUData kvmCPU; virQEMUCapsHostCPUData tcgCPU; }; @@ -1694,6 +1697,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "sclplmconsole", QEMU_CAPS_DEVICE_SCLPLMCONSOLE }, { "isa-serial", QEMU_CAPS_DEVICE_ISA_SERIAL }, { "pl011", QEMU_CAPS_DEVICE_PL011 }, + { "sev-guest", QEMU_CAPS_SEV }, };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { @@ -2770,6 +2774,21 @@ virQEMUCapsSetGICCapabilities(virQEMUCapsPtr qemuCaps, qemuCaps->ngicCapabilities = ncapabilities; }
+void +virQEMUCapsSetSEVCapabilities(virQEMUCapsPtr qemuCaps, + virSEVCapability *capabilities) +{ + virSEVCapability *cap = qemuCaps->sevCapabilities; + + if (cap) { + VIR_FREE(cap->pdh); + VIR_FREE(cap->cert_chain); + } + + VIR_FREE(qemuCaps->sevCapabilities); + + qemuCaps->sevCapabilities = capabilities; +}
static int virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps, @@ -3273,6 +3292,19 @@ virQEMUCapsProbeQMPGICCapabilities(virQEMUCapsPtr qemuCaps, return 0; }
+static int +virQEMUCapsProbeQMPSEVCapabilities(virQEMUCapsPtr qemuCaps, + qemuMonitorPtr mon) +{ + virSEVCapability *caps = NULL; + + if (qemuMonitorGetSEVCapabilities(mon, &caps) < 0) + return -1; + + virQEMUCapsSetSEVCapabilities(qemuCaps, caps); + + return 0; +}
bool virQEMUCapsCPUFilterFeatures(const char *name, @@ -4906,6 +4938,12 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_CACHE);
+ /* Probe for SEV capabilities */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV)) { + if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0) + virQEMUCapsClear(qemuCaps, QEMU_CAPS_SEV);
[1] If I change the Clear to be a goto cleanup, then a slightly different error appears: libvirt: QEMU Driver error : internal error: 'cbitpos' field is missing Checking the output of a LIBVIRT_DEBUG=1 VIR_TEST_DEBUG=1 VIR_TEST_RANGE=29 make -C tests check TESTS=qemucapabilitiestest gives a bit of a hint in the test-suite.log output: 2018-03-23 19:08:52.819+0000: 11727: debug : qemuMonitorTestAddResponse:108 : Adding response to monitor command: '{ "return": { }, "id": "libvirt-1" } ... So that says to me that we perhaps need some mocked output. Although I'm not 100% sure - it doesn't seem previous adjustments needed those, but there's lots that changes in this capabilities space and I'm not always up to date on the mocking code. BTW: resetting the last error in this case didn't fix the problem - it just moved the cheese back to the original error. So there's some mocked buffer somewhere that's being read - I'm hoping someone with a fresher memory on how this works will be able to debug faster than I.
+ } + ret = 0; cleanup: return ret; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c2ec2be19311..02acae491ab5 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -444,6 +444,7 @@ typedef enum { QEMU_CAPS_DEVICE_PL011, /* -device pl011 (not user-instantiable) */ QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT, /* -machine pseries,max-cpu-compat= */ QEMU_CAPS_DUMP_COMPLETED, /* DUMP_COMPLETED event */ + QEMU_CAPS_SEV, /* -object sev-guest,... */
I think this should be QEMU_CAPS_SEV_GUEST (it's a consistency in naming thing) I'll defer to Dan and Peter on the rest... John
QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h index 222f3368e3b6..1fa85cc14f07 100644 --- a/src/qemu/qemu_capspriv.h +++ b/src/qemu/qemu_capspriv.h @@ -86,6 +86,10 @@ virQEMUCapsSetGICCapabilities(virQEMUCapsPtr qemuCaps, virGICCapability *capabilities, size_t ncapabilities);
+void +virQEMUCapsSetSEVCapabilities(virQEMUCapsPtr qemuCaps, + virSEVCapability *capabilities); + int virQEMUCapsParseHelpStr(const char *qemu, const char *str, diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 1d67a97789e7..2820714b5c55 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4007,6 +4007,15 @@ qemuMonitorGetGICCapabilities(qemuMonitorPtr mon, return qemuMonitorJSONGetGICCapabilities(mon, capabilities); }
+int +qemuMonitorGetSEVCapabilities(qemuMonitorPtr mon, + virSEVCapability **capabilities) +{ + QEMU_CHECK_MONITOR_JSON(mon); + + return qemuMonitorJSONGetSEVCapabilities(mon, capabilities); +} +
int qemuMonitorNBDServerStart(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index adfa87aba91b..aaa14f66fdfb 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -767,6 +767,9 @@ int qemuMonitorSetMigrationCapability(qemuMonitorPtr mon, int qemuMonitorGetGICCapabilities(qemuMonitorPtr mon, virGICCapability **capabilities);
+int qemuMonitorGetSEVCapabilities(qemuMonitorPtr mon, + virSEVCapability **capabilities); + typedef enum { QEMU_MONITOR_MIGRATE_BACKGROUND = 1 << 0, QEMU_MONITOR_MIGRATE_NON_SHARED_DISK = 1 << 1, /* migration with non-shared storage with full disk copy */ diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 08dfffdf6435..c51b98d2bda7 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6398,6 +6398,79 @@ qemuMonitorJSONGetGICCapabilities(qemuMonitorPtr mon, return ret; }
+int +qemuMonitorJSONGetSEVCapabilities(qemuMonitorPtr mon, + virSEVCapability **capabilities) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr caps; + virSEVCapability *capability = NULL; + const char *pdh = NULL, *cert_chain = NULL; + int cbitpos, reduced_phys_bits; + + *capabilities = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-sev-capabilities", + NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + caps = virJSONValueObjectGetObject(reply, "return"); + + if (virJSONValueObjectGetNumberInt(caps, "cbitpos", &cbitpos) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'cbitpos' field is missing")); + goto cleanup; + } + + if (virJSONValueObjectGetNumberInt(caps, "reduced-phys-bits", + &reduced_phys_bits) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'reduced-phys-bits' field is missing")); + goto cleanup; + } + + if (!(pdh = virJSONValueObjectGetString(caps, "pdh"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'pdh' field is missing")); + goto cleanup; + } + + if (!(cert_chain = virJSONValueObjectGetString(caps, "cert-chain"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'cert-chain' field is missing")); + goto cleanup; + } + + if (VIR_ALLOC(capability) < 0) + goto cleanup; + + if (VIR_STRDUP(capability->pdh, pdh) < 0) + goto cleanup; + + if (VIR_STRDUP(capability->cert_chain, cert_chain) < 0) + goto cleanup; + + capability->cbitpos = cbitpos; + capability->reduced_phys_bits = reduced_phys_bits; + *capabilities = capability; + ret = 0; + + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + + return ret; +} + static virJSONValuePtr qemuMonitorJSONBuildInetSocketAddress(const char *host, const char *port) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index ec243becc4ae..305f789902e9 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -152,6 +152,9 @@ int qemuMonitorJSONSetMigrationCapability(qemuMonitorPtr mon, int qemuMonitorJSONGetGICCapabilities(qemuMonitorPtr mon, virGICCapability **capabilities);
+int qemuMonitorJSONGetSEVCapabilities(qemuMonitorPtr mon, + virSEVCapability **capabilities); + int qemuMonitorJSONMigrate(qemuMonitorPtr mon, unsigned int flags, const char *uri);
Hi John, On 03/23/2018 02:18 PM, John Ferlan wrote:
On 03/14/2018 11:44 AM, Brijesh Singh wrote:
QEMU version >= 2.12 provides support for launching an encrypted VMs on AMD x86 platform using Secure Encrypted Virtualization (SEV) feature. This patch adds support to query the SEV capability from the qemu.
Reviewed-by: "Daniel P. Berrangé" <berrange@redhat.com> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- src/conf/domain_capabilities.h | 13 ++++++++ src/qemu/qemu_capabilities.c | 38 ++++++++++++++++++++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_capspriv.h | 4 +++ src/qemu/qemu_monitor.c | 9 ++++++ src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 73 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 ++ 8 files changed, 144 insertions(+)
Now that the 2.12 capabilities were added, I started looking through each of the upstream patch series that would make use of it - this is just "next" on my list.
I tried applying just this patch, but kept getting:
29) caps_2.12.0(x86_64) ... libvirt: QEMU Driver error : internal error: query-cpu-definitions reply data was not an array
a small bit of debugging found that qemuMonitorJSONGetCPUDefinitions was returning NULL for @data after/when the "query-sev-capabilities".
I narrowed it down into the virQEMUCapsInitQMPMonitor when run during qemucapabilitiestest (see [1])
I have not tried latest libvirt yet, I will try this today and debug to see what we are missing. I did the 'make check' before submitting the patch but at that time QEMU 2.12 was not available and we did not had updated caps_2.12.0.x86_64.xml and caps_2.12.0.x86_64.replies. I will investigate a bit more and update with my findings. thanks
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h index fa4c1e442f57..72e9daf9120f 100644 --- a/src/conf/domain_capabilities.h +++ b/src/conf/domain_capabilities.h @@ -137,6 +137,19 @@ struct _virDomainCapsCPU { virDomainCapsCPUModelsPtr custom; };
+/* + * SEV capabilities + */ +typedef struct _virSEVCapability virSEVCapability; +typedef virSEVCapability *virSEVCapabilityPtr; +struct _virSEVCapability { + char *pdh; + char *cert_chain; + unsigned int cbitpos; + unsigned int reduced_phys_bits; +}; + + struct _virDomainCaps { virObjectLockable parent;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3eb5ed6d1a60..6da7cf7477c7 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -459,6 +459,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "pl011", "machine.pseries.max-cpu-compat", "dump-completed", + "sev",
This should also be "sev-guest" to be consistent with other virQEMUCapsObjectTypes entries naming.
BTW: You can/should pull out this entry, the one in virQEMUCapsObjectTypes and the qemu_capabilities.h change to add the entry to virQEMUCapsFlags, then build and run:
VIR_TEST_REGENERATE_OUTPUT=1 tests/qemucapabilitiestest
in order to get/see the changed tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml file.
);
@@ -525,6 +526,8 @@ struct _virQEMUCaps { size_t ngicCapabilities; virGICCapability *gicCapabilities;
+ virSEVCapability *sevCapabilities; + virQEMUCapsHostCPUData kvmCPU; virQEMUCapsHostCPUData tcgCPU; }; @@ -1694,6 +1697,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "sclplmconsole", QEMU_CAPS_DEVICE_SCLPLMCONSOLE }, { "isa-serial", QEMU_CAPS_DEVICE_ISA_SERIAL }, { "pl011", QEMU_CAPS_DEVICE_PL011 }, + { "sev-guest", QEMU_CAPS_SEV }, };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { @@ -2770,6 +2774,21 @@ virQEMUCapsSetGICCapabilities(virQEMUCapsPtr qemuCaps, qemuCaps->ngicCapabilities = ncapabilities; }
+void +virQEMUCapsSetSEVCapabilities(virQEMUCapsPtr qemuCaps, + virSEVCapability *capabilities) +{ + virSEVCapability *cap = qemuCaps->sevCapabilities; + + if (cap) { + VIR_FREE(cap->pdh); + VIR_FREE(cap->cert_chain); + } + + VIR_FREE(qemuCaps->sevCapabilities); + + qemuCaps->sevCapabilities = capabilities; +}
static int virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps, @@ -3273,6 +3292,19 @@ virQEMUCapsProbeQMPGICCapabilities(virQEMUCapsPtr qemuCaps, return 0; }
+static int +virQEMUCapsProbeQMPSEVCapabilities(virQEMUCapsPtr qemuCaps, + qemuMonitorPtr mon) +{ + virSEVCapability *caps = NULL; + + if (qemuMonitorGetSEVCapabilities(mon, &caps) < 0) + return -1; + + virQEMUCapsSetSEVCapabilities(qemuCaps, caps); + + return 0; +}
bool virQEMUCapsCPUFilterFeatures(const char *name, @@ -4906,6 +4938,12 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_CACHE);
+ /* Probe for SEV capabilities */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV)) { + if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0) + virQEMUCapsClear(qemuCaps, QEMU_CAPS_SEV);
[1] If I change the Clear to be a goto cleanup, then a slightly different error appears:
libvirt: QEMU Driver error : internal error: 'cbitpos' field is missing
Checking the output of a
LIBVIRT_DEBUG=1 VIR_TEST_DEBUG=1 VIR_TEST_RANGE=29 make -C tests check TESTS=qemucapabilitiestest
gives a bit of a hint in the test-suite.log output:
2018-03-23 19:08:52.819+0000: 11727: debug : qemuMonitorTestAddResponse:108 : Adding response to monitor command: '{ "return": { }, "id": "libvirt-1" } ...
So that says to me that we perhaps need some mocked output. Although I'm not 100% sure - it doesn't seem previous adjustments needed those, but there's lots that changes in this capabilities space and I'm not always up to date on the mocking code.
BTW: resetting the last error in this case didn't fix the problem - it just moved the cheese back to the original error. So there's some mocked buffer somewhere that's being read - I'm hoping someone with a fresher memory on how this works will be able to debug faster than I.
+ } + ret = 0; cleanup: return ret; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c2ec2be19311..02acae491ab5 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -444,6 +444,7 @@ typedef enum { QEMU_CAPS_DEVICE_PL011, /* -device pl011 (not user-instantiable) */ QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT, /* -machine pseries,max-cpu-compat= */ QEMU_CAPS_DUMP_COMPLETED, /* DUMP_COMPLETED event */ + QEMU_CAPS_SEV, /* -object sev-guest,... */
I think this should be QEMU_CAPS_SEV_GUEST (it's a consistency in naming thing)
I'll defer to Dan and Peter on the rest...
John
QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h index 222f3368e3b6..1fa85cc14f07 100644 --- a/src/qemu/qemu_capspriv.h +++ b/src/qemu/qemu_capspriv.h @@ -86,6 +86,10 @@ virQEMUCapsSetGICCapabilities(virQEMUCapsPtr qemuCaps, virGICCapability *capabilities, size_t ncapabilities);
+void +virQEMUCapsSetSEVCapabilities(virQEMUCapsPtr qemuCaps, + virSEVCapability *capabilities); + int virQEMUCapsParseHelpStr(const char *qemu, const char *str, diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 1d67a97789e7..2820714b5c55 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4007,6 +4007,15 @@ qemuMonitorGetGICCapabilities(qemuMonitorPtr mon, return qemuMonitorJSONGetGICCapabilities(mon, capabilities); }
+int +qemuMonitorGetSEVCapabilities(qemuMonitorPtr mon, + virSEVCapability **capabilities) +{ + QEMU_CHECK_MONITOR_JSON(mon); + + return qemuMonitorJSONGetSEVCapabilities(mon, capabilities); +} +
int qemuMonitorNBDServerStart(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index adfa87aba91b..aaa14f66fdfb 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -767,6 +767,9 @@ int qemuMonitorSetMigrationCapability(qemuMonitorPtr mon, int qemuMonitorGetGICCapabilities(qemuMonitorPtr mon, virGICCapability **capabilities);
+int qemuMonitorGetSEVCapabilities(qemuMonitorPtr mon, + virSEVCapability **capabilities); + typedef enum { QEMU_MONITOR_MIGRATE_BACKGROUND = 1 << 0, QEMU_MONITOR_MIGRATE_NON_SHARED_DISK = 1 << 1, /* migration with non-shared storage with full disk copy */ diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 08dfffdf6435..c51b98d2bda7 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6398,6 +6398,79 @@ qemuMonitorJSONGetGICCapabilities(qemuMonitorPtr mon, return ret; }
+int +qemuMonitorJSONGetSEVCapabilities(qemuMonitorPtr mon, + virSEVCapability **capabilities) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr caps; + virSEVCapability *capability = NULL; + const char *pdh = NULL, *cert_chain = NULL; + int cbitpos, reduced_phys_bits; + + *capabilities = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-sev-capabilities", + NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + caps = virJSONValueObjectGetObject(reply, "return"); + + if (virJSONValueObjectGetNumberInt(caps, "cbitpos", &cbitpos) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'cbitpos' field is missing")); + goto cleanup; + } + + if (virJSONValueObjectGetNumberInt(caps, "reduced-phys-bits", + &reduced_phys_bits) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'reduced-phys-bits' field is missing")); + goto cleanup; + } + + if (!(pdh = virJSONValueObjectGetString(caps, "pdh"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'pdh' field is missing")); + goto cleanup; + } + + if (!(cert_chain = virJSONValueObjectGetString(caps, "cert-chain"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'cert-chain' field is missing")); + goto cleanup; + } + + if (VIR_ALLOC(capability) < 0) + goto cleanup; + + if (VIR_STRDUP(capability->pdh, pdh) < 0) + goto cleanup; + + if (VIR_STRDUP(capability->cert_chain, cert_chain) < 0) + goto cleanup; + + capability->cbitpos = cbitpos; + capability->reduced_phys_bits = reduced_phys_bits; + *capabilities = capability; + ret = 0; + + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + + return ret; +} + static virJSONValuePtr qemuMonitorJSONBuildInetSocketAddress(const char *host, const char *port) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index ec243becc4ae..305f789902e9 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -152,6 +152,9 @@ int qemuMonitorJSONSetMigrationCapability(qemuMonitorPtr mon, int qemuMonitorJSONGetGICCapabilities(qemuMonitorPtr mon, virGICCapability **capabilities);
+int qemuMonitorJSONGetSEVCapabilities(qemuMonitorPtr mon, + virSEVCapability **capabilities); + int qemuMonitorJSONMigrate(qemuMonitorPtr mon, unsigned int flags, const char *uri);
On Mon, 2018-03-26 at 08:52 -0500, Brijesh Singh wrote:
I tried applying just this patch, but kept getting:
29) caps_2.12.0(x86_64) ... libvirt: QEMU Driver error : internal error: query-cpu-definitions reply data was not an array
a small bit of debugging found that qemuMonitorJSONGetCPUDefinitions was returning NULL for @data after/when the "query-sev-capabilities".
I narrowed it down into the virQEMUCapsInitQMPMonitor when run during qemucapabilitiestest (see [1])
I have not tried latest libvirt yet, I will try this today and debug to see what we are missing. I did the 'make check' before submitting the patch but at that time QEMU 2.12 was not available and we did not had updated caps_2.12.0.x86_64.xml and caps_2.12.0.x86_64.replies.
I thought the lack of churn in tests/qemucapabilitiesdata/ was weird, but thanks to your explanation it makes perfect sense now. Your code adds a call to query-sev-capabilities, but the replies file doesn't contain the corresponding return data, which makes the parser go out of sync. You're going to have to either fetch capabilities from your own QEMU 2.12 binary or hack it up by adding the return data in the right spot and call tests/qemucapsfixreplies to re-align the ids. I think you can get away with the latter, as we're going to want to refresh the replies files once 2.12 is released anyway. -- Andrea Bolognani / Red Hat / Virtualization
On 03/26/2018 09:32 AM, Andrea Bolognani wrote:
On Mon, 2018-03-26 at 08:52 -0500, Brijesh Singh wrote:
I tried applying just this patch, but kept getting:
29) caps_2.12.0(x86_64) ... libvirt: QEMU Driver error : internal error: query-cpu-definitions reply data was not an array
a small bit of debugging found that qemuMonitorJSONGetCPUDefinitions was returning NULL for @data after/when the "query-sev-capabilities".
I narrowed it down into the virQEMUCapsInitQMPMonitor when run during qemucapabilitiestest (see [1])
I have not tried latest libvirt yet, I will try this today and debug to see what we are missing. I did the 'make check' before submitting the patch but at that time QEMU 2.12 was not available and we did not had updated caps_2.12.0.x86_64.xml and caps_2.12.0.x86_64.replies.
I thought the lack of churn in tests/qemucapabilitiesdata/ was weird, but thanks to your explanation it makes perfect sense now.
Your code adds a call to query-sev-capabilities, but the replies file doesn't contain the corresponding return data, which makes the parser go out of sync.
You're going to have to either fetch capabilities from your own QEMU 2.12 binary or hack it up by adding the return data in the right spot and call tests/qemucapsfixreplies to re-align the ids.
Right, running the tests/qemucapsprobe on my system I see that query-sev-capabilities command returns a valid data. In my local branch, I updated the caps_2.12.0.x86_64.replies with response. With those changes I no longer get the internal error but tests/qemucapabilitiestest still fails for me with below error message: 29) caps_2.12.0(x86_64) ... In '/home/amd/workdir/upstream/libvirt/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml': Offset 7143 Expect [060] Actual [306] It is basically pointing to microcode version change, do I need to update the cap with new version ?
I think you can get away with the latter, as we're going to want to refresh the replies files once 2.12 is released anyway.
I am not able to follow this comment, let me explain the situation. The QEMU_CAPS_SEV flag was set to indicate QEMU supports the 'query-sev-capabilities' QMP command and sev-guest object. That merely indicates that command exist but does not means that command will always execute successfully. e.g If hypervisor does not support the SEV feature then query-sev-capabilities will return error. That means if tests/qemucapsprobe is used to generate the replies on non-SEV capable system then .replies will not contain output of query-sev-capabilities command. Will this be an issue ? -Brijesh
On Tue, 2018-03-27 at 09:26 -0500, Brijesh Singh wrote:
You're going to have to either fetch capabilities from your own QEMU 2.12 binary or hack it up by adding the return data in the right spot and call tests/qemucapsfixreplies to re-align the ids.
Right, running the tests/qemucapsprobe on my system I see that query-sev-capabilities command returns a valid data. In my local branch, I updated the caps_2.12.0.x86_64.replies with response. With those changes I no longer get the internal error but tests/qemucapabilitiestest still fails for me with below error message:
29) caps_2.12.0(x86_64) ... In '/home/amd/workdir/upstream/libvirt/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml': Offset 7143 Expect [060] Actual [306]
It is basically pointing to microcode version change, do I need to update the cap with new version ?
After you've tweaked the .replies file, you can just run $ VIR_TEST_REGENERATE_OUTPUT=1 make check and the .xml file will be refreshed, microcode version and all.
I think you can get away with the latter, as we're going to want to refresh the replies files once 2.12 is released anyway.
I am not able to follow this comment, let me explain the situation. The QEMU_CAPS_SEV flag was set to indicate QEMU supports the 'query-sev-capabilities' QMP command and sev-guest object. That merely indicates that command exist but does not means that command will always execute successfully. e.g If hypervisor does not support the SEV feature then query-sev-capabilities will return error. That means if tests/qemucapsprobe is used to generate the replies on non-SEV capable system then .replies will not contain output of query-sev-capabilities command. Will this be an issue ?
Not at all. That's already the case for a lot of features. My comment was merely pointing out that the capabilities data we have in the tree right now is for QEMU 2.12.0-rc0, and once QEMU 2.12 is officially released we will want to collect it again. It's not something you need to worry about, really :) -- Andrea Bolognani / Red Hat / Virtualization
Extend hypervisor capabilities to include sev feature. When available, hypervisor supports launching an encrypted VM on AMD platform. The sev feature tag provides additional details like platform diffie-hellman key and certificate chain which can be used by the guest owner to establish a cryptographic session with the SEV firmware to negotiate keys used for attestation or to provide secret during launch. Reviewed-by: "Daniel P. Berrangé" <berrange@redhat.com> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- docs/formatdomaincaps.html.in | 40 ++++++++++++++++++++++++++++++++++++++++ docs/schemas/domaincaps.rng | 20 ++++++++++++++++++++ src/conf/domain_capabilities.c | 20 ++++++++++++++++++++ src/conf/domain_capabilities.h | 1 + src/qemu/qemu_capabilities.c | 2 ++ 5 files changed, 83 insertions(+) diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in index 6bfcaf61caae..f38314166ac3 100644 --- a/docs/formatdomaincaps.html.in +++ b/docs/formatdomaincaps.html.in @@ -417,6 +417,12 @@ <value>3</value> </enum> </gic> + <sev> + <pdh> </pdh> + <cert-chain> </cert-chain> + <cbitpos> </cbitpos> + <reduced-phys-bits> </reduced-phys-bits> + </sev> </features> </domainCapabilities> </pre> @@ -441,5 +447,39 @@ <code>gic</code> element.</dd> </dl> + <h4><a id="elementsSEV">SEV capabilities</a></h4> + + <p>AMD Secure Encrypted Virtualization (SEV) capabilities are exposed under + the <code>sev</code> element. + SEV is an extension to the AMD-V architecture which supports running + virtual machines (VMs) under the control of a hypervisor. When supported, + guest owner can create a VM whose memory contents will be transparently + encrypted with a key unique to that VM. + + For more details on SEV feature see: + <a href="https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf"> + SEV API spec</a> and <a href="http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2013/12/AMD_Memory_Encryption_Whitepaper_v7-Public.pdf"> + SEV White Paper</a> + + </p> + + <dl> + <dt><code>pdh</code></dt> + <dd>Platform diffie-hellman key, which can be exported to remote entities + which wish to establish a secure transport context with the SEV platform + in order to transmit data securely. The key is encoded in base64</dd> + <dt><code>cert-chain</code></dt> + <dd> Platform certificate chain -- which includes platform endorsement key + (PEK), owners certificate authory (OCA) and chip endorsement key (CEK). + The certificate chain is encoded in base64.</dd> + <dt><code>cbitpos</code></dt> + <dd>When memory encryption is enabled, one of the physical address bit + (aka the C-bit) is utilized to mark if a memory page is protected. The + C-bit position is Hypervisor dependent.</dd> + <dt><code>reduced-phys-bits</code></dt> + <dd>When memory encryption is enabled, we loose certain bits in physical + address space. The number of bits we loose is hypervisor dependent.</dd> + </dl> + </body> </html> diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng index 39053181eb9a..53b33eb83c1f 100644 --- a/docs/schemas/domaincaps.rng +++ b/docs/schemas/domaincaps.rng @@ -173,6 +173,9 @@ <element name='features'> <interleave> <ref name='gic'/> + <optional> + <ref name='sev'/> + </optional> </interleave> </element> </define> @@ -184,6 +187,23 @@ </element> </define> + <define name='sev'> + <element name='sev'> + <element name='pdh'> + <data type='string'/> + </element> + <element name='cert-chain'> + <data type='string'/> + </element> + <element name='cbitpos'> + <data type='unsignedInt'/> + </element> + <element name='reduced-phys-bits'> + <data type='unsignedInt'/> + </element> + </element> + </define> + <define name='value'> <zeroOrMore> <element name='value'> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index f7d9be50f82d..082065fb4733 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -549,6 +549,25 @@ virDomainCapsFeatureGICFormat(virBufferPtr buf, FORMAT_EPILOGUE(gic); } +static void +virDomainCapsFeatureSEVFormat(virBufferPtr buf, + virSEVCapabilityPtr const sev) +{ + if (!sev) + return; + + virBufferAddLit(buf, "<sev supported='yes'>\n"); + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos); + virBufferAsprintf(buf, "<reduced-phys-bits>%d</reduced-phys-bits>\n", + sev->reduced_phys_bits); + virBufferAsprintf(buf, "<pdh>%s</pdh>\n", sev->pdh); + virBufferAsprintf(buf, "<cert-chain>%s</cert-chain>\n", + sev->cert_chain); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</sev>\n"); +} + char * virDomainCapsFormat(virDomainCapsPtr const caps) @@ -587,6 +606,7 @@ virDomainCapsFormat(virDomainCapsPtr const caps) virBufferAdjustIndent(&buf, 2); virDomainCapsFeatureGICFormat(&buf, &caps->gic); + virDomainCapsFeatureSEVFormat(&buf, caps->sev); virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</features>\n"); diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h index 72e9daf9120f..2e8596ceea58 100644 --- a/src/conf/domain_capabilities.h +++ b/src/conf/domain_capabilities.h @@ -170,6 +170,7 @@ struct _virDomainCaps { /* add new domain devices here */ virDomainCapsFeatureGIC gic; + virSEVCapabilityPtr sev; /* add new domain features here */ }; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6da7cf7477c7..4517ea19224d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5925,6 +5925,8 @@ virQEMUCapsFillDomainCaps(virCapsPtr caps, virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev) < 0 || virQEMUCapsFillDomainFeatureGICCaps(qemuCaps, domCaps) < 0) return -1; + + domCaps->sev = qemuCaps->sevCapabilities; return 0; } -- 2.14.3
The launch-security element can be used to define the security model to use when launching a domain. Currently we support 'sev'. When 'sev' is used, the VM will be launched with AMD SEV feature enabled. SEV feature supports running encrypted VM under the control of KVM. Encrypted VMs have their pages (code and data) secured such that only the guest itself has access to the unencrypted version. Each encrypted VM is associated with a unique encryption key; if its data is accessed to a different entity using a different key the encrypted guests data will be incorrectly decrypted, leading to unintelligible data. Reviewed-by: "Daniel P. Berrangé" <berrange@redhat.com> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- docs/formatdomain.html.in | 120 ++++++++++++++++++++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 39 ++++++++++++++ src/conf/domain_conf.c | 110 ++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 26 +++++++++ 4 files changed, 295 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6fd2189cd2f4..830d2a3c59be 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -8195,6 +8195,126 @@ qemu-kvm -net nic,model=? /dev/null <p>Note: DEA/TDEA is synonymous with DES/TDES.</p> + <h3><a id="sev">Secure Encrypted Virtualization (SEV)</a></h3> + + <p> + The contents of the <code><launch-security type='sev'></code> element + is used to provide the guest owners input used for creating an encrypted + VM using the AMD SEV feature. + + SEV is an extension to the AMD-V architecture which supports running + encrypted virtual machine (VMs) under the control of KVM. Encrypted + VMs have their pages (code and data) secured such that only the guest + itself has access to the unencrypted version. Each encrypted VM is + associated with a unique encryption key; if its data is accessed to a + different entity using a different key the encrypted guests data will + be incorrectly decrypted, leading to unintelligible data. + + For more information see various input parameters and its format see SEV API spec + <a href="https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf"> https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf </a> + <span class="since">Since 4.2.0</span> + </p> + <pre> +<domain> + ... + <launch-security type='sev'> + <policy> 0 </policy> + <cbitpos> 47 </cbitpos> + <reduced-phys-bits> 5 </reduced-phys-bits> + <session> ... </session> + <dh-cert> ... </dh> + </sev> + ... +</domain> +</pre> + + <p> + A least <code>cbitpos</code> and <code>reduced-phys-bits</code> must be + nested within the <code>launch-security</code> element. + </p> + <dl> + <dt><code>cbitpos</code></dt> + <dd>The <code>cbitpos</code> element provides the C-bit (aka encryption bit) + location in guest page table entry. The value of <code>cbitpos</code> is + hypervisor dependent and can be obtained through the <code>sev</code> element + from domaincapabilities. + </dd> + <dt><code>reduced-phys-bits</code></dt> + <dd>The <code>reduced-phys-bits</code> element provides the physical + address bit reducation. Similar to <code>cbitpos</code> the value of <code> + reduced-phys-bit</code> is hypervisor dependent and can be obtained + through the <code>sev</code> element from domaincapabilities. + </dd> + <dt><code>policy</code></dt> + <dd>The optional <code>policy</code> element provides the guest policy + which must be maintained by the SEV 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 4-byte structure with the fields shown in Table: + + <table class="top_table"> + <tr> + <th> Bit(s) </th> + <th> Description </th> + </tr> + <tr> + <td> 0 </td> + <td> Debugging of the guest is disallowed when set </td> + </tr> + <tr> + <td> 1 </td> + <td> Sharing keys with other guests is disallowed when set </td> + </tr> + <tr> + <td> 2 </td> + <td> SEV-ES is required when set</td> + </tr> + <tr> + <td> 3 </td> + <td> Sending the guest to another platform is disallowed when set</td> + </tr> + <tr> + <td> 4 </td> + <td> The guest must not be transmitted to another platform that is + not in the domain when set. </td> + </tr> + <tr> + <td> 5 </td> + <td> The guest must not be transmitted to another platform that is + not SEV capable when set. </td> + </tr> + <tr> + <td> 15:6 </td> + <td> reserved </td> + </tr> + <tr> + <td> 16:32 </td> + <td> The guest must not be transmitted to another platform with a + lower firmware version. </td> + </tr> + </table> + Default value is 0x1 + + </dd> + <dt><code>dh-cert</code></dt> + <dd>The optional <code>dh-cert</code> element provides the guest owners public + Diffie-Hellman (DH) key. The key is used to negotiate a master secret + key between the SEV firmware and guest owner. This master secret key is + then used to establish a trusted channel between SEV firmware and guest + owner. The value must be encoded in base64. + </dd> + <dt><code>session</code></dt> + <dd>The optional <code>session</code> element provides the guest owners + session blob defined in SEV API spec. The value must be encoded in base64. + + See SEV spec LAUNCH_START section for session blob format. + </dd> + </dl> + <h2><a id="examples">Example configs</a></h2> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 8165e699d67e..e3dcc69067c2 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -77,6 +77,9 @@ <optional> <ref name='keywrap'/> </optional> + <optional> + <ref name='launch-security'/> + </optional> </interleave> </element> </define> @@ -436,6 +439,42 @@ </element> </define> + <define name="launch-security"> + <element name="launch-security"> + <attribute name="type"> + <value>sev</value> + </attribute> + <interleave> + <element name="cbitpos"> + <data type='unsignedInt'/> + </element> + <element name="reduced-phys-bits"> + <data type='unsignedInt'/> + </element> + <optional> + <element name="policy"> + <ref name='hexuint'/> + </element> + </optional> + <optional> + <element name="handle"> + <ref name='unsignedInt'/> + </element> + </optional> + <optional> + <element name="dh-cert"> + <data type="string"/> + </element> + </optional> + <optional> + <element name="session"> + <data type="string"/> + </element> + </optional> + </interleave> + </element> + </define> + <!-- Enable or disable perf events for the domain. For each of the events the following rules apply: diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 04a6ee77af6f..c18532ca6f74 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -929,6 +929,10 @@ VIR_ENUM_IMPL(virDomainShmemModel, VIR_DOMAIN_SHMEM_MODEL_LAST, "ivshmem-plain", "ivshmem-doorbell") +VIR_ENUM_IMPL(virDomainLaunchSecurity, VIR_DOMAIN_LAUNCH_SECURITY_LAST, + "", + "sev") + static virClassPtr virDomainObjClass; static virClassPtr virDomainXMLOptionClass; static void virDomainObjDispose(void *obj); @@ -2897,6 +2901,14 @@ virDomainCachetuneDefFree(virDomainCachetuneDefPtr cachetune) VIR_FREE(cachetune); } +static void +virDomainSevDefFree(virDomainSevDefPtr def) +{ + VIR_FREE(def->dh_cert); + VIR_FREE(def->session); + + VIR_FREE(def); +} void virDomainDefFree(virDomainDefPtr def) { @@ -3079,6 +3091,9 @@ void virDomainDefFree(virDomainDefPtr def) if (def->namespaceData && def->ns.free) (def->ns.free)(def->namespaceData); + if (def->sev) + virDomainSevDefFree(def->sev); + xmlFreeNode(def->metadata); VIR_FREE(def); @@ -15500,6 +15515,70 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node, return ret; } +static virDomainSevDefPtr +virDomainSevDefParseXML(xmlNodePtr sevNode, xmlXPathContextPtr ctxt) +{ + char *tmp = NULL, *type = NULL; + xmlNodePtr save = ctxt->node; + virDomainSevDefPtr def; + unsigned long policy; + + ctxt->node = sevNode; + + if (VIR_ALLOC(def) < 0) + return NULL; + + if (!(type = virXMLPropString(sevNode, "type"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing launch-security type")); + goto error; + } + + if (virDomainLaunchSecurityTypeFromString(type) != + VIR_DOMAIN_LAUNCH_SECURITY_SEV) { + goto error; + } + + if (virXPathUInt("string(./cbitpos)", ctxt, &def->cbitpos) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", _("failed to get cbitpos")); + goto error; + } + + if (virXPathUInt("string(./reduced-phys-bits)", ctxt, + &def->reduced_phys_bits) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("failed to get reduced-phys-bits")); + goto error; + } + + if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) + policy = 0x1; + + def->policy = policy; + + if ((tmp = virXPathString("string(./dh-cert)", ctxt))) { + if (VIR_STRDUP(def->dh_cert, tmp) < 0) + goto error; + + VIR_FREE(tmp); + } + + if ((tmp = virXPathString("string(./session)", ctxt))) { + if (VIR_STRDUP(def->session, tmp) < 0) + goto error; + + VIR_FREE(tmp); + } + + ctxt->node = save; + return def; + + error: + VIR_FREE(tmp); + virDomainSevDefFree(def); + ctxt->node = save; + return NULL; +} static virDomainMemoryDefPtr virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt, @@ -20173,6 +20252,13 @@ virDomainDefParseXML(xmlDocPtr xml, ctxt->node = node; VIR_FREE(nodes); + /* Check for SEV feature */ + if ((node = virXPathNode("./launch-security", ctxt)) != NULL) { + def->sev = virDomainSevDefParseXML(node, ctxt); + if (!def->sev) + goto error; + } + /* analysis of memory devices */ if ((n = virXPathNodeSet("./devices/memory", ctxt, &nodes)) < 0) goto error; @@ -26063,6 +26149,27 @@ virDomainKeyWrapDefFormat(virBufferPtr buf, virDomainKeyWrapDefPtr keywrap) virBufferAddLit(buf, "</keywrap>\n"); } +static void +virDomainSevDefFormat(virBufferPtr buf, virDomainSevDefPtr sev) +{ + virBufferAddLit(buf, "<launch-security type='sev'>\n"); + virBufferAdjustIndent(buf, 2); + + virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos); + virBufferAsprintf(buf, "<reduced-phys-bits>%d</reduced-phys-bits>\n", + sev->reduced_phys_bits); + virBufferAsprintf(buf, "<policy>%d</policy>\n", sev->policy); + if (sev->dh_cert) + virBufferAsprintf(buf, "<dh_cert>%s</dh_cert>\n", sev->dh_cert); + + if (sev->session) + virBufferAsprintf(buf, "<session>%s</session>\n", sev->session); + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</launch-security>\n"); +} + + static void virDomainPerfDefFormat(virBufferPtr buf, virDomainPerfDefPtr perf) { @@ -27235,6 +27342,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->keywrap) virDomainKeyWrapDefFormat(buf, def->keywrap); + if (def->sev) + virDomainSevDefFormat(buf, def->sev); + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</domain>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 337ce79425a7..16f997071de9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -142,6 +142,9 @@ typedef virDomainPanicDef *virDomainPanicDefPtr; typedef struct _virDomainMemoryDef virDomainMemoryDef; typedef virDomainMemoryDef *virDomainMemoryDefPtr; +typedef struct _virDomainSevDef virDomainSevDef; +typedef virDomainSevDef *virDomainSevDefPtr; + /* forward declarations virDomainChrSourceDef, required by * virDomainNetDef */ @@ -2289,6 +2292,25 @@ struct _virDomainKeyWrapDef { int dea; /* enum virTristateSwitch */ }; +typedef enum { + VIR_DOMAIN_LAUNCH_SECURITY_NONE, + VIR_DOMAIN_LAUNCH_SECURITY_SEV, + + VIR_DOMAIN_LAUNCH_SECURITY_LAST, +} virDomainLaunchSecurity; + +typedef struct _virDomainSevDef virDomainSevDef; +typedef virDomainSevDef *virDomainSevDefPtr; + +struct _virDomainSevDef { + char *dh_cert; + char *session; + unsigned int policy; + unsigned int cbitpos; + unsigned int reduced_phys_bits; +}; + + typedef enum { VIR_DOMAIN_IOMMU_MODEL_INTEL, @@ -2454,6 +2476,9 @@ struct _virDomainDef { virDomainKeyWrapDefPtr keywrap; + /* SEV-specific domain */ + virDomainSevDefPtr sev; + /* Application-specific custom metadata */ xmlNodePtr metadata; @@ -3345,6 +3370,7 @@ VIR_ENUM_DECL(virDomainMemorySource) VIR_ENUM_DECL(virDomainMemoryAllocation) VIR_ENUM_DECL(virDomainIOMMUModel) VIR_ENUM_DECL(virDomainShmemModel) +VIR_ENUM_DECL(virDomainLaunchSecurity) /* from libvirt.h */ VIR_ENUM_DECL(virDomainState) VIR_ENUM_DECL(virDomainNostateReason) -- 2.14.3
QEMU >= 2.12 provides 'sev-guest' object which is used to launch encrypted VMs on AMD platform using SEV feature. The various inputs required to launch SEV guest is provided through the <launch-security> tag. A typical SEV guest launch command line looks like this: # $QEMU ...\ -object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=5 ...\ -machine memory-encryption=sev0 \ Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- src/qemu/qemu_command.c | 35 +++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fa0aa5d5c3d4..7e0f515d0d7e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7354,6 +7354,9 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, virQEMUCapsGet(qemuCaps, QEMU_CAPS_LOADPARM)) qemuAppendLoadparmMachineParm(&buf, def); + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV) && def->sev) + virBufferAddLit(&buf, ",memory-encryption=sev0"); + virCommandAddArgBuffer(cmd, &buf); } @@ -9663,6 +9666,35 @@ qemuBuildTPMCommandLine(virCommandPtr cmd, return 0; } +static void +qemuBuildSevCommandLine(virDomainObjPtr vm, virCommandPtr cmd, + virDomainSevDefPtr sev) +{ + virBuffer obj = VIR_BUFFER_INITIALIZER; + qemuDomainObjPrivatePtr priv = vm->privateData; + char *path = NULL; + + VIR_DEBUG("policy=0x%x cbitpos=%d reduced_phys_bits=%d", + sev->policy, sev->cbitpos, sev->reduced_phys_bits); + + virBufferAsprintf(&obj, "sev-guest,id=sev0,cbitpos=%d", sev->cbitpos); + virBufferAsprintf(&obj, ",reduced-phys-bits=%d", sev->reduced_phys_bits); + virBufferAsprintf(&obj, ",policy=0x%x", sev->policy); + + if (sev->dh_cert) { + ignore_value(virAsprintf(&path, "%s/dh_cert.base64", priv->libDir)); + virBufferAsprintf(&obj, ",dh-cert-file=%s", path); + VIR_FREE(path); + } + + if (sev->session) { + ignore_value(virAsprintf(&path, "%s/session.base64", priv->libDir)); + virBufferAsprintf(&obj, ",session-file=%s", path); + VIR_FREE(path); + } + + virCommandAddArgList(cmd, "-object", virBufferContentAndReset(&obj), NULL); +} static int qemuBuildVMCoreInfoCommandLine(virCommandPtr cmd, @@ -10108,6 +10140,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildVMCoreInfoCommandLine(cmd, def, qemuCaps) < 0) goto error; + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV) && def->sev) + qemuBuildSevCommandLine(vm, cmd, def->sev); + if (snapshot) virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 57c06c7c1550..5c102de03582 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5741,6 +5741,61 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver, return ret; } +static int +qemuBuildSevCreateFile(const char *configDir, const char *name, + const char *data) +{ + char *configFile; + + if (!(configFile = virFileBuildPath(configDir, name, ".base64"))) + return -1; + + if (virFileRewriteStr(configFile, S_IRUSR | S_IWUSR, data) < 0) { + virReportSystemError(errno, _("failed to write data to config '%s'"), + configFile); + goto error; + } + + VIR_FREE(configFile); + return 0; + + error: + VIR_FREE(configFile); + return -1; +} + +static int +qemuProcessPrepareSevGuestInput(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDefPtr def = vm->def; + virQEMUCapsPtr qemuCaps = priv->qemuCaps; + virDomainSevDefPtr sev = def->sev; + + if (!sev) + return 0; + + VIR_DEBUG("Prepare SEV guest"); + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Domain %s asked for 'sev' launch but " + "QEMU does not support SEV feature"), vm->def->name); + return -1; + } + + if (sev->dh_cert) { + if (qemuBuildSevCreateFile(priv->libDir, "dh_cert", sev->dh_cert) < 0) + return -1; + } + + if (sev->session) { + if (qemuBuildSevCreateFile(priv->libDir, "session", sev->session) < 0) + return -1; + } + + return 0; +} static int qemuProcessPrepareHostStorage(virQEMUDriverPtr driver, @@ -5866,6 +5921,9 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver, if (qemuProcessPrepareHostStorage(driver, vm, flags) < 0) goto cleanup; + if (qemuProcessPrepareSevGuestInput(vm) < 0) + goto cleanup; + ret = 0; cleanup: virObjectUnref(cfg); -- 2.14.3
The API can be used outside the libvirt to get the launch security information. When SEV is enabled, the API can be used to get the measurement of the launch process. Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- include/libvirt/libvirt-domain.h | 17 ++++++++++++++ src/driver-hypervisor.h | 7 ++++++ src/libvirt-domain.c | 48 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++++ 4 files changed, 77 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 4128d5585255..733c4109549c 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -4763,4 +4763,21 @@ int virDomainSetLifecycleAction(virDomainPtr domain, unsigned int action, unsigned int flags); +/** + * Launch Security API + */ + +/** + * VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT: + * + * Macro represents the launch measurement of the SEV guest, + * as VIR_TYPED_PARAM_STRING. + */ +#define VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT "sev-measurement" + +int virDomainGetLaunchSecurityInfo(virDomainPtr domain, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags); + #endif /* __VIR_LIBVIRT_DOMAIN_H__ */ diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index ce0e2b252552..b06641394db9 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1283,6 +1283,12 @@ typedef int unsigned int action, unsigned int flags); +typedef int +(*virDrvDomainGetLaunchSecurityInfo)(virDomainPtr domain, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags); + typedef struct _virHypervisorDriver virHypervisorDriver; typedef virHypervisorDriver *virHypervisorDriverPtr; @@ -1528,6 +1534,7 @@ struct _virHypervisorDriver { virDrvDomainSetVcpu domainSetVcpu; virDrvDomainSetBlockThreshold domainSetBlockThreshold; virDrvDomainSetLifecycleAction domainSetLifecycleAction; + virDrvDomainGetLaunchSecurityInfo domainGetLaunchSecurityInfo; }; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index eaec0979ad49..71a43b0c2c75 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12095,3 +12095,51 @@ int virDomainSetLifecycleAction(virDomainPtr domain, virDispatchError(domain->conn); return -1; } + +/** + * virDomainGetLaunchSecurityInfo: + * @domain: a domain object + * @params: where to store security info + * @nparams: number of items in @params + * @flags: currently used, set to 0. + * + * Get the launch security info. In case of the SEV guest, this will + * return the launch measurement. + * + * Returns -1 in case of failure, 0 in case of success. + */ +int virDomainGetLaunchSecurityInfo(virDomainPtr domain, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + virConnectPtr conn = domain->conn; + + VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%p flags=0x%x", + params, nparams, flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + virCheckNonNullArgGoto(params, error); + virCheckNonNullArgGoto(nparams, error); + virCheckReadOnlyGoto(conn->flags, error); + + if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, + VIR_DRV_FEATURE_TYPED_PARAM_STRING)) + flags |= VIR_TYPED_PARAM_STRING_OKAY; + + if (conn->driver->domainGetLaunchSecurityInfo) { + int ret; + ret = conn->driver->domainGetLaunchSecurityInfo(domain, params, + nparams, flags); + if (ret < 0) + goto error; + return ret; + } + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 95df3a0dbc7b..caba2862d371 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -785,4 +785,9 @@ LIBVIRT_4.1.0 { virStoragePoolLookupByTargetPath; } LIBVIRT_3.9.0; +LIBVIRT_4.2.0 { + global: + virDomainGetLaunchSecurityInfo; +} LIBVIRT_4.1.0; + # .... define new API here using predicted next version number .... -- 2.14.3
Add remote support for launch security info. Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- src/remote/remote_daemon_dispatch.c | 47 +++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 42 ++++++++++++++++++++++++++++++++- src/remote/remote_protocol.x | 20 +++++++++++++++- src/remote_protocol-structs | 11 +++++++++ 4 files changed, 118 insertions(+), 2 deletions(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index ea67cb7bc018..b22ed2222a6b 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -3087,6 +3087,53 @@ remoteDispatchNodeGetMemoryStats(virNetServerPtr server ATTRIBUTE_UNUSED, return rv; } +static int +remoteDispatchDomainGetLaunchSecurityInfo(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_launch_security_info_args *args, + remote_domain_get_launch_security_info_ret *ret) +{ + virDomainPtr dom = NULL; + virTypedParameterPtr params = NULL; + int nparams = 0; + int rv = -1; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + if (virDomainGetLaunchSecurityInfo(dom, ¶ms, &nparams, args->flags) < 0) + goto cleanup; + + if (nparams > REMOTE_DOMAIN_LAUNCH_SECURITY_INFO_PARAMS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); + goto cleanup; + } + + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &ret->params.params_val, + &ret->params.params_len, + args->flags) < 0) + goto cleanup; + + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virTypedParamsFree(params, nparams); + virObjectUnref(dom); + return rv; +} + static int remoteDispatchDomainGetPerfEvents(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 93cba5daa3e8..ea974e77490f 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1951,6 +1951,45 @@ remoteDomainGetNumaParameters(virDomainPtr domain, return rv; } +static int +remoteDomainGetLaunchSecurityInfo(virDomainPtr domain, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + int rv = -1; + remote_domain_get_launch_security_info_args args; + remote_domain_get_launch_security_info_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, domain); + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_LAUNCH_SECURITY_INFO, + (xdrproc_t) xdr_remote_domain_get_launch_security_info_args, (char *) &args, + (xdrproc_t) xdr_remote_domain_get_launch_security_info_ret, (char *) &ret) == -1) + goto done; + + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_LAUNCH_SECURITY_INFO_PARAMS_MAX, + params, + nparams) < 0) + goto cleanup; + + rv = 0; + + cleanup: + xdr_free((xdrproc_t) xdr_remote_domain_get_launch_security_info_ret, + (char *) &ret); + done: + remoteDriverUnlock(priv); + return rv; +} + static int remoteDomainGetPerfEvents(virDomainPtr domain, virTypedParameterPtr *params, @@ -8497,7 +8536,8 @@ static virHypervisorDriver hypervisor_driver = { .domainSetGuestVcpus = remoteDomainSetGuestVcpus, /* 2.0.0 */ .domainSetVcpu = remoteDomainSetVcpu, /* 3.1.0 */ .domainSetBlockThreshold = remoteDomainSetBlockThreshold, /* 3.2.0 */ - .domainSetLifecycleAction = remoteDomainSetLifecycleAction /* 3.9.0 */ + .domainSetLifecycleAction = remoteDomainSetLifecycleAction, /* 3.9.0 */ + .domainGetLaunchSecurityInfo = remoteDomainGetLaunchSecurityInfo /* 4.2.0 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 9dbd497b2fff..4c0144c52d50 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -253,6 +253,9 @@ const REMOTE_DOMAIN_IP_ADDR_MAX = 2048; /* Upper limit on number of guest vcpu information entries */ const REMOTE_DOMAIN_GUEST_VCPU_PARAMS_MAX = 64; +/* Upper limit on number of launch security information entries */ +const REMOTE_DOMAIN_LAUNCH_SECURITY_INFO_PARAMS_MAX = 64; + /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ typedef opaque remote_uuid[VIR_UUID_BUFLEN]; @@ -3448,6 +3451,15 @@ struct remote_domain_set_lifecycle_action_args { unsigned int flags; }; +struct remote_domain_get_launch_security_info_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + +struct remote_domain_get_launch_security_info_ret { + remote_typed_param params<REMOTE_DOMAIN_LAUNCH_SECURITY_INFO_PARAMS_MAX>; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -6135,5 +6147,11 @@ enum remote_procedure { * @priority: high * @acl: storage_pool:getattr */ - REMOTE_PROC_STORAGE_POOL_LOOKUP_BY_TARGET_PATH = 391 + REMOTE_PROC_STORAGE_POOL_LOOKUP_BY_TARGET_PATH = 391, + + /** + * @generate: none + * @acl: domain:read + */ + REMOTE_PROC_DOMAIN_GET_LAUNCH_SECURITY_INFO = 392 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index f45aba27a202..8f19d98d7ad9 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2877,6 +2877,16 @@ struct remote_domain_set_lifecycle_action_args { u_int action; u_int flags; }; +struct remote_domain_get_launch_security_info_args { + remote_nonnull_domain dom; + u_int flags; +}; +struct remote_domain_get_launch_security_info_ret { + struct { + u_int params_len; + remote_typed_param * params_val; + } params; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -3269,4 +3279,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_MANAGED_SAVE_DEFINE_XML = 389, REMOTE_PROC_DOMAIN_SET_LIFECYCLE_ACTION = 390, REMOTE_PROC_STORAGE_POOL_LOOKUP_BY_TARGET_PATH = 391, + REMOTE_PROC_DOMAIN_GET_LAUNCH_SECURITY_INFO = 392, }; -- 2.14.3
This patch implement the internal driver API for launch event into qemu driver. When SEV is enabled, execute 'query-sev-launch-measurement' to get the measurement of memory encrypted through launch sequence. Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- src/qemu/qemu_driver.c | 66 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 8 ++++++ src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 32 +++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 2 ++ 5 files changed, 111 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8c872c1f08d3..2f38f0366dde 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21254,6 +21254,71 @@ qemuDomainSetLifecycleAction(virDomainPtr dom, return ret; } +static int qemuDomainGetSevMeasurement(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + int ret = -1; + char *tmp; + int maxpar = 0; + + virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + return -1; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) + goto endjob; + + tmp = qemuMonitorGetSevMeasurement(QEMU_DOMAIN_PRIVATE(vm)->mon); + if (tmp == NULL) + goto endjob; + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto endjob; + + if (virTypedParamsAddString(params, nparams, &maxpar, + VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT, + tmp) < 0) + goto endjob; + + ret = 0; + + endjob: + qemuDomainObjEndJob(driver, vm); + return ret; +} + + +static int +qemuDomainGetLaunchSecurityInfo(virDomainPtr domain, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + virQEMUDriverPtr driver = domain->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + + if (!(vm = qemuDomObjFromDomain(domain))) + goto cleanup; + + if (virDomainGetLaunchSecurityInfoEnsureACL(domain->conn, vm->def) < 0) + goto cleanup; + + if (vm->def->sev) { + if (qemuDomainGetSevMeasurement(driver, vm, params, nparams, flags) < 0) + goto cleanup; + } + + ret = 0; + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} static virHypervisorDriver qemuHypervisorDriver = { .name = QEMU_DRIVER_NAME, @@ -21474,6 +21539,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainSetVcpu = qemuDomainSetVcpu, /* 3.1.0 */ .domainSetBlockThreshold = qemuDomainSetBlockThreshold, /* 3.2.0 */ .domainSetLifecycleAction = qemuDomainSetLifecycleAction, /* 3.9.0 */ + .domainGetLaunchSecurityInfo = qemuDomainGetLaunchSecurityInfo, /* 4.2.0 */ }; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2820714b5c55..b0a4e0ea75ad 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4417,3 +4417,11 @@ qemuMonitorSetWatchdogAction(qemuMonitorPtr mon, return qemuMonitorJSONSetWatchdogAction(mon, action); } + +char * +qemuMonitorGetSevMeasurement(qemuMonitorPtr mon) +{ + QEMU_CHECK_MONITOR_NULL(mon); + + return qemuMonitorJSONGetSevMeasurement(mon); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index aaa14f66fdfb..6a4f28796767 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1188,4 +1188,7 @@ virJSONValuePtr qemuMonitorQueryNamedBlockNodes(qemuMonitorPtr mon); int qemuMonitorSetWatchdogAction(qemuMonitorPtr mon, const char *action); +char * +qemuMonitorGetSevMeasurement(qemuMonitorPtr mon); + #endif /* QEMU_MONITOR_H */ diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index c51b98d2bda7..805c2f3b81e3 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -7991,3 +7991,35 @@ qemuMonitorJSONSetWatchdogAction(qemuMonitorPtr mon, virJSONValueFree(reply); return ret; } + +char * +qemuMonitorJSONGetSevMeasurement(qemuMonitorPtr mon) +{ + const char *tmp; + char *measurement = NULL; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-sev-launch-measure", NULL))) + return NULL; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + data = virJSONValueObjectGetObject(reply, "return"); + + if (!(tmp = virJSONValueObjectGetString(data, "data"))) + goto cleanup; + + if (VIR_STRDUP(measurement, tmp) < 0) + goto cleanup; + + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return measurement; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 305f789902e9..b83160a20e00 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -342,6 +342,8 @@ int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, int qemuMonitorJSONSystemWakeup(qemuMonitorPtr mon); +char *qemuMonitorJSONGetSevMeasurement(qemuMonitorPtr mon); + int qemuMonitorJSONGetVersion(qemuMonitorPtr mon, int *major, int *minor, -- 2.14.3
Add new 'launch-security' command, the command can be used to get or set the launch security information when booting encrypted VMs. Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- tools/virsh-domain.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 2b775fc4cca6..4dca19139f88 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -13877,6 +13877,84 @@ cmdDomFSInfo(vshControl *ctl, const vshCmd *cmd) return ret >= 0; } +/* + * "launch-security" command + */ +static const vshCmdInfo info_launch_security[] = { + {.name = "help", + .data = N_("Get or set launch-security information") + }, + {.name = "desc", + .data = N_("Get or set the current launch-security information for a guest" + " domain.\n" + " To get the launch-security information use following command: \n\n" + " virsh # launch-security <domain>") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_launch_security[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(0), + {.name = "get", + .type = VSH_OT_STRING, + .help = N_("Show the launch-security info") + }, + VIRSH_COMMON_OPT_DOMAIN_CONFIG, + VIRSH_COMMON_OPT_DOMAIN_LIVE, + VIRSH_COMMON_OPT_DOMAIN_CURRENT, + {.name = NULL} +}; + +static void +virshPrintLaunchSecurityInfo(vshControl *ctl, virTypedParameterPtr params, + int nparams) +{ + size_t i; + + for (i = 0; i < nparams; i++) { + if (params[i].type == VIR_TYPED_PARAM_STRING) + vshPrintExtra(ctl, "%-15s: %s\n", params[i].field, params[i].value.s); + } +} + +static bool +cmdLaunchSecurity(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + int nparams = 0; + virTypedParameterPtr params = NULL; + bool ret = false; + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + bool current = vshCommandOptBool(cmd, "current"); + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + + VSH_EXCLUSIVE_OPTIONS_VAR(current, live); + VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (virDomainGetLaunchSecurityInfo(dom, ¶ms, &nparams, flags) != 0) { + vshError(ctl, "%s", _("Unable to get launch security info")); + goto cleanup; + } + + virshPrintLaunchSecurityInfo(ctl, params, nparams); + + ret = true; + cleanup: + virTypedParamsFree(params, nparams); + virshDomainFree(dom); + return ret; +} + + const vshCmdDef domManagementCmds[] = { {.name = "attach-device", .handler = cmdAttachDevice, @@ -14492,5 +14570,11 @@ const vshCmdDef domManagementCmds[] = { .info = info_domblkthreshold, .flags = 0 }, + {.name = "launch-security", + .handler = cmdLaunchSecurity, + .opts = opts_launch_security, + .info = info_launch_security, + .flags = 0 + }, {.name = NULL} }; -- 2.14.3
From: Xiaogang Chen <Xiaogang.Chen@amd.com> Update qemuxml2xmltest, genericxml2xmltest and qemuxml2argvtest to include sev specific tag, a typical SEV specific tag looks like <launch-security type='sev> <cbitpos>47</cbitpos> <reduced-phys-bits>1</reduced-phys-bits> <policy>1</policy> </launch-security> Signed-off-by: Xiaogang Chen <Xiaogang.Chen@amd.com> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- tests/genericxml2xmlindata/sev.xml | 20 +++++++++++++++++++ tests/genericxml2xmloutdata/sev.xml | 22 +++++++++++++++++++++ tests/genericxml2xmltest.c | 2 ++ tests/qemuxml2argvdata/sev.args | 24 +++++++++++++++++++++++ tests/qemuxml2argvdata/sev.xml | 35 +++++++++++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ tests/qemuxml2xmloutdata/sev.xml | 39 +++++++++++++++++++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 8 files changed, 146 insertions(+) create mode 100644 tests/genericxml2xmlindata/sev.xml create mode 100644 tests/genericxml2xmloutdata/sev.xml create mode 100644 tests/qemuxml2argvdata/sev.args create mode 100644 tests/qemuxml2argvdata/sev.xml create mode 100644 tests/qemuxml2xmloutdata/sev.xml diff --git a/tests/genericxml2xmlindata/sev.xml b/tests/genericxml2xmlindata/sev.xml new file mode 100644 index 000000000000..aeb0c6a0a709 --- /dev/null +++ b/tests/genericxml2xmlindata/sev.xml @@ -0,0 +1,20 @@ +<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-1.0'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <launch-security type='sev'> + <cbitpos>47</cbitpos> + <reduced-phys-bits>1</reduced-phys-bits> + <policy>1</policy> + </launch-security> +</domain> diff --git a/tests/genericxml2xmloutdata/sev.xml b/tests/genericxml2xmloutdata/sev.xml new file mode 100644 index 000000000000..70065b861861 --- /dev/null +++ b/tests/genericxml2xmloutdata/sev.xml @@ -0,0 +1,22 @@ +<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-1.0'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + </devices> + <launch-security type='sev'> + <cbitpos>47</cbitpos> + <reduced-phys-bits>1</reduced-phys-bits> + <policy>1</policy> + </launch-security> +</domain> diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index d8270a6cae82..3b75b43feda3 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -141,6 +141,8 @@ mymain(void) DO_TEST_FULL("cachetune-colliding-types", false, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_DIFFERENT("sev"); + virObjectUnref(caps); virObjectUnref(xmlopt); diff --git a/tests/qemuxml2argvdata/sev.args b/tests/qemuxml2argvdata/sev.args new file mode 100644 index 000000000000..312dbcfd3c6c --- /dev/null +++ b/tests/qemuxml2argvdata/sev.args @@ -0,0 +1,24 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name QEMUGuest1 \ +-S \ +-machine pc-1.0,accel=kvm,memory-encryption=sev0 \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=1,policy=0x1 diff --git a/tests/qemuxml2argvdata/sev.xml b/tests/qemuxml2argvdata/sev.xml new file mode 100644 index 000000000000..2476b586d72e --- /dev/null +++ b/tests/qemuxml2argvdata/sev.xml @@ -0,0 +1,35 @@ +<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-1.0'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-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'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> + <launch-security type='sev'> + <cbitpos>47</cbitpos> + <reduced-phys-bits>1</reduced-phys-bits> + <policy>1</policy> + </launch-security> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 731db9ed5221..5d43457730cc 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -3004,6 +3004,8 @@ mymain(void) DO_TEST("disk-many-format-probing", QEMU_CAPS_DRIVE_BOOT); driver.config->allowDiskFormatProbing = false; + DO_TEST("sev", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_KVM, QEMU_CAPS_SEV); + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakerootdir); diff --git a/tests/qemuxml2xmloutdata/sev.xml b/tests/qemuxml2xmloutdata/sev.xml new file mode 100644 index 000000000000..80017fe07691 --- /dev/null +++ b/tests/qemuxml2xmloutdata/sev.xml @@ -0,0 +1,39 @@ +<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-1.0'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-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'> + <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'/> + <memballoon model='none'/> + </devices> + <launch-security type='sev'> + <cbitpos>47</cbitpos> + <reduced-phys-bits>1</reduced-phys-bits> + <policy>1</policy> + </launch-security> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 28ba46efb2bc..1c2dcea7cf90 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1243,6 +1243,8 @@ mymain(void) DO_TEST_STATUS("modern"); DO_TEST_STATUS("migration-out-nbd"); + DO_TEST("sev", NONE); + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakerootdir); -- 2.14.3
Hi Daniel and Peter. Any feedback on the series ? -Brijesh On 3/14/18 10:44 AM, Brijesh Singh wrote:
The patch series is test with QEMU recent pull which includes SEV support:
https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg03826.html
This patch series provides support for launching an encrypted guest using AMD's new Secure Encrypted Virtualization (SEV) feature.
SEV is an extension to the AMD-V architecture which supports running multiple VMs under the control of a hypervisor. When enabled, SEV feature allows the memory contents of a virtual machine (VM) to be transparently encrypted with a key unique to the guest VM.
At very high level the flow looks this:
1. mgmt tool calls virConnectGetDomainCapabilities. This returns an XML document that includes the following
<feature> ... <sev supported='yes'> <cbitpos> </cbitpos> <reduced-phys-bits> </reduced-phys-bits> <pdh> </pdh> <cert-chain> </cert-chain> </feature>
If <sev> is provided then we indicate that hypervisor is capable of launching SEV guest.
2. (optional) mgmt tool can provide the PDH and Cert-chain to guest owner in case if guest owner wish to establish a secure connection with SEV firmware to negotiate a key used for validating the measurement.
3. mgmt tool requests to start a guest calling virCreateXML(), passing VIR_DOMAIN_START_PAUSED. The xml would include
<launch-security type='sev'> <cbitpos> </cbitpos> /* the value is same as what is obtained via virConnectGetDomainCapabilities() <reduced-phys-bits> </reduced-phys-bits> /* the value is same as what is obtained via virConnectGetDomainCapabilities() <dh-cert> .. </dh> /* guest owners diffie-hellman key */ (optional) <session> ..</session> /* guest owners session blob */ (optional) <policy> ..</policy> /* guest policy */ (optional)
4. Libvirt generate the QEMU cli arg to enable the SEV feature, a typical args looks like this:
# $QEMU .. -machine memory-encryption=sev0 \ -object sev-guest,id=sev0,dh-cert-file=<file>....
5. Libvirt generates lifecycle VIR_DOMAIN_EVENT_SUSPENDED_PAUSED event
6. mgmt tool gets the VIR_DOMAIN_EVENT_SUSPENDED_PAUSED and calls virDomainGetLaunchSecretInfo() to retrieve the measurement of encrypted memory.
7. (optional) mgmt tool can provide the measurement value to guest owner, which can validate the measurement and gives GO/NO-GO answer. If mgmt tool gets GO then it resumes the guest otherwise it calls destroy() to kill the guest.
8. mgmt tool resumes the guest
TODO: * SEV guest require to use DMA apis for the virtio devices. In order to use the DMA apis the virtio devices must have this tag
<driver iommu=on ats=on>
It is a bit unclear to me where these changes need to go. Do we need to modify the libvirt to automatically add these when SEV is enabled or we ask mgmt tool to make sure that it creates XML with right tag to enable the DMA APIs for virtio devices. I am looking for some suggestions.
Using these patches we have succesfully booted and tested a guest both with and without SEV enabled.
SEV Firmware API spec is available at: https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf
Changes since v2: * make cbitpos, policy and reduced-phys-bits as unsigned int * update virDomainGetLaunchSecurityInfo to accept virTypedParameterPtr *params instead of virTypedParameterPtr params.
Changes since v1: * rename <sev> -> <launch-security> for domain * add more information about policy and other fields in domaincaps.html * split the domain_conf support in two patches * add virDomainGetLaunchInfo() to retrieve the SEV measurement * extend virsh command to show the domain's launch security information * add test cases to validate newly added <launch-security> element * fix issues reported with 'make check' and 'make syntax-check'
The complete git tree is available at: https://github.com/codomania/libvirt/tree/v3
Brijesh Singh (8): qemu: provide support to query the SEV capability qemu: introduce SEV feature in hypervisor capabilities conf: introduce launch-security element in domain qemu: add support to launch SEV guest libvirt: add new public API to get launch security info remote: implement the remote protocol for launch security qemu_driver: add support to launch security info virsh: implement new command for launch security
Xiaogang Chen (1): tests: extend tests to include sev specific tag parsing
docs/formatdomain.html.in | 120 ++++++++++++++++++++++++++++++++++++ docs/formatdomaincaps.html.in | 40 ++++++++++++ docs/schemas/domaincaps.rng | 20 ++++++ docs/schemas/domaincommon.rng | 39 ++++++++++++ include/libvirt/libvirt-domain.h | 17 +++++ src/conf/domain_capabilities.c | 20 ++++++ src/conf/domain_capabilities.h | 14 +++++ src/conf/domain_conf.c | 110 +++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 26 ++++++++ src/driver-hypervisor.h | 7 +++ src/libvirt-domain.c | 48 +++++++++++++++ src/libvirt_public.syms | 5 ++ src/qemu/qemu_capabilities.c | 40 ++++++++++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_capspriv.h | 4 ++ src/qemu/qemu_command.c | 35 +++++++++++ src/qemu/qemu_driver.c | 66 ++++++++++++++++++++ src/qemu/qemu_monitor.c | 17 +++++ src/qemu/qemu_monitor.h | 6 ++ src/qemu/qemu_monitor_json.c | 105 +++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 5 ++ src/qemu/qemu_process.c | 58 +++++++++++++++++ src/remote/remote_daemon_dispatch.c | 47 ++++++++++++++ src/remote/remote_driver.c | 42 ++++++++++++- src/remote/remote_protocol.x | 20 +++++- src/remote_protocol-structs | 11 ++++ tests/genericxml2xmlindata/sev.xml | 20 ++++++ tests/genericxml2xmloutdata/sev.xml | 22 +++++++ tests/genericxml2xmltest.c | 2 + tests/qemuxml2argvdata/sev.args | 24 ++++++++ tests/qemuxml2argvdata/sev.xml | 35 +++++++++++ tests/qemuxml2argvtest.c | 2 + tests/qemuxml2xmloutdata/sev.xml | 39 ++++++++++++ tests/qemuxml2xmltest.c | 2 + tools/virsh-domain.c | 84 +++++++++++++++++++++++++ 35 files changed, 1151 insertions(+), 2 deletions(-) create mode 100644 tests/genericxml2xmlindata/sev.xml create mode 100644 tests/genericxml2xmloutdata/sev.xml create mode 100644 tests/qemuxml2argvdata/sev.args create mode 100644 tests/qemuxml2argvdata/sev.xml create mode 100644 tests/qemuxml2xmloutdata/sev.xml
participants (3)
-
Andrea Bolognani -
Brijesh Singh -
John Ferlan