Re: [PATCH v22 19/23] KVM: arm64: CCA: Don't expose unsupported capabilities for realm guests

[Gavin Shan <gshan@redhat.com>]

On 10/5/26 7:07 PM, Suzuki K Poulose wrote:
> Limit the capabilities that are allowed for Realm VMs. Similarly block
> the vm_ioctls backed by the capabilities.
> 
> Repurpose the kvm_pkvm_ioctl_allowed() to support both pKVM and Realm
> ioctls. Rename the helper to kvm_vm_ioctl_allowed() and move it
> into arch/arm64/kvm/arm.c. Also add a generic kvm_vm_ext_allowed()
> to handle pKVM and Realm capability filtering and route them accordingly.
> 
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> Changes since v21:
>   - Drop WARN_ON_ONCE() from kvm_vm_ioctl_allowed to synchronise with
>     what is in -rc5 and make the conflict resolution easier.
> ---
>   arch/arm64/include/asm/kvm_pkvm.h | 19 ------------
>   arch/arm64/include/asm/kvm_rmi.h  | 24 +++++++++++++++
>   arch/arm64/kvm/arm.c              | 49 +++++++++++++++++++++++++++++--
>   3 files changed, 70 insertions(+), 22 deletions(-)
> 

One nitpick below. In either way:

Reviewed-by: Gavin Shan <gshan@redhat.com>

> diff --git a/arch/arm64/include/asm/kvm_pkvm.h b/arch/arm64/include/asm/kvm_pkvm.h
> index 2addc37c500e1..b833abb1be9c7 100644
> --- a/arch/arm64/include/asm/kvm_pkvm.h
> +++ b/arch/arm64/include/asm/kvm_pkvm.h
> @@ -53,25 +53,6 @@ static inline bool kvm_pkvm_ext_allowed(struct kvm *kvm, long ext)
>   	}
>   }
>   
> -/*
> - * Check whether the KVM VM IOCTL is allowed in pKVM.
> - *
> - * Certain features are allowed only for non-protected VMs in pKVM, which is why
> - * this takes the VM (kvm) as a parameter.
> - */
> -static inline bool kvm_pkvm_ioctl_allowed(struct kvm *kvm, unsigned int ioctl)
> -{
> -	long ext;
> -	int r;
> -
> -	r = kvm_get_cap_for_kvm_ioctl(ioctl, &ext);
> -
> -	if (WARN_ON_ONCE(r < 0))
> -		return false;
> -
> -	return kvm_pkvm_ext_allowed(kvm, ext);
> -}
> -
>   extern struct memblock_region kvm_nvhe_sym(hyp_memory)[];
>   extern unsigned int kvm_nvhe_sym(hyp_memblock_nr);
>   
> diff --git a/arch/arm64/include/asm/kvm_rmi.h b/arch/arm64/include/asm/kvm_rmi.h
> index 44f5c75a27b5b..ea1450e0c8619 100644
> --- a/arch/arm64/include/asm/kvm_rmi.h
> +++ b/arch/arm64/include/asm/kvm_rmi.h
> @@ -6,6 +6,8 @@
>   #ifndef __ASM_KVM_RMI_H
>   #define __ASM_KVM_RMI_H
>   
> +#include <linux/kvm.h>
> +
>   /**
>    * enum realm_state - State of a Realm
>    *
> @@ -58,4 +60,26 @@ struct realm {
>   
>   void kvm_init_rmi(void);
>   
> +static inline bool kvm_realm_ext_allowed(long ext)
> +{
> +	switch (ext) {
> +	case KVM_CAP_IRQCHIP:
> +	case KVM_CAP_ARM_PSCI:
> +	case KVM_CAP_ARM_PSCI_0_2:
> +	case KVM_CAP_DEVICE_CTRL:
> +	case KVM_CAP_NR_VCPUS:
> +	case KVM_CAP_MAX_VCPUS:
> +	case KVM_CAP_MAX_VCPU_ID:
> +	case KVM_CAP_MSI_DEVID:
> +	case KVM_CAP_ARM_VM_IPA_SIZE:
> +	case KVM_CAP_ARM_SVE:
> +	case KVM_CAP_ONE_REG:
> +	case KVM_CAP_ARM_PTRAUTH_ADDRESS:
> +	case KVM_CAP_ARM_PTRAUTH_GENERIC:
> +	case KVM_CAP_SYNC_MMU:
> +		return true;
> +	}
> +	return false;
> +}
> +
>   #endif /* __ASM_KVM_RMI_H */
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 37ef2ba4e43e2..fe707a0c47308 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -136,6 +136,49 @@ int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
>   	return kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE;
>   }
>   
> +static inline bool kvm_vm_ext_allowed(struct kvm *kvm, long ext)
> +{
> +	/*
> +	 * We could be called with kvm as NULL, so can't use kvm_vm_* for pKVM
> +	 * flavors
> +	 */
> +	if (is_protected_kvm_enabled())
> +		return kvm_pkvm_ext_allowed(kvm, ext);
> +	else if (kvm && kvm_vm_is_realm(kvm))
> +		return kvm_realm_ext_allowed(ext);
> +	else
> +		return true;
> +}
> +
> +/*
> + * Check whether the KVM VM IOCTL is allowed. For pKVM and Realm VMs, certain
> + * ioctls are not allowed. Further, certain features are allowed only for
> + * non-protected VMs in pKVM.
> + */
> +static inline bool kvm_vm_ioctl_allowed(struct kvm *kvm, unsigned int ioctl)
> +{
> +	long ext;
> +	int r;

	long ext, r;

	kvm_get_cap_for_kvm_ioctl() returns 'long' instead of 'int' value.

> +
> +	/*
> +	 * We are guaranteed to be called with a valid kvm instance, as the
> +	 * only caller is kvm_arch_vm_ioctl(). Catch any deviations, as we
> +	 * rely on the kvm instance below.
> +	 */
> +	if (WARN_ON_ONCE(!kvm))
> +		return false;
> +
> +	/* Cover both pKVM host and Realm VMs */
> +	if (!kvm_vm_hyp_is_distrusting(kvm))
> +		return true;
> +
> +	r = kvm_get_cap_for_kvm_ioctl(ioctl, &ext);
> +	if (r < 0)
> +		return false;
> +
> +	return kvm_vm_ext_allowed(kvm, ext);
> +}
> +
>   int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>   			    struct kvm_enable_cap *cap)
>   {
> @@ -144,7 +187,7 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>   	if (cap->flags)
>   		return -EINVAL;
>   
> -	if (is_protected_kvm_enabled() && !kvm_pkvm_ext_allowed(kvm, cap->cap))
> +	if (!kvm_vm_ext_allowed(kvm, cap->cap))
>   		return -EINVAL;
>   
>   	switch (cap->cap) {
> @@ -403,7 +446,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>   {
>   	int r;
>   
> -	if (is_protected_kvm_enabled() && !kvm_pkvm_ext_allowed(kvm, ext))
> +	if (!kvm_vm_ext_allowed(kvm, ext))
>   		return 0;
>   
>   	switch (ext) {
> @@ -2146,7 +2189,7 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>   	void __user *argp = (void __user *)arg;
>   	struct kvm_device_attr attr;
>   
> -	if (is_protected_kvm_enabled() && !kvm_pkvm_ioctl_allowed(kvm, ioctl))
> +	if (!kvm_vm_ioctl_allowed(kvm, ioctl))
>   		return -EINVAL;
>   
>   	switch (ioctl) {

Thanks,
Gavin