Re: [PATCH] arm64: contpte: fix set_access_flags() no-op check for SMMU/ATS faults

[Catalin Marinas <catalin.marinas@arm.com>]

On Wed, Mar 04, 2026 at 09:43:13AM -0400, Jason Gunthorpe wrote:
> On Wed, Mar 04, 2026 at 11:17:08AM +0000, Catalin Marinas wrote:
> > > @@ -399,13 +416,35 @@ int contpte_ptep_set_access_flags(struct vm_area_struct *vma,
> > >  	int i;
> > >  
> > >  	/*
> > > -	 * Gather the access/dirty bits for the contiguous range. If nothing has
> > > -	 * changed, its a noop.
> > > +	 * Check whether all sub-PTEs in the CONT block already have the
> > > +	 * requested access flags, using raw per-PTE values rather than the
> > > +	 * gathered ptep_get() view.
> > > +	 *
> > > +	 * ptep_get() gathers AF/dirty state across the whole CONT block,
> > > +	 * which is correct for CPU TLB semantics: with FEAT_HAFDBS the
> > > +	 * hardware may set AF/dirty on any sub-PTE and the CPU TLB treats
> > > +	 * the gathered result as authoritative for the entire range. But an
> > > +	 * SMMU without HTTU (or with HA/HD disabled in CD.TCR) evaluates
> > > +	 * each descriptor individually and will keep faulting on the target
> > > +	 * sub-PTE if its flags haven't actually been updated. Gathering can
> > > +	 * therefore cause false no-ops when only a sibling has been updated:
> > > +	 *  - write faults: target still has PTE_RDONLY (needs PTE_RDONLY cleared)
> > > +	 *  - read faults:  target still lacks PTE_AF
> > > +	 *
> > > +	 * Per Arm ARM (DDI 0487) D8.7.1, any sub-PTE in a CONT range may
> > > +	 * become the effective cached translation, so all entries must have
> > > +	 * consistent attributes. Check the full CONT block before returning
> > > +	 * no-op, and when any sub-PTE mismatches, proceed to update the whole
> > > +	 * range.
> > >  	 */
> > > -	orig_pte = pte_mknoncont(ptep_get(ptep));
> > > -	if (pte_val(orig_pte) == pte_val(entry))
> > > +	if (contpte_all_subptes_match_access_flags(ptep, entry))
> > >  		return 0;
> > 
> > Actually, do we need to loop over all the ptes? I think it sufficient to
> > only check the one at ptep since it is the one that triggered the
> > fault.
> 
> With CONT we should not be thinking "the one that triggered the
> fault".
> 
> The PTE that triggered the fault is the PTE that the HW happened to
> load into the TLB, we cannot assume it is the sub PTE we are faulting
> at. For instance it could be a sub PTE for a completely unrelated
> access at a different VA that got cached.

Good point. For the AF bit, the hardware is not allowed to cache it in
the TLB, so we can't get an AF fault for an unrelated VA nearby. We can,
however, for the dirty bit since PTE_RDONLY is allowed to be cached in
the TLB.

> Again, the requirement here is that a fault on a CONT PTE must fix all
> the access flags to be consistent or fail. It cannot resume the fault
> and leave the sub PTEs inconsistent as the HW is always allowed to
> load the RDONLY one for any access to the CONT.

It should fix all of them to be consistent if it got a fault. I was
wondering whether we can simplify this with a single pte read (but still
setting all in the range). It only works for the AF bit, not dirty. We
could add a check if it makes things slightly faster on this path.

Now I also wonder if the `pte_write(orig_pte) == pte_write(entry)` check
to elide BBM is still valid if we have hardware that does not support
DBM. I need to dig some more into the Arm ARM.

-- 
Catalin