Re: [RFC PATCH v2 2/2] KVM: arm64: Support BBM level 3

[Mostafa Saleh <smostafa@google.com>]

On Thu, Jul 23, 2026 at 06:43:45PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Missing DSB between MTE tag stores and the stage-2 PTE update.
> --
> 
> --- Patch [2]: [RFC PATCH v2 2/2] KVM: arm64: Support BBM level 3 ---
> commit be481f0e17bfc16626c7f5b1b8b652be3ec92909
> Author: Mostafa Saleh <smostafa@google.com>
> 
> KVM: arm64: Support BBM level 3
> 
> If the system supports hardware Break-Before-Make (BBM) level 3, use it
> to replace stage-2 PTEs directly. Otherwise, fall back to the software
> BBM sequence.
> 
> > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> > index 127b7f9541b16..5e16ed7e9b909 100644
> > --- a/arch/arm64/kvm/hyp/pgtable.c
> > +++ b/arch/arm64/kvm/hyp/pgtable.c
> 
> [ ... ]
> 
> > @@ -873,16 +891,35 @@ static bool stage2_try_break_pte(const struct kvm_pgtable_visit_ctx *ctx,
> >  	return true;
> >  }
> >  
> > -static void stage2_make_pte(const struct kvm_pgtable_visit_ctx *ctx, kvm_pte_t new)
> > +static bool stage2_make_pte(const struct kvm_pgtable_visit_ctx *ctx, struct kvm_s2_mmu *mmu,
> > +			    kvm_pte_t new)
> >  {
> >  	struct kvm_pgtable_mm_ops *mm_ops = ctx->mm_ops;
> >  
> > -	WARN_ON(!stage2_pte_is_locked(*ctx->ptep));
> > -
> >  	if (stage2_pte_is_counted(new))
> >  		mm_ops->get_page(ctx->ptep);
> >  
> > +	if (stage2_use_bbml3() && kvm_pte_valid(ctx->old)) {
> > +		/*
> > +		 * Barrier is required because stage2_try_set_pte() uses
> > +		 * WRITE_ONCE for non-shared walks, lacking release semantics
> > +		 * used in the software BBM case.
> > +		 */
> > +		smp_wmb();
> > +		if (!stage2_try_set_pte(ctx, new)) {
> > +			/* Raced with another core. */
> > +			if (stage2_pte_is_counted(new))
> > +				mm_ops->put_page(ctx->ptep);
> > +			return false;
> > +		}
> > +
> > +		stage2_clean_old_pte(ctx, mmu);
> > +		return true;
> > +	}
> > +
> > +	WARN_ON(!stage2_pte_is_locked(*ctx->ptep));
> >  	smp_store_release(ctx->ptep, new);
> > +	return true;
> >  }
> 
> [Severity: High]
> This is a pre-existing issue, but does this sequence lack a required
> synchronization barrier for MTE tag stores?
> 
> When KVM maps a page with MTE enabled, it clears the tags using STGM:
> 
> arch/arm64/kvm/mmu.c:kvm_s2_fault_map() {
>         ...
>         if (!perm_fault_granule && !s2vi->map_non_cacheable && kvm_has_mte(kvm))
>                 sanitise_mte_tags(kvm, pfn, mapping_size);
>         ...
> }
> 
> The PTE is then updated here in stage2_make_pte(), which uses either
> smp_wmb() + cmpxchg() (mapping to DMB ISHST + CASAL) for BBML3, or
> smp_store_release() (mapping to STLR) for the software BBM case. Neither
> of these provides the architecturally mandated DSB semantics required
> after MTE tag stores.
> 
> Without a DSB between sanitise_mte_tags() and the stage-2 PTE update, is
> it possible for the hardware page table walker to observe the new PTE
> before the explicit tag stores are globally visible? Could this result in
> the guest observing uninitialized MTE tags, leading to memory permission
> corruption or spurious tag check faults?

I do not know much about MTE, but AFAIU, STLR wouldn’t order against
page table walks. So there is a window between the STGM and the DSB at
the end of the map where another core can observe a valid PTE and not
a the new tag. My guess we need a DSB at the end of
sanitise_mte_tags()

Thanks,
Mostafa

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260723182140.4025575-1-smostafa@google.com?part=2