only CLEAR PTE_RDONLY, never set it None of these create PTE_DIRTY | PTE_WRITE | PTE_RDONLY. The invariant holds: for writable pages, PTE_DIRTY => !PTE_RDONLY. Tracing the RDONLY|DBM scenario through hugetlb_fault() For the specific scenario: all sub-PTEs are PTE_WRITE | PTE_RDONLY (RDONLY|DBM, clean writable), and handle_mm_fault() is called: 1. huge_ptep_get() gathers – no dirty siblings, so vmf.orig_pte = PTE_WRITE | PTE_RDONLY | PTE_AF 2. huge_pte_mkdirty() sets PTE_DIRTY, clears PTE_RDONLY 3. entry = PTE_WRITE | PTE_DIRTY | !PTE_RDONLY | PTE_AF 4. __cont_access_flags_changed(): - All sub-PTEs: PTE_WRITE | PTE_RDONLY | PTE_AF - pte_dirty(entry) = true, pte_dirty(sub) = false (RDONLY set, no PTE_DIRTY) - Mismatch detected -> returns 1 -> BBM proceeds 5. BBM rewrites ALL PTEs with PTE_WRITE | PTE_DIRTY | !PTE_RDONLY | PTE_AF 6. RDONLY cleared for all entries. Correct. With some siblings HW-dirtied by CPU (e.g., PTE #3 has !PTE_RDONLY): 1. huge_ptep_get() gathers: sees HW-dirty sibling, calls pte_mkdirty() on gathered 2. entry = PTE_WRITE | PTE_DIRTY | !PTE_RDONLY | PTE_AF 3. __cont_access_flags_changed(): - PTE #3 (!PTE_RDONLY): pte_dirty() = true -> match - PTE #0 (PTE_RDONLY): pte_dirty() = false -> mismatch -> returns 1 4. BBM proceeds. All PTEs rewritten. Correct. With ALL siblings HW-dirtied by CPU: 1. All sub-PTEs: PTE_WRITE | !PTE_RDONLY | PTE_AF (all HW dirty) 2. entry: PTE_WRITE | PTE_DIRTY | !PTE_RDONLY | PTE_AF 3. __cont_access_flags_changed(): all pte_dirty() match -> returns 0 (no-op) 4. But RDONLY is already clear on all PTEs, so SMMU won’t fault. Correct. Conclusion ========== The hugetlb code does NOT have the same bug in the sense that it doesn’t produce false no-ops with current code. The key difference is __cont_access_flags_changed() checks each sub-PTE individually, not a gathered view. However, Ryan Roberts’ observation is valid: __cont_access_flags_changed() has a latent weakness. It relies on the invariant PTE_DIRTY => !PTE_RDONLY (for writable pages) rather than checking PTE_RDONLY directly. If this invariant were ever violated (e.g., by a future code change), the no-op check could falsely pass, leaving RDONLY set in hardware and causing SMMU fault loops. The contpte fix’s approach of checking raw bit values is more robust and should be adopted by the hugetlb code as a defense-in-depth measure. The recommended fix for hugetlb: replace the pte_dirty()/pte_young() checks in __cont_access_flags_changed() with a raw-bitmask comparison similar to contpte_all_subptes_match_access_flags(), using the mask PTE_RDONLY | PTE_AF | PTE_WRITE | PTE_DIRTY. diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c index a42c05cf564082..34e091b398123e 100644 --- a/arch/arm64/mm/hugetlbpage.c +++ b/arch/arm64/mm/hugetlbpage.c @@ -389,28 +389,24 @@ pte_t huge_ptep_get_and_clear(struct mm_struct *mm, unsigned long addr, } /* - * huge_ptep_set_access_flags will update access flags (dirty, accesssed) + * huge_ptep_set_access_flags will update access flags (dirty, accessed) * and write permission. * - * For a contiguous huge pte range we need to check whether or not write - * permission has to change only on the first pte in the set. Then for - * all the contiguous ptes we need to check whether or not there is a - * discrepancy between dirty or young. + * Check all sub-PTEs' raw access flag bits rather than using the abstracted + * pte_dirty()/pte_young() helpers which conflate HW-dirty and SW-dirty. + * This ensures PTE_RDONLY is checked directly: a sub-PTE that is SW-dirty + * (PTE_DIRTY set) but still has PTE_RDONLY would be missed by pte_dirty() + * but will cause an SMMU without HTTU to keep faulting. The access flag + * mask matches the one used by __ptep_set_access_flags(). */ static int __cont_access_flags_changed(pte_t *ptep, pte_t pte, int ncontig) { + const pteval_t access_mask = PTE_RDONLY | PTE_AF | PTE_WRITE | PTE_DIRTY; + pteval_t pte_access = pte_val(pte) & access_mask; int i; - if (pte_write(pte) != pte_write(__ptep_get(ptep))) - return 1; - for (i = 0; i < ncontig; i++) { - pte_t orig_pte = __ptep_get(ptep + i); - - if (pte_dirty(pte) != pte_dirty(orig_pte)) - return 1; - - if (pte_young(pte) != pte_young(orig_pte)) + if ((pte_val(__ptep_get(ptep + i)) & access_mask) != pte_access) return 1; } Jason[PATCH] arm64: contpte: fix set_access_flags() no-op check for SMMU/ATS faultsJason Gunthorpe undefinedPiotr Jaroszynski undefined undefined undefined undefined undefined undefined undefined undefined undefined undefined undefined&¹<