Re: [PATCH v2 06/13] KVM: arm64: dirty_bit: Add base FEAT_HACDBS cleaning routine

[Oliver Upton <oupton@kernel.org>]

On Tue, Jun 30, 2026 at 03:59:38PM +0100, Leonardo Bras wrote:
> > > +	hcr_el2 = read_sysreg(HCR_EL2);
> > > +	write_sysreg(hcr_el2 | HCR_EL2_VM, HCR_EL2);
> > 
> > sysreg_clear_set_hcr(). I'm pretty sure all the speculative AT errata
> > depend on HCR_EL2.VM being set _after_ the stage-2 MMU has been loaded.
> > 
> 
> So, move this to after __load_stage2()?
> ok

Yes.

> > > +	__load_stage2(&kvm->arch.mmu);
> > 
> > Pretty sure you need an ISB here to ensure loading the MMU is ordered
> > with enabling HACDBS.
> >
> 
> does not __load_stage2() have an isb() here?
> In any case, will add an isb() after sysreg_clear_set_hcr(), which should 
> come after __load_stage2() IIUC.

No, __load_stage2() inserts an ISB only for hardware subject to the
speculative AT errata. If an implementation has broken AT and HACDBS in
the future then it gets an additional ISB. Oh well.

> > > +	hacdbs_start(hw_entries, size);
> > > +
> > > +	do {
> > > +		wfi();
> > > +	} while (this_cpu_read(hacdbs_pcp.status) == HACDBS_RUNNING);
> > 
> > This is exactly why I said you should just poll hardware instead. It is
> > entirely possible that the IRQ arrives before you WFI.
> 
> It should be fine with WFIT, though, right?

Sure, but we shouldn't assume a functional WFxT even if we have HACDBS.
Just rely on pre-existing kernel infrastructure to do the thing you want
to.

> I understand the reason in pooling, and even done some workaround in 
> pooling for getting this to run in the model. 
> 
> Based on the previous reply, do you think I should only use polling for 
> now, and implement the IRQ later?

Yes.

Thanks,
Oliver