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

2023-10-10 Thread 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 
---
  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 00..5be1faccf6
--- /dev/null
+++ b/subprojects/sev.wrap
@@ -0,0 +1,6 @@
+[wrap-git]
+url = 

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

2023-10-10 Thread 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 
---
  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 00..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

2023-10-05 Thread 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 
---
  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 00..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




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

2023-10-04 Thread 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 
---
 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 8/9] i386/sev: Replace LAUNCH_FINISH ioctl with sev library equivalent

2023-10-04 Thread 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 
---
 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 7/9] i386/sev: Replace LAUNCH_SECRET ioctl with sev library equivalent

2023-10-04 Thread 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 
---
 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/se

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

2023-10-04 Thread 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 
---
 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 9/9] i386/sev: Replace SEV_ATTESTATION_REPORT with sev library equivalent

2023-10-04 Thread 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 
---
 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




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

2023-10-04 Thread 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 
---
 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 
 
+#include 
+
 #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 0/9] i386/sev: Use C API of Rust SEV library

2023-10-04 Thread 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 6/9] i386/sev: Replace LAUNCH_MEASURE ioctl with sev library equivalent

2023-10-04 Thread 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 
---
 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 3/9] i386/sev: Replace LAUNCH_START ioctl with sev library equivalent

2023-10-04 Thread 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 
---
 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 1/9] Add Rust SEV library as subproject

2023-10-04 Thread 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 
---
 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 00..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




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

2023-09-15 Thread Tyler Fanelli

On 9/15/23 7:33 AM, Stefan Hajnoczi wrote:

On Fri, 15 Sept 2023 at 05:54, Daniel P. Berrangé  wrote:

On Thu, Sep 14, 2023 at 01:58:27PM -0400, Tyler Fanelli wrote:

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.

We've talked about Rust alot, but thus far most focus has been on
areas peripheral to QEMU. Projects that might have been part of
QEMU in the past, and now being done as separate efforts, and
have bene taking advantage of Rust. eg virtiofsd Rust replacing
QEMU's in -tree C impl. eg passt providing an alternative to
slirp. eg the dbus display in QEMU allowing a remote display
frontend to be provided, written in rust. eg libblkio providing
a block backend in Rust.

The libblkio work is likely closest to what you've proposed
here, in that it is a Rust create exposed as a C shared library
for apps to consume. In theory apps don't need to care that it
is written in Rust, as it is opaque.

The one key difference though is that it was not replacing
existing functionality, it was adding a new feature. So users
who didn't have libblkio or whom want to avoid Rust dependancies
didn't loose anything they were already using.

If we use the libsev.so we create a hard dependancy on the Rust
sev crate, otherwise users loose the SEV feature in QEMU. Right
now the sev crate C library is not present in *any* distro that
I can see.


Yes, the C API is very new and not packaged in any distro at the moment.



If we treat 'sev' as just another opaque 3rd party library to be
provided by the distro, this creates a problem. Our support
policy is that we usually won't drop features in existing distros,
but that is what would happen if we applied this patchset today.
We did bend that rule slightly with virtiofsd, but that was already
a separate binary and we followed our deprecation path before
deleting it, giving distros time to adapt.


If we rollback the curtain, however, and decide to expose Rust
directly to QEMU we could address this problem. We could bundle
the dependant Rust crates directly with QEMU tarballs, and
generate the FFI C library as part of QEMU build and static
link the library. Distros would not have todo anything, though
they could have the choice of dyn linking if they really wanted
to.

If we directly exposed the notion of Rust to QEMU, then we are
also not limited by whether a Rust crate provides a C FFI itself.
QEMU could provide C FFI glue for any Rust crate it sees as
useful to its code.

This all forces us, however, to have the difficult discussion
about whether we're willing to make Rust a mandatory dependancy
of QEMU and permit (or even welcome) its use /anywhere/ in the
QEMU tree that looks relevant.

We've already queried whether Rust will actually benefit the
core QEMU codebase, or whether we'll end up punching too many
holes in its safety net to make it worthwhile. My opinion is
that we probably shouldn't obsess over that as I think it is
hard to predict the future, it has a habit of surprising us.
Your patch series here doesn't demonstrate an obvious safety
benefit, since we have existing working code and that code is
not especially complex.


Correct, there isn't any new features being added here. SEV on QEMU 
should work _exactly_ how it did before these patches.



Once we open the doors to Rust code
in QEMU though, we will probably surprise ourselves with the
range of benefits we'll see 2, 3, 5 years down the road.

IOW, we shouldn't judge future benefits based on this patch
series. It is great that this series is actually quite simple,
because it lets us focus on how we might integrate Rust more
directly into QEMU, without worrying much about the actual
code being replaced.


It also shows that much of Rust's security benefits in QEMU would depend 
on Rust being integrated more directly, rather than just using C FFI. 
Most of the code in the libsev C API is unsafe Rust anyway (it must be 
in order to interact with C). Therefore there is not much of an added 
security benefit here. However, if Rust can be further expanded in QEMU, 
much of the unsafe bits can be removed entirely (i.e. by bypassing the C 
API).





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.

My summary, is that I'd personally be in favour of opening the door
to Rust code as a mandatory pre-requisite for QEMU, at the very least
for system emulators. Not because this particular series is compelling,
but because I think Rust could be more beneficial to QEMU over the long
term than we expect. In terms of consuming it though, if we're going
to replace existing QEMU

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

2023-09-14 Thread Tyler Fanelli

On 9/14/23 3:04 PM, Philippe Mathieu-Daudé wrote:

Hi Tyler,

On 14/9/23 19:58, Tyler Fanelli wrote:

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, the
attestation report fetching, 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

Tyler Fanelli (8):
   Add SEV Rust library as dependency with CONFIG_SEV
   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


There is still one ioctl use, GET_ATTESTATION_REPORT. No libsev
equivalent for this one yet?

There is an equivalent, however the machine that I'm using currently 
hangs when trying to fetch an attestation report (not a libsev issue, as 
it hangs when I try with latest qemu release as well). When I can either 
update its firmware or get access to another SEV machine, I can test and 
confirm it behaves as intended with the libsev API. Once this is done, I 
can add that API to the patch series.



Tyler




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

2023-09-14 Thread 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 
---
 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 49be072cbc..615021a1a3 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 8/8] i386/sev: Replace LAUNCH_FINISH ioctl with sev library equivalent

2023-09-14 Thread 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 
---
 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 a4510b5437..e52dcc67c3 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 7/8] i386/sev: Replace LAUNCH_SECRET ioctl with sev library equivalent

2023-09-14 Thread 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 
---
 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 f53ff140e3..a4510b5437 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/se

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

2023-09-14 Thread 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 
---
 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 adb35291e8..f53ff140e3 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 1/8] Add SEV Rust library as dependency with CONFIG_SEV

2023-09-14 Thread Tyler Fanelli
The Rust sev library provides a type-safe implementation of the AMD
Secure Encrypted Virtualization (SEV) APIs.

Signed-off-by: Tyler Fanelli 
---
 meson.build   | 7 +++
 meson_options.txt | 2 ++
 scripts/meson-buildoptions.sh | 3 +++
 target/i386/meson.build   | 2 +-
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 5150a74831..7114a4a2b9 100644
--- a/meson.build
+++ b/meson.build
@@ -1079,6 +1079,12 @@ if targetos == 'linux' and (have_system or have_tools)
method: 'pkg-config',
required: get_option('libudev'))
 endif
+sev = not_found
+if not get_option('sev').auto()
+  sev = dependency('sev', version: '1.2.1',
+  method: 'pkg-config',
+  required: get_option('sev'))
+endif
 
 mpathlibs = [libudev]
 mpathpersist = not_found
@@ -4283,6 +4289,7 @@ summary_info += {'PAM':   pam}
 summary_info += {'iconv support': iconv}
 summary_info += {'virgl support': virgl}
 summary_info += {'blkio support': blkio}
+summary_info += {'sev support':   sev}
 summary_info += {'curl support':  curl}
 summary_info += {'Multipath support': mpathpersist}
 summary_info += {'Linux AIO support': libaio}
diff --git a/meson_options.txt b/meson_options.txt
index f82d88b7c6..c57d542c0b 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -134,6 +134,8 @@ option('cap_ng', type : 'feature', value : 'auto',
description: 'cap_ng support')
 option('blkio', type : 'feature', value : 'auto',
description: 'libblkio block device driver')
+option('sev', type : 'feature', value : 'auto',
+description: 'SEV Rust library')
 option('bpf', type : 'feature', value : 'auto',
 description: 'eBPF support')
 option('cocoa', type : 'feature', value : 'auto',
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index e1d178370c..d7deb50bda 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -83,6 +83,7 @@ meson_options_help() {
   printf "%s\n" '  avx512bwAVX512BW optimizations'
   printf "%s\n" '  avx512f AVX512F optimizations'
   printf "%s\n" '  blkio   libblkio block device driver'
+  printf "%s\n" '  sev SEV Rust library'
   printf "%s\n" '  bochs   bochs image format support'
   printf "%s\n" '  bpf eBPF support'
   printf "%s\n" '  brlapi  brlapi character device driver'
@@ -227,6 +228,8 @@ _meson_option_parse() {
 --disable-lto) printf "%s" -Db_lto=false ;;
 --enable-blkio) printf "%s" -Dblkio=enabled ;;
 --disable-blkio) printf "%s" -Dblkio=disabled ;;
+--enable-sev) printf "%s" -Dsev=enabled ;;
+--disable-sev) printf "%s" -Dsev=disabled ;;
 --block-drv-ro-whitelist=*) quote_sh "-Dblock_drv_ro_whitelist=$2" ;;
 --block-drv-rw-whitelist=*) quote_sh "-Dblock_drv_rw_whitelist=$2" ;;
 --enable-block-drv-whitelist-in-tools) printf "%s" 
-Dblock_drv_whitelist_in_tools=true ;;
diff --git a/target/i386/meson.build b/target/i386/meson.build
index 6f1036d469..18450dc134 100644
--- a/target/i386/meson.build
+++ b/target/i386/meson.build
@@ -6,7 +6,7 @@ i386_ss.add(files(
   'xsave_helper.c',
   'cpu-dump.c',
 ))
-i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'))
+i386_ss.add(when: 'CONFIG_SEV', if_true: [sev, files('host-cpu.c')])
 
 # x86 cpu type
 i386_ss.add(when: 'CONFIG_KVM', if_true: files('host-cpu.c'))
-- 
2.40.1




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

2023-09-14 Thread 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 
---
 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 f0fd291e68..49be072cbc 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 0/8] i386/sev: Use C API of Rust SEV library

2023-09-14 Thread 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, the
attestation report fetching, 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

Tyler Fanelli (8):
  Add SEV Rust library as dependency with CONFIG_SEV
  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

 meson.build   |   7 +
 meson_options.txt |   2 +
 scripts/meson-buildoptions.sh |   3 +
 target/i386/meson.build   |   2 +-
 target/i386/sev.c | 311 --
 target/i386/sev.h |   4 +-
 target/i386/trace-events  |   1 +
 7 files changed, 123 insertions(+), 207 deletions(-)

-- 
2.40.1




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

2023-09-14 Thread 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 
---
 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 615021a1a3..adb35291e8 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 2/8] i386/sev: Replace INIT and ES_INIT ioctls with sev library equivalents

2023-09-14 Thread 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 
---
 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..f0fd291e68 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -18,6 +18,8 @@
 
 #include 
 
+#include 
+
 #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 7/8] i386/sev: Replace LAUNCH_SECRET ioctl with sev library equivalent

2023-09-14 Thread 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 
---
 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 f53ff140e3..a4510b5437 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/se

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

2023-09-14 Thread 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 
---
 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 615021a1a3..adb35291e8 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 4/8] i386/sev: Replace UPDATE_DATA ioctl with sev library equivalent

2023-09-14 Thread 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 
---
 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 49be072cbc..615021a1a3 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 8/8] i386/sev: Replace LAUNCH_FINISH ioctl with sev library equivalent

2023-09-14 Thread 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 
---
 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 a4510b5437..e52dcc67c3 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 6/8] i386/sev: Replace LAUNCH_MEASURE ioctl with sev library equivalent

2023-09-14 Thread 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 
---
 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 adb35291e8..f53ff140e3 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 1/8] Add SEV Rust library as dependency with CONFIG_SEV

2023-09-14 Thread Tyler Fanelli
The Rust sev library provides a type-safe implementation of the AMD
Secure Encrypted Virtualization (SEV) APIs.

Signed-off-by: Tyler Fanelli 
---
 meson.build   | 7 +++
 meson_options.txt | 2 ++
 scripts/meson-buildoptions.sh | 3 +++
 target/i386/meson.build   | 2 +-
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 5150a74831..7114a4a2b9 100644
--- a/meson.build
+++ b/meson.build
@@ -1079,6 +1079,12 @@ if targetos == 'linux' and (have_system or have_tools)
method: 'pkg-config',
required: get_option('libudev'))
 endif
+sev = not_found
+if not get_option('sev').auto()
+  sev = dependency('sev', version: '1.2.1',
+  method: 'pkg-config',
+  required: get_option('sev'))
+endif
 
 mpathlibs = [libudev]
 mpathpersist = not_found
@@ -4283,6 +4289,7 @@ summary_info += {'PAM':   pam}
 summary_info += {'iconv support': iconv}
 summary_info += {'virgl support': virgl}
 summary_info += {'blkio support': blkio}
+summary_info += {'sev support':   sev}
 summary_info += {'curl support':  curl}
 summary_info += {'Multipath support': mpathpersist}
 summary_info += {'Linux AIO support': libaio}
diff --git a/meson_options.txt b/meson_options.txt
index f82d88b7c6..c57d542c0b 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -134,6 +134,8 @@ option('cap_ng', type : 'feature', value : 'auto',
description: 'cap_ng support')
 option('blkio', type : 'feature', value : 'auto',
description: 'libblkio block device driver')
+option('sev', type : 'feature', value : 'auto',
+description: 'SEV Rust library')
 option('bpf', type : 'feature', value : 'auto',
 description: 'eBPF support')
 option('cocoa', type : 'feature', value : 'auto',
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index e1d178370c..d7deb50bda 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -83,6 +83,7 @@ meson_options_help() {
   printf "%s\n" '  avx512bwAVX512BW optimizations'
   printf "%s\n" '  avx512f AVX512F optimizations'
   printf "%s\n" '  blkio   libblkio block device driver'
+  printf "%s\n" '  sev SEV Rust library'
   printf "%s\n" '  bochs   bochs image format support'
   printf "%s\n" '  bpf eBPF support'
   printf "%s\n" '  brlapi  brlapi character device driver'
@@ -227,6 +228,8 @@ _meson_option_parse() {
 --disable-lto) printf "%s" -Db_lto=false ;;
 --enable-blkio) printf "%s" -Dblkio=enabled ;;
 --disable-blkio) printf "%s" -Dblkio=disabled ;;
+--enable-sev) printf "%s" -Dsev=enabled ;;
+--disable-sev) printf "%s" -Dsev=disabled ;;
 --block-drv-ro-whitelist=*) quote_sh "-Dblock_drv_ro_whitelist=$2" ;;
 --block-drv-rw-whitelist=*) quote_sh "-Dblock_drv_rw_whitelist=$2" ;;
 --enable-block-drv-whitelist-in-tools) printf "%s" 
-Dblock_drv_whitelist_in_tools=true ;;
diff --git a/target/i386/meson.build b/target/i386/meson.build
index 6f1036d469..18450dc134 100644
--- a/target/i386/meson.build
+++ b/target/i386/meson.build
@@ -6,7 +6,7 @@ i386_ss.add(files(
   'xsave_helper.c',
   'cpu-dump.c',
 ))
-i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'))
+i386_ss.add(when: 'CONFIG_SEV', if_true: [sev, files('host-cpu.c')])
 
 # x86 cpu type
 i386_ss.add(when: 'CONFIG_KVM', if_true: files('host-cpu.c'))
-- 
2.40.1




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

2023-09-14 Thread 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 
---
 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..f0fd291e68 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -18,6 +18,8 @@
 
 #include 
 
+#include 
+
 #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 3/8] i386/sev: Replace LAUNCH_START ioctl with sev library equivalent

2023-09-14 Thread 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 
---
 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 f0fd291e68..49be072cbc 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 0/8] i386/sev: Use C API of Rust SEV library

2023-09-14 Thread 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, the
attestation report fetching, 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

Tyler Fanelli (8):
  Add SEV Rust library as dependency with CONFIG_SEV
  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

 meson.build   |   7 +
 meson_options.txt |   2 +
 scripts/meson-buildoptions.sh |   3 +
 target/i386/meson.build   |   2 +-
 target/i386/sev.c | 311 --
 target/i386/sev.h |   4 +-
 target/i386/trace-events  |   1 +
 7 files changed, 123 insertions(+), 207 deletions(-)

-- 
2.40.1




[PATCH v3] i386/sev: Ensure attestation report length is valid before retrieving

2022-03-04 Thread Tyler Fanelli
The length of the attestation report buffer is never checked to be
valid before allocation is made. If the length of the report is returned
to be 0, the buffer to retrieve the attestation buffer is allocated with
length 0 and passed to the kernel to fill with contents of the attestation
report. Leaving this unchecked is dangerous and could lead to undefined
behavior.

Signed-off-by: Tyler Fanelli 
---
 target/i386/sev.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 025ff7a6f8..e82be3e350 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -616,6 +616,8 @@ static SevAttestationReport 
*sev_get_attestation_report(const char *mnonce,
 return NULL;
 }
 
+input.len = 0;
+
 /* Query the report length */
 ret = sev_ioctl(sev->sev_fd, KVM_SEV_GET_ATTESTATION_REPORT,
 &input, &err);
@@ -626,6 +628,11 @@ static SevAttestationReport 
*sev_get_attestation_report(const char *mnonce,
ret, err, fw_error_to_str(err));
 return NULL;
 }
+} else if (input.len == 0) {
+error_setg(errp, "SEV: Failed to query attestation report:"
+ " length returned=%u",
+   input.len);
+return NULL;
 }
 
 data = g_malloc(input.len);
-- 
2.31.1




[PATCH v2] i386/sev: Ensure attestation report length is valid before retrieving

2022-03-04 Thread Tyler Fanelli
The length of the attestation report buffer is never checked to be
valid before allocation is made. If the length of the report is returned
to be 0, the buffer to retrieve the attestation buffer is allocated with
length 0 and passed to the kernel to fill with contents of the attestation
report. Leaving this unchecked is dangerous and could lead to undefined
behavior.

Signed-off-by: Tyler Fanelli 
---
 target/i386/sev.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 025ff7a6f8..80d958369b 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -616,6 +616,8 @@ static SevAttestationReport 
*sev_get_attestation_report(const char *mnonce,
 return NULL;
 }
 
+input.len = 0;
+
 /* Query the report length */
 ret = sev_ioctl(sev->sev_fd, KVM_SEV_GET_ATTESTATION_REPORT,
 &input, &err);
@@ -626,6 +628,11 @@ static SevAttestationReport 
*sev_get_attestation_report(const char *mnonce,
ret, err, fw_error_to_str(err));
 return NULL;
 }
+} else if (input.len == 0) {
+error_setg(errp, "SEV: Failed to query attestation report:"
+ " length returned=%d",
+   input.len);
+return NULL;
 }
 
 data = g_malloc(input.len);
-- 
2.31.1




[PATCH] i386/sev: Ensure attestation report length is valid before retrieving

2022-03-04 Thread Tyler Fanelli
The length of the attestation report buffer is never checked to be
valid before allocation is made. If the length of the report is returned
to be 0, the buffer to retrieve the attestation report is allocated with
length 0 and passed to the kernel to fill with contents of the attestation
report. Leaving this unchecked is dangerous and could lead to undefined
behavior.

Signed-off-by: Tyler Fanelli 
---
 target/i386/sev.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 025ff7a6f8..215acd7c6b 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -616,6 +616,8 @@ static SevAttestationReport 
*sev_get_attestation_report(const char *mnonce,
 return NULL;
 }
 
+input.len = 0;
+
 /* Query the report length */
 ret = sev_ioctl(sev->sev_fd, KVM_SEV_GET_ATTESTATION_REPORT,
 &input, &err);
@@ -626,6 +628,11 @@ static SevAttestationReport 
*sev_get_attestation_report(const char *mnonce,
ret, err, fw_error_to_str(err));
 return NULL;
 }
+} else if (input.len <= 0) {
+error_setg(errp, "SEV: Failed to query attestation report:"
+ " length returned=%d",
+   input.len);
+return NULL;
 }
 
 data = g_malloc(input.len);
-- 
2.31.1




Re: SEV guest attestation

2021-11-24 Thread Tyler Fanelli

On 11/24/21 12:49 PM, Dr. David Alan Gilbert wrote:

* Tyler Fanelli (tfane...@redhat.com) wrote:

Hi,

We recently discussed a way for remote SEV guest attestation through QEMU.
My initial approach was to get data needed for attestation through different
QMP commands (all of which are already available, so no changes required
there), deriving hashes and certificate data; and collecting all of this
into a new QMP struct (SevLaunchStart, which would include the VM's policy,
secret, and GPA) which would need to be upstreamed into QEMU. Once this is
provided, QEMU would then need to have support for attestation before a VM
is started. Upon speaking to Dave about this proposal, he mentioned that
this may not be the best approach, as some situations would render the
attestation unavailable, such as the instance where a VM is running in a
cloud, and a guest owner would like to perform attestation via QMP (a likely
scenario), yet a cloud provider cannot simply let anyone pass arbitrary QMP
commands, as this could be an issue.

So I ask, does anyone involved in QEMU's SEV implementation have any input
on a quality way to perform guest attestation? If so, I'd be interested.
Thanks.

QMP is the right way to talk to QEMU; the question is whether something
sits between qemu and the attestation program - e.g. libvirt or possibly
subsequently something even higher level.

Can we start by you putting down what your interfaces look like at the
moment?


Basically, I just establish a connection with a QMP socket at the 
beginning, serialize different QMP structs to get the data I need 
(query-sev, query-sev-capabilities, etc..), get the results and 
deserialize that data. In the original attempt, I would keep this 
protocol for issuing "sev-launch-start", "sev-inject-secret", and 
others. From a mgmt app perspective (in my case, I'm looking at it from 
a sevctl perspective), it's relatively straightforward. Any work 
required for getting certificates, sessions, measurements, and OVMF data 
is handled by sevctl.



Dave


Tyler.

--
Tyler Fanelli (tfanelli)




Re: SEV guest attestation

2021-11-24 Thread Tyler Fanelli

On 11/24/21 11:34 AM, Tyler Fanelli wrote:

We recently discussed a way for remote SEV guest attestation through QEMU.


For those interested, here is where some of the discussion took place 
before.


[1] https://listman.redhat.com/archives/libvir-list/2021-May/msg00196.html

[2] 
https://listman.redhat.com/archives/libvir-list/2021-October/msg01052.html



Tyler.

--
Tyler Fanelli (tfanelli)




SEV guest attestation

2021-11-24 Thread Tyler Fanelli

Hi,

We recently discussed a way for remote SEV guest attestation through 
QEMU. My initial approach was to get data needed for attestation through 
different QMP commands (all of which are already available, so no 
changes required there), deriving hashes and certificate data; and 
collecting all of this into a new QMP struct (SevLaunchStart, which 
would include the VM's policy, secret, and GPA) which would need to be 
upstreamed into QEMU. Once this is provided, QEMU would then need to 
have support for attestation before a VM is started. Upon speaking to 
Dave about this proposal, he mentioned that this may not be the best 
approach, as some situations would render the attestation unavailable, 
such as the instance where a VM is running in a cloud, and a guest owner 
would like to perform attestation via QMP (a likely scenario), yet a 
cloud provider cannot simply let anyone pass arbitrary QMP commands, as 
this could be an issue.


So I ask, does anyone involved in QEMU's SEV implementation have any 
input on a quality way to perform guest attestation? If so, I'd be 
interested. Thanks.



Tyler.

--
Tyler Fanelli (tfanelli)




[PATCH] sev: check which processor the ASK/ARK chain should match

2021-11-16 Thread Tyler Fanelli
The AMD ASK/ARK certificate chain differs between AMD SEV
processor generations. SEV capabilities should provide
which ASK/ARK certificate should be used based on the host
processor.

Signed-off-by: Tyler Fanelli 
---
 qapi/misc-target.json | 28 ++--
 target/i386/sev.c | 17 ++---
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 5aa2b95b7d..c64aa3ff57 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -166,6 +166,24 @@
 { 'command': 'query-sev-launch-measure', 'returns': 'SevLaunchMeasureInfo',
   'if': 'TARGET_I386' }
 
+##
+# @SevAskArkCertName:
+#
+# This enum describes which ASK/ARK certificate should be
+# used based on the generation of an AMD Secure Encrypted
+# Virtualization processor.
+#
+# @naples: AMD Naples processor (SEV 1st generation)
+#
+# @rome: AMD Rome processor (SEV 2nd generation)
+#
+# @milan: AMD Milan processor (SEV 3rd generation)
+#
+# Since: 7.0
+##
+{ 'enum': 'SevAskArkCertName',
+  'data': ['naples', 'rome', 'milan'],
+  'if': 'TARGET_I386' }
 
 ##
 # @SevCapability:
@@ -182,13 +200,18 @@
 # @reduced-phys-bits: Number of physical Address bit reduction when SEV is
 # enabled
 #
+# @ask-ark-cert-name: The generation in which the AMD
+# ARK/ASK should be derived from
+# (since 7.0)
+#
 # Since: 2.12
 ##
 { 'struct': 'SevCapability',
   'data': { 'pdh': 'str',
 'cert-chain': 'str',
 'cbitpos': 'int',
-'reduced-phys-bits': 'int'},
+'reduced-phys-bits': 'int',
+'ask-ark-cert-name': 'SevAskArkCertName'},
   'if': 'TARGET_I386' }
 
 ##
@@ -205,7 +228,8 @@
 #
 # -> { "execute": "query-sev-capabilities" }
 # <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
-#  "cbitpos": 47, "reduced-phys-bits": 5}}
+#  "cbitpos": 47, "reduced-phys-bits": 5,
+#  "ask-ark-cert-name": "naples"}}
 #
 ##
 { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
diff --git a/target/i386/sev.c b/target/i386/sev.c
index eede07f11d..f30171e5ba 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -506,8 +506,9 @@ static SevCapability *sev_get_capabilities(Error **errp)
 guchar *pdh_data = NULL;
 guchar *cert_chain_data = NULL;
 size_t pdh_len = 0, cert_chain_len = 0;
-uint32_t ebx;
-int fd;
+uint32_t eax, ebx;
+int fd, es, snp;
+
 
 if (!kvm_enabled()) {
 error_setg(errp, "KVM not enabled");
@@ -534,9 +535,19 @@ static SevCapability *sev_get_capabilities(Error **errp)
 cap->pdh = g_base64_encode(pdh_data, pdh_len);
 cap->cert_chain = g_base64_encode(cert_chain_data, cert_chain_len);
 
-host_cpuid(0x801F, 0, NULL, &ebx, NULL, NULL);
+host_cpuid(0x801F, 0, &eax, &ebx, NULL, NULL);
 cap->cbitpos = ebx & 0x3f;
 
+es = eax & 0x8;
+snp = eax & 0x10;
+if (!es && !snp) {
+   cap->ask_ark_cert_name = SEV_ASK_ARK_CERT_NAME_NAPLES;
+} else if (es && !snp) {
+   cap->ask_ark_cert_name = SEV_ASK_ARK_CERT_NAME_ROME;
+} else {
+   cap->ask_ark_cert_name = SEV_ASK_ARK_CERT_NAME_MILAN;
+}
+
 /*
  * When SEV feature is enabled, we loose one bit in guest physical
  * addressing.
-- 
2.31.1




Re: [PATCH] sev: allow capabilities to check for SEV-ES support

2021-11-16 Thread Tyler Fanelli

On 11/16/21 12:23 PM, Daniel P. Berrangé wrote:

On Tue, Nov 16, 2021 at 11:58:12AM -0500, Tyler Fanelli wrote:

On 11/16/21 10:53 AM, Daniel P. Berrangé wrote:

On Tue, Nov 16, 2021 at 10:29:35AM -0500, Tyler Fanelli wrote:

On 11/16/21 4:17 AM, Daniel P. Berrangé wrote:

On Mon, Nov 15, 2021 at 02:38:04PM -0500, Tyler Fanelli wrote:

Probe for SEV-ES and SEV-SNP capabilities to distinguish between Rome,
Naples, and Milan processors. Use the CPUID function to probe if a
processor is capable of running SEV-ES or SEV-SNP, rather than if it
actually is running SEV-ES or SEV-SNP.

Signed-off-by: Tyler Fanelli 
---
qapi/misc-target.json | 11 +--
target/i386/sev.c |  6 --
2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 5aa2b95b7d..c3e9bce12b 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -182,13 +182,19 @@
# @reduced-phys-bits: Number of physical Address bit reduction when SEV is
# enabled
#
+# @es: SEV-ES capability of the machine.
+#
+# @snp: SEV-SNP capability of the machine.
+#
# Since: 2.12
##
{ 'struct': 'SevCapability',
  'data': { 'pdh': 'str',
'cert-chain': 'str',
'cbitpos': 'int',
-'reduced-phys-bits': 'int'},
+'reduced-phys-bits': 'int',
+'es': 'bool',
+'snp': 'bool'},
  'if': 'TARGET_I386' }
##
@@ -205,7 +211,8 @@
#
# -> { "execute": "query-sev-capabilities" }
# <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
-#  "cbitpos": 47, "reduced-phys-bits": 5}}
+#  "cbitpos": 47, "reduced-phys-bits": 5
+#  "es": false, "snp": false}}

We've previously had patches posted to support SNP in QEMU

 https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg04761.html

and this included an update to query-sev for reporting info
about the VM instance.

Your patch is updating query-sev-capabilities, which is a
counterpart for detecting host capabilities separate from
a guest instance.

Yes, that's because with this patch, I'm more interested in determining
which AMD processor is running on a host, and less if ES or SNP is actually
running on a guest instance or not.

None the less I wonder if the same design questions from
query-sev apply. ie do we need to have the ability to
report any SNP specific information fields, if so we need
to use a discriminated union of structs, not just bool
flags.

More generally I'm some what wary of adding this to
query-sev-capabilities at all, unless it is part of the
main SEV-SNP series.

Also what's the intended usage for the mgmt app from just
having these boolean fields ? Are they other more explicit
feature flags we should be reporting, instead of what are
essentially SEV generation codenames.

If by "mgmt app" you're referring to sevctl, in order to determine which
certificate chain to use (Naples vs Rome vs Milan ARK/ASK) we must query
which processor we are running on. Although sevctl has a feature which can
do this already, we cannot guarantee that sevctl is running on the same host
that a VM is running on, so we must query this capability from QEMU. My
logic was determining the processor would have been the following:

I'm not really talking about a specific, rather any tool which wants
to deal with SEV and QEMU, whether libvirt or an app using libvirt,
or something else using QEMU directly.

Ah, my mistake.


Where does the actual cert chain payload come from ? Is that something
the app has to acquire out of band, or can the full cert chain be
acquired from the hardware itself ?

The cert chain (or the ARK/ASK specifically) comes from AMD's KDS, yet
sevctl is able to cache the values, and has them on-hand when needed. This
patch would tell sevctl *which* of the cert chains to use (Naples vs Rome vs
Milan chain). If need be, I could just focus on Naples and Rome processors
for now and bring support for SNP (Milan processors) later on when it is
more mature.


!es && !snp --> Naples

es && !snp --> Rome

es && snp --> Milan

This approach isn't future proof if subsequent generations introduce
new certs. It feels like we should be explicitly reporting something
about the certs rather than relying on every app to re-implement tihs
logic.

Alright, like an encoding of which processor generation the host is running
on?

IIUC (from looking at sev-tool), the certificates can be acquired
from

https://developer.amd.com/wp-content/resources/ask_

Re: [PATCH] sev: allow capabilities to check for SEV-ES support

2021-11-16 Thread Tyler Fanelli

On 11/16/21 10:53 AM, Daniel P. Berrangé wrote:

On Tue, Nov 16, 2021 at 10:29:35AM -0500, Tyler Fanelli wrote:

On 11/16/21 4:17 AM, Daniel P. Berrangé wrote:

On Mon, Nov 15, 2021 at 02:38:04PM -0500, Tyler Fanelli wrote:

Probe for SEV-ES and SEV-SNP capabilities to distinguish between Rome,
Naples, and Milan processors. Use the CPUID function to probe if a
processor is capable of running SEV-ES or SEV-SNP, rather than if it
actually is running SEV-ES or SEV-SNP.

Signed-off-by: Tyler Fanelli 
---
   qapi/misc-target.json | 11 +--
   target/i386/sev.c |  6 --
   2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 5aa2b95b7d..c3e9bce12b 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -182,13 +182,19 @@
   # @reduced-phys-bits: Number of physical Address bit reduction when SEV is
   # enabled
   #
+# @es: SEV-ES capability of the machine.
+#
+# @snp: SEV-SNP capability of the machine.
+#
   # Since: 2.12
   ##
   { 'struct': 'SevCapability',
 'data': { 'pdh': 'str',
   'cert-chain': 'str',
   'cbitpos': 'int',
-'reduced-phys-bits': 'int'},
+'reduced-phys-bits': 'int',
+'es': 'bool',
+'snp': 'bool'},
 'if': 'TARGET_I386' }
   ##
@@ -205,7 +211,8 @@
   #
   # -> { "execute": "query-sev-capabilities" }
   # <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
-#  "cbitpos": 47, "reduced-phys-bits": 5}}
+#  "cbitpos": 47, "reduced-phys-bits": 5
+#  "es": false, "snp": false}}

We've previously had patches posted to support SNP in QEMU

https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg04761.html

and this included an update to query-sev for reporting info
about the VM instance.

Your patch is updating query-sev-capabilities, which is a
counterpart for detecting host capabilities separate from
a guest instance.

Yes, that's because with this patch, I'm more interested in determining
which AMD processor is running on a host, and less if ES or SNP is actually
running on a guest instance or not.

None the less I wonder if the same design questions from
query-sev apply. ie do we need to have the ability to
report any SNP specific information fields, if so we need
to use a discriminated union of structs, not just bool
flags.

More generally I'm some what wary of adding this to
query-sev-capabilities at all, unless it is part of the
main SEV-SNP series.

Also what's the intended usage for the mgmt app from just
having these boolean fields ? Are they other more explicit
feature flags we should be reporting, instead of what are
essentially SEV generation codenames.

If by "mgmt app" you're referring to sevctl, in order to determine which
certificate chain to use (Naples vs Rome vs Milan ARK/ASK) we must query
which processor we are running on. Although sevctl has a feature which can
do this already, we cannot guarantee that sevctl is running on the same host
that a VM is running on, so we must query this capability from QEMU. My
logic was determining the processor would have been the following:

I'm not really talking about a specific, rather any tool which wants
to deal with SEV and QEMU, whether libvirt or an app using libvirt,
or something else using QEMU directly.


Ah, my mistake.


Where does the actual cert chain payload come from ? Is that something
the app has to acquire out of band, or can the full cert chain be
acquired from the hardware itself ?


The cert chain (or the ARK/ASK specifically) comes from AMD's KDS, yet 
sevctl is able to cache the values, and has them on-hand when needed. 
This patch would tell sevctl *which* of the cert chains to use (Naples 
vs Rome vs Milan chain). If need be, I could just focus on Naples and 
Rome processors for now and bring support for SNP (Milan processors) 
later on when it is more mature.



!es && !snp --> Naples

es && !snp --> Rome

es && snp --> Milan

This approach isn't future proof if subsequent generations introduce
new certs. It feels like we should be explicitly reporting something
about the certs rather than relying on every app to re-implement tihs
logic.


Alright, like an encoding of which processor generation the host is 
running on?




Regards,
Daniel


Tyler.

--
Tyler Fanelli (tfanelli)




Re: [PATCH] sev: allow capabilities to check for SEV-ES support

2021-11-16 Thread Tyler Fanelli

On 11/16/21 4:17 AM, Daniel P. Berrangé wrote:

On Mon, Nov 15, 2021 at 02:38:04PM -0500, Tyler Fanelli wrote:

Probe for SEV-ES and SEV-SNP capabilities to distinguish between Rome,
Naples, and Milan processors. Use the CPUID function to probe if a
processor is capable of running SEV-ES or SEV-SNP, rather than if it
actually is running SEV-ES or SEV-SNP.

Signed-off-by: Tyler Fanelli 
---
  qapi/misc-target.json | 11 +--
  target/i386/sev.c |  6 --
  2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 5aa2b95b7d..c3e9bce12b 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -182,13 +182,19 @@
  # @reduced-phys-bits: Number of physical Address bit reduction when SEV is
  # enabled
  #
+# @es: SEV-ES capability of the machine.
+#
+# @snp: SEV-SNP capability of the machine.
+#
  # Since: 2.12
  ##
  { 'struct': 'SevCapability',
'data': { 'pdh': 'str',
  'cert-chain': 'str',
  'cbitpos': 'int',
-'reduced-phys-bits': 'int'},
+'reduced-phys-bits': 'int',
+'es': 'bool',
+'snp': 'bool'},
'if': 'TARGET_I386' }
  
  ##

@@ -205,7 +211,8 @@
  #
  # -> { "execute": "query-sev-capabilities" }
  # <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
-#  "cbitpos": 47, "reduced-phys-bits": 5}}
+#  "cbitpos": 47, "reduced-phys-bits": 5
+#  "es": false, "snp": false}}

We've previously had patches posted to support SNP in QEMU

   https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg04761.html

and this included an update to query-sev for reporting info
about the VM instance.

Your patch is updating query-sev-capabilities, which is a
counterpart for detecting host capabilities separate from
a guest instance.


Yes, that's because with this patch, I'm more interested in determining 
which AMD processor is running on a host, and less if ES or SNP is 
actually running on a guest instance or not.




None the less I wonder if the same design questions from
query-sev apply. ie do we need to have the ability to
report any SNP specific information fields, if so we need
to use a discriminated union of structs, not just bool
flags.

More generally I'm some what wary of adding this to
query-sev-capabilities at all, unless it is part of the
main SEV-SNP series.

Also what's the intended usage for the mgmt app from just
having these boolean fields ? Are they other more explicit
feature flags we should be reporting, instead of what are
essentially SEV generation codenames.


If by "mgmt app" you're referring to sevctl, in order to determine which 
certificate chain to use (Naples vs Rome vs Milan ARK/ASK) we must query 
which processor we are running on. Although sevctl has a feature which 
can do this already, we cannot guarantee that sevctl is running on the 
same host that a VM is running on, so we must query this capability from 
QEMU. My logic was determining the processor would have been the following:


!es && !snp --> Naples

es && !snp --> Rome

es && snp --> Milan




Regards,
Daniel


Tyler.

--
Tyler Fanelli (tfanelli)




[PATCH] sev: allow capabilities to check for SEV-ES support

2021-11-15 Thread Tyler Fanelli
Probe for SEV-ES and SEV-SNP capabilities to distinguish between Rome,
Naples, and Milan processors. Use the CPUID function to probe if a
processor is capable of running SEV-ES or SEV-SNP, rather than if it
actually is running SEV-ES or SEV-SNP.

Signed-off-by: Tyler Fanelli 
---
 qapi/misc-target.json | 11 +--
 target/i386/sev.c |  6 --
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 5aa2b95b7d..c3e9bce12b 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -182,13 +182,19 @@
 # @reduced-phys-bits: Number of physical Address bit reduction when SEV is
 # enabled
 #
+# @es: SEV-ES capability of the machine.
+#
+# @snp: SEV-SNP capability of the machine.
+#
 # Since: 2.12
 ##
 { 'struct': 'SevCapability',
   'data': { 'pdh': 'str',
 'cert-chain': 'str',
 'cbitpos': 'int',
-'reduced-phys-bits': 'int'},
+'reduced-phys-bits': 'int',
+'es': 'bool',
+'snp': 'bool'},
   'if': 'TARGET_I386' }
 
 ##
@@ -205,7 +211,8 @@
 #
 # -> { "execute": "query-sev-capabilities" }
 # <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
-#  "cbitpos": 47, "reduced-phys-bits": 5}}
+#  "cbitpos": 47, "reduced-phys-bits": 5
+#  "es": false, "snp": false}}
 #
 ##
 { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
diff --git a/target/i386/sev.c b/target/i386/sev.c
index eede07f11d..6d78dcd744 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -506,7 +506,7 @@ static SevCapability *sev_get_capabilities(Error **errp)
 guchar *pdh_data = NULL;
 guchar *cert_chain_data = NULL;
 size_t pdh_len = 0, cert_chain_len = 0;
-uint32_t ebx;
+uint32_t eax, ebx;
 int fd;
 
 if (!kvm_enabled()) {
@@ -534,8 +534,10 @@ static SevCapability *sev_get_capabilities(Error **errp)
 cap->pdh = g_base64_encode(pdh_data, pdh_len);
 cap->cert_chain = g_base64_encode(cert_chain_data, cert_chain_len);
 
-host_cpuid(0x801F, 0, NULL, &ebx, NULL, NULL);
+host_cpuid(0x801F, 0, &eax, &ebx, NULL, NULL);
 cap->cbitpos = ebx & 0x3f;
+cap->es = (eax & 0x8) ? true : false;
+cap->snp = (eax & 0x10) ? true : false;
 
 /*
  * When SEV feature is enabled, we loose one bit in guest physical
-- 
2.31.1