[libvirt] [PATCH 1/4] qemu: provide support to query the SEV capability
Brijesh Singh
brijesh.singh at amd.com
Tue Feb 27 15:47:25 UTC 2018
On 02/27/2018 02:09 AM, Peter Krempa wrote:
> On Mon, Feb 26, 2018 at 11:53:33 -0600, 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.
>>
>> Signed-off-by: Brijesh Singh <brijesh.singh at amd.com>
>> ---
>>
>> QEMU SEV v9 patch does not have implementation of query-sev-capabilities command
>> and I am will be adding this command in next QEMU patch round. Command result
>> will look like this:
>>
>> { "execute": "query-sev-capabilities" }
>> { "return": { "sev": 1, "pdh": "....", "cert-chain": "...",
>> "cbitpos": 47, "reduced-phys-bits": 5}}
>
> This patch fails both 'make check' and 'make syntax-check' please make
> sure that for the next posting you send a series where every single
> patch passes both test suites.
Will do thanks.
>
>>
>> src/conf/domain_capabilities.h | 14 +++++++
>> src/qemu/qemu_capabilities.c | 28 +++++++++++++
>> src/qemu/qemu_capspriv.h | 4 ++
>> src/qemu/qemu_monitor.c | 9 +++++
>> src/qemu/qemu_monitor.h | 3 ++
>> src/qemu/qemu_monitor_json.c | 92 ++++++++++++++++++++++++++++++++++++++++++
>> src/qemu/qemu_monitor_json.h | 3 ++
>> 7 files changed, 153 insertions(+)
>>
>> diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
>> index fa4c1e442f57..e13a7fd6ba1b 100644
>> --- a/src/conf/domain_capabilities.h
>> +++ b/src/conf/domain_capabilities.h
>> @@ -137,6 +137,20 @@ struct _virDomainCapsCPU {
>> virDomainCapsCPUModelsPtr custom;
>> };
>>
>> +/*
>> + * SEV capabilities
>> + */
>> +typedef struct _virSEVCapability virSEVCapability;
>> +typedef virSEVCapability *virSEVCapabilityPtr;
>> +struct _virSEVCapability {
>> + bool sev;
>> + char *pdh;
>> + char *cert_chain;
>> + int cbitpos;
>> + int reduced_phys_bits;
>> +};
>> +
>> +
>> struct _virDomainCaps {
>> virObjectLockable parent;
>>
>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>> index b5eb8cf46a52..2c680528deb8 100644
>> --- a/src/qemu/qemu_capabilities.c
>> +++ b/src/qemu/qemu_capabilities.c
>> @@ -525,6 +525,8 @@ struct _virQEMUCaps {
>> size_t ngicCapabilities;
>> virGICCapability *gicCapabilities;
>>
>> + virSEVCapability *sevCapabilities;
>> +
>> virQEMUCapsHostCPUData kvmCPU;
>> virQEMUCapsHostCPUData tcgCPU;
>> };
>> @@ -2811,6 +2813,14 @@ virQEMUCapsSetGICCapabilities(virQEMUCapsPtr qemuCaps,
>> qemuCaps->ngicCapabilities = ncapabilities;
>> }
>>
>> +void
>> +virQEMUCapsSetSEVCapabilities(virQEMUCapsPtr qemuCaps,
>> + virSEVCapability *capabilities)
>> +{
>> + VIR_FREE(qemuCaps->sevCapabilities);
>
> This structure contains also some pointers which would be leaked.
>
Ah good catch, I will fix them in next rev.
>> +
>> + qemuCaps->sevCapabilities = capabilities;
>> +}
>>
>> static int
>> virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps,
>> @@ -3318,6 +3328,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,
>> @@ -4951,6 +4974,11 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
>> virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION))
>> virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_CACHE);
>>
>> + /* SEV capabilities */
>> + if (ARCH_IS_X86(qemuCaps->arch)) {
>> + virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon);
>> + }
>
> I think you can add a capability bit for the presence of the
> query-sev-capabilities command and call this function based on this
> fact.
>
I will explore this option and add new CAP.
>> +
>> ret = 0;
>> cleanup:
>> return ret;
>> 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 ad5c572aeefb..195248c88ae1 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 954ae88e4f64..1b2513650c58 100644
>> --- a/src/qemu/qemu_monitor.h
>> +++ b/src/qemu/qemu_monitor.h
>> @@ -755,6 +755,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 a09e93e464b3..4424abfa7148 100644
>> --- a/src/qemu/qemu_monitor_json.c
>> +++ b/src/qemu/qemu_monitor_json.c
>> @@ -6362,6 +6362,98 @@ 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;
>> + bool sev;
>> + 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 the 'query-sev-capabilities' QMP command was not available
>> + * we simply successfully return zero capabilities.
>> + * This is the case for QEMU <2.12 */
>> + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
>> + ret = 0;
>> + goto cleanup;
>
> Making this whole function depend on the presence of the new command
> will greatly simplify your pain with the qemucapabilitiestest which this
> will inflict. If you make sure that only new qemus call this command you
> won't have to fix all older test suites.
>
> Also note that you'll need to add a test case for the new qemu with the
> command supported so we can test this capability detection.
Yes adding new cap will help this, and I will also look into adding test
suite for this command.
>
>> + }
>> +
>> + if (qemuMonitorJSONCheckError(cmd, reply) < 0)
>> + goto cleanup;
>> +
>> + caps = virJSONValueObjectGetObject(reply, "return");
>> +
>> + if (virJSONValueObjectGetBoolean(caps, "sev", &sev) < 0) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("'sev' field is missing"));
>> + goto cleanup;
>> + }
>> +
>> + if (!sev) {
>> + goto cleanup;
>> + }
>> +
>> + 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_N(capability, 1) < 0)
>
>
> 'VIR_ALLOC' should be enough
>
Agreed.
>> + goto cleanup;
>> +
>> + if (VIR_STRDUP(capability->pdh, pdh) < 0)
>> + goto cleanup;
>> +
>> + if (VIR_STRDUP(capability->cert_chain, cert_chain) < 0)
>> + goto cleanup;
>> +
>> + capability->sev = true;
>
> This field should not be necessary. The presence of the structure itself
> should be good enough in this case.
>
Will remove this field.
>> + 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
>>
>> --
>> libvir-list mailing list
>> libvir-list at redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
More information about the libvir-list
mailing list