[RFC PATCH v2 0/9] i386/sev: Use C API of Rust SEV library

[RFC PATCH v2 0/9] i386/sev: Use C API of Rust SEV library

[Tyler Fanelli]

These patches are submitted as an RFC mainly because I'm a relative
newcomer to QEMU with no knowledge of the community's views on
including Rust code, nor it's preference of using library APIs for
ioctls that were previously implemented in QEMU directly.

Recently, the Rust sev library [0] has introduced a C API to take
advantage of the library outside of Rust.

Should the inclusion of the library as a dependency be desired, it can
be extended further to include the firmware/platform ioctls and more.
This would result in much of the AMD-SEV portion of QEMU being offloaded
to the library.

This series looks to explore the possibility of using the library and
show a bit of what it would look like. I'm looking for comments
regarding if this feature is desired.

[0] https://github.com/virtee/sev

NOTE: The required meson changes in the Rust library are not merged yet.
Therefore, the git repository URL in subprojects/sev.wrap points to a
personal fork of the library (for testing purposes). The meson patches
for the library are required before these patches can be merged.

Changes since v1:
- Add sev Rust library as a QEMU subproject, rather than using
  pkg-config for linking
- Rebased to upstream/master
- Use C API for SEV_ATTESTATION_REPORT ioctl

Tyler Fanelli (9):
  Add Rust SEV library as subproject
  i386/sev: Replace INIT and ES_INIT ioctls with sev library equivalents
  i386/sev: Replace LAUNCH_START ioctl with sev library equivalent
  i386/sev: Replace UPDATE_DATA ioctl with sev library equivalent
  i386/sev: Replace LAUNCH_UPDATE_VMSA ioctl with sev library equivalent
  i386/sev: Replace LAUNCH_MEASURE ioctl with sev library equivalent
  i386/sev: Replace LAUNCH_SECRET ioctl with sev library equivalent
  i386/sev: Replace LAUNCH_FINISH ioctl with sev library equivalent
  i386/sev: Replace SEV_ATTESTATION_REPORT with sev library equivalent

 meson.build                   |   8 +
 meson_options.txt             |   2 +
 scripts/meson-buildoptions.sh |   3 +
 subprojects/sev.wrap          |   6 +
 target/i386/meson.build       |   2 +-
 target/i386/sev.c             | 392 +++++++++++-----------------------
 target/i386/sev.h             |   6 +-
 target/i386/trace-events      |   1 +
 8 files changed, 148 insertions(+), 272 deletions(-)
 create mode 100644 subprojects/sev.wrap

-- 
2.40.1

[RFC PATCH v2 1/9] Add Rust SEV library as subproject

[Tyler Fanelli]

The Rust sev library provides a C API for the AMD SEV launch ioctls, as
well as the ability to build with meson. Add the Rust sev library as a
QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
APIs provided by it.

Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
---
 meson.build                   | 8 ++++++++
 meson_options.txt             | 2 ++
 scripts/meson-buildoptions.sh | 3 +++
 subprojects/sev.wrap          | 6 ++++++
 target/i386/meson.build       | 2 +-
 5 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 subprojects/sev.wrap

diff --git a/meson.build b/meson.build
index 20ceeb8158..8a17c29de8 100644
--- a/meson.build
+++ b/meson.build
@@ -960,6 +960,13 @@ if not get_option('slirp').auto() or have_system
   endif
 endif
 
+sev = not_found
+if not get_option('sev').auto()
+  sev = dependency('sev',
+                   method: 'pkg-config',
+                   required: get_option('sev'))
+endif
+
 vde = not_found
 if not get_option('vde').auto() or have_system or have_tools
   vde = cc.find_library('vdeplug', has_headers: ['libvdeplug.h'],
@@ -4331,6 +4338,7 @@ summary_info += {'libudev':           libudev}
 # Dummy dependency, keep .found()
 summary_info += {'FUSE lseek':        fuse_lseek.found()}
 summary_info += {'selinux':           selinux}
+summary_info += {'sev':               sev}
 summary_info += {'libdw':             libdw}
 summary(summary_info, bool_yn: true, section: 'Dependencies')
 
diff --git a/meson_options.txt b/meson_options.txt
index 57e265c871..5b8d283717 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -204,6 +204,8 @@ option('sdl_image', type : 'feature', value : 'auto',
        description: 'SDL Image support for icons')
 option('seccomp', type : 'feature', value : 'auto',
        description: 'seccomp support')
+option('sev', type : 'feature', value : 'auto',
+        description: 'Rust AMD SEV library')
 option('smartcard', type : 'feature', value : 'auto',
        description: 'CA smartcard emulation support')
 option('snappy', type : 'feature', value : 'auto',
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index e4b46d5715..e585a548fa 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -161,6 +161,7 @@ meson_options_help() {
   printf "%s\n" '  sdl-image       SDL Image support for icons'
   printf "%s\n" '  seccomp         seccomp support'
   printf "%s\n" '  selinux         SELinux support in qemu-nbd'
+  printf "%s\n" '  sev             SEV library support'
   printf "%s\n" '  slirp           libslirp user mode network backend support'
   printf "%s\n" '  slirp-smbd      use smbd (at path --smbd=*) in slirp networking'
   printf "%s\n" '  smartcard       CA smartcard emulation support'
@@ -440,6 +441,8 @@ _meson_option_parse() {
     --disable-seccomp) printf "%s" -Dseccomp=disabled ;;
     --enable-selinux) printf "%s" -Dselinux=enabled ;;
     --disable-selinux) printf "%s" -Dselinux=disabled ;;
+    --enable-sev) printf "%s" -Dsev=enabled ;;
+    --disable-sev) printf "%s" -Dsev=disabled ;;
     --enable-slirp) printf "%s" -Dslirp=enabled ;;
     --disable-slirp) printf "%s" -Dslirp=disabled ;;
     --enable-slirp-smbd) printf "%s" -Dslirp_smbd=enabled ;;
diff --git a/subprojects/sev.wrap b/subprojects/sev.wrap
new file mode 100644
index 0000000000..5be1faccf6
--- /dev/null
+++ b/subprojects/sev.wrap
@@ -0,0 +1,6 @@
+[wrap-git]
+url = https://github.com/tylerfanelli/sev
+revision = b81b1da5df50055600a5b0349b0c4afda677cccb
+
+[provide]
+sev = sev_dep
diff --git a/target/i386/meson.build b/target/i386/meson.build
index 6f1036d469..8972a4fb17 100644
--- a/target/i386/meson.build
+++ b/target/i386/meson.build
@@ -20,7 +20,7 @@ i386_system_ss.add(files(
   'monitor.c',
   'cpu-sysemu.c',
 ))
-i386_system_ss.add(when: 'CONFIG_SEV', if_true: files('sev.c'), if_false: files('sev-sysemu-stub.c'))
+i386_system_ss.add(when: 'CONFIG_SEV', if_true: [sev, files('sev.c')], if_false: files('sev-sysemu-stub.c'))
 
 i386_user_ss = ss.source_set()
 
-- 
2.40.1

[RFC PATCH v2 2/9] i386/sev: Replace INIT and ES_INIT ioctls with sev library equivalents

[Tyler Fanelli]

The sev library offers APIs for SEV_INIT and SEV_ES_INIT, both taking
the file descriptors of the encrypting VM and /dev/sev as input.

If this API ioctl call fails, fw_error will be set accordingly.

Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
---
 target/i386/sev.c        | 14 +++++++++-----
 target/i386/trace-events |  1 +
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index fe2144c038..97388f5fa2 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -18,6 +18,8 @@
 
 #include <sys/ioctl.h>
 
+#include <libsev.h>
+
 #include "qapi/error.h"
 #include "qom/object_interfaces.h"
 #include "qemu/base64.h"
@@ -27,6 +29,7 @@
 #include "crypto/hash.h"
 #include "sysemu/kvm.h"
 #include "sev.h"
+#include "sysemu/kvm_int.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/runstate.h"
 #include "trace.h"
@@ -911,10 +914,11 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
     SevGuestState *sev
         = (SevGuestState *)object_dynamic_cast(OBJECT(cgs), TYPE_SEV_GUEST);
     char *devname;
-    int ret, fw_error, cmd;
+    int ret, fw_error;
     uint32_t ebx;
     uint32_t host_cbitpos;
     struct sev_user_data_status status = {};
+    KVMState *s = kvm_state;
 
     if (!sev) {
         return 0;
@@ -990,13 +994,13 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
                          __func__);
             goto err;
         }
-        cmd = KVM_SEV_ES_INIT;
+        trace_kvm_sev_es_init();
+        ret = sev_es_init(s->vmfd, sev->sev_fd, &fw_error);
     } else {
-        cmd = KVM_SEV_INIT;
+        trace_kvm_sev_init();
+        ret = sev_init(s->vmfd, sev->sev_fd, &fw_error);
     }
 
-    trace_kvm_sev_init();
-    ret = sev_ioctl(sev->sev_fd, cmd, NULL, &fw_error);
     if (ret) {
         error_setg(errp, "%s: failed to initialize ret=%d fw_error=%d '%s'",
                    __func__, ret, fw_error, fw_error_to_str(fw_error));
diff --git a/target/i386/trace-events b/target/i386/trace-events
index 2cd8726eeb..2dca4ee117 100644
--- a/target/i386/trace-events
+++ b/target/i386/trace-events
@@ -2,6 +2,7 @@
 
 # sev.c
 kvm_sev_init(void) ""
+kvm_sev_es_init(void) ""
 kvm_memcrypt_register_region(void *addr, size_t len) "addr %p len 0x%zx"
 kvm_memcrypt_unregister_region(void *addr, size_t len) "addr %p len 0x%zx"
 kvm_sev_change_state(const char *old, const char *new) "%s -> %s"
-- 
2.40.1

[RFC PATCH v2 3/9] i386/sev: Replace LAUNCH_START ioctl with sev library equivalent

[Tyler Fanelli]

The sev library offers an equivalent API for SEV_LAUNCH_START. The
library contains some internal state for each VM it's currently running,
and organizes the internal state for each VM via it's file descriptor.
Therefore, the VM's file descriptor must be provided as input.

If this API ioctl call fails, fw_error will be set accordingly.

Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
---
 target/i386/sev.c | 80 ++++++++++++++++++-----------------------------
 1 file changed, 30 insertions(+), 50 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 97388f5fa2..4c888fa77f 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -715,51 +715,6 @@ sev_read_file_base64(const char *filename, guchar **data, gsize *len)
     return 0;
 }
 
-static int
-sev_launch_start(SevGuestState *sev)
-{
-    gsize sz;
-    int ret = 1;
-    int fw_error, rc;
-    struct kvm_sev_launch_start start = {
-        .handle = sev->handle, .policy = sev->policy
-    };
-    guchar *session = NULL, *dh_cert = NULL;
-
-    if (sev->session_file) {
-        if (sev_read_file_base64(sev->session_file, &session, &sz) < 0) {
-            goto out;
-        }
-        start.session_uaddr = (unsigned long)session;
-        start.session_len = sz;
-    }
-
-    if (sev->dh_cert_file) {
-        if (sev_read_file_base64(sev->dh_cert_file, &dh_cert, &sz) < 0) {
-            goto out;
-        }
-        start.dh_uaddr = (unsigned long)dh_cert;
-        start.dh_len = sz;
-    }
-
-    trace_kvm_sev_launch_start(start.policy, session, dh_cert);
-    rc = sev_ioctl(sev->sev_fd, KVM_SEV_LAUNCH_START, &start, &fw_error);
-    if (rc < 0) {
-        error_report("%s: LAUNCH_START ret=%d fw_error=%d '%s'",
-                __func__, ret, fw_error, fw_error_to_str(fw_error));
-        goto out;
-    }
-
-    sev_set_guest_state(sev, SEV_STATE_LAUNCH_UPDATE);
-    sev->handle = start.handle;
-    ret = 0;
-
-out:
-    g_free(session);
-    g_free(dh_cert);
-    return ret;
-}
-
 static int
 sev_launch_update_data(SevGuestState *sev, uint8_t *addr, uint64_t len)
 {
@@ -913,11 +868,13 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
 {
     SevGuestState *sev
         = (SevGuestState *)object_dynamic_cast(OBJECT(cgs), TYPE_SEV_GUEST);
+    gsize sz;
     char *devname;
-    int ret, fw_error;
+    int ret = -1, fw_error;
     uint32_t ebx;
     uint32_t host_cbitpos;
     struct sev_user_data_status status = {};
+    guchar *session = NULL, *dh_cert = NULL;
     KVMState *s = kvm_state;
 
     if (!sev) {
@@ -1007,23 +964,46 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
         goto err;
     }
 
-    ret = sev_launch_start(sev);
+    if (!sev->session_file || !sev->dh_cert_file) {
+        goto err;
+    }
+
+    if (sev_read_file_base64(sev->session_file, &session, &sz) < 0) {
+        goto err;
+    }
+
+    if (sev_read_file_base64(sev->dh_cert_file, &dh_cert, &sz) < 0) {
+        goto err;
+    }
+
+    ret = sev_launch_start(s->vmfd, sev->policy, (void *) dh_cert,
+                           (void *) session, &fw_error);
     if (ret) {
-        error_setg(errp, "%s: failed to create encryption context", __func__);
+        error_setg(errp, "%s: LAUNCH_START ret=%d fw_error=%d '%s'",
+                   __func__, ret, fw_error, fw_error_to_str(fw_error));
         goto err;
     }
 
+    sev_set_guest_state(sev, SEV_STATE_LAUNCH_UPDATE);
+
     ram_block_notifier_add(&sev_ram_notifier);
     qemu_add_machine_init_done_notifier(&sev_machine_done_notify);
     qemu_add_vm_change_state_handler(sev_vm_state_change, sev);
 
     cgs->ready = true;
 
-    return 0;
+    ret = 0;
+    goto out;
+
 err:
     sev_guest = NULL;
     ram_block_discard_disable(false);
-    return -1;
+out:
+    g_free(session);
+    g_free(dh_cert);
+
+    return ret;
+
 }
 
 int
-- 
2.40.1

[RFC PATCH v2 4/9] i386/sev: Replace UPDATE_DATA ioctl with sev library equivalent

[Tyler Fanelli]

UPDATE_DATA takes the VM's file descriptor, a guest memory region to
be encrypted, as well as the size of the aforementioned guest memory
region.

If this API ioctl call fails, fw_error will be set accordingly.

Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
---
 target/i386/sev.c | 31 ++++++-------------------------
 1 file changed, 6 insertions(+), 25 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 4c888fa77f..73d3820364 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -715,29 +715,6 @@ sev_read_file_base64(const char *filename, guchar **data, gsize *len)
     return 0;
 }
 
-static int
-sev_launch_update_data(SevGuestState *sev, uint8_t *addr, uint64_t len)
-{
-    int ret, fw_error;
-    struct kvm_sev_launch_update_data update;
-
-    if (!addr || !len) {
-        return 1;
-    }
-
-    update.uaddr = (__u64)(unsigned long)addr;
-    update.len = len;
-    trace_kvm_sev_launch_update_data(addr, len);
-    ret = sev_ioctl(sev->sev_fd, KVM_SEV_LAUNCH_UPDATE_DATA,
-                    &update, &fw_error);
-    if (ret) {
-        error_report("%s: LAUNCH_UPDATE ret=%d fw_error=%d '%s'",
-                __func__, ret, fw_error, fw_error_to_str(fw_error));
-    }
-
-    return ret;
-}
-
 static int
 sev_launch_update_vmsa(SevGuestState *sev)
 {
@@ -1009,15 +986,19 @@ out:
 int
 sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)
 {
+    KVMState *s = kvm_state;
+    int fw_error;
+
     if (!sev_guest) {
         return 0;
     }
 
     /* if SEV is in update state then encrypt the data else do nothing */
     if (sev_check_state(sev_guest, SEV_STATE_LAUNCH_UPDATE)) {
-        int ret = sev_launch_update_data(sev_guest, ptr, len);
+        int ret = sev_launch_update_data(s->vmfd, (__u64) ptr, len, &fw_error);
         if (ret < 0) {
-            error_setg(errp, "SEV: Failed to encrypt pflash rom");
+            error_setg(errp, "SEV: Failed to encrypt pflash rom fw_err=%d",
+                       fw_error);
             return ret;
         }
     }
-- 
2.40.1

[RFC PATCH v2 5/9] i386/sev: Replace LAUNCH_UPDATE_VMSA ioctl with sev library equivalent

[Tyler Fanelli]

The LAUNCH_UPDATE_VMSA API takes the VM's file descriptor, as well as a
field for any firmware errors as input.

If this API ioctl call fails, fw_error will be set accordingly.

Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
---
 target/i386/sev.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 73d3820364..a5bd1653ef 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -715,27 +715,14 @@ sev_read_file_base64(const char *filename, guchar **data, gsize *len)
     return 0;
 }
 
-static int
-sev_launch_update_vmsa(SevGuestState *sev)
-{
-    int ret, fw_error;
-
-    ret = sev_ioctl(sev->sev_fd, KVM_SEV_LAUNCH_UPDATE_VMSA, NULL, &fw_error);
-    if (ret) {
-        error_report("%s: LAUNCH_UPDATE_VMSA ret=%d fw_error=%d '%s'",
-                __func__, ret, fw_error, fw_error_to_str(fw_error));
-    }
-
-    return ret;
-}
-
 static void
 sev_launch_get_measure(Notifier *notifier, void *unused)
 {
     SevGuestState *sev = sev_guest;
-    int ret, error;
+    int ret, fw_error;
     g_autofree guchar *data = NULL;
     struct kvm_sev_launch_measure measurement = {};
+    KVMState *s = kvm_state;
 
     if (!sev_check_state(sev, SEV_STATE_LAUNCH_UPDATE)) {
         return;
@@ -743,18 +730,20 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
 
     if (sev_es_enabled()) {
         /* measure all the VM save areas before getting launch_measure */
-        ret = sev_launch_update_vmsa(sev);
+        ret = sev_launch_update_vmsa(s->vmfd, &fw_error);
         if (ret) {
+            error_report("%s: LAUNCH_UPDATE_VMSA ret=%d fw_error=%d '%s'",
+                __func__, ret, fw_error, fw_error_to_str(fw_error));
             exit(1);
         }
     }
 
     /* query the measurement blob length */
     ret = sev_ioctl(sev->sev_fd, KVM_SEV_LAUNCH_MEASURE,
-                    &measurement, &error);
+                    &measurement, &fw_error);
     if (!measurement.len) {
         error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'",
-                     __func__, ret, error, fw_error_to_str(errno));
+                     __func__, ret, fw_error, fw_error_to_str(fw_error));
         return;
     }
 
@@ -763,10 +752,10 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
 
     /* get the measurement blob */
     ret = sev_ioctl(sev->sev_fd, KVM_SEV_LAUNCH_MEASURE,
-                    &measurement, &error);
+                    &measurement, &fw_error);
     if (ret) {
         error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'",
-                     __func__, ret, error, fw_error_to_str(errno));
+                     __func__, ret, fw_error, fw_error_to_str(fw_error));
         return;
     }
 
-- 
2.40.1

[RFC PATCH v2 6/9] i386/sev: Replace LAUNCH_MEASURE ioctl with sev library equivalent

[Tyler Fanelli]

The LAUNCH_MEASURE API returns the measurement of the launched guest's
memory pages (and VMCB save areas if ES is enabled). The caller is
responsible for ensuring that the pointer (identified as the "data"
argument) is a valid pointer that can hold the guest's measurement (a
measurement in SEV is 48 bytes in size).

If this API ioctl call fails, fw_error will be set accordingly.

Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
---
 target/i386/sev.c | 24 ++++++------------------
 target/i386/sev.h |  2 ++
 2 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index a5bd1653ef..3e2a3e07a7 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -721,7 +721,6 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
     SevGuestState *sev = sev_guest;
     int ret, fw_error;
     g_autofree guchar *data = NULL;
-    struct kvm_sev_launch_measure measurement = {};
     KVMState *s = kvm_state;
 
     if (!sev_check_state(sev, SEV_STATE_LAUNCH_UPDATE)) {
@@ -738,31 +737,20 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
         }
     }
 
-    /* query the measurement blob length */
-    ret = sev_ioctl(sev->sev_fd, KVM_SEV_LAUNCH_MEASURE,
-                    &measurement, &fw_error);
-    if (!measurement.len) {
-        error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'",
-                     __func__, ret, fw_error, fw_error_to_str(fw_error));
-        return;
-    }
+    data = g_malloc(SEV_MEASUREMENT_SIZE);
 
-    data = g_new0(guchar, measurement.len);
-    measurement.uaddr = (unsigned long)data;
-
-    /* get the measurement blob */
-    ret = sev_ioctl(sev->sev_fd, KVM_SEV_LAUNCH_MEASURE,
-                    &measurement, &fw_error);
+    ret = sev_launch_measure(s->vmfd, data, &fw_error);
     if (ret) {
-        error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'",
-                     __func__, ret, fw_error, fw_error_to_str(fw_error));
+        error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'", __func__,
+                       ret, fw_error, fw_error_to_str(fw_error));
+
         return;
     }
 
     sev_set_guest_state(sev, SEV_STATE_LAUNCH_SECRET);
 
     /* encode the measurement value and emit the event */
-    sev->measurement = g_base64_encode(data, measurement.len);
+    sev->measurement = g_base64_encode(data, SEV_MEASUREMENT_SIZE);
     trace_kvm_sev_launch_measurement(sev->measurement);
 }
 
diff --git a/target/i386/sev.h b/target/i386/sev.h
index e7499c95b1..acb181358e 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -38,6 +38,8 @@ typedef struct SevKernelLoaderContext {
     size_t cmdline_size;
 } SevKernelLoaderContext;
 
+#define SEV_MEASUREMENT_SIZE 48
+
 #ifdef CONFIG_SEV
 bool sev_enabled(void);
 bool sev_es_enabled(void);
-- 
2.40.1

[RFC PATCH v2 7/9] i386/sev: Replace LAUNCH_SECRET ioctl with sev library equivalent

[Tyler Fanelli]

The LAUNCH_SECRET API can inject a secret into the VM once the
measurement has been retrieved.

If this API ioctl call fails, fw_error will be set accordingly.

Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
---
 target/i386/sev.c | 105 ++++++++++++++++------------------------------
 target/i386/sev.h |   2 -
 2 files changed, 36 insertions(+), 71 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 3e2a3e07a7..38a90d4f00 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -983,88 +983,44 @@ sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)
     return 0;
 }
 
-int sev_inject_launch_secret(const char *packet_hdr, const char *secret,
-                             uint64_t gpa, Error **errp)
-{
-    struct kvm_sev_launch_secret input;
-    g_autofree guchar *data = NULL, *hdr = NULL;
-    int error, ret = 1;
-    void *hva;
-    gsize hdr_sz = 0, data_sz = 0;
-    MemoryRegion *mr = NULL;
-
-    if (!sev_guest) {
-        error_setg(errp, "SEV not enabled for guest");
-        return 1;
-    }
-
-    /* secret can be injected only in this state */
-    if (!sev_check_state(sev_guest, SEV_STATE_LAUNCH_SECRET)) {
-        error_setg(errp, "SEV: Not in correct state. (LSECRET) %x",
-                     sev_guest->state);
-        return 1;
-    }
-
-    hdr = g_base64_decode(packet_hdr, &hdr_sz);
-    if (!hdr || !hdr_sz) {
-        error_setg(errp, "SEV: Failed to decode sequence header");
-        return 1;
-    }
-
-    data = g_base64_decode(secret, &data_sz);
-    if (!data || !data_sz) {
-        error_setg(errp, "SEV: Failed to decode data");
-        return 1;
-    }
-
-    hva = gpa2hva(&mr, gpa, data_sz, errp);
-    if (!hva) {
-        error_prepend(errp, "SEV: Failed to calculate guest address: ");
-        return 1;
-    }
-
-    input.hdr_uaddr = (uint64_t)(unsigned long)hdr;
-    input.hdr_len = hdr_sz;
-
-    input.trans_uaddr = (uint64_t)(unsigned long)data;
-    input.trans_len = data_sz;
-
-    input.guest_uaddr = (uint64_t)(unsigned long)hva;
-    input.guest_len = data_sz;
-
-    trace_kvm_sev_launch_secret(gpa, input.guest_uaddr,
-                                input.trans_uaddr, input.trans_len);
-
-    ret = sev_ioctl(sev_guest->sev_fd, KVM_SEV_LAUNCH_SECRET,
-                    &input, &error);
-    if (ret) {
-        error_setg(errp, "SEV: failed to inject secret ret=%d fw_error=%d '%s'",
-                     ret, error, fw_error_to_str(error));
-        return ret;
-    }
-
-    return 0;
-}
-
 #define SEV_SECRET_GUID "4c2eb361-7d9b-4cc3-8081-127c90d3d294"
 struct sev_secret_area {
     uint32_t base;
     uint32_t size;
 };
 
-void qmp_sev_inject_launch_secret(const char *packet_hdr,
-                                  const char *secret,
+void qmp_sev_inject_launch_secret(const char *hdr_b64,
+                                  const char *secret_b64,
                                   bool has_gpa, uint64_t gpa,
                                   Error **errp)
 {
+    int ret, fw_error = 0;
+    g_autofree guchar *hdr = NULL, *secret = NULL;
+    uint8_t *data = NULL;
+    KVMState *s = kvm_state;
+    gsize hdr_sz = 0, secret_sz = 0;
+    MemoryRegion *mr = NULL;
+    void *hva;
+    struct sev_secret_area *area = NULL;
+
     if (!sev_enabled()) {
         error_setg(errp, "SEV not enabled for guest");
         return;
     }
-    if (!has_gpa) {
-        uint8_t *data;
-        struct sev_secret_area *area;
 
+    hdr = g_base64_decode(hdr_b64, &hdr_sz);
+    if (!hdr || !hdr_sz) {
+        error_setg(errp, "SEV: Failed to decode sequence header");
+        return;
+    }
+
+    secret = g_base64_decode(secret_b64, &secret_sz);
+    if (!secret || !secret_sz) {
+        error_setg(errp, "SEV: Failed to decode secret");
+        return;
+    }
+
+    if (!has_gpa) {
         if (!pc_system_ovmf_table_find(SEV_SECRET_GUID, &data, NULL)) {
             error_setg(errp, "SEV: no secret area found in OVMF,"
                        " gpa must be specified.");
@@ -1074,7 +1030,18 @@ void qmp_sev_inject_launch_secret(const char *packet_hdr,
         gpa = area->base;
     }
 
-    sev_inject_launch_secret(packet_hdr, secret, gpa, errp);
+    hva = gpa2hva(&mr, gpa, secret_sz, errp);
+    if (!hva) {
+        error_prepend(errp, "SEV: Failed to calculate guest address: ");
+        return;
+    }
+
+    ret = sev_inject_launch_secret(s->vmfd, hdr, secret, secret_sz,
+                                   hva, &fw_error);
+    if (ret < 0) {
+        error_setg(errp, "%s: LAUNCH_SECRET ret=%d fw_error=%d '%s'", __func__,
+                   ret, fw_error, fw_error_to_str(fw_error));
+    }
 }
 
 static int
diff --git a/target/i386/sev.h b/target/i386/sev.h
index acb181358e..f1af28eca0 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -53,8 +53,6 @@ uint32_t sev_get_reduced_phys_bits(void);
 bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp);
 
 int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp);
-int sev_inject_launch_secret(const char *hdr, const char *secret,
-                             uint64_t gpa, Error **errp);
 
 int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size);
 void sev_es_set_reset_vector(CPUState *cpu);
-- 
2.40.1

[RFC PATCH v2 8/9] i386/sev: Replace LAUNCH_FINISH ioctl with sev library equivalent

[Tyler Fanelli]

The LAUNCH_FINISH ioctl finishes the guest launch flow and transitions
the guest into a state ready to be run.

If this API ioctl call fails, fw_error will be set accordingly.

Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
---
 target/i386/sev.c | 38 ++++++++++++++++----------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 38a90d4f00..764a89d3a4 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -785,35 +785,29 @@ static Notifier sev_machine_done_notify = {
     .notify = sev_launch_get_measure,
 };
 
-static void
-sev_launch_finish(SevGuestState *sev)
-{
-    int ret, error;
-
-    trace_kvm_sev_launch_finish();
-    ret = sev_ioctl(sev->sev_fd, KVM_SEV_LAUNCH_FINISH, 0, &error);
-    if (ret) {
-        error_report("%s: LAUNCH_FINISH ret=%d fw_error=%d '%s'",
-                     __func__, ret, error, fw_error_to_str(error));
-        exit(1);
-    }
-
-    sev_set_guest_state(sev, SEV_STATE_RUNNING);
-
-    /* add migration blocker */
-    error_setg(&sev_mig_blocker,
-               "SEV: Migration is not implemented");
-    migrate_add_blocker(sev_mig_blocker, &error_fatal);
-}
-
 static void
 sev_vm_state_change(void *opaque, bool running, RunState state)
 {
     SevGuestState *sev = opaque;
+    int ret, fw_error;
+    KVMState *s = kvm_state;
 
     if (running) {
         if (!sev_check_state(sev, SEV_STATE_RUNNING)) {
-            sev_launch_finish(sev);
+            trace_kvm_sev_launch_finish();
+            ret = sev_launch_finish(s->vmfd, &fw_error);
+            if (ret) {
+                error_report("%s: LAUNCH_FINISH ret=%d fw_error=%d '%s'",
+                             __func__, ret, fw_error,
+                             fw_error_to_str(fw_error));
+                exit(1);
+            }
+
+            sev_set_guest_state(sev, SEV_STATE_RUNNING);
+
+            // add migration blocker.
+            error_setg(&sev_mig_blocker, "SEV: Migration is not implemented");
+            migrate_add_blocker(sev_mig_blocker, &error_fatal);
         }
     }
 }
-- 
2.40.1

[RFC PATCH v2 9/9] i386/sev: Replace SEV_ATTESTATION_REPORT with sev library equivalent

[Tyler Fanelli]

The LAUNCH_ATTESTATION ioctl fetches the guest VM's attestation report
from the PSP.

If the API ioctl call fails, fw_error will be set accordingly.

Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
---
 target/i386/sev.c | 81 ++++++++++-------------------------------------
 target/i386/sev.h |  2 ++
 2 files changed, 18 insertions(+), 65 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 764a89d3a4..bedb8f379e 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -160,27 +160,6 @@ static const char *const sev_fw_errlist[] = {
 
 #define SEV_FW_MAX_ERROR      ARRAY_SIZE(sev_fw_errlist)
 
-static int
-sev_ioctl(int fd, int cmd, void *data, int *error)
-{
-    int r;
-    struct kvm_sev_cmd input;
-
-    memset(&input, 0x0, sizeof(input));
-
-    input.id = cmd;
-    input.sev_fd = fd;
-    input.data = (__u64)(unsigned long)data;
-
-    r = kvm_vm_ioctl(kvm_state, KVM_MEMORY_ENCRYPT_OP, &input);
-
-    if (error) {
-        *error = input.error;
-    }
-
-    return r;
-}
-
 static int
 sev_platform_ioctl(int fd, int cmd, void *data, int *error)
 {
@@ -629,75 +608,47 @@ SevCapability *qmp_query_sev_capabilities(Error **errp)
     return sev_get_capabilities(errp);
 }
 
-static SevAttestationReport *sev_get_attestation_report(const char *mnonce,
-                                                        Error **errp)
+SevAttestationReport *qmp_query_sev_attestation_report(const char *mnonce_b64,
+                                                       Error **errp)
 {
-    struct kvm_sev_attestation_report input = {};
     SevAttestationReport *report = NULL;
-    SevGuestState *sev = sev_guest;
-    g_autofree guchar *data = NULL;
-    g_autofree guchar *buf = NULL;
-    gsize len;
-    int err = 0, ret;
+    g_autofree guchar *data = NULL, *mnonce = NULL;
+    gsize len, data_len;
+    int ret, fw_error;
+    KVMState *s = kvm_state;
 
     if (!sev_enabled()) {
         error_setg(errp, "SEV is not enabled");
         return NULL;
     }
 
-    /* lets decode the mnonce string */
-    buf = g_base64_decode(mnonce, &len);
-    if (!buf) {
+    mnonce = g_base64_decode(mnonce_b64, &len);
+    if (!mnonce) {
         error_setg(errp, "SEV: failed to decode mnonce input");
         return NULL;
     }
 
-    /* verify the input mnonce length */
-    if (len != sizeof(input.mnonce)) {
-        error_setg(errp, "SEV: mnonce must be %zu bytes (got %" G_GSIZE_FORMAT ")",
-                sizeof(input.mnonce), len);
+    if (len != SEV_ATTESTATION_REPORT_MNONCE_SIZE) {
+        error_setg(errp, "SEV: mnonce must be %d bytes (found %" G_GSIZE_FORMAT ")",
+            SEV_ATTESTATION_REPORT_MNONCE_SIZE, len);
         return NULL;
     }
 
-    /* Query the report length */
-    ret = sev_ioctl(sev->sev_fd, KVM_SEV_GET_ATTESTATION_REPORT,
-            &input, &err);
-    if (ret < 0) {
-        if (err != SEV_RET_INVALID_LEN) {
-            error_setg(errp, "SEV: Failed to query the attestation report"
-                             " length ret=%d fw_err=%d (%s)",
-                       ret, err, fw_error_to_str(err));
-            return NULL;
-        }
-    }
-
-    data = g_malloc(input.len);
-    input.uaddr = (unsigned long)data;
-    memcpy(input.mnonce, buf, sizeof(input.mnonce));
-
-    /* Query the report */
-    ret = sev_ioctl(sev->sev_fd, KVM_SEV_GET_ATTESTATION_REPORT,
-            &input, &err);
+    ret = sev_attestation_report(s->vmfd, mnonce, len, (void *) data,
+                                (unsigned int *) &data_len, &fw_error);
     if (ret) {
         error_setg_errno(errp, errno, "SEV: Failed to get attestation report"
-                " ret=%d fw_err=%d (%s)", ret, err, fw_error_to_str(err));
-        return NULL;
+          " ret = %d fw_err=%d (%s)", ret, fw_error, fw_error_to_str(fw_error));
     }
 
     report = g_new0(SevAttestationReport, 1);
-    report->data = g_base64_encode(data, input.len);
+    report->data = g_base64_encode(data, data_len);
 
-    trace_kvm_sev_attestation_report(mnonce, report->data);
+    trace_kvm_sev_attestation_report((char *) mnonce, report->data);
 
     return report;
 }
 
-SevAttestationReport *qmp_query_sev_attestation_report(const char *mnonce,
-                                                       Error **errp)
-{
-    return sev_get_attestation_report(mnonce, errp);
-}
-
 static int
 sev_read_file_base64(const char *filename, guchar **data, gsize *len)
 {
diff --git a/target/i386/sev.h b/target/i386/sev.h
index f1af28eca0..a90909450c 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -48,6 +48,8 @@ bool sev_es_enabled(void);
 #define sev_es_enabled() 0
 #endif
 
+#define SEV_ATTESTATION_REPORT_MNONCE_SIZE 16
+
 uint32_t sev_get_cbit_position(void);
 uint32_t sev_get_reduced_phys_bits(void);
 bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp);
-- 
2.40.1

Re: [RFC PATCH v2 1/9] Add Rust SEV library as subproject

[Philippe Mathieu-Daudé]

Hi Tyler,

On 4/10/23 22:34, Tyler Fanelli wrote:
> The Rust sev library provides a C API for the AMD SEV launch ioctls, as
> well as the ability to build with meson. Add the Rust sev library as a
> QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
> APIs provided by it.
> 
> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
> ---
>   meson.build                   | 8 ++++++++
>   meson_options.txt             | 2 ++
>   scripts/meson-buildoptions.sh | 3 +++
>   subprojects/sev.wrap          | 6 ++++++
>   target/i386/meson.build       | 2 +-
>   5 files changed, 20 insertions(+), 1 deletion(-)
>   create mode 100644 subprojects/sev.wrap


> diff --git a/subprojects/sev.wrap b/subprojects/sev.wrap
> new file mode 100644
> index 0000000000..5be1faccf6
> --- /dev/null
> +++ b/subprojects/sev.wrap
> @@ -0,0 +1,6 @@
> +[wrap-git]
> +url = https://github.com/tylerfanelli/sev
> +revision = b81b1da5df50055600a5b0349b0c4afda677cccb

Why use your tree instead of the mainstream one?

Before this gets merged we need to mirror the subproject
on our GitLab namespace, then use the mirror URL here.

Re: [RFC PATCH v2 1/9] Add Rust SEV library as subproject

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 5142 bytes --]

On Wed, Oct 04, 2023 at 04:34:10PM -0400, Tyler Fanelli wrote:
> The Rust sev library provides a C API for the AMD SEV launch ioctls, as
> well as the ability to build with meson. Add the Rust sev library as a
> QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
> APIs provided by it.
> 
> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
> ---
>  meson.build                   | 8 ++++++++
>  meson_options.txt             | 2 ++
>  scripts/meson-buildoptions.sh | 3 +++
>  subprojects/sev.wrap          | 6 ++++++
>  target/i386/meson.build       | 2 +-
>  5 files changed, 20 insertions(+), 1 deletion(-)
>  create mode 100644 subprojects/sev.wrap
> 
> diff --git a/meson.build b/meson.build
> index 20ceeb8158..8a17c29de8 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -960,6 +960,13 @@ if not get_option('slirp').auto() or have_system
>    endif
>  endif
>  
> +sev = not_found
> +if not get_option('sev').auto()

When 'sev' is auto, then it won't be built. That seems strange. The
auto-detection part is missing! I did you test this on a system that
doesn't have libsev installed system-wide?

I guess the auto-detection would look something like:

  cargo = find_program('cargo', required: true)

  if not get_option('sev').auto() or cargo.found()
      ...

That way 'sev' is only built automatically on systems that have cargo
installed.

> +  sev = dependency('sev',
> +                   method: 'pkg-config',
> +                   required: get_option('sev'))
> +endif

If you update the auto logic, see the documentation about fallbacks to
subprojects for optional dependencies:
https://mesonbuild.com/Wrap-dependency-system-manual.html#provide-section

It might be necessary to add dependency(..., fallback='sev').

> +
>  vde = not_found
>  if not get_option('vde').auto() or have_system or have_tools
>    vde = cc.find_library('vdeplug', has_headers: ['libvdeplug.h'],
> @@ -4331,6 +4338,7 @@ summary_info += {'libudev':           libudev}
>  # Dummy dependency, keep .found()
>  summary_info += {'FUSE lseek':        fuse_lseek.found()}
>  summary_info += {'selinux':           selinux}
> +summary_info += {'sev':               sev}
>  summary_info += {'libdw':             libdw}
>  summary(summary_info, bool_yn: true, section: 'Dependencies')
>  
> diff --git a/meson_options.txt b/meson_options.txt
> index 57e265c871..5b8d283717 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -204,6 +204,8 @@ option('sdl_image', type : 'feature', value : 'auto',
>         description: 'SDL Image support for icons')
>  option('seccomp', type : 'feature', value : 'auto',
>         description: 'seccomp support')
> +option('sev', type : 'feature', value : 'auto',
> +        description: 'Rust AMD SEV library')
>  option('smartcard', type : 'feature', value : 'auto',
>         description: 'CA smartcard emulation support')
>  option('snappy', type : 'feature', value : 'auto',
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index e4b46d5715..e585a548fa 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -161,6 +161,7 @@ meson_options_help() {
>    printf "%s\n" '  sdl-image       SDL Image support for icons'
>    printf "%s\n" '  seccomp         seccomp support'
>    printf "%s\n" '  selinux         SELinux support in qemu-nbd'
> +  printf "%s\n" '  sev             SEV library support'
>    printf "%s\n" '  slirp           libslirp user mode network backend support'
>    printf "%s\n" '  slirp-smbd      use smbd (at path --smbd=*) in slirp networking'
>    printf "%s\n" '  smartcard       CA smartcard emulation support'
> @@ -440,6 +441,8 @@ _meson_option_parse() {
>      --disable-seccomp) printf "%s" -Dseccomp=disabled ;;
>      --enable-selinux) printf "%s" -Dselinux=enabled ;;
>      --disable-selinux) printf "%s" -Dselinux=disabled ;;
> +    --enable-sev) printf "%s" -Dsev=enabled ;;
> +    --disable-sev) printf "%s" -Dsev=disabled ;;
>      --enable-slirp) printf "%s" -Dslirp=enabled ;;
>      --disable-slirp) printf "%s" -Dslirp=disabled ;;
>      --enable-slirp-smbd) printf "%s" -Dslirp_smbd=enabled ;;
> diff --git a/subprojects/sev.wrap b/subprojects/sev.wrap
> new file mode 100644
> index 0000000000..5be1faccf6
> --- /dev/null
> +++ b/subprojects/sev.wrap
> @@ -0,0 +1,6 @@
> +[wrap-git]
> +url = https://github.com/tylerfanelli/sev
> +revision = b81b1da5df50055600a5b0349b0c4afda677cccb
> +
> +[provide]
> +sev = sev_dep
> diff --git a/target/i386/meson.build b/target/i386/meson.build
> index 6f1036d469..8972a4fb17 100644
> --- a/target/i386/meson.build
> +++ b/target/i386/meson.build
> @@ -20,7 +20,7 @@ i386_system_ss.add(files(
>    'monitor.c',
>    'cpu-sysemu.c',
>  ))
> -i386_system_ss.add(when: 'CONFIG_SEV', if_true: files('sev.c'), if_false: files('sev-sysemu-stub.c'))
> +i386_system_ss.add(when: 'CONFIG_SEV', if_true: [sev, files('sev.c')], if_false: files('sev-sysemu-stub.c'))
>  
>  i386_user_ss = ss.source_set()
>  
> -- 
> 2.40.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC PATCH v2 1/9] Add Rust SEV library as subproject

[Tyler Fanelli]

On 10/5/23 2:03 AM, Philippe Mathieu-Daudé wrote:
> Hi Tyler,
>
> On 4/10/23 22:34, Tyler Fanelli wrote:
>> The Rust sev library provides a C API for the AMD SEV launch ioctls, as
>> well as the ability to build with meson. Add the Rust sev library as a
>> QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
>> APIs provided by it.
>>
>> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
>> ---
>>   meson.build                   | 8 ++++++++
>>   meson_options.txt             | 2 ++
>>   scripts/meson-buildoptions.sh | 3 +++
>>   subprojects/sev.wrap          | 6 ++++++
>>   target/i386/meson.build       | 2 +-
>>   5 files changed, 20 insertions(+), 1 deletion(-)
>>   create mode 100644 subprojects/sev.wrap
>
>
>> diff --git a/subprojects/sev.wrap b/subprojects/sev.wrap
>> new file mode 100644
>> index 0000000000..5be1faccf6
>> --- /dev/null
>> +++ b/subprojects/sev.wrap
>> @@ -0,0 +1,6 @@
>> +[wrap-git]
>> +url = https://github.com/tylerfanelli/sev
>> +revision = b81b1da5df50055600a5b0349b0c4afda677cccb
>
> Why use your tree instead of the mainstream one?
>
> Before this gets merged we need to mirror the subproject
> on our GitLab namespace, then use the mirror URL here.
>
The required meson changes for the sev library are still in review, so 
I'm still working on a personal branch. Those patches are a blocker for 
this series right now.

This is moreso another RFC to get feedback on building Rust libraries as 
QEMU subprojects (and if this is the proper way to do so).


Tyler

Re: [RFC PATCH v2 1/9] Add Rust SEV library as subproject

[Tyler Fanelli]

On 10/5/23 2:03 AM, Philippe Mathieu-Daudé wrote:
> Hi Tyler,
>
> On 4/10/23 22:34, Tyler Fanelli wrote:
>> The Rust sev library provides a C API for the AMD SEV launch ioctls, as
>> well as the ability to build with meson. Add the Rust sev library as a
>> QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
>> APIs provided by it.
>>
>> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
>> ---
>>   meson.build                   | 8 ++++++++
>>   meson_options.txt             | 2 ++
>>   scripts/meson-buildoptions.sh | 3 +++
>>   subprojects/sev.wrap          | 6 ++++++
>>   target/i386/meson.build       | 2 +-
>>   5 files changed, 20 insertions(+), 1 deletion(-)
>>   create mode 100644 subprojects/sev.wrap
>
>
>> diff --git a/subprojects/sev.wrap b/subprojects/sev.wrap
>> new file mode 100644
>> index 0000000000..5be1faccf6
>> --- /dev/null
>> +++ b/subprojects/sev.wrap
>> @@ -0,0 +1,6 @@
>> +[wrap-git]
>> +url = https://github.com/tylerfanelli/sev
>> +revision = b81b1da5df50055600a5b0349b0c4afda677cccb
>
> Why use your tree instead of the mainstream one?
>
> Before this gets merged we need to mirror the subproject
> on our GitLab namespace, then use the mirror URL here.
>
Hi Philippe,

Why must the subproject be mirrored on qemu's GitLab namespace? With the 
changes being accepted in the upstream sev repository, meson will be 
able to fetch it from there. I see that libblkio (another Rust project) 
is not mirrored in the GitLab namespace [0] (assuming I'm looking in the 
right place) and that meson also fetches it from its upstream repo [1].

[0] https://gitlab.com/qemu-project

[1] 
https://gitlab.com/qemu-project/qemu/-/blob/master/subprojects/libblkio.wrap?ref_type=heads#L2


Tyler

Re: [RFC PATCH v2 1/9] Add Rust SEV library as subproject

[Tyler Fanelli]

On 10/5/23 11:54 AM, Stefan Hajnoczi wrote:
> On Wed, Oct 04, 2023 at 04:34:10PM -0400, Tyler Fanelli wrote:
>> The Rust sev library provides a C API for the AMD SEV launch ioctls, as
>> well as the ability to build with meson. Add the Rust sev library as a
>> QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
>> APIs provided by it.
>>
>> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
>> ---
>>   meson.build                   | 8 ++++++++
>>   meson_options.txt             | 2 ++
>>   scripts/meson-buildoptions.sh | 3 +++
>>   subprojects/sev.wrap          | 6 ++++++
>>   target/i386/meson.build       | 2 +-
>>   5 files changed, 20 insertions(+), 1 deletion(-)
>>   create mode 100644 subprojects/sev.wrap
>>
>> diff --git a/meson.build b/meson.build
>> index 20ceeb8158..8a17c29de8 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -960,6 +960,13 @@ if not get_option('slirp').auto() or have_system
>>     endif
>>   endif
>>   
>> +sev = not_found
>> +if not get_option('sev').auto()
> When 'sev' is auto, then it won't be built. That seems strange. The
> auto-detection part is missing! I did you test this on a system that
> doesn't have libsev installed system-wide?

My testing environment had libsev installed system-wide. Thanks for 
pointing this out.

>
> I guess the auto-detection would look something like:
>
>    cargo = find_program('cargo', required: true)
>
>    if not get_option('sev').auto() or cargo.found()
>        ...
>
> That way 'sev' is only built automatically on systems that have cargo
> installed.
>
>> +  sev = dependency('sev',
>> +                   method: 'pkg-config',
>> +                   required: get_option('sev'))
>> +endif
> If you update the auto logic, see the documentation about fallbacks to
> subprojects for optional dependencies:
> https://mesonbuild.com/Wrap-dependency-system-manual.html#provide-section
>
> It might be necessary to add dependency(..., fallback='sev').

Noted. Thanks!

>
>> +
>>   vde = not_found
>>   if not get_option('vde').auto() or have_system or have_tools
>>     vde = cc.find_library('vdeplug', has_headers: ['libvdeplug.h'],
>> @@ -4331,6 +4338,7 @@ summary_info += {'libudev':           libudev}
>>   # Dummy dependency, keep .found()
>>   summary_info += {'FUSE lseek':        fuse_lseek.found()}
>>   summary_info += {'selinux':           selinux}
>> +summary_info += {'sev':               sev}
>>   summary_info += {'libdw':             libdw}
>>   summary(summary_info, bool_yn: true, section: 'Dependencies')
>>   
>> diff --git a/meson_options.txt b/meson_options.txt
>> index 57e265c871..5b8d283717 100644
>> --- a/meson_options.txt
>> +++ b/meson_options.txt
>> @@ -204,6 +204,8 @@ option('sdl_image', type : 'feature', value : 'auto',
>>          description: 'SDL Image support for icons')
>>   option('seccomp', type : 'feature', value : 'auto',
>>          description: 'seccomp support')
>> +option('sev', type : 'feature', value : 'auto',
>> +        description: 'Rust AMD SEV library')
>>   option('smartcard', type : 'feature', value : 'auto',
>>          description: 'CA smartcard emulation support')
>>   option('snappy', type : 'feature', value : 'auto',
>> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
>> index e4b46d5715..e585a548fa 100644
>> --- a/scripts/meson-buildoptions.sh
>> +++ b/scripts/meson-buildoptions.sh
>> @@ -161,6 +161,7 @@ meson_options_help() {
>>     printf "%s\n" '  sdl-image       SDL Image support for icons'
>>     printf "%s\n" '  seccomp         seccomp support'
>>     printf "%s\n" '  selinux         SELinux support in qemu-nbd'
>> +  printf "%s\n" '  sev             SEV library support'
>>     printf "%s\n" '  slirp           libslirp user mode network backend support'
>>     printf "%s\n" '  slirp-smbd      use smbd (at path --smbd=*) in slirp networking'
>>     printf "%s\n" '  smartcard       CA smartcard emulation support'
>> @@ -440,6 +441,8 @@ _meson_option_parse() {
>>       --disable-seccomp) printf "%s" -Dseccomp=disabled ;;
>>       --enable-selinux) printf "%s" -Dselinux=enabled ;;
>>       --disable-selinux) printf "%s" -Dselinux=disabled ;;
>> +    --enable-sev) printf "%s" -Dsev=enabled ;;
>> +    --disable-sev) printf "%s" -Dsev=disabled ;;
>>       --enable-slirp) printf "%s" -Dslirp=enabled ;;
>>       --disable-slirp) printf "%s" -Dslirp=disabled ;;
>>       --enable-slirp-smbd) printf "%s" -Dslirp_smbd=enabled ;;
>> diff --git a/subprojects/sev.wrap b/subprojects/sev.wrap
>> new file mode 100644
>> index 0000000000..5be1faccf6
>> --- /dev/null
>> +++ b/subprojects/sev.wrap
>> @@ -0,0 +1,6 @@
>> +[wrap-git]
>> +url = https://github.com/tylerfanelli/sev
>> +revision = b81b1da5df50055600a5b0349b0c4afda677cccb
>> +
>> +[provide]
>> +sev = sev_dep
>> diff --git a/target/i386/meson.build b/target/i386/meson.build
>> index 6f1036d469..8972a4fb17 100644
>> --- a/target/i386/meson.build
>> +++ b/target/i386/meson.build
>> @@ -20,7 +20,7 @@ i386_system_ss.add(files(
>>     'monitor.c',
>>     'cpu-sysemu.c',
>>   ))
>> -i386_system_ss.add(when: 'CONFIG_SEV', if_true: files('sev.c'), if_false: files('sev-sysemu-stub.c'))
>> +i386_system_ss.add(when: 'CONFIG_SEV', if_true: [sev, files('sev.c')], if_false: files('sev-sysemu-stub.c'))
>>   
>>   i386_user_ss = ss.source_set()
>>   
>> -- 
>> 2.40.1
>>
Tyler

Re: [RFC PATCH v2 1/9] Add Rust SEV library as subproject

[Manos Pitsidianakis]

Hello Tyler!

With Rust stable 1.72.1, I get:

error: unneeded `return` statement
   --> tests/launch.rs:103:26
    |
103 |         VcpuExit::Hlt => return,
    |                          ^^^^^^
    |
    = help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
    = note: `-D clippy::needless-return` implied by `-D warnings`
help: replace `return` with a unit value
    |
103 |         VcpuExit::Hlt => (),
    |                          ~~

error: could not compile `sev` (test "launch") due to previous error
warning: build failed, waiting for other jobs to finish...


When doing make.

--
Manos

On Wed, 11 Oct 2023 at 06:11, Tyler Fanelli <tfanelli@redhat.com> wrote:
>
> On 10/5/23 11:54 AM, Stefan Hajnoczi wrote:
> > On Wed, Oct 04, 2023 at 04:34:10PM -0400, Tyler Fanelli wrote:
> >> The Rust sev library provides a C API for the AMD SEV launch ioctls, as
> >> well as the ability to build with meson. Add the Rust sev library as a
> >> QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
> >> APIs provided by it.
> >>
> >> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
> >> ---
> >>   meson.build                   | 8 ++++++++
> >>   meson_options.txt             | 2 ++
> >>   scripts/meson-buildoptions.sh | 3 +++
> >>   subprojects/sev.wrap          | 6 ++++++
> >>   target/i386/meson.build       | 2 +-
> >>   5 files changed, 20 insertions(+), 1 deletion(-)
> >>   create mode 100644 subprojects/sev.wrap
> >>
> >> diff --git a/meson.build b/meson.build
> >> index 20ceeb8158..8a17c29de8 100644
> >> --- a/meson.build
> >> +++ b/meson.build
> >> @@ -960,6 +960,13 @@ if not get_option('slirp').auto() or have_system
> >>     endif
> >>   endif
> >>
> >> +sev = not_found
> >> +if not get_option('sev').auto()
> > When 'sev' is auto, then it won't be built. That seems strange. The
> > auto-detection part is missing! I did you test this on a system that
> > doesn't have libsev installed system-wide?
>
> My testing environment had libsev installed system-wide. Thanks for
> pointing this out.
>
> >
> > I guess the auto-detection would look something like:
> >
> >    cargo = find_program('cargo', required: true)
> >
> >    if not get_option('sev').auto() or cargo.found()
> >        ...
> >
> > That way 'sev' is only built automatically on systems that have cargo
> > installed.
> >
> >> +  sev = dependency('sev',
> >> +                   method: 'pkg-config',
> >> +                   required: get_option('sev'))
> >> +endif
> > If you update the auto logic, see the documentation about fallbacks to
> > subprojects for optional dependencies:
> > https://mesonbuild.com/Wrap-dependency-system-manual.html#provide-section
> >
> > It might be necessary to add dependency(..., fallback='sev').
>
> Noted. Thanks!
>
> >
> >> +
> >>   vde = not_found
> >>   if not get_option('vde').auto() or have_system or have_tools
> >>     vde = cc.find_library('vdeplug', has_headers: ['libvdeplug.h'],
> >> @@ -4331,6 +4338,7 @@ summary_info += {'libudev':           libudev}
> >>   # Dummy dependency, keep .found()
> >>   summary_info += {'FUSE lseek':        fuse_lseek.found()}
> >>   summary_info += {'selinux':           selinux}
> >> +summary_info += {'sev':               sev}
> >>   summary_info += {'libdw':             libdw}
> >>   summary(summary_info, bool_yn: true, section: 'Dependencies')
> >>
> >> diff --git a/meson_options.txt b/meson_options.txt
> >> index 57e265c871..5b8d283717 100644
> >> --- a/meson_options.txt
> >> +++ b/meson_options.txt
> >> @@ -204,6 +204,8 @@ option('sdl_image', type : 'feature', value : 'auto',
> >>          description: 'SDL Image support for icons')
> >>   option('seccomp', type : 'feature', value : 'auto',
> >>          description: 'seccomp support')
> >> +option('sev', type : 'feature', value : 'auto',
> >> +        description: 'Rust AMD SEV library')
> >>   option('smartcard', type : 'feature', value : 'auto',
> >>          description: 'CA smartcard emulation support')
> >>   option('snappy', type : 'feature', value : 'auto',
> >> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> >> index e4b46d5715..e585a548fa 100644
> >> --- a/scripts/meson-buildoptions.sh
> >> +++ b/scripts/meson-buildoptions.sh
> >> @@ -161,6 +161,7 @@ meson_options_help() {
> >>     printf "%s\n" '  sdl-image       SDL Image support for icons'
> >>     printf "%s\n" '  seccomp         seccomp support'
> >>     printf "%s\n" '  selinux         SELinux support in qemu-nbd'
> >> +  printf "%s\n" '  sev             SEV library support'
> >>     printf "%s\n" '  slirp           libslirp user mode network backend support'
> >>     printf "%s\n" '  slirp-smbd      use smbd (at path --smbd=*) in slirp networking'
> >>     printf "%s\n" '  smartcard       CA smartcard emulation support'
> >> @@ -440,6 +441,8 @@ _meson_option_parse() {
> >>       --disable-seccomp) printf "%s" -Dseccomp=disabled ;;
> >>       --enable-selinux) printf "%s" -Dselinux=enabled ;;
> >>       --disable-selinux) printf "%s" -Dselinux=disabled ;;
> >> +    --enable-sev) printf "%s" -Dsev=enabled ;;
> >> +    --disable-sev) printf "%s" -Dsev=disabled ;;
> >>       --enable-slirp) printf "%s" -Dslirp=enabled ;;
> >>       --disable-slirp) printf "%s" -Dslirp=disabled ;;
> >>       --enable-slirp-smbd) printf "%s" -Dslirp_smbd=enabled ;;
> >> diff --git a/subprojects/sev.wrap b/subprojects/sev.wrap
> >> new file mode 100644
> >> index 0000000000..5be1faccf6
> >> --- /dev/null
> >> +++ b/subprojects/sev.wrap
> >> @@ -0,0 +1,6 @@
> >> +[wrap-git]
> >> +url = https://github.com/tylerfanelli/sev
> >> +revision = b81b1da5df50055600a5b0349b0c4afda677cccb
> >> +
> >> +[provide]
> >> +sev = sev_dep
> >> diff --git a/target/i386/meson.build b/target/i386/meson.build
> >> index 6f1036d469..8972a4fb17 100644
> >> --- a/target/i386/meson.build
> >> +++ b/target/i386/meson.build
> >> @@ -20,7 +20,7 @@ i386_system_ss.add(files(
> >>     'monitor.c',
> >>     'cpu-sysemu.c',
> >>   ))
> >> -i386_system_ss.add(when: 'CONFIG_SEV', if_true: files('sev.c'), if_false: files('sev-sysemu-stub.c'))
> >> +i386_system_ss.add(when: 'CONFIG_SEV', if_true: [sev, files('sev.c')], if_false: files('sev-sysemu-stub.c'))
> >>
> >>   i386_user_ss = ss.source_set()
> >>
> >> --
> >> 2.40.1
> >>
> Tyler
>
>

Re: [RFC PATCH v2 1/9] Add Rust SEV library as subproject

[Tyler Fanelli]

Hi Manos,

Thanks for the heads up, I was using rust 1.71.1. Will update the series 
with 1.72.1

Stefan, Philippe, or Daniel: is there a specific policy for the Rust 
version we should be developing on for crates in qemu?

Tyler

On 10/13/23 2:09 PM, Manos Pitsidianakis wrote:
> Hello Tyler!
>
> With Rust stable 1.72.1, I get:
>
> error: unneeded `return` statement
>     --> tests/launch.rs:103:26
>      |
> 103 |         VcpuExit::Hlt => return,
>      |                          ^^^^^^
>      |
>      = help: for further information visit
> https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
>      = note: `-D clippy::needless-return` implied by `-D warnings`
> help: replace `return` with a unit value
>      |
> 103 |         VcpuExit::Hlt => (),
>      |                          ~~
>
> error: could not compile `sev` (test "launch") due to previous error
> warning: build failed, waiting for other jobs to finish...
>
>
> When doing make.
>
> --
> Manos
>
> On Wed, 11 Oct 2023 at 06:11, Tyler Fanelli <tfanelli@redhat.com> wrote:
>> On 10/5/23 11:54 AM, Stefan Hajnoczi wrote:
>>> On Wed, Oct 04, 2023 at 04:34:10PM -0400, Tyler Fanelli wrote:
>>>> The Rust sev library provides a C API for the AMD SEV launch ioctls, as
>>>> well as the ability to build with meson. Add the Rust sev library as a
>>>> QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
>>>> APIs provided by it.
>>>>
>>>> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
>>>> ---
>>>>    meson.build                   | 8 ++++++++
>>>>    meson_options.txt             | 2 ++
>>>>    scripts/meson-buildoptions.sh | 3 +++
>>>>    subprojects/sev.wrap          | 6 ++++++
>>>>    target/i386/meson.build       | 2 +-
>>>>    5 files changed, 20 insertions(+), 1 deletion(-)
>>>>    create mode 100644 subprojects/sev.wrap
>>>>
>>>> diff --git a/meson.build b/meson.build
>>>> index 20ceeb8158..8a17c29de8 100644
>>>> --- a/meson.build
>>>> +++ b/meson.build
>>>> @@ -960,6 +960,13 @@ if not get_option('slirp').auto() or have_system
>>>>      endif
>>>>    endif
>>>>
>>>> +sev = not_found
>>>> +if not get_option('sev').auto()
>>> When 'sev' is auto, then it won't be built. That seems strange. The
>>> auto-detection part is missing! I did you test this on a system that
>>> doesn't have libsev installed system-wide?
>> My testing environment had libsev installed system-wide. Thanks for
>> pointing this out.
>>
>>> I guess the auto-detection would look something like:
>>>
>>>     cargo = find_program('cargo', required: true)
>>>
>>>     if not get_option('sev').auto() or cargo.found()
>>>         ...
>>>
>>> That way 'sev' is only built automatically on systems that have cargo
>>> installed.
>>>
>>>> +  sev = dependency('sev',
>>>> +                   method: 'pkg-config',
>>>> +                   required: get_option('sev'))
>>>> +endif
>>> If you update the auto logic, see the documentation about fallbacks to
>>> subprojects for optional dependencies:
>>> https://mesonbuild.com/Wrap-dependency-system-manual.html#provide-section
>>>
>>> It might be necessary to add dependency(..., fallback='sev').
>> Noted. Thanks!
>>
>>>> +
>>>>    vde = not_found
>>>>    if not get_option('vde').auto() or have_system or have_tools
>>>>      vde = cc.find_library('vdeplug', has_headers: ['libvdeplug.h'],
>>>> @@ -4331,6 +4338,7 @@ summary_info += {'libudev':           libudev}
>>>>    # Dummy dependency, keep .found()
>>>>    summary_info += {'FUSE lseek':        fuse_lseek.found()}
>>>>    summary_info += {'selinux':           selinux}
>>>> +summary_info += {'sev':               sev}
>>>>    summary_info += {'libdw':             libdw}
>>>>    summary(summary_info, bool_yn: true, section: 'Dependencies')
>>>>
>>>> diff --git a/meson_options.txt b/meson_options.txt
>>>> index 57e265c871..5b8d283717 100644
>>>> --- a/meson_options.txt
>>>> +++ b/meson_options.txt
>>>> @@ -204,6 +204,8 @@ option('sdl_image', type : 'feature', value : 'auto',
>>>>           description: 'SDL Image support for icons')
>>>>    option('seccomp', type : 'feature', value : 'auto',
>>>>           description: 'seccomp support')
>>>> +option('sev', type : 'feature', value : 'auto',
>>>> +        description: 'Rust AMD SEV library')
>>>>    option('smartcard', type : 'feature', value : 'auto',
>>>>           description: 'CA smartcard emulation support')
>>>>    option('snappy', type : 'feature', value : 'auto',
>>>> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
>>>> index e4b46d5715..e585a548fa 100644
>>>> --- a/scripts/meson-buildoptions.sh
>>>> +++ b/scripts/meson-buildoptions.sh
>>>> @@ -161,6 +161,7 @@ meson_options_help() {
>>>>      printf "%s\n" '  sdl-image       SDL Image support for icons'
>>>>      printf "%s\n" '  seccomp         seccomp support'
>>>>      printf "%s\n" '  selinux         SELinux support in qemu-nbd'
>>>> +  printf "%s\n" '  sev             SEV library support'
>>>>      printf "%s\n" '  slirp           libslirp user mode network backend support'
>>>>      printf "%s\n" '  slirp-smbd      use smbd (at path --smbd=*) in slirp networking'
>>>>      printf "%s\n" '  smartcard       CA smartcard emulation support'
>>>> @@ -440,6 +441,8 @@ _meson_option_parse() {
>>>>        --disable-seccomp) printf "%s" -Dseccomp=disabled ;;
>>>>        --enable-selinux) printf "%s" -Dselinux=enabled ;;
>>>>        --disable-selinux) printf "%s" -Dselinux=disabled ;;
>>>> +    --enable-sev) printf "%s" -Dsev=enabled ;;
>>>> +    --disable-sev) printf "%s" -Dsev=disabled ;;
>>>>        --enable-slirp) printf "%s" -Dslirp=enabled ;;
>>>>        --disable-slirp) printf "%s" -Dslirp=disabled ;;
>>>>        --enable-slirp-smbd) printf "%s" -Dslirp_smbd=enabled ;;
>>>> diff --git a/subprojects/sev.wrap b/subprojects/sev.wrap
>>>> new file mode 100644
>>>> index 0000000000..5be1faccf6
>>>> --- /dev/null
>>>> +++ b/subprojects/sev.wrap
>>>> @@ -0,0 +1,6 @@
>>>> +[wrap-git]
>>>> +url = https://github.com/tylerfanelli/sev
>>>> +revision = b81b1da5df50055600a5b0349b0c4afda677cccb
>>>> +
>>>> +[provide]
>>>> +sev = sev_dep
>>>> diff --git a/target/i386/meson.build b/target/i386/meson.build
>>>> index 6f1036d469..8972a4fb17 100644
>>>> --- a/target/i386/meson.build
>>>> +++ b/target/i386/meson.build
>>>> @@ -20,7 +20,7 @@ i386_system_ss.add(files(
>>>>      'monitor.c',
>>>>      'cpu-sysemu.c',
>>>>    ))
>>>> -i386_system_ss.add(when: 'CONFIG_SEV', if_true: files('sev.c'), if_false: files('sev-sysemu-stub.c'))
>>>> +i386_system_ss.add(when: 'CONFIG_SEV', if_true: [sev, files('sev.c')], if_false: files('sev-sysemu-stub.c'))
>>>>
>>>>    i386_user_ss = ss.source_set()
>>>>
>>>> --
>>>> 2.40.1
>>>>
>> Tyler
>>
>>

Re: [RFC PATCH v2 1/9] Add Rust SEV library as subproject

[Daniel P. Berrangé]

On Fri, Oct 13, 2023 at 02:20:16PM -0400, Tyler Fanelli wrote:
> Hi Manos,
> 
> Thanks for the heads up, I was using rust 1.71.1. Will update the series
> with 1.72.1
> 
> Stefan, Philippe, or Daniel: is there a specific policy for the Rust version
> we should be developing on for crates in qemu?

There are a couple of dimensions to this.

First is the matter of what operating system and architecture pairs are
supported as targets for the Rust toolchain, and the standard library.
We'll need both to work of course.

Second there is the matter of what versions of Rust are shipped in the
various operating systems currently.

In a previous discussion there was a wiki page fleshed out with this
info:

   https://wiki.qemu.org/RustInQemu

but the min versions are certainly out of date now.

Third there is the question of whether distros have facility for pulling
in newer toolchain versions, and if so should we be willing to use them.
This is relevant for the long life distros like RHEL, which might ship
with a variety of Rust versions. Historically we've been very conservative
but with Python last year we adopted a more aggressive policy of being
willing to take any newer version available from the distro vendor, not
merely the oldest baseline. I suspect we'll want a similar approach with
rust.

Anyhow, I think you could probably start by updatnig that RustInQemu
wiki page so that it reflects the current state of the world in terms
of support tiers and versions.

> 
> Tyler
> 
> On 10/13/23 2:09 PM, Manos Pitsidianakis wrote:
> > Hello Tyler!
> > 
> > With Rust stable 1.72.1, I get:
> > 
> > error: unneeded `return` statement
> >     --> tests/launch.rs:103:26
> >      |
> > 103 |         VcpuExit::Hlt => return,
> >      |                          ^^^^^^
> >      |
> >      = help: for further information visit
> > https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
> >      = note: `-D clippy::needless-return` implied by `-D warnings`
> > help: replace `return` with a unit value
> >      |
> > 103 |         VcpuExit::Hlt => (),
> >      |                          ~~
> > 
> > error: could not compile `sev` (test "launch") due to previous error
> > warning: build failed, waiting for other jobs to finish...
> > 
> > 
> > When doing make.
> > 
> > --
> > Manos
> > 
> > On Wed, 11 Oct 2023 at 06:11, Tyler Fanelli <tfanelli@redhat.com> wrote:
> > > On 10/5/23 11:54 AM, Stefan Hajnoczi wrote:
> > > > On Wed, Oct 04, 2023 at 04:34:10PM -0400, Tyler Fanelli wrote:
> > > > > The Rust sev library provides a C API for the AMD SEV launch ioctls, as
> > > > > well as the ability to build with meson. Add the Rust sev library as a
> > > > > QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
> > > > > APIs provided by it.
> > > > > 
> > > > > Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
> > > > > ---
> > > > >    meson.build                   | 8 ++++++++
> > > > >    meson_options.txt             | 2 ++
> > > > >    scripts/meson-buildoptions.sh | 3 +++
> > > > >    subprojects/sev.wrap          | 6 ++++++
> > > > >    target/i386/meson.build       | 2 +-
> > > > >    5 files changed, 20 insertions(+), 1 deletion(-)
> > > > >    create mode 100644 subprojects/sev.wrap
> > > > > 
> > > > > diff --git a/meson.build b/meson.build
> > > > > index 20ceeb8158..8a17c29de8 100644
> > > > > --- a/meson.build
> > > > > +++ b/meson.build
> > > > > @@ -960,6 +960,13 @@ if not get_option('slirp').auto() or have_system
> > > > >      endif
> > > > >    endif
> > > > > 
> > > > > +sev = not_found
> > > > > +if not get_option('sev').auto()
> > > > When 'sev' is auto, then it won't be built. That seems strange. The
> > > > auto-detection part is missing! I did you test this on a system that
> > > > doesn't have libsev installed system-wide?
> > > My testing environment had libsev installed system-wide. Thanks for
> > > pointing this out.
> > > 
> > > > I guess the auto-detection would look something like:
> > > > 
> > > >     cargo = find_program('cargo', required: true)
> > > > 
> > > >     if not get_option('sev').auto() or cargo.found()
> > > >         ...
> > > > 
> > > > That way 'sev' is only built automatically on systems that have cargo
> > > > installed.
> > > > 
> > > > > +  sev = dependency('sev',
> > > > > +                   method: 'pkg-config',
> > > > > +                   required: get_option('sev'))
> > > > > +endif
> > > > If you update the auto logic, see the documentation about fallbacks to
> > > > subprojects for optional dependencies:
> > > > https://mesonbuild.com/Wrap-dependency-system-manual.html#provide-section
> > > > 
> > > > It might be necessary to add dependency(..., fallback='sev').
> > > Noted. Thanks!
> > > 
> > > > > +
> > > > >    vde = not_found
> > > > >    if not get_option('vde').auto() or have_system or have_tools
> > > > >      vde = cc.find_library('vdeplug', has_headers: ['libvdeplug.h'],
> > > > > @@ -4331,6 +4338,7 @@ summary_info += {'libudev':           libudev}
> > > > >    # Dummy dependency, keep .found()
> > > > >    summary_info += {'FUSE lseek':        fuse_lseek.found()}
> > > > >    summary_info += {'selinux':           selinux}
> > > > > +summary_info += {'sev':               sev}
> > > > >    summary_info += {'libdw':             libdw}
> > > > >    summary(summary_info, bool_yn: true, section: 'Dependencies')
> > > > > 
> > > > > diff --git a/meson_options.txt b/meson_options.txt
> > > > > index 57e265c871..5b8d283717 100644
> > > > > --- a/meson_options.txt
> > > > > +++ b/meson_options.txt
> > > > > @@ -204,6 +204,8 @@ option('sdl_image', type : 'feature', value : 'auto',
> > > > >           description: 'SDL Image support for icons')
> > > > >    option('seccomp', type : 'feature', value : 'auto',
> > > > >           description: 'seccomp support')
> > > > > +option('sev', type : 'feature', value : 'auto',
> > > > > +        description: 'Rust AMD SEV library')
> > > > >    option('smartcard', type : 'feature', value : 'auto',
> > > > >           description: 'CA smartcard emulation support')
> > > > >    option('snappy', type : 'feature', value : 'auto',
> > > > > diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> > > > > index e4b46d5715..e585a548fa 100644
> > > > > --- a/scripts/meson-buildoptions.sh
> > > > > +++ b/scripts/meson-buildoptions.sh
> > > > > @@ -161,6 +161,7 @@ meson_options_help() {
> > > > >      printf "%s\n" '  sdl-image       SDL Image support for icons'
> > > > >      printf "%s\n" '  seccomp         seccomp support'
> > > > >      printf "%s\n" '  selinux         SELinux support in qemu-nbd'
> > > > > +  printf "%s\n" '  sev             SEV library support'
> > > > >      printf "%s\n" '  slirp           libslirp user mode network backend support'
> > > > >      printf "%s\n" '  slirp-smbd      use smbd (at path --smbd=*) in slirp networking'
> > > > >      printf "%s\n" '  smartcard       CA smartcard emulation support'
> > > > > @@ -440,6 +441,8 @@ _meson_option_parse() {
> > > > >        --disable-seccomp) printf "%s" -Dseccomp=disabled ;;
> > > > >        --enable-selinux) printf "%s" -Dselinux=enabled ;;
> > > > >        --disable-selinux) printf "%s" -Dselinux=disabled ;;
> > > > > +    --enable-sev) printf "%s" -Dsev=enabled ;;
> > > > > +    --disable-sev) printf "%s" -Dsev=disabled ;;
> > > > >        --enable-slirp) printf "%s" -Dslirp=enabled ;;
> > > > >        --disable-slirp) printf "%s" -Dslirp=disabled ;;
> > > > >        --enable-slirp-smbd) printf "%s" -Dslirp_smbd=enabled ;;
> > > > > diff --git a/subprojects/sev.wrap b/subprojects/sev.wrap
> > > > > new file mode 100644
> > > > > index 0000000000..5be1faccf6
> > > > > --- /dev/null
> > > > > +++ b/subprojects/sev.wrap
> > > > > @@ -0,0 +1,6 @@
> > > > > +[wrap-git]
> > > > > +url = https://github.com/tylerfanelli/sev
> > > > > +revision = b81b1da5df50055600a5b0349b0c4afda677cccb
> > > > > +
> > > > > +[provide]
> > > > > +sev = sev_dep
> > > > > diff --git a/target/i386/meson.build b/target/i386/meson.build
> > > > > index 6f1036d469..8972a4fb17 100644
> > > > > --- a/target/i386/meson.build
> > > > > +++ b/target/i386/meson.build
> > > > > @@ -20,7 +20,7 @@ i386_system_ss.add(files(
> > > > >      'monitor.c',
> > > > >      'cpu-sysemu.c',
> > > > >    ))
> > > > > -i386_system_ss.add(when: 'CONFIG_SEV', if_true: files('sev.c'), if_false: files('sev-sysemu-stub.c'))
> > > > > +i386_system_ss.add(when: 'CONFIG_SEV', if_true: [sev, files('sev.c')], if_false: files('sev-sysemu-stub.c'))
> > > > > 
> > > > >    i386_user_ss = ss.source_set()
> > > > > 
> > > > > --
> > > > > 2.40.1
> > > > > 
> > > Tyler
> > > 
> > > 
> 

With 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 :|

Re: [RFC PATCH v2 1/9] Add Rust SEV library as subproject

[Philippe Mathieu-Daudé]

On 16/10/23 11:16, Daniel P. Berrangé wrote:
> On Fri, Oct 13, 2023 at 02:20:16PM -0400, Tyler Fanelli wrote:
>> Hi Manos,
>>
>> Thanks for the heads up, I was using rust 1.71.1. Will update the series
>> with 1.72.1
>>
>> Stefan, Philippe, or Daniel: is there a specific policy for the Rust version
>> we should be developing on for crates in qemu?
> 
> There are a couple of dimensions to this.
> 
> First is the matter of what operating system and architecture pairs are
> supported as targets for the Rust toolchain, and the standard library.
> We'll need both to work of course.
> 
> Second there is the matter of what versions of Rust are shipped in the
> various operating systems currently.
> 
> In a previous discussion there was a wiki page fleshed out with this
> info:
> 
>     https://wiki.qemu.org/RustInQemu
> 
> but the min versions are certainly out of date now.
> 
> Third there is the question of whether distros have facility for pulling
> in newer toolchain versions, and if so should we be willing to use them.
> This is relevant for the long life distros like RHEL, which might ship
> with a variety of Rust versions. Historically we've been very conservative
> but with Python last year we adopted a more aggressive policy of being
> willing to take any newer version available from the distro vendor, not
> merely the oldest baseline. I suspect we'll want a similar approach with
> rust.

Cc'ing distrib package maintainers to have their feedback on this.

> Anyhow, I think you could probably start by updatnig that RustInQemu
> wiki page so that it reflects the current state of the world in terms
> of support tiers and versions.

Re: [RFC PATCH v2 1/9] Add Rust SEV library as subproject

[Stefan Hajnoczi]

On Mon, 16 Oct 2023 at 05:17, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Fri, Oct 13, 2023 at 02:20:16PM -0400, Tyler Fanelli wrote:
> > Hi Manos,
> >
> > Thanks for the heads up, I was using rust 1.71.1. Will update the series
> > with 1.72.1
> >
> > Stefan, Philippe, or Daniel: is there a specific policy for the Rust version
> > we should be developing on for crates in qemu?
>
> There are a couple of dimensions to this.
>
> First is the matter of what operating system and architecture pairs are
> supported as targets for the Rust toolchain, and the standard library.
> We'll need both to work of course.
>
> Second there is the matter of what versions of Rust are shipped in the
> various operating systems currently.
>
> In a previous discussion there was a wiki page fleshed out with this
> info:
>
>    https://wiki.qemu.org/RustInQemu
>
> but the min versions are certainly out of date now.
>
> Third there is the question of whether distros have facility for pulling
> in newer toolchain versions, and if so should we be willing to use them.
> This is relevant for the long life distros like RHEL, which might ship
> with a variety of Rust versions. Historically we've been very conservative
> but with Python last year we adopted a more aggressive policy of being
> willing to take any newer version available from the distro vendor, not
> merely the oldest baseline. I suspect we'll want a similar approach with
> rust.
>
> Anyhow, I think you could probably start by updatnig that RustInQemu
> wiki page so that it reflects the current state of the world in terms
> of support tiers and versions.

I have two specific scenarios in mind that should build successfully:
1. On the oldest operating system version supported by QEMU where Rust
code previously built successfully. In other words, once QEMU Rust
code starts building, it keeps building on that operating system
version with the distro's Rust toolchain until QEMU increases the
minimum supported distro version. The rationale here is for both
end-users that build from source and for distro packagers to be able
to build QEMU easily.
2. On the latest Rust stable toolchain from rustup. The rationale is
that developers often use rustup instead of the distro toolchain, so
it's nice to support it as a convenience.

Stefan

Re: [RFC PATCH v2 1/9] Add Rust SEV library as subproject

[Daniel P. Berrangé]

On Wed, Oct 04, 2023 at 04:34:10PM -0400, Tyler Fanelli wrote:
> The Rust sev library provides a C API for the AMD SEV launch ioctls, as
> well as the ability to build with meson. Add the Rust sev library as a
> QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
> APIs provided by it.
> 
> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
> ---
>  meson.build                   | 8 ++++++++
>  meson_options.txt             | 2 ++
>  scripts/meson-buildoptions.sh | 3 +++
>  subprojects/sev.wrap          | 6 ++++++
>  target/i386/meson.build       | 2 +-
>  5 files changed, 20 insertions(+), 1 deletion(-)
>  create mode 100644 subprojects/sev.wrap
> 
> diff --git a/meson.build b/meson.build
> index 20ceeb8158..8a17c29de8 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -960,6 +960,13 @@ if not get_option('slirp').auto() or have_system
>    endif
>  endif
>  
> +sev = not_found
> +if not get_option('sev').auto()
> +  sev = dependency('sev',
> +                   method: 'pkg-config',
> +                   required: get_option('sev'))
> +endif
> +

I've revisited this series and tested it now. As Stefan already
mentioned, this logic is flawed.

Currently QEMU is self-contained for SEV support. If we swap to the
sev crate, then we introduce libsev.so as a build time system library
dependancy, and it is highly unlikely that many existing distros will
add the package. IOW we'll cause a regression for users.

Thus we need to be able to *statically* link to the sev crate when it
is not available in the system.

I had a crack at changing this patch to support that and came up with
this diff on top of your patch here:

diff --git a/meson.build b/meson.build
index 1beb9e9f40..d6aba3fd7d 100644
--- a/meson.build
+++ b/meson.build
@@ -1116,12 +1116,6 @@ if not get_option('slirp').auto() or have_system
   endif
 endif
 
-sev = not_found
-if not get_option('sev').auto()
-  sev = dependency('sev',
-                   method: 'pkg-config',
-                   required: get_option('sev'))
-endif
 
 vde = not_found
 if not get_option('vde').auto() or have_system or have_tools
@@ -3003,6 +2997,7 @@ ignored = [ 'TARGET_XML_FILES', 'TARGET_ABI_DIR', 'TARGET_ARCH' ]
 default_targets = 'CONFIG_DEFAULT_TARGETS' in config_host
 actual_target_dirs = []
 fdt_required = []
+sev_required = []
 foreach target : target_dirs
   config_target = { 'TARGET_NAME': target.split('-')[0] }
   if target.endswith('linux-user')
@@ -3124,6 +3119,9 @@ foreach target : target_dirs
     foreach k, v: config_devices
       config_devices_data.set(k, 1)
     endforeach
+    if 'CONFIG_SEV' in config_devices
+        sev_required += target
+    endif
     config_devices_mak_list += config_devices_mak
     config_devices_h += {target: configure_file(output: target + '-config-devices.h',
                                                 configuration: config_devices_data)}
@@ -3206,6 +3204,39 @@ if have_libvduse
   libvduse = libvduse_proj.get_variable('libvduse_dep')
 endif
 
+sev = not_found
+sev_opt = get_option('sev')
+if sev_required.length() > 0 or sev_opt == 'enabled'
+  if sev_opt == 'disabled'
+    error('sev disabled but required by targets ' + ', '.join(fdt_required))
+  endif
+
+  if sev_opt in ['enabled', 'auto', 'system']
+      if get_option('wrap_mode') == 'nodownload'
+          sev_opt = 'system'
+      endif
+      sev = dependency('sev',
+                       method: 'pkg-config',
+                       required: sev_opt == 'system')
+      if sev.found()
+          sev_opt = 'system'
+      elif sev_opt == 'system'
+          error('system libsev requested')
+      else
+          sev_opt = 'internal'
+          sev = not_found
+      endif
+  endif
+  if not sev.found()
+      assert(sev_opt == 'internal')
+      libsev_proj = subproject('sev', required: true,
+                               default_options: ['default_library=static'])
+      sev = libsev_proj.get_variable('sev_dep')
+  endif
+else
+  sev_opt = 'disabled'
+endif
+
 #####################
 # Generated sources #
 #####################
@@ -4453,7 +4484,7 @@ summary_info += {'libudev':           libudev}
 # Dummy dependency, keep .found()
 summary_info += {'FUSE lseek':        fuse_lseek.found()}
 summary_info += {'selinux':           selinux}
-summary_info += {'sev':               sev}
+summary_info += {'sev support':       sev_opt == 'disabled' ? false : sev_opt}
 summary_info += {'libdw':             libdw}
 if host_os == 'freebsd'
   summary_info += {'libinotify-kqueue': inotify}
diff --git a/meson_options.txt b/meson_options.txt
index 749fc87fd7..405d1abfd4 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -208,8 +208,6 @@ option('sdl_image', type : 'feature', value : 'auto',
        description: 'SDL Image support for icons')
 option('seccomp', type : 'feature', value : 'auto',
        description: 'seccomp support')
-option('sev', type : 'feature', value : 'auto',
-        description: 'Rust AMD SEV library')
 option('smartcard', type : 'feature', value : 'auto',
        description: 'CA smartcard emulation support')
 option('snappy', type : 'feature', value : 'auto',
@@ -313,6 +311,9 @@ option('capstone', type: 'feature', value: 'auto',
 option('fdt', type: 'combo', value: 'auto',
        choices: ['disabled', 'enabled', 'auto', 'system', 'internal'],
        description: 'Whether and how to find the libfdt library')
+option('sev', type: 'combo', value: 'auto',
+       choices: ['disabled', 'enabled', 'auto', 'system', 'internal'],
+       description: 'Whether and how to find the libsev library')
 
 option('selinux', type: 'feature', value: 'auto',
        description: 'SELinux support in qemu-nbd')




The caveat is that this does not actually work because the way the
meson rules in the sev crate are written prevents building it as a
static library:

$ cd sev
$ meson build -Ddefault_library=static
The Meson build system
Version: 1.2.3

...snip...

src/meson.build:18:6: ERROR: Cannot link_whole a custom or Rust target 'libsev.a' into a static library 'sev'. Instead, pass individual object files with the "objects:" keyword argument if possible.


The problem is that it is running 'cargo-build.sh' to compile the
SEV crate, which creates a libsev.a static library. It is then
using that as an input to "library('sev',....)" which will also
want to create a new static library with the contents of the first
static library. Never mind that the 2nd library will also be called
libsev.so, meson simply doesn't support linking static libraries
into static libraries AFAICT. There's an open RFE for it that has
no recent attention.


I observe, however, that if cargo-build.sh has already created a
libsev.a static library, then there's no need to tell meson to
create another static library from that. We should be able to use
the first libsev.a directly.

So I tried this change to the 'sev' crate:

diff --git a/meson.build b/meson.build
index 2bf68c3..915ebfb 100644
--- a/meson.build
+++ b/meson.build
@@ -16,5 +16,10 @@ subdir('docs')
 subdir('include')
 subdir('src') # requires: include
 
-sev_dep = declare_dependency(include_directories: inc,
-                             link_with: lib)
+if get_option('default_library') != 'static'
+  sev_dep = declare_dependency(include_directories: inc,
+                               link_with: libsev_so)
+else
+  sev_dep = declare_dependency(include_directories: inc,
+                               link_with: libsev_a)
+endif
diff --git a/src/meson.build b/src/meson.build
index 30bfe49..0632303 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -2,25 +2,27 @@ cargo_build = find_program('cargo-build.sh')
 
 v = meson.project_version().split('.')
 
-staticlib_name = 'libsev.a'
+libsev_a_name = 'libsev.a'
 
-staticlib_target = custom_target(staticlib_name,
+libsev_a = custom_target(libsev_a_name,
   build_by_default : true,
   build_always_stale : true,
   command : [cargo_build, get_option('debug').to_string(),
              get_option('optimization'), meson.current_build_dir() / 'target',
              '@OUTPUT@'],
   console : true,
-  output : [staticlib_name])
+  output : [libsev_a_name])
 
 math = meson.get_compiler('c').find_library('m', required: true)
 
-lib = library('sev',
-  link_whole: staticlib_target,
-  dependencies: [math],
-  install: true,
-  soversion: meson.project_version())
-
+if get_option('default_library') != 'static'
+  libsev_so = shared_library('sev',
+    link_whole: libsev_a,
+    dependencies: [math],
+    install: true,
+    soversion: meson.project_version())
+endif
+  
 # generate pkg-config file
 
 import('pkgconfig').generate(libraries : ['-lsev'],


It is a little bit gross, but it seems to work making it possible to
static link to the sev crate from QEMU with my QEMU patch earlier.


Now, the second issue is that my patch to QEMU's meson.build where
I look for "CONFIG_SEV" is wrong. I've not tested whether it behaves
correctly on non-x86 hosts - basically I'm hoping that CONFIG_SEV is
*NOT* present if building qemu-system-x86_64 on an aarch64 host.

Assuming we get this logic correct though, this unblocks one issue
with getting this merged - Rust platform support.

We've said we want Rust platform support to be a match for QEMU's
platform support. We're probably pretty close, but still it is a
review stumbling block.

If, however, we demonstrate that we /only/ try to use libsev crate
when building on an x86_64 host, then we don't need to think about
Rust platform support in any detail. We know Rust is fully supported
on x86_64 on Linux, and we're not introducing any Rust dependency
for QEMU on other build target arches.

With 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 :|

Re: [RFC PATCH v2 1/9] Add Rust SEV library as subproject

[Philippe Mathieu-Daudé]

On 5/3/24 14:47, Daniel P. Berrangé wrote:
> On Wed, Oct 04, 2023 at 04:34:10PM -0400, Tyler Fanelli wrote:
>> The Rust sev library provides a C API for the AMD SEV launch ioctls, as
>> well as the ability to build with meson. Add the Rust sev library as a
>> QEMU subproject with the goal of outsourcing all SEV launch ioctls to C
>> APIs provided by it.
>>
>> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
>> ---
>>   meson.build                   | 8 ++++++++
>>   meson_options.txt             | 2 ++
>>   scripts/meson-buildoptions.sh | 3 +++
>>   subprojects/sev.wrap          | 6 ++++++
>>   target/i386/meson.build       | 2 +-
>>   5 files changed, 20 insertions(+), 1 deletion(-)
>>   create mode 100644 subprojects/sev.wrap


> Now, the second issue is that my patch to QEMU's meson.build where
> I look for "CONFIG_SEV" is wrong. I've not tested whether it behaves
> correctly on non-x86 hosts - basically I'm hoping that CONFIG_SEV is
> *NOT* present if building qemu-system-x86_64 on an aarch64 host.

See hw/i386/Kconfig:

   config SEV
       bool
       ...
       depends on KVM

and meson.build ($cpu is the host):

   ...
elif cpu == 'x86'
   host_arch = 'i386'

if cpu in ['x86', 'x86_64']
   kvm_targets = ['i386-softmmu', 'x86_64-softmmu']

So SEV is only on selectable on x86 hosts, with KVM enabled.

> Assuming we get this logic correct though, this unblocks one issue
> with getting this merged - Rust platform support.
> 
> We've said we want Rust platform support to be a match for QEMU's
> platform support. We're probably pretty close, but still it is a
> review stumbling block.
> 
> If, however, we demonstrate that we /only/ try to use libsev crate
> when building on an x86_64 host, then we don't need to think about
> Rust platform support in any detail. We know Rust is fully supported
> on x86_64 on Linux, and we're not introducing any Rust dependency
> for QEMU on other build target arches.
> 
> With regards,
> Daniel