Subject: Re: [PATCH 08/17] KVM: arm64: Kill topup_memcache from kvm_s2_fault

From: Marc Zyngier <[email-masked]>
Date: Tue, 17 Mar 2026 13:31:33 +0000

On Tue, 17 Mar 2026 12:12:57 +0000,
Fuad Tabba <[email-masked]> wrote:
> 
> Hi Marc,
> 
> On Mon, 16 Mar 2026 at 17:55, Marc Zyngier <[email-masked]> wrote:
> >
> > The topup_memcache field can be easily replaced by the equivalent
> > conditions, and the resulting code is not much worse.
> >
> > Signed-off-by: Marc Zyngier <[email-masked]>
> > ---
> >  arch/arm64/kvm/mmu.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > index 11820e39ad8e1..abe239752c696 100644
> > --- a/arch/arm64/kvm/mmu.c
> > +++ b/arch/arm64/kvm/mmu.c
> > @@ -1712,7 +1712,6 @@ static short kvm_s2_resolve_vma_size(const struct kvm_s2_fault_desc *s2fd,
> >
> >  struct kvm_s2_fault {
> >         bool writable;
> > -       bool topup_memcache;
> >         bool mte_allowed;
> >         bool is_vma_cacheable;
> >         bool s2_force_noncacheable;
> > @@ -1983,7 +1982,6 @@ static int user_mem_abort(const struct kvm_s2_fault_desc *s2fd)
> >                 .logging_active = logging_active,
> >                 .force_pte = logging_active,
> >                 .prot = KVM_PGTABLE_PROT_R,
> > -               .topup_memcache = !perm_fault || (logging_active && kvm_is_write_fault(s2fd->vcpu)),
> >         };
> >         void *memcache;
> >         int ret;
> > @@ -1994,9 +1992,11 @@ static int user_mem_abort(const struct kvm_s2_fault_desc *s2fd)
> >          * only exception to this is when dirty logging is enabled at runtime
> >          * and a write fault needs to collapse a block entry into a table.
> >          */
> > -       ret = prepare_mmu_memcache(s2fd->vcpu, fault.topup_memcache, &memcache);
> > -       if (ret)
> > -               return ret;
> > +       if (!perm_fault || (logging_active && kvm_is_write_fault(s2fd->vcpu))) {
> > +               ret = prepare_mmu_memcache(s2fd->vcpu, true, &memcache);
> > +               if (ret)
> > +                       return ret;
> > +       }
> 
> Further up in user_mem_abort(), when memcache is declared it should be
> initialized to NULL, since prepare_mmu_memcache() isn't called if this
> evaluates to false.

I had that at some point, but then realised that there was no case
where memcache could be used and yet not be initialised via
prepare_mmu_memcache(). But given that this is still a bit fragile,
I'll add it back.

> 
> With that fixed:
> Reviewed-by: Fuad Tabba <[email-masked]>

Thanks!

	M.

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