[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

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

[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

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

[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

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

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