Re: [PATCH v22 11/23] KVM: arm64: Add VM specific callback for S2 MMU operations

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

On 06/10/2026 04:00, Gavin Shan wrote:
> On 10/5/26 7:07 PM, Suzuki K Poulose wrote:
>> Add VM type specific S2 MMU operation backends which can be 
>> initialized per
>> VM flavor, to keep the handling cleaner.
>>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>> Change since v21:
>>   - Define all vm_s2_ops call back. All calls are mandatory.
>>   - Define callback for each flavor, disjointing the non-protetcted 
>> pKVM and
>>     normal KVM (VHE & nVHE) and remove the KVM_PGT_FN() hacks.
>>   - Dropped Reviews due to the changes.
>>   - Add "no_age_gfn" and "no_stage2_unmap_range" for pKVM callbacks, 
>> no_age_*
>>     to be also reused by Realms later.
>>   - Move kvm_vm_s2_ops field to keep the structure packed

...

>>    */
>>   int kvm_arch_flush_remote_tlbs(struct kvm *kvm)
>>   {
>> -    if (is_protected_kvm_enabled())
>> -        kvm_call_hyp_nvhe(__pkvm_tlb_flush_vmid, kvm->arch.pkvm.handle);
>> -    else
>> -        kvm_call_hyp(__kvm_tlb_flush_vmid, &kvm->arch.mmu);
>> -    return 0;
>> +    return kvm->arch.vm_s2_ops->vm_flush_remote_tlbs(kvm);
>>   }
>> -int kvm_arch_flush_remote_tlbs_range(struct kvm *kvm,
>> -                      gfn_t gfn, u64 nr_pages)
>> +static int pkvm_flush_remote_tlbs_range(struct kvm *kvm,
>> +                    gfn_t gfn, u64 nr_pages)
>> +{
>> +    return pkvm_flush_remote_tlbs(kvm);
> 
> No need to have another function call, which causes unnecessary overhead?
> 
>      kvm_call_hyp_nvhe(__pkvm_tlb_flush_vmid, kvm->arch.pkvm.handle);

Maybe, but that is in another way, telling the reader that pKVM can only
do full VM TLB flush, no range TLB flush.

>      return 0;
> 
>> +}
>> +
>> +static int kvm_vm_flush_remote_tlbs_range(struct kvm *kvm,
>> +                     gfn_t gfn, u64 nr_pages)
>>   {
>>       u64 size = nr_pages << PAGE_SHIFT;
>>       u64 addr = gfn << PAGE_SHIFT;
>> -    if (is_protected_kvm_enabled())
>> -        kvm_call_hyp_nvhe(__pkvm_tlb_flush_vmid, kvm->arch.pkvm.handle);
>> -    else
>> -        kvm_tlb_flush_vmid_range(&kvm->arch.mmu, addr, size);
>> +    kvm_tlb_flush_vmid_range(&kvm->arch.mmu, addr, size);
>>       return 0;
>>   }
>>
> 
> Since we're here, the local variable 'addr' and 'size' can be dropped by:
> 
>      kvm_tlb_flush_vmid_range(&kvm->arch.mmu,
>                                   gfn << PAGE_SHIFT,
>                                   nr_pages << PAGE_SHIFT);
> 

Does it really matter, the compiler can optimise this anyways and looks
more readable ?

> 
>> +int kvm_arch_flush_remote_tlbs_range(struct kvm *kvm,
>> +                     gfn_t gfn, u64 nr_pages)
>> +{
>> +    return kvm->arch.vm_s2_ops->vm_flush_remote_tlbs_range(kvm, gfn, 
>> nr_pages);
>> +}
>> +

...

>> +static const struct kvm_vm_s2_ops kvm_default_vm_s2_ops = {
>> +    .vm_flush_remote_tlbs        = kvm_vm_flush_remote_tlbs,
>> +    .vm_flush_remote_tlbs_range    = kvm_vm_flush_remote_tlbs_range,
>> +    .vm_age_gfn            = kvm_vm_age_gfn,
>> +    .vm_test_age_gfn        = kvm_vm_test_age_gfn,
>> +    .vm_stage2_unmap_range        = kvm_vm_stage2_unmap_range,
>> +};
>> +
>> +#define KVM_VM_S2_OPS(flavor, ops)        \
>> +        [flavor] = &(ops)
> 
> s/[flavor]/[(flavor)]

Ack


Thanks !
Suzuki