[PATCH v4 0/7] arm: cache: cp15: don't map reserved region with no-map property

Patrick Delaunay patrick.delaunay at foss.st.com
Fri May 7 14:50:28 CEST 2021


Hi,

It it the v4 serie of [1].

This v4 serie is rebased on top of master branch with update after Simon
Glass review and added tags.

On STM32MP15x platform we can use OP-TEE, loaded in DDR in a region
protected by a firewall. This region is reserved in the device with
the "no-map" property as defined in the binding file
doc/device-tree-bindings/reserved-memory/reserved-memory.txt.

Sometime the platform boot failed in U-Boot on a Cortex A7 access to
this region (depending of the binary and the issue can change with compiler
version or with code alignment), then the firewall raise an error,
for example:

E/TC:0   tzc_it_handler:19 TZC permission failure
E/TC:0   dump_fail_filter:420 Permission violation on filter 0
E/TC:0   dump_fail_filter:425 Violation @0xde5c6bf0, non-secure privileged read,
         AXI ID 5c0
E/TC:0   Panic

After investigation, the forbidden access is a speculative request performed
by the Cortex A7 because all the DDR is mapped as MEMORY with CACHEABLE
property.

The issue is solved only when the region reserved by OP-TEE is no more
mapped in U-Boot as it is already done in Linux kernel.

Tested on DK2 board with OP-TEE 3.12 / TF-A 2.4:

With hard-coded address for OP-TEE reserved memory,
the error doesn't occur.

 void dram_bank_mmu_setup(int bank)
 {
 ....

    	for (i = start >> MMU_SECTION_SHIFT;
 	     i < (start >> MMU_SECTION_SHIFT) + (size >> MMU_SECTION_SHIFT);
 	     i++) {
 		option = DCACHE_DEFAULT_OPTION;
 		if (i >= 0xde0)
 			option = INVALID_ENTRY;
 		set_section_dcache(i, option);
 	}
 }

Just by modifying the test on 0xde0 to 0xdf0, the OP-TEE memory protected
by firewall is mapped cacheable and the error occurs.

I think that it can be a general issue for ARM architecture: the "no-map" tag
of reserved memory in device should be respected by U-Boot if firewall
is configured before U-Boot execution.

But I don't propose a generic solution in
arm/lib/cache-cp15.c:dram_bank_mmu_setup()
because the device tree parsing done in lmb_init_and_reserve() takes a
long time when it is executed without data cache.

=> the previous path 7/7 of v2 series is dropped to avoid
  performance issue on other ARM target.

To avoid this performance issue on stm32mp32mp platform, the lmb
initialization is done in enable_caches() when dcache is still enable.

This v3 series is composed by 7 patches
- 1..3/7: preliminary steps to support flags in library in lmb
  (as it is done in memblock.c in Linux)
- 4/7: unitary test on the added feature in lmb lib
- 5/7: save the no-map flags in lmb when the device tree is parsed
- 6/7: solve issue for the size of cacheable area in pre-reloc case
- 7/7: update the stm32mp mmu support

See also [2] which handle same speculative access on armv8 for area
with Executable attribute.

[1] http://patchwork.ozlabs.org/project/uboot/list/?series=241122&state=*
[2] http://patchwork.ozlabs.org/project/uboot/patch/20200903000106.5016-1-marek.bykowski@gmail.com/

Regards
Patrick

Changes in v4:
- Add comment for lmb_reserve_flags and remove extern
- Remove unnecessary !! on return of boolean in lmb_is_nomap
- Add comment for lmb_is_reserved_flags and remove extern
- Remove unnecessary !! on return of boolean in lmb_is_reserved_flags
- map the end of the DDR only before relocation, in board_f.c;
  this test avoid issue when board_get_usable_ram_top() is called
  in efi_loader function efi_add_known_memory()

Changes in v3:
- NEW: solve performance issue as relocated DT is not marked cacheable
- call lmb_init_and_reserve when data cache is activated in enable_caches()
- drop v2 patch "arm: cache: cp15: don't map the reserved region
  with no-map property"

Changes in v2:
- remove unnecessary comments in lmb.h
- rebase on latest lmb patches
- NEW: update in stm32mp specific MMU setup functions

Patrick Delaunay (7):
  lmb: Add support of flags for no-map properties
  lmb: add lmb_is_reserved_flags
  lmb: add lmb_dump_region() function
  test: lmb: add test for lmb_reserve_flags
  image-fdt: save no-map parameter of reserve-memory
  stm32mp: Increase the reserved memory in board_get_usable_ram_top
  stm32mp: don't map the reserved region with no-map property

 arch/arm/mach-stm32mp/cpu.c       | 17 +++++-
 arch/arm/mach-stm32mp/dram_init.c |  7 ++-
 common/image-fdt.c                | 23 +++++---
 include/lmb.h                     | 38 +++++++++++++
 lib/lmb.c                         | 93 ++++++++++++++++++++++---------
 test/lib/lmb.c                    | 89 +++++++++++++++++++++++++++++
 6 files changed, 228 insertions(+), 39 deletions(-)

-- 
2.17.1



More information about the U-Boot mailing list