Re: [PATCH 2/6] KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation

[Marc Zyngier <maz@kernel.org>]

On Sat, 01 Aug 2026 14:02:35 +0100,
sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Stage-1 huge block mappings (e.g., 4TB, 64GB) are incorrectly truncated to 1GB due to a 32-bit return type and a hardcoded fallback, leading to under-invalidation of the VNCR software cache.
> --
> 
> --- Patch [2]: [PATCH 2/6] KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation ---
> commit 55a3817e6985fb11462e68c9b396c2d3ee056461
> Author: Marc Zyngier <maz@kernel.org>
> 
> KVM: arm64: Handle negative S1 walk levels in VNCR TLB size evaluation
> 
> This patch fixes a bug where negative Stage-1 mapping levels (used to indicate
> S1_MMU_DISABLED) were mishandled by pgshift_level_to_ttl(), leading to an
> invalidation size of 0. It introduces __ttl_to_size() and ensures ttl_to_size()
> always falls back to SZ_1G when no valid TTL size is found.
> 
> Link: https://lore.kernel.org/r/ameGoxbn2wzBq2kL@v4bel
> 
> > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> > index 983ba0adaf4e6..b026d160f253d 100644
> > --- a/arch/arm64/kvm/nested.c
> > +++ b/arch/arm64/kvm/nested.c
> 
> [ ... ]
> 
> > @@ -506,7 +506,7 @@ int kvm_walk_nested_s2(struct kvm_vcpu *vcpu, phys_addr_t gipa,
> >  	return ret;
> >  }
> >  
> > -static unsigned int ttl_to_size(u8 ttl)
> > +static unsigned int __ttl_to_size(u8 ttl)
> 
> [Severity: High]
> This is a pre-existing issue, but does this unsigned int return type cause
> huge block mapping sizes to be incorrectly truncated?
> 
> Since __ttl_to_size() and ttl_to_size() return a 32-bit unsigned int, they
> cannot represent sizes 4GB or larger (like 4TB for 64K granule Level 1, or
> 64GB for 16K granule Level 1).

Which is adequate since we don't support these sizes.

> 
> > @@ -562,10 +562,22 @@ static unsigned int ttl_to_size(u8 ttl)
> >  	return max_size;
> >  }
> >  
> > -static u8 pgshift_level_to_ttl(u16 shift, u8 level)
> > +static unsigned int ttl_to_size(u8 ttl)
> > +{
> > +	return __ttl_to_size(ttl) ?: SZ_1G;
> > +}
> 
> If a nested L1 guest maps its VNCR page in such a huge block using 52-bit
> VAs and later issues a TLBI with a VA far from the VNCR page VA, KVM will
> calculate a severely undersized invalidation window. Because the true size
> overflows unsigned int (or evaluates to 0), the fallback restricts it to 1GB.
> 
> This could leave KVM using a stale IPA, potentially corrupting nested guest
> state by writing into unmapped or reassigned guest physical memory.
> 
> Should these size calculation functions be returning an unsigned long or u64
> instead?

No, we don't support LVA/LVA2 for nested guests.

	M.

-- 
Jazz isn't dead. It just smells funny.