Re: [PATCH v3 2/2] KVM: arm64: nv: Delay freeing of shadow S2 structures until VM destruction

On Fri, 21 Aug 2026 19:15:45 +0100,
"Lorenzo Stoakes (ARM)" <[email protected]> wrote:
> 
> On Fri, Aug 21, 2026 at 05:18:29PM +0100, Marc Zyngier wrote:
> > We free the shadow S2 structures from kvm_arch_flush_shadow_all(), which
> > is a Bad Idea(tm). Freeing the page tables is fair game (this is what
> > this callback is for), but freeing the container that could still be
> > referenced by another part of the system is not great.
> 
> Yes.
> 
> >
> > Instead, grow separate destructors that gets called when we tear the VM
> > down for good. From there, we can nuke both the individual MMUs as well
> > as the global array that points to them, safe in the knowledge that the
> > vcpus themselves have been destroyed already.
> >
> > Fixes: 4f128f8e1aaac ("KVM: arm64: nv: Support multiple nested Stage-2 mmu structures")
> > Signed-off-by: Marc Zyngier <[email protected]>
> 
> LGTM so:
> 
> Reviewed-by: Lorenzo Stoakes (ARM) <[email protected]>

Thanks.

> A couple thoughts/questions below.

[...]

> > @@ -1310,16 +1319,12 @@ void kvm_nested_s2_flush(struct kvm *kvm)
> >
> >  void kvm_arch_flush_shadow_all(struct kvm *kvm)
> >  {
> > -	for (int i = kvm->arch.nested_mmus_size - 1; i >= 0; i--) {
> > +	for (int i = 0; i < kvm->arch.nested_mmus_size; i++) {
> 
> Hmm why this was in reverse before? :) I guess some product of the
> kvfree() bit or maybe something else?

We allocate s2_mmus in S2_MMU_PER_VCPU chunks. Which means that it
complicates the freeing of the these structures, as they can only be
freed once all S2 PTs of that chunk have been freed. We have three
options:

- scan forward, and use complicated logic to work out that you have
  freed the last PTs of a chunk, freeing with a negative offset from
  the current point in the loop. Works, but hard to reason about in
  fewer than 3 seconds.

- scan backward, use simpler logic to ensure you have reached the
  beginning of a chunk, nuke it.

- have two loops, one for the PTs, one for the MMUs. That's what we
  end-up with due to the different garbage collection phases.

Cheers,

	M.

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