Re: [PATCH v4 3/6] KVM: arm64: Add auto DBM support for hardware dirty tracking

[Tian Zheng <zhengtian10@huawei.com>]

On 7/9/2026 7:14 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Critical] kvm_pgtable_stage2_wrprotect() fails to clear the DBM bit, allowing hardware to silently bypass KVM's dirty page tracking.
> - [Medium] VTCR_EL2.HD is never set, rendering the hardware auto-DBM feature inactive.
> - [Medium] DBM is never enabled for VMs running under protected KVM (pKVM) because the hypervisor initializes stage-2 page tables with flags=0.
> --
> 
> --- Patch [3]: [PATCH v4 3/6] KVM: arm64: Add auto DBM support for hardware dirty tracking ---
> commit abb55a98f1d8a61624044b5be480dccd88753db2
> Author: Tian Zheng <zhengtian10@huawei.com>
> KVM: arm64: Add auto DBM support for hardware dirty tracking
> 
> This patch introduces support for the ARMv8.1 Dirty Bit Modifier (DBM)
> attribute in KVM. It allows hardware to automatically promote write-clean
> pages to write-dirty without trapping to EL2. The DBM behavior is controlled
> via a new page-table level flag KVM_PGTABLE_S2_DBM.
> 
> [ ... ]
> 
>> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
>> index 91a7dfad66866..21ec456ecc411 100644
>> --- a/arch/arm64/kvm/hyp/pgtable.c
>> +++ b/arch/arm64/kvm/hyp/pgtable.c
>> @@ -1367,9 +1381,26 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,
>>   	if (prot & KVM_PGTABLE_PROT_R)
>>   		set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
>>   
>> -	if (prot & KVM_PGTABLE_PROT_W)
>> +	if (prot & KVM_PGTABLE_PROT_W) {
>>   		set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
>>   
>> +		/*
>> +		 * No DEVICE filter needed here: relax_perms is only called
>> +		 * on FSC_PERM faults. Device pages always get full RW from
>> +		 * initial mapping and are never write-protected during
>> +		 * migration, so they never trigger a permission fault.
>> +		 */
>> +		if (pgt->flags & KVM_PGTABLE_S2_DBM)
>> +			set |= KVM_PTE_LEAF_ATTR_HI_S2_DBM;
>> +	} else {
>> +		/*
>> +		 * Clear DBM on W→RO downgrade to prevent hardware from
>> +		 * silently upgrading RO+DBM back to W+dirty, which would
>> +		 * bypass KVM's write tracking and cause data corruption.
>> +		 */
>> +		clr |= KVM_PTE_LEAF_ATTR_HI_S2_DBM;
>> +	}
> 
> [Severity: Critical]
> The commit message mentions clearing DBM on W->RO downgrade. Should this logic
> be in kvm_pgtable_stage2_wrprotect() instead?
> 
> kvm_pgtable_stage2_relax_perms() is used for resolving permission faults, not
> for bulk W->RO downgrades. Meanwhile, kvm_pgtable_stage2_wrprotect() clears
> the KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W bit but misses clearing the newly introduced
> DBM bit:
> 
> arch/arm64/kvm/hyp/pgtable.c:kvm_pgtable_stage2_wrprotect() {
>      ...
>      return stage2_update_leaf_attrs(pgt, addr, size, 0,
>                                      KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W,
>                                      NULL, NULL,
>                                      KVM_PGTABLE_WALK_IGNORE_EAGAIN);
> }
> 
> If DBM remains set on a read-only PTE, won't hardware with HDBSS enabled
> automatically upgrade the PTE back to writable upon a guest write, bypassing
> the stage-2 permission fault and causing data corruption during live migration?
> 
> [ ... ]
> 
>> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
>> index e086c01a9325e..346efed6e6050 100644
>> --- a/arch/arm64/kvm/mmu.c
>> +++ b/arch/arm64/kvm/mmu.c
>> @@ -1014,6 +1014,9 @@ int kvm_init_stage2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu, unsigned long t
>>   	if (err)
>>   		goto out_free_pgtable;
>>   
>> +	if (system_supports_hdbss())
>> +		pgt->flags |= KVM_PGTABLE_S2_DBM;
>> +
> 

This is addressed in v5, but I want to clarify the logic:

The key distinction is between block entries (level <
KVM_PGTABLE_LAST_LEVEL) and level-3 pages:

- For block entries, we do strip DBM during write-protect. This ensures
lazy split triggers on the first write fault, as HDBSS only supports
page-granular tracking and cannot handle block-level hardware upgrades.

- For level-3 pages, we do not clear DBM. Clearing DBM would disable
hardware auto-dirtying, which defeats the purpose of HDBSS.

So in stage2_attr_walker(), the write-protect path now behaves as:

- Block entries -> RO (DBM=0, S2AP_W=0): forces fault to trigger split

- Level-3 pages -> WC (DBM=1, S2AP_W=0): re-arms HDBSS tracking on next
write

This preserves hardware dirty tracking for page-granular mappings while
ensuring blocks are properly split before any write is allowed.

> [Severity: Medium]
> Are we missing the corresponding VTCR_EL2.HD update?
> 
> The architectural requirement for the hardware to interpret the DBM bit is
> that VTCR_EL2.HD must be 1. kvm_get_vtcr() in arch/arm64/kvm/hyp/pgtable.c
> does not appear to include the VTCR_EL2_HD bit, which would mean VTCR_EL2.HD
> remains 0 and the hardware will ignore the DBM bit.
> 

False positive. kvm_arm_enable_hdbss_global() already set VTCR_EL2_HD
together with VTCR_EL2_HDBSS and VTCR_EL2_HA.

> [Severity: Medium]
> Also, does this logic properly enable DBM for protected VMs?
> 
> In pKVM mode, the host's pgt is merely a dummy structure. The actual stage-2
> page tables for protected VMs are initialized at EL2 via
> pkvm_guest_prepare_stage2() calling __kvm_pgtable_stage2_init() in
> arch/arm64/kvm/hyp/nvhe/mem_protect.c, which unconditionally passes 0 for the
> flags parameter. Will this cause KVM_PGTABLE_S2_DBM to be ignored in the
> actual page table used for pKVM?
> 
>>   	mmu->pgt = pgt;
>>   	if (is_protected_kvm_enabled())
>>   		return 0;
> 

In v5, KVM_PGTABLE_S2_DBM is removed and DBM is set unconditionally in
stage2_set_prot_attr(). pKVM support is out of scope for this series --
we target VHE only and leave pKVM as future work.

Thanks!
Tian