* [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM
2026-09-01 17:15 [RFC PATCH 0/5] KVM: arm64: New PTE dirty-page encoding, HAFDBS new usage Leonardo Bras
@ 2026-09-01 17:15 ` Leonardo Bras
2026-09-13 9:00 ` Marc Zyngier
2026-09-01 17:15 ` [RFC PATCH 2/5] KVM: arm64: Add KVM_PGTABLE_PROT_DIRTY Leonardo Bras
` (4 subsequent siblings)
5 siblings, 1 reply; 22+ messages in thread
From: Leonardo Bras @ 2026-09-01 17:15 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Fuad Tabba, Joey Gouly,
Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Leonardo Bras, Raghavendra Rao Ananta,
Tian Zheng
Cc: linux-arm-kernel, kvmarm, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM
2026-09-01 17:15 ` [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM Leonardo Bras
@ 2026-09-13 9:00 ` Marc Zyngier
2026-09-15 17:12 ` Leonardo Bras
0 siblings, 1 reply; 22+ messages in thread
From: Marc Zyngier @ 2026-09-13 9:00 UTC (permalink / raw)
To: Leonardo Bras
Cc: Oliver Upton, Fuad Tabba, Joey Gouly, Steffen Eiden,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
Mark Rutland, Raghavendra Rao Ananta, Tian Zheng,
linux-arm-kernel, kvmarm, linux-kernel
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.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM
2026-09-13 9:00 ` Marc Zyngier
@ 2026-09-15 17:12 ` Leonardo Bras
2026-09-16 0:37 ` Oliver Upton
2026-09-16 8:30 ` Marc Zyngier
0 siblings, 2 replies; 22+ messages in thread
From: Leonardo Bras @ 2026-09-15 17:12 UTC (permalink / raw)
To: Marc Zyngier
Cc: Leonardo Bras, Oliver Upton, Fuad Tabba, Joey Gouly,
Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Raghavendra Rao Ananta, Tian Zheng,
linux-arm-kernel, kvmarm, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM
2026-09-15 17:12 ` Leonardo Bras
@ 2026-09-16 0:37 ` Oliver Upton
2026-09-16 11:22 ` Leonardo Bras
2026-09-16 8:30 ` Marc Zyngier
1 sibling, 1 reply; 22+ messages in thread
From: Oliver Upton @ 2026-09-16 0:37 UTC (permalink / raw)
To: Leonardo Bras
Cc: Marc Zyngier, Fuad Tabba, Joey Gouly, Steffen Eiden,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
Mark Rutland, Raghavendra Rao Ananta, Tian Zheng,
linux-arm-kernel, kvmarm, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM
2026-09-16 0:37 ` Oliver Upton
@ 2026-09-16 11:22 ` Leonardo Bras
2026-09-16 12:20 ` Marc Zyngier
0 siblings, 1 reply; 22+ messages in thread
From: Leonardo Bras @ 2026-09-16 11:22 UTC (permalink / raw)
To: Oliver Upton
Cc: Leonardo Bras, Marc Zyngier, Fuad Tabba, Joey Gouly,
Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Raghavendra Rao Ananta, Tian Zheng,
linux-arm-kernel, kvmarm, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM
2026-09-16 11:22 ` Leonardo Bras
@ 2026-09-16 12:20 ` Marc Zyngier
2026-09-16 13:25 ` Leonardo Bras
0 siblings, 1 reply; 22+ messages in thread
From: Marc Zyngier @ 2026-09-16 12:20 UTC (permalink / raw)
To: Leonardo Bras
Cc: Oliver Upton, Fuad Tabba, Joey Gouly, Steffen Eiden,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
Mark Rutland, Raghavendra Rao Ananta, Tian Zheng,
linux-arm-kernel, kvmarm, linux-kernel
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.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM
2026-09-16 12:20 ` Marc Zyngier
@ 2026-09-16 13:25 ` Leonardo Bras
0 siblings, 0 replies; 22+ messages in thread
From: Leonardo Bras @ 2026-09-16 13:25 UTC (permalink / raw)
To: Marc Zyngier
Cc: Leonardo Bras, Oliver Upton, Fuad Tabba, Joey Gouly,
Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Raghavendra Rao Ananta, Tian Zheng,
linux-arm-kernel, kvmarm, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM
2026-09-15 17:12 ` Leonardo Bras
2026-09-16 0:37 ` Oliver Upton
@ 2026-09-16 8:30 ` Marc Zyngier
2026-09-16 13:03 ` Leonardo Bras
1 sibling, 1 reply; 22+ messages in thread
From: Marc Zyngier @ 2026-09-16 8:30 UTC (permalink / raw)
To: Leonardo Bras
Cc: Oliver Upton, Fuad Tabba, Joey Gouly, Steffen Eiden,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
Mark Rutland, Raghavendra Rao Ananta, Tian Zheng,
linux-arm-kernel, kvmarm, linux-kernel
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.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM
2026-09-16 8:30 ` Marc Zyngier
@ 2026-09-16 13:03 ` Leonardo Bras
0 siblings, 0 replies; 22+ messages in thread
From: Leonardo Bras @ 2026-09-16 13:03 UTC (permalink / raw)
To: Marc Zyngier
Cc: Leonardo Bras, Oliver Upton, Fuad Tabba, Joey Gouly,
Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Raghavendra Rao Ananta, Tian Zheng,
linux-arm-kernel, kvmarm, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC PATCH 2/5] KVM: arm64: Add KVM_PGTABLE_PROT_DIRTY
2026-09-01 17:15 [RFC PATCH 0/5] KVM: arm64: New PTE dirty-page encoding, HAFDBS new usage Leonardo Bras
2026-09-01 17:15 ` [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM Leonardo Bras
@ 2026-09-01 17:15 ` Leonardo Bras
2026-09-13 9:09 ` Marc Zyngier
2026-09-01 17:15 ` [RFC PATCH 3/5] KVM: arm64: Introduce a dedicated walker for stage2 write-protect Leonardo Bras
` (3 subsequent siblings)
5 siblings, 1 reply; 22+ messages in thread
From: Leonardo Bras @ 2026-09-01 17:15 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Fuad Tabba, Joey Gouly,
Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Leonardo Bras, Raghavendra Rao Ananta,
Tian Zheng
Cc: linux-arm-kernel, kvmarm, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH 2/5] KVM: arm64: Add KVM_PGTABLE_PROT_DIRTY
2026-09-01 17:15 ` [RFC PATCH 2/5] KVM: arm64: Add KVM_PGTABLE_PROT_DIRTY Leonardo Bras
@ 2026-09-13 9:09 ` Marc Zyngier
2026-09-15 17:33 ` Leonardo Bras
0 siblings, 1 reply; 22+ messages in thread
From: Marc Zyngier @ 2026-09-13 9:09 UTC (permalink / raw)
To: Leonardo Bras
Cc: Oliver Upton, Fuad Tabba, Joey Gouly, Steffen Eiden,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
Mark Rutland, Raghavendra Rao Ananta, Tian Zheng,
linux-arm-kernel, kvmarm, linux-kernel
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.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH 2/5] KVM: arm64: Add KVM_PGTABLE_PROT_DIRTY
2026-09-13 9:09 ` Marc Zyngier
@ 2026-09-15 17:33 ` Leonardo Bras
0 siblings, 0 replies; 22+ messages in thread
From: Leonardo Bras @ 2026-09-15 17:33 UTC (permalink / raw)
To: Marc Zyngier
Cc: Leonardo Bras, Oliver Upton, Fuad Tabba, Joey Gouly,
Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Raghavendra Rao Ananta, Tian Zheng,
linux-arm-kernel, kvmarm, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC PATCH 3/5] KVM: arm64: Introduce a dedicated walker for stage2 write-protect
2026-09-01 17:15 [RFC PATCH 0/5] KVM: arm64: New PTE dirty-page encoding, HAFDBS new usage Leonardo Bras
2026-09-01 17:15 ` [RFC PATCH 1/5] KVM: arm64: pgtables: Change write bit from S2AP_W to DBM Leonardo Bras
2026-09-01 17:15 ` [RFC PATCH 2/5] KVM: arm64: Add KVM_PGTABLE_PROT_DIRTY Leonardo Bras
@ 2026-09-01 17:15 ` Leonardo Bras
2026-09-01 17:15 ` [RFC PATCH 4/5] KVM: arm64: Add KVM_REQ_RELOAD_STAGE2 Leonardo Bras
` (2 subsequent siblings)
5 siblings, 0 replies; 22+ messages in thread
From: Leonardo Bras @ 2026-09-01 17:15 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Fuad Tabba, Joey Gouly,
Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Leonardo Bras, Raghavendra Rao Ananta,
Tian Zheng
Cc: linux-arm-kernel, kvmarm, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread* [RFC PATCH 4/5] KVM: arm64: Add KVM_REQ_RELOAD_STAGE2
2026-09-01 17:15 [RFC PATCH 0/5] KVM: arm64: New PTE dirty-page encoding, HAFDBS new usage Leonardo Bras
` (2 preceding siblings ...)
2026-09-01 17:15 ` [RFC PATCH 3/5] KVM: arm64: Introduce a dedicated walker for stage2 write-protect Leonardo Bras
@ 2026-09-01 17:15 ` Leonardo Bras
2026-09-02 3:41 ` Tian Zheng
2026-09-01 17:15 ` [RFC PATCH 5/5] KVM: arm64: Enable HAFDBS for guests not on migration Leonardo Bras
2026-09-12 12:24 ` [RFC PATCH 0/5] KVM: arm64: New PTE dirty-page encoding, HAFDBS new usage Marc Zyngier
5 siblings, 1 reply; 22+ messages in thread
From: Leonardo Bras @ 2026-09-01 17:15 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Fuad Tabba, Joey Gouly,
Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Leonardo Bras, Raghavendra Rao Ananta,
Tian Zheng
Cc: linux-arm-kernel, kvmarm, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH 4/5] KVM: arm64: Add KVM_REQ_RELOAD_STAGE2
2026-09-01 17:15 ` [RFC PATCH 4/5] KVM: arm64: Add KVM_REQ_RELOAD_STAGE2 Leonardo Bras
@ 2026-09-02 3:41 ` Tian Zheng
2026-09-02 10:53 ` Leonardo Bras
0 siblings, 1 reply; 22+ messages in thread
From: Tian Zheng @ 2026-09-02 3:41 UTC (permalink / raw)
To: Leonardo Bras, Marc Zyngier, Oliver Upton, Fuad Tabba,
Joey Gouly, Steffen Eiden, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Mark Rutland,
Raghavendra Rao Ananta
Cc: linux-arm-kernel, kvmarm, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH 4/5] KVM: arm64: Add KVM_REQ_RELOAD_STAGE2
2026-09-02 3:41 ` Tian Zheng
@ 2026-09-02 10:53 ` Leonardo Bras
0 siblings, 0 replies; 22+ messages in thread
From: Leonardo Bras @ 2026-09-02 10:53 UTC (permalink / raw)
To: Tian Zheng
Cc: Leonardo Bras, Marc Zyngier, Oliver Upton, Fuad Tabba,
Joey Gouly, Steffen Eiden, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Mark Rutland,
Raghavendra Rao Ananta, linux-arm-kernel, kvmarm, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC PATCH 5/5] KVM: arm64: Enable HAFDBS for guests not on migration
2026-09-01 17:15 [RFC PATCH 0/5] KVM: arm64: New PTE dirty-page encoding, HAFDBS new usage Leonardo Bras
` (3 preceding siblings ...)
2026-09-01 17:15 ` [RFC PATCH 4/5] KVM: arm64: Add KVM_REQ_RELOAD_STAGE2 Leonardo Bras
@ 2026-09-01 17:15 ` Leonardo Bras
2026-09-16 0:10 ` Oliver Upton
2026-09-12 12:24 ` [RFC PATCH 0/5] KVM: arm64: New PTE dirty-page encoding, HAFDBS new usage Marc Zyngier
5 siblings, 1 reply; 22+ messages in thread
From: Leonardo Bras @ 2026-09-01 17:15 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Fuad Tabba, Joey Gouly,
Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Leonardo Bras, Raghavendra Rao Ananta,
Tian Zheng
Cc: linux-arm-kernel, kvmarm, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH 5/5] KVM: arm64: Enable HAFDBS for guests not on migration
2026-09-01 17:15 ` [RFC PATCH 5/5] KVM: arm64: Enable HAFDBS for guests not on migration Leonardo Bras
@ 2026-09-16 0:10 ` Oliver Upton
2026-09-16 14:00 ` Leonardo Bras
0 siblings, 1 reply; 22+ messages in thread
From: Oliver Upton @ 2026-09-16 0:10 UTC (permalink / raw)
To: Leonardo Bras
Cc: Marc Zyngier, Fuad Tabba, Joey Gouly, Steffen Eiden,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
Mark Rutland, Raghavendra Rao Ananta, Tian Zheng,
linux-arm-kernel, kvmarm, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH 5/5] KVM: arm64: Enable HAFDBS for guests not on migration
2026-09-16 0:10 ` Oliver Upton
@ 2026-09-16 14:00 ` Leonardo Bras
0 siblings, 0 replies; 22+ messages in thread
From: Leonardo Bras @ 2026-09-16 14:00 UTC (permalink / raw)
To: Oliver Upton
Cc: Leonardo Bras, Marc Zyngier, Fuad Tabba, Joey Gouly,
Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Raghavendra Rao Ananta, Tian Zheng,
linux-arm-kernel, kvmarm, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH 0/5] KVM: arm64: New PTE dirty-page encoding, HAFDBS new usage
2026-09-01 17:15 [RFC PATCH 0/5] KVM: arm64: New PTE dirty-page encoding, HAFDBS new usage Leonardo Bras
` (4 preceding siblings ...)
2026-09-01 17:15 ` [RFC PATCH 5/5] KVM: arm64: Enable HAFDBS for guests not on migration Leonardo Bras
@ 2026-09-12 12:24 ` Marc Zyngier
2026-09-15 15:31 ` Leonardo Bras
5 siblings, 1 reply; 22+ messages in thread
From: Marc Zyngier @ 2026-09-12 12:24 UTC (permalink / raw)
To: Leonardo Bras
Cc: Oliver Upton, Fuad Tabba, Joey Gouly, Steffen Eiden,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
Mark Rutland, Raghavendra Rao Ananta, Tian Zheng,
linux-arm-kernel, kvmarm, linux-kernel
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.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH 0/5] KVM: arm64: New PTE dirty-page encoding, HAFDBS new usage
2026-09-12 12:24 ` [RFC PATCH 0/5] KVM: arm64: New PTE dirty-page encoding, HAFDBS new usage Marc Zyngier
@ 2026-09-15 15:31 ` Leonardo Bras
0 siblings, 0 replies; 22+ messages in thread
From: Leonardo Bras @ 2026-09-15 15:31 UTC (permalink / raw)
To: Marc Zyngier
Cc: Leonardo Bras, Oliver Upton, Fuad Tabba, Joey Gouly,
Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas,
Will Deacon, Mark Rutland, Raghavendra Rao Ananta, Tian Zheng,
linux-arm-kernel, kvmarm, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread