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

[Tian Zheng <zhengtian10@huawei.com>]

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@huawei.com/

Thanks!
Tian