[RFC PATCH 0/5] KVM: arm64: New PTE dirty-page encoding, HAFDBS new usage

[RFC PATCH 0/5] KVM: arm64: New PTE dirty-page encoding, HAFDBS new usage

[Leonardo Bras]

This series have 2 main goals:

1 - Patches #1,#2,#3 : Change the PTE descriptor to use WD/WC/RO encodings
    making use of the DBM bit, adapting all usages, and
2 - Patches #4,#5 are an RFC on using HAFDBS on a guest to avoid resetting
    all PTEs to WC when dirty-logging starts, speeding-up startup.

(1) will also introduce a new walker for cleaning the dirty-bit, which will
clear the DBM bit if it's a block mapping (hugepage). This is needed as on
lazy-splitting we need to fault a write so we can do the lazy splitting.
This is needed for both the next patches, and for HDBSS & HACDBS
enablement.

On (2), I really just want feedback to understand if it's worth pursuing.
My main idea is that we can use HAFDBS _outside_ dirty-logging to only mark
dirty the pages that were actually written to. That is supposed to make it
faster to transverse the pagetables when we need to clean the dirty-bit, as
there is potentially less atomic writes to perform. The price paid for that
is disabling HAFDBS on every vcpu before we can start cleaning the pages,
done by a (new) vcpu request.

Please let me know of what do you think!

Thanks!
Leo

Leonardo Bras (5):
  KVM: arm64: pgtables: Change write bit from S2AP_W to DBM
  KVM: arm64: Add KVM_PGTABLE_PROT_DIRTY
  KVM: arm64: Introduce a dedicated walker for stage2 write-protect
  KVM: arm64: Add KVM_REQ_RELOAD_STAGE2
  KVM: arm64: Enable HAFDBS for guests not on migration

 arch/arm64/include/asm/kvm_host.h    |  2 ++
 arch/arm64/include/asm/kvm_mmu.h     |  6 ++++
 arch/arm64/include/asm/kvm_nested.h  |  9 +++--
 arch/arm64/include/asm/kvm_pgtable.h | 12 +++++--
 arch/arm64/kvm/arm.c                 | 15 ++++++++
 arch/arm64/kvm/hyp/pgtable.c         | 49 +++++++++++++++++++++-----
 arch/arm64/kvm/mmu.c                 | 51 ++++++++++++++++++++++------
 arch/arm64/kvm/nested.c              |  4 ++-
 arch/arm64/kvm/ptdump.c              | 10 ++++--
 9 files changed, 130 insertions(+), 28 deletions(-)


base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.55.0

[RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM

[Leonardo Bras]

As a first step of changing the encoding for the Stage2 PTE descriptor,
introduce the DBM bit, and adapt every usage of writable to use the DBM bit
(51) instead of S2AP[1]/Dirty bit (7).

For this step, we convert usages of RW(Dirty) -> WD(DBM|Dirty).

Signed-off-by: Leonardo Bras <leo.bras@arm.com>
---
 arch/arm64/include/asm/kvm_pgtable.h | 3 +++
 arch/arm64/kvm/hyp/pgtable.c         | 8 +++++---
 arch/arm64/kvm/nested.c              | 4 +++-
 arch/arm64/kvm/ptdump.c              | 4 ++--
 4 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index 41a8687938eb..37baa86d6fd8 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -86,24 +86,27 @@ typedef u64 kvm_pte_t;
 #define KVM_PTE_LEAF_ATTR_HI		GENMASK(63, 50)
 
 #define KVM_PTE_LEAF_ATTR_HI_SW		GENMASK(58, 55)
 
 #define KVM_PTE_LEAF_ATTR_HI_S1_XN	BIT(54)
 #define KVM_PTE_LEAF_ATTR_HI_S1_UXN	BIT(54)
 #define KVM_PTE_LEAF_ATTR_HI_S1_PXN	BIT(53)
 
 #define KVM_PTE_LEAF_ATTR_HI_S2_XN	GENMASK(54, 53)
 
+#define KVM_PTE_LEAF_ATTR_HI_S2_DBM	BIT(51)
+
 #define KVM_PTE_LEAF_ATTR_HI_S1_GP	BIT(50)
 
 #define KVM_PTE_LEAF_ATTR_S2_PERMS	(KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R | \
 					 KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W | \
+					 KVM_PTE_LEAF_ATTR_HI_S2_DBM | \
 					 KVM_PTE_LEAF_ATTR_HI_S2_XN)
 
 /* pKVM invalid pte encodings */
 #define KVM_INVALID_PTE_TYPE_MASK	GENMASK(63, 60)
 #define KVM_INVALID_PTE_ANNOT_MASK	~(KVM_PTE_VALID | \
 					  KVM_INVALID_PTE_TYPE_MASK)
 
 enum kvm_invalid_pte_type {
 	/*
 	 * Used to indicate a pte for which a 'break-before-make'
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index b74dd5ce1efd..ca49f1bd7c34 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -725,42 +725,43 @@ static int stage2_set_prot_attr(struct kvm_pgtable *pgt, enum kvm_pgtable_prot p
 	}
 
 	r = stage2_set_xn_attr(prot, &attr);
 	if (r)
 		return r;
 
 	if (prot & KVM_PGTABLE_PROT_R)
 		attr |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
 
 	if (prot & KVM_PGTABLE_PROT_W)
-		attr |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
+		attr |= KVM_PTE_LEAF_ATTR_HI_S2_DBM | KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
+
 
 	if (!kvm_lpa2_is_enabled())
 		attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S2_SH, sh);
 
 	attr |= KVM_PTE_LEAF_ATTR_LO_S2_AF;
 	attr |= prot & KVM_PTE_LEAF_ATTR_HI_SW;
 	*ptep = attr;
 
 	return 0;
 }
 
 enum kvm_pgtable_prot kvm_pgtable_stage2_pte_prot(kvm_pte_t pte)
 {
 	enum kvm_pgtable_prot prot = pte & KVM_PTE_LEAF_ATTR_HI_SW;
 
 	if (!kvm_pte_valid(pte))
 		return prot;
 
 	if (pte & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R)
 		prot |= KVM_PGTABLE_PROT_R;
-	if (pte & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W)
+	if (pte & KVM_PTE_LEAF_ATTR_HI_S2_DBM)
 		prot |= KVM_PGTABLE_PROT_W;
 
 	switch (FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, pte)) {
 	case 0b00:
 		prot |= KVM_PGTABLE_PROT_PX | KVM_PGTABLE_PROT_UX;
 		break;
 	case 0b01:
 		prot |= KVM_PGTABLE_PROT_UX;
 		break;
 	case 0b11:
@@ -1281,20 +1282,21 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,
 		*orig_pte = data.pte;
 
 	if (level)
 		*level = data.level;
 	return 0;
 }
 
 int kvm_pgtable_stage2_wrprotect(struct kvm_pgtable *pgt, u64 addr, u64 size)
 {
 	return stage2_update_leaf_attrs(pgt, addr, size, 0,
+					KVM_PTE_LEAF_ATTR_HI_S2_DBM |
 					KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W,
 					NULL, NULL,
 					KVM_PGTABLE_WALK_IGNORE_EAGAIN);
 }
 
 void kvm_pgtable_stage2_mkyoung(struct kvm_pgtable *pgt, u64 addr,
 				enum kvm_pgtable_walk_flags flags)
 {
 	int ret;
 
@@ -1361,21 +1363,21 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,
 	s8 level;
 	int ret;
 
 	if (prot & KVM_PTE_LEAF_ATTR_HI_SW)
 		return -EINVAL;
 
 	if (prot & KVM_PGTABLE_PROT_R)
 		set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
 
 	if (prot & KVM_PGTABLE_PROT_W)
-		set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
+		set |= KVM_PTE_LEAF_ATTR_HI_S2_DBM | KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
 
 	if (prot & KVM_PGTABLE_PROT_X) {
 		ret = stage2_set_xn_attr(prot, &xn);
 		if (ret)
 			return ret;
 
 		set |= xn & KVM_PTE_LEAF_ATTR_HI_S2_XN;
 		clr |= ~xn & KVM_PTE_LEAF_ATTR_HI_S2_XN;
 	}
 
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 17123f0b6dab..eb8dfffc32c7 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -379,21 +379,23 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
 	}
 
 	addr_bottom += contiguous_bit_shift(desc, wi, level);
 
 	/* Calculate and return the result */
 	paddr = (desc & GENMASK_ULL(47, addr_bottom)) |
 		(ipa & GENMASK_ULL(addr_bottom - 1, 0));
 	out->output = paddr;
 	out->block_size = 1UL << ((3 - level) * stride + wi->pgshift);
 	out->readable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
-	out->writable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
+	/* Takes care of both RO/RW and RO/WC/WD encodings */
+	out->writable = desc & (KVM_PTE_LEAF_ATTR_HI_S2_DBM |
+				KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W);
 	out->level = level;
 	out->desc = desc;
 	return 0;
 }
 
 #define _has_tgran_2(__r, __sz)						\
 	({								\
 		u64 _s1, _s2, _mmfr0 = __r;				\
 									\
 		_s2 = SYS_FIELD_GET(ID_AA64MMFR0_EL1,			\
diff --git a/arch/arm64/kvm/ptdump.c b/arch/arm64/kvm/ptdump.c
index 69899797dbad..b0cb8d84a9e9 100644
--- a/arch/arm64/kvm/ptdump.c
+++ b/arch/arm64/kvm/ptdump.c
@@ -33,22 +33,22 @@ static const struct ptdump_prot_bits stage2_pte_bits[] = {
 		.set	= " ",
 		.clear	= "F",
 	},
 	{
 		.mask	= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R,
 		.val	= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R,
 		.set	= "R",
 		.clear	= " ",
 	},
 	{
-		.mask	= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W,
-		.val	= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W,
+		.mask	= KVM_PTE_LEAF_ATTR_HI_S2_DBM,
+		.val	= KVM_PTE_LEAF_ATTR_HI_S2_DBM,
 		.set	= "W",
 		.clear	= " ",
 	},
 	{
 		.mask	= KVM_PTE_LEAF_ATTR_HI_S2_XN,
 		.val	= 0b00UL << __bf_shf(KVM_PTE_LEAF_ATTR_HI_S2_XN),
 		.set	= "px ux ",
 	},
 	{
 		.mask	= KVM_PTE_LEAF_ATTR_HI_S2_XN,
-- 
2.55.0

[RFC PATCH 2/5] KVM: arm64: Add KVM_PGTABLE_PROT_DIRTY

[Leonardo Bras]

Second step of changing the encoding for the Stage2 PTE descriptor,
introduce the concept of dirty page, so we can have a writable but not
dirty (WC) page, and a writable and dirty (WD) page.

In order to do so, evaluate uses in a per-case basis, and figure what
concept was important in each case (being dirty, or writable).

Signed-off-by: Leonardo Bras <leo.bras@arm.com>
---
 arch/arm64/include/asm/kvm_pgtable.h |  9 ++++++---
 arch/arm64/kvm/hyp/pgtable.c         | 23 +++++++++++++++++------
 arch/arm64/kvm/mmu.c                 | 27 ++++++++++++++++-----------
 arch/arm64/kvm/ptdump.c              |  6 ++++++
 4 files changed, 45 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index 37baa86d6fd8..379031c74cbc 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -258,45 +258,48 @@ enum kvm_pgtable_stage2_flags {
 	KVM_PGTABLE_S2_AS_S1			= BIT(1),
 };
 
 /**
  * enum kvm_pgtable_prot - Page-table permissions and attributes.
  * @KVM_PGTABLE_PROT_UX:	Unprivileged execute permission.
  * @KVM_PGTABLE_PROT_PX:	Privileged execute permission.
  * @KVM_PGTABLE_PROT_X:		Privileged and unprivileged execute permission.
  * @KVM_PGTABLE_PROT_W:		Write permission.
  * @KVM_PGTABLE_PROT_R:		Read permission.
+ * @KVM_PGTABLE_PROT_DIRTY:	Dirty attribute.
  * @KVM_PGTABLE_PROT_DEVICE:	Device attributes.
  * @KVM_PGTABLE_PROT_NORMAL_NC:	Normal noncacheable attributes.
  * @KVM_PGTABLE_PROT_SW0:	Software bit 0.
  * @KVM_PGTABLE_PROT_SW1:	Software bit 1.
  * @KVM_PGTABLE_PROT_SW2:	Software bit 2.
  * @KVM_PGTABLE_PROT_SW3:	Software bit 3.
  */
 enum kvm_pgtable_prot {
 	KVM_PGTABLE_PROT_PX			= BIT(0),
 	KVM_PGTABLE_PROT_UX			= BIT(1),
 	KVM_PGTABLE_PROT_X			= KVM_PGTABLE_PROT_PX	|
 						  KVM_PGTABLE_PROT_UX,
 	KVM_PGTABLE_PROT_W			= BIT(2),
 	KVM_PGTABLE_PROT_R			= BIT(3),
+	KVM_PGTABLE_PROT_DIRTY			= BIT(4),
 
-	KVM_PGTABLE_PROT_DEVICE			= BIT(4),
-	KVM_PGTABLE_PROT_NORMAL_NC		= BIT(5),
+	KVM_PGTABLE_PROT_DEVICE			= BIT(5),
+	KVM_PGTABLE_PROT_NORMAL_NC		= BIT(6),
 
 	KVM_PGTABLE_PROT_SW0			= BIT(55),
 	KVM_PGTABLE_PROT_SW1			= BIT(56),
 	KVM_PGTABLE_PROT_SW2			= BIT(57),
 	KVM_PGTABLE_PROT_SW3			= BIT(58),
 };
 
-#define KVM_PGTABLE_PROT_RW	(KVM_PGTABLE_PROT_R | KVM_PGTABLE_PROT_W)
+#define KVM_PGTABLE_PROT_RW	(KVM_PGTABLE_PROT_R | KVM_PGTABLE_PROT_W | \
+				 KVM_PGTABLE_PROT_DIRTY)
 #define KVM_PGTABLE_PROT_RWX	(KVM_PGTABLE_PROT_RW | KVM_PGTABLE_PROT_X)
 
 #define PKVM_HOST_MEM_PROT	KVM_PGTABLE_PROT_RWX
 #define PKVM_HOST_MMIO_PROT	KVM_PGTABLE_PROT_RW
 
 #define PAGE_HYP		KVM_PGTABLE_PROT_RW
 #define PAGE_HYP_EXEC		(KVM_PGTABLE_PROT_R | KVM_PGTABLE_PROT_X)
 #define PAGE_HYP_RO		(KVM_PGTABLE_PROT_R)
 #define PAGE_HYP_DEVICE		(PAGE_HYP | KVM_PGTABLE_PROT_DEVICE)
 
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index ca49f1bd7c34..2ff33d3e371e 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -724,22 +724,26 @@ static int stage2_set_prot_attr(struct kvm_pgtable *pgt, enum kvm_pgtable_prot p
 		attr = KVM_S2_MEMATTR(pgt, NORMAL);
 	}
 
 	r = stage2_set_xn_attr(prot, &attr);
 	if (r)
 		return r;
 
 	if (prot & KVM_PGTABLE_PROT_R)
 		attr |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
 
-	if (prot & KVM_PGTABLE_PROT_W)
-		attr |= KVM_PTE_LEAF_ATTR_HI_S2_DBM | KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
+	if (prot & KVM_PGTABLE_PROT_W) {
+		attr |= KVM_PTE_LEAF_ATTR_HI_S2_DBM;
+
+		if (prot & KVM_PGTABLE_PROT_DIRTY)
+			attr |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
+	}
 
 
 	if (!kvm_lpa2_is_enabled())
 		attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S2_SH, sh);
 
 	attr |= KVM_PTE_LEAF_ATTR_LO_S2_AF;
 	attr |= prot & KVM_PTE_LEAF_ATTR_HI_SW;
 	*ptep = attr;
 
 	return 0;
@@ -747,23 +751,27 @@ static int stage2_set_prot_attr(struct kvm_pgtable *pgt, enum kvm_pgtable_prot p
 
 enum kvm_pgtable_prot kvm_pgtable_stage2_pte_prot(kvm_pte_t pte)
 {
 	enum kvm_pgtable_prot prot = pte & KVM_PTE_LEAF_ATTR_HI_SW;
 
 	if (!kvm_pte_valid(pte))
 		return prot;
 
 	if (pte & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R)
 		prot |= KVM_PGTABLE_PROT_R;
-	if (pte & KVM_PTE_LEAF_ATTR_HI_S2_DBM)
+	if (pte & KVM_PTE_LEAF_ATTR_HI_S2_DBM) {
 		prot |= KVM_PGTABLE_PROT_W;
 
+		if (pte & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W)
+			prot |= KVM_PGTABLE_PROT_DIRTY;
+	}
+
 	switch (FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, pte)) {
 	case 0b00:
 		prot |= KVM_PGTABLE_PROT_PX | KVM_PGTABLE_PROT_UX;
 		break;
 	case 0b01:
 		prot |= KVM_PGTABLE_PROT_UX;
 		break;
 	case 0b11:
 		prot |= KVM_PGTABLE_PROT_PX;
 		break;
@@ -1282,21 +1290,20 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,
 		*orig_pte = data.pte;
 
 	if (level)
 		*level = data.level;
 	return 0;
 }
 
 int kvm_pgtable_stage2_wrprotect(struct kvm_pgtable *pgt, u64 addr, u64 size)
 {
 	return stage2_update_leaf_attrs(pgt, addr, size, 0,
-					KVM_PTE_LEAF_ATTR_HI_S2_DBM |
 					KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W,
 					NULL, NULL,
 					KVM_PGTABLE_WALK_IGNORE_EAGAIN);
 }
 
 void kvm_pgtable_stage2_mkyoung(struct kvm_pgtable *pgt, u64 addr,
 				enum kvm_pgtable_walk_flags flags)
 {
 	int ret;
 
@@ -1362,22 +1369,26 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,
 	kvm_pte_t xn = 0, set = 0, clr = 0;
 	s8 level;
 	int ret;
 
 	if (prot & KVM_PTE_LEAF_ATTR_HI_SW)
 		return -EINVAL;
 
 	if (prot & KVM_PGTABLE_PROT_R)
 		set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
 
-	if (prot & KVM_PGTABLE_PROT_W)
-		set |= KVM_PTE_LEAF_ATTR_HI_S2_DBM | KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
+	if (prot & KVM_PGTABLE_PROT_W) {
+		set |= KVM_PTE_LEAF_ATTR_HI_S2_DBM;
+
+		if (prot & KVM_PGTABLE_PROT_DIRTY)
+			set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
+	}
 
 	if (prot & KVM_PGTABLE_PROT_X) {
 		ret = stage2_set_xn_attr(prot, &xn);
 		if (ret)
 			return ret;
 
 		set |= xn & KVM_PTE_LEAF_ATTR_HI_S2_XN;
 		clr |= ~xn & KVM_PTE_LEAF_ATTR_HI_S2_XN;
 	}
 
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 9ba86450fe4a..9d4f70430bbe 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1205,21 +1205,23 @@ int topup_hyp_memcache(struct kvm_hyp_memcache *mc, unsigned long min_pages)
 int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa,
 			  phys_addr_t pa, unsigned long size, bool writable)
 {
 	phys_addr_t addr;
 	int ret = 0;
 	struct kvm_mmu_memory_cache cache = { .gfp_zero = __GFP_ZERO };
 	struct kvm_s2_mmu *mmu = &kvm->arch.mmu;
 	struct kvm_pgtable *pgt = mmu->pgt;
 	enum kvm_pgtable_prot prot = KVM_PGTABLE_PROT_DEVICE |
 				     KVM_PGTABLE_PROT_R |
-				     (writable ? KVM_PGTABLE_PROT_W : 0);
+				     (writable ?
+				      (KVM_PGTABLE_PROT_W | KVM_PGTABLE_PROT_DIRTY) :
+				      0);
 
 	if (is_protected_kvm_enabled())
 		return -EPERM;
 
 	size += offset_in_page(guest_ipa);
 	guest_ipa &= PAGE_MASK;
 
 	for (addr = guest_ipa; addr < guest_ipa + size; addr += PAGE_SIZE) {
 		ret = kvm_mmu_topup_memory_cache(&cache,
 						 kvm_mmu_cache_min_pages(mmu));
@@ -1571,21 +1573,21 @@ static int topup_mmu_memcache(struct kvm_vcpu *vcpu, void *memcache)
  *
  * Also encode the level of the original translation in the SW bits of the leaf
  * entry as a proxy for the span of that translation. This will be retrieved on
  * TLB invalidation from the guest and used to limit the invalidation scope if a
  * TTL hint or a range isn't provided.
  */
 static enum kvm_pgtable_prot adjust_nested_fault_perms(struct kvm_s2_trans *nested,
 						       enum kvm_pgtable_prot prot)
 {
 	if (!kvm_s2_trans_writable(nested))
-		prot &= ~KVM_PGTABLE_PROT_W;
+		prot &= ~(KVM_PGTABLE_PROT_W | KVM_PGTABLE_PROT_DIRTY);
 	if (!kvm_s2_trans_readable(nested))
 		prot &= ~KVM_PGTABLE_PROT_R;
 
 	return prot | kvm_encode_nested_level(nested);
 }
 
 static enum kvm_pgtable_prot adjust_nested_exec_perms(struct kvm *kvm,
 						      struct kvm_s2_trans *nested,
 						      enum kvm_pgtable_prot prot)
 {
@@ -1642,21 +1644,21 @@ static int gmem_abort(const struct kvm_s2_fault_desc *s2fd)
 	smp_rmb();
 
 	ret = kvm_gmem_get_pfn(kvm, s2fd->memslot, gfn, &pfn, &page, NULL);
 	if (ret) {
 		kvm_prepare_memory_fault_exit(s2fd->vcpu, s2fd->fault_ipa, PAGE_SIZE,
 					      write_fault, exec_fault, false);
 		return ret;
 	}
 
 	if (!(s2fd->memslot->flags & KVM_MEM_READONLY))
-		prot |= KVM_PGTABLE_PROT_W;
+		prot |= KVM_PGTABLE_PROT_W | KVM_PGTABLE_PROT_DIRTY;
 
 	if (s2fd->nested)
 		prot = adjust_nested_fault_perms(s2fd->nested, prot);
 
 	if (exec_fault || cpus_have_final_cap(ARM64_HAS_CACHE_DIC))
 		prot |= KVM_PGTABLE_PROT_X;
 
 	if (s2fd->nested)
 		prot = adjust_nested_exec_perms(kvm, s2fd->nested, prot);
 
@@ -1674,24 +1676,24 @@ static int gmem_abort(const struct kvm_s2_fault_desc *s2fd)
 		prot &= ~KVM_NV_GUEST_MAP_SZ;
 		ret = KVM_PGT_FN(kvm_pgtable_stage2_relax_perms)(pgt, s2fd->fault_ipa,
 								 prot, flags);
 	} else {
 		ret = KVM_PGT_FN(kvm_pgtable_stage2_map)(pgt, s2fd->fault_ipa, PAGE_SIZE,
 							 __pfn_to_phys(pfn), prot,
 							 memcache, flags);
 	}
 
 out_unlock:
-	kvm_release_faultin_page(kvm, page, !!ret, prot & KVM_PGTABLE_PROT_W);
+	kvm_release_faultin_page(kvm, page, !!ret, prot & KVM_PGTABLE_PROT_DIRTY);
 	kvm_fault_unlock(kvm);
 
-	if ((prot & KVM_PGTABLE_PROT_W) && !ret)
+	if ((prot & KVM_PGTABLE_PROT_DIRTY) && !ret)
 		mark_page_dirty_in_slot(kvm, s2fd->memslot, gfn);
 
 	return ret != -EAGAIN ? ret : 0;
 }
 
 struct kvm_s2_fault_vma_info {
 	unsigned long	mmu_seq;
 	long		vma_pagesize;
 	vm_flags_t	vm_flags;
 	unsigned long	max_map_size;
@@ -1977,25 +1979,28 @@ static int kvm_s2_fault_compute_prot(const struct kvm_s2_fault_desc *s2fd,
 	 * and trigger the exception here. Since the memslot is valid, inject
 	 * the fault back to the guest.
 	 */
 	if (esr_fsc_is_excl_atomic_fault(kvm_vcpu_get_esr(s2fd->vcpu))) {
 		kvm_inject_dabt_excl_atomic(s2fd->vcpu, kvm_vcpu_get_hfar(s2fd->vcpu));
 		return 1;
 	}
 
 	*prot = KVM_PGTABLE_PROT_R;
 
-	if (s2vi->map_writable && (s2vi->device ||
-				   !memslot_is_logging(s2fd->memslot) ||
-				   kvm_is_write_fault(s2fd->vcpu)))
+	if (s2vi->map_writable) {
 		*prot |= KVM_PGTABLE_PROT_W;
 
+		if (s2vi->device || !memslot_is_logging(s2fd->memslot) ||
+		    kvm_is_write_fault(s2fd->vcpu))
+			*prot |= KVM_PGTABLE_PROT_DIRTY;
+	}
+
 	if (s2fd->nested)
 		*prot = adjust_nested_fault_perms(s2fd->nested, *prot);
 
 	if (kvm_vcpu_trap_is_exec_fault(s2fd->vcpu))
 		*prot |= KVM_PGTABLE_PROT_X;
 
 	if (s2vi->map_non_cacheable)
 		*prot |= (s2vi->vm_flags & VM_ALLOW_ANY_UNCACHED) ?
 			KVM_PGTABLE_PROT_NORMAL_NC : KVM_PGTABLE_PROT_DEVICE;
 	else if (cpus_have_final_cap(ARM64_HAS_CACHE_DIC))
@@ -2012,21 +2017,21 @@ static int kvm_s2_fault_compute_prot(const struct kvm_s2_fault_desc *s2fd,
 
 	return 0;
 }
 
 static int kvm_s2_fault_map(const struct kvm_s2_fault_desc *s2fd,
 			    const struct kvm_s2_fault_vma_info *s2vi,
 			    enum kvm_pgtable_prot prot,
 			    void *memcache)
 {
 	enum kvm_pgtable_walk_flags flags = KVM_PGTABLE_WALK_SHARED;
-	bool writable = prot & KVM_PGTABLE_PROT_W;
+	bool dirty = prot & KVM_PGTABLE_PROT_DIRTY;
 	struct kvm *kvm = s2fd->vcpu->kvm;
 	struct kvm_pgtable *pgt;
 	long perm_fault_granule;
 	long mapping_size;
 	kvm_pfn_t pfn;
 	gfn_t gfn;
 	int ret;
 
 	kvm_fault_lock(kvm);
 	pgt = s2fd->vcpu->arch.hw_mmu->pgt;
@@ -2075,29 +2080,29 @@ static int kvm_s2_fault_map(const struct kvm_s2_fault_desc *s2fd,
 		prot &= ~KVM_NV_GUEST_MAP_SZ;
 		ret = KVM_PGT_FN(kvm_pgtable_stage2_relax_perms)(pgt, gfn_to_gpa(gfn),
 								 prot, flags);
 	} else {
 		ret = KVM_PGT_FN(kvm_pgtable_stage2_map)(pgt, gfn_to_gpa(gfn), mapping_size,
 							 __pfn_to_phys(pfn), prot,
 							 memcache, flags);
 	}
 
 out_unlock:
-	kvm_release_faultin_page(kvm, s2vi->page, !!ret, writable);
+	kvm_release_faultin_page(kvm, s2vi->page, !!ret, dirty);
 	kvm_fault_unlock(kvm);
 
 	/*
 	 * Mark the page dirty only if the fault is handled successfully,
 	 * making sure we adjust the canonical IPA if the mapping size has
 	 * been updated (via a THP upgrade, for example).
 	 */
-	if (writable && !ret) {
+	if (dirty && !ret) {
 		phys_addr_t ipa = gfn_to_gpa(get_canonical_gfn(s2fd, s2vi));
 		ipa &= ~(mapping_size - 1);
 		mark_page_dirty_in_slot(kvm, s2fd->memslot, gpa_to_gfn(ipa));
 	}
 
 	if (ret != -EAGAIN)
 		return ret;
 	return 0;
 }
 
diff --git a/arch/arm64/kvm/ptdump.c b/arch/arm64/kvm/ptdump.c
index b0cb8d84a9e9..a1251e252b4f 100644
--- a/arch/arm64/kvm/ptdump.c
+++ b/arch/arm64/kvm/ptdump.c
@@ -38,20 +38,26 @@ static const struct ptdump_prot_bits stage2_pte_bits[] = {
 		.val	= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R,
 		.set	= "R",
 		.clear	= " ",
 	},
 	{
 		.mask	= KVM_PTE_LEAF_ATTR_HI_S2_DBM,
 		.val	= KVM_PTE_LEAF_ATTR_HI_S2_DBM,
 		.set	= "W",
 		.clear	= " ",
 	},
+	{
+		.mask	= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W,
+		.val	= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W,
+		.set	= "D",
+		.clear	= "C",
+	},
 	{
 		.mask	= KVM_PTE_LEAF_ATTR_HI_S2_XN,
 		.val	= 0b00UL << __bf_shf(KVM_PTE_LEAF_ATTR_HI_S2_XN),
 		.set	= "px ux ",
 	},
 	{
 		.mask	= KVM_PTE_LEAF_ATTR_HI_S2_XN,
 		.val	= 0b01UL << __bf_shf(KVM_PTE_LEAF_ATTR_HI_S2_XN),
 		.set	= "PXNux ",
 	},
-- 
2.55.0

[RFC PATCH 3/5] KVM: arm64: Introduce a dedicated walker for stage2 write-protect

[Leonardo Bras]

The new walker cleans the dirty bit on leaf entries, as well as clean
the DBM bit in blocks so it still faults for lazy hugepage splitting when
we enable FEAT_HDBSS in future patches.

With disabled HDBSS, there should be no change in faulting behavior.

Signed-off-by: Leonardo Bras <leo.bras@arm.com>
---
 arch/arm64/kvm/hyp/pgtable.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index 2ff33d3e371e..2849c136f366 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -1287,26 +1287,44 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,
 		return ret;
 
 	if (orig_pte)
 		*orig_pte = data.pte;
 
 	if (level)
 		*level = data.level;
 	return 0;
 }
 
+static int stage2_wrprotect_walker(const struct kvm_pgtable_visit_ctx *ctx,
+				   enum kvm_pgtable_walk_flags visit)
+{
+	kvm_pte_t new = ctx->old & ~KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
+
+	/* We remove DBM on blocks so they can fault and get split */
+	if (ctx->level < KVM_PGTABLE_LAST_LEVEL)
+		new &= ~KVM_PTE_LEAF_ATTR_HI_S2_DBM;
+
+	if (kvm_pte_valid(ctx->old) && ctx->old != new)
+		WRITE_ONCE(*ctx->ptep, new);
+
+	return 0;
+}
+
+
 int kvm_pgtable_stage2_wrprotect(struct kvm_pgtable *pgt, u64 addr, u64 size)
 {
-	return stage2_update_leaf_attrs(pgt, addr, size, 0,
-					KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W,
-					NULL, NULL,
-					KVM_PGTABLE_WALK_IGNORE_EAGAIN);
+	struct kvm_pgtable_walker walker = {
+		.cb	= stage2_wrprotect_walker,
+		.flags	= KVM_PGTABLE_WALK_LEAF,
+	};
+
+	return kvm_pgtable_walk(pgt, addr, size, &walker);
 }
 
 void kvm_pgtable_stage2_mkyoung(struct kvm_pgtable *pgt, u64 addr,
 				enum kvm_pgtable_walk_flags flags)
 {
 	int ret;
 
 	ret = stage2_update_leaf_attrs(pgt, addr, 1, KVM_PTE_LEAF_ATTR_LO_S2_AF, 0,
 				       NULL, NULL, flags);
 	if (!ret)
-- 
2.55.0

[RFC PATCH 4/5] KVM: arm64: Add KVM_REQ_RELOAD_STAGE2

[Leonardo Bras]

Add a vcpu request to exit guest, reload Stage2, and then come back to
guest.

This will be used on future patches that enable S2 HAFDBS and HDBSS, as
they may need to change VTCR bits for enabling/disabling the feature when
the vcpus are still running.

Signed-off-by: Leonardo Bras <leo.bras@arm.com>
---
 arch/arm64/include/asm/kvm_host.h | 2 ++
 arch/arm64/kvm/arm.c              | 8 ++++++++
 2 files changed, 10 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 27fe0cd5b2d7..00fe169f239f 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -48,20 +48,22 @@
 #define KVM_REQ_VCPU_RESET		KVM_ARCH_REQ(2)
 #define KVM_REQ_RECORD_STEAL		KVM_ARCH_REQ(3)
 #define KVM_REQ_RELOAD_GICv4		KVM_ARCH_REQ(4)
 #define KVM_REQ_RELOAD_PMU		KVM_ARCH_REQ(5)
 #define KVM_REQ_SUSPEND			KVM_ARCH_REQ(6)
 #define KVM_REQ_RESYNC_PMU_EL0		KVM_ARCH_REQ(7)
 #define KVM_REQ_NESTED_S2_UNMAP		KVM_ARCH_REQ(8)
 #define KVM_REQ_GUEST_HYP_IRQ_PENDING	KVM_ARCH_REQ(9)
 #define KVM_REQ_MAP_L1_VNCR_EL2		KVM_ARCH_REQ(10)
 #define KVM_REQ_VGIC_PROCESS_UPDATE	KVM_ARCH_REQ(11)
+#define KVM_REQ_RELOAD_STAGE2 \
+	KVM_ARCH_REQ_FLAGS(12, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
 
 #define KVM_DIRTY_LOG_MANUAL_CAPS   (KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE | \
 				     KVM_DIRTY_LOG_INITIALLY_SET)
 
 #define KVM_HAVE_MMU_RWLOCK
 
 /*
  * Mode of operation configurable with kvm-arm.mode early param.
  * See Documentation/admin-guide/kernel-parameters.txt for more information.
  */
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 8b080804bc90..1e528d53d093 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1164,20 +1164,28 @@ static int check_vcpu_requests(struct kvm_vcpu *vcpu)
 
 		if (kvm_check_request(KVM_REQ_RESYNC_PMU_EL0, vcpu))
 			kvm_vcpu_pmu_restore_guest(vcpu);
 
 		if (kvm_check_request(KVM_REQ_SUSPEND, vcpu))
 			return kvm_vcpu_suspend(vcpu);
 
 		if (kvm_dirty_ring_check_request(vcpu))
 			return 0;
 
+		if (kvm_check_request(KVM_REQ_RELOAD_STAGE2, vcpu)) {
+			unsigned long flags;
+
+			local_irq_save(flags);
+			__load_stage2(vcpu->arch.hw_mmu);
+			local_irq_restore(flags);
+		}
+
 		check_nested_vcpu_requests(vcpu);
 	}
 
 	return 1;
 }
 
 static bool vcpu_mode_is_bad_32bit(struct kvm_vcpu *vcpu)
 {
 	if (likely(!vcpu_mode_is_32bit(vcpu)))
 		return false;
-- 
2.55.0

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

[Leonardo Bras]

When dirty-logging is disabled, even non-write faults make a page dirty,
which avoids a second fault when the page is actually written to.

On dirty-logging enable, this approach causes all (writable) pages on the
memslot to be marked clean, even if they were not written to, which can
take a lot of time, while holding the MMU lock, doing atomic writes to
PTEs.

Systems with HAFDBS can use HW to mark a writable-clean page as
writable-dirty when a write occurs, avoiding the mentioned second fault,
while keeping dirty only the pages that have been actually written to.

So, if the system supports VHE + HAFDBS, keep the non-write-faulted page as
writable-clean, and let HAFDBS update that on demand when a write happens.

When dirty-tracking actually starts, disable HAFDBS as having it on
avoids the same fault that is used for dirty-logging.

Signed-off-by: Leonardo Bras <leo.bras@arm.com>
---
 arch/arm64/include/asm/kvm_mmu.h    |  6 ++++++
 arch/arm64/include/asm/kvm_nested.h |  9 +++++++--
 arch/arm64/kvm/arm.c                |  7 +++++++
 arch/arm64/kvm/mmu.c                | 26 +++++++++++++++++++++++++-
 4 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index 6eae7e7e2a68..3defa1a988d3 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -383,20 +383,26 @@ static inline void kvm_fault_unlock(struct kvm *kvm)
  * and CMOs are NOP'd. This has the effect of no longer requiring a
  * KVA for addresses mapped into the S2. The presence of these features
  * are thus necessary to support cacheable S2 mapping of VM_PFNMAP.
  */
 static inline bool kvm_supports_cacheable_pfnmap(void)
 {
 	return cpus_have_final_cap(ARM64_HAS_STAGE2_FWB) &&
 	       cpus_have_final_cap(ARM64_HAS_CACHE_DIC);
 }
 
+static inline bool kvm_supports_hafdbs(struct kvm *kvm)
+{
+	return IS_ENABLED(CONFIG_ARM64_HW_AFDBM) && has_vhe() &&
+		!kvm_vcpu_has_nv(kvm) && cpus_have_final_cap(ARM64_HW_DBM);
+}
+
 #ifdef CONFIG_PTDUMP_STAGE2_DEBUGFS
 void kvm_s2_ptdump_create_debugfs(struct kvm *kvm);
 void kvm_nested_s2_ptdump_create_debugfs(struct kvm_s2_mmu *mmu);
 void kvm_nested_s2_ptdump_remove_debugfs(struct kvm_s2_mmu *mmu);
 #else
 static inline void kvm_s2_ptdump_create_debugfs(struct kvm *kvm) {}
 static inline void kvm_nested_s2_ptdump_create_debugfs(struct kvm_s2_mmu *mmu) {}
 static inline void kvm_nested_s2_ptdump_remove_debugfs(struct kvm_s2_mmu *mmu) {}
 #endif /* CONFIG_PTDUMP_STAGE2_DEBUGFS */
 
diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
index 1ed708335809..9242b5d665af 100644
--- a/arch/arm64/include/asm/kvm_nested.h
+++ b/arch/arm64/include/asm/kvm_nested.h
@@ -1,24 +1,29 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 #ifndef __ARM64_KVM_NESTED_H
 #define __ARM64_KVM_NESTED_H
 
 #include <linux/bitfield.h>
 #include <linux/kvm_host.h>
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_pgtable.h>
 
-static inline bool vcpu_has_nv(const struct kvm_vcpu *vcpu)
+static inline bool kvm_vcpu_has_nv(const struct kvm *kvm)
 {
 	return (!__is_defined(__KVM_NVHE_HYPERVISOR__) &&
 		cpus_have_final_cap(ARM64_HAS_NESTED_VIRT) &&
-		vcpu_has_feature(vcpu, KVM_ARM_VCPU_HAS_EL2));
+		kvm_vcpu_has_feature(kvm, KVM_ARM_VCPU_HAS_EL2));
+}
+
+static inline bool vcpu_has_nv(const struct kvm_vcpu *vcpu)
+{
+	return kvm_vcpu_has_nv(vcpu->kvm);
 }
 
 /* Translation helpers from non-VHE EL2 to EL1 */
 static inline u64 tcr_el2_ps_to_tcr_el1_ips(u64 tcr_el2)
 {
 	return (u64)FIELD_GET(TCR_EL2_PS_MASK, tcr_el2) << TCR_IPS_SHIFT;
 }
 
 static inline u64 translate_tcr_el2_to_tcr_el1(u64 tcr)
 {
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 1e528d53d093..76aebaa95cda 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1676,20 +1676,27 @@ static int kvm_setup_vcpu(struct kvm_vcpu *vcpu)
 	 * KVM_ARM_VCPU_PMU_V3_SET_PMU.
 	 */
 	if (kvm_vcpu_has_pmu(vcpu) && !kvm->arch.arm_pmu &&
 	    !kvm_vcpu_has_pmuv3_strict(vcpu))
 		ret = kvm_arm_set_default_pmu(kvm);
 
 	/* Prepare for nested if required */
 	if (!ret && vcpu_has_nv(vcpu))
 		ret = kvm_vcpu_init_nested(vcpu);
 
+	/* Enable HAFDBS by default if VHE && !nested  */
+	if (kvm_supports_hafdbs(kvm) &&
+	    atomic_read(&kvm->nr_memslots_dirty_logging) == 0)
+		kvm->arch.mmu.vtcr |= VTCR_EL2_HD;
+	else
+		kvm->arch.mmu.vtcr &= ~VTCR_EL2_HD;
+
 	return ret;
 }
 
 static int __kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
 				 const struct kvm_vcpu_init *init)
 {
 	unsigned long features = init->features[0];
 	struct kvm *kvm = vcpu->kvm;
 	int ret = -EINVAL;
 
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 9d4f70430bbe..94094ab56d90 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1982,21 +1982,22 @@ static int kvm_s2_fault_compute_prot(const struct kvm_s2_fault_desc *s2fd,
 	if (esr_fsc_is_excl_atomic_fault(kvm_vcpu_get_esr(s2fd->vcpu))) {
 		kvm_inject_dabt_excl_atomic(s2fd->vcpu, kvm_vcpu_get_hfar(s2fd->vcpu));
 		return 1;
 	}
 
 	*prot = KVM_PGTABLE_PROT_R;
 
 	if (s2vi->map_writable) {
 		*prot |= KVM_PGTABLE_PROT_W;
 
-		if (s2vi->device || !memslot_is_logging(s2fd->memslot) ||
+		if (s2vi->device ||
+		    !(memslot_is_logging(s2fd->memslot) || kvm_supports_hafdbs(kvm)) ||
 		    kvm_is_write_fault(s2fd->vcpu))
 			*prot |= KVM_PGTABLE_PROT_DIRTY;
 	}
 
 	if (s2fd->nested)
 		*prot = adjust_nested_fault_perms(s2fd->nested, *prot);
 
 	if (kvm_vcpu_trap_is_exec_fault(s2fd->vcpu))
 		*prot |= KVM_PGTABLE_PROT_X;
 
@@ -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);
+}
+
 void kvm_arch_commit_memory_region(struct kvm *kvm,
 				   struct kvm_memory_slot *old,
 				   const struct kvm_memory_slot *new,
 				   enum kvm_mr_change change)
 {
 	bool log_dirty_pages = new && new->flags & KVM_MEM_LOG_DIRTY_PAGES;
 
 	/*
 	 * At this point memslot has been committed and there is an
 	 * allocated dirty_bitmap[], dirty pages will be tracked while the
 	 * memory slot is write protected.
 	 */
 	if (log_dirty_pages) {
 
 		if (change == KVM_MR_DELETE)
 			return;
 
+		/* Disable HAFDBS when dirty-logging starts */
+		if (kvm_supports_hafdbs(kvm))
+			kvm_set_hafdbs(kvm, 0);
+
 		/*
 		 * Huge and normal pages are write-protected and split
 		 * on either of these two cases:
 		 *
 		 * 1. with initial-all-set: gradually with CLEAR ioctls,
 		 */
 		if (kvm_dirty_log_manual_protect_and_init_set(kvm))
 			return;
 		/*
 		 * or
 		 * 2. without initial-all-set: all in one shot when
 		 *    enabling dirty logging.
 		 */
 		kvm_mmu_wp_memory_region(kvm, new->id);
 		kvm_mmu_split_memory_region(kvm, new->id);
 	} else {
+		/* If dirty-logging was canceled, set HAFDBS back on */
+		if (kvm_supports_hafdbs(kvm) &&
+		    atomic_read(&kvm->nr_memslots_dirty_logging) == 0)
+			kvm_set_hafdbs(kvm, 1);
+
 		/*
 		 * Free any leftovers from the eager page splitting cache. Do
 		 * this when deleting, moving, disabling dirty logging, or
 		 * creating the memslot (a nop). Doing it for deletes makes
 		 * sure we don't leak memory, and there's no need to keep the
 		 * cache around for any of the other cases.
 		 */
 		kvm_mmu_free_memory_cache(&kvm->arch.mmu.split_page_cache);
 	}
 }
-- 
2.55.0

Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] walk_nested_s2_pgd() incorrectly evaluates the DBM bit in L1's stage-2 page tables, leading to L2 memory protection bypass and broken L1 dirty tracking.
--

commit 6c412868c1a4af97b366b23a517db706c4c3511a
Author: Leonardo Bras <leo.bras@arm.com>

KVM: arm64: pgtables: Change write bit from S2AP_W to DBM

As a first step of changing the encoding for the Stage2 PTE descriptor,
introduce the DBM bit, and adapt every usage of writable to use the DBM bit
(51) instead of S2AP[1]/Dirty bit (7).

[ ... ]

> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 17123f0b6daba..eb8dfffc32c76 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -386,7 +386,9 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
>  	out->output = paddr;
>  	out->block_size = 1UL << ((3 - level) * stride + wi->pgshift);
>  	out->readable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
> -	out->writable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> +	/* Takes care of both RO/RW and RO/WC/WD encodings */
> +	out->writable = desc & (KVM_PTE_LEAF_ATTR_HI_S2_DBM |
> +				KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W);

[Severity: High]
Does this code in walk_nested_s2_pgd() incorrectly evaluate the DBM bit in
the L1 hypervisor's stage-2 page tables?

By checking for the DBM bit unconditionally here, L0 treats L1's read-only
stage-2 PTEs (where bit 51 is 1 and S2AP_W is 0) as writable. This allows an
L2 guest to write to memory that the L1 hypervisor intended to be read-only,
bypassing L1's memory protection.

Additionally, this breaks L1's dirty tracking because L0 never updates L1's
PTE to set the S2AP_W bit on a write.

Since KVM restricts L1's HAFDBS capability to AF-only (no DBM), bit 51 is
architecturally RES0 or ignored for L1. An honest L1 hypervisor might safely
use this ignored bit for software metadata. A malicious L2 guest could then
exploit this to write to read-only pages (such as shared firmware), breaching
the L1-to-L2 security boundary.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901171558.2674031-1-leo.bras@arm.com?part=1

Re: [RFC PATCH 2/5] KVM: arm64: Add KVM_PGTABLE_PROT_DIRTY

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Silent data corruption and loss of guest memory due to missing host folio dirty tracking with hardware Dirty Bit Management (HW DBM).
--

commit c5bf76c3d6bf3edf785c54c123328d74e2186585
Author: Leonardo Bras <leo.bras@arm.com>
KVM: arm64: Add KVM_PGTABLE_PROT_DIRTY

This commit introduces KVM_PGTABLE_PROT_DIRTY to distinguish between writable
but not dirty pages and writable and dirty pages in the Stage2 PTE descriptor.
It evaluates these concepts on a per-case basis across KVM ARM64 page tables.

> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
[ ... ]
> @@ -731,8 +731,12 @@ static int stage2_set_prot_attr(struct kvm_pgtable *pgt, enum kvm_pgtable_prot p
>  	if (prot & KVM_PGTABLE_PROT_R)
>  		attr |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
>  
> -	if (prot & KVM_PGTABLE_PROT_W)
> -		attr |= KVM_PTE_LEAF_ATTR_HI_S2_DBM | KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> +	if (prot & KVM_PGTABLE_PROT_W) {
> +		attr |= KVM_PTE_LEAF_ATTR_HI_S2_DBM;
> +
> +		if (prot & KVM_PGTABLE_PROT_DIRTY)
> +			attr |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> +	}

[Severity: Critical]
Could this translation enable hardware DBM bypass for pages not marked
PROT_DIRTY? This translates the software PROT_W without PROT_DIRTY into
the hardware PTE state (DBM=1, S2AP_W=0), setting up the HW DBM bypass.

[ ... ]
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
[ ... ]
> @@ -1681,10 +1683,10 @@ static int gmem_abort(const struct kvm_s2_fault_desc *s2fd)
>  	}
>  
>  out_unlock:
> -	kvm_release_faultin_page(kvm, page, !!ret, prot & KVM_PGTABLE_PROT_W);
> +	kvm_release_faultin_page(kvm, page, !!ret, prot & KVM_PGTABLE_PROT_DIRTY);

[Severity: Critical]
Does limiting the dirty tracking here to PROT_DIRTY instead of the mapping
writability (PROT_W) cause host folio dirty tracking to be missed when HAFDBS
is enabled?

[ ... ]
> @@ -1984,11 +1986,14 @@ static int kvm_s2_fault_compute_prot(const struct kvm_s2_fault_desc *s2fd,
>  
>  	*prot = KVM_PGTABLE_PROT_R;
>  
> -	if (s2vi->map_writable && (s2vi->device ||
> -				   !memslot_is_logging(s2fd->memslot) ||
> -				   kvm_is_write_fault(s2fd->vcpu)))
> +	if (s2vi->map_writable) {
>  		*prot |= KVM_PGTABLE_PROT_W;
>  
> +		if (s2vi->device || !memslot_is_logging(s2fd->memslot) ||
> +		    kvm_is_write_fault(s2fd->vcpu))
> +			*prot |= KVM_PGTABLE_PROT_DIRTY;
> +	}

[Severity: Critical]
On a read fault when HAFDBS is enabled, this leaves KVM_PGTABLE_PROT_DIRTY
unset while KVM_PGTABLE_PROT_W is set. Can this allow the hardware to
locklessly dirty the page later without KVM knowing?

[ ... ]
> @@ -2019,7 +2024,7 @@ static int kvm_s2_fault_map(const struct kvm_s2_fault_desc *s2fd,
>  			    void *memcache)
>  {
>  	enum kvm_pgtable_walk_flags flags = KVM_PGTABLE_WALK_SHARED;
> -	bool writable = prot & KVM_PGTABLE_PROT_W;
> +	bool dirty = prot & KVM_PGTABLE_PROT_DIRTY;

[Severity: Critical]
By deriving the dirty status for host folio tracking strictly from PROT_DIRTY
instead of PROT_W, are we setting up the host MM to track the folio as clean
on read faults?

[ ... ]
> @@ -2082,7 +2087,7 @@ static int kvm_s2_fault_map(const struct kvm_s2_fault_desc *s2fd,
>  	}
>  
>  out_unlock:
> -	kvm_release_faultin_page(kvm, s2vi->page, !!ret, writable);
> +	kvm_release_faultin_page(kvm, s2vi->page, !!ret, dirty);

[Severity: Critical]
Releasing the folio to the host MM as clean on read faults could break host
folio dirty tracking.

If a guest later writes to this page and the CPU locklessly sets S2AP_W=1
without trapping, KVM won't be aware. Since KVM ARM64 does not harvest this
dirty bit during MMU notifier invalidations, the host Linux MM never learns
the folio was dirtied. When the host reclaims the folio under memory
pressure, does this result in data corruption and loss of guest memory by
discarding the modified data instead of writing it to swap?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901171558.2674031-1-leo.bras@arm.com?part=2

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] Data race and TOCTOU on `kvm->arch.mmu.vtcr` between vCPU setup and memslot updates, breaking dirty logging.
- [High] Missing context-wide TLB invalidation after modifying `VTCR_EL2.HD` results in hardware bypassing write-protection.
- [High] Disabling HAFDBS globally for dirty logging causes massive performance degradation on non-logging memslots due to unnecessary permission faults.
- [Low] Unreachable `KVM_MR_DELETE` check inside `log_dirty_pages` block.
--

commit 54d7950cd735bc1edc3a9503d03ebffb8e6ca046
Author: Leonardo Bras <leo.bras@arm.com>

KVM: arm64: Enable HAFDBS for guests not on migration

This patch enables hardware management of access and dirty bits (HAFDBS)
for KVM guests on arm64 when dirty logging is disabled. It modifies
vCPU setup and memslot commit paths to dynamically toggle VTCR_EL2.HD
based on the presence of dirty-logged memslots.

> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 1e528d53d0931..76aebaa95cdab 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c

[ ... ]

> @@ -1683,6 +1683,13 @@ static int kvm_setup_vcpu(struct kvm_vcpu *vcpu)
>  	if (!ret && vcpu_has_nv(vcpu))
>  		ret = kvm_vcpu_init_nested(vcpu);
>  
> +	/* Enable HAFDBS by default if VHE && !nested  */
> +	if (kvm_supports_hafdbs(kvm) &&
> +	    atomic_read(&kvm->nr_memslots_dirty_logging) == 0)
> +		kvm->arch.mmu.vtcr |= VTCR_EL2_HD;
> +	else
> +		kvm->arch.mmu.vtcr &= ~VTCR_EL2_HD;
> +

[Severity: Critical]
Does this read-modify-write of kvm->arch.mmu.vtcr (holding config_lock)
create a data race against kvm->slots_lock?

In kvm_set_hafdbs() below, this same variable is modified while holding
kvm->slots_lock. If userspace concurrently initializes vCPUs and enables
dirty logging, VTCR_EL2_HD could be incorrectly left at 1. This would cause
the hardware to silently update dirty bits without trapping, bypassing
software dirty logging and resulting in lost dirty pages during migration.

>  	return ret;
>  }

[ ... ]

> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 9d4f70430bbea..94094ab56d90f 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1989,7 +1989,8 @@ static int kvm_s2_fault_compute_prot(const struct kvm_s2_fault_desc *s2fd,
>  	if (s2vi->map_writable) {
>  		*prot |= KVM_PGTABLE_PROT_W;
>  
> -		if (s2vi->device || !memslot_is_logging(s2fd->memslot) ||
> +		if (s2vi->device ||
> +		    !(memslot_is_logging(s2fd->memslot) || kvm_supports_hafdbs(kvm)) ||
>  		    kvm_is_write_fault(s2fd->vcpu))
>  			*prot |= KVM_PGTABLE_PROT_DIRTY;

[Severity: High]
Can checking the static capability kvm_supports_hafdbs() here instead of
the dynamic vtcr state starve non-logging slots of the software fallback?

When dirty logging is enabled on one memslot, kvm_set_hafdbs() disables
VTCR_EL2.HD globally. However, if a guest writes to an unrelated
non-logging slot that was mapped writable-clean, won't the hardware now
generate a stage-2 permission fault?

This could cause performance degradation across the entire VM whenever any
single memslot (like a VGA framebuffer) enables dirty logging.

[ ... ]

> @@ -2577,6 +2578,20 @@ int __init kvm_mmu_init(u32 hyp_va_bits)
>  	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;

[Severity: Critical]
Does modifying kvm->arch.mmu.vtcr here under kvm->slots_lock race against
the unprotected updates in kvm_setup_vcpu() as noted above?

> +
> +	kvm_make_all_cpus_request(kvm, KVM_REQ_RELOAD_STAGE2);

[Severity: High]
Is KVM_REQ_RELOAD_STAGE2 sufficient here to ensure the hardware observes
the changed VTCR_EL2 state?

The ARM architecture permits translation regime registers like VTCR_EL2 to
be cached in the TLB (or a context cache). Modifying this register
typically requires a full VMID TLB invalidation (like calling
kvm_flush_remote_tlbs(kvm)) to guarantee global visibility. 

Without it, the CPU might continue using cached translations with HD=1 in
the write-protected memslot, silently bypassing write-protection.

> +}
> +
>  void kvm_arch_commit_memory_region(struct kvm *kvm,
>  				   struct kvm_memory_slot *old,
>  				   const struct kvm_memory_slot *new,
> @@ -2594,6 +2609,10 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
>  		if (change == KVM_MR_DELETE)
>  			return;

[Severity: Low]
Is this KVM_MR_DELETE condition reachable?

The log_dirty_pages variable checked just before this is initialized as
'new && new->flags & KVM_MEM_LOG_DIRTY_PAGES'. During a KVM_MR_DELETE
change, the 'new' parameter passed into kvm_arch_commit_memory_region() is
strictly NULL. Therefore, log_dirty_pages will always be false for
deletes, making this nested check dead code.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901171558.2674031-1-leo.bras@arm.com?part=5

Re: [RFC PATCH 4/5] KVM: arm64: Add KVM_REQ_RELOAD_STAGE2

[Tian Zheng]

On 9/2/2026 1:15 AM, Leonardo Bras wrote:
> Add a vcpu request to exit guest, reload Stage2, and then come back to
> guest.
> 
> This will be used on future patches that enable S2 HAFDBS and HDBSS, as
> they may need to change VTCR bits for enabling/disabling the feature when
> the vcpus are still running.
> 
> Signed-off-by: Leonardo Bras <leo.bras@arm.com>
> ---
>   arch/arm64/include/asm/kvm_host.h | 2 ++
>   arch/arm64/kvm/arm.c              | 8 ++++++++
>   2 files changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 27fe0cd5b2d7..00fe169f239f 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -48,20 +48,22 @@
>   #define KVM_REQ_VCPU_RESET		KVM_ARCH_REQ(2)
>   #define KVM_REQ_RECORD_STEAL		KVM_ARCH_REQ(3)
>   #define KVM_REQ_RELOAD_GICv4		KVM_ARCH_REQ(4)
>   #define KVM_REQ_RELOAD_PMU		KVM_ARCH_REQ(5)
>   #define KVM_REQ_SUSPEND			KVM_ARCH_REQ(6)
>   #define KVM_REQ_RESYNC_PMU_EL0		KVM_ARCH_REQ(7)
>   #define KVM_REQ_NESTED_S2_UNMAP		KVM_ARCH_REQ(8)
>   #define KVM_REQ_GUEST_HYP_IRQ_PENDING	KVM_ARCH_REQ(9)
>   #define KVM_REQ_MAP_L1_VNCR_EL2		KVM_ARCH_REQ(10)
>   #define KVM_REQ_VGIC_PROCESS_UPDATE	KVM_ARCH_REQ(11)
> +#define KVM_REQ_RELOAD_STAGE2 \
> +	KVM_ARCH_REQ_FLAGS(12, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
>   
>   #define KVM_DIRTY_LOG_MANUAL_CAPS   (KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE | \
>   				     KVM_DIRTY_LOG_INITIALLY_SET)
>   
>   #define KVM_HAVE_MMU_RWLOCK
>   
>   /*
>    * Mode of operation configurable with kvm-arm.mode early param.
>    * See Documentation/admin-guide/kernel-parameters.txt for more information.
>    */
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 8b080804bc90..1e528d53d093 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1164,20 +1164,28 @@ static int check_vcpu_requests(struct kvm_vcpu *vcpu)
>   
>   		if (kvm_check_request(KVM_REQ_RESYNC_PMU_EL0, vcpu))
>   			kvm_vcpu_pmu_restore_guest(vcpu);
>   
>   		if (kvm_check_request(KVM_REQ_SUSPEND, vcpu))
>   			return kvm_vcpu_suspend(vcpu);
>   
>   		if (kvm_dirty_ring_check_request(vcpu))
>   			return 0;
>   
> +		if (kvm_check_request(KVM_REQ_RELOAD_STAGE2, vcpu)) {
> +			unsigned long flags;
> +
> +			local_irq_save(flags);
> +			__load_stage2(vcpu->arch.hw_mmu);
> +			local_irq_restore(flags);
> +		}
> +
>   		check_nested_vcpu_requests(vcpu);
>   	}
>   
>   	return 1;
>   }
>   
>   static bool vcpu_mode_is_bad_32bit(struct kvm_vcpu *vcpu)
>   {
>   	if (likely(!vcpu_mode_is_32bit(vcpu)))
>   		return false;
Hi Leo,

Thanks for sending this out. I actually implemented a similar request in
my v5 HDBSS series to reload VTCR on enable/disable, but I named it
KVM_REQ_HDBSS_RELOAD_VTCR.

Without such a request, the new VTCR value only takes effect on the next
vcpu_load, which means enable/disable could be delayed indefinitely if
the vCPU never goes through a load/put cycle. So I think this request is
necessary for HDBSS as well.

Since your KVM_REQ_RELOAD_STAGE2 is more generic, I'd like to rebase on
top of your patch and reuse it in v5. Would that work for you? Or do you
prefer to keep them separate for now?

Thanks,
Tian

Re: [RFC PATCH 4/5] KVM: arm64: Add KVM_REQ_RELOAD_STAGE2

[Leonardo Bras]

On Wed, Sep 02, 2026 at 11:41:39AM +0800, Tian Zheng wrote:
> 
> 
> On 9/2/2026 1:15 AM, Leonardo Bras wrote:
> > Add a vcpu request to exit guest, reload Stage2, and then come back to
> > guest.
> > 
> > This will be used on future patches that enable S2 HAFDBS and HDBSS, as
> > they may need to change VTCR bits for enabling/disabling the feature when
> > the vcpus are still running.
> > 
> > Signed-off-by: Leonardo Bras <leo.bras@arm.com>
> > ---
> >   arch/arm64/include/asm/kvm_host.h | 2 ++
> >   arch/arm64/kvm/arm.c              | 8 ++++++++
> >   2 files changed, 10 insertions(+)
> > 
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index 27fe0cd5b2d7..00fe169f239f 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -48,20 +48,22 @@
> >   #define KVM_REQ_VCPU_RESET		KVM_ARCH_REQ(2)
> >   #define KVM_REQ_RECORD_STEAL		KVM_ARCH_REQ(3)
> >   #define KVM_REQ_RELOAD_GICv4		KVM_ARCH_REQ(4)
> >   #define KVM_REQ_RELOAD_PMU		KVM_ARCH_REQ(5)
> >   #define KVM_REQ_SUSPEND			KVM_ARCH_REQ(6)
> >   #define KVM_REQ_RESYNC_PMU_EL0		KVM_ARCH_REQ(7)
> >   #define KVM_REQ_NESTED_S2_UNMAP		KVM_ARCH_REQ(8)
> >   #define KVM_REQ_GUEST_HYP_IRQ_PENDING	KVM_ARCH_REQ(9)
> >   #define KVM_REQ_MAP_L1_VNCR_EL2		KVM_ARCH_REQ(10)
> >   #define KVM_REQ_VGIC_PROCESS_UPDATE	KVM_ARCH_REQ(11)
> > +#define KVM_REQ_RELOAD_STAGE2 \
> > +	KVM_ARCH_REQ_FLAGS(12, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
> >   #define KVM_DIRTY_LOG_MANUAL_CAPS   (KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE | \
> >   				     KVM_DIRTY_LOG_INITIALLY_SET)
> >   #define KVM_HAVE_MMU_RWLOCK
> >   /*
> >    * Mode of operation configurable with kvm-arm.mode early param.
> >    * See Documentation/admin-guide/kernel-parameters.txt for more information.
> >    */
> > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > index 8b080804bc90..1e528d53d093 100644
> > --- a/arch/arm64/kvm/arm.c
> > +++ b/arch/arm64/kvm/arm.c
> > @@ -1164,20 +1164,28 @@ static int check_vcpu_requests(struct kvm_vcpu *vcpu)
> >   		if (kvm_check_request(KVM_REQ_RESYNC_PMU_EL0, vcpu))
> >   			kvm_vcpu_pmu_restore_guest(vcpu);
> >   		if (kvm_check_request(KVM_REQ_SUSPEND, vcpu))
> >   			return kvm_vcpu_suspend(vcpu);
> >   		if (kvm_dirty_ring_check_request(vcpu))
> >   			return 0;
> > +		if (kvm_check_request(KVM_REQ_RELOAD_STAGE2, vcpu)) {
> > +			unsigned long flags;
> > +
> > +			local_irq_save(flags);
> > +			__load_stage2(vcpu->arch.hw_mmu);
> > +			local_irq_restore(flags);
> > +		}
> > +
> >   		check_nested_vcpu_requests(vcpu);
> >   	}
> >   	return 1;
> >   }
> >   static bool vcpu_mode_is_bad_32bit(struct kvm_vcpu *vcpu)
> >   {
> >   	if (likely(!vcpu_mode_is_32bit(vcpu)))
> >   		return false;
> Hi Leo,
> 
> Thanks for sending this out. I actually implemented a similar request in
> my v5 HDBSS series to reload VTCR on enable/disable, but I named it
> KVM_REQ_HDBSS_RELOAD_VTCR.
> 
> Without such a request, the new VTCR value only takes effect on the next
> vcpu_load, which means enable/disable could be delayed indefinitely if
> the vCPU never goes through a load/put cycle. So I think this request is
> necessary for HDBSS as well.
> 
> Since your KVM_REQ_RELOAD_STAGE2 is more generic, I'd like to rebase on
> top of your patch and reuse it in v5. Would that work for you? Or do you
> prefer to keep them separate for now?
> 

Hi Tian, 
Thanks for reviewing!

The main reason for this patch series (at least up to patch #3) was to give 
you a base for HDBSS, so please rebase as much as you want on top of it.

Thanks!
Leo

Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM

[Leonardo Bras]

On Tue, Sep 01, 2026 at 05:30:14PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] walk_nested_s2_pgd() incorrectly evaluates the DBM bit in L1's stage-2 page tables, leading to L2 memory protection bypass and broken L1 dirty tracking.
> --
> 
> commit 6c412868c1a4af97b366b23a517db706c4c3511a
> Author: Leonardo Bras <leo.bras@arm.com>
> 
> KVM: arm64: pgtables: Change write bit from S2AP_W to DBM
> 
> As a first step of changing the encoding for the Stage2 PTE descriptor,
> introduce the DBM bit, and adapt every usage of writable to use the DBM bit
> (51) instead of S2AP[1]/Dirty bit (7).
> 
> [ ... ]
> 
> > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> > index 17123f0b6daba..eb8dfffc32c76 100644
> > --- a/arch/arm64/kvm/nested.c
> > +++ b/arch/arm64/kvm/nested.c
> > @@ -386,7 +386,9 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
> >  	out->output = paddr;
> >  	out->block_size = 1UL << ((3 - level) * stride + wi->pgshift);
> >  	out->readable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
> > -	out->writable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > +	/* Takes care of both RO/RW and RO/WC/WD encodings */
> > +	out->writable = desc & (KVM_PTE_LEAF_ATTR_HI_S2_DBM |
> > +				KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W);
> 
> [Severity: High]
> Does this code in walk_nested_s2_pgd() incorrectly evaluate the DBM bit in
> the L1 hypervisor's stage-2 page tables?
> 
> By checking for the DBM bit unconditionally here, L0 treats L1's read-only
> stage-2 PTEs (where bit 51 is 1 and S2AP_W is 0) as writable. This allows an
> L2 guest to write to memory that the L1 hypervisor intended to be read-only,
> bypassing L1's memory protection.

Yeah, for this to work on NV we need Oliver's patch series enabling HAFDBS 
emulation.

> 
> Additionally, this breaks L1's dirty tracking because L0 never updates L1's
> PTE to set the S2AP_W bit on a write.
> 
> Since KVM restricts L1's HAFDBS capability to AF-only (no DBM), bit 51 is
> architecturally RES0 or ignored for L1. An honest L1 hypervisor might safely
> use this ignored bit for software metadata. A malicious L2 guest could then
> exploit this to write to read-only pages (such as shared firmware), breaching
> the L1-to-L2 security boundary.
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260901171558.2674031-1-leo.bras@arm.com?part=1

Re: [RFC PATCH 2/5] KVM: arm64: Add KVM_PGTABLE_PROT_DIRTY

[Leonardo Bras]

On Tue, Sep 01, 2026 at 05:34:52PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] Silent data corruption and loss of guest memory due to missing host folio dirty tracking with hardware Dirty Bit Management (HW DBM).
> --
> 
> commit c5bf76c3d6bf3edf785c54c123328d74e2186585
> Author: Leonardo Bras <leo.bras@arm.com>
> KVM: arm64: Add KVM_PGTABLE_PROT_DIRTY
> 
> This commit introduces KVM_PGTABLE_PROT_DIRTY to distinguish between writable
> but not dirty pages and writable and dirty pages in the Stage2 PTE descriptor.
> It evaluates these concepts on a per-case basis across KVM ARM64 page tables.
> 
> > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> > --- a/arch/arm64/kvm/hyp/pgtable.c
> > +++ b/arch/arm64/kvm/hyp/pgtable.c
> [ ... ]
> > @@ -731,8 +731,12 @@ static int stage2_set_prot_attr(struct kvm_pgtable *pgt, enum kvm_pgtable_prot p
> >  	if (prot & KVM_PGTABLE_PROT_R)
> >  		attr |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
> >  
> > -	if (prot & KVM_PGTABLE_PROT_W)
> > -		attr |= KVM_PTE_LEAF_ATTR_HI_S2_DBM | KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > +	if (prot & KVM_PGTABLE_PROT_W) {
> > +		attr |= KVM_PTE_LEAF_ATTR_HI_S2_DBM;
> > +
> > +		if (prot & KVM_PGTABLE_PROT_DIRTY)
> > +			attr |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > +	}
> 
> [Severity: Critical]
> Could this translation enable hardware DBM bypass for pages not marked
> PROT_DIRTY? This translates the software PROT_W without PROT_DIRTY into
> the hardware PTE state (DBM=1, S2AP_W=0), setting up the HW DBM bypass.

That's the desired effect, as we can use HAFDBS/HDBSS to automatically 
update and track them.

> 
> [ ... ]
> > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > --- a/arch/arm64/kvm/mmu.c
> > +++ b/arch/arm64/kvm/mmu.c
> [ ... ]
> > @@ -1681,10 +1683,10 @@ static int gmem_abort(const struct kvm_s2_fault_desc *s2fd)
> >  	}
> >  
> >  out_unlock:
> > -	kvm_release_faultin_page(kvm, page, !!ret, prot & KVM_PGTABLE_PROT_W);
> > +	kvm_release_faultin_page(kvm, page, !!ret, prot & KVM_PGTABLE_PROT_DIRTY);
> 
> [Severity: Critical]
> Does limiting the dirty tracking here to PROT_DIRTY instead of the mapping
> writability (PROT_W) cause host folio dirty tracking to be missed when HAFDBS
> is enabled?
> 

Yes, that's why for dirty-tracking host should either disable HAFDBS or 
enable HDBSS if available.

> [ ... ]
> > @@ -1984,11 +1986,14 @@ static int kvm_s2_fault_compute_prot(const struct kvm_s2_fault_desc *s2fd,
> >  
> >  	*prot = KVM_PGTABLE_PROT_R;
> >  
> > -	if (s2vi->map_writable && (s2vi->device ||
> > -				   !memslot_is_logging(s2fd->memslot) ||
> > -				   kvm_is_write_fault(s2fd->vcpu)))
> > +	if (s2vi->map_writable) {
> >  		*prot |= KVM_PGTABLE_PROT_W;
> >  
> > +		if (s2vi->device || !memslot_is_logging(s2fd->memslot) ||
> > +		    kvm_is_write_fault(s2fd->vcpu))
> > +			*prot |= KVM_PGTABLE_PROT_DIRTY;
> > +	}
> 
> [Severity: Critical]
> On a read fault when HAFDBS is enabled, this leaves KVM_PGTABLE_PROT_DIRTY
> unset while KVM_PGTABLE_PROT_W is set. Can this allow the hardware to
> locklessly dirty the page later without KVM knowing?

That's the desired behavior. If we need tracking, see above comment.

> 
> [ ... ]
> > @@ -2019,7 +2024,7 @@ static int kvm_s2_fault_map(const struct kvm_s2_fault_desc *s2fd,
> >  			    void *memcache)
> >  {
> >  	enum kvm_pgtable_walk_flags flags = KVM_PGTABLE_WALK_SHARED;
> > -	bool writable = prot & KVM_PGTABLE_PROT_W;
> > +	bool dirty = prot & KVM_PGTABLE_PROT_DIRTY;
> 
> [Severity: Critical]
> By deriving the dirty status for host folio tracking strictly from PROT_DIRTY
> instead of PROT_W, are we setting up the host MM to track the folio as clean
> on read faults?
> 

Yes. The main idea is that a read fault does _not_ dirty a folio, just as 
it already happens when dirty-tracking is enabled.

> [ ... ]
> > @@ -2082,7 +2087,7 @@ static int kvm_s2_fault_map(const struct kvm_s2_fault_desc *s2fd,
> >  	}
> >  
> >  out_unlock:
> > -	kvm_release_faultin_page(kvm, s2vi->page, !!ret, writable);
> > +	kvm_release_faultin_page(kvm, s2vi->page, !!ret, dirty);
> 
> [Severity: Critical]
> Releasing the folio to the host MM as clean on read faults could break host
> folio dirty tracking.
> 
> If a guest later writes to this page and the CPU locklessly sets S2AP_W=1
> without trapping, KVM won't be aware. Since KVM ARM64 does not harvest this
> dirty bit during MMU notifier invalidations, the host Linux MM never learns
> the folio was dirtied. When the host reclaims the folio under memory
> pressure, does this result in data corruption and loss of guest memory by
> discarding the modified data instead of writing it to swap?

Ah, good catch. IIUC:
CPU0					CPU1
kvm_s2_fault_map - WC - pageA
	sets dirty=false
	...				HAFDBS sets pageA dirty
	kvm_release_faultin_page
		kvm_release_page_clean

So, in this case, I think it would be better to use writable instead of 
clean, as IIUC there should be no issue marking a clean page as dirty other 
than a bit of overhead on calling kvm_set_page_dirty().

Does it work?

Thanks!
Leo

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

[Leonardo Bras]

On Tue, Sep 01, 2026 at 05:49:41PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> - [Critical] Data race and TOCTOU on `kvm->arch.mmu.vtcr` between vCPU setup and memslot updates, breaking dirty logging.
> - [High] Missing context-wide TLB invalidation after modifying `VTCR_EL2.HD` results in hardware bypassing write-protection.
> - [High] Disabling HAFDBS globally for dirty logging causes massive performance degradation on non-logging memslots due to unnecessary permission faults.
> - [Low] Unreachable `KVM_MR_DELETE` check inside `log_dirty_pages` block.
> --
> 
> commit 54d7950cd735bc1edc3a9503d03ebffb8e6ca046
> Author: Leonardo Bras <leo.bras@arm.com>
> 
> KVM: arm64: Enable HAFDBS for guests not on migration
> 
> This patch enables hardware management of access and dirty bits (HAFDBS)
> for KVM guests on arm64 when dirty logging is disabled. It modifies
> vCPU setup and memslot commit paths to dynamically toggle VTCR_EL2.HD
> based on the presence of dirty-logged memslots.
> 
> > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > index 1e528d53d0931..76aebaa95cdab 100644
> > --- a/arch/arm64/kvm/arm.c
> > +++ b/arch/arm64/kvm/arm.c
> 
> [ ... ]
> 
> > @@ -1683,6 +1683,13 @@ static int kvm_setup_vcpu(struct kvm_vcpu *vcpu)
> >  	if (!ret && vcpu_has_nv(vcpu))
> >  		ret = kvm_vcpu_init_nested(vcpu);
> >  
> > +	/* Enable HAFDBS by default if VHE && !nested  */
> > +	if (kvm_supports_hafdbs(kvm) &&
> > +	    atomic_read(&kvm->nr_memslots_dirty_logging) == 0)
> > +		kvm->arch.mmu.vtcr |= VTCR_EL2_HD;
> > +	else
> > +		kvm->arch.mmu.vtcr &= ~VTCR_EL2_HD;
> > +
> 
> [Severity: Critical]
> Does this read-modify-write of kvm->arch.mmu.vtcr (holding config_lock)
> create a data race against kvm->slots_lock?
> 
> In kvm_set_hafdbs() below, this same variable is modified while holding
> kvm->slots_lock. If userspace concurrently initializes vCPUs and enables
> dirty logging, VTCR_EL2_HD could be incorrectly left at 1. This would cause
> the hardware to silently update dirty bits without trapping, bypassing
> software dirty logging and resulting in lost dirty pages during migration.

I think that this may be a hard to hit scenario, but to be sure I think it 
should not hurt to hold the slots lock here as well.

Have to properly check if I won't cause any kind of dead/live lock here, 
though.

> 
> >  	return ret;
> >  }
> 
> [ ... ]
> 
> > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > index 9d4f70430bbea..94094ab56d90f 100644
> > --- a/arch/arm64/kvm/mmu.c
> > +++ b/arch/arm64/kvm/mmu.c
> > @@ -1989,7 +1989,8 @@ static int kvm_s2_fault_compute_prot(const struct kvm_s2_fault_desc *s2fd,
> >  	if (s2vi->map_writable) {
> >  		*prot |= KVM_PGTABLE_PROT_W;
> >  
> > -		if (s2vi->device || !memslot_is_logging(s2fd->memslot) ||
> > +		if (s2vi->device ||
> > +		    !(memslot_is_logging(s2fd->memslot) || kvm_supports_hafdbs(kvm)) ||
> >  		    kvm_is_write_fault(s2fd->vcpu))
> >  			*prot |= KVM_PGTABLE_PROT_DIRTY;
> 
> [Severity: High]
> Can checking the static capability kvm_supports_hafdbs() here instead of
> the dynamic vtcr state starve non-logging slots of the software fallback?
> 
> When dirty logging is enabled on one memslot, kvm_set_hafdbs() disables
> VTCR_EL2.HD globally. However, if a guest writes to an unrelated
> non-logging slot that was mapped writable-clean, won't the hardware now
> generate a stage-2 permission fault?
> 
> This could cause performance degradation across the entire VM whenever any
> single memslot (like a VGA framebuffer) enables dirty logging.
> 

Yes, that's an expected tradeoff. I believed it would be fine as the 
usecase I had in mind for dirty-tracking was live migration, which usually 
tracks all memory at once, causing no performance degradation.

If we are dirty-tracking a memslot during the whole VM runtime, as is the 
case for a VGA framebuffer, maybe this is a special scenario, and we should 
make the page dirty as well.

If there is a common use case for tracking a single memory slot of regular 
memory (not used as device memory and so on), then maybe this patch has to 
be reworked to take that into account.

> [ ... ]
> 
> > @@ -2577,6 +2578,20 @@ int __init kvm_mmu_init(u32 hyp_va_bits)
> >  	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;
> 
> [Severity: Critical]
> Does modifying kvm->arch.mmu.vtcr here under kvm->slots_lock race against
> the unprotected updates in kvm_setup_vcpu() as noted above?
> 

There is a comment on that above.

> > +
> > +	kvm_make_all_cpus_request(kvm, KVM_REQ_RELOAD_STAGE2);
> 
> [Severity: High]
> Is KVM_REQ_RELOAD_STAGE2 sufficient here to ensure the hardware observes
> the changed VTCR_EL2 state?
> 

This is called when we enable dirty-tracking, and the next steps after that 
will be to clean the dirty-bits and then do a TLBi, so it should be fine 
for that scenario.

Maybe I should add a comment to explain the caller is responsible to carry 
a TLBi.


> The ARM architecture permits translation regime registers like VTCR_EL2 to
> be cached in the TLB (or a context cache). Modifying this register
> typically requires a full VMID TLB invalidation (like calling
> kvm_flush_remote_tlbs(kvm)) to guarantee global visibility. 
> 
> Without it, the CPU might continue using cached translations with HD=1 in
> the write-protected memslot, silently bypassing write-protection.
> 
> > +}
> > +
> >  void kvm_arch_commit_memory_region(struct kvm *kvm,
> >  				   struct kvm_memory_slot *old,
> >  				   const struct kvm_memory_slot *new,
> > @@ -2594,6 +2609,10 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
> >  		if (change == KVM_MR_DELETE)
> >  			return;
> 
> [Severity: Low]
> Is this KVM_MR_DELETE condition reachable?
> 
> The log_dirty_pages variable checked just before this is initialized as
> 'new && new->flags & KVM_MEM_LOG_DIRTY_PAGES'. During a KVM_MR_DELETE
> change, the 'new' parameter passed into kvm_arch_commit_memory_region() is
> strictly NULL. Therefore, log_dirty_pages will always be false for
> deletes, making this nested check dead code.

Not sure what this is trying to point. 

Yes, disabling HAFDBS will not happen if we are deleting the memslot, and 
also it will not enable it again if it's already enabled, so we should be 
safe.

Thanks!
Leo

Re: [RFC PATCH 0/5] KVM: arm64: New PTE dirty-page encoding, HAFDBS new usage

[Marc Zyngier]

On Tue, 01 Sep 2026 18:15:51 +0100,
Leonardo Bras <leo.bras@arm.com> wrote:
> 
> This series have 2 main goals:
> 
> 1 - Patches #1,#2,#3 : Change the PTE descriptor to use WD/WC/RO encodings
>     making use of the DBM bit, adapting all usages, and

What are WD and WC? I can sort of guess that this is write-dirty and
write clean, but that's not exactly obvious. More importantly, you
don't even explain *why* anything needs changing...

> 2 - Patches #4,#5 are an RFC on using HAFDBS on a guest to avoid resetting
>     all PTEs to WC when dirty-logging starts, speeding-up startup.
> 
> (1) will also introduce a new walker for cleaning the dirty-bit, which will
> clear the DBM bit if it's a block mapping (hugepage). This is needed as on
> lazy-splitting we need to fault a write so we can do the lazy splitting.
> This is needed for both the next patches, and for HDBSS & HACDBS
> enablement.

Again, this is incredibly opaque to the reviewer. What is the problem
you are trying to solve? This is what a cover letter is for.

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM

[Marc Zyngier]

On Tue, 01 Sep 2026 18:15:52 +0100,
Leonardo Bras <leo.bras@arm.com> wrote:
> 
> As a first step of changing the encoding for the Stage2 PTE descriptor,
> introduce the DBM bit, and adapt every usage of writable to use the DBM bit
> (51) instead of S2AP[1]/Dirty bit (7).
> 
> For this step, we convert usages of RW(Dirty) -> WD(DBM|Dirty).
> 
> Signed-off-by: Leonardo Bras <leo.bras@arm.com>
> ---
>  arch/arm64/include/asm/kvm_pgtable.h | 3 +++
>  arch/arm64/kvm/hyp/pgtable.c         | 8 +++++---
>  arch/arm64/kvm/nested.c              | 4 +++-
>  arch/arm64/kvm/ptdump.c              | 4 ++--
>  4 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> index 41a8687938eb..37baa86d6fd8 100644
> --- a/arch/arm64/include/asm/kvm_pgtable.h
> +++ b/arch/arm64/include/asm/kvm_pgtable.h
> @@ -86,24 +86,27 @@ typedef u64 kvm_pte_t;
>  #define KVM_PTE_LEAF_ATTR_HI		GENMASK(63, 50)
>  
>  #define KVM_PTE_LEAF_ATTR_HI_SW		GENMASK(58, 55)
>  
>  #define KVM_PTE_LEAF_ATTR_HI_S1_XN	BIT(54)
>  #define KVM_PTE_LEAF_ATTR_HI_S1_UXN	BIT(54)
>  #define KVM_PTE_LEAF_ATTR_HI_S1_PXN	BIT(53)
>  
>  #define KVM_PTE_LEAF_ATTR_HI_S2_XN	GENMASK(54, 53)
>  
> +#define KVM_PTE_LEAF_ATTR_HI_S2_DBM	BIT(51)
> +
>  #define KVM_PTE_LEAF_ATTR_HI_S1_GP	BIT(50)
>  
>  #define KVM_PTE_LEAF_ATTR_S2_PERMS	(KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R | \
>  					 KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W | \
> +					 KVM_PTE_LEAF_ATTR_HI_S2_DBM | \
>  					 KVM_PTE_LEAF_ATTR_HI_S2_XN)
>  
>  /* pKVM invalid pte encodings */
>  #define KVM_INVALID_PTE_TYPE_MASK	GENMASK(63, 60)
>  #define KVM_INVALID_PTE_ANNOT_MASK	~(KVM_PTE_VALID | \
>  					  KVM_INVALID_PTE_TYPE_MASK)
>  
>  enum kvm_invalid_pte_type {
>  	/*
>  	 * Used to indicate a pte for which a 'break-before-make'
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index b74dd5ce1efd..ca49f1bd7c34 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -725,42 +725,43 @@ static int stage2_set_prot_attr(struct kvm_pgtable *pgt, enum kvm_pgtable_prot p
>  	}
>  
>  	r = stage2_set_xn_attr(prot, &attr);
>  	if (r)
>  		return r;
>  
>  	if (prot & KVM_PGTABLE_PROT_R)
>  		attr |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
>  
>  	if (prot & KVM_PGTABLE_PROT_W)
> -		attr |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> +		attr |= KVM_PTE_LEAF_ATTR_HI_S2_DBM | KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;

What makes it acceptable to always set DBM? This is an optional
feature, and I'm not exactly comfortable setting bits that are
supposed to be RES0. Yes, the HW should ignore it. But we have also
seen quite a few broken designs in this area...

> +
>

Spurious newline.

>  	if (!kvm_lpa2_is_enabled())
>  		attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S2_SH, sh);
>  
>  	attr |= KVM_PTE_LEAF_ATTR_LO_S2_AF;
>  	attr |= prot & KVM_PTE_LEAF_ATTR_HI_SW;
>  	*ptep = attr;
>  
>  	return 0;
>  }

I don't know how you have configured git on your end, but there is *a
lot* of context...

>  
>  enum kvm_pgtable_prot kvm_pgtable_stage2_pte_prot(kvm_pte_t pte)
>  {
>  	enum kvm_pgtable_prot prot = pte & KVM_PTE_LEAF_ATTR_HI_SW;
>  
>  	if (!kvm_pte_valid(pte))
>  		return prot;
>  
>  	if (pte & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R)
>  		prot |= KVM_PGTABLE_PROT_R;
> -	if (pte & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W)
> +	if (pte & KVM_PTE_LEAF_ATTR_HI_S2_DBM)
>  		prot |= KVM_PGTABLE_PROT_W;
>  
>  	switch (FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, pte)) {
>  	case 0b00:
>  		prot |= KVM_PGTABLE_PROT_PX | KVM_PGTABLE_PROT_UX;
>  		break;
>  	case 0b01:
>  		prot |= KVM_PGTABLE_PROT_UX;
>  		break;
>  	case 0b11:
> @@ -1281,20 +1282,21 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,
>  		*orig_pte = data.pte;
>  
>  	if (level)
>  		*level = data.level;
>  	return 0;
>  }
>  
>  int kvm_pgtable_stage2_wrprotect(struct kvm_pgtable *pgt, u64 addr, u64 size)
>  {
>  	return stage2_update_leaf_attrs(pgt, addr, size, 0,
> +					KVM_PTE_LEAF_ATTR_HI_S2_DBM |
>  					KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W,
>  					NULL, NULL,
>  					KVM_PGTABLE_WALK_IGNORE_EAGAIN);
>  }
>  
>  void kvm_pgtable_stage2_mkyoung(struct kvm_pgtable *pgt, u64 addr,
>  				enum kvm_pgtable_walk_flags flags)
>  {
>  	int ret;
>  
> @@ -1361,21 +1363,21 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,
>  	s8 level;
>  	int ret;
>  
>  	if (prot & KVM_PTE_LEAF_ATTR_HI_SW)
>  		return -EINVAL;
>  
>  	if (prot & KVM_PGTABLE_PROT_R)
>  		set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
>  
>  	if (prot & KVM_PGTABLE_PROT_W)
> -		set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> +		set |= KVM_PTE_LEAF_ATTR_HI_S2_DBM | KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
>  
>  	if (prot & KVM_PGTABLE_PROT_X) {
>  		ret = stage2_set_xn_attr(prot, &xn);
>  		if (ret)
>  			return ret;
>  
>  		set |= xn & KVM_PTE_LEAF_ATTR_HI_S2_XN;
>  		clr |= ~xn & KVM_PTE_LEAF_ATTR_HI_S2_XN;
>  	}
>  
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 17123f0b6dab..eb8dfffc32c7 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -379,21 +379,23 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
>  	}
>  
>  	addr_bottom += contiguous_bit_shift(desc, wi, level);
>  
>  	/* Calculate and return the result */
>  	paddr = (desc & GENMASK_ULL(47, addr_bottom)) |
>  		(ipa & GENMASK_ULL(addr_bottom - 1, 0));
>  	out->output = paddr;
>  	out->block_size = 1UL << ((3 - level) * stride + wi->pgshift);
>  	out->readable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
> -	out->writable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> +	/* Takes care of both RO/RW and RO/WC/WD encodings */
> +	out->writable = desc & (KVM_PTE_LEAF_ATTR_HI_S2_DBM |
> +				KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W);

Absolutely NOT. For a start, NV doesn't support FEAT_HAFDBS. But even
if it did, you are now actively corrupting memory by turning a RO
mapping with a spurious DBM bit set into a writable mapping.
VTCR_EL2.HD exists for a reason.

Do you see why your blanket approach of equating DBM with writable is
plain wrong?

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [RFC PATCH 2/5] KVM: arm64: Add KVM_PGTABLE_PROT_DIRTY

[Marc Zyngier]

On Tue, 01 Sep 2026 18:15:53 +0100,
Leonardo Bras <leo.bras@arm.com> wrote:
> 
> Second step of changing the encoding for the Stage2 PTE descriptor,
> introduce the concept of dirty page, so we can have a writable but not
> dirty (WC) page, and a writable and dirty (WD) page.

Why should we care about *setting* the dirty bit in the PTE? Under
what circumstance do we want to establish a mapping as being dirty?

The whole point of DBM is to only set something dirty when it is
written to, and this patch breaks this invariant.

Maybe you have a good reason to do so, but that's not explained.

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [RFC PATCH 0/5] KVM: arm64: New PTE dirty-page encoding, HAFDBS new usage

[Leonardo Bras]

On Sat, Sep 12, 2026 at 01:24:11PM +0100, Marc Zyngier wrote:
> On Tue, 01 Sep 2026 18:15:51 +0100,
> Leonardo Bras <leo.bras@arm.com> wrote:
> > 
> > This series have 2 main goals:
> > 
> > 1 - Patches #1,#2,#3 : Change the PTE descriptor to use WD/WC/RO encodings
> >     making use of the DBM bit, adapting all usages, and
> 
> What are WD and WC? I can sort of guess that this is write-dirty and
> write clean, but that's not exactly obvious. More importantly, you
> don't even explain *why* anything needs changing...
> 
> > 2 - Patches #4,#5 are an RFC on using HAFDBS on a guest to avoid resetting
> >     all PTEs to WC when dirty-logging starts, speeding-up startup.
> > 
> > (1) will also introduce a new walker for cleaning the dirty-bit, which will
> > clear the DBM bit if it's a block mapping (hugepage). This is needed as on
> > lazy-splitting we need to fault a write so we can do the lazy splitting.
> > This is needed for both the next patches, and for HDBSS & HACDBS
> > enablement.
> 
> Again, this is incredibly opaque to the reviewer. What is the problem
> you are trying to solve? This is what a cover letter is for.

Hi Marc, thanks for reviewing!

Okay, I will try to explain it better on the next version. What do you 
think of this text:

===================

This series have 2 main goals:
1 - Introduce a new PTE encoding (Patches #1, #2, #3)
2 - An idea to use HAFDBS on a guest to avoid resetting all PTEs to WC 
    when dirty-logging starts

Goal 1:

Patches #1, #2, #3: Before adding Stage-2 support to HAFDBS, HDBSS and 
HACDBS, we need to change the PTE descriptor encoding, as the DBM bit 
is required on mappings for those hardware engines to actually being 
able to update the PTEs. Currently what we have is:

- Read-Only  (RO): S2AP[1]=0
- Read-Write (RW): S2AP[1]=1 

and for them to work with the new features, we need to have: 

- Read-Only (RO):      DBM=0, S2AP[1]=0
- Writable-Clean (WC): DBM=1, S2AP[1]=0
- Writable-Dirty (WD): DBM=1, S2AP[1]=1

WC and WD are described in the Arm ARM, on R_XZFQH and R_BRFGY.

We also need to prepare for dealing with lazy splitting when HAFDBS is 
enabled: since it updates the PTE without taking a fault on guest write, it 
means we can't have lazy splitting if we mark all PTEs as WC. 
To address that, there is a suggestion to set, on dirty-track enable:

- All pages as WC, as they don't need splitting, and
- All blocks as RO, as they are required to fault to do lazy splitting

In order to have that, a new walker is introduced to have a different 
behavior depending on the entry's level.

This is needed for the Goal 2, as well as for HDBSS enablement.


Goal 2:

Patches #4,#5 are an RFC on using HAFDBS on a guest to avoid resetting
all PTEs to WC when dirty-logging starts, making it faster.

I really just want feedback to understand if it's worth pursuing. 

My main idea is that we can use HAFDBS _outside_ dirty-logging to only mark
dirty the pages that were actually written to. 

That is supposed to make it faster to transverse the pagetables when we 
need to clean the dirty-bit, as there is potentially less atomic writes to 
perform. 

The price paid for that is disabling HAFDBS on every vcpu before we can 
start cleaning the pages, during a dirty-track request.

Please let me know of what you think!

Thanks!
Leo

Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM

[Leonardo Bras]

On Sun, Sep 13, 2026 at 10:00:40AM +0100, Marc Zyngier wrote:
> On Tue, 01 Sep 2026 18:15:52 +0100,
> Leonardo Bras <leo.bras@arm.com> wrote:
> > 
> > As a first step of changing the encoding for the Stage2 PTE descriptor,
> > introduce the DBM bit, and adapt every usage of writable to use the DBM bit
> > (51) instead of S2AP[1]/Dirty bit (7).
> > 
> > For this step, we convert usages of RW(Dirty) -> WD(DBM|Dirty).
> > 
> > Signed-off-by: Leonardo Bras <leo.bras@arm.com>
> > ---
> >  arch/arm64/include/asm/kvm_pgtable.h | 3 +++
> >  arch/arm64/kvm/hyp/pgtable.c         | 8 +++++---
> >  arch/arm64/kvm/nested.c              | 4 +++-
> >  arch/arm64/kvm/ptdump.c              | 4 ++--
> >  4 files changed, 13 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> > index 41a8687938eb..37baa86d6fd8 100644
> > --- a/arch/arm64/include/asm/kvm_pgtable.h
> > +++ b/arch/arm64/include/asm/kvm_pgtable.h
> > @@ -86,24 +86,27 @@ typedef u64 kvm_pte_t;
> >  #define KVM_PTE_LEAF_ATTR_HI		GENMASK(63, 50)
> >  
> >  #define KVM_PTE_LEAF_ATTR_HI_SW		GENMASK(58, 55)
> >  
> >  #define KVM_PTE_LEAF_ATTR_HI_S1_XN	BIT(54)
> >  #define KVM_PTE_LEAF_ATTR_HI_S1_UXN	BIT(54)
> >  #define KVM_PTE_LEAF_ATTR_HI_S1_PXN	BIT(53)
> >  
> >  #define KVM_PTE_LEAF_ATTR_HI_S2_XN	GENMASK(54, 53)
> >  
> > +#define KVM_PTE_LEAF_ATTR_HI_S2_DBM	BIT(51)
> > +
> >  #define KVM_PTE_LEAF_ATTR_HI_S1_GP	BIT(50)
> >  
> >  #define KVM_PTE_LEAF_ATTR_S2_PERMS	(KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R | \
> >  					 KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W | \
> > +					 KVM_PTE_LEAF_ATTR_HI_S2_DBM | \
> >  					 KVM_PTE_LEAF_ATTR_HI_S2_XN)
> >  
> >  /* pKVM invalid pte encodings */
> >  #define KVM_INVALID_PTE_TYPE_MASK	GENMASK(63, 60)
> >  #define KVM_INVALID_PTE_ANNOT_MASK	~(KVM_PTE_VALID | \
> >  					  KVM_INVALID_PTE_TYPE_MASK)
> >  
> >  enum kvm_invalid_pte_type {
> >  	/*
> >  	 * Used to indicate a pte for which a 'break-before-make'
> > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> > index b74dd5ce1efd..ca49f1bd7c34 100644
> > --- a/arch/arm64/kvm/hyp/pgtable.c
> > +++ b/arch/arm64/kvm/hyp/pgtable.c
> > @@ -725,42 +725,43 @@ static int stage2_set_prot_attr(struct kvm_pgtable *pgt, enum kvm_pgtable_prot p
> >  	}
> >  
> >  	r = stage2_set_xn_attr(prot, &attr);
> >  	if (r)
> >  		return r;
> >  
> >  	if (prot & KVM_PGTABLE_PROT_R)
> >  		attr |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
> >  
> >  	if (prot & KVM_PGTABLE_PROT_W)
> > -		attr |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > +		attr |= KVM_PTE_LEAF_ATTR_HI_S2_DBM | KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> 
> What makes it acceptable to always set DBM? This is an optional
> feature, and I'm not exactly comfortable setting bits that are
> supposed to be RES0. 

The alternative would be to introduce WC/WD as an alternative encoding
infrastructure, and keep making decisions based on having HAFDBS available 
or in the system. My intention was to adapt things in a way to avoid 
multiple behaviors if we could avoid.

My rationale is, based on our possibility of setting DBM=1:
If HAFDBS/HDBSS/HACDBS are not available, or available but disabled, 
then the DBM bit is just ignored:
RO -> RO
WC -> RO
WD -> RW

(for reference:
- Read-Only (RO):      DBM=0, S2AP[1]=0
- Writable-Clean (WC): DBM=1, S2AP[1]=0
- Writable-Dirty (WD): DBM=1, S2AP[1]=1
)

And the behavior should just the same as what we have today.


> Yes, the HW should ignore it. But we have also
> seen quite a few broken designs in this area...
> 

I lack experience on what bad thing could happen. So I will expand on what 
I belive to understand up to here:

- The PTE is in memory, so the DBM bit can be set regardless of being RES0
- For SW pagetable walking, I don't think 'bit 51 == 0' is checked
- For HW pagetable walking, maybe some faulty implementation may rely on 
  bit51 being RES0, and fault otherwise.

If that's the case, then we would have to actually support both encodings, 
and only enable the new one if HAFDBS is available in the system. 

I just wonder how high are the chances to have such a broken design, 
or other broken designs did not come to my mind, and if we have to start 
with that multiple-encoding option.

> > +
> >
> 
> Spurious newline.
> 

Oh, ok, will get rid of it.

> >  	if (!kvm_lpa2_is_enabled())
> >  		attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S2_SH, sh);
> >  
> >  	attr |= KVM_PTE_LEAF_ATTR_LO_S2_AF;
> >  	attr |= prot & KVM_PTE_LEAF_ATTR_HI_SW;
> >  	*ptep = attr;
> >  
> >  	return 0;
> >  }
> 
> I don't know how you have configured git on your end, but there is *a
> lot* of context...
> 

diff.context=10, i.e. 10 lines before and after a change.
To me it looks helpful to get more context around the change. 

I can change it back to 5 if it bothers you.

> >  
> >  enum kvm_pgtable_prot kvm_pgtable_stage2_pte_prot(kvm_pte_t pte)
> >  {
> >  	enum kvm_pgtable_prot prot = pte & KVM_PTE_LEAF_ATTR_HI_SW;
> >  
> >  	if (!kvm_pte_valid(pte))
> >  		return prot;
> >  
> >  	if (pte & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R)
> >  		prot |= KVM_PGTABLE_PROT_R;
> > -	if (pte & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W)
> > +	if (pte & KVM_PTE_LEAF_ATTR_HI_S2_DBM)
> >  		prot |= KVM_PGTABLE_PROT_W;
> >  
> >  	switch (FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, pte)) {
> >  	case 0b00:
> >  		prot |= KVM_PGTABLE_PROT_PX | KVM_PGTABLE_PROT_UX;
> >  		break;
> >  	case 0b01:
> >  		prot |= KVM_PGTABLE_PROT_UX;
> >  		break;
> >  	case 0b11:
> > @@ -1281,20 +1282,21 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,
> >  		*orig_pte = data.pte;
> >  
> >  	if (level)
> >  		*level = data.level;
> >  	return 0;
> >  }
> >  
> >  int kvm_pgtable_stage2_wrprotect(struct kvm_pgtable *pgt, u64 addr, u64 size)
> >  {
> >  	return stage2_update_leaf_attrs(pgt, addr, size, 0,
> > +					KVM_PTE_LEAF_ATTR_HI_S2_DBM |
> >  					KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W,
> >  					NULL, NULL,
> >  					KVM_PGTABLE_WALK_IGNORE_EAGAIN);
> >  }
> >  
> >  void kvm_pgtable_stage2_mkyoung(struct kvm_pgtable *pgt, u64 addr,
> >  				enum kvm_pgtable_walk_flags flags)
> >  {
> >  	int ret;
> >  
> > @@ -1361,21 +1363,21 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,
> >  	s8 level;
> >  	int ret;
> >  
> >  	if (prot & KVM_PTE_LEAF_ATTR_HI_SW)
> >  		return -EINVAL;
> >  
> >  	if (prot & KVM_PGTABLE_PROT_R)
> >  		set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
> >  
> >  	if (prot & KVM_PGTABLE_PROT_W)
> > -		set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > +		set |= KVM_PTE_LEAF_ATTR_HI_S2_DBM | KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> >  
> >  	if (prot & KVM_PGTABLE_PROT_X) {
> >  		ret = stage2_set_xn_attr(prot, &xn);
> >  		if (ret)
> >  			return ret;
> >  
> >  		set |= xn & KVM_PTE_LEAF_ATTR_HI_S2_XN;
> >  		clr |= ~xn & KVM_PTE_LEAF_ATTR_HI_S2_XN;
> >  	}
> >  
> > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> > index 17123f0b6dab..eb8dfffc32c7 100644
> > --- a/arch/arm64/kvm/nested.c
> > +++ b/arch/arm64/kvm/nested.c
> > @@ -379,21 +379,23 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
> >  	}
> >  
> >  	addr_bottom += contiguous_bit_shift(desc, wi, level);
> >  
> >  	/* Calculate and return the result */
> >  	paddr = (desc & GENMASK_ULL(47, addr_bottom)) |
> >  		(ipa & GENMASK_ULL(addr_bottom - 1, 0));
> >  	out->output = paddr;
> >  	out->block_size = 1UL << ((3 - level) * stride + wi->pgshift);
> >  	out->readable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
> > -	out->writable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > +	/* Takes care of both RO/RW and RO/WC/WD encodings */
> > +	out->writable = desc & (KVM_PTE_LEAF_ATTR_HI_S2_DBM |
> > +				KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W);
> 
> Absolutely NOT. For a start, NV doesn't support FEAT_HAFDBS. But even
> if it did, you are now actively corrupting memory by turning a RO
> mapping with a spurious DBM bit set into a writable mapping.
> VTCR_EL2.HD exists for a reason.
> 
> Do you see why your blanket approach of equating DBM with writable is
> plain wrong?

Sorry, not really... please help me understand it.

When you say a spurious DBM bit, what does it mean?

All I can think is something like a buggy HW implementation setting DBM in 
a HAFDBS-disabled machine, which has been unnoticed up to now, and now 
could cause problems. 

If that's what you mean, then yes, that would be a problem. But that would 
also violate the architecture, and become a errata, right?

On DBM=writable, that's basically what the Arm ARM says now:

R_LRMSH
For the purpose of FEAT_HAFDBS dirty state management, a Block descriptor 
or Page descriptor can be described as having one of the following states:
    Non-writable.
    Writable-clean.
    Writable-dirty.

R_XSTDV

    If a Block descriptor or Page descriptor is not writable-clean and not 
    writable-dirty, then it is described as non-writable.

And since both RBRFGY and RXZFQH define Writable-Clean and Writable-Dirty 
as seting DBM=1, Writable <=> DBM=1


I am _not_ trying to say I am right here, as I really value your experience 
over whatever I could understand of the documentation, I am honestly 
confused right now on what I have done wrong.

Thanks for your patience, 
Leo

Re: [RFC PATCH 2/5] KVM: arm64: Add KVM_PGTABLE_PROT_DIRTY

[Leonardo Bras]

On Sun, Sep 13, 2026 at 10:09:25AM +0100, Marc Zyngier wrote:
> On Tue, 01 Sep 2026 18:15:53 +0100,
> Leonardo Bras <leo.bras@arm.com> wrote:
> > 
> > Second step of changing the encoding for the Stage2 PTE descriptor,
> > introduce the concept of dirty page, so we can have a writable but not
> > dirty (WC) page, and a writable and dirty (WD) page.
> 
> Why should we care about *setting* the dirty bit in the PTE? Under
> what circumstance do we want to establish a mapping as being dirty?

We want a mapping to be dirty whenever it's writable and we don't want to 
track it being changed anymore.

A writable-dirty is for when a mapping can be written to, but still did not 
happen.

> 
> The whole point of DBM is to only set something dirty when it is
> written to, and this patch breaks this invariant.
> 
> Maybe you have a good reason to do so, but that's not explained.

Sorry it was not clear.
The idea of this patch is to introduce the dirty state, without causing 
any change in the behavior of the system.

Before patchset: 
- RW : S2AP = 1
- RO : S2AP = 0

After patch 1:
- RW = WD: S2AP = 1, DBM = 1
- RO     : S2AP = 0, DBM = 0

After patch 2:
- WD : S2AP = 1, DBM = 1
- WC : S2AP = 0, DBM = 1
- RO : S2AP = 0, DBM = 0

That splits the concept of writable and dirty from the previous RW state, 
so they can be independent.

We can mark a page writable, without it being dirty, which allows HAFDBS in 
the future to mark it dirty whenever it happens to receive a write. (and 
use HDBSS to register it on a buffer, and so on)

As of now there is no enablement of the HAFDBS, so up to this patch 
there should not be any impact to users, as the DBM bit is ignored if 
VTCR.HD=0.

Patch 5 introduces an possible use of this using HAFDBS when dirty-tracking 
is disabled to avoid marking all PTEs as clean at the dirty-track enable. 

Does it look more clear now?
Do you think adding parts of the above text in the commit message would 
help?

Thanks again!
Leo

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

[Oliver Upton]

Hi,

On Tue, Sep 01, 2026 at 06:15:56PM +0100, Leonardo Bras wrote:
> When dirty-logging is disabled, even non-write faults make a page dirty,
> which avoids a second fault when the page is actually written to.
> 
> On dirty-logging enable, this approach causes all (writable) pages on the
> memslot to be marked clean, even if they were not written to, which can
> take a lot of time, while holding the MMU lock, doing atomic writes to
> PTEs.

Do you have any performance numbers for this? Enabling HAFDBS seems a
bit involved to avoid some stores on the first pass.

> So, if the system supports VHE + HAFDBS, keep the non-write-faulted page as
> writable-clean, and let HAFDBS update that on demand when a write happens.
> 
> When dirty-tracking actually starts, disable HAFDBS as having it on
> avoids the same fault that is used for dirty-logging.
> 
> Signed-off-by: Leonardo Bras <leo.bras@arm.com>
> ---
>  arch/arm64/include/asm/kvm_mmu.h    |  6 ++++++
>  arch/arm64/include/asm/kvm_nested.h |  9 +++++++--
>  arch/arm64/kvm/arm.c                |  7 +++++++
>  arch/arm64/kvm/mmu.c                | 26 +++++++++++++++++++++++++-
>  4 files changed, 45 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> index 6eae7e7e2a68..3defa1a988d3 100644
> --- a/arch/arm64/include/asm/kvm_mmu.h
> +++ b/arch/arm64/include/asm/kvm_mmu.h
> @@ -383,20 +383,26 @@ static inline void kvm_fault_unlock(struct kvm *kvm)
>   * and CMOs are NOP'd. This has the effect of no longer requiring a
>   * KVA for addresses mapped into the S2. The presence of these features
>   * are thus necessary to support cacheable S2 mapping of VM_PFNMAP.
>   */
>  static inline bool kvm_supports_cacheable_pfnmap(void)
>  {
>  	return cpus_have_final_cap(ARM64_HAS_STAGE2_FWB) &&
>  	       cpus_have_final_cap(ARM64_HAS_CACHE_DIC);
>  }
>  
> +static inline bool kvm_supports_hafdbs(struct kvm *kvm)
> +{
> +	return IS_ENABLED(CONFIG_ARM64_HW_AFDBM) &&

cpucap_is_possible() is the right place to park this.

> 						      has_vhe() &&

I don't see a reason why this needs to be constrained to VHE-only.

> +		!kvm_vcpu_has_nv(kvm) && cpus_have_final_cap(ARM64_HW_DBM);

Same thing goes for nested... KVM can make use of HAFDBS in the
canonical stage-2 MMU (or even a shadow stage-2) independent of the
guest hypervisor.

>  #ifdef CONFIG_PTDUMP_STAGE2_DEBUGFS
>  void kvm_s2_ptdump_create_debugfs(struct kvm *kvm);
>  void kvm_nested_s2_ptdump_create_debugfs(struct kvm_s2_mmu *mmu);
>  void kvm_nested_s2_ptdump_remove_debugfs(struct kvm_s2_mmu *mmu);
>  #else
>  static inline void kvm_s2_ptdump_create_debugfs(struct kvm *kvm) {}
>  static inline void kvm_nested_s2_ptdump_create_debugfs(struct kvm_s2_mmu *mmu) {}
>  static inline void kvm_nested_s2_ptdump_remove_debugfs(struct kvm_s2_mmu *mmu) {}
>  #endif /* CONFIG_PTDUMP_STAGE2_DEBUGFS */
>  
> diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
> index 1ed708335809..9242b5d665af 100644
> --- a/arch/arm64/include/asm/kvm_nested.h
> +++ b/arch/arm64/include/asm/kvm_nested.h
> @@ -1,24 +1,29 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
>  #ifndef __ARM64_KVM_NESTED_H
>  #define __ARM64_KVM_NESTED_H
>  
>  #include <linux/bitfield.h>
>  #include <linux/kvm_host.h>
>  #include <asm/kvm_emulate.h>
>  #include <asm/kvm_pgtable.h>
>  
> -static inline bool vcpu_has_nv(const struct kvm_vcpu *vcpu)
> +static inline bool kvm_vcpu_has_nv(const struct kvm *kvm)

The name would suggest this thing takes a vcpu pointer...

Thanks,
Oliver

Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM

[Oliver Upton]

On Tue, Sep 15, 2026 at 06:12:45PM +0100, Leonardo Bras wrote:
> > Yes, the HW should ignore it. But we have also
> > seen quite a few broken designs in this area...
> > 
> 
> I lack experience on what bad thing could happen. So I will expand on what 
> I belive to understand up to here:
> 
> - The PTE is in memory, so the DBM bit can be set regardless of being RES0
> - For SW pagetable walking, I don't think 'bit 51 == 0' is checked
> - For HW pagetable walking, maybe some faulty implementation may rely on 
>   bit51 being RES0, and fault otherwise.
> 
> If that's the case, then we would have to actually support both encodings, 
> and only enable the new one if HAFDBS is available in the system. 
> 
> I just wonder how high are the chances to have such a broken design, 
> or other broken designs did not come to my mind, and if we have to start 
> with that multiple-encoding option.

FWIW, the host stage-1 already uses the DBM bit unconditionally,
treating it as a software bit on implementations without HAFDBS.
Although given the quality of any garden variety Arm MMU I understand
where Marc is coming from.

I don't think the HAFDBS enablement is complicated enough to be done in
a separate series without any meaningful users, nor would I really be
interested in taking it without, say, HDBSS.

Can you please work with Tian to get a combined series out for this?

> > > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> > > index 17123f0b6dab..eb8dfffc32c7 100644
> > > --- a/arch/arm64/kvm/nested.c
> > > +++ b/arch/arm64/kvm/nested.c
> > > @@ -379,21 +379,23 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
> > >  	}
> > >  
> > >  	addr_bottom += contiguous_bit_shift(desc, wi, level);
> > >  
> > >  	/* Calculate and return the result */
> > >  	paddr = (desc & GENMASK_ULL(47, addr_bottom)) |
> > >  		(ipa & GENMASK_ULL(addr_bottom - 1, 0));
> > >  	out->output = paddr;
> > >  	out->block_size = 1UL << ((3 - level) * stride + wi->pgshift);
> > >  	out->readable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
> > > -	out->writable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > > +	/* Takes care of both RO/RW and RO/WC/WD encodings */
> > > +	out->writable = desc & (KVM_PTE_LEAF_ATTR_HI_S2_DBM |
> > > +				KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W);
> > 
> > Absolutely NOT. For a start, NV doesn't support FEAT_HAFDBS. But even
> > if it did, you are now actively corrupting memory by turning a RO
> > mapping with a spurious DBM bit set into a writable mapping.
> > VTCR_EL2.HD exists for a reason.
> > 
> > Do you see why your blanket approach of equating DBM with writable is
> > plain wrong?
> 
> Sorry, not really... please help me understand it.
> 
> When you say a spurious DBM bit, what does it mean?

You've implemented the exact sort of bug that was alluded to above. In
this case it's a software page table walker consuming DBM regardless of
the value of VTCR_EL2.HD.

If the guest hypervisor sets VTCR_EL2.HD=0, the expectation is that the
shadow stage-2 MMU treats the corresponding bit in the PTE as RES0.

Thanks,
Oliver

Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM

[Marc Zyngier]

On Tue, 15 Sep 2026 18:12:45 +0100,
Leonardo Bras <leo.bras@arm.com> wrote:
> 
> On Sun, Sep 13, 2026 at 10:00:40AM +0100, Marc Zyngier wrote:
> > On Tue, 01 Sep 2026 18:15:52 +0100,
> > Leonardo Bras <leo.bras@arm.com> wrote:
> > > 
> > > As a first step of changing the encoding for the Stage2 PTE descriptor,
> > > introduce the DBM bit, and adapt every usage of writable to use the DBM bit
> > > (51) instead of S2AP[1]/Dirty bit (7).
> > > 
> > > For this step, we convert usages of RW(Dirty) -> WD(DBM|Dirty).
> > > 
> > > Signed-off-by: Leonardo Bras <leo.bras@arm.com>
> > > ---
> > >  arch/arm64/include/asm/kvm_pgtable.h | 3 +++
> > >  arch/arm64/kvm/hyp/pgtable.c         | 8 +++++---
> > >  arch/arm64/kvm/nested.c              | 4 +++-
> > >  arch/arm64/kvm/ptdump.c              | 4 ++--
> > >  4 files changed, 13 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> > > index 41a8687938eb..37baa86d6fd8 100644
> > > --- a/arch/arm64/include/asm/kvm_pgtable.h
> > > +++ b/arch/arm64/include/asm/kvm_pgtable.h
> > > @@ -86,24 +86,27 @@ typedef u64 kvm_pte_t;
> > >  #define KVM_PTE_LEAF_ATTR_HI		GENMASK(63, 50)
> > >  
> > >  #define KVM_PTE_LEAF_ATTR_HI_SW		GENMASK(58, 55)
> > >  
> > >  #define KVM_PTE_LEAF_ATTR_HI_S1_XN	BIT(54)
> > >  #define KVM_PTE_LEAF_ATTR_HI_S1_UXN	BIT(54)
> > >  #define KVM_PTE_LEAF_ATTR_HI_S1_PXN	BIT(53)
> > >  
> > >  #define KVM_PTE_LEAF_ATTR_HI_S2_XN	GENMASK(54, 53)
> > >  
> > > +#define KVM_PTE_LEAF_ATTR_HI_S2_DBM	BIT(51)
> > > +
> > >  #define KVM_PTE_LEAF_ATTR_HI_S1_GP	BIT(50)
> > >  
> > >  #define KVM_PTE_LEAF_ATTR_S2_PERMS	(KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R | \
> > >  					 KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W | \
> > > +					 KVM_PTE_LEAF_ATTR_HI_S2_DBM | \
> > >  					 KVM_PTE_LEAF_ATTR_HI_S2_XN)
> > >  
> > >  /* pKVM invalid pte encodings */
> > >  #define KVM_INVALID_PTE_TYPE_MASK	GENMASK(63, 60)
> > >  #define KVM_INVALID_PTE_ANNOT_MASK	~(KVM_PTE_VALID | \
> > >  					  KVM_INVALID_PTE_TYPE_MASK)
> > >  
> > >  enum kvm_invalid_pte_type {
> > >  	/*
> > >  	 * Used to indicate a pte for which a 'break-before-make'
> > > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> > > index b74dd5ce1efd..ca49f1bd7c34 100644
> > > --- a/arch/arm64/kvm/hyp/pgtable.c
> > > +++ b/arch/arm64/kvm/hyp/pgtable.c
> > > @@ -725,42 +725,43 @@ static int stage2_set_prot_attr(struct kvm_pgtable *pgt, enum kvm_pgtable_prot p
> > >  	}
> > >  
> > >  	r = stage2_set_xn_attr(prot, &attr);
> > >  	if (r)
> > >  		return r;
> > >  
> > >  	if (prot & KVM_PGTABLE_PROT_R)
> > >  		attr |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
> > >  
> > >  	if (prot & KVM_PGTABLE_PROT_W)
> > > -		attr |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > > +		attr |= KVM_PTE_LEAF_ATTR_HI_S2_DBM | KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > 
> > What makes it acceptable to always set DBM? This is an optional
> > feature, and I'm not exactly comfortable setting bits that are
> > supposed to be RES0. 
> 
> The alternative would be to introduce WC/WD as an alternative encoding
> infrastructure, and keep making decisions based on having HAFDBS available 
> or in the system. My intention was to adapt things in a way to avoid 
> multiple behaviors if we could avoid.
> 
> My rationale is, based on our possibility of setting DBM=1:
> If HAFDBS/HDBSS/HACDBS are not available, or available but disabled, 
> then the DBM bit is just ignored:
> RO -> RO
> WC -> RO
> WD -> RW
> 
> (for reference:
> - Read-Only (RO):      DBM=0, S2AP[1]=0
> - Writable-Clean (WC): DBM=1, S2AP[1]=0
> - Writable-Dirty (WD): DBM=1, S2AP[1]=1
> )
> 
> And the behavior should just the same as what we have today.
> 
> 
> > Yes, the HW should ignore it. But we have also
> > seen quite a few broken designs in this area...
> > 
> 
> I lack experience on what bad thing could happen. So I will expand on what 
> I belive to understand up to here:
> 
> - The PTE is in memory, so the DBM bit can be set regardless of being RES0
> - For SW pagetable walking, I don't think 'bit 51 == 0' is checked
> - For HW pagetable walking, maybe some faulty implementation may rely on 
>   bit51 being RES0, and fault otherwise.
> 
> If that's the case, then we would have to actually support both encodings, 
> and only enable the new one if HAFDBS is available in the system. 
> 
> I just wonder how high are the chances to have such a broken design, 
> or other broken designs did not come to my mind, and if we have to start 
> with that multiple-encoding option.
> 
> > > +
> > >
> > 
> > Spurious newline.
> > 
> 
> Oh, ok, will get rid of it.
> 
> > >  	if (!kvm_lpa2_is_enabled())
> > >  		attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S2_SH, sh);
> > >  
> > >  	attr |= KVM_PTE_LEAF_ATTR_LO_S2_AF;
> > >  	attr |= prot & KVM_PTE_LEAF_ATTR_HI_SW;
> > >  	*ptep = attr;
> > >  
> > >  	return 0;
> > >  }
> > 
> > I don't know how you have configured git on your end, but there is *a
> > lot* of context...
> > 
> 
> diff.context=10, i.e. 10 lines before and after a change.
> To me it looks helpful to get more context around the change. 
> 
> I can change it back to 5 if it bothers you.
> 
> > >  
> > >  enum kvm_pgtable_prot kvm_pgtable_stage2_pte_prot(kvm_pte_t pte)
> > >  {
> > >  	enum kvm_pgtable_prot prot = pte & KVM_PTE_LEAF_ATTR_HI_SW;
> > >  
> > >  	if (!kvm_pte_valid(pte))
> > >  		return prot;
> > >  
> > >  	if (pte & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R)
> > >  		prot |= KVM_PGTABLE_PROT_R;
> > > -	if (pte & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W)
> > > +	if (pte & KVM_PTE_LEAF_ATTR_HI_S2_DBM)
> > >  		prot |= KVM_PGTABLE_PROT_W;
> > >  
> > >  	switch (FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, pte)) {
> > >  	case 0b00:
> > >  		prot |= KVM_PGTABLE_PROT_PX | KVM_PGTABLE_PROT_UX;
> > >  		break;
> > >  	case 0b01:
> > >  		prot |= KVM_PGTABLE_PROT_UX;
> > >  		break;
> > >  	case 0b11:
> > > @@ -1281,20 +1282,21 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,
> > >  		*orig_pte = data.pte;
> > >  
> > >  	if (level)
> > >  		*level = data.level;
> > >  	return 0;
> > >  }
> > >  
> > >  int kvm_pgtable_stage2_wrprotect(struct kvm_pgtable *pgt, u64 addr, u64 size)
> > >  {
> > >  	return stage2_update_leaf_attrs(pgt, addr, size, 0,
> > > +					KVM_PTE_LEAF_ATTR_HI_S2_DBM |
> > >  					KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W,
> > >  					NULL, NULL,
> > >  					KVM_PGTABLE_WALK_IGNORE_EAGAIN);
> > >  }
> > >  
> > >  void kvm_pgtable_stage2_mkyoung(struct kvm_pgtable *pgt, u64 addr,
> > >  				enum kvm_pgtable_walk_flags flags)
> > >  {
> > >  	int ret;
> > >  
> > > @@ -1361,21 +1363,21 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,
> > >  	s8 level;
> > >  	int ret;
> > >  
> > >  	if (prot & KVM_PTE_LEAF_ATTR_HI_SW)
> > >  		return -EINVAL;
> > >  
> > >  	if (prot & KVM_PGTABLE_PROT_R)
> > >  		set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
> > >  
> > >  	if (prot & KVM_PGTABLE_PROT_W)
> > > -		set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > > +		set |= KVM_PTE_LEAF_ATTR_HI_S2_DBM | KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > >  
> > >  	if (prot & KVM_PGTABLE_PROT_X) {
> > >  		ret = stage2_set_xn_attr(prot, &xn);
> > >  		if (ret)
> > >  			return ret;
> > >  
> > >  		set |= xn & KVM_PTE_LEAF_ATTR_HI_S2_XN;
> > >  		clr |= ~xn & KVM_PTE_LEAF_ATTR_HI_S2_XN;
> > >  	}
> > >  
> > > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> > > index 17123f0b6dab..eb8dfffc32c7 100644
> > > --- a/arch/arm64/kvm/nested.c
> > > +++ b/arch/arm64/kvm/nested.c
> > > @@ -379,21 +379,23 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
> > >  	}
> > >  
> > >  	addr_bottom += contiguous_bit_shift(desc, wi, level);
> > >  
> > >  	/* Calculate and return the result */
> > >  	paddr = (desc & GENMASK_ULL(47, addr_bottom)) |
> > >  		(ipa & GENMASK_ULL(addr_bottom - 1, 0));
> > >  	out->output = paddr;
> > >  	out->block_size = 1UL << ((3 - level) * stride + wi->pgshift);
> > >  	out->readable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
> > > -	out->writable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > > +	/* Takes care of both RO/RW and RO/WC/WD encodings */
> > > +	out->writable = desc & (KVM_PTE_LEAF_ATTR_HI_S2_DBM |
> > > +				KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W);
> > 
> > Absolutely NOT. For a start, NV doesn't support FEAT_HAFDBS. But even
> > if it did, you are now actively corrupting memory by turning a RO
> > mapping with a spurious DBM bit set into a writable mapping.
> > VTCR_EL2.HD exists for a reason.
> > 
> > Do you see why your blanket approach of equating DBM with writable is
> > plain wrong?
> 
> Sorry, not really... please help me understand it.
> 
> When you say a spurious DBM bit, what does it mean?
> 
> All I can think is something like a buggy HW implementation setting DBM in 
> a HAFDBS-disabled machine, which has been unnoticed up to now, and now 
> could cause problems.

And pray tell, what is it to have HAFDBS being disabled at S2? Instead
of throwing the book at me, you should probably read it, and specially
the effects of VTCR_EL2.HD.

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM

[Leonardo Bras]

On Tue, Sep 15, 2026 at 05:37:15PM -0700, Oliver Upton wrote:
> On Tue, Sep 15, 2026 at 06:12:45PM +0100, Leonardo Bras wrote:
> > > Yes, the HW should ignore it. But we have also
> > > seen quite a few broken designs in this area...
> > > 
> > 
> > I lack experience on what bad thing could happen. So I will expand on what 
> > I belive to understand up to here:
> > 
> > - The PTE is in memory, so the DBM bit can be set regardless of being RES0
> > - For SW pagetable walking, I don't think 'bit 51 == 0' is checked
> > - For HW pagetable walking, maybe some faulty implementation may rely on 
> >   bit51 being RES0, and fault otherwise.
> > 
> > If that's the case, then we would have to actually support both encodings, 
> > and only enable the new one if HAFDBS is available in the system. 
> > 
> > I just wonder how high are the chances to have such a broken design, 
> > or other broken designs did not come to my mind, and if we have to start 
> > with that multiple-encoding option.
> 
> FWIW, the host stage-1 already uses the DBM bit unconditionally,
> treating it as a software bit on implementations without HAFDBS.
> Although given the quality of any garden variety Arm MMU I understand
> where Marc is coming from.
> 
> I don't think the HAFDBS enablement is complicated enough to be done in
> a separate series without any meaningful users, nor would I really be
> interested in taking it without, say, HDBSS.
> 
> Can you please work with Tian to get a combined series out for this?
> 

Hi Oliver, thanks for reviewing!

Sure, one of the reasons I sent like this is so Tian could use it as a base 
for his next version.


> > > > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> > > > index 17123f0b6dab..eb8dfffc32c7 100644
> > > > --- a/arch/arm64/kvm/nested.c
> > > > +++ b/arch/arm64/kvm/nested.c
> > > > @@ -379,21 +379,23 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
> > > >  	}
> > > >  
> > > >  	addr_bottom += contiguous_bit_shift(desc, wi, level);
> > > >  
> > > >  	/* Calculate and return the result */
> > > >  	paddr = (desc & GENMASK_ULL(47, addr_bottom)) |
> > > >  		(ipa & GENMASK_ULL(addr_bottom - 1, 0));
> > > >  	out->output = paddr;
> > > >  	out->block_size = 1UL << ((3 - level) * stride + wi->pgshift);
> > > >  	out->readable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
> > > > -	out->writable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > > > +	/* Takes care of both RO/RW and RO/WC/WD encodings */
> > > > +	out->writable = desc & (KVM_PTE_LEAF_ATTR_HI_S2_DBM |
> > > > +				KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W);
> > > 
> > > Absolutely NOT. For a start, NV doesn't support FEAT_HAFDBS. But even
> > > if it did, you are now actively corrupting memory by turning a RO
> > > mapping with a spurious DBM bit set into a writable mapping.
> > > VTCR_EL2.HD exists for a reason.
> > > 
> > > Do you see why your blanket approach of equating DBM with writable is
> > > plain wrong?
> > 
> > Sorry, not really... please help me understand it.
> > 
> > When you say a spurious DBM bit, what does it mean?
> 
> You've implemented the exact sort of bug that was alluded to above. In
> this case it's a software page table walker consuming DBM regardless of
> the value of VTCR_EL2.HD.
> 

So you mean that DBM being treated as "writable" could _only_ happen if we 
have VTCR_EL2.HD=1? I was previously under the impression that it could be 
used regardless of HD value.

> If the guest hypervisor sets VTCR_EL2.HD=0, the expectation is that the
> shadow stage-2 MMU treats the corresponding bit in the PTE as RES0.
> 

Okay, I think I can see it now: since the guest hypervisor could use RO/RW, 
and has no decoupled concept of dirty and writable, it could think all 
writable pages are dirty when we run above function.

So.. would it make sense to have an out->dirty, which we would check based 
on S2AP while out->writable is compared against DBM, and we change the 
logic that uses out->writable to properly match it, maybe based on the 
guest having the feature enabled?

Thanks for your patience on explaining this!
Leo

Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM

[Marc Zyngier]

On Wed, 16 Sep 2026 12:22:05 +0100,
Leonardo Bras <leo.bras@arm.com> wrote:
> 
> On Tue, Sep 15, 2026 at 05:37:15PM -0700, Oliver Upton wrote:
> > On Tue, Sep 15, 2026 at 06:12:45PM +0100, Leonardo Bras wrote:
> > > > Yes, the HW should ignore it. But we have also
> > > > seen quite a few broken designs in this area...
> > > > 
> > > 
> > > I lack experience on what bad thing could happen. So I will expand on what 
> > > I belive to understand up to here:
> > > 
> > > - The PTE is in memory, so the DBM bit can be set regardless of being RES0
> > > - For SW pagetable walking, I don't think 'bit 51 == 0' is checked
> > > - For HW pagetable walking, maybe some faulty implementation may rely on 
> > >   bit51 being RES0, and fault otherwise.
> > > 
> > > If that's the case, then we would have to actually support both encodings, 
> > > and only enable the new one if HAFDBS is available in the system. 
> > > 
> > > I just wonder how high are the chances to have such a broken design, 
> > > or other broken designs did not come to my mind, and if we have to start 
> > > with that multiple-encoding option.
> > 
> > FWIW, the host stage-1 already uses the DBM bit unconditionally,
> > treating it as a software bit on implementations without HAFDBS.
> > Although given the quality of any garden variety Arm MMU I understand
> > where Marc is coming from.
> > 
> > I don't think the HAFDBS enablement is complicated enough to be done in
> > a separate series without any meaningful users, nor would I really be
> > interested in taking it without, say, HDBSS.
> > 
> > Can you please work with Tian to get a combined series out for this?
> > 
> 
> Hi Oliver, thanks for reviewing!
> 
> Sure, one of the reasons I sent like this is so Tian could use it as a base 
> for his next version.
> 
> 
> > > > > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> > > > > index 17123f0b6dab..eb8dfffc32c7 100644
> > > > > --- a/arch/arm64/kvm/nested.c
> > > > > +++ b/arch/arm64/kvm/nested.c
> > > > > @@ -379,21 +379,23 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
> > > > >  	}
> > > > >  
> > > > >  	addr_bottom += contiguous_bit_shift(desc, wi, level);
> > > > >  
> > > > >  	/* Calculate and return the result */
> > > > >  	paddr = (desc & GENMASK_ULL(47, addr_bottom)) |
> > > > >  		(ipa & GENMASK_ULL(addr_bottom - 1, 0));
> > > > >  	out->output = paddr;
> > > > >  	out->block_size = 1UL << ((3 - level) * stride + wi->pgshift);
> > > > >  	out->readable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
> > > > > -	out->writable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > > > > +	/* Takes care of both RO/RW and RO/WC/WD encodings */
> > > > > +	out->writable = desc & (KVM_PTE_LEAF_ATTR_HI_S2_DBM |
> > > > > +				KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W);
> > > > 
> > > > Absolutely NOT. For a start, NV doesn't support FEAT_HAFDBS. But even
> > > > if it did, you are now actively corrupting memory by turning a RO
> > > > mapping with a spurious DBM bit set into a writable mapping.
> > > > VTCR_EL2.HD exists for a reason.
> > > > 
> > > > Do you see why your blanket approach of equating DBM with writable is
> > > > plain wrong?
> > > 
> > > Sorry, not really... please help me understand it.
> > > 
> > > When you say a spurious DBM bit, what does it mean?
> > 
> > You've implemented the exact sort of bug that was alluded to above. In
> > this case it's a software page table walker consuming DBM regardless of
> > the value of VTCR_EL2.HD.
> > 
> 
> So you mean that DBM being treated as "writable" could _only_ happen if we 
> have VTCR_EL2.HD=1? I was previously under the impression that it could be 
> used regardless of HD value.

Then you have failed to understand the architecture. When
VTCR_EL2.HD==0, DBM can be treated as RES0, RES0, or RES0.

R_XZFQH and R_BRFGY are pretty clear that DBM can only be evaluated
when dirty hw update is enabled, and I_MHJZP tells you what it means
for S2 to have this enabled.

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM

[Leonardo Bras]

On Wed, Sep 16, 2026 at 09:30:35AM +0100, Marc Zyngier wrote:
> On Tue, 15 Sep 2026 18:12:45 +0100,
> Leonardo Bras <leo.bras@arm.com> wrote:
> > 
> > On Sun, Sep 13, 2026 at 10:00:40AM +0100, Marc Zyngier wrote:
> > > On Tue, 01 Sep 2026 18:15:52 +0100,
> > > Leonardo Bras <leo.bras@arm.com> wrote:
> > > > 
> > > > As a first step of changing the encoding for the Stage2 PTE descriptor,
> > > > introduce the DBM bit, and adapt every usage of writable to use the DBM bit
> > > > (51) instead of S2AP[1]/Dirty bit (7).
> > > > 
> > > > For this step, we convert usages of RW(Dirty) -> WD(DBM|Dirty).
> > > > 
> > > > Signed-off-by: Leonardo Bras <leo.bras@arm.com>
> > > > ---
> > > >  arch/arm64/include/asm/kvm_pgtable.h | 3 +++
> > > >  arch/arm64/kvm/hyp/pgtable.c         | 8 +++++---
> > > >  arch/arm64/kvm/nested.c              | 4 +++-
> > > >  arch/arm64/kvm/ptdump.c              | 4 ++--
> > > >  4 files changed, 13 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
> > > > index 41a8687938eb..37baa86d6fd8 100644
> > > > --- a/arch/arm64/include/asm/kvm_pgtable.h
> > > > +++ b/arch/arm64/include/asm/kvm_pgtable.h
> > > > @@ -86,24 +86,27 @@ typedef u64 kvm_pte_t;
> > > >  #define KVM_PTE_LEAF_ATTR_HI		GENMASK(63, 50)
> > > >  
> > > >  #define KVM_PTE_LEAF_ATTR_HI_SW		GENMASK(58, 55)
> > > >  
> > > >  #define KVM_PTE_LEAF_ATTR_HI_S1_XN	BIT(54)
> > > >  #define KVM_PTE_LEAF_ATTR_HI_S1_UXN	BIT(54)
> > > >  #define KVM_PTE_LEAF_ATTR_HI_S1_PXN	BIT(53)
> > > >  
> > > >  #define KVM_PTE_LEAF_ATTR_HI_S2_XN	GENMASK(54, 53)
> > > >  
> > > > +#define KVM_PTE_LEAF_ATTR_HI_S2_DBM	BIT(51)
> > > > +
> > > >  #define KVM_PTE_LEAF_ATTR_HI_S1_GP	BIT(50)
> > > >  
> > > >  #define KVM_PTE_LEAF_ATTR_S2_PERMS	(KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R | \
> > > >  					 KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W | \
> > > > +					 KVM_PTE_LEAF_ATTR_HI_S2_DBM | \
> > > >  					 KVM_PTE_LEAF_ATTR_HI_S2_XN)
> > > >  
> > > >  /* pKVM invalid pte encodings */
> > > >  #define KVM_INVALID_PTE_TYPE_MASK	GENMASK(63, 60)
> > > >  #define KVM_INVALID_PTE_ANNOT_MASK	~(KVM_PTE_VALID | \
> > > >  					  KVM_INVALID_PTE_TYPE_MASK)
> > > >  
> > > >  enum kvm_invalid_pte_type {
> > > >  	/*
> > > >  	 * Used to indicate a pte for which a 'break-before-make'
> > > > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> > > > index b74dd5ce1efd..ca49f1bd7c34 100644
> > > > --- a/arch/arm64/kvm/hyp/pgtable.c
> > > > +++ b/arch/arm64/kvm/hyp/pgtable.c
> > > > @@ -725,42 +725,43 @@ static int stage2_set_prot_attr(struct kvm_pgtable *pgt, enum kvm_pgtable_prot p
> > > >  	}
> > > >  
> > > >  	r = stage2_set_xn_attr(prot, &attr);
> > > >  	if (r)
> > > >  		return r;
> > > >  
> > > >  	if (prot & KVM_PGTABLE_PROT_R)
> > > >  		attr |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
> > > >  
> > > >  	if (prot & KVM_PGTABLE_PROT_W)
> > > > -		attr |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > > > +		attr |= KVM_PTE_LEAF_ATTR_HI_S2_DBM | KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > > 
> > > What makes it acceptable to always set DBM? This is an optional
> > > feature, and I'm not exactly comfortable setting bits that are
> > > supposed to be RES0. 
> > 
> > The alternative would be to introduce WC/WD as an alternative encoding
> > infrastructure, and keep making decisions based on having HAFDBS available 
> > or in the system. My intention was to adapt things in a way to avoid 
> > multiple behaviors if we could avoid.
> > 
> > My rationale is, based on our possibility of setting DBM=1:
> > If HAFDBS/HDBSS/HACDBS are not available, or available but disabled, 
> > then the DBM bit is just ignored:
> > RO -> RO
> > WC -> RO
> > WD -> RW
> > 
> > (for reference:
> > - Read-Only (RO):      DBM=0, S2AP[1]=0
> > - Writable-Clean (WC): DBM=1, S2AP[1]=0
> > - Writable-Dirty (WD): DBM=1, S2AP[1]=1
> > )
> > 
> > And the behavior should just the same as what we have today.
> > 
> > 
> > > Yes, the HW should ignore it. But we have also
> > > seen quite a few broken designs in this area...
> > > 
> > 
> > I lack experience on what bad thing could happen. So I will expand on what 
> > I belive to understand up to here:
> > 
> > - The PTE is in memory, so the DBM bit can be set regardless of being RES0
> > - For SW pagetable walking, I don't think 'bit 51 == 0' is checked
> > - For HW pagetable walking, maybe some faulty implementation may rely on 
> >   bit51 being RES0, and fault otherwise.
> > 
> > If that's the case, then we would have to actually support both encodings, 
> > and only enable the new one if HAFDBS is available in the system. 
> > 
> > I just wonder how high are the chances to have such a broken design, 
> > or other broken designs did not come to my mind, and if we have to start 
> > with that multiple-encoding option.
> > 
> > > > +
> > > >
> > > 
> > > Spurious newline.
> > > 
> > 
> > Oh, ok, will get rid of it.
> > 
> > > >  	if (!kvm_lpa2_is_enabled())
> > > >  		attr |= FIELD_PREP(KVM_PTE_LEAF_ATTR_LO_S2_SH, sh);
> > > >  
> > > >  	attr |= KVM_PTE_LEAF_ATTR_LO_S2_AF;
> > > >  	attr |= prot & KVM_PTE_LEAF_ATTR_HI_SW;
> > > >  	*ptep = attr;
> > > >  
> > > >  	return 0;
> > > >  }
> > > 
> > > I don't know how you have configured git on your end, but there is *a
> > > lot* of context...
> > > 
> > 
> > diff.context=10, i.e. 10 lines before and after a change.
> > To me it looks helpful to get more context around the change. 
> > 
> > I can change it back to 5 if it bothers you.
> > 
> > > >  
> > > >  enum kvm_pgtable_prot kvm_pgtable_stage2_pte_prot(kvm_pte_t pte)
> > > >  {
> > > >  	enum kvm_pgtable_prot prot = pte & KVM_PTE_LEAF_ATTR_HI_SW;
> > > >  
> > > >  	if (!kvm_pte_valid(pte))
> > > >  		return prot;
> > > >  
> > > >  	if (pte & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R)
> > > >  		prot |= KVM_PGTABLE_PROT_R;
> > > > -	if (pte & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W)
> > > > +	if (pte & KVM_PTE_LEAF_ATTR_HI_S2_DBM)
> > > >  		prot |= KVM_PGTABLE_PROT_W;
> > > >  
> > > >  	switch (FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, pte)) {
> > > >  	case 0b00:
> > > >  		prot |= KVM_PGTABLE_PROT_PX | KVM_PGTABLE_PROT_UX;
> > > >  		break;
> > > >  	case 0b01:
> > > >  		prot |= KVM_PGTABLE_PROT_UX;
> > > >  		break;
> > > >  	case 0b11:
> > > > @@ -1281,20 +1282,21 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,
> > > >  		*orig_pte = data.pte;
> > > >  
> > > >  	if (level)
> > > >  		*level = data.level;
> > > >  	return 0;
> > > >  }
> > > >  
> > > >  int kvm_pgtable_stage2_wrprotect(struct kvm_pgtable *pgt, u64 addr, u64 size)
> > > >  {
> > > >  	return stage2_update_leaf_attrs(pgt, addr, size, 0,
> > > > +					KVM_PTE_LEAF_ATTR_HI_S2_DBM |
> > > >  					KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W,
> > > >  					NULL, NULL,
> > > >  					KVM_PGTABLE_WALK_IGNORE_EAGAIN);
> > > >  }
> > > >  
> > > >  void kvm_pgtable_stage2_mkyoung(struct kvm_pgtable *pgt, u64 addr,
> > > >  				enum kvm_pgtable_walk_flags flags)
> > > >  {
> > > >  	int ret;
> > > >  
> > > > @@ -1361,21 +1363,21 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,
> > > >  	s8 level;
> > > >  	int ret;
> > > >  
> > > >  	if (prot & KVM_PTE_LEAF_ATTR_HI_SW)
> > > >  		return -EINVAL;
> > > >  
> > > >  	if (prot & KVM_PGTABLE_PROT_R)
> > > >  		set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
> > > >  
> > > >  	if (prot & KVM_PGTABLE_PROT_W)
> > > > -		set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > > > +		set |= KVM_PTE_LEAF_ATTR_HI_S2_DBM | KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > > >  
> > > >  	if (prot & KVM_PGTABLE_PROT_X) {
> > > >  		ret = stage2_set_xn_attr(prot, &xn);
> > > >  		if (ret)
> > > >  			return ret;
> > > >  
> > > >  		set |= xn & KVM_PTE_LEAF_ATTR_HI_S2_XN;
> > > >  		clr |= ~xn & KVM_PTE_LEAF_ATTR_HI_S2_XN;
> > > >  	}
> > > >  
> > > > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> > > > index 17123f0b6dab..eb8dfffc32c7 100644
> > > > --- a/arch/arm64/kvm/nested.c
> > > > +++ b/arch/arm64/kvm/nested.c
> > > > @@ -379,21 +379,23 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
> > > >  	}
> > > >  
> > > >  	addr_bottom += contiguous_bit_shift(desc, wi, level);
> > > >  
> > > >  	/* Calculate and return the result */
> > > >  	paddr = (desc & GENMASK_ULL(47, addr_bottom)) |
> > > >  		(ipa & GENMASK_ULL(addr_bottom - 1, 0));
> > > >  	out->output = paddr;
> > > >  	out->block_size = 1UL << ((3 - level) * stride + wi->pgshift);
> > > >  	out->readable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
> > > > -	out->writable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > > > +	/* Takes care of both RO/RW and RO/WC/WD encodings */
> > > > +	out->writable = desc & (KVM_PTE_LEAF_ATTR_HI_S2_DBM |
> > > > +				KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W);
> > > 
> > > Absolutely NOT. For a start, NV doesn't support FEAT_HAFDBS. But even
> > > if it did, you are now actively corrupting memory by turning a RO
> > > mapping with a spurious DBM bit set into a writable mapping.
> > > VTCR_EL2.HD exists for a reason.
> > > 
> > > Do you see why your blanket approach of equating DBM with writable is
> > > plain wrong?
> > 
> > Sorry, not really... please help me understand it.
> > 
> > When you say a spurious DBM bit, what does it mean?
> > 
> > All I can think is something like a buggy HW implementation setting DBM in 
> > a HAFDBS-disabled machine, which has been unnoticed up to now, and now 
> > could cause problems.
> 
> And pray tell, what is it to have HAFDBS being disabled at S2? Instead
> of throwing the book at me, you should probably read it, and specially
> the effects of VTCR_EL2.HD.

Sorry, I did not mean to "trow a book" at you here, I was just trying to 
figure out what was wrong, and given I was not able to do it by myself I 
thought it was a good idea to ask you.

I just did not want to do that in an effortless way, so I gathered the 
stuff that I previously thougt to understand, and brought them to the 
discussion so it could be easier to point what I got wrong. 

As for your question, the effects of VTCR_EL2.HD is to actually mark the 
page as dirty (S2AP=1) when an writable-clean page (DBM=1, S2AP=0) gets 
writen to. DBM seems to act as permission to let the hardware change the 
S2AP bit when HAFDBS available & enabled. 

So if we want to use DBM as writable, we have to make sure we have 
VTCR_EL2.HD=0, or we are indeed intending to use HAFDBS here.

Not sure if I get your point here.

Thanks!
Leo

Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM

[Leonardo Bras]

On Wed, Sep 16, 2026 at 01:20:33PM +0100, Marc Zyngier wrote:
> On Wed, 16 Sep 2026 12:22:05 +0100,
> Leonardo Bras <leo.bras@arm.com> wrote:
> > 
> > On Tue, Sep 15, 2026 at 05:37:15PM -0700, Oliver Upton wrote:
> > > On Tue, Sep 15, 2026 at 06:12:45PM +0100, Leonardo Bras wrote:
> > > > > Yes, the HW should ignore it. But we have also
> > > > > seen quite a few broken designs in this area...
> > > > > 
> > > > 
> > > > I lack experience on what bad thing could happen. So I will expand on what 
> > > > I belive to understand up to here:
> > > > 
> > > > - The PTE is in memory, so the DBM bit can be set regardless of being RES0
> > > > - For SW pagetable walking, I don't think 'bit 51 == 0' is checked
> > > > - For HW pagetable walking, maybe some faulty implementation may rely on 
> > > >   bit51 being RES0, and fault otherwise.
> > > > 
> > > > If that's the case, then we would have to actually support both encodings, 
> > > > and only enable the new one if HAFDBS is available in the system. 
> > > > 
> > > > I just wonder how high are the chances to have such a broken design, 
> > > > or other broken designs did not come to my mind, and if we have to start 
> > > > with that multiple-encoding option.
> > > 
> > > FWIW, the host stage-1 already uses the DBM bit unconditionally,
> > > treating it as a software bit on implementations without HAFDBS.
> > > Although given the quality of any garden variety Arm MMU I understand
> > > where Marc is coming from.
> > > 
> > > I don't think the HAFDBS enablement is complicated enough to be done in
> > > a separate series without any meaningful users, nor would I really be
> > > interested in taking it without, say, HDBSS.
> > > 
> > > Can you please work with Tian to get a combined series out for this?
> > > 
> > 
> > Hi Oliver, thanks for reviewing!
> > 
> > Sure, one of the reasons I sent like this is so Tian could use it as a base 
> > for his next version.
> > 
> > 
> > > > > > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> > > > > > index 17123f0b6dab..eb8dfffc32c7 100644
> > > > > > --- a/arch/arm64/kvm/nested.c
> > > > > > +++ b/arch/arm64/kvm/nested.c
> > > > > > @@ -379,21 +379,23 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
> > > > > >  	}
> > > > > >  
> > > > > >  	addr_bottom += contiguous_bit_shift(desc, wi, level);
> > > > > >  
> > > > > >  	/* Calculate and return the result */
> > > > > >  	paddr = (desc & GENMASK_ULL(47, addr_bottom)) |
> > > > > >  		(ipa & GENMASK_ULL(addr_bottom - 1, 0));
> > > > > >  	out->output = paddr;
> > > > > >  	out->block_size = 1UL << ((3 - level) * stride + wi->pgshift);
> > > > > >  	out->readable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
> > > > > > -	out->writable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
> > > > > > +	/* Takes care of both RO/RW and RO/WC/WD encodings */
> > > > > > +	out->writable = desc & (KVM_PTE_LEAF_ATTR_HI_S2_DBM |
> > > > > > +				KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W);
> > > > > 
> > > > > Absolutely NOT. For a start, NV doesn't support FEAT_HAFDBS. But even
> > > > > if it did, you are now actively corrupting memory by turning a RO
> > > > > mapping with a spurious DBM bit set into a writable mapping.
> > > > > VTCR_EL2.HD exists for a reason.
> > > > > 
> > > > > Do you see why your blanket approach of equating DBM with writable is
> > > > > plain wrong?
> > > > 
> > > > Sorry, not really... please help me understand it.
> > > > 
> > > > When you say a spurious DBM bit, what does it mean?
> > > 
> > > You've implemented the exact sort of bug that was alluded to above. In
> > > this case it's a software page table walker consuming DBM regardless of
> > > the value of VTCR_EL2.HD.
> > > 
> > 
> > So you mean that DBM being treated as "writable" could _only_ happen if we 
> > have VTCR_EL2.HD=1? I was previously under the impression that it could be 
> > used regardless of HD value.
> 
> Then you have failed to understand the architecture. When
> VTCR_EL2.HD==0, DBM can be treated as RES0, RES0, or RES0.
> 
> R_XZFQH and R_BRFGY are pretty clear that DBM can only be evaluated
> when dirty hw update is enabled, and I_MHJZP tells you what it means
> for S2 to have this enabled.

Right, I_MHJZP says that if VTCR_EL2.HD is 1, then dirty state hardware 
management is enabled.

Then, R_XZFQH and R_BRFGY say that if (DBM+S2AP) bit combination is the 
given and the dirty state hardware management is enabled then a block 
descriptor is WC/WD.

R_XZFQH header, as an example:
For each translation stage using Direct permissions, if all of the 
following apply, then a Block descriptor or Page descriptor is described 
as writable-clean[...]

It says: 
"if all apply, the block is WC", 
it does not say 
"only if all apply, the block is WC".
so it forces one way, but not the other.

So I previously understood that we could also use the encoding for WC/WD in 
software, to avoid having 2 distinct encodings, when VTCR_EL2.HD=0. 

If that's not possible, as you mentioned, then yes, I failed to get that 
part. I also see that you mention it being RES0 when VTCR_EL2.HD==0, but I 
honestly could not find reference to that. :(

What I could find on Arm ARM M.cc was Table D8-53, which states for 
bit 51 that it's DBM if indirect permissions are disabled, or PIINDEX[1] 
if they are enabled. There is also no such information in D8.5.2 about this 
res0 behavior. 

So I am possibly missing the proper place to look for that information. 
Could you please share that with me so I can improve and maybe avoid being 
a inconvenience in the future?

Thanks!
Leo

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

[Leonardo Bras]

On Tue, Sep 15, 2026 at 05:10:02PM -0700, Oliver Upton wrote:
> Hi,
> 
> On Tue, Sep 01, 2026 at 06:15:56PM +0100, Leonardo Bras wrote:
> > When dirty-logging is disabled, even non-write faults make a page dirty,
> > which avoids a second fault when the page is actually written to.
> > 
> > On dirty-logging enable, this approach causes all (writable) pages on the
> > memslot to be marked clean, even if they were not written to, which can
> > take a lot of time, while holding the MMU lock, doing atomic writes to
> > PTEs.
> 
> Do you have any performance numbers for this? Enabling HAFDBS seems a
> bit involved to avoid some stores on the first pass.

Not yet, but if the idea does not look too crazy I can find hardware and 
collect some data :)

> 
> > So, if the system supports VHE + HAFDBS, keep the non-write-faulted page as
> > writable-clean, and let HAFDBS update that on demand when a write happens.
> > 
> > When dirty-tracking actually starts, disable HAFDBS as having it on
> > avoids the same fault that is used for dirty-logging.
> > 
> > Signed-off-by: Leonardo Bras <leo.bras@arm.com>
> > ---
> >  arch/arm64/include/asm/kvm_mmu.h    |  6 ++++++
> >  arch/arm64/include/asm/kvm_nested.h |  9 +++++++--
> >  arch/arm64/kvm/arm.c                |  7 +++++++
> >  arch/arm64/kvm/mmu.c                | 26 +++++++++++++++++++++++++-
> >  4 files changed, 45 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> > index 6eae7e7e2a68..3defa1a988d3 100644
> > --- a/arch/arm64/include/asm/kvm_mmu.h
> > +++ b/arch/arm64/include/asm/kvm_mmu.h
> > @@ -383,20 +383,26 @@ static inline void kvm_fault_unlock(struct kvm *kvm)
> >   * and CMOs are NOP'd. This has the effect of no longer requiring a
> >   * KVA for addresses mapped into the S2. The presence of these features
> >   * are thus necessary to support cacheable S2 mapping of VM_PFNMAP.
> >   */
> >  static inline bool kvm_supports_cacheable_pfnmap(void)
> >  {
> >  	return cpus_have_final_cap(ARM64_HAS_STAGE2_FWB) &&
> >  	       cpus_have_final_cap(ARM64_HAS_CACHE_DIC);
> >  }
> >  
> > +static inline bool kvm_supports_hafdbs(struct kvm *kvm)
> > +{
> > +	return IS_ENABLED(CONFIG_ARM64_HW_AFDBM) &&
> 
> cpucap_is_possible() is the right place to park this.
> 

Noted!

> > 						      has_vhe() &&
> 
> I don't see a reason why this needs to be constrained to VHE-only.
> 

Humm, in nVHE would not the host kernel run in EL1?
I thought that this being a feature that depends on EL2 registers host 
would need to be in EL2 to make use of it.

That being said, I understand very little of how this works, so I 
constrained to VHE only at the start.

Would this work in nVHE?

> > +		!kvm_vcpu_has_nv(kvm) && cpus_have_final_cap(ARM64_HW_DBM);
> 
> Same thing goes for nested... KVM can make use of HAFDBS in the
> canonical stage-2 MMU (or even a shadow stage-2) independent of the
> guest hypervisor.
> 

Humm, I remember reaching the conclusion that it could not be used if the 
guest supported NV. Let's say:

L0 - Host	- Has HAFDBS enabled
L1 - Hypervisor - Has HAFDBS disabled
L2 - Guest	-

Let's say guest writes to a page, and the shadow S2 has DBM=1, so it's 
marked as WD by HAFDBS. Since no fault was taken, how would the L1 be able 
to update it's S2 pagetables to mark the page dirty?

(We would have to transverse the Shadow S2 Pagetable updating the original 
S2 pagetable)

I was wondering, thought, that we could emulate it in the last level 
hypervisor, if it's guest does not support nested guests. That would mean 
we can have the last-1 level hypervisor to update the S2 pagetable on the 
last level hypervisor without it having to fault. Ex:

L0 Host - HAFDBS disabled
[...]
Ln-1 Hypervisor - HAFDBS disabled
Ln   Hypervisor - HAFDBS enabled
Ln+1 Guest - No E2H feature

When the guest writes to a page, the host should receive a fault, that IIUC 
have to propagate down up to Ln Hyp. If Ln Hyp has HAFDBS, we could skip 
injecting a fault in Ln Hyp, as  Ln-1 Hyp could emulate HAFDBS and write 
the dirty bit to S2 pagetagle of Ln+1 guest, that resides in Ln memory.

Not sure if the troulbe would be worth, though.
Does it make sense?



> >  #ifdef CONFIG_PTDUMP_STAGE2_DEBUGFS
> >  void kvm_s2_ptdump_create_debugfs(struct kvm *kvm);
> >  void kvm_nested_s2_ptdump_create_debugfs(struct kvm_s2_mmu *mmu);
> >  void kvm_nested_s2_ptdump_remove_debugfs(struct kvm_s2_mmu *mmu);
> >  #else
> >  static inline void kvm_s2_ptdump_create_debugfs(struct kvm *kvm) {}
> >  static inline void kvm_nested_s2_ptdump_create_debugfs(struct kvm_s2_mmu *mmu) {}
> >  static inline void kvm_nested_s2_ptdump_remove_debugfs(struct kvm_s2_mmu *mmu) {}
> >  #endif /* CONFIG_PTDUMP_STAGE2_DEBUGFS */
> >  
> > diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
> > index 1ed708335809..9242b5d665af 100644
> > --- a/arch/arm64/include/asm/kvm_nested.h
> > +++ b/arch/arm64/include/asm/kvm_nested.h
> > @@ -1,24 +1,29 @@
> >  /* SPDX-License-Identifier: GPL-2.0 */
> >  #ifndef __ARM64_KVM_NESTED_H
> >  #define __ARM64_KVM_NESTED_H
> >  
> >  #include <linux/bitfield.h>
> >  #include <linux/kvm_host.h>
> >  #include <asm/kvm_emulate.h>
> >  #include <asm/kvm_pgtable.h>
> >  
> > -static inline bool vcpu_has_nv(const struct kvm_vcpu *vcpu)
> > +static inline bool kvm_vcpu_has_nv(const struct kvm *kvm)
> 
> The name would suggest this thing takes a vcpu pointer...
> 

Ah, that name was based on
#define kvm_vcpu_has_feature(k, f)  __vcpu_has_feature(&(k)->arch, #(f))

That takes a kvm struct to check the kvm_arch one, instead of looking into 
the vcpu. I did it like this because there were some scenarios it was not 
quite straightforward to get the vcpu to use vcpu_has_nv(), which takes a 
vcpu.


Thanks!
Leo

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

[Oliver Upton]

On Wed, Sep 16, 2026 at 03:00:46PM +0100, Leonardo Bras wrote:
> On Tue, Sep 15, 2026 at 05:10:02PM -0700, Oliver Upton wrote:
> > Hi,
> > 
> > On Tue, Sep 01, 2026 at 06:15:56PM +0100, Leonardo Bras wrote:
> > > When dirty-logging is disabled, even non-write faults make a page dirty,
> > > which avoids a second fault when the page is actually written to.
> > > 
> > > On dirty-logging enable, this approach causes all (writable) pages on the
> > > memslot to be marked clean, even if they were not written to, which can
> > > take a lot of time, while holding the MMU lock, doing atomic writes to
> > > PTEs.
> > 
> > Do you have any performance numbers for this? Enabling HAFDBS seems a
> > bit involved to avoid some stores on the first pass.
> 
> Not yet, but if the idea does not look too crazy I can find hardware and 
> collect some data :)

TBH this looks like a micro-optimization so I'm not expecting the
performance gains to justify the behavior change.

> > > 						      has_vhe() &&
> > 
> > I don't see a reason why this needs to be constrained to VHE-only.
> > 
> 
> Humm, in nVHE would not the host kernel run in EL1?
> I thought that this being a feature that depends on EL2 registers host 
> would need to be in EL2 to make use of it.
> 
> That being said, I understand very little of how this works, so I 
> constrained to VHE only at the start.
> 
> Would this work in nVHE?

We already pass a stage-2 MMU configuration to EL2 from EL1 in nVHE and
hVHE. How is this any different?

I'm not opposed to making features VHE-only, but there needs to be some
amount of reasoning to justify it.

> > > +		!kvm_vcpu_has_nv(kvm) && cpus_have_final_cap(ARM64_HW_DBM);
> > 
> > Same thing goes for nested... KVM can make use of HAFDBS in the
> > canonical stage-2 MMU (or even a shadow stage-2) independent of the
> > guest hypervisor.
> > 
> 
> Humm, I remember reaching the conclusion that it could not be used if the 
> guest supported NV. Let's say:
> 
> L0 - Host	- Has HAFDBS enabled
> L1 - Hypervisor - Has HAFDBS disabled
> L2 - Guest	-
> 
> Let's say guest writes to a page, and the shadow S2 has DBM=1, so it's 
> marked as WD by HAFDBS. Since no fault was taken, how would the L1 be able 
> to update it's S2 pagetables to mark the page dirty?
> 
> (We would have to transverse the Shadow S2 Pagetable updating the original 
> S2 pagetable)
> 
> I was wondering, thought, that we could emulate it in the last level 
> hypervisor, if it's guest does not support nested guests. That would mean 
> we can have the last-1 level hypervisor to update the S2 pagetable on the 
> last level hypervisor without it having to fault. Ex:
> 
> L0 Host - HAFDBS disabled
> [...]
> Ln-1 Hypervisor - HAFDBS disabled
> Ln   Hypervisor - HAFDBS enabled
> Ln+1 Guest - No E2H feature
> 
> When the guest writes to a page, the host should receive a fault, that IIUC 
> have to propagate down up to Ln Hyp. If Ln Hyp has HAFDBS, we could skip 
> injecting a fault in Ln Hyp, as  Ln-1 Hyp could emulate HAFDBS and write 
> the dirty bit to S2 pagetagle of Ln+1 guest, that resides in Ln memory.
> 
> Not sure if the troulbe would be worth, though.
> Does it make sense?

I'm not following your reasoning here. Treat the shadow stage-2 MMU as a
TLB; that TLB is filled with a writable translation when S2AP[1]=1 in the
L1 translation.

The dirty state of the pseudo-TLB is completely internal. You could then
layer HAFDBS for the L1 translation on top of this (which we don't
support) by potentially relaxing the descriptor _before_ evaluating the
resulting permissions. You'd then take write permission faults to set
S2AP[1] in the L1 translation.

> > The name would suggest this thing takes a vcpu pointer...
> > 
> 
> Ah, that name was based on
> #define kvm_vcpu_has_feature(k, f)  __vcpu_has_feature(&(k)->arch, #(f))
> 
> That takes a kvm struct to check the kvm_arch one, instead of looking into 
> the vcpu. I did it like this because there were some scenarios it was not 
> quite straightforward to get the vcpu to use vcpu_has_nv(), which takes a 
> vcpu.

This thing probably should've been "kvm_has_vcpu_feature()" or similar to
massage the expected typing.

Thanks,
Oliver

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

[Leonardo Bras]

On Wed, Sep 16, 2026 at 04:27:19PM -0700, Oliver Upton wrote:
> On Wed, Sep 16, 2026 at 03:00:46PM +0100, Leonardo Bras wrote:
> > On Tue, Sep 15, 2026 at 05:10:02PM -0700, Oliver Upton wrote:
> > > Hi,
> > > 
> > > On Tue, Sep 01, 2026 at 06:15:56PM +0100, Leonardo Bras wrote:
> > > > When dirty-logging is disabled, even non-write faults make a page dirty,
> > > > which avoids a second fault when the page is actually written to.
> > > > 
> > > > On dirty-logging enable, this approach causes all (writable) pages on the
> > > > memslot to be marked clean, even if they were not written to, which can
> > > > take a lot of time, while holding the MMU lock, doing atomic writes to
> > > > PTEs.
> > > 
> > > Do you have any performance numbers for this? Enabling HAFDBS seems a
> > > bit involved to avoid some stores on the first pass.
> > 
> > Not yet, but if the idea does not look too crazy I can find hardware and 
> > collect some data :)
> 
> TBH this looks like a micro-optimization so I'm not expecting the
> performance gains to justify the behavior change.
> 

Okay then, I will get a machine to figure the performance gains :)

> > > > 						      has_vhe() &&
> > > 
> > > I don't see a reason why this needs to be constrained to VHE-only.
> > > 
> > 
> > Humm, in nVHE would not the host kernel run in EL1?
> > I thought that this being a feature that depends on EL2 registers host 
> > would need to be in EL2 to make use of it.
> > 
> > That being said, I understand very little of how this works, so I 
> > constrained to VHE only at the start.
> > 
> > Would this work in nVHE?
> 
> We already pass a stage-2 MMU configuration to EL2 from EL1 in nVHE and
> hVHE. How is this any different?
> 

Ah, so we can just ask the hypervisor in EL2 to enable that behavior in 
those cpus? I have dig deeper in code to get a better understanding on how 
that would work.

Would that make sense in pKVM as well?

> I'm not opposed to making features VHE-only, but there needs to be some
> amount of reasoning to justify it.

Likewise, I am not opposed to make it work outside VHE as well, I just 
don't understand enough of nVHE/hVHE/pKVM yet, so I thought starting with 
VHE would be simpler.

> 
> > > > +		!kvm_vcpu_has_nv(kvm) && cpus_have_final_cap(ARM64_HW_DBM);
> > > 
> > > Same thing goes for nested... KVM can make use of HAFDBS in the
> > > canonical stage-2 MMU (or even a shadow stage-2) independent of the
> > > guest hypervisor.
> > > 
> > 
> > Humm, I remember reaching the conclusion that it could not be used if the 
> > guest supported NV. Let's say:
> > 
> > L0 - Host	- Has HAFDBS enabled
> > L1 - Hypervisor - Has HAFDBS disabled
> > L2 - Guest	-
> > 
> > Let's say guest writes to a page, and the shadow S2 has DBM=1, so it's 
> > marked as WD by HAFDBS. Since no fault was taken, how would the L1 be able 
> > to update it's S2 pagetables to mark the page dirty?
> > 
> > (We would have to transverse the Shadow S2 Pagetable updating the original 
> > S2 pagetable)

(*)

> > 
> > I was wondering, thought, that we could emulate it in the last level 
> > hypervisor, if it's guest does not support nested guests. That would mean 
> > we can have the last-1 level hypervisor to update the S2 pagetable on the 
> > last level hypervisor without it having to fault. Ex:
> > 
> > L0 Host - HAFDBS disabled
> > [...]
> > Ln-1 Hypervisor - HAFDBS disabled
> > Ln   Hypervisor - HAFDBS enabled
> > Ln+1 Guest - No E2H feature
> > 
> > When the guest writes to a page, the host should receive a fault, that IIUC 
> > have to propagate down up to Ln Hyp. If Ln Hyp has HAFDBS, we could skip 
> > injecting a fault in Ln Hyp, as  Ln-1 Hyp could emulate HAFDBS and write 
> > the dirty bit to S2 pagetagle of Ln+1 guest, that resides in Ln memory.
> > 
> > Not sure if the troulbe would be worth, though.
> > Does it make sense?
> 
> I'm not following your reasoning here.

Oh, sorry, that got confusing up there. Everything above (*) was my 
perception of why would not that work. Lines below (*) are a new idea on 
how we could use it in the last level hypervisor.

> Treat the shadow stage-2 MMU as a
> TLB; that TLB is filled with a writable translation when S2AP[1]=1 in the
> L1 translation.

L1 considering that L1 is the last level hypervisor, right? So the example 
above the (*). 

If so, yeah it makes sense. Shadow S2 acs as a TLB so we don't have to 
walk all intermediate pagetables to figure out the translation every time 
we need to access it and the entry is not in the HW TLB.

Thanks for sharing this... thinking about the shadow S2 just became easier 
:)

> 
> The dirty state of the pseudo-TLB is completely internal. You could then
> layer HAFDBS for the L1 translation on top of this (which we don't
> support) by potentially relaxing the descriptor _before_ evaluating the
> resulting permissions. You'd then take write permission faults to set
> S2AP[1] in the L1 translation.
> 

Would relaxing here mean mark the shadow S2 PTE as RO?
I think you got what I meant on the idea below (*), but exemplified with 
the example above (*).

What I meant in the idea below the (*) is that the level (n-1) hypervisor 
could just flip the dirty-bit for it's level n guest (which is the last 
level  hypervisor), emulating a hardware that supports HAFDBS. That would 
avoid injecting the fault in the level n guest and just flip the bit 
directly from level (n-1) viewpoint. For that, only the Ln hypervisor 
should have HAFDBS enabled. (this idea does not depend on the 
underlying hardware actually supporting HAFDBS).

Does it make sense, then?

> > > The name would suggest this thing takes a vcpu pointer...
> > > 
> > 
> > Ah, that name was based on
> > #define kvm_vcpu_has_feature(k, f)  __vcpu_has_feature(&(k)->arch, #(f))
> > 
> > That takes a kvm struct to check the kvm_arch one, instead of looking into 
> > the vcpu. I did it like this because there were some scenarios it was not 
> > quite straightforward to get the vcpu to use vcpu_has_nv(), which takes a 
> > vcpu.
> 
> This thing probably should've been "kvm_has_vcpu_feature()" or similar to
> massage the expected typing.

Humm, makes sense.
Would you like me to change the new one to kvm_has_nv() in the next 
version?

Thanks!
Leo

Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM

[Tian Zheng]

On 9/16/2026 7:22 PM, Leonardo Bras wrote:
> On Tue, Sep 15, 2026 at 05:37:15PM -0700, Oliver Upton wrote:
>> On Tue, Sep 15, 2026 at 06:12:45PM +0100, Leonardo Bras wrote:
>>>> Yes, the HW should ignore it. But we have also
>>>> seen quite a few broken designs in this area...
>>>>
>>>
>>> I lack experience on what bad thing could happen. So I will expand on what
>>> I belive to understand up to here:
>>>
>>> - The PTE is in memory, so the DBM bit can be set regardless of being RES0
>>> - For SW pagetable walking, I don't think 'bit 51 == 0' is checked
>>> - For HW pagetable walking, maybe some faulty implementation may rely on
>>>    bit51 being RES0, and fault otherwise.
>>>
>>> If that's the case, then we would have to actually support both encodings,
>>> and only enable the new one if HAFDBS is available in the system.
>>>
>>> I just wonder how high are the chances to have such a broken design,
>>> or other broken designs did not come to my mind, and if we have to start
>>> with that multiple-encoding option.
>>
>> FWIW, the host stage-1 already uses the DBM bit unconditionally,
>> treating it as a software bit on implementations without HAFDBS.
>> Although given the quality of any garden variety Arm MMU I understand
>> where Marc is coming from.
>>
>> I don't think the HAFDBS enablement is complicated enough to be done in
>> a separate series without any meaningful users, nor would I really be
>> interested in taking it without, say, HDBSS.
>>
>> Can you please work with Tian to get a combined series out for this?
>>
> 
> Hi Oliver, thanks for reviewing!
> 
> Sure, one of the reasons I sent like this is so Tian could use it as a base
> for his next version.
> 
> 

Hi Oliver, Leo,

Works for us. I plan to send HDBSS v5 maybe next week with this series
merged in. Both dirty-tracking consumers are already built on top of the
DBM approach: dirty ring and dirty bitmap.

Leo, with your blessing, I'd like to pick patches 1-4 into the HDBSS
tree with your Signed-off-by preserved and mine added on top, plus some
bug fixes on top of this RFC series.

For patch 5, I'd like to rework it into a derived hardware dirty mode
that replaces both kvm_set_hafdbs() and our earlier HDBSS enable/disable
hooks, so the whole thing lands as one series.

Performance looks good in both dirty ring and dirty bitmap scenarios so
far.

Thanks,
Tian

>>>>> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
>>>>> index 17123f0b6dab..eb8dfffc32c7 100644
>>>>> --- a/arch/arm64/kvm/nested.c
>>>>> +++ b/arch/arm64/kvm/nested.c
>>>>> @@ -379,21 +379,23 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
>>>>>   	}
>>>>>   
>>>>>   	addr_bottom += contiguous_bit_shift(desc, wi, level);
>>>>>   
>>>>>   	/* Calculate and return the result */
>>>>>   	paddr = (desc & GENMASK_ULL(47, addr_bottom)) |
>>>>>   		(ipa & GENMASK_ULL(addr_bottom - 1, 0));
>>>>>   	out->output = paddr;
>>>>>   	out->block_size = 1UL << ((3 - level) * stride + wi->pgshift);
>>>>>   	out->readable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
>>>>> -	out->writable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
>>>>> +	/* Takes care of both RO/RW and RO/WC/WD encodings */
>>>>> +	out->writable = desc & (KVM_PTE_LEAF_ATTR_HI_S2_DBM |
>>>>> +				KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W);
>>>>
>>>> Absolutely NOT. For a start, NV doesn't support FEAT_HAFDBS. But even
>>>> if it did, you are now actively corrupting memory by turning a RO
>>>> mapping with a spurious DBM bit set into a writable mapping.
>>>> VTCR_EL2.HD exists for a reason.
>>>>
>>>> Do you see why your blanket approach of equating DBM with writable is
>>>> plain wrong?
>>>
>>> Sorry, not really... please help me understand it.
>>>
>>> When you say a spurious DBM bit, what does it mean?
>>
>> You've implemented the exact sort of bug that was alluded to above. In
>> this case it's a software page table walker consuming DBM regardless of
>> the value of VTCR_EL2.HD.
>>
> 
> So you mean that DBM being treated as "writable" could _only_ happen if we
> have VTCR_EL2.HD=1? I was previously under the impression that it could be
> used regardless of HD value.
> 
>> If the guest hypervisor sets VTCR_EL2.HD=0, the expectation is that the
>> shadow stage-2 MMU treats the corresponding bit in the PTE as RES0.
>>
> 
> Okay, I think I can see it now: since the guest hypervisor could use RO/RW,
> and has no decoupled concept of dirty and writable, it could think all
> writable pages are dirty when we run above function.
> 
> So.. would it make sense to have an out->dirty, which we would check based
> on S2AP while out->writable is compared against DBM, and we change the
> logic that uses out->writable to properly match it, maybe based on the
> guest having the feature enabled?
> 
> Thanks for your patience on explaining this!
> Leo
> 

Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM

[Tian Zheng]

On 9/16/2026 9:25 PM, Leonardo Bras wrote:
> On Wed, Sep 16, 2026 at 01:20:33PM +0100, Marc Zyngier wrote:
>> On Wed, 16 Sep 2026 12:22:05 +0100,
>> Leonardo Bras <leo.bras@arm.com> wrote:
>>>
>>> On Tue, Sep 15, 2026 at 05:37:15PM -0700, Oliver Upton wrote:
>>>> On Tue, Sep 15, 2026 at 06:12:45PM +0100, Leonardo Bras wrote:
>>>>>> Yes, the HW should ignore it. But we have also
>>>>>> seen quite a few broken designs in this area...
>>>>>>
>>>>>
>>>>> I lack experience on what bad thing could happen. So I will expand on what
>>>>> I belive to understand up to here:
>>>>>
>>>>> - The PTE is in memory, so the DBM bit can be set regardless of being RES0
>>>>> - For SW pagetable walking, I don't think 'bit 51 == 0' is checked
>>>>> - For HW pagetable walking, maybe some faulty implementation may rely on
>>>>>    bit51 being RES0, and fault otherwise.
>>>>>
>>>>> If that's the case, then we would have to actually support both encodings,
>>>>> and only enable the new one if HAFDBS is available in the system.
>>>>>
>>>>> I just wonder how high are the chances to have such a broken design,
>>>>> or other broken designs did not come to my mind, and if we have to start
>>>>> with that multiple-encoding option.
>>>>
>>>> FWIW, the host stage-1 already uses the DBM bit unconditionally,
>>>> treating it as a software bit on implementations without HAFDBS.
>>>> Although given the quality of any garden variety Arm MMU I understand
>>>> where Marc is coming from.
>>>>
>>>> I don't think the HAFDBS enablement is complicated enough to be done in
>>>> a separate series without any meaningful users, nor would I really be
>>>> interested in taking it without, say, HDBSS.
>>>>
>>>> Can you please work with Tian to get a combined series out for this?
>>>>
>>>
>>> Hi Oliver, thanks for reviewing!
>>>
>>> Sure, one of the reasons I sent like this is so Tian could use it as a base
>>> for his next version.
>>>
>>>
>>>>>>> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
>>>>>>> index 17123f0b6dab..eb8dfffc32c7 100644
>>>>>>> --- a/arch/arm64/kvm/nested.c
>>>>>>> +++ b/arch/arm64/kvm/nested.c
>>>>>>> @@ -379,21 +379,23 @@ static int walk_nested_s2_pgd(struct kvm_vcpu *vcpu, phys_addr_t ipa,
>>>>>>>   	}
>>>>>>>   
>>>>>>>   	addr_bottom += contiguous_bit_shift(desc, wi, level);
>>>>>>>   
>>>>>>>   	/* Calculate and return the result */
>>>>>>>   	paddr = (desc & GENMASK_ULL(47, addr_bottom)) |
>>>>>>>   		(ipa & GENMASK_ULL(addr_bottom - 1, 0));
>>>>>>>   	out->output = paddr;
>>>>>>>   	out->block_size = 1UL << ((3 - level) * stride + wi->pgshift);
>>>>>>>   	out->readable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
>>>>>>> -	out->writable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
>>>>>>> +	/* Takes care of both RO/RW and RO/WC/WD encodings */
>>>>>>> +	out->writable = desc & (KVM_PTE_LEAF_ATTR_HI_S2_DBM |
>>>>>>> +				KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W);
>>>>>>
>>>>>> Absolutely NOT. For a start, NV doesn't support FEAT_HAFDBS. But even
>>>>>> if it did, you are now actively corrupting memory by turning a RO
>>>>>> mapping with a spurious DBM bit set into a writable mapping.
>>>>>> VTCR_EL2.HD exists for a reason.
>>>>>>
>>>>>> Do you see why your blanket approach of equating DBM with writable is
>>>>>> plain wrong?
>>>>>
>>>>> Sorry, not really... please help me understand it.
>>>>>
>>>>> When you say a spurious DBM bit, what does it mean?
>>>>
>>>> You've implemented the exact sort of bug that was alluded to above. In
>>>> this case it's a software page table walker consuming DBM regardless of
>>>> the value of VTCR_EL2.HD.
>>>>
>>>
>>> So you mean that DBM being treated as "writable" could _only_ happen if we
>>> have VTCR_EL2.HD=1? I was previously under the impression that it could be
>>> used regardless of HD value.
>>
>> Then you have failed to understand the architecture. When
>> VTCR_EL2.HD==0, DBM can be treated as RES0, RES0, or RES0.
>>
>> R_XZFQH and R_BRFGY are pretty clear that DBM can only be evaluated
>> when dirty hw update is enabled, and I_MHJZP tells you what it means
>> for S2 to have this enabled.
> 
> Right, I_MHJZP says that if VTCR_EL2.HD is 1, then dirty state hardware
> management is enabled.
> 
> Then, R_XZFQH and R_BRFGY say that if (DBM+S2AP) bit combination is the
> given and the dirty state hardware management is enabled then a block
> descriptor is WC/WD.
> 
> R_XZFQH header, as an example:
> For each translation stage using Direct permissions, if all of the
> following apply, then a Block descriptor or Page descriptor is described
> as writable-clean[...]
> 
> It says:
> "if all apply, the block is WC",
> it does not say
> "only if all apply, the block is WC".
> so it forces one way, but not the other.
> 
> So I previously understood that we could also use the encoding for WC/WD in
> software, to avoid having 2 distinct encodings, when VTCR_EL2.HD=0.
> 
> If that's not possible, as you mentioned, then yes, I failed to get that
> part. I also see that you mention it being RES0 when VTCR_EL2.HD==0, but I
> honestly could not find reference to that. :(
> 
> What I could find on Arm ARM M.cc was Table D8-53, which states for
> bit 51 that it's DBM if indirect permissions are disabled, or PIINDEX[1]
> if they are enabled. There is also no such information in D8.5.2 about this
> res0 behavior.
> 
> So I am possibly missing the proper place to look for that information.
> Could you please share that with me so I can improve and maybe avoid being
> a inconvenience in the future?
> 
> Thanks!
> Leo
> 

Hi Leo, Marc,

On the NV side, KVM itself already limits the L1-visible
ID_AA64MMFR1_EL1.HAFDBS to AF-only in limit_nv_id_reg(), so bit 51 has
no architectural meaning to L1, and an L1 using it as software metadata
would have its read-only pages misread as writable by the union read in
walk_nested_s2_pgd() — the memory corruption Marc described. Since NV
has no HAFDBS emulation today, I'd suggest we simply revert this hunk
for now and keep reading writability from S2AP[1] alone:

```
out->writable = desc & KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
```

It seems we can address the NV side later, once nested HAFDBS is
actually supported.

That's what I did when picking the series up for HDBSS v5: patch 1 drops
the nested.c hunk.

Thanks,
Tian

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

[Tian Zheng]

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.

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)
  */
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;
	...
}
```

HA is always set alongside HD by construction, so the no-op issue goes
away.

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.

Thanks,
Tian

Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM

[Leonardo Bras]

On Fri, Sep 18, 2026 at 05:39:17PM +0800, Tian Zheng wrote:
> 
> 
> On 9/16/2026 7:22 PM, Leonardo Bras wrote:
> > On Tue, Sep 15, 2026 at 05:37:15PM -0700, Oliver Upton wrote:
> > > On Tue, Sep 15, 2026 at 06:12:45PM +0100, Leonardo Bras wrote:
> > > > > Yes, the HW should ignore it. But we have also
> > > > > seen quite a few broken designs in this area...
> > > > > 
> > > > 
> > > > I lack experience on what bad thing could happen. So I will expand on what
> > > > I belive to understand up to here:
> > > > 
> > > > - The PTE is in memory, so the DBM bit can be set regardless of being RES0
> > > > - For SW pagetable walking, I don't think 'bit 51 == 0' is checked
> > > > - For HW pagetable walking, maybe some faulty implementation may rely on
> > > >    bit51 being RES0, and fault otherwise.
> > > > 
> > > > If that's the case, then we would have to actually support both encodings,
> > > > and only enable the new one if HAFDBS is available in the system.
> > > > 
> > > > I just wonder how high are the chances to have such a broken design,
> > > > or other broken designs did not come to my mind, and if we have to start
> > > > with that multiple-encoding option.
> > > 
> > > FWIW, the host stage-1 already uses the DBM bit unconditionally,
> > > treating it as a software bit on implementations without HAFDBS.
> > > Although given the quality of any garden variety Arm MMU I understand
> > > where Marc is coming from.
> > > 
> > > I don't think the HAFDBS enablement is complicated enough to be done in
> > > a separate series without any meaningful users, nor would I really be
> > > interested in taking it without, say, HDBSS.
> > > 
> > > Can you please work with Tian to get a combined series out for this?
> > > 
> > 
> > Hi Oliver, thanks for reviewing!
> > 
> > Sure, one of the reasons I sent like this is so Tian could use it as a base
> > for his next version.
> > 
> > 
> 
> Hi Oliver, Leo,
> 
> Works for us. I plan to send HDBSS v5 maybe next week with this series
> merged in. Both dirty-tracking consumers are already built on top of the
> DBM approach: dirty ring and dirty bitmap.
> 
> Leo, with your blessing, I'd like to pick patches 1-4 into the HDBSS
> tree with your Signed-off-by preserved and mine added on top, plus some
> bug fixes on top of this RFC series.

Yeah, no problem on my side. I would just observe the maintainers' comments 
on those before merging them.

> 
> For patch 5, I'd like to rework it into a derived hardware dirty mode
> that replaces both kvm_set_hafdbs() and our earlier HDBSS enable/disable
> hooks, so the whole thing lands as one series.
> 

My intention when I wrote that patch was to add a base so you could add 
HDBSS on kvm_arch_commit_memory_region() with new patch such as:

	/* Disable HAFDBS when dirty-logging starts */
	if (kvm_supports_hafdbs(kvm))
		kvm_set_hafdbs(kvm, 0);
+	else
+		kvm_enable_hdbss(kvm);
...

	/* If dirty-logging was canceled, set HAFDBS back on */
	if (kvm_supports_hafdbs(kvm) &&
	    atomic_read(&kvm->nr_memslots_dirty_logging) == 0)
		kvm_set_hafdbs(kvm, 1);
+	else
+		kvm_disable_hdbss(kvm);


That being said, I need to run tests to make sure the usage of HAFDBS 
outside of dirty_tracking makes any sense in terms of performance, but if 
that's not the case, it would be fine to rework it so it does not 
enable/disable HAFDBS there.

> Performance looks good in both dirty ring and dirty bitmap scenarios so
> far.
> 

Awesome!

Thanks!
Leo

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

[Leonardo Bras]

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

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

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

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

Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM

[Tian Zheng]

On 9/21/2026 10:15 PM, Leonardo Bras wrote:
> On Fri, Sep 18, 2026 at 05:39:17PM +0800, Tian Zheng wrote:
>>
>>
>> On 9/16/2026 7:22 PM, Leonardo Bras wrote:
>>> On Tue, Sep 15, 2026 at 05:37:15PM -0700, Oliver Upton wrote:
>>>> On Tue, Sep 15, 2026 at 06:12:45PM +0100, Leonardo Bras wrote:
>>>>>> Yes, the HW should ignore it. But we have also
>>>>>> seen quite a few broken designs in this area...
>>>>>>
>>>>>
>>>>> I lack experience on what bad thing could happen. So I will expand on what
>>>>> I belive to understand up to here:
>>>>>
>>>>> - The PTE is in memory, so the DBM bit can be set regardless of being RES0
>>>>> - For SW pagetable walking, I don't think 'bit 51 == 0' is checked
>>>>> - For HW pagetable walking, maybe some faulty implementation may rely on
>>>>>     bit51 being RES0, and fault otherwise.
>>>>>
>>>>> If that's the case, then we would have to actually support both encodings,
>>>>> and only enable the new one if HAFDBS is available in the system.
>>>>>
>>>>> I just wonder how high are the chances to have such a broken design,
>>>>> or other broken designs did not come to my mind, and if we have to start
>>>>> with that multiple-encoding option.
>>>>
>>>> FWIW, the host stage-1 already uses the DBM bit unconditionally,
>>>> treating it as a software bit on implementations without HAFDBS.
>>>> Although given the quality of any garden variety Arm MMU I understand
>>>> where Marc is coming from.
>>>>
>>>> I don't think the HAFDBS enablement is complicated enough to be done in
>>>> a separate series without any meaningful users, nor would I really be
>>>> interested in taking it without, say, HDBSS.
>>>>
>>>> Can you please work with Tian to get a combined series out for this?
>>>>
>>>
>>> Hi Oliver, thanks for reviewing!
>>>
>>> Sure, one of the reasons I sent like this is so Tian could use it as a base
>>> for his next version.
>>>
>>>
>>
>> Hi Oliver, Leo,
>>
>> Works for us. I plan to send HDBSS v5 maybe next week with this series
>> merged in. Both dirty-tracking consumers are already built on top of the
>> DBM approach: dirty ring and dirty bitmap.
>>
>> Leo, with your blessing, I'd like to pick patches 1-4 into the HDBSS
>> tree with your Signed-off-by preserved and mine added on top, plus some
>> bug fixes on top of this RFC series.
> 
> Yeah, no problem on my side. I would just observe the maintainers' comments
> on those before merging them.
> 

All right, thanks.

Tian

>>
>> For patch 5, I'd like to rework it into a derived hardware dirty mode
>> that replaces both kvm_set_hafdbs() and our earlier HDBSS enable/disable
>> hooks, so the whole thing lands as one series.
>>
> 
> My intention when I wrote that patch was to add a base so you could add
> HDBSS on kvm_arch_commit_memory_region() with new patch such as:
> 
> 	/* Disable HAFDBS when dirty-logging starts */
> 	if (kvm_supports_hafdbs(kvm))
> 		kvm_set_hafdbs(kvm, 0);
> +	else
> +		kvm_enable_hdbss(kvm);
> ...
> 
> 	/* If dirty-logging was canceled, set HAFDBS back on */
> 	if (kvm_supports_hafdbs(kvm) &&
> 	    atomic_read(&kvm->nr_memslots_dirty_logging) == 0)
> 		kvm_set_hafdbs(kvm, 1);
> +	else
> +		kvm_disable_hdbss(kvm);
> 
> 
> That being said, I need to run tests to make sure the usage of HAFDBS
> outside of dirty_tracking makes any sense in terms of performance, but if
> that's not the case, it would be fine to rework it so it does not
> enable/disable HAFDBS there.
> 
>> Performance looks good in both dirty ring and dirty bitmap scenarios so
>> far.
>>
> 
> Awesome!
> 
> Thanks!
> Leo
> 

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

Thanks!
Tian