All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm: armv8: mmu: fix DCACHE_OFF incorrectly unmapping region
@ 2026-07-08 11:17 Akshay Belsare
  2026-07-08 11:47 ` Ilias Apalodimas
  2026-07-24  1:02 ` Tom Rini
  0 siblings, 2 replies; 4+ messages in thread
From: Akshay Belsare @ 2026-07-08 11:17 UTC (permalink / raw)
  To: u-boot, michal.simek
  Cc: git, padmarao.begari, Akshay Belsare, Tom Rini, Ilias Apalodimas,
	Anshul Dalal, Casey Connolly, Dhruva Gole, Simon Glass,
	Mark Kettenis

DCACHE_OFF is defined as (0 << 2) = 0, and PTE_TYPE_FAULT is defined
as (0 << 0) = 0. In mmu_set_region_dcache_behaviour(), the cache
attribute passed to set_regions() is computed as:
  attrs = PMD_ATTRINDX(option >> 2)
For DCACHE_OFF=0 this evaluates to PMD_ATTRINDX(0) = 0, which equals
PTE_TYPE_FAULT.

Commit 6468ca13ffd6f ("armv8: mmu: fix and optimise explicitly unmapping
regions") added an unmap path to set_one_region() that
triggers when attrs == PTE_TYPE_FAULT. Because DCACHE_OFF and
PTE_TYPE_FAULT share the same numerical value (0), any call to
mmu_set_region_dcache_behaviour() with DCACHE_OFF silently unmaps the
target region instead of changing its cache attributes to non-cached.

The subsequent flush_dcache_range() call at the end of
mmu_set_region_dcache_behaviour() then crashes with a Level 3
translation fault because the region it tries to flush has just been
unmapped.

The existing flag parameter already distinguishes the two callers:
 - mmu_set_region_dcache_behaviour() always passes flag=false
 - mmu_change_region_attr_nobreak() always passes flag=true, and is the
   only legitimate caller that passes PTE_TYPE_FAULT to unmap a region

Guard the unmap path with flag so that DCACHE_OFF attribute changes
take the correct else branch, which ORs in the ATTRINDX bits only,
leaving the PTE valid.

This was observed as a boot crash on Versal, Versal Net, and ZynqMP
platforms during network initialisation. The zynq_gem driver calls
mmu_set_region_dcache_behaviour() with DCACHE_OFF to make its BD
descriptor ring non-cached. With the bug the BD memory is unmapped,
and the subsequent dcache flush inside
mmu_set_region_dcache_behaviour() faults.

Fixes: 6468ca13ffd6f ("armv8: mmu: fix and optimise explicitly unmapping regions")
Signed-off-by: Akshay Belsare <akshay.belsare@amd.com>
---
 arch/arm/cpu/armv8/cache_v8.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
index e59528e576e..ad232435eb2 100644
--- a/arch/arm/cpu/armv8/cache_v8.c
+++ b/arch/arm/cpu/armv8/cache_v8.c
@@ -1006,7 +1006,7 @@ static u64 set_one_region(u64 start, u64 size, u64 attrs, bool flag, int level)
 
 	/* Can we can just modify the current level block/page? */
 	if (is_aligned(start, size, levelsize)) {
-		if (attrs == PTE_TYPE_FAULT) {
+		if (attrs == PTE_TYPE_FAULT && flag) {
 			if (pte_type(pte) == PTE_TYPE_TABLE && level < 3)
 				*pte = 0;
 			else
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] arm: armv8: mmu: fix DCACHE_OFF incorrectly unmapping region
  2026-07-08 11:17 [PATCH] arm: armv8: mmu: fix DCACHE_OFF incorrectly unmapping region Akshay Belsare
@ 2026-07-08 11:47 ` Ilias Apalodimas
  2026-07-21  9:17   ` Michal Simek via U-Boot
  2026-07-24  1:02 ` Tom Rini
  1 sibling, 1 reply; 4+ messages in thread
From: Ilias Apalodimas @ 2026-07-08 11:47 UTC (permalink / raw)
  To: Akshay Belsare
  Cc: u-boot, michal.simek, git, padmarao.begari, Tom Rini,
	Anshul Dalal, Casey Connolly, Dhruva Gole, Simon Glass,
	Mark Kettenis

Hi Akshay

On Wed, 8 Jul 2026 at 14:17, Akshay Belsare <akshay.belsare@amd.com> wrote:
>
> DCACHE_OFF is defined as (0 << 2) = 0, and PTE_TYPE_FAULT is defined
> as (0 << 0) = 0. In mmu_set_region_dcache_behaviour(), the cache
> attribute passed to set_regions() is computed as:
>   attrs = PMD_ATTRINDX(option >> 2)
> For DCACHE_OFF=0 this evaluates to PMD_ATTRINDX(0) = 0, which equals
> PTE_TYPE_FAULT.
>
> Commit 6468ca13ffd6f ("armv8: mmu: fix and optimise explicitly unmapping
> regions") added an unmap path to set_one_region() that
> triggers when attrs == PTE_TYPE_FAULT. Because DCACHE_OFF and
> PTE_TYPE_FAULT share the same numerical value (0), any call to
> mmu_set_region_dcache_behaviour() with DCACHE_OFF silently unmaps the
> target region instead of changing its cache attributes to non-cached.
>
> The subsequent flush_dcache_range() call at the end of
> mmu_set_region_dcache_behaviour() then crashes with a Level 3
> translation fault because the region it tries to flush has just been
> unmapped.
>
> The existing flag parameter already distinguishes the two callers:
>  - mmu_set_region_dcache_behaviour() always passes flag=false
>  - mmu_change_region_attr_nobreak() always passes flag=true, and is the
>    only legitimate caller that passes PTE_TYPE_FAULT to unmap a region
>
> Guard the unmap path with flag so that DCACHE_OFF attribute changes
> take the correct else branch, which ORs in the ATTRINDX bits only,
> leaving the PTE valid.

I don't really love the 'flag' attribute that distinguishes between
preserving or overwriting the permission bits...

However, this does fix an issue.

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

>
> This was observed as a boot crash on Versal, Versal Net, and ZynqMP
> platforms during network initialisation. The zynq_gem driver calls
> mmu_set_region_dcache_behaviour() with DCACHE_OFF to make its BD
> descriptor ring non-cached. With the bug the BD memory is unmapped,
> and the subsequent dcache flush inside
> mmu_set_region_dcache_behaviour() faults.
>
> Fixes: 6468ca13ffd6f ("armv8: mmu: fix and optimise explicitly unmapping regions")
> Signed-off-by: Akshay Belsare <akshay.belsare@amd.com>
> ---
>  arch/arm/cpu/armv8/cache_v8.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
> index e59528e576e..ad232435eb2 100644
> --- a/arch/arm/cpu/armv8/cache_v8.c
> +++ b/arch/arm/cpu/armv8/cache_v8.c
> @@ -1006,7 +1006,7 @@ static u64 set_one_region(u64 start, u64 size, u64 attrs, bool flag, int level)
>
>         /* Can we can just modify the current level block/page? */
>         if (is_aligned(start, size, levelsize)) {
> -               if (attrs == PTE_TYPE_FAULT) {
> +               if (attrs == PTE_TYPE_FAULT && flag) {
>                         if (pte_type(pte) == PTE_TYPE_TABLE && level < 3)
>                                 *pte = 0;
>                         else
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] arm: armv8: mmu: fix DCACHE_OFF incorrectly unmapping region
  2026-07-08 11:47 ` Ilias Apalodimas
@ 2026-07-21  9:17   ` Michal Simek via U-Boot
  0 siblings, 0 replies; 4+ messages in thread
From: Michal Simek via U-Boot @ 2026-07-21  9:17 UTC (permalink / raw)
  To: Ilias Apalodimas, Akshay Belsare, Tom Rini
  Cc: u-boot, git, padmarao.begari, Tom Rini, Anshul Dalal,
	Casey Connolly, Dhruva Gole, Simon Glass, Mark Kettenis, u-boot

Hi Tom,

On 7/8/26 13:47, Ilias Apalodimas wrote:
> Hi Akshay
> 
> On Wed, 8 Jul 2026 at 14:17, Akshay Belsare <akshay.belsare@amd.com> wrote:
>>
>> DCACHE_OFF is defined as (0 << 2) = 0, and PTE_TYPE_FAULT is defined
>> as (0 << 0) = 0. In mmu_set_region_dcache_behaviour(), the cache
>> attribute passed to set_regions() is computed as:
>>    attrs = PMD_ATTRINDX(option >> 2)
>> For DCACHE_OFF=0 this evaluates to PMD_ATTRINDX(0) = 0, which equals
>> PTE_TYPE_FAULT.
>>
>> Commit 6468ca13ffd6f ("armv8: mmu: fix and optimise explicitly unmapping
>> regions") added an unmap path to set_one_region() that
>> triggers when attrs == PTE_TYPE_FAULT. Because DCACHE_OFF and
>> PTE_TYPE_FAULT share the same numerical value (0), any call to
>> mmu_set_region_dcache_behaviour() with DCACHE_OFF silently unmaps the
>> target region instead of changing its cache attributes to non-cached.
>>
>> The subsequent flush_dcache_range() call at the end of
>> mmu_set_region_dcache_behaviour() then crashes with a Level 3
>> translation fault because the region it tries to flush has just been
>> unmapped.
>>
>> The existing flag parameter already distinguishes the two callers:
>>   - mmu_set_region_dcache_behaviour() always passes flag=false
>>   - mmu_change_region_attr_nobreak() always passes flag=true, and is the
>>     only legitimate caller that passes PTE_TYPE_FAULT to unmap a region
>>
>> Guard the unmap path with flag so that DCACHE_OFF attribute changes
>> take the correct else branch, which ORs in the ATTRINDX bits only,
>> leaving the PTE valid.
> 
> I don't really love the 'flag' attribute that distinguishes between
> preserving or overwriting the permission bits...
> 
> However, this does fix an issue.
> 
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

Can you please cherry pick this patch?

Thanks,
Michal

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] arm: armv8: mmu: fix DCACHE_OFF incorrectly unmapping region
  2026-07-08 11:17 [PATCH] arm: armv8: mmu: fix DCACHE_OFF incorrectly unmapping region Akshay Belsare
  2026-07-08 11:47 ` Ilias Apalodimas
@ 2026-07-24  1:02 ` Tom Rini
  1 sibling, 0 replies; 4+ messages in thread
From: Tom Rini @ 2026-07-24  1:02 UTC (permalink / raw)
  To: u-boot, michal.simek, Akshay Belsare
  Cc: git, padmarao.begari, Ilias Apalodimas, Anshul Dalal,
	Casey Connolly, Dhruva Gole, Simon Glass, Mark Kettenis

On Wed, 08 Jul 2026 16:47:10 +0530, Akshay Belsare wrote:

> DCACHE_OFF is defined as (0 << 2) = 0, and PTE_TYPE_FAULT is defined
> as (0 << 0) = 0. In mmu_set_region_dcache_behaviour(), the cache
> attribute passed to set_regions() is computed as:
>   attrs = PMD_ATTRINDX(option >> 2)
> For DCACHE_OFF=0 this evaluates to PMD_ATTRINDX(0) = 0, which equals
> PTE_TYPE_FAULT.
> 
> [...]

Applied to u-boot/main, thanks!

[1/1] arm: armv8: mmu: fix DCACHE_OFF incorrectly unmapping region
      commit: d7ef23c07b8e34f65b3e15e3b4e7f58d074923c8
-- 
Tom



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-24  1:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-08 11:17 [PATCH] arm: armv8: mmu: fix DCACHE_OFF incorrectly unmapping region Akshay Belsare
2026-07-08 11:47 ` Ilias Apalodimas
2026-07-21  9:17   ` Michal Simek via U-Boot
2026-07-24  1:02 ` Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.