On Tue, 6 Oct 2020 at 18:36, Patrick Delaunay <patrick.delau...@st.com> wrote: > > > Hi, > > On STM32MP15x platform we can use OP-TEE, loaded in DDR in a region > protected by a firewall. This region is reserved in device with "no-map" > property. > > But 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 a 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 (mapped as DEVICE/NON-CACHEABLE wasn't enough) as it is > already done in Linux kernel. >
Spurious peculative accesses to device regions would be a severe silicon bug, so I wonder what is going on here. (Apologies if we are rehashing stuff here that has already been discussed - I don't remember the details) Are you sure that the speculative accesses were not caused by misconfigured CPU or page tables, missing TLB maintenance, etc etc? Because it really does smell like a software issue not a hardware issue. > I think that can be a general issue for ARM architecture: the no-map tag > in device should be respected by U-boot, so I propose a generic solution > in arm/lib/cache-cp15.c:dram_bank_mmu_setup(). > > This serie is composed by 7 patches > - 1..4/7: preliminary steps to support flags in library in lmb > (as it is done in memblock.c in Linux) > - 5/7: unitary test on the added feature in lmb lib > - 6/7: save the no-map flags in lmb when the device tree is parsed > - 7/7: update the generic behavior for "no-map" region in > arm/lib/cache-cp15.c::dram_bank_mmu_setup() > > It is a rebase on master branch of previous RFC [2]. > > I can change this last patch if this feature is note required by other > ARM architecture; it is a weak function so I can avoid to map the region > with "no-map" property in device tree only for STM32MP architecture > (in arch/arm/mach-stm32mp/cpu.c). > > See also [1] which handle same speculative access on armv8 for area > with Executable attribute. > > [1] > http://patchwork.ozlabs.org/project/uboot/patch/20200903000106.5016-1-marek.bykow...@gmail.com/ > [2] > http://patchwork.ozlabs.org/project/uboot/list/?series=199486&archive=both&state=* > > Regards > Patrick > > > Patrick Delaunay (7): > lmb: Add support of flags for no-map properties > lmb: add lmb_is_reserved_flags > lmb: remove lmb_region.size > lmb: add lmb_dump_region() function > test: lmb: add test for lmb_reserve_flags > image-fdt: save no-map parameter of reserve-memory > arm: cache: cp15: don't map the reserved region with no-map property > > arch/arm/include/asm/system.h | 3 + > arch/arm/lib/cache-cp15.c | 19 ++++++- > common/image-fdt.c | 23 +++++--- > include/lmb.h | 22 +++++++- > lib/lmb.c | 100 +++++++++++++++++++++++----------- > test/lib/lmb.c | 89 ++++++++++++++++++++++++++++++ > 6 files changed, 212 insertions(+), 44 deletions(-) > > -- > 2.17.1 >
