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

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

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