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

[Marcin Wojtas <mw@semihalf.com>]

Hi Leif,

śr., 23 sty 2019 o 10:42 Leif Lindholm <leif.lindholm@linaro.org> napisał(a):
>
> 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.
>

Thank you for your input. I will explicitly handle each region then.

Best regards,
Marcin