On 29/01/2025 04:07, Gavin Shan wrote:

On 12/13/24 1:55 AM, Steven Price wrote:

quoted

Previously machine type was used purely for specifying the physical
address size of the guest. Reserve the higher bits to specify an ARM
specific machine type and declare a new type 'KVM_VM_TYPE_ARM_REALM'
used to create a realm guest.

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Steven Price <steven.price@arm.com>
---
  arch/arm64/kvm/arm.c     | 17 +++++++++++++++++
  arch/arm64/kvm/mmu.c     |  3 ---
  include/uapi/linux/kvm.h | 19 +++++++++++++++----
  3 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index c505ec61180a..73016e1e0067 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c

@@ -207,6 +207,23 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned

long type)
      mutex_unlock(&kvm->lock);
  #endif
  +    if (type & ~(KVM_VM_TYPE_ARM_MASK |
KVM_VM_TYPE_ARM_IPA_SIZE_MASK))
+        return -EINVAL;
+
+    switch (type & KVM_VM_TYPE_ARM_MASK) {
+    case KVM_VM_TYPE_ARM_NORMAL:
+        break;
+    case KVM_VM_TYPE_ARM_REALM:
+        kvm->arch.is_realm = true;
+        if (!kvm_is_realm(kvm)) {
+            /* Realm support unavailable */
+            return -EINVAL;
+        }
+        break;
+    default:
+        return -EINVAL;
+    }
+
      kvm_init_nested(kvm);
        ret = kvm_share_hyp(kvm, kvm + 1);

Corresponding to comments for PATCH[6], the block of the code can be
modified
to avoid using kvm_is_realm() here. In this way, kvm_is_realm() can be
simplifed
as I commented for PATCH[6].

    case KVM_VM_TYPE_ARM_REALM:
        if (static_branch_unlikely(&kvm_rme_is_available))
            return -EPERM;    /* -EPERM may be more suitable than -
EINVAL */

        kvm->arch.is_realm = true;
        break;

Yes that's more readable. I'd used kvm_is_realm() because I wanted to
keep the check on kvm_rme_is_available to one place, but coming back to
the code there's definitely a "Huh?" moment from setting 'is_realm' and
then testing if it's a realm!

I also agree -EPERM is probably better to signify that the kernel
supports realms but the hardware doesn't.

Thanks,

Steve