Re: [platforms: PATCH v3 0/5] Armada7k8k memory handling update

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

On Thu, Jan 31, 2019 at 01:06:15PM +0100, Marcin Wojtas wrote:
> czw., 31 sty 2019 o 11:28 Leif Lindholm <leif.lindholm@linaro.org> napisał(a):
> >
> > On Thu, Jan 31, 2019 at 08:01:08AM +0100, Marcin Wojtas wrote:
> > > Hi Leif,
> > >
> > > Thanks a lot. While at it - do you think ArmPkg/Include/Library/ArmLib.h
> > > / ArmPkg/Library/ArmLib/ArmLib.c would be a proper place for it?
> >
> > As good a place as any. While not ARM-architecture specific, I feel
> > it's probably ARM-platform specific.
> >
> > I mean, hopefully we'll some day we'll get a sane reporting mechanism
> > of Secure-reserved regions by ARM-TF and we can drop this juggling in
> > Non-secure firmware.
> 
> Agree on ARM-TF dependency, but the code is IMO pretty generic itself.
> In the merged version I also used this routine to cut out a hole in
> the HoB for the reason not related to NS region - therefore I think
> even MdePkg/Library would be good to go :)

I don't really disagree with you, but that would be something to take
up with MdeModulePkg maintainers.

Also, I'm not seeing an obvious existing library to put it into. Would
it really be worth creating a new one for just this function?

/
    Leif

> Marcin
> 
> > > Best regards,
> > > Marcin
> > >
> > > śr., 30 sty 2019 o 17:47 Leif Lindholm <leif.lindholm@linaro.org>
> > > napisał(a):
> > >
> > > > Thanks for the rework.
> > > >
> > > > (We should probably move that broken-out function to ArmPkg at some point.)
> > > >
> > > > For the series:
> > > > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> > > >
> > > > Pushed as b0bb325f20..0a7d8e7d93.
> > > >
> > > > On Mon, Jan 28, 2019 at 10:45:10AM +0100, Marcin Wojtas wrote:
> > > > > Hi,
> > > > >
> > > > > The third version of the patchset moves the new common
> > > > > header for Marvell SMC ID's to the IndustryStandard directory.
> > > > > What is more important, now 3 regions (described by new PCDs)
> > > > > are reserved separately. For that purpose a preparation
> > > > > patch was added, which extract existing reservation code
> > > > > into a new subroutine. More details can be found in
> > > > > the changelog below and the commit messages.
> > > > >
> > > > > Patches are available in the github:
> > > > >
> > > > https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/dram-upstream-r20190128
> > > > >
> > > > > I'm looking forward to the comments and remarks.
> > > > >
> > > > > Best regards,
> > > > > Marcin
> > > > >
> > > > > Changelog:
> > > > > v2 -> v3
> > > > > * 1/5
> > > > >   - New patch - extract memory reservation to a separate routine
> > > > >
> > > > > * 2/2
> > > > >   - Add new PCDs and reserve 3 regions (ARM-TF, PEI stack, OP-TEE)
> > > > >     separately
> > > > >   - Update commit message accordingly
> > > > >
> > > > > * 3/5
> > > > >   - Move MvSmc.h to Include/IndustryStandard
> > > > >
> > > > > * 4,5/5
> > > > >   - Add Leif's RB
> > > > >
> > > > > v1 -> v2:
> > > > > * 1/4
> > > > >   - Improve commit log - mention single area size and new PEI stack base
> > > > >
> > > > > * 2/4 (new patch)
> > > > >   - Add common header for Marvell SMC ID's
> > > > >
> > > > > * 3/4
> > > > >   - Add function description comment
> > > > >   - Define and use ARMADA7K8K_AP806_INDEX
> > > > >   - Change function argument to EFI_PHYSICAL_ADDRESS
> > > > >
> > > > > * 4/4
> > > > >   - Move new SMC ID to MvSmc.h
> > > > >   - Include ArmadaSoCDescLib.h directly (instead indirectly via
> > > > BoardDesc.h)
> > > > >   - Remove ARMADA7K8K_AP806_INDEX macro
> > > > >
> > > > > Grzegorz Jaszczyk (2):
> > > > >   Marvell/Library: ArmadaSoCDescLib: Add North Bridge description
> > > > >   Marvell/Armada7k8k: Read DRAM settings from ARM-TF
> > > > >
> > > > > Marcin Wojtas (3):
> > > > >   Marvell/Armada7k8k: Refactor reserving memory regions
> > > > >   Marvell/Armada7k8k: Shift PEI stack base and extend memory reservation
> > > > >   Marvell/Library: Introduce common header for the SMC ID's
> > > > >
> > > > >  Silicon/Marvell/Marvell.dec
> > > >                       |   8 +-
> > > > >  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > > >                       |  16 ++-
> > > > >  Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf
> > > >                      |   3 +
> > > > >
> > > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kMemoryInitPeiLib/Armada7k8kMemoryInitPeiLib.inf
> > > > |   8 +-
> > > > >  Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h
> > > >                       |  25 -----
> > > > >
> > > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
> > > >              |   6 ++
> > > > >  Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> > > >                      |  24 +++++
> > > > >  Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
> > > >                      |  28 +++++
> > > > >  Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h
> > > >                      |   8 +-
> > > > >  Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c
> > > >                       |  60 ++++-------
> > > > >
> > > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kMemoryInitPeiLib/Armada7k8kMemoryInitPeiLib.c
> > > >  | 107 +++++++++++++-------
> > > > >
> > > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
> > > >              |  34 +++++++
> > > > >  Silicon/Marvell/Library/ComPhyLib/ComPhyCp110.c
> > > >                       |  14 +--
> > > > >  13 files changed, 220 insertions(+), 121 deletions(-)
> > > > >  create mode 100644 Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> > > > >
> > > > > --
> > > > > 2.7.4
> > > > >
> > > >