[libvirt] [PATCH v2 4/9] qemu: add support to launch SEV guest

Brijesh Singh posted 9 patches 7 years, 2 months ago
There is a newer version of this series
[libvirt] [PATCH v2 4/9] qemu: add support to launch SEV guest
Posted by Brijesh Singh 7 years, 2 months ago
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 | 33 ++++++++++++++++++
 src/qemu/qemu_process.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index fa0aa5d5c3d4..39f136a389cb 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9663,6 +9663,36 @@ qemuBuildTPMCommandLine(virCommandPtr cmd,
     return 0;
 }
 
+static void
+qemuBuildSevCommandLine(virCommandPtr cmd,
+                        virDomainSevDefPtr sev)
+{
+    virBuffer obj = VIR_BUFFER_INITIALIZER;
+    char *path = NULL;
+
+    VIR_DEBUG("policy=0x%x cbitpos=%d reduced_phys_bits=%d",
+              sev->policy, sev->cbitpos, sev->reduced_phys_bits);
+
+    virCommandAddArgList(cmd, "-machine", "memory-encryption=sev0", NULL);
+
+    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", sev->configDir));
+        virBufferAsprintf(&obj, ",dh-cert-file=%s", path);
+        VIR_FREE(path);
+    }
+
+    if (sev->session) {
+        ignore_value(virAsprintf(&path, "%s/session.base64", sev->configDir));
+        virBufferAsprintf(&obj, ",session-file=%s", path);
+        VIR_FREE(path);
+    }
+
+    virCommandAddArgList(cmd, "-object", virBufferContentAndReset(&obj), NULL);
+}
 
 static int
 qemuBuildVMCoreInfoCommandLine(virCommandPtr cmd,
@@ -10108,6 +10138,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
     if (qemuBuildVMCoreInfoCommandLine(cmd, def, qemuCaps) < 0)
         goto error;
 
+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV) && def->sev)
+        qemuBuildSevCommandLine(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..349e12b6dc12 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3457,6 +3457,16 @@ qemuProcessBuildDestroyMemoryPathsImpl(virQEMUDriverPtr driver,
 }
 
 
+static void
+qemuProcessDestroySevPaths(virDomainSevDefPtr sev)
+{
+    if (!sev)
+        return;
+
+    virFileDeleteTree(sev->configDir);
+    VIR_FREE(sev->configDir);
+}
+
 int
 qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr driver,
                                    virDomainObjPtr vm,
@@ -5741,6 +5751,83 @@ 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(virQEMUDriverPtr driver,
+                                virDomainObjPtr vm)
+{
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    virDomainDefPtr def = vm->def;
+    virQEMUCapsPtr qemuCaps = priv->qemuCaps;
+    virDomainSevDefPtr sev = def->sev;
+    char *configDir = NULL;
+    char *domPath = virDomainDefGetShortName(def);
+    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+
+    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 (virAsprintf(&configDir, "%s/sev/%s", cfg->configDir, domPath) < 0)
+        goto error;
+
+    if (virFileMakePathWithMode(configDir, S_IRWXU) < 0) {
+        virReportSystemError(errno, _("cannot create config directory '%s'"),
+                             configDir);
+        goto error;
+    }
+
+    if (sev->dh_cert) {
+        if (qemuBuildSevCreateFile(configDir, "dh_cert", sev->dh_cert) < 0)
+            goto error1;
+    }
+
+    if (sev->session) {
+        if (qemuBuildSevCreateFile(configDir, "session", sev->session) < 0)
+            goto error1;
+    }
+
+    VIR_FREE(domPath);
+    sev->configDir = configDir;
+    return 0;
+
+ error1:
+    virFileDeleteTree(configDir);
+ error:
+    VIR_FREE(configDir);
+    VIR_FREE(domPath);
+    return -1;
+}
 
 static int
 qemuProcessPrepareHostStorage(virQEMUDriverPtr driver,
@@ -5866,6 +5953,9 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver,
     if (qemuProcessPrepareHostStorage(driver, vm, flags) < 0)
         goto cleanup;
 
+    if (qemuProcessPrepareSevGuestInput(driver, vm) < 0)
+        goto cleanup;
+
     ret = 0;
  cleanup:
     virObjectUnref(cfg);
@@ -6535,6 +6625,7 @@ void qemuProcessStop(virQEMUDriverPtr driver,
     }
 
     qemuProcessBuildDestroyMemoryPaths(driver, vm, NULL, false);
+    qemuProcessDestroySevPaths(vm->def->sev);
 
     vm->def->id = -1;
 
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/9] qemu: add support to launch SEV guest
Posted by Daniel P. Berrangé 7 years, 2 months ago
On Thu, Mar 08, 2018 at 11:12:03AM -0600, Brijesh Singh wrote:
> 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 | 33 ++++++++++++++++++
>  src/qemu/qemu_process.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 124 insertions(+)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index fa0aa5d5c3d4..39f136a389cb 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -9663,6 +9663,36 @@ qemuBuildTPMCommandLine(virCommandPtr cmd,
>      return 0;
>  }
>  
> +static void
> +qemuBuildSevCommandLine(virCommandPtr cmd,
> +                        virDomainSevDefPtr sev)
> +{
> +    virBuffer obj = VIR_BUFFER_INITIALIZER;
> +    char *path = NULL;
> +
> +    VIR_DEBUG("policy=0x%x cbitpos=%d reduced_phys_bits=%d",
> +              sev->policy, sev->cbitpos, sev->reduced_phys_bits);
> +
> +    virCommandAddArgList(cmd, "-machine", "memory-encryption=sev0", NULL);


Hmm, so this will result in us passing  '-machine' many times, and relying
on QEMU to acculate the args. It should work, but IME it could be a little
surprising to people debuggnig qemu to see repeated args for -machine.

Could we just call this method from the place that  constructs the
current -machine arg so we append to that.

> +
> +    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", sev->configDir));
> +        virBufferAsprintf(&obj, ",dh-cert-file=%s", path);
> +        VIR_FREE(path);
> +    }
> +
> +    if (sev->session) {
> +        ignore_value(virAsprintf(&path, "%s/session.base64", sev->configDir));
> +        virBufferAsprintf(&obj, ",session-file=%s", path);
> +        VIR_FREE(path);
> +    }
> +
> +    virCommandAddArgList(cmd, "-object", virBufferContentAndReset(&obj), NULL);
> +}
>  
>  static int
>  qemuBuildVMCoreInfoCommandLine(virCommandPtr cmd,
> @@ -10108,6 +10138,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
>      if (qemuBuildVMCoreInfoCommandLine(cmd, def, qemuCaps) < 0)
>          goto error;
>  
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV) && def->sev)
> +        qemuBuildSevCommandLine(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..349e12b6dc12 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3457,6 +3457,16 @@ qemuProcessBuildDestroyMemoryPathsImpl(virQEMUDriverPtr driver,
>  }
>  
>  
> +static void
> +qemuProcessDestroySevPaths(virDomainSevDefPtr sev)
> +{
> +    if (!sev)
> +        return;
> +
> +    virFileDeleteTree(sev->configDir);

Just delete the individual files we created, no recursive delete please.

> +    VIR_FREE(sev->configDir);
> +}
> +
>  int
>  qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr driver,
>                                     virDomainObjPtr vm,
> @@ -5741,6 +5751,83 @@ 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(virQEMUDriverPtr driver,
> +                                virDomainObjPtr vm)
> +{
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    virDomainDefPtr def = vm->def;
> +    virQEMUCapsPtr qemuCaps = priv->qemuCaps;
> +    virDomainSevDefPtr sev = def->sev;
> +    char *configDir = NULL;
> +    char *domPath = virDomainDefGetShortName(def);
> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +
> +    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 (virAsprintf(&configDir, "%s/sev/%s", cfg->configDir, domPath) < 0)
> +        goto error;

We already have a directory where we put transient things like the QEMU
monitor socket, pid file, etc. This is defineed by 'priv->libDir', so
just use that directory and don't create extra sub-dirs.



> +    if (virFileMakePathWithMode(configDir, S_IRWXU) < 0) {
> +        virReportSystemError(errno, _("cannot create config directory '%s'"),
> +                             configDir);
> +        goto error;
> +    }
> +
> +    if (sev->dh_cert) {
> +        if (qemuBuildSevCreateFile(configDir, "dh_cert", sev->dh_cert) < 0)
> +            goto error1;
> +    }
> +
> +    if (sev->session) {
> +        if (qemuBuildSevCreateFile(configDir, "session", sev->session) < 0)
> +            goto error1;
> +    }
> +
> +    VIR_FREE(domPath);
> +    sev->configDir = configDir;
> +    return 0;
> +
> + error1:
> +    virFileDeleteTree(configDir);
> + error:
> +    VIR_FREE(configDir);
> +    VIR_FREE(domPath);
> +    return -1;
> +}
>  
>  static int
>  qemuProcessPrepareHostStorage(virQEMUDriverPtr driver,
> @@ -5866,6 +5953,9 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver,
>      if (qemuProcessPrepareHostStorage(driver, vm, flags) < 0)
>          goto cleanup;
>  
> +    if (qemuProcessPrepareSevGuestInput(driver, vm) < 0)
> +        goto cleanup;
> +
>      ret = 0;
>   cleanup:
>      virObjectUnref(cfg);
> @@ -6535,6 +6625,7 @@ void qemuProcessStop(virQEMUDriverPtr driver,
>      }
>  
>      qemuProcessBuildDestroyMemoryPaths(driver, vm, NULL, false);
> +    qemuProcessDestroySevPaths(vm->def->sev);
>  
>      vm->def->id = -1;
>  
> -- 
> 2.14.3
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/9] qemu: add support to launch SEV guest
Posted by Brijesh Singh 7 years, 2 months ago

On 03/12/2018 08:41 AM, Daniel P. Berrangé wrote:
> On Thu, Mar 08, 2018 at 11:12:03AM -0600, Brijesh Singh wrote:
>> 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 | 33 ++++++++++++++++++
>>   src/qemu/qemu_process.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 124 insertions(+)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index fa0aa5d5c3d4..39f136a389cb 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -9663,6 +9663,36 @@ qemuBuildTPMCommandLine(virCommandPtr cmd,
>>       return 0;
>>   }
>>   
>> +static void
>> +qemuBuildSevCommandLine(virCommandPtr cmd,
>> +                        virDomainSevDefPtr sev)
>> +{
>> +    virBuffer obj = VIR_BUFFER_INITIALIZER;
>> +    char *path = NULL;
>> +
>> +    VIR_DEBUG("policy=0x%x cbitpos=%d reduced_phys_bits=%d",
>> +              sev->policy, sev->cbitpos, sev->reduced_phys_bits);
>> +
>> +    virCommandAddArgList(cmd, "-machine", "memory-encryption=sev0", NULL);
> 
> 
> Hmm, so this will result in us passing  '-machine' many times, and relying
> on QEMU to acculate the args. It should work, but IME it could be a little
> surprising to people debuggnig qemu to see repeated args for -machine.
> 
> Could we just call this method from the place that  constructs the
> current -machine arg so we append to that.


OK, I will take a look and try to avoid duplicate "-machine".

> 
>> +
>> +    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", sev->configDir));
>> +        virBufferAsprintf(&obj, ",dh-cert-file=%s", path);
>> +        VIR_FREE(path);
>> +    }
>> +
>> +    if (sev->session) {
>> +        ignore_value(virAsprintf(&path, "%s/session.base64", sev->configDir));
>> +        virBufferAsprintf(&obj, ",session-file=%s", path);
>> +        VIR_FREE(path);
>> +    }
>> +
>> +    virCommandAddArgList(cmd, "-object", virBufferContentAndReset(&obj), NULL);
>> +}
>>   
>>   static int
>>   qemuBuildVMCoreInfoCommandLine(virCommandPtr cmd,
>> @@ -10108,6 +10138,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
>>       if (qemuBuildVMCoreInfoCommandLine(cmd, def, qemuCaps) < 0)
>>           goto error;
>>   
>> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV) && def->sev)
>> +        qemuBuildSevCommandLine(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..349e12b6dc12 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
>> @@ -3457,6 +3457,16 @@ qemuProcessBuildDestroyMemoryPathsImpl(virQEMUDriverPtr driver,
>>   }
>>   
>>   
>> +static void
>> +qemuProcessDestroySevPaths(virDomainSevDefPtr sev)
>> +{
>> +    if (!sev)
>> +        return;
>> +
>> +    virFileDeleteTree(sev->configDir);
> 
> Just delete the individual files we created, no recursive delete please.
> 

Will do.


>> +    VIR_FREE(sev->configDir);
>> +}
>> +
>>   int
>>   qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr driver,
>>                                      virDomainObjPtr vm,
>> @@ -5741,6 +5751,83 @@ 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(virQEMUDriverPtr driver,
>> +                                virDomainObjPtr vm)
>> +{
>> +    qemuDomainObjPrivatePtr priv = vm->privateData;
>> +    virDomainDefPtr def = vm->def;
>> +    virQEMUCapsPtr qemuCaps = priv->qemuCaps;
>> +    virDomainSevDefPtr sev = def->sev;
>> +    char *configDir = NULL;
>> +    char *domPath = virDomainDefGetShortName(def);
>> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>> +
>> +    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 (virAsprintf(&configDir, "%s/sev/%s", cfg->configDir, domPath) < 0)
>> +        goto error;
> 
> We already have a directory where we put transient things like the QEMU
> monitor socket, pid file, etc. This is defineed by 'priv->libDir', so
> just use that directory and don't create extra sub-dirs.
> 
> 

Will use priv->libDir in next rev. thanks


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list