When converting a region via contpte_convert() to use mTHP, we have two
different goals. We have to mark each entry as contiguous, and we would
like to smear the dirty and young (access) bits across all entries in
the contiguous block. Currently, we do this by first accumulating the
dirty and young bits in the block, using an atomic
__ptep_get_and_clear() and the relevant pte_{dirty,young}() calls,
performing a tlbi, and finally smearing the correct bits across the
block using __set_ptes().

This approach works fine for BBM level 0, but with support for BBM level
2 we are allowed to reorder the tlbi to after setting the pagetable
entries. We expect the time cost of a tlbi to be much greater than the
cost of clearing and resetting the PTEs. As such, this reordering of the
tlbi outside the window where our PTEs are invalid greatly reduces the
duration the PTE are visibly invalid for other threads. This reduces the
likelyhood of a concurrent page walk finding an invalid PTE, reducing
the likelyhood of a fault in other threads, and improving performance
(more so when there are more threads).

Because we support via allowlist only bbml2 implementations that never
raise conflict aborts and instead invalidate the tlb entries
automatically in hardware, we can avoid the final flush altogether.

However, avoiding the intermediate tlbi+dsb must be carefully considered
to ensure that we remain both correct and performant. We document our
reasoning and the expected interactions further in the contpte_convert()
source. To do so we rely on the aarch64 spec (DDI 0487L.a D8.7.1.1)
requirements RNGLXZ and RJQQTC to provide guarantees that the elision is
correct.

Signed-off-by: Mikołaj Lenczewski <miko.lenczewski@arm.com>
Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>
---
 arch/arm64/mm/contpte.c | 139 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 138 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c
index bcac4f55f9c1..203357061d0a 100644
--- a/arch/arm64/mm/contpte.c
+++ b/arch/arm64/mm/contpte.c
@@ -68,7 +68,144 @@ static void contpte_convert(struct mm_struct *mm, unsigned long addr,
 			pte = pte_mkyoung(pte);
 	}
 
-	__flush_tlb_range(&vma, start_addr, addr, PAGE_SIZE, true, 3);
+	/*
+	 * On eliding the __tlb_flush_range() under BBML2+noabort:
+	 *
+	 * NOTE: Instead of using N=16 as the contiguous block length, we use
+	 *       N=4 for clarity.
+	 *
+	 * NOTE: 'n' and 'c' are used to denote the "contiguous bit" being
+	 *       unset and set, respectively.
+	 *
+	 * We worry about two cases where contiguous bit is used:
+	 *  - When folding N smaller non-contiguous ptes as 1 contiguous block.
+	 *  - When unfolding a contiguous block into N smaller non-contiguous ptes.
+	 *
+	 * Currently, the BBML0 folding case looks as follows:
+	 *
+	 *  0) Initial page-table layout:
+	 *
+	 *   +----+----+----+----+
+	 *   |RO,n|RO,n|RO,n|RW,n| <--- last page being set as RO
+	 *   +----+----+----+----+
+	 *
+	 *  1) Aggregate AF + dirty flags using __ptep_get_and_clear():
+	 *
+	 *   +----+----+----+----+
+	 *   |  0 |  0 |  0 |  0 |
+	 *   +----+----+----+----+
+	 *
+	 *  2) __flush_tlb_range():
+	 *
+	 *   |____ tlbi + dsb ____|
+	 *
+	 *  3) __set_ptes() to repaint contiguous block:
+	 *
+	 *   +----+----+----+----+
+	 *   |RO,c|RO,c|RO,c|RO,c|
+	 *   +----+----+----+----+
+	 *
+	 *  4) The kernel will eventually __flush_tlb() for changed page:
+	 *
+	 *                  |____| <--- tlbi + dsb
+	 *
+	 * As expected, the intermediate tlbi+dsb ensures that other PEs
+	 * only ever see an invalid (0) entry, or the new contiguous TLB entry.
+	 * The final tlbi+dsb will always throw away the newly installed
+	 * contiguous TLB entry, which is a micro-optimisation opportunity,
+	 * but does not affect correctness.
+	 *
+	 * In the BBML2 case, the change is avoiding the intermediate tlbi+dsb.
+	 * This means a few things, but notably other PEs will still "see" any
+	 * stale cached TLB entries. This could lead to a "contiguous bit
+	 * misprogramming" issue until the final tlbi+dsb of the changed page,
+	 * which would clear out both the stale (RW,n) entry and the new (RO,c)
+	 * contiguous entry installed in its place.
+	 *
+	 * What this is saying, is the following:
+	 *
+	 *  +----+----+----+----+
+	 *  |RO,n|RO,n|RO,n|RW,n| <--- old page tables, all non-contiguous
+	 *  +----+----+----+----+
+	 *
+	 *  +----+----+----+----+
+	 *  |RO,c|RO,c|RO,c|RO,c| <--- new page tables, all contiguous
+	 *  +----+----+----+----+
+	 *   /\
+	 *   ||
+	 *
+	 *  If both the old single (RW,n) and new contiguous (RO,c) TLB entries
+	 *  are present, and a write is made to this address, do we fault or
+	 *  is the write permitted (via amalgamation)?
+	 *
+	 * The relevant Arm ARM DDI 0487L.a requirements are RNGLXZ and RJQQTC,
+	 * and together state that when BBML1 or BBML2 are implemented, either
+	 * a TLB conflict abort is raised (which we expressly forbid), or will
+	 * "produce an OA, access permissions, and memory attributes that are
+	 * consistent with any of the programmed translation table values".
+	 *
+	 * That is to say, will either raise a TLB conflict, or produce one of
+	 * the cached TLB entries, but never amalgamate.
+	 *
+	 * Thus, as the page tables are only considered "consistent" after
+	 * the final tlbi+dsb (which evicts both the single stale (RW,n) TLB
+	 * entry as well as the new contiguous (RO,c) TLB entry), omitting the
+	 * initial tlbi+dsb is correct.
+	 *
+	 * It is also important to note that at the end of the BBML2 folding
+	 * case, we are still left with potentially all N TLB entries still
+	 * cached (the N-1 non-contiguous ptes, and the single contiguous
+	 * block). However, over time, natural TLB pressure will cause the
+	 * non-contiguous pte TLB entries to be flushed, leaving only the
+	 * contiguous block TLB entry. This means that omitting the tlbi+dsb is
+	 * not only correct, but also keeps our eventual performance benefits.
+	 *
+	 * For the unfolding case, BBML0 looks as follows:
+	 *
+	 *  0) Initial page-table layout:
+	 *
+	 *   +----+----+----+----+
+	 *   |RW,c|RW,c|RW,c|RW,c| <--- last page being set as RO
+	 *   +----+----+----+----+
+	 *
+	 *  1) Aggregate AF + dirty flags using __ptep_get_and_clear():
+	 *
+	 *   +----+----+----+----+
+	 *   |  0 |  0 |  0 |  0 |
+	 *   +----+----+----+----+
+	 *
+	 *  2) __flush_tlb_range():
+	 *
+	 *   |____ tlbi + dsb ____|
+	 *
+	 *  3) __set_ptes() to repaint as non-contiguous:
+	 *
+	 *   +----+----+----+----+
+	 *   |RW,n|RW,n|RW,n|RW,n|
+	 *   +----+----+----+----+
+	 *
+	 *  4) Update changed page permissions:
+	 *
+	 *   +----+----+----+----+
+	 *   |RW,n|RW,n|RW,n|RO,n| <--- last page permissions set
+	 *   +----+----+----+----+
+	 *
+	 *  5) The kernel will eventually __flush_tlb() for changed page:
+	 *
+	 *                  |____| <--- tlbi + dsb
+	 *
+	 * For BBML2, we again remove the intermediate tlbi+dsb. Here, there
+	 * are no issues, as the final tlbi+dsb covering the changed page is
+	 * guaranteed to remove the original large contiguous (RW,c) TLB entry,
+	 * as well as the intermediate (RW,n) TLB entry; the next access will
+	 * install the new (RO,n) TLB entry and the page tables are only
+	 * considered "consistent" after the final tlbi+dsb, so software must
+	 * be prepared for this inconsistency prior to finishing the mm dance
+	 * regardless.
+	 */
+
+	if (!system_supports_bbml2_noabort())
+		__flush_tlb_range(&vma, start_addr, addr, PAGE_SIZE, true, 3);
 
 	__set_ptes(mm, start_addr, start_ptep, pte, CONT_PTES);
 }
-- 
2.49.0

On Tue, Jun 17, 2025 at 09:51:04AM +0000, Mikołaj Lenczewski wrote:
> diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c
> index bcac4f55f9c1..203357061d0a 100644
> --- a/arch/arm64/mm/contpte.c
> +++ b/arch/arm64/mm/contpte.c
> @@ -68,7 +68,144 @@ static void contpte_convert(struct mm_struct *mm, unsigned long addr,
>  			pte = pte_mkyoung(pte);
>  	}
>  
> -	__flush_tlb_range(&vma, start_addr, addr, PAGE_SIZE, true, 3);
> +	/*
> +	 * On eliding the __tlb_flush_range() under BBML2+noabort:
> +	 *
> +	 * NOTE: Instead of using N=16 as the contiguous block length, we use
> +	 *       N=4 for clarity.
> +	 *
> +	 * NOTE: 'n' and 'c' are used to denote the "contiguous bit" being
> +	 *       unset and set, respectively.
> +	 *
> +	 * We worry about two cases where contiguous bit is used:
> +	 *  - When folding N smaller non-contiguous ptes as 1 contiguous block.
> +	 *  - When unfolding a contiguous block into N smaller non-contiguous ptes.
> +	 *
> +	 * Currently, the BBML0 folding case looks as follows:
> +	 *
> +	 *  0) Initial page-table layout:
> +	 *
> +	 *   +----+----+----+----+
> +	 *   |RO,n|RO,n|RO,n|RW,n| <--- last page being set as RO
> +	 *   +----+----+----+----+
> +	 *
> +	 *  1) Aggregate AF + dirty flags using __ptep_get_and_clear():
> +	 *
> +	 *   +----+----+----+----+
> +	 *   |  0 |  0 |  0 |  0 |
> +	 *   +----+----+----+----+
> +	 *
> +	 *  2) __flush_tlb_range():
> +	 *
> +	 *   |____ tlbi + dsb ____|
> +	 *
> +	 *  3) __set_ptes() to repaint contiguous block:
> +	 *
> +	 *   +----+----+----+----+
> +	 *   |RO,c|RO,c|RO,c|RO,c|
> +	 *   +----+----+----+----+

From the initial layout to point (3), we are also changing the
permission. Given the rules you mentioned in the Arm ARM, I think that's
safe (hardware seeing either the old or the new attributes). The
FEAT_BBM description, however, only talks about change between larger
and smaller blocks but no mention of also changing the attributes at the
same time. Hopefully the microarchitects claiming certain CPUs don't
generate conflict aborts understood what Linux does.

> +	 *
> +	 *  4) The kernel will eventually __flush_tlb() for changed page:
> +	 *
> +	 *                  |____| <--- tlbi + dsb
[...]
> +	 * It is also important to note that at the end of the BBML2 folding
> +	 * case, we are still left with potentially all N TLB entries still
> +	 * cached (the N-1 non-contiguous ptes, and the single contiguous
> +	 * block). However, over time, natural TLB pressure will cause the
> +	 * non-contiguous pte TLB entries to be flushed, leaving only the
> +	 * contiguous block TLB entry. This means that omitting the tlbi+dsb is
> +	 * not only correct, but also keeps our eventual performance benefits.

Step 4 above implies some TLB flushing from the core code eventually.
What is the situation mentioned in the paragraph above? Is it only until
we get the TLB flushing from the core code?

[...]
> +	if (!system_supports_bbml2_noabort())
> +		__flush_tlb_range(&vma, start_addr, addr, PAGE_SIZE, true, 3);
>  
>  	__set_ptes(mm, start_addr, start_ptep, pte, CONT_PTES);

Eliding the TLBI here is all good but looking at the overall set_ptes(),
why do we bother with unfold+fold for BBML2? Can we not just change
them in place without going through __ptep_get_and_clear()?

-- 
Catalin

On 19/06/2025 20:29, Catalin Marinas wrote:
> On Tue, Jun 17, 2025 at 09:51:04AM +0000, Mikołaj Lenczewski wrote:
>> diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c
>> index bcac4f55f9c1..203357061d0a 100644
>> --- a/arch/arm64/mm/contpte.c
>> +++ b/arch/arm64/mm/contpte.c
>> @@ -68,7 +68,144 @@ static void contpte_convert(struct mm_struct *mm, unsigned long addr,
>>  			pte = pte_mkyoung(pte);
>>  	}
>>  
>> -	__flush_tlb_range(&vma, start_addr, addr, PAGE_SIZE, true, 3);
>> +	/*
>> +	 * On eliding the __tlb_flush_range() under BBML2+noabort:
>> +	 *
>> +	 * NOTE: Instead of using N=16 as the contiguous block length, we use
>> +	 *       N=4 for clarity.
>> +	 *
>> +	 * NOTE: 'n' and 'c' are used to denote the "contiguous bit" being
>> +	 *       unset and set, respectively.
>> +	 *
>> +	 * We worry about two cases where contiguous bit is used:
>> +	 *  - When folding N smaller non-contiguous ptes as 1 contiguous block.
>> +	 *  - When unfolding a contiguous block into N smaller non-contiguous ptes.
>> +	 *
>> +	 * Currently, the BBML0 folding case looks as follows:
>> +	 *
>> +	 *  0) Initial page-table layout:
>> +	 *
>> +	 *   +----+----+----+----+
>> +	 *   |RO,n|RO,n|RO,n|RW,n| <--- last page being set as RO
>> +	 *   +----+----+----+----+
>> +	 *
>> +	 *  1) Aggregate AF + dirty flags using __ptep_get_and_clear():
>> +	 *
>> +	 *   +----+----+----+----+
>> +	 *   |  0 |  0 |  0 |  0 |
>> +	 *   +----+----+----+----+
>> +	 *
>> +	 *  2) __flush_tlb_range():
>> +	 *
>> +	 *   |____ tlbi + dsb ____|
>> +	 *
>> +	 *  3) __set_ptes() to repaint contiguous block:
>> +	 *
>> +	 *   +----+----+----+----+
>> +	 *   |RO,c|RO,c|RO,c|RO,c|
>> +	 *   +----+----+----+----+
> 
> From the initial layout to point (3), we are also changing the
> permission. Given the rules you mentioned in the Arm ARM, I think that's
> safe (hardware seeing either the old or the new attributes). The
> FEAT_BBM description, however, only talks about change between larger
> and smaller blocks but no mention of also changing the attributes at the
> same time. Hopefully the microarchitects claiming certain CPUs don't
> generate conflict aborts understood what Linux does.
> 
>> +	 *
>> +	 *  4) The kernel will eventually __flush_tlb() for changed page:
>> +	 *
>> +	 *                  |____| <--- tlbi + dsb
> [...]
>> +	 * It is also important to note that at the end of the BBML2 folding
>> +	 * case, we are still left with potentially all N TLB entries still
>> +	 * cached (the N-1 non-contiguous ptes, and the single contiguous
>> +	 * block). However, over time, natural TLB pressure will cause the
>> +	 * non-contiguous pte TLB entries to be flushed, leaving only the
>> +	 * contiguous block TLB entry. This means that omitting the tlbi+dsb is
>> +	 * not only correct, but also keeps our eventual performance benefits.
> 
> Step 4 above implies some TLB flushing from the core code eventually.
> What is the situation mentioned in the paragraph above? Is it only until
> we get the TLB flushing from the core code?

I think the point that Miko is making here is that at some point between step 3
and 4, we could end up with up to any 3 of the original small entries as well as
the large entry in the TLB. Then the flush at step 4 will only remove the small
entry at the last entry and the large entry - i.e. it will only remove the
entries that overlap the region that the tlbi targets, and up to 3 of the small
entries may be left in the TLB after step 4. Our rationale here was that this is
safe due to BBML2 and it will naturally sort itself out over time with natural
eviction.

> 
> [...]
>> +	if (!system_supports_bbml2_noabort())
>> +		__flush_tlb_range(&vma, start_addr, addr, PAGE_SIZE, true, 3);
>>  
>>  	__set_ptes(mm, start_addr, start_ptep, pte, CONT_PTES);
> 
> Eliding the TLBI here is all good but looking at the overall set_ptes(),
> why do we bother with unfold+fold for BBML2? Can we not just change
> them in place without going through __ptep_get_and_clear()?

We need to collect the access and dirty bits so we can apply the aggregate
across all entries. We can only accurately collect the access and dirty bits if
we go via invalid, otherwise the HW could update after we have read the old
entry but before we have written the new entry.

Thanks,
Ryan

On 20/06/2025 17:10, Ryan Roberts wrote:
> On 19/06/2025 20:29, Catalin Marinas wrote:
>> On Tue, Jun 17, 2025 at 09:51:04AM +0000, Mikołaj Lenczewski wrote:
>>> diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c
>>> index bcac4f55f9c1..203357061d0a 100644
>>> --- a/arch/arm64/mm/contpte.c
>>> +++ b/arch/arm64/mm/contpte.c
>>> @@ -68,7 +68,144 @@ static void contpte_convert(struct mm_struct *mm, unsigned long addr,
>>>  			pte = pte_mkyoung(pte);
>>>  	}
>>>  
>>> -	__flush_tlb_range(&vma, start_addr, addr, PAGE_SIZE, true, 3);
>>> +	/*
>>> +	 * On eliding the __tlb_flush_range() under BBML2+noabort:
>>> +	 *
>>> +	 * NOTE: Instead of using N=16 as the contiguous block length, we use
>>> +	 *       N=4 for clarity.
>>> +	 *
>>> +	 * NOTE: 'n' and 'c' are used to denote the "contiguous bit" being
>>> +	 *       unset and set, respectively.
>>> +	 *
>>> +	 * We worry about two cases where contiguous bit is used:
>>> +	 *  - When folding N smaller non-contiguous ptes as 1 contiguous block.
>>> +	 *  - When unfolding a contiguous block into N smaller non-contiguous ptes.
>>> +	 *
>>> +	 * Currently, the BBML0 folding case looks as follows:
>>> +	 *
>>> +	 *  0) Initial page-table layout:
>>> +	 *
>>> +	 *   +----+----+----+----+
>>> +	 *   |RO,n|RO,n|RO,n|RW,n| <--- last page being set as RO
>>> +	 *   +----+----+----+----+
>>> +	 *
>>> +	 *  1) Aggregate AF + dirty flags using __ptep_get_and_clear():
>>> +	 *
>>> +	 *   +----+----+----+----+
>>> +	 *   |  0 |  0 |  0 |  0 |
>>> +	 *   +----+----+----+----+
>>> +	 *
>>> +	 *  2) __flush_tlb_range():
>>> +	 *
>>> +	 *   |____ tlbi + dsb ____|
>>> +	 *
>>> +	 *  3) __set_ptes() to repaint contiguous block:
>>> +	 *
>>> +	 *   +----+----+----+----+
>>> +	 *   |RO,c|RO,c|RO,c|RO,c|
>>> +	 *   +----+----+----+----+
>>
>> From the initial layout to point (3), we are also changing the
>> permission. Given the rules you mentioned in the Arm ARM, I think that's
>> safe (hardware seeing either the old or the new attributes). The
>> FEAT_BBM description, however, only talks about change between larger
>> and smaller blocks but no mention of also changing the attributes at the
>> same time. Hopefully the microarchitects claiming certain CPUs don't
>> generate conflict aborts understood what Linux does.

I think what you are saying is that despite going via invalid, the HW may see
this direct transition:

+----+----+----+----+
|RO,n|RO,n|RO,n|RW,n|
+----+----+----+----+
to:
+----+----+----+----+
|RO,c|RO,c|RO,c|RO,c|
+----+----+----+----+

There are 2 logical operations here. The first is changing the permissions of
the last entry. The second is changing the size of the entry.

As I understand it, it's permissible in the architecture to update the
permissions of the a PTE without break-before-make and without issuing a tlbi
afterwards; in that case the HW may apply either the old permissions or the new
permissions up until a future tlbi (after which the new permissions are
guarranteed). That's the first logical operation.

The second logical operation is permitted by FEAT_BBM level 2.

So both logical operations are permitted and the Arm ARM doesn't mention any
requirement to "separate" these operations with a tlbi or anything else, as far
as I can see.

So I would interpret that combining these 2 in the way we have should be safe.
RNGLXZ and RJQQTC give further insight into the spirit of the spec. But I agree
this isn't spelled out super clearly.

Perhaps we can move forwards based on this understanding, and I will seek some
clarifying words to be added to the Arm ARM?

Thanks,
Ryan

On Wed, Jun 25, 2025 at 02:07:23PM +0100, Ryan Roberts wrote:
> I think what you are saying is that despite going via invalid, the HW may see
> this direct transition:
> 
> +----+----+----+----+
> |RO,n|RO,n|RO,n|RW,n|
> +----+----+----+----+
> to:
> +----+----+----+----+
> |RO,c|RO,c|RO,c|RO,c|
> +----+----+----+----+
> 
> There are 2 logical operations here. The first is changing the permissions of
> the last entry. The second is changing the size of the entry.
> 
> As I understand it, it's permissible in the architecture to update the
> permissions of the a PTE without break-before-make and without issuing a tlbi
> afterwards; in that case the HW may apply either the old permissions or the new
> permissions up until a future tlbi (after which the new permissions are
> guarranteed). That's the first logical operation.
> 
> The second logical operation is permitted by FEAT_BBM level 2.
> 
> So both logical operations are permitted and the Arm ARM doesn't mention any
> requirement to "separate" these operations with a tlbi or anything else, as far
> as I can see.
> 
> So I would interpret that combining these 2 in the way we have should be safe.
> RNGLXZ and RJQQTC give further insight into the spirit of the spec. But I agree
> this isn't spelled out super clearly.
> 
> Perhaps we can move forwards based on this understanding, and I will seek some
> clarifying words to be added to the Arm ARM?

FWIW this matches my understanding as well.

AFAIK the actual physical issue for BBM < 2 is the TLB lookup HW
cannot tolerate having two entries that overlap in address because of
different sizes. Every lookup must return exactly one result. If more
results are returned the HW fails in some way.

For BBM 2 if the HW lookup gets more than one result the HW selects
one unpredictably and uses that.

Permissions shouldn't have any impact because they do not effect how
the lookup is performed, they are the result of the lookup.

I've been looking at implementing BBM 2 support on the iommu side and
I didn't see anything that said we cannot do arbitary transformations
under BBM 2? For instance when degrading from a contiguous range of
block descriptors down to single page descriptors I'm expecting to go
like:
    16x2M -> 2M -> 4k -> FLUSH

Jason

On 25/06/2025 14:37, Jason Gunthorpe wrote:
> On Wed, Jun 25, 2025 at 02:07:23PM +0100, Ryan Roberts wrote:
>> I think what you are saying is that despite going via invalid, the HW may see
>> this direct transition:
>>
>> +----+----+----+----+
>> |RO,n|RO,n|RO,n|RW,n|
>> +----+----+----+----+
>> to:
>> +----+----+----+----+
>> |RO,c|RO,c|RO,c|RO,c|
>> +----+----+----+----+
>>
>> There are 2 logical operations here. The first is changing the permissions of
>> the last entry. The second is changing the size of the entry.
>>
>> As I understand it, it's permissible in the architecture to update the
>> permissions of the a PTE without break-before-make and without issuing a tlbi
>> afterwards; in that case the HW may apply either the old permissions or the new
>> permissions up until a future tlbi (after which the new permissions are
>> guarranteed). That's the first logical operation.
>>
>> The second logical operation is permitted by FEAT_BBM level 2.
>>
>> So both logical operations are permitted and the Arm ARM doesn't mention any
>> requirement to "separate" these operations with a tlbi or anything else, as far
>> as I can see.
>>
>> So I would interpret that combining these 2 in the way we have should be safe.
>> RNGLXZ and RJQQTC give further insight into the spirit of the spec. But I agree
>> this isn't spelled out super clearly.
>>
>> Perhaps we can move forwards based on this understanding, and I will seek some
>> clarifying words to be added to the Arm ARM?
> 
> FWIW this matches my understanding as well.
> 
> AFAIK the actual physical issue for BBM < 2 is the TLB lookup HW
> cannot tolerate having two entries that overlap in address because of
> different sizes. Every lookup must return exactly one result. If more
> results are returned the HW fails in some way.
> 
> For BBM 2 if the HW lookup gets more than one result the HW selects
> one unpredictably and uses that.
> 
> Permissions shouldn't have any impact because they do not effect how
> the lookup is performed, they are the result of the lookup.

Totally agree with all of this! But ideally we want the Arm ARM to be
unambiguous so that we can implement against that rather than against our
understanding of the HW issues and what the Arm ARM "almost certainly intended".

> 
> I've been looking at implementing BBM 2 support on the iommu side and
> I didn't see anything that said we cannot do arbitary transformations
> under BBM 2? For instance when degrading from a contiguous range of
> block descriptors down to single page descriptors I'm expecting to go
> like:
>     16x2M -> 2M -> 4k -> FLUSH

If this is just changing sizes, it's definitely, explcitly safe in the Arm ARM
(I haven't read the equivalent SMMU text). And there isn't a requirement to
flush at the end. You only need to flush the (sub-)ranges where permissions or
whatever have changed.

> 
> Jason

On Thu, Jun 19, 2025 at 08:29:24PM +0100, Catalin Marinas wrote:
> On Tue, Jun 17, 2025 at 09:51:04AM +0000, Mikołaj Lenczewski wrote:
> > +	if (!system_supports_bbml2_noabort())
> > +		__flush_tlb_range(&vma, start_addr, addr, PAGE_SIZE, true, 3);
> >  
> >  	__set_ptes(mm, start_addr, start_ptep, pte, CONT_PTES);
> 
> Eliding the TLBI here is all good but looking at the overall set_ptes(),
> why do we bother with unfold+fold for BBML2? Can we not just change
> them in place without going through __ptep_get_and_clear()?

Ah, it's unlikely that we'd be able to fold them back if only one pte in
the range was modified, so this optimisation would very rarely/never
trigger. So, for this patch:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>