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

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

On Tue, Jan 22, 2019 at 07:26:58PM +0100, Marcin Wojtas wrote:
> Hi Leif,
> 
> wt., 22 sty 2019 o 18:26 Leif Lindholm <leif.lindholm@linaro.org> napisał(a):
> >
> > On Tue, Jan 22, 2019 at 02:32:19AM +0100, Marcin Wojtas wrote:
> > > Recent changes in the ARM-TF configure its runtime serices region
> > > as protected, hence the hitherto PEI stack base address (0x41F0000)
> > > violated it.
> > >
> > > In order to fix this, extend the region which is non-accessible
> > > by the OS to cover both the ARM-TF (0x4000000 - 0x4200000) and OPTEE
> > > (0x4400000 - 0x5400000) within a single area (0x4000000 - 0x5400000).
> > > Set the PEI stack base address between both images (0x43F0000).
> >
> > OK, that is a much better description.
> > But I'm getting slight cognitive dissonance from placing the PEI stack
> > inside something we've just claimed belongs to Secure world...
> >
> > Could you instead break this out into two separate protected regions?
> > PcdSecureOpteeBase/Size and PcdSecureTfBase/Size?
> >
> > Alternatively, nudge the stackbase to 0x5400000?
> 
> As discussed some time ago with Ard, when the PEI stack base was
> introduced, it is recommended that this stack is placed in the
> location, which is not accessible by OS. Most preferred is to have it
> in the SRAM (cannot do it on Armada7k8k) or in a reserved region - cut
> out from the memory map passed to the OS.
> 
> Currently we have a single region (a "hole") that covers:
> 2MB for EL3 runtime services
> 2MB of nothing
> 16MB for OPTEE image
> 
> The 2MB space between images IMO seems perfect for PEI stack to place.
> If it was placed e.g. @0x5400000 and we kept the reserved regions
> separate, the outcome would be:
> 2MB for EL3 runtime services
> 2MB of DRAM normal memory
> 16MB + 64kB for Optee and PEI stack base.
> 
> This is the reason, I'd like to keep original setting, proposed in the
> patch. Please let know your opinion.

I have no issue with the placement of the PEI stack between the ARM-TF
region and the Op-TEE region. I _have_ an issue with the PEI stack
being placed between PcdSecureRegionBase and (PcdSecureRegionBase +
PcdSecureRegionSize). I.e. something that we describe as "the Secure
region".

I think I gave my suggestion for the resolution of this problem (with
moving StackBase to 0x05400000 as the alternative) in my previous
reply.

Best Regards,

Leif

> 
> Best regards,
> Marcin
> 
> 
> >
> > /
> >     Leif
> >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> > > ---
> > >  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > > index eafcd6e..c8c597f 100644
> > > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > > @@ -376,12 +376,12 @@
> > >
> > >    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|36
> > >
> > > -  gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x41F0000
> > > +  gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x43F0000
> > >    gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x10000
> > >
> > >    # Secure region reservation
> > >    gMarvellTokenSpaceGuid.PcdSecureRegionBase|0x4000000
> > > -  gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x0200000
> > > +  gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x1400000
> > >
> > >    # TRNG
> > >    gMarvellTokenSpaceGuid.PcdEip76TrngBaseAddress|0xF2760000
> > > --
> > > 2.7.4
> > >