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

[sashiko-bot@kernel.org]

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;
> +

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

[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;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709104026.2612599-1-zhengtian10@huawei.com?part=3