Re: [PATCH v5 5/9] KVM: arm64: Include VM type when checking VM capabilities in pKVM

[Marc Zyngier <maz@kernel.org>]

On Tue, 18 Nov 2025 10:38:02 +0000,
Fuad Tabba <tabba@google.com> wrote:
> 
> Certian features and capabilities are restricted in protected mode. Most
> of these features are restricted only for protected VMs, but some
> are restricted for ALL VMs in protected mode.
> 
> Extend the pKVM capability check to pass the VM (kvm), and use that when
> determining supported features. Moreover, extend the check to disallow
> MTE for all VM types in protected mode.
> 
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  arch/arm64/include/asm/kvm_pkvm.h | 10 ++++++----
>  arch/arm64/kvm/arm.c              |  4 ++--
>  arch/arm64/kvm/hyp/nvhe/pkvm.c    | 10 +++++-----
>  3 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_pkvm.h b/arch/arm64/include/asm/kvm_pkvm.h
> index 08be89c95466..7195be508d99 100644
> --- a/arch/arm64/include/asm/kvm_pkvm.h
> +++ b/arch/arm64/include/asm/kvm_pkvm.h
> @@ -23,10 +23,12 @@ void pkvm_destroy_hyp_vm(struct kvm *kvm);
>  int pkvm_create_hyp_vcpu(struct kvm_vcpu *vcpu);
>  
>  /*
> - * This functions as an allow-list of protected VM capabilities.
> - * Features not explicitly allowed by this function are denied.
> + * Check whether the specific capability 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_pvm_ext_allowed(long ext)
> +static inline bool kvm_pkvm_ext_allowed(struct kvm *kvm, long ext)
>  {
>  	switch (ext) {
>  	case KVM_CAP_IRQCHIP:
> @@ -43,7 +45,7 @@ static inline bool kvm_pvm_ext_allowed(long ext)
>  	case KVM_CAP_ARM_PTRAUTH_GENERIC:
>  		return true;
>  	default:
> -		return false;
> +		return !kvm || !kvm_vm_is_protected(kvm);

I find this expression a bit unreadable. IMO it would be better if
written as:

		return !(kvm && kvm_vm_is_protected(kvm));

which makes it "clear" that you claim to support everything when
either kvm == NULL or described an unprotected VM.

But it then begs the question:

- in what circumstances is kvm == NULL? Is there any case outside of
  kvm_vm_check_extension()?

- do you really support everything?

>  	}
>  }
>  
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 870953b4a8a7..10d853f2722e 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -87,7 +87,7 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>  	if (cap->flags)
>  		return -EINVAL;
>  
> -	if (kvm_vm_is_protected(kvm) && !kvm_pvm_ext_allowed(cap->cap))
> +	if (is_protected_kvm_enabled() && !kvm_pkvm_ext_allowed(kvm, cap->cap))
>  		return -EINVAL;
>  
>  	switch (cap->cap) {
> @@ -299,7 +299,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  {
>  	int r;
>  
> -	if (kvm && kvm_vm_is_protected(kvm) && !kvm_pvm_ext_allowed(ext))
> +	if (is_protected_kvm_enabled() && !kvm_pkvm_ext_allowed(kvm, ext))
>  		return 0;

But that's a change in semantics here. Calling this function outside
of the context of a VM (kvm == NULL) will now report that *ALL*
extensions are valid, instead of limiting it to the allow-list.

With that, userspace can no longer detect what is available and what
isn't before creating a VM with the correct VM type.

I don't think this is the right way to do this, unless you really want
to break the existing UAPI (rhetorical question).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.