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~
