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

[Leonardo Bras <leo.bras@arm.com>]

On Tue, Jun 30, 2026 at 12:06:50PM -0700, Oliver Upton wrote:
> 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.
>

Makes sense.
 
> > > > +	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.

Got it.

> 
> > 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.
> 

Will do, then.

Thanks!
Leo