[2/3] arm: mach-k3: Remove non-cached memory map areas

Message ID	20231123172214.653268-3-afd@ti.com
State	Deferred
Delegated to:	Tom Rini
Headers	show Return-Path: <u-boot-bounces@lists.denx.de> From: Andrew Davis <afd@ti.com> To: Neha Malcom Francis <n-francis@ti.com>, Vignesh Raghavendra <vigneshr@ti.com>, Nishanth Menon <nm@ti.com>, Simon Glass <sjg@chromium.org>, Tom Rini <trini@konsulko.com>, Apurva Nandan <a-nandan@ti.com> CC: <u-boot@lists.denx.de>, Andrew Davis <afd@ti.com> Subject: [PATCH 2/3] arm: mach-k3: Remove non-cached memory map areas Date: Thu, 23 Nov 2023 11:22:13 -0600 Message-ID: <20231123172214.653268-3-afd@ti.com> In-Reply-To: <20231123172214.653268-1-afd@ti.com> References: <20231123172214.653268-1-afd@ti.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain Precedence: list Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" <u-boot-bounces@lists.denx.de>
Series	Unify K3 initial memory map \| expand [0/3] Unify K3 initial memory map [1/3] arm: mach-k3: Let the compiler size the mem_map lists [2/3] arm: mach-k3: Remove non-cached memory map areas [3/3] arm: mach-k3: Merge initial memory maps

Message ID

20231123172214.653268-3-afd@ti.com

State

Deferred

Delegated to:

Tom Rini

Headers

From: Andrew Davis <afd@ti.com>
To: Neha Malcom Francis <n-francis@ti.com>, Vignesh Raghavendra
 <vigneshr@ti.com>, Nishanth Menon <nm@ti.com>, Simon Glass
 <sjg@chromium.org>, Tom Rini <trini@konsulko.com>, Apurva Nandan
 <a-nandan@ti.com>
CC: <u-boot@lists.denx.de>, Andrew Davis <afd@ti.com>
Subject: [PATCH 2/3] arm: mach-k3: Remove non-cached memory map areas
Date: Thu, 23 Nov 2023 11:22:13 -0600
Message-ID: <20231123172214.653268-3-afd@ti.com>
In-Reply-To: <20231123172214.653268-1-afd@ti.com>
References: <20231123172214.653268-1-afd@ti.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
Precedence: list
Errors-To: u-boot-bounces@lists.denx.de
Sender: "U-Boot" <u-boot-bounces@lists.denx.de>

Series

Unify K3 initial memory map | expand

Commit Message

Andrew Davis Nov. 23, 2023, 5:22 p.m. UTC

All normal memory areas should be mapped as such.

We added these un-cached holes in our memory map to hack around the
remoteproc driver missing the proper cache maintenance operations.

The problem is having these non-cached memory map areas causes stability
issues later in system operation due to the nature of the K3 coherency
architecture. Plus these are board specific carveouts and instead
should have been added at the board level, not here in the SoC common
code area.

Remove these non-cached memory map areas.

Signed-off-by: Andrew Davis <afd@ti.com>
Reviewed-by: Nishanth Menon <nm@ti.com>
Tested-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-k3/arm64-mmu.c | 65 +++---------------------------------
 1 file changed, 5 insertions(+), 60 deletions(-)

Comments

Vignesh Raghavendra Nov. 23, 2023, 5:34 p.m. UTC | #1

On 11/23/2023 10:52 PM, Andrew Davis wrote:
> @@ -219,16 +177,9 @@ struct mm_region am62_mem_map[] = {
>  	}, {
>  		.virt = 0x80000000UL,
>  		.phys = 0x80000000UL,
> -		.size = 0x1E780000UL,
> -		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
> -			PTE_BLOCK_INNER_SHARE
> -	}, {
> -		.virt = 0xA0000000UL,
> -		.phys = 0xA0000000UL,
> -		.size = 0x60000000UL,
> +		.size = 0x80000000UL,
>  		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
>  			 PTE_BLOCK_INNER_SHARE
> -
>  	}, {
>  		.virt = 0x880000000UL,
>  		.phys = 0x880000000UL,

This causes issues when TF-A region @0x9e780000 is firewalled off from
non-secure world. A53 does speculative accesses to this region from EL1
leading to FW exceptions being reported on TIFS log (although A53 itself
doesn't seem to abort)

Nishanth Menon Nov. 27, 2023, 3:47 p.m. UTC | #2

On 23:04-20231123, Vignesh Raghavendra wrote:
> 
> 
> On 11/23/2023 10:52 PM, Andrew Davis wrote:
> > @@ -219,16 +177,9 @@ struct mm_region am62_mem_map[] = {
> >  	}, {
> >  		.virt = 0x80000000UL,
> >  		.phys = 0x80000000UL,
> > -		.size = 0x1E780000UL,
> > -		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
> > -			PTE_BLOCK_INNER_SHARE
> > -	}, {
> > -		.virt = 0xA0000000UL,
> > -		.phys = 0xA0000000UL,
> > -		.size = 0x60000000UL,
> > +		.size = 0x80000000UL,
> >  		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
> >  			 PTE_BLOCK_INNER_SHARE
> > -
> >  	}, {
> >  		.virt = 0x880000000UL,
> >  		.phys = 0x880000000UL,
> 
> 
> This causes issues when TF-A region @0x9e780000 is firewalled off from
> non-secure world. A53 does speculative accesses to this region from EL1
> leading to FW exceptions being reported on TIFS log (although A53 itself
> doesn't seem to abort)

Hmm... Why not just manage the ones that use TFA in DDR seperately? I
suppose OPTEE will come in the way?

Looks like the no-map is not honored well.. as you were mentioning, a
zephyr style generating the map from dts might be much better, but then
we do support multiple dtbs as well..

OK - I guess we need to leave this cleanup for now.

Andrew Davis Nov. 28, 2023, 5 p.m. UTC | #3

On 11/27/23 9:47 AM, Nishanth Menon wrote:
> On 23:04-20231123, Vignesh Raghavendra wrote:
>>
>>
>> On 11/23/2023 10:52 PM, Andrew Davis wrote:
>>> @@ -219,16 +177,9 @@ struct mm_region am62_mem_map[] = {
>>>   	}, {
>>>   		.virt = 0x80000000UL,
>>>   		.phys = 0x80000000UL,
>>> -		.size = 0x1E780000UL,
>>> -		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
>>> -			PTE_BLOCK_INNER_SHARE
>>> -	}, {
>>> -		.virt = 0xA0000000UL,
>>> -		.phys = 0xA0000000UL,
>>> -		.size = 0x60000000UL,
>>> +		.size = 0x80000000UL,
>>>   		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
>>>   			 PTE_BLOCK_INNER_SHARE
>>> -
>>>   	}, {
>>>   		.virt = 0x880000000UL,
>>>   		.phys = 0x880000000UL,
>>
>>
>> This causes issues when TF-A region @0x9e780000 is firewalled off from
>> non-secure world. A53 does speculative accesses to this region from EL1
>> leading to FW exceptions being reported on TIFS log (although A53 itself
>> doesn't seem to abort)
> 
> Hmm... Why not just manage the ones that use TFA in DDR seperately? I
> suppose OPTEE will come in the way?
> 
> Looks like the no-map is not honored well.. as you were mentioning, a
> zephyr style generating the map from dts might be much better, but then
> we do support multiple dtbs as well..
> 
> OK - I guess we need to leave this cleanup for now.
> 

We can make the same cleanup while leaving the firewall hole in place
(it is in the same spot for all SoCs today so no problem here). I'll
send a v2 with that.

Andrew

diff --git a/arch/arm/mach-k3/arm64-mmu.c b/arch/arm/mach-k3/arm64-mmu.c
index d872ed714c4..97a029af13f 100644
--- a/arch/arm/mach-k3/arm64-mmu.c
+++ b/arch/arm/mach-k3/arm64-mmu.c
@@ -24,19 +24,7 @@  struct mm_region am654_mem_map[] = {
 	}, {
 		.virt = 0x80000000UL,
 		.phys = 0x80000000UL,
-		.size = 0x20000000UL,
-		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
-			 PTE_BLOCK_INNER_SHARE
-	}, {
-		.virt = 0xa0000000UL,
-		.phys = 0xa0000000UL,
-		.size = 0x02100000UL,
-		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL_NC) |
-			 PTE_BLOCK_INNER_SHARE
-	}, {
-		.virt = 0xa2100000UL,
-		.phys = 0xa2100000UL,
-		.size = 0x5df00000UL,
+		.size = 0x80000000UL,
 		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
 			 PTE_BLOCK_INNER_SHARE
 	}, {
@@ -75,19 +63,7 @@  struct mm_region j7200_mem_map[] = {
 	}, {
 		.virt = 0x80000000UL,
 		.phys = 0x80000000UL,
-		.size = 0x20000000UL,
-		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
-			 PTE_BLOCK_INNER_SHARE
-	}, {
-		.virt = 0xa0000000UL,
-		.phys = 0xa0000000UL,
-		.size = 0x04800000UL,
-		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL_NC) |
-			 PTE_BLOCK_NON_SHARE
-	}, {
-		.virt = 0xa4800000UL,
-		.phys = 0xa4800000UL,
-		.size = 0x5b800000UL,
+		.size = 0x80000000UL,
 		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
 			 PTE_BLOCK_INNER_SHARE
 	}, {
@@ -123,19 +99,7 @@  struct mm_region j721e_mem_map[] = {
 	}, {
 		.virt = 0x80000000UL,
 		.phys = 0x80000000UL,
-		.size = 0x20000000UL,
-		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
-			 PTE_BLOCK_INNER_SHARE
-	}, {
-		.virt = 0xa0000000UL,
-		.phys = 0xa0000000UL,
-		.size = 0x1bc00000UL,
-		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL_NC) |
-			 PTE_BLOCK_NON_SHARE
-	}, {
-		.virt = 0xbbc00000UL,
-		.phys = 0xbbc00000UL,
-		.size = 0x44400000UL,
+		.size = 0x80000000UL,
 		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
 			 PTE_BLOCK_INNER_SHARE
 	}, {
@@ -151,12 +115,6 @@  struct mm_region j721e_mem_map[] = {
 		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
 			 PTE_BLOCK_NON_SHARE |
 			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
-	}, {
-		.virt = 0x4d80000000UL,
-		.phys = 0x4d80000000UL,
-		.size = 0x0002000000UL,
-		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL_NC) |
-			 PTE_BLOCK_INNER_SHARE
 	}, {
 		/* List terminator */
 		0,
@@ -219,16 +177,9 @@  struct mm_region am62_mem_map[] = {
 	}, {
 		.virt = 0x80000000UL,
 		.phys = 0x80000000UL,
-		.size = 0x1E780000UL,
-		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
-			PTE_BLOCK_INNER_SHARE
-	}, {
-		.virt = 0xA0000000UL,
-		.phys = 0xA0000000UL,
-		.size = 0x60000000UL,
+		.size = 0x80000000UL,
 		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
 			 PTE_BLOCK_INNER_SHARE
-
 	}, {
 		.virt = 0x880000000UL,
 		.phys = 0x880000000UL,
@@ -264,13 +215,7 @@  struct mm_region am64_mem_map[] = {
 	}, {
 		.virt = 0x80000000UL,
 		.phys = 0x80000000UL,
-		.size = 0x1E800000UL,
-		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
-			PTE_BLOCK_INNER_SHARE
-	}, {
-		.virt = 0xA0000000UL,
-		.phys = 0xA0000000UL,
-		.size = 0x60000000UL,
+		.size = 0x80000000UL,
 		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
 			 PTE_BLOCK_INNER_SHARE
 	}, {

[2/3] arm: mach-k3: Remove non-cached memory map areas

Commit Message

Comments

Patch