[PULL 30/45] i386/sev: Add handling to encrypt/finalize guest launch data

Paolo Bonzini posted 45 patches 5 months, 3 weeks ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Marcelo Tosatti <mtosatti@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Stefan Hajnoczi <stefanha@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Peter Xu <peterx@redhat.com>, David Hildenbrand <david@redhat.com>, Cornelia Huck <cohuck@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
[PULL 30/45] i386/sev: Add handling to encrypt/finalize guest launch data
Posted by Paolo Bonzini 5 months, 3 weeks ago
From: Brijesh Singh <brijesh.singh@amd.com>

Process any queued up launch data and encrypt/measure it into the SNP
guest instance prior to initial guest launch.

This also updates the KVM_SEV_SNP_LAUNCH_UPDATE call to handle partial
update responses.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Co-developed-by: Michael Roth <michael.roth@amd.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
Co-developed-by: Pankaj Gupta <pankaj.gupta@amd.com>
Signed-off-by: Pankaj Gupta <pankaj.gupta@amd.com>
Message-ID: <20240530111643.1091816-17-pankaj.gupta@amd.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/sev.c        | 112 ++++++++++++++++++++++++++++++++++++++-
 target/i386/trace-events |   2 +
 2 files changed, 113 insertions(+), 1 deletion(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index e89b87d2f55..ef2e592ca76 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -756,6 +756,76 @@ out:
     return ret;
 }
 
+static const char *
+snp_page_type_to_str(int type)
+{
+    switch (type) {
+    case KVM_SEV_SNP_PAGE_TYPE_NORMAL: return "Normal";
+    case KVM_SEV_SNP_PAGE_TYPE_ZERO: return "Zero";
+    case KVM_SEV_SNP_PAGE_TYPE_UNMEASURED: return "Unmeasured";
+    case KVM_SEV_SNP_PAGE_TYPE_SECRETS: return "Secrets";
+    case KVM_SEV_SNP_PAGE_TYPE_CPUID: return "Cpuid";
+    default: return "unknown";
+    }
+}
+
+static int
+sev_snp_launch_update(SevSnpGuestState *sev_snp_guest,
+                      SevLaunchUpdateData *data)
+{
+    int ret, fw_error;
+    struct kvm_sev_snp_launch_update update = {0};
+
+    if (!data->hva || !data->len) {
+        error_report("SNP_LAUNCH_UPDATE called with invalid address"
+                     "/ length: %p / %lx",
+                     data->hva, data->len);
+        return 1;
+    }
+
+    update.uaddr = (__u64)(unsigned long)data->hva;
+    update.gfn_start = data->gpa >> TARGET_PAGE_BITS;
+    update.len = data->len;
+    update.type = data->type;
+
+    /*
+     * KVM_SEV_SNP_LAUNCH_UPDATE requires that GPA ranges have the private
+     * memory attribute set in advance.
+     */
+    ret = kvm_set_memory_attributes_private(data->gpa, data->len);
+    if (ret) {
+        error_report("SEV-SNP: failed to configure initial"
+                     "private guest memory");
+        goto out;
+    }
+
+    while (update.len || ret == -EAGAIN) {
+        trace_kvm_sev_snp_launch_update(update.uaddr, update.gfn_start <<
+                                        TARGET_PAGE_BITS, update.len,
+                                        snp_page_type_to_str(update.type));
+
+        ret = sev_ioctl(SEV_COMMON(sev_snp_guest)->sev_fd,
+                        KVM_SEV_SNP_LAUNCH_UPDATE,
+                        &update, &fw_error);
+        if (ret && ret != -EAGAIN) {
+            error_report("SNP_LAUNCH_UPDATE ret=%d fw_error=%d '%s'",
+                         ret, fw_error, fw_error_to_str(fw_error));
+            break;
+        }
+    }
+
+out:
+    if (!ret && update.gfn_start << TARGET_PAGE_BITS != data->gpa + data->len) {
+        error_report("SEV-SNP: expected update of GPA range %lx-%lx,"
+                     "got GPA range %lx-%llx",
+                     data->gpa, data->gpa + data->len, data->gpa,
+                     update.gfn_start << TARGET_PAGE_BITS);
+        ret = -EIO;
+    }
+
+    return ret;
+}
+
 static int
 sev_launch_update_data(SevGuestState *sev_guest, uint8_t *addr, uint64_t len)
 {
@@ -901,6 +971,46 @@ sev_launch_finish(SevCommonState *sev_common)
     migrate_add_blocker(&sev_mig_blocker, &error_fatal);
 }
 
+static void
+sev_snp_launch_finish(SevCommonState *sev_common)
+{
+    int ret, error;
+    Error *local_err = NULL;
+    SevLaunchUpdateData *data;
+    SevSnpGuestState *sev_snp = SEV_SNP_GUEST(sev_common);
+    struct kvm_sev_snp_launch_finish *finish = &sev_snp->kvm_finish_conf;
+
+    QTAILQ_FOREACH(data, &launch_update, next) {
+        ret = sev_snp_launch_update(sev_snp, data);
+        if (ret) {
+            exit(1);
+        }
+    }
+
+    trace_kvm_sev_snp_launch_finish(sev_snp->id_block, sev_snp->id_auth,
+                                    sev_snp->host_data);
+    ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_SNP_LAUNCH_FINISH,
+                    finish, &error);
+    if (ret) {
+        error_report("SNP_LAUNCH_FINISH ret=%d fw_error=%d '%s'",
+                     ret, error, fw_error_to_str(error));
+        exit(1);
+    }
+
+    sev_set_guest_state(sev_common, SEV_STATE_RUNNING);
+
+    /* add migration blocker */
+    error_setg(&sev_mig_blocker,
+               "SEV-SNP: Migration is not implemented");
+    ret = migrate_add_blocker(&sev_mig_blocker, &local_err);
+    if (local_err) {
+        error_report_err(local_err);
+        error_free(sev_mig_blocker);
+        exit(1);
+    }
+}
+
+
 static void
 sev_vm_state_change(void *opaque, bool running, RunState state)
 {
@@ -1832,10 +1942,10 @@ sev_snp_guest_class_init(ObjectClass *oc, void *data)
     X86ConfidentialGuestClass *x86_klass = X86_CONFIDENTIAL_GUEST_CLASS(oc);
 
     klass->launch_start = sev_snp_launch_start;
+    klass->launch_finish = sev_snp_launch_finish;
     klass->kvm_init = sev_snp_kvm_init;
     x86_klass->kvm_type = sev_snp_kvm_type;
 
-
     object_class_property_add(oc, "policy", "uint64",
                               sev_snp_guest_get_policy,
                               sev_snp_guest_set_policy, NULL, NULL);
diff --git a/target/i386/trace-events b/target/i386/trace-events
index cb26d8a9257..06b44ead2e2 100644
--- a/target/i386/trace-events
+++ b/target/i386/trace-events
@@ -12,3 +12,5 @@ kvm_sev_launch_finish(void) ""
 kvm_sev_launch_secret(uint64_t hpa, uint64_t hva, uint64_t secret, int len) "hpa 0x%" PRIx64 " hva 0x%" PRIx64 " data 0x%" PRIx64 " len %d"
 kvm_sev_attestation_report(const char *mnonce, const char *data) "mnonce %s data %s"
 kvm_sev_snp_launch_start(uint64_t policy, char *gosvw) "policy 0x%" PRIx64 " gosvw %s"
+kvm_sev_snp_launch_update(uint64_t src, uint64_t gpa, uint64_t len, const char *type) "src 0x%" PRIx64 " gpa 0x%" PRIx64 " len 0x%" PRIx64 " (%s page)"
+kvm_sev_snp_launch_finish(char *id_block, char *id_auth, char *host_data) "id_block %s id_auth %s host_data %s"
-- 
2.45.1
Re: [PULL 30/45] i386/sev: Add handling to encrypt/finalize guest launch data
Posted by Richard Henderson 5 months ago
On 6/3/24 23:43, Paolo Bonzini wrote:
> From: Brijesh Singh <brijesh.singh@amd.com>
> 
> Process any queued up launch data and encrypt/measure it into the SNP
> guest instance prior to initial guest launch.
> 
> This also updates the KVM_SEV_SNP_LAUNCH_UPDATE call to handle partial
> update responses.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> Co-developed-by: Michael Roth <michael.roth@amd.com>
> Signed-off-by: Michael Roth <michael.roth@amd.com>
> Co-developed-by: Pankaj Gupta <pankaj.gupta@amd.com>
> Signed-off-by: Pankaj Gupta <pankaj.gupta@amd.com>
> Message-ID: <20240530111643.1091816-17-pankaj.gupta@amd.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/sev.c        | 112 ++++++++++++++++++++++++++++++++++++++-
>   target/i386/trace-events |   2 +
>   2 files changed, 113 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index e89b87d2f55..ef2e592ca76 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -756,6 +756,76 @@ out:
>       return ret;
>   }
>   
> +static const char *
> +snp_page_type_to_str(int type)
> +{
> +    switch (type) {
> +    case KVM_SEV_SNP_PAGE_TYPE_NORMAL: return "Normal";
> +    case KVM_SEV_SNP_PAGE_TYPE_ZERO: return "Zero";
> +    case KVM_SEV_SNP_PAGE_TYPE_UNMEASURED: return "Unmeasured";
> +    case KVM_SEV_SNP_PAGE_TYPE_SECRETS: return "Secrets";
> +    case KVM_SEV_SNP_PAGE_TYPE_CPUID: return "Cpuid";
> +    default: return "unknown";
> +    }
> +}
> +
> +static int
> +sev_snp_launch_update(SevSnpGuestState *sev_snp_guest,
> +                      SevLaunchUpdateData *data)
> +{
> +    int ret, fw_error;
> +    struct kvm_sev_snp_launch_update update = {0};
> +
> +    if (!data->hva || !data->len) {
> +        error_report("SNP_LAUNCH_UPDATE called with invalid address"
> +                     "/ length: %p / %lx",
> +                     data->hva, data->len);

This patch does not compile on 32-bit x86:

../src/target/i386/sev.c: In function 'sev_snp_launch_update':
../src/target/i386/sev.c:886:22: error: format '%lx' expects argument of type 'long 
unsigned int', but argument 3 has type 'uint64_t' {aka 'long long unsigned int'} 
[-Werror=format=]
   886 |         error_report("SNP_LAUNCH_UPDATE called with invalid address"
       |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   887 |                      "/ length: %p / %lx",
   888 |                      data->hva, data->len);
       |                                 ~~~~~~~~~
       |                                     |
       |                                     uint64_t {aka long long unsigned int}
../src/target/i386/sev.c:935:22: error: format '%lx' expects argument of type 'long 
unsigned int', but argument 2 has type 'hwaddr' {aka 'long long unsigned int'} 
[-Werror=format=]
   935 |         error_report("SEV-SNP: expected update of GPA range %lx-%lx,"
       |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   936 |                      "got GPA range %lx-%llx",
   937 |                      data->gpa, data->gpa + data->len, data->gpa,
       |                      ~~~~~~~~~
       |                          |
       |                          hwaddr {aka long long unsigned int}
../src/target/i386/sev.c:935:22: error: format '%lx' expects argument of type 'long 
unsigned int', but argument 3 has type 'long long unsigned int' [-Werror=format=]
   935 |         error_report("SEV-SNP: expected update of GPA range %lx-%lx,"
       |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   936 |                      "got GPA range %lx-%llx",
   937 |                      data->gpa, data->gpa + data->len, data->gpa,
       |                                 ~~~~~~~~~~~~~~~~~~~~~
       |                                           |
       |                                           long long unsigned int
../src/target/i386/sev.c:935:22: error: format '%lx' expects argument of type 'long 
unsigned int', but argument 4 has type 'hwaddr' {aka 'long long unsigned int'} 
[-Werror=format=]
   935 |         error_report("SEV-SNP: expected update of GPA range %lx-%lx,"
       |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   936 |                      "got GPA range %lx-%llx",
   937 |                      data->gpa, data->gpa + data->len, data->gpa,
       |                                                        ~~~~~~~~~
       |                                                            |
       |                                                            hwaddr {aka long long 
unsigned int}
In file included from ../src/target/i386/sev.c:22:
../src/target/i386/sev.c: In function 'sev_snp_guest_set_guest_visible_workarounds':
/home/rth/qemu/src/include/qapi/error.h:319:25: error: format '%lu' expects argument of 
type 'long unsigned int', but argument 6 has type 'gsize' {aka 'unsigned int'} 
[-Werror=format=]
   319 |                         (fmt), ## __VA_ARGS__)
       |                         ^~~~~
../src/target/i386/sev.c:2149:9: note: in expansion of macro 'error_setg'
  2149 |         error_setg(errp, "parameter length of %lu exceeds max of %lu",
       |         ^~~~~~~~~~
/home/rth/qemu/src/include/qapi/error.h:319:25: error: format '%lu' expects argument of 
type 'long unsigned int', but argument 7 has type 'unsigned int' [-Werror=format=]
   319 |                         (fmt), ## __VA_ARGS__)
       |                         ^~~~~
../src/target/i386/sev.c:2149:9: note: in expansion of macro 'error_setg'
  2149 |         error_setg(errp, "parameter length of %lu exceeds max of %lu",
       |         ^~~~~~~~~~
../src/target/i386/sev.c: In function 'sev_snp_guest_set_id_block':
../src/target/i386/sev.c:2174:12: error: cast to pointer from integer of different size 
[-Werror=int-to-pointer-cast]
  2174 |     g_free((guchar *)finish->id_block_uaddr);
       |            ^
../src/target/i386/sev.c:2180:9: error: cast from pointer to integer of different size 
[-Werror=pointer-to-int-cast]
  2180 |         (uint64_t)qbase64_decode(sev_snp_guest->id_block, -1, &len, errp);
       |         ^
/home/rth/qemu/src/include/qapi/error.h:319:25: error: format '%lu' expects argument of 
type 'long unsigned int', but argument 6 has type 'gsize' {aka 'unsigned int'} 
[-Werror=format=]
   319 |                         (fmt), ## __VA_ARGS__)
       |                         ^~~~~
../src/target/i386/sev.c:2187:9: note: in expansion of macro 'error_setg'
  2187 |         error_setg(errp, "parameter length of %lu not equal to %u",
       |         ^~~~~~~~~~
../src/target/i386/sev.c: In function 'sev_snp_guest_set_id_auth':
../src/target/i386/sev.c:2211:12: error: cast to pointer from integer of different size 
[-Werror=int-to-pointer-cast]
  2211 |     g_free((guchar *)finish->id_auth_uaddr);
       |            ^
../src/target/i386/sev.c:2217:9: error: cast from pointer to integer of different size 
[-Werror=pointer-to-int-cast]
  2217 |         (uint64_t)qbase64_decode(sev_snp_guest->id_auth, -1, &len, errp);
       |         ^
/home/rth/qemu/src/include/qapi/error.h:319:25: error: format '%lu' expects argument of 
type 'long unsigned int', but argument 6 has type 'gsize' {aka 'unsigned int'} 
[-Werror=format=]
   319 |                         (fmt), ## __VA_ARGS__)
       |                         ^~~~~
../src/target/i386/sev.c:2224:9: note: in expansion of macro 'error_setg'
  2224 |         error_setg(errp, "parameter length:ID_AUTH %lu exceeds max of %u",
       |         ^~~~~~~~~~
../src/target/i386/sev.c: In function 'sev_snp_guest_set_host_data':
/home/rth/qemu/src/include/qapi/error.h:319:25: error: format '%lu' expects argument of 
type 'long unsigned int', but argument 6 has type 'gsize' {aka 'unsigned int'} 
[-Werror=format=]
   319 |                         (fmt), ## __VA_ARGS__)
       |                         ^~~~~
../src/target/i386/sev.c:2290:9: note: in expansion of macro 'error_setg'
  2290 |         error_setg(errp, "parameter length of %lu not equal to %lu",
       |         ^~~~~~~~~~
/home/rth/qemu/src/include/qapi/error.h:319:25: error: format '%lu' expects argument of 
type 'long unsigned int', but argument 7 has type 'unsigned int' [-Werror=format=]
   319 |                         (fmt), ## __VA_ARGS__)
       |                         ^~~~~
../src/target/i386/sev.c:2290:9: note: in expansion of macro 'error_setg'
  2290 |         error_setg(errp, "parameter length of %lu not equal to %lu",
       |         ^~~~~~~~~~
cc1: all warnings being treated as errors
ninja: build stopped: subcommand failed.


r~