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

[Mostafa Saleh <smostafa@google.com>]

On Tue, Jul 21, 2026 at 07:36:16AM +0100, Marc Zyngier wrote:
> On Mon, 20 Jul 2026 21:41:04 +0100,
> Mostafa Saleh <smostafa@google.com> wrote:
> > 
> > On Sat, Jul 18, 2026 at 8:55 PM Mostafa Saleh <smostafa@google.com> wrote:
> > >
> > > Hi Oliver,
> > >
> > > On Fri, Jul 17, 2026 at 01:56:03PM -0700, Oliver Upton wrote:
> > > > Hi Mostafa,
> > > >
> > > > On Fri, Jul 17, 2026 at 01:09:00PM +0000, Mostafa Saleh wrote:
> > > > > If the system supports hardware Break-Before-Make (BBM) level 3, use it
> > > > > to replace stage-2 PTEs directly instead of falling back to the software
> > > > > break-before-make sequence.
> > > > >
> > > > > 1) Get a reference count on the containing table for the new PTE.
> > > > > 2) Atomically update the PTE with the new valid descriptor.
> > > > > 3) Invalidate the TLB for the old PTE.
> > > > > 4) Drop the reference count holding the old PTE.
> > > > >
> > > > > One interesting case, as BBML3 will update the PTE atomically, it
> > > > > can only know it raced with another core at the point of the cmpxchg
> > > > > failing, unlike the SW implementation which locks the PTE first.
> > > > > And as we must issue CMOs to the new mapped page before the update,
> > > > > that means with BBML3 racing cores will issue redundant CMOs,
> > > >
> > > > I'd rather we just predicate BBML3-style transformations on an
> > > > implementation having FEAT_S2FWB and DIC. You can definitely come along
> > > > later and enable it when using a stage-2 in an SMMU makes this
> > > > mandatory, possibly at the expense of some extra CMOs.
> > >
> > > Makes sense, I will do that in v2.
> > >
> > 
> > Looking into this, I see some existing inefficiencies (or maybe I do
> > not understand it well)
> > - pKVM still do some work for dcache with FWB I posted a patch for that:
> > https://lore.kernel.org/all/20260720203529.1276355-1-smostafa@google.com/
> > 
> > - KVM does not elide the icache maintainence with DIC, it seems we
> > should have something similar for the FWB check in
> > __clean_dcache_guest_page() as
> > 
> > diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> > index 6eae7e7e2a68..d0a4ae66b069 100644
> > --- a/arch/arm64/include/asm/kvm_mmu.h
> > +++ b/arch/arm64/include/asm/kvm_mmu.h
> > @@ -247,6 +247,9 @@ static inline size_t __invalidate_icache_max_range(void)
> > 
> >  static inline void __invalidate_icache_guest_page(void *va, size_t size)
> >  {
> > +       if (cpus_have_final_cap(ARM64_HAS_CACHE_DIC))
> > +               return;
> > +
> >         /*
> >          * Blow the whole I-cache if it is aliasing (i.e. VIPT) or the
> >          * invalidation range exceeds our arbitrary limit on invadations by
> > 
> > or I am missing something?
> 
> The latter. The shortcuts are in the individual helpers.
> We have:
> 
> 	static __always_inline void icache_inval_all_pou(void)
> 	{
> 		if (alternative_has_cap_unlikely(ARM64_HAS_CACHE_DIC))
> 			return;
> 
> 		asm("ic	ialluis");
> 		dsb(ish);
> 	}
> 
> and
> 
> 	SYM_FUNC_START(icache_inval_pou)
> 	alternative_if ARM64_HAS_CACHE_DIC
>         	isb
>         	ret
> 	alternative_else_nop_endif
> 
>         	invalidate_icache_by_line x0, x1, x2, x3
> 	        ret
> 	SYM_FUNC_END(icache_inval_pou)
> 
> it's not completely obvious to me why we have an ISB in
> icache_inval_pou(), but at least the invalidation elision is already
> there.

Ah, sorry about the noise! That’s why I shouldn't send late emails :)

I am not sure why the isb is needed, at least for KVM it should be
OK, besides that kexec uses it.

Thanks,
Mostafa

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