Re: [PATCH v2 1/2] KVM: arm64: Fix spurious warning for benign stage 2 teardown race

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Sun, Aug 23, 2026 at 01:38:18PM +0800, Yao Yuan wrote:
> On Sat, Aug 22, 2026 at 06:46:53PM +0800, Lorenzo Stoakes (ARM) wrote:
> > A batch of kernel warnings were triggered in the L0 host kernel when using
> > kvmtool to experiment with nested virtualisation.
>
> ...
>
> > Fixes: ec14c272408a ("KVM: arm64: nv: Unmap/flush shadow stage 2 page tables")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> > ---
> >  arch/arm64/kvm/mmu.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > index 74e7e7f7564c..31e049ded093 100644
> > --- a/arch/arm64/kvm/mmu.c
> > +++ b/arch/arm64/kvm/mmu.c
> > @@ -59,19 +59,25 @@ static phys_addr_t stage2_range_addr_end(phys_addr_t addr, phys_addr_t end)
> >   * long will also starve other vCPUs. We have to also make sure that the page
> >   * tables are not freed while we released the lock.
> >   */
> > -static int stage2_apply_range(struct kvm_s2_mmu *mmu, phys_addr_t addr,
> > +static int stage2_apply_range(struct kvm_s2_mmu *mmu, phys_addr_t start,
> >  			      phys_addr_t end,
> >  			      int (*fn)(struct kvm_pgtable *, u64, u64),
> >  			      bool resched)
> >  {
> >  	struct kvm *kvm = kvm_s2_mmu_to_kvm(mmu);
> > +	phys_addr_t addr = start;
> >  	int ret;
> >  	u64 next;
> >
> >  	do {
> >  		struct kvm_pgtable *pgt = mmu->pgt;
> > +		/*
> > +		 * We may be raced on PGT teardown when we release the
> > +		 * kvm->mmu_lock. That's fine as the PGT is legitimately no
> > +		 * longer present.
> > +		 */
> >  		if (!pgt)
> > -			return -EINVAL;
> > +			return resched && addr > start ? 0 : -EINVAL;
>
> Hi,
>
> I can understand the addr checking makes sure only return 0
> when the mmu_lock is dropped at least once and get pgt =
> NULL, may a variable like drop_lock = true when
> cond_resched_rwlock_write(&kvm->mmu_lock) happens can have
> better readability, but depends on you and others' opinion.

Yeah good point, also the condition is resched && next != end, so this was far
too forgiving already :)

So perhaps:

static int stage2_apply_range(struct kvm_s2_mmu *mmu, phys_addr_t start,
			      phys_addr_t end,
			      int (*fn)(struct kvm_pgtable *, u64, u64),
			      bool resched)
{
	...
	bool lock_dropped = false;
	...
	do {
		...
		if (!pgt)
			return lock_dropped ? 0 : -EINVAL;
		...
		if (resched && next != end) {
			cond_resched_rwlock_write(&kvm->mmu_lock);
			lock_dropped = true;
		}
	} while (...);
	...
}

That keeps it simple and clear and we don't try to infer anything too
complicated from the state just 'the lock was dropped'.

>
> Reviewed-by: Yuan Yao <yaoyuan@linux.alibaba.com>

Thanks :)


>
> >
> >  		next = stage2_range_addr_end(addr, end);
> >  		ret = fn(pgt, addr, next - addr);
> >
> > --
> > 2.55.0

--
Cheers, Lorenzo