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

[Marc Zyngier]

On Mon, 20 Jul 2026 21:41:04 +0100,
Mostafa Saleh <smostafa@xxxxxxxxxx> wrote:
> 
> On Sat, Jul 18, 2026 at 8:55 PM Mostafa Saleh <smostafa@xxxxxxxxxx> 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@xxxxxxxxxx/
> 
> - 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.

Thanks,

	M.

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