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

[Leonardo Bras]

On Mon, Oct 26, 2026 at 12:08:59PM -0700, Oliver Upton wrote:
> On Mon, Oct 26, 2026 at 02:44:06PM +0000, Leonardo Bras wrote:
> > When HAFDBS is enabled (VTCR_EL2.{HA,HD}={1,1}), having a DBM bit allows 
> > hardware to promote a non-writable page to writable by setting (S2AP[1]=1),
> > without having to take a fault.
> > 
> > The reason for this patchset is to add the DBM bit when setting-up writable 
> > pages, so the code is prepared for future patches that make use of 
> > HDBSS/HACDBS to accelerate dirty-tracking. Write-bit detection still 
> > happens based only in S2AP[1], so no change in that aspect.
> > 
> > Having DBM set on block entries is undesired, as we benefit from the first 
> > fault for splitting it in PAGE_SIZE entries, which is the granularity used 
> > to track the dirty pages in KVM. So introduce a new walker that, on top of 
> > clearing S2AP[1] for every entry, also clear DBM for block entries.
> > 
> > Also introduce a new VCPU request to reload Stage2, as it will be needed on 
> > future HDBSS/HAFDBS patchsets. 
> > 
> > Please let me know of what do you think!
> 
> 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.
 
> > Changes since RFCv1:
> > - Dropped patch #5 on enabling HAFDBS on non-dirty-tracking guests, as it 
> >   proved to be counter-productive on wrprotect, contrary to expectations
> > - No more moving writable-bit to DBM. That does not seem to be the intended 
> >   use. It makes sense and it's easier to convert code to new DBM approach.
> 
> Sorry, where was this concluded?
> 

Sorry, this is my viewpoint after re-reading the documentation and 
being really pedantic while trying to get a proper answer from the people 
responsible for keeping/improving it. 

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

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

What do you think?

Thanks for reviewing,
Leo