Re: [RFC PATCH 5/5] KVM: arm64: Enable HAFDBS for guests not on migration

[Tian Zheng]

On 9/21/2026 10:28 PM, Leonardo Bras wrote:
> On Fri, Sep 18, 2026 at 07:58:43PM +0800, Tian Zheng wrote:
> > On 9/2/2026 1:15 AM, Leonardo Bras wrote:
> > > @@ -2570,53 +2571,76 @@ int __init kvm_mmu_init(u32 hyp_va_bits)
> > >    out_destroy_pgtable:
> > >    	kvm_pgtable_hyp_destroy(hyp_pgtable);
> > >    out_free_pgtable:
> > >    	kfree(hyp_pgtable);
> > >    	hyp_pgtable = NULL;
> > >    out:
> > >    	return err;
> > >    }
> > > +static void kvm_set_hafdbs(struct kvm *kvm, bool set)
> > > +{
> > > +	/* Check if no action required */
> > > +	if (!!(kvm->arch.mmu.vtcr & VTCR_EL2_HD) == set)
> > > +		return;
> > > +
> > > +	if (set)
> > > +		kvm->arch.mmu.vtcr |= VTCR_EL2_HD;
> > > +	else
> > > +		kvm->arch.mmu.vtcr &= ~VTCR_EL2_HD;
> > > +
> > > +	kvm_make_all_cpus_request(kvm, KVM_REQ_RELOAD_STAGE2);
> > > +}
> > > +
> >
> > Hi Leo,
> >
> > HD alone is architecturally a no-op. According to the Arm spec, the
> > VTCR_EL2.HD field description reads: "When the Effective value of
> > VTCR_EL2.HA is 0, this field behaves as 0 for all purposes other than a
> > direct read of the value of this bit." So patch 5 as it stands never
> > actually enables stage-2 dirty management.
>
> Oh, right, I missed that. :/
>
> For the above to work it would need to have the HA bit:
>
> +	if (set)
> +		kvm->arch.mmu.vtcr |= VTCR_EL2_HA | VTCR_EL2_HD;
> +	else
> +		kvm->arch.mmu.vtcr &= ~(VTCR_EL2_HA |VTCR_EL2_HD);

Hi Leo,

Yep, I've already fixed this in v5.

> > In the combined series, I'm planning to replace both kvm_set_hafdbs()
> > and our earlier enable/disable hooks with a single derived mode:
> >
> > ```
> > /*
> >   *   logging && HDBSS-capable  ->  HDBSS  (HD|HA|HDBSS)
> >   *   logging, no HDBSS         ->  off
> >   *   !logging, HAFDBS-capable  ->  HAFDBS (HD|HA)
> >   */
>
> Yes, seems correct, if the performance numbers can show patch 5 provides
> performance improvements.

Right, I've added this function in patch 12 of the HDBSS v5 series.
Performance numbers look good so far.

> > void kvm_arch_update_hw_dirty_mode(struct kvm *kvm)
> > {
> > 	bool logging = atomic_read(&kvm->nr_memslots_dirty_logging) != 0;
> > 	unsigned long target;
> >
> > 	if (logging && kvm_supports_hdbss(kvm))
> > 		target = VTCR_EL2_HD | VTCR_EL2_HA | VTCR_EL2_HDBSS;
> > 	else if (logging || !kvm_supports_hafdbs(kvm))
> > 		target = 0;
> > 	else
> > 		target = VTCR_EL2_HD | VTCR_EL2_HA;
> > 	...
> > }
> > ```
>
> Humm, do we actually need the atomic_read being ran when we are enabling
> dirty_logging? Maybe we can restrain it only to the disabling part.

True, but it's just a plain load. Keeping it unconditional avoids
tracking the transition direction, so I'd prefer to keep it. Looking
forward to your further review.

> > HA is always set alongside HD by construction, so the no-op issue goes
> > away.
>
> Right.
>
> > I'll fold this into the HDBSS v5 series when I send it out. Let me know
> > if you'd rather keep it in your v2 instead.
>
> Feel free to merge your part on top of that, I will work on collecting the
> numbers, though.
>
> Thanks!
> Leo

Sorry for the late reply. I was running HDBSS+DBM tests over the past
few days. The HDBSS v5 series with your DBM code merged is now sent out
to the community. Feedback welcome:

https://lore.kernel.org/all/20260929103655.85107-1-zhengtian10@xxxxxxxxxx/

Thanks!
Tian