diff mbox series

[RFC,08/16] target/arm/kvm-rme: Populate the realm with boot images

Message ID 20230127150727.612594-9-jean-philippe@linaro.org
State New
Headers show
Series arm: Run Arm CCA VMs with KVM | expand

Commit Message

Jean-Philippe Brucker Jan. 27, 2023, 3:07 p.m. UTC
Initialize the GPA space and populate it with boot images (kernel,
initrd, firmware, etc). Populating has to be done at VM start time,
because the images are loaded during reset by rom_reset()

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 target/arm/kvm_arm.h |  6 ++++
 target/arm/kvm-rme.c | 79 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)

Comments

Richard Henderson Jan. 27, 2023, 11:54 p.m. UTC | #1
On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> Initialize the GPA space and populate it with boot images (kernel,
> initrd, firmware, etc). Populating has to be done at VM start time,
> because the images are loaded during reset by rom_reset()
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>   target/arm/kvm_arm.h |  6 ++++
>   target/arm/kvm-rme.c | 79 ++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 85 insertions(+)
> 
> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> index e4dc7fbb8d..cec6500603 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -371,6 +371,7 @@ int kvm_arm_set_irq(int cpu, int irqtype, int irq, int level);
>   
>   int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp);
>   int kvm_arm_rme_vm_type(MachineState *ms);
> +void kvm_arm_rme_add_blob(hwaddr start, hwaddr src_size, hwaddr dst_size);
>   
>   bool kvm_arm_rme_enabled(void);
>   int kvm_arm_rme_vcpu_init(CPUState *cs);
> @@ -458,6 +459,11 @@ static inline int kvm_arm_rme_vm_type(MachineState *ms)
>   {
>       return 0;
>   }
> +
> +static inline void kvm_arm_rme_add_blob(hwaddr start, hwaddr src_size,
> +                                        hwaddr dst_size)
> +{
> +}
>   #endif
>   
>   static inline const char *gic_class_name(void)
> diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
> index 3833b187f9..c8c019f78a 100644
> --- a/target/arm/kvm-rme.c
> +++ b/target/arm/kvm-rme.c
> @@ -9,6 +9,7 @@
>   #include "exec/confidential-guest-support.h"
>   #include "hw/boards.h"
>   #include "hw/core/cpu.h"
> +#include "hw/loader.h"
>   #include "kvm_arm.h"
>   #include "migration/blocker.h"
>   #include "qapi/error.h"
> @@ -19,12 +20,22 @@
>   #define TYPE_RME_GUEST "rme-guest"
>   OBJECT_DECLARE_SIMPLE_TYPE(RmeGuest, RME_GUEST)
>   
> +#define RME_PAGE_SIZE qemu_real_host_page_size()
> +
>   typedef struct RmeGuest RmeGuest;
>   
>   struct RmeGuest {
>       ConfidentialGuestSupport parent_obj;
>   };
>   
> +struct RmeImage {
> +    hwaddr base;
> +    hwaddr src_size;
> +    hwaddr dst_size;
> +};
> +
> +static GSList *rme_images;
> +
>   static RmeGuest *cgs_to_rme(ConfidentialGuestSupport *cgs)
>   {
>       if (!cgs) {
> @@ -51,6 +62,38 @@ static int rme_create_rd(RmeGuest *guest, Error **errp)
>       return ret;
>   }
>   
> +static void rme_populate_realm(gpointer data, gpointer user_data)
> +{
> +    int ret;
> +    struct RmeImage *image = data;
> +    struct kvm_cap_arm_rme_init_ipa_args init_args = {
> +        .init_ipa_base = image->base,
> +        .init_ipa_size = image->dst_size,
> +    };
> +    struct kvm_cap_arm_rme_populate_realm_args populate_args = {
> +        .populate_ipa_base = image->base,
> +        .populate_ipa_size = image->src_size,
> +    };
> +
> +    ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0,
> +                            KVM_CAP_ARM_RME_INIT_IPA_REALM,
> +                            (intptr_t)&init_args);
> +    if (ret) {
> +        error_setg_errno(&error_fatal, -ret,
> +                         "RME: failed to initialize GPA range (0x%"HWADDR_PRIx", 0x%"HWADDR_PRIx")",
> +                         image->base, image->dst_size);
> +    }

Using error_fatal with error_setg is discouraged.
This should be error_report(), exit().


> +
> +    ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0,
> +                            KVM_CAP_ARM_RME_POPULATE_REALM,
> +                            (intptr_t)&populate_args);
> +    if (ret) {
> +        error_setg_errno(&error_fatal, -ret,
> +                         "RME: failed to populate realm (0x%"HWADDR_PRIx", 0x%"HWADDR_PRIx")",
> +                         image->base, image->src_size);
> +    }
> +}
> +
>   static void rme_vm_state_change(void *opaque, bool running, RunState state)
>   {
>       int ret;
> @@ -72,6 +115,9 @@ static void rme_vm_state_change(void *opaque, bool running, RunState state)
>           }
>       }
>   
> +    g_slist_foreach(rme_images, rme_populate_realm, NULL);
> +    g_slist_free_full(g_steal_pointer(&rme_images), g_free);

I suppose this technically works because you clear the list, and thus the hook is called 
only on the first transition to RUNNING.  On all subsequent transitions the list is empty.

I see that i386 sev does this immediately during machine init, alongside the kernel setup. 
  Since kvm_init has already been called, that seems workable, rather than queuing 
anything for later.

But I think ideally this would be handled generically in (approximately) 
kvm_cpu_synchronize_post_init, looping over all blobs.  This would handle any usage of 
'-device loader,...', instead of the 4 specific things you handle in the next patch.


r~
Jean-Philippe Brucker Feb. 8, 2023, 12:10 p.m. UTC | #2
On Fri, Jan 27, 2023 at 01:54:23PM -1000, Richard Henderson wrote:
> >   static void rme_vm_state_change(void *opaque, bool running, RunState state)
> >   {
> >       int ret;
> > @@ -72,6 +115,9 @@ static void rme_vm_state_change(void *opaque, bool running, RunState state)
> >           }
> >       }
> > +    g_slist_foreach(rme_images, rme_populate_realm, NULL);
> > +    g_slist_free_full(g_steal_pointer(&rme_images), g_free);
> 
> I suppose this technically works because you clear the list, and thus the
> hook is called only on the first transition to RUNNING.  On all subsequent
> transitions the list is empty.
> 
> I see that i386 sev does this immediately during machine init, alongside the
> kernel setup.  Since kvm_init has already been called, that seems workable,
> rather than queuing anything for later.

The problem I faced was that RME_POPULATE_REALM needs to be called after
rom_reset(), which copies all the blobs into guest memory, and that
happens at device reset time, after machine init and
kvm_cpu_synchronize_post_init().

> But I think ideally this would be handled generically in (approximately)
> kvm_cpu_synchronize_post_init, looping over all blobs.  This would handle
> any usage of '-device loader,...', instead of the 4 specific things you
> handle in the next patch.

I'd definitely prefer something generic that hooks into the loader, I'll
look into that. I didn't do it right away because the arm64 Linux kernel
loading is special, requires reserving extra RAM in addition to the blob
(hence the two parameters to kvm_arm_rme_add_blob()). But we could just
have a special case for the extra memory needed by Linux and make the rest
generic.

Thanks,
Jean
diff mbox series

Patch

diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index e4dc7fbb8d..cec6500603 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -371,6 +371,7 @@  int kvm_arm_set_irq(int cpu, int irqtype, int irq, int level);
 
 int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp);
 int kvm_arm_rme_vm_type(MachineState *ms);
+void kvm_arm_rme_add_blob(hwaddr start, hwaddr src_size, hwaddr dst_size);
 
 bool kvm_arm_rme_enabled(void);
 int kvm_arm_rme_vcpu_init(CPUState *cs);
@@ -458,6 +459,11 @@  static inline int kvm_arm_rme_vm_type(MachineState *ms)
 {
     return 0;
 }
+
+static inline void kvm_arm_rme_add_blob(hwaddr start, hwaddr src_size,
+                                        hwaddr dst_size)
+{
+}
 #endif
 
 static inline const char *gic_class_name(void)
diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
index 3833b187f9..c8c019f78a 100644
--- a/target/arm/kvm-rme.c
+++ b/target/arm/kvm-rme.c
@@ -9,6 +9,7 @@ 
 #include "exec/confidential-guest-support.h"
 #include "hw/boards.h"
 #include "hw/core/cpu.h"
+#include "hw/loader.h"
 #include "kvm_arm.h"
 #include "migration/blocker.h"
 #include "qapi/error.h"
@@ -19,12 +20,22 @@ 
 #define TYPE_RME_GUEST "rme-guest"
 OBJECT_DECLARE_SIMPLE_TYPE(RmeGuest, RME_GUEST)
 
+#define RME_PAGE_SIZE qemu_real_host_page_size()
+
 typedef struct RmeGuest RmeGuest;
 
 struct RmeGuest {
     ConfidentialGuestSupport parent_obj;
 };
 
+struct RmeImage {
+    hwaddr base;
+    hwaddr src_size;
+    hwaddr dst_size;
+};
+
+static GSList *rme_images;
+
 static RmeGuest *cgs_to_rme(ConfidentialGuestSupport *cgs)
 {
     if (!cgs) {
@@ -51,6 +62,38 @@  static int rme_create_rd(RmeGuest *guest, Error **errp)
     return ret;
 }
 
+static void rme_populate_realm(gpointer data, gpointer user_data)
+{
+    int ret;
+    struct RmeImage *image = data;
+    struct kvm_cap_arm_rme_init_ipa_args init_args = {
+        .init_ipa_base = image->base,
+        .init_ipa_size = image->dst_size,
+    };
+    struct kvm_cap_arm_rme_populate_realm_args populate_args = {
+        .populate_ipa_base = image->base,
+        .populate_ipa_size = image->src_size,
+    };
+
+    ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0,
+                            KVM_CAP_ARM_RME_INIT_IPA_REALM,
+                            (intptr_t)&init_args);
+    if (ret) {
+        error_setg_errno(&error_fatal, -ret,
+                         "RME: failed to initialize GPA range (0x%"HWADDR_PRIx", 0x%"HWADDR_PRIx")",
+                         image->base, image->dst_size);
+    }
+
+    ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0,
+                            KVM_CAP_ARM_RME_POPULATE_REALM,
+                            (intptr_t)&populate_args);
+    if (ret) {
+        error_setg_errno(&error_fatal, -ret,
+                         "RME: failed to populate realm (0x%"HWADDR_PRIx", 0x%"HWADDR_PRIx")",
+                         image->base, image->src_size);
+    }
+}
+
 static void rme_vm_state_change(void *opaque, bool running, RunState state)
 {
     int ret;
@@ -72,6 +115,9 @@  static void rme_vm_state_change(void *opaque, bool running, RunState state)
         }
     }
 
+    g_slist_foreach(rme_images, rme_populate_realm, NULL);
+    g_slist_free_full(g_steal_pointer(&rme_images), g_free);
+
     ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0,
                             KVM_CAP_ARM_RME_ACTIVATE_REALM);
     if (ret) {
@@ -118,6 +164,39 @@  int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp)
     return 0;
 }
 
+/*
+ * kvm_arm_rme_add_blob - Initialize the Realm IPA range and set up the image.
+ *
+ * @src_size is the number of bytes of the source image, to be populated into
+ *   Realm memory.
+ * @dst_size is the effective image size, which may be larger than @src_size.
+ *   For a kernel @dst_size may include zero-initialized regions such as the BSS
+ *   and initial page directory.
+ */
+void kvm_arm_rme_add_blob(hwaddr base, hwaddr src_size, hwaddr dst_size)
+{
+    struct RmeImage *image;
+
+    if (!kvm_arm_rme_enabled()) {
+        return;
+    }
+
+    base = QEMU_ALIGN_DOWN(base, RME_PAGE_SIZE);
+    src_size = QEMU_ALIGN_UP(src_size, RME_PAGE_SIZE);
+    dst_size = QEMU_ALIGN_UP(dst_size, RME_PAGE_SIZE);
+
+    image = g_malloc0(sizeof(*image));
+    image->base = base;
+    image->src_size = src_size;
+    image->dst_size = dst_size;
+
+    /*
+     * The ROM loader will only load the images during reset, so postpone the
+     * populate call until VM start.
+     */
+    rme_images = g_slist_prepend(rme_images, image);
+}
+
 int kvm_arm_rme_vcpu_init(CPUState *cs)
 {
     ARMCPU *cpu = ARM_CPU(cs);