[PATCH v2 0/3] arm: caches: allow to activate dcache in SPL and in U-Boot pre-reloc

[PATCH v2 0/3] arm: caches: allow to activate dcache in SPL and in U-Boot pre-reloc

[Patrick Delaunay]

Hi

It is a V2 serie after Marek feedback for
http://patchwork.ozlabs.org/project/uboot/list/?series=168378

This serie allows dcache activation in SPL or in U-Boot preloc stage
for ARM board.

See "arm: stm32mp1: activate data cache in SPL and before relocation"
for example of usage in SPL and in U-Boot pre-reloc of the function
dcache_enable() and of mmu_set_region_dcache_behaviour().

A branch named "dcache" with the needed patches for stm32mp1 boards
is available in:
https://gitlab.denx.de/u-boot/custodians/u-boot-stm.git


Changes in v2:
- update patch after Marek's proposal. but I just divided by 2 instead
  of 4kB (minimal MMU page size)

Patrick Delaunay (3):
  arm: caches: protect dram_bank_mmu_setup access to bi_dram
  arm: caches: add DCACHE_DEFAULT_OPTION
  arm: caches: manage phys_addr_t overflow in
    mmu_set_region_dcache_behaviour

 arch/arm/include/asm/system.h |  8 ++++++++
 arch/arm/lib/cache-cp15.c     | 20 ++++++++++----------
 2 files changed, 18 insertions(+), 10 deletions(-)

-- 
2.17.1

[PATCH v2 1/3] arm: caches: protect dram_bank_mmu_setup access to bi_dram

[Patrick Delaunay]

Add protection in dram_bank_mmu_setup() to avoid access to bd->bi_dram
before relocation.

This patch allow to use the generic weak function dram_bank_mmu_setup
to activate the MMU and the data cache in SPL or in U-Boot before
relocation, when bd->bi_dram is not yet initialized.

In this cases, the MMU must be initialized explicitly with
mmu_set_region_dcache_behaviour function.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v2: None

 arch/arm/lib/cache-cp15.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
index f8d20960da..54509f11c3 100644
--- a/arch/arm/lib/cache-cp15.c
+++ b/arch/arm/lib/cache-cp15.c
@@ -91,6 +91,10 @@ __weak void dram_bank_mmu_setup(int bank)
 	bd_t *bd = gd->bd;
 	int	i;
 
+	/* bd->bi_dram is available only after relocation */
+	if ((gd->flags & GD_FLG_RELOC) == 0)
+		return;
+
 	debug("%s: bank: %d\n", __func__, bank);
 	for (i = bd->bi_dram[bank].start >> MMU_SECTION_SHIFT;
 	     i < (bd->bi_dram[bank].start >> MMU_SECTION_SHIFT) +
-- 
2.17.1

[PATCH v2 2/3] arm: caches: add DCACHE_DEFAULT_OPTION

[Patrick Delaunay]

Add the new flags DCACHE_DEFAULT_OPTION to define the default
option to use according the compilation flags
CONFIG_SYS_ARM_CACHE_*.

This new compilation flag allows to simplify dram_bank_mmu_setup()
and can be used as third parameter (option=dcache option to select)
of mmu_set_region_dcache_behaviour function.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---
CONFIG_SYS_ARM_CACHE_WRITEBACK dependency with
[v2] configs: migrate CONFIG_SYS_ARM_CACHE_* in Kconfig
http://patchwork.ozlabs.org/patch/1269103/


Changes in v2: None

 arch/arm/include/asm/system.h |  8 ++++++++
 arch/arm/lib/cache-cp15.c     | 11 ++---------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 81ccead112..a3147fde14 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -485,6 +485,14 @@ enum dcache_option {
 };
 #endif
 
+#if defined(CONFIG_SYS_ARM_CACHE_WRITETHROUGH)
+#define DCACHE_DEFAULT_OPTION	DCACHE_WRITETHROUGH
+#elif defined(CONFIG_SYS_ARM_CACHE_WRITEALLOC)
+#define DCACHE_DEFAULT_OPTION	DCACHE_WRITEALLOC
+#elif defined(CONFIG_SYS_ARM_CACHE_WRITEBACK)
+#define DCACHE_DEFAULT_OPTION	DCACHE_WRITEBACK
+#endif
+
 /* Size of an MMU section */
 enum {
 #ifdef CONFIG_ARMV7_LPAE
diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
index 54509f11c3..d15144188b 100644
--- a/arch/arm/lib/cache-cp15.c
+++ b/arch/arm/lib/cache-cp15.c
@@ -99,15 +99,8 @@ __weak void dram_bank_mmu_setup(int bank)
 	for (i = bd->bi_dram[bank].start >> MMU_SECTION_SHIFT;
 	     i < (bd->bi_dram[bank].start >> MMU_SECTION_SHIFT) +
 		 (bd->bi_dram[bank].size >> MMU_SECTION_SHIFT);
-	     i++) {
-#if defined(CONFIG_SYS_ARM_CACHE_WRITETHROUGH)
-		set_section_dcache(i, DCACHE_WRITETHROUGH);
-#elif defined(CONFIG_SYS_ARM_CACHE_WRITEALLOC)
-		set_section_dcache(i, DCACHE_WRITEALLOC);
-#else
-		set_section_dcache(i, DCACHE_WRITEBACK);
-#endif
-	}
+	     i++)
+		set_section_dcache(i, DCACHE_DEFAULT_OPTION);
 }
 
 /* to activate the MMU we need to set up virtual memory: use 1M areas */
-- 
2.17.1

[PATCH v2 3/3] arm: caches: manage phys_addr_t overflow in mmu_set_region_dcache_behaviour

[Patrick Delaunay]

Solved the overflow on phys_addr_t type for start + size in
mmu_set_region_dcache_behaviour() function.

This overflow is avoided by dividing start and end by 2 before addition,
and we only expecting that start and size are even.

This patch doesn't change the current function behavior if the
parameters (start or size) are not aligned on MMU_SECTION_SIZE.

For example, this overflow occurs on ARM32 with:
start = 0xC0000000 and size = 0x40000000
then start + size = 0x100000000 and end = 0x0.

For information the function behavior change with risk of regression,
if we just shift start and size before the addition.
Example with 2MB section size:
  MMU_SECTION_SIZE 0x200000 and MMU_SECTION_SHIFT = 21
  with start = 0x1000000, size = 0x1000000,
  - with the proposed patch, start = 0 and end = 0x1 as previously
  - with the more simple patch:
    end = (start >> MMU_SECTION_SHIFT) + (size >> MMU_SECTION_SHIFT)
    the value of end change:
    start >> 21 = 0, size >> 21 = 0 and end = 0x0 !!!

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v2:
- update patch after Marek's proposal. but I just divided by 2 instead
  of 4kB (minimal MMU page size)

 arch/arm/lib/cache-cp15.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
index d15144188b..f803d6fb8c 100644
--- a/arch/arm/lib/cache-cp15.c
+++ b/arch/arm/lib/cache-cp15.c
@@ -61,8 +61,11 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
 	unsigned long startpt, stoppt;
 	unsigned long upto, end;
 
-	end = ALIGN(start + size, MMU_SECTION_SIZE) >> MMU_SECTION_SHIFT;
+	/* div by 2 before start + size to avoid phys_addr_t overflow */
+	end = ALIGN((start / 2) + (size / 2), MMU_SECTION_SIZE / 2)
+	      >> (MMU_SECTION_SHIFT - 1);
 	start = start >> MMU_SECTION_SHIFT;
+
 #ifdef CONFIG_ARMV7_LPAE
 	debug("%s: start=%pa, size=%zu, option=%llx\n", __func__, &start, size,
 	      option);
-- 
2.17.1

[PATCH v2 0/3] arm: caches: allow to activate dcache in SPL and in U-Boot pre-reloc

[Tom Rini]

On Fri, Apr 24, 2020 at 08:20:14PM +0200, Patrick Delaunay wrote:

> 
> Hi
> 
> It is a V2 serie after Marek feedback for
> http://patchwork.ozlabs.org/project/uboot/list/?series=168378
> 
> This serie allows dcache activation in SPL or in U-Boot preloc stage
> for ARM board.
> 
> See "arm: stm32mp1: activate data cache in SPL and before relocation"
> for example of usage in SPL and in U-Boot pre-reloc of the function
> dcache_enable() and of mmu_set_region_dcache_behaviour().
> 
> A branch named "dcache" with the needed patches for stm32mp1 boards
> is available in:
> https://gitlab.denx.de/u-boot/custodians/u-boot-stm.git
> 
> 
> Changes in v2:
> - update patch after Marek's proposal. but I just divided by 2 instead
>   of 4kB (minimal MMU page size)
> 
> Patrick Delaunay (3):
>   arm: caches: protect dram_bank_mmu_setup access to bi_dram
>   arm: caches: add DCACHE_DEFAULT_OPTION
>   arm: caches: manage phys_addr_t overflow in
>     mmu_set_region_dcache_behaviour
> 
>  arch/arm/include/asm/system.h |  8 ++++++++
>  arch/arm/lib/cache-cp15.c     | 20 ++++++++++----------
>  2 files changed, 18 insertions(+), 10 deletions(-)

All in all, I like it, good work.  I'll pick it up not right now but
before -rc2.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200428/b08d7ad8/attachment.sig>

[PATCH v2 1/3] arm: caches: protect dram_bank_mmu_setup access to bi_dram

[Tom Rini]

On Fri, Apr 24, 2020 at 08:20:15PM +0200, Patrick Delaunay wrote:

> Add protection in dram_bank_mmu_setup() to avoid access to bd->bi_dram
> before relocation.
> 
> This patch allow to use the generic weak function dram_bank_mmu_setup
> to activate the MMU and the data cache in SPL or in U-Boot before
> relocation, when bd->bi_dram is not yet initialized.
> 
> In this cases, the MMU must be initialized explicitly with
> mmu_set_region_dcache_behaviour function.
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200501/1dc99cc4/attachment.sig>

[PATCH v2 2/3] arm: caches: add DCACHE_DEFAULT_OPTION

[Tom Rini]

On Fri, Apr 24, 2020 at 08:20:16PM +0200, Patrick Delaunay wrote:

> Add the new flags DCACHE_DEFAULT_OPTION to define the default
> option to use according the compilation flags
> CONFIG_SYS_ARM_CACHE_*.
> 
> This new compilation flag allows to simplify dram_bank_mmu_setup()
> and can be used as third parameter (option=dcache option to select)
> of mmu_set_region_dcache_behaviour function.
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200501/0d12076a/attachment.sig>

[PATCH v2 3/3] arm: caches: manage phys_addr_t overflow in mmu_set_region_dcache_behaviour

[Tom Rini]

On Fri, Apr 24, 2020 at 08:20:17PM +0200, Patrick Delaunay wrote:

> Solved the overflow on phys_addr_t type for start + size in
> mmu_set_region_dcache_behaviour() function.
> 
> This overflow is avoided by dividing start and end by 2 before addition,
> and we only expecting that start and size are even.
> 
> This patch doesn't change the current function behavior if the
> parameters (start or size) are not aligned on MMU_SECTION_SIZE.
> 
> For example, this overflow occurs on ARM32 with:
> start = 0xC0000000 and size = 0x40000000
> then start + size = 0x100000000 and end = 0x0.
> 
> For information the function behavior change with risk of regression,
> if we just shift start and size before the addition.
> Example with 2MB section size:
>   MMU_SECTION_SIZE 0x200000 and MMU_SECTION_SHIFT = 21
>   with start = 0x1000000, size = 0x1000000,
>   - with the proposed patch, start = 0 and end = 0x1 as previously
>   - with the more simple patch:
>     end = (start >> MMU_SECTION_SHIFT) + (size >> MMU_SECTION_SHIFT)
>     the value of end change:
>     start >> 21 = 0, size >> 21 = 0 and end = 0x0 !!!
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200501/c41619c2/attachment.sig>