| Message ID | 20230127150727.612594-4-jean-philippe@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | arm: Run Arm CCA VMs with KVM | expand |
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~
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/20220519134204.5379-1-will@kernel.org/) Thanks, Jean
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) { }
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 <jean-philippe@linaro.org> --- target/arm/kvm_arm.h | 14 ++++++ target/arm/kvm-rme.c | 101 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+)