Re: [PATCH v19 09/20] KVM: arm64: Add VM specific callback for S2 MMU operations

[Suzuki K Poulose]

On 28/09/2026 02:09, Gavin Shan wrote:
> On 9/21/26 7:28 AM, 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@xxxxxxx>
> > ---
> >   arch/arm64/include/asm/kvm_host.h |  15 ++++
> >   arch/arm64/kvm/mmu.c              | 137 +++++++++++++++++++++++++-----
> >   2 files changed, 131 insertions(+), 21 deletions(-)
>
> Apart from the comments from Jonathan, some nitpicks and questions below.
>
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/ 
> > asm/kvm_host.h
> > index 149f4582c8b6a..7664d8b8cce5a 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -155,6 +155,19 @@ struct kvm_vcpu_ops {
> >       void (*vcpu_put)(struct kvm_vcpu *vcpu);
> >   };
> > +struct kvm_gfn_range;
> > +
> > +struct kvm_vm_s2_ops {
> > +    bool (*vm_age_gfn)(struct kvm *kvm, struct kvm_gfn_range *range);
> > +    bool (*vm_test_age_gfn)(struct kvm *kvm, struct kvm_gfn_range 
> > *range);
> > +    int (*vm_flush_remote_tlbs)(struct kvm *kvm);

...

> > @@ -166,6 +168,18 @@ static bool memslot_is_logging(struct 
> > kvm_memory_slot *memslot)
> >       return memslot->dirty_bitmap && !(memslot->flags & 
> > KVM_MEM_READONLY);
> >   }
> > +static int pkvm_flush_remote_tlbs(struct kvm *kvm)
> > +{
> > +    kvm_call_hyp_nvhe(__pkvm_tlb_flush_vmid, kvm->arch.pkvm.handle);
> > +    return 0;
> > +}
> > +
> > +static int kvm_vm_flush_remote_tlbs(struct kvm *kvm)
> > +{
> > +    kvm_call_hyp(__kvm_tlb_flush_vmid, &kvm->arch.mmu);
> > +    return 0;
> > +}
> > +
> >   /**
> >    * kvm_arch_flush_remote_tlbs() - flush all VM TLB entries for v7/8
> >    * @kvm:    pointer to kvm structure.
> > @@ -174,26 +188,36 @@ static bool memslot_is_logging(struct 
> > kvm_memory_slot *memslot)
> >    */
> >   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;
> > +    if (!kvm->arch.vm_s2_ops->vm_flush_remote_tlbs)
> > +        return 1;
>
> For the return value, I'm wandering if 0 should be returned. More details
> can be found below.

Answered below.

>

...

> > +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;
> >   }
> > +int kvm_arch_flush_remote_tlbs_range(struct kvm *kvm,
> > +                     gfn_t gfn, u64 nr_pages)
> > +{
> > +    if (!kvm->arch.vm_s2_ops->vm_flush_remote_tlbs_range)
> > +        return 1;
> > +
>
> Realm would the only case where vm_s2_ops->vm_flush_remote_{tlbs, 
> tlbs_range) 
> are NULL. On request to flush remote TLBs by 
> kvm_flush_remote_tlbs_range(), it
> ends up with event KVM_REQ_TLB_FLUSH queued for each vCPU. How this 
> queued event
> is linked to a remote TLB flush for realm? The problem is TLBs are owned 
> by EL2
> realm and there are no RMI calls for the management. So I'm wandering we 
> should
> return 0 here?

Please note that, the Realm s2 callbacks are not NULL for 
flush_remote_tlb*. They all return 0, indicating that everything
is taken care of. (See Patch 14/20: KVM: arm64: CCA: Add bare minimal S2 
operations for Realm)
...

> > +
> > +#define KVM_VM_S2_OPS(flavor, ops)        \
> > +        [flavor] = ops
>
> Parentheses are needed, to be consistent with KVM_VCPU_OPS at least.
>
> #define KVM_VM_S2_OPS(flavor, ops)        \
>          [(flavor)] = (ops)

Ack


> Actually, KVM_{VCPU, VM_S2}_OPS() can be combined to one in kvm_host.h 
> as below.
>
> #define KVM_FLAVOR_OPS() [(flavor)] = (ops)

I would leave it as they are to avoid confusion.

Cheers
Suzuki