Re: [RFC PATCH v2 0/3] Set DBM bit on writable pages

[Leonardo Bras <leo.bras@arm.com>]

On Tue, Oct 27, 2026 at 04:19:35PM -0700, Oliver Upton wrote:
> On Tue, Oct 27, 2026 at 11:44:15AM +0000, Leonardo Bras wrote:
> > > I'm pretty sure I shared this feedback before, but patches that enable
> > > DBM do not make sense in isolation. We need a single series that
> > > contains the DBM enablement as well as the corresponding HDBSS support.
> > >
> > 
> > Right, 
> > My reasoning is that we can discuss this new version, and make the needed 
> > changes, so Tian could pick this for his own patch series for the next 
> > version. I do not intend to get this merged by itself.
> > 
> > Next time I will make sure to state this in the cover letter so it's clear.
> 
> Can you just work this out off-list? Let's keep the on-list dialogue
> consolidated to a single series to review.

Sure, will try to reduce this noise in kvmarm.

> 
> > > The abstract permission flags need to have a WRITE an DIRTY state, which
> > > we then map to hardware based on the capabilities of the system.
> > > 
> > > !FEAT_HAFDBS:
> > > 
> > > 	DIRTY	=> S2AP[1]
> > > 	WRITE	=> S2AP[1]
> > > 
> > > FEAT_HAFDBS:
> > > 
> > > 	DIRTY	=> S2AP[1]
> > > 	WRITE	=> DBM
> > > 
> > 
> > I agree that it could work, and my first version was based on that.
> > 
> > But looks weird to me that we have to decide the writable bit based on 
> > having a feature enabled.
> > 
> > Take in account that we can only have DBM being the writable bit while 
> > HAFDBS is enabled (not only present), and that it is just enabled during 
> > dirty-tracking. 
> > 
> > It means we would need to convert the whole pagetable between DIRTY=S2AP[1] 
> > and DIRTY=DBM every time we enable or disable dirty-tracking.
> > 
> > My suggestion with this series is to have WRITE=DIRTY=S2AP[1] at all 
> > times, and have DBM to act as a "can be promoted to writable on write" bit.
> > This way we don't actually need to convert between encodings, and it still 
> > works and complies with the architecture.
> 
> There's no such "can be promoted to writable on write" in the indirect
> permission model.

Well, IIUC it's kind of the default behavior, right?
Direct - DBM means HW can promote that page RO->RW, with HAFDBS=on
S2PIE  - All pages can be promoted RO->RW, as long as HAFDBS=on

> This is why I'm suggesting that we align the abstract
> permission bits (i.e. KVM_PGTABLE_PROT_*) with the architecture's own
> abstracted permissions (e.g. uX, pX, RO, RW).
> 

I think the approach you suggest makes sense, and it makes it simpler to 
deal with permission indirection in the future. In order to do like that, 
while keeping a single encoding for direct permission, I suggest we have:

-	KVM_PGTABLE_PROT_W			= BIT(2),
+	KVM_PGTABLE_PROT_DIRTY			= BIT(2),
...
+	KVM_PGTABLE_PROT_DBM			= BIT(6),
...

+#define KVM_PGTABLE_PROT_W	(KVM_PGTABLE_PROT_DBM | 
+				 KVM_PGTABLE_PROT_DIRTY)

(Rationale is: it's writable if it's either dirty, or can be made dirty by 
hardware on a write)

And then replacing by PROT_DIRTY the uses of PROT_W which were only 
tracking the fact it's dirty. I tracked just a couple of those.

When detecting the PROT_* based in PTE encoding, we track PROT_DIRTY, and 
PROT_DBM individually. The same can be done in nested, as we can just 
ignore the PROT_DBM for shadow pagetables, given we want, IIUC, to fault 
and update the original pagetables.

That would avoid switching between encodings while toggling HACDBS, and 
while making use of the DBM without directly associating it with the write 
permission.

What do you think?

> That composes nicely with nested virt as it is unambiguous what
> permissions we're trying to represent in a shadow stage-2. And in the
> non-nested case you can just lay down writable-dirty translations for
> most things and use writable-clean when necessary for HDBSS.
> 
> > > Beyond making the whole HAFDBS easy to reason about, I want this for
> > > FEAT_S2PIE which reimagines bit 7 as a literal dirty bit that can be
> > > managed by software or hardware.
> > 
> > Humm, and would not that work with the above suggested approach?
> > I mean, bit 7 (Dirty, S2AP[1]) will both mean WRITE/DIRTY, but it will be 
> > able to get promoted (nonW->W, or clean->dirty) based on the indirect 
> > permissions.
> 
> There's a bit more though. We need to have an entry in S2PIR_EL2 that
> encodes write permission and select that with PIIndex. So then we wind
> up mapping the abstract WRITE permission bit to two fields into the
> descriptor rather than 1.

Makes sense to me now.

Sorry I took long to reply your message, I was reading the docs for S2PIE 
and thinking on a solution that would make sense for it in the future as 
well.

I hope it's enough, but if it's not, let me know of any suggestions or 
requisites.

Thanks!
Leo