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

[Ard Biesheuvel]

On Mon, 12 Oct 2020 at 11:09, Etienne Carriere
<etienne.carriere at linaro.org> wrote:
>
> On Fri, 9 Oct 2020 at 19:13, Ahmad Fatoum <a.fatoum at pengutronix.de> wrote:
> >
> > Hello Patrick,
> >
> > On 10/9/20 5:52 PM, Patrick DELAUNAY wrote:
> > > I checked DACR behavior and CheckDomain /  CheckPermission
> > >
> > > In my case the cortex A7 try to access to part of DDR / mapped cacheable and bufferable, protected by firewall.
> > >
> > > So to use DACR I always need to configure the MMU with several Domain
> > > - unsecure part of DDR as Domain 0 (DCACHE_WRITEALLOC)
> > > - secure part of DDR as  Domain 1 (DCACHE_OFF)
> > >
> > > For other part of MMU region, the I/O registers are mapped as register with Domain 0 (D_CACHE_OFF)
> > >
> > > Then I can set DACR = 0x55555555
> > > => Client Accesses are checked against the access permission bits in the TLB entry
> > >
> > > You ar right, the access permission is check only for domain configurated as client in DACR
> > >
> > > But in ARM architecture
> > >
> > > B2.4.8 Access permission checking
> > >
> > > CheckPermission() pseudo code
> > > Only check perms.ap is checked
> > > And perms.xp is not checked
> > >
> > > But as the secure memory is mapped cacheable by secure OS, OP-TEE
> > > I think to avoid to map the region is the ARM preconized solution
> > > As explain in my answer to ard in [1]
> > >
> > > [1] http://u-boot.10912.n7.nabble.com/PATCH-0-7-arm-cache-cp15-don-t-map-reserved-region-with-no-map-property-tt428715.html#a428958
> >
> > I don't think the aliasing described in "A3.5.7 Memory access restrictions" applies if the
> > same memory is mapped twice only, once in secure and another in normal world.
> > Data is already segregated in the caches by means of a NS bit. Only thing you should need
> > to do within normal world is mapping it XN, cacheable and not be in manager domain.
> > Unmapping sounds unnecessary to me. (You don't unmap peripherals you aren't using either.
> > Why handle OP-TEE DRAM specially?)
>
> This is right regarding the secure memory/NS=0. But the
> reserved-memory node for OP-TEE can also cover non-secure (shared)
> memory that OP-TEE secure world maps Normal/NS=1. So U-boot should not
> map such memory as Device/NS=0. That would jeopardize non-secure
> memory content.
>

Agreed. If secure and non-secure need to have a coherent, cacheable
view of that shared memory, non-secure device mappings must be
avoided.

But is no-map really needed for that memory? It needs to be mapped in
any case to be usable for the non-secure world to communicate with the
secure world, right?

I'd assume the no-map is only needed if cacheable, inner shareable
mappings are a problem.

> Note that platforms can define either a single reserved-memory node in
> the FDT for both contiguous secure and shared DDR
> or platforms can alternatively define 2 reserved_memory nodes: one for
> the secure DDR and one for the non-secure shared DDR.
>
> In the 1st case, the no-map property of the single reserved-memory
> node should really not map the area in U-Boot.
>
> In the 2nd case, the no-map property on the secure DDR reserved-memory
> node must prevent U-Boot speculative accesses while node for shared
> memory obviously doesn't need no-map.
>
> This is to say that mapping as Device memory that has the no-map
> property can be an issue.
>

So in summary, there are two separate requirements resulting from this:
- the DT should not describe secure world memory as ordinary memory,
as the S and NS address spaces could overlap, and the distinction only
makes sense for agents running in the secure world;
- no-map should be honored by u-boot, but it should only be used if
the existence of a cacheable, inner shareable mapping of that memory
poses a problem.

-- 
Ard.