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

[Tian Zheng <zhengtian10@huawei.com>]

On 9/16/2026 7:22 PM, Leonardo Bras 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.
> 
> 

Hi Oliver, Leo,

Works for us. I plan to send HDBSS v5 maybe next week with this series
merged in. Both dirty-tracking consumers are already built on top of the
DBM approach: dirty ring and dirty bitmap.

Leo, with your blessing, I'd like to pick patches 1-4 into the HDBSS
tree with your Signed-off-by preserved and mine added on top, plus some
bug fixes on top of this RFC series.

For patch 5, I'd like to rework it into a derived hardware dirty mode
that replaces both kvm_set_hafdbs() and our earlier HDBSS enable/disable
hooks, so the whole thing lands as one series.

Performance looks good in both dirty ring and dirty bitmap scenarios so
far.

Thanks,
Tian

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