Re: [platforms: PATCH v2 1/4] Marvell/Armada7k8k: Shift PEI stack base

[Leif Lindholm <leif.lindholm@linaro.org>]

On Wed, Jan 23, 2019 at 09:28:40AM +0100, Marcin Wojtas wrote:
> wt., 22 sty 2019 o 22:10 Leif Lindholm <leif.lindholm@linaro.org> napisał(a):
> >
> > On Tue, Jan 22, 2019 at 09:56:14PM +0100, Marcin Wojtas wrote:
> > > > > > I think I gave my suggestion for the resolution of this problem (with
> > > > > > moving StackBase to 0x05400000 as the alternative) in my previous
> > > > > > reply.
> > > > > >
> > > > >
> > > > > Yes, and I answered, presenting the alternative memory map with
> > > > > additional 64kB "cut out" on top of 20MB "hole" of memory, which I'm
> > > > > not fancy, given available space inside the 20MB chunk.
> > > >
> > > > Please go back and reread my first and my second email.
> > > > Then please point out where I have, other than as an alternative
> > > > solution, suggested growing the cutout size.
> > > >
> > > > Then perhaps we can rewind this conversation and try again?
> > >
> > > Ok. So would it be sufficient to replace
> > > gMarvellTokenSpaceGuid.PcdSecureRegionBase with two sets of separate
> > > PCDs for ARM-TF runtime services and OPTEE leaving the PEI stack base
> > > @0x43f0000?
> >
> > That would be lovely, thank you :)
> >
> > (Although your reference to wanting to keep the PEI stack area out of
> > the hands of the operating system might mean that you want three? I'll
> > leave that to your discretion.)
> >
> 
> PEI stack has its own PCDs:
>    gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase
>    gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize
> 
> I want to keep it simple (and btw aligned with U-Boot booting the
> mainline DTB with single 20MB reserved memory area), so what I intend
> to do is to limit reserved region in Armada7k8kMemoryInitPeiLib.c with
> PcdArmTFRegionBase (@0x4000000) up to PcdOpteeRegionBase +
> PcdOpteeRegionSize (@0x5400000).

I am totally online with you wanting to align the reservation of 20MB
of RAM with U-Boot.

If you want to remove the 2MB gap between ARM-TF and Optee from use by
the OS, you need to reserve that 2MB window. Not pretend that it forms
part of an adjacent region that you also happen to want to keep out of
the hands of the OS.

The point of the source code is not wiggling the correct signal lines
to create an expected behaviour. Were that the case, we'd be hacking
programs directly into binary.
The point of the source code is to describe what is being done such
that someone else can come in and understand it.

Saving 15 (or 30, or whatever) lines of boilerplate text by making the
code misleading is not a win.

You want to solve this by making PcdCPUCorePrimaryStackSize 2MB?
Fine. It's not misleading, and you could always shrink it if you need
the remainder for something else.

You want to solve this by setting up a third reserved area of
(2MB - PcdCPUCorePrimaryStackSize)?
Fine.

You want to solve this by making the source code say that a memory
region is simultaneously reserved for Secure world and where our
Non-secure stack resides?
Not fine. That is what I mean by semantic sense.

Best Regards,

Leif