[RFC,03/16] target/arm/kvm-rme: Initialize realm

Commit Message

Jean-Philippe Brucker Jan. 27, 2023, 3:07 p.m. UTC

The machine code calls kvm_arm_rme_vm_type() to get the VM flag and
kvm_arm_rme_init() to issue KVM hypercalls in the required order:

* create the realm descriptor early,
* finalize the REC (vCPU) after the registers are reset,
* load images into Realm RAM (in another patch),
* activate the realm at the end, at which point the realm is sealed.

Signed-off-by: Jean-Philippe Brucker <[e-mail]>
---
 target/arm/kvm_arm.h |  14 ++++++
 target/arm/kvm-rme.c | 101 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 115 insertions(+)

Comments

Richard Henderson Jan. 27, 2023, 8:37 p.m. UTC | #1

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> +static inline int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp)
> +{
> +    return 0;
> +}
> +
> +static inline int kvm_arm_rme_vm_type(MachineState *ms)
> +{
> +    return 0;
> +}

Should the stubs really return 0, or g_assert_not_reached()?

> +static RmeGuest *cgs_to_rme(ConfidentialGuestSupport *cgs)
> +{
> +    if (!cgs) {
> +        return NULL;
> +    }
> +    return (RmeGuest *)object_dynamic_cast(OBJECT(cgs), TYPE_RME_GUEST);
> +}

Direct usage of object_dynamic_cast is usually not correct.

Normally one would use DECLARE_INSTANCE_CHECKER to define the entire function.  But if you 
prefer to type-check the input argument as ConfidentialGuestSupport I can see defining 
your own function.  But in that case I think you probably want to use OBJECT_CHECK, which 
asserts that the cast succeeds.

But then I see that cgs_to_rme is, in all instances so far, also used as a predicate.


> +bool kvm_arm_rme_enabled(void)
> +{
> +    ConfidentialGuestSupport *cgs = MACHINE(qdev_get_machine())->cgs;
> +
> +    return !!cgs_to_rme(cgs);
> +}

Ah, hmm.  Well, probably wouldn't want an assert here.

At present I would expect exactly one object class to be present in the 
qemu-system-aarch64 binary that would pass the machine_check_confidential_guest_support 
test done by core code.  But we are hoping to move toward a heterogeneous model where e.g. 
the TYPE_SEV_GUEST object might be discoverable within the same executable.

Therefore, if cgs_to_rme above uses assert, this might use object_dynamic_cast here 
directly.  It seems like we ought to have a boolean test for this kind of thing, but no.


> +int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp)
> +{
> +    int ret;
> +    static Error *rme_mig_blocker;
> +    RmeGuest *guest = cgs_to_rme(cgs);
> +
> +    if (!guest) {
> +        /* Either no cgs, or another confidential guest type */
> +        return 0;
> +    }
> +
> +    if (!kvm_enabled()) {
> +        error_setg(errp, "KVM required for RME");
> +        return -ENODEV;
> +    }

I think this null check, and the kvm_enabled check, belong in virt.c.
You'll not get the error with the !CONFIG_KVM version of this function above.


r~

Jean-Philippe Brucker Feb. 8, 2023, 12:07 p.m. UTC | #2

Hi Richard,

Thanks a lot for the review

On Fri, Jan 27, 2023 at 10:37:12AM -1000, Richard Henderson wrote:
> At present I would expect exactly one object class to be present in the
> qemu-system-aarch64 binary that would pass the
> machine_check_confidential_guest_support test done by core code.  But we are
> hoping to move toward a heterogeneous model where e.g. the TYPE_SEV_GUEST
> object might be discoverable within the same executable.

Yes, I'm not sure SEV can be supported on qemu-system-aarch64, but pKVM could
probably coexist with RME as another type of confidential guest support
(https://lwn.net/ml/linux-arm-kernel/[e-mail]/)

Thanks,
Jean

Patch

diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 99017b635c..00d3df8cac 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -369,6 +369,11 @@  void kvm_arm_pvtime_init(CPUState *cs, uint64_t ipa);
 
 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);
+
+bool kvm_arm_rme_enabled(void);
+
 #else
 
 /*
@@ -443,6 +448,15 @@  static inline uint32_t kvm_arm_sve_get_vls(CPUState *cs)
     g_assert_not_reached();
 }
 
+static inline int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp)
+{
+    return 0;
+}
+
+static inline int kvm_arm_rme_vm_type(MachineState *ms)
+{
+    return 0;
+}
 #endif
 
 static inline const char *gic_class_name(void)
diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
index 22aa3dc712..d7cdca1cbf 100644
--- a/target/arm/kvm-rme.c
+++ b/target/arm/kvm-rme.c
@@ -25,6 +25,107 @@  struct RmeGuest {
     ConfidentialGuestSupport parent_obj;
 };
 
+static RmeGuest *cgs_to_rme(ConfidentialGuestSupport *cgs)
+{
+    if (!cgs) {
+        return NULL;
+    }
+    return (RmeGuest *)object_dynamic_cast(OBJECT(cgs), TYPE_RME_GUEST);
+}
+
+bool kvm_arm_rme_enabled(void)
+{
+    ConfidentialGuestSupport *cgs = MACHINE(qdev_get_machine())->cgs;
+
+    return !!cgs_to_rme(cgs);
+}
+
+static int rme_create_rd(RmeGuest *guest, Error **errp)
+{
+    int ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0,
+                                KVM_CAP_ARM_RME_CREATE_RD);
+
+    if (ret) {
+        error_setg_errno(errp, -ret, "RME: failed to create Realm Descriptor");
+    }
+    return ret;
+}
+
+static void rme_vm_state_change(void *opaque, bool running, RunState state)
+{
+    int ret;
+    CPUState *cs;
+
+    if (state != RUN_STATE_RUNNING) {
+        return;
+    }
+
+    /*
+     * Now that do_cpu_reset() initialized the boot PC and
+     * kvm_cpu_synchronize_post_reset() registered it, we can finalize the REC.
+     */
+    CPU_FOREACH(cs) {
+        ret = kvm_arm_vcpu_finalize(cs, KVM_ARM_VCPU_REC);
+        if (ret) {
+            error_setg_errno(&error_fatal, -ret,
+                             "RME: failed to finalize vCPU");
+        }
+    }
+
+    ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0,
+                            KVM_CAP_ARM_RME_ACTIVATE_REALM);
+    if (ret) {
+        error_setg_errno(&error_fatal, -ret, "RME: failed to activate realm");
+    }
+}
+
+int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp)
+{
+    int ret;
+    static Error *rme_mig_blocker;
+    RmeGuest *guest = cgs_to_rme(cgs);
+
+    if (!guest) {
+        /* Either no cgs, or another confidential guest type */
+        return 0;
+    }
+
+    if (!kvm_enabled()) {
+        error_setg(errp, "KVM required for RME");
+        return -ENODEV;
+    }
+
+    if (!kvm_check_extension(kvm_state, KVM_CAP_ARM_RME)) {
+        error_setg(errp, "KVM does not support RME");
+        return -ENODEV;
+    }
+
+    ret = rme_create_rd(guest, errp);
+    if (ret) {
+        return ret;
+    }
+
+    error_setg(&rme_mig_blocker, "RME: migration is not implemented");
+    migrate_add_blocker(rme_mig_blocker, &error_fatal);
+
+    /*
+     * The realm activation is done last, when the VM starts, after all images
+     * have been loaded and all vcpus finalized.
+     */
+    qemu_add_vm_change_state_handler(rme_vm_state_change, guest);
+
+    cgs->ready = true;
+    return 0;
+}
+
+int kvm_arm_rme_vm_type(MachineState *ms)
+{
+    if (cgs_to_rme(ms->cgs)) {
+        return KVM_VM_TYPE_ARM_REALM;
+    }
+    return 0;
+}
+
 static void rme_guest_class_init(ObjectClass *oc, void *data)
 {
 }