Re: [PATCH v22 09/23] KVM: arm64: Prevent unsupported vcpu features for VM types

[Suzuki K Poulose <suzuki.poulose@arm.com>]

On 06/10/2026 03:25, Gavin Shan wrote:
> On 10/5/26 7:07 PM, Suzuki K Poulose wrote:
>> Prevent unsupported VCPU features for the protected VCPUs. Realms and 
>> pVMs
>> not support 32bit EL1 or NV yet. pKVM doesn't rely on the host vcpu
>> features and it clears the unsupported features while hyp_vcpu is
>> initialised. Block the features early in the vcpu init if we detect
>> incompatible features.
>>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>   arch/arm64/kvm/arm.c | 10 ++++++----
>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>
> 
> One nitpick below. In either way:
> 
> Reviewed-by: Gavin Shan <gshan@redhat.com>

Thanks !

> 
>> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
>> index f31d31fa27ad9..9c2ef7ca6a961 100644
>> --- a/arch/arm64/kvm/arm.c
>> +++ b/arch/arm64/kvm/arm.c
>> @@ -1668,11 +1668,12 @@ int kvm_vm_ioctl_irq_line(struct kvm *kvm, 
>> struct kvm_irq_level *irq_level,
>>       return -EINVAL;
>>   }
>> -static unsigned long system_supported_vcpu_features(void)
>> +static unsigned long system_supported_vcpu_features(struct kvm_vcpu 
>> *vcpu)
>>   {
>>       unsigned long features = KVM_VCPU_VALID_FEATURES;
>> -    if (!cpus_have_final_cap(ARM64_HAS_32BIT_EL1))
>> +    if (vcpu_is_protected(vcpu) ||
>> +        !cpus_have_final_cap(ARM64_HAS_32BIT_EL1))
>>           clear_bit(KVM_ARM_VCPU_EL1_32BIT, &features);
>>       if (!kvm_supports_guest_pmuv3()) {
>> @@ -1688,7 +1689,8 @@ static unsigned long 
>> system_supported_vcpu_features(void)
>>           clear_bit(KVM_ARM_VCPU_PTRAUTH_GENERIC, &features);
>>       }
>> -    if (!cpus_have_final_cap(ARM64_HAS_NESTED_VIRT))
>> +    if (vcpu_is_protected(vcpu) ||
>> +        !cpus_have_final_cap(ARM64_HAS_NESTED_VIRT))
>>           clear_bit(KVM_ARM_VCPU_HAS_EL2, &features);
>>       return features;
>> @@ -1708,7 +1710,7 @@ static int kvm_vcpu_init_check_features(struct 
>> kvm_vcpu *vcpu,
>>               return -ENOENT;
>>       }
>> -    if (features & ~system_supported_vcpu_features())
>> +    if (features & ~system_supported_vcpu_features(vcpu))
>>           return -EINVAL;
>>
> 
> The following check at the beginning of kvm_vcpu_init_check_features() 
> is redundant to
> the check here. We can drop that in this patch or a preparatory patch.
> 
> static int kvm_vcpu_init_check_features(...)
> {
>         /*
>          * This check can be dropped since the same check has been done
>      * by the followup "if (features & 
> ~system_supported_vcpu_features(vcpu))"
>          */
>         if (features & ~KVM_VCPU_VALID_FEATURES)
>                  return -ENOENT;

We could, but the errno is different with -ENOENT vs -EINVAL, with a
relevant meaning. So I would leave that alone.

Cheers
Suzuki