On 02/02/2025 06:00, Gavin Shan wrote:

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

quoted

From: Suzuki K Poulose <suzuki.poulose@arm.com>

Given we have different types of VMs supported, check the
support for SVE for the given instance of the VM to accurately
report the status.

Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Steven Price <steven.price@arm.com>
---
  arch/arm64/include/asm/kvm_rme.h | 2 ++
  arch/arm64/kvm/arm.c             | 5 ++++-
  arch/arm64/kvm/rme.c             | 5 +++++
  3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/kvm_rme.h b/arch/arm64/include/

asm/kvm_rme.h
index 90a4537ad38d..0d89ab1645c1 100644

--- a/arch/arm64/include/asm/kvm_rme.h
+++ b/arch/arm64/include/asm/kvm_rme.h

@@ -85,6 +85,8 @@ void kvm_init_rme(void);

  u32 kvm_realm_ipa_limit(void);
  u32 kvm_realm_vgic_nr_lr(void);
  +bool kvm_rme_supports_sve(void);
+
  int kvm_realm_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap);
  int kvm_init_realm_vm(struct kvm *kvm);
  void kvm_destroy_realm(struct kvm *kvm);

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 134acb4ee26f..6f7f96ab781d 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c

@@ -456,7 +456,10 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm,

long ext)
          r = get_kvm_ipa_limit();
          break;
      case KVM_CAP_ARM_SVE:
-        r = system_supports_sve();
+        if (kvm_is_realm(kvm))
+            r = kvm_rme_supports_sve();
+        else
+            r = system_supports_sve();
          break;

kvm_vm_ioctl_check_extension() can be called by
ioctl(KVM_CHECK_EXTENSION) on the
file descriptor of '/dev/kvm'. kvm is NULL and kvm_is_realm() returns
false in
this case.

kvm_dev_ioctl
  kvm_vm_ioctl_check_extension_generic  // kvm is NULL
    kvm_vm_ioctl_check_extension

See my reply in patch 25

quoted

      case KVM_CAP_ARM_PTRAUTH_ADDRESS:
      case KVM_CAP_ARM_PTRAUTH_GENERIC:

diff --git a/arch/arm64/kvm/rme.c b/arch/arm64/kvm/rme.c
index 5831d379760a..27a479feb907 100644
--- a/arch/arm64/kvm/rme.c
+++ b/arch/arm64/kvm/rme.c

@@ -20,6 +20,11 @@ static bool rme_supports(unsigned long feature)

      return !!u64_get_bits(rmm_feat_reg0, feature);
  }
  +bool kvm_rme_supports_sve(void)
+{
+    return rme_supports(RMI_FEATURE_REGISTER_0_SVE_EN);
+}
+

If rme_supports() becomes a public helper, it can be directly used. In
turn,
kvm_rme_supports_sve() can be dropped. RMI_FEATURE_REGISTER_0_SVE_EN is
obvious
to indicate the corresponding feature.

I agree this seem reasonable. Sadly the use of u64_get_bits() is
assuming that the 'feature' parameter is constant at runtime. I could
rework it to not require a constant, but considering this is the only
function which is exposing rme_supports() without any further checks I
feel it's better to leave the code as it is.

Obviously if we get more feature bits like this in the future then it
would be worth revisiting.

Thanks,
Steve

quoted

  static int rmi_check_version(void)
  {
      struct arm_smccc_res res;

Thanks,
Gavin