* [platforms: PATCH 0/3] Armada7k8k memory handling update
@ 2019-01-21 10:52 Marcin Wojtas
2019-01-21 10:52 ` [platforms: PATCH 1/3] Marvell: Armada7k8k: Shift PEI stack base Marcin Wojtas
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Marcin Wojtas @ 2019-01-21 10:52 UTC (permalink / raw)
To: edk2-devel; +Cc: leif.lindholm, ard.biesheuvel, nadavh, mw, jsd, jaz, kostap
Hi,
This short patchset adjust to the latest Marvell v18.12
ARM-TF sources. They change DRAM configuration
registers to be secure, as well as extend the region,
which is non-accessible by OS. The patches avoid
the issues by shifitng PEI stack base and configuring
the memory map, using SiP services.
Patches are available in the github:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/dram-upstream-r20190121
I'm looking forward to the comments and remarks.
Best regards,
Marcin
Grzegorz Jaszczyk (2):
Marvell/Library: ArmadaSoCDescLib: Add North Bridge description
Marvell/Armada7k8k: Read DRAM settings from ARM-TF
Marcin Wojtas (1):
Marvell: Armada7k8k: Shift PEI stack base
Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 4 +-
Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf | 3 ++
Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h | 27 ++--------
Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h | 1 +
Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h | 10 ++++
Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c | 55 +++++---------------
Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c | 17 ++++++
7 files changed, 51 insertions(+), 66 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread* [platforms: PATCH 1/3] Marvell: Armada7k8k: Shift PEI stack base 2019-01-21 10:52 [platforms: PATCH 0/3] Armada7k8k memory handling update Marcin Wojtas @ 2019-01-21 10:52 ` Marcin Wojtas 2019-01-21 11:26 ` Leif Lindholm 2019-01-21 10:52 ` [platforms: PATCH 2/3] Marvell/Library: ArmadaSoCDescLib: Add North Bridge description Marcin Wojtas 2019-01-21 10:52 ` [platforms: PATCH 3/3] Marvell/Armada7k8k: Read DRAM settings from ARM-TF Marcin Wojtas 2 siblings, 1 reply; 12+ messages in thread From: Marcin Wojtas @ 2019-01-21 10:52 UTC (permalink / raw) To: edk2-devel; +Cc: leif.lindholm, ard.biesheuvel, nadavh, mw, jsd, jaz, kostap 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 and set the PEI stack base address between both images. 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [platforms: PATCH 1/3] Marvell: Armada7k8k: Shift PEI stack base 2019-01-21 10:52 ` [platforms: PATCH 1/3] Marvell: Armada7k8k: Shift PEI stack base Marcin Wojtas @ 2019-01-21 11:26 ` Leif Lindholm 2019-01-21 15:34 ` Marcin Wojtas 0 siblings, 1 reply; 12+ messages in thread From: Leif Lindholm @ 2019-01-21 11:26 UTC (permalink / raw) To: Marcin Wojtas; +Cc: edk2-devel, ard.biesheuvel, nadavh, jsd, jaz, kostap On Mon, Jan 21, 2019 at 11:52:09AM +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 and set the PEI stack What is the single area? > base address between both images. > > 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 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [platforms: PATCH 1/3] Marvell: Armada7k8k: Shift PEI stack base 2019-01-21 11:26 ` Leif Lindholm @ 2019-01-21 15:34 ` Marcin Wojtas 2019-01-21 15:44 ` Leif Lindholm 0 siblings, 1 reply; 12+ messages in thread From: Marcin Wojtas @ 2019-01-21 15:34 UTC (permalink / raw) To: Leif Lindholm Cc: edk2-devel-01, Ard Biesheuvel, nadavh, jsd@semihalf.com, Grzegorz Jaszczyk, Kostya Porotchkin Hi Leif, pon., 21 sty 2019 o 12:26 Leif Lindholm <leif.lindholm@linaro.org> napisał(a): > > On Mon, Jan 21, 2019 at 11:52:09AM +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 and set the PEI stack > > What is the single area? The single region is set to: 0x4000000 - 0x5400000 PEI stack base is shifted right below the OPTEE region, i.e. to: 0x43F0000 Do you wish to add above to the commit message as well? Best regards, Marcin > > > base address between both images. > > > > 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 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [platforms: PATCH 1/3] Marvell: Armada7k8k: Shift PEI stack base 2019-01-21 15:34 ` Marcin Wojtas @ 2019-01-21 15:44 ` Leif Lindholm 0 siblings, 0 replies; 12+ messages in thread From: Leif Lindholm @ 2019-01-21 15:44 UTC (permalink / raw) To: Marcin Wojtas Cc: edk2-devel-01, Ard Biesheuvel, nadavh, jsd@semihalf.com, Grzegorz Jaszczyk, Kostya Porotchkin On Mon, Jan 21, 2019 at 04:34:39PM +0100, Marcin Wojtas wrote: > Hi Leif, > > > pon., 21 sty 2019 o 12:26 Leif Lindholm <leif.lindholm@linaro.org> napisał(a): > > > > On Mon, Jan 21, 2019 at 11:52:09AM +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 and set the PEI stack > > > > What is the single area? > > The single region is set to: > 0x4000000 - 0x5400000 > > PEI stack base is shifted right below the OPTEE region, i.e. to: > 0x43F0000 > > Do you wish to add above to the commit message as well? Yes please. Best Regards, Leif > Best regards, > Marcin > > > > > > base address between both images. > > > > > > 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 > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [platforms: PATCH 2/3] Marvell/Library: ArmadaSoCDescLib: Add North Bridge description 2019-01-21 10:52 [platforms: PATCH 0/3] Armada7k8k memory handling update Marcin Wojtas 2019-01-21 10:52 ` [platforms: PATCH 1/3] Marvell: Armada7k8k: Shift PEI stack base Marcin Wojtas @ 2019-01-21 10:52 ` Marcin Wojtas 2019-01-21 11:32 ` Leif Lindholm 2019-01-21 10:52 ` [platforms: PATCH 3/3] Marvell/Armada7k8k: Read DRAM settings from ARM-TF Marcin Wojtas 2 siblings, 1 reply; 12+ messages in thread From: Marcin Wojtas @ 2019-01-21 10:52 UTC (permalink / raw) To: edk2-devel; +Cc: leif.lindholm, ard.biesheuvel, nadavh, mw, jsd, jaz, kostap From: Grzegorz Jaszczyk <jaz@semihalf.com> For upcomming patch there is need to get AP806 base, provide required getter function for it. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Marcin Wojtas <mw@semihalf.com> --- Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h | 1 + Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h | 10 ++++++++++ Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c | 17 +++++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h index bfc8639..6caee6c 100644 --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h @@ -22,6 +22,7 @@ // Common macros // #define MV_SOC_CP_BASE(Cp) (0xF2000000 + ((Cp) * 0x2000000)) +#define MV_SOC_AP806_BASE 0xF0000000 #define MV_SOC_AP806_COUNT 1 // diff --git a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h index 26b075a..7aec9be 100644 --- a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h +++ b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h @@ -20,6 +20,16 @@ #include <Protocol/EmbeddedGpio.h> // +// North Bridge description +// +EFI_STATUS +EFIAPI +ArmadaSoCAp8xxBaseGet ( + IN OUT UINT64 *ApBase, + IN UINTN ApIndex + ); + +// // ComPhy SoC description // typedef struct { diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c index 5b72c20..089ac2d 100644 --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c @@ -30,6 +30,23 @@ EFI_STATUS EFIAPI +ArmadaSoCAp8xxBaseGet ( + IN OUT UINT64 *ApBase, + IN UINTN ApIndex + ) +{ + if (ApIndex != 0) { + DEBUG ((DEBUG_ERROR, "%a: Only one AP806 in A7K/A8K SoC\n", __FUNCTION__)); + return EFI_INVALID_PARAMETER; + } + + *ApBase = MV_SOC_AP806_BASE; + + return EFI_SUCCESS; +} + +EFI_STATUS +EFIAPI ArmadaSoCDescComPhyGet ( IN OUT MV_SOC_COMPHY_DESC **ComPhyDesc, IN OUT UINTN *DescCount -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [platforms: PATCH 2/3] Marvell/Library: ArmadaSoCDescLib: Add North Bridge description 2019-01-21 10:52 ` [platforms: PATCH 2/3] Marvell/Library: ArmadaSoCDescLib: Add North Bridge description Marcin Wojtas @ 2019-01-21 11:32 ` Leif Lindholm 2019-01-21 15:35 ` Marcin Wojtas 0 siblings, 1 reply; 12+ messages in thread From: Leif Lindholm @ 2019-01-21 11:32 UTC (permalink / raw) To: Marcin Wojtas; +Cc: edk2-devel, ard.biesheuvel, nadavh, jsd, jaz, kostap On Mon, Jan 21, 2019 at 11:52:10AM +0100, Marcin Wojtas wrote: > From: Grzegorz Jaszczyk <jaz@semihalf.com> > > For upcomming patch there is need to get AP806 base, provide required > getter function for it. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > --- > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h | 1 + > Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h | 10 ++++++++++ > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c | 17 +++++++++++++++++ > 3 files changed, 28 insertions(+) > > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > index bfc8639..6caee6c 100644 > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > @@ -22,6 +22,7 @@ > // Common macros > // > #define MV_SOC_CP_BASE(Cp) (0xF2000000 + ((Cp) * 0x2000000)) > +#define MV_SOC_AP806_BASE 0xF0000000 > #define MV_SOC_AP806_COUNT 1 > > // > diff --git a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > index 26b075a..7aec9be 100644 > --- a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > +++ b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > @@ -20,6 +20,16 @@ > #include <Protocol/EmbeddedGpio.h> > > // > +// North Bridge description > +// > +EFI_STATUS > +EFIAPI > +ArmadaSoCAp8xxBaseGet ( > + IN OUT UINT64 *ApBase, > + IN UINTN ApIndex > + ); > + > +// > // ComPhy SoC description > // > typedef struct { > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > index 5b72c20..089ac2d 100644 > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > @@ -30,6 +30,23 @@ > As I said, I was going to get stricter about adding function description comments - please add one here. (And clone it to ArmadaSoCDescLib.h, for the obious place to go look for a template when implementing a variant for a new platform.) > EFI_STATUS > EFIAPI > +ArmadaSoCAp8xxBaseGet ( > + IN OUT UINT64 *ApBase, > + IN UINTN ApIndex > + ) > +{ > + if (ApIndex != 0) { This test should be using ARMADA7K8K_AP806_INDEX, and that definition should be added to this patch. > + DEBUG ((DEBUG_ERROR, "%a: Only one AP806 in A7K/A8K SoC\n", __FUNCTION__)); > + return EFI_INVALID_PARAMETER; > + } > + > + *ApBase = MV_SOC_AP806_BASE; > + > + return EFI_SUCCESS; > +} > + > +EFI_STATUS > +EFIAPI > ArmadaSoCDescComPhyGet ( > IN OUT MV_SOC_COMPHY_DESC **ComPhyDesc, > IN OUT UINTN *DescCount > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [platforms: PATCH 2/3] Marvell/Library: ArmadaSoCDescLib: Add North Bridge description 2019-01-21 11:32 ` Leif Lindholm @ 2019-01-21 15:35 ` Marcin Wojtas 0 siblings, 0 replies; 12+ messages in thread From: Marcin Wojtas @ 2019-01-21 15:35 UTC (permalink / raw) To: Leif Lindholm Cc: edk2-devel-01, Ard Biesheuvel, nadavh, jsd@semihalf.com, Grzegorz Jaszczyk, Kostya Porotchkin Hi Leif, pon., 21 sty 2019 o 12:32 Leif Lindholm <leif.lindholm@linaro.org> napisał(a): > > On Mon, Jan 21, 2019 at 11:52:10AM +0100, Marcin Wojtas wrote: > > From: Grzegorz Jaszczyk <jaz@semihalf.com> > > > > For upcomming patch there is need to get AP806 base, provide required > > getter function for it. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > > --- > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h | 1 + > > Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h | 10 ++++++++++ > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c | 17 +++++++++++++++++ > > 3 files changed, 28 insertions(+) > > > > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > > index bfc8639..6caee6c 100644 > > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h > > @@ -22,6 +22,7 @@ > > // Common macros > > // > > #define MV_SOC_CP_BASE(Cp) (0xF2000000 + ((Cp) * 0x2000000)) > > +#define MV_SOC_AP806_BASE 0xF0000000 > > #define MV_SOC_AP806_COUNT 1 > > > > // > > diff --git a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > > index 26b075a..7aec9be 100644 > > --- a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > > +++ b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h > > @@ -20,6 +20,16 @@ > > #include <Protocol/EmbeddedGpio.h> > > > > // > > +// North Bridge description > > +// > > +EFI_STATUS > > +EFIAPI > > +ArmadaSoCAp8xxBaseGet ( > > + IN OUT UINT64 *ApBase, > > + IN UINTN ApIndex > > + ); > > + > > +// > > // ComPhy SoC description > > // > > typedef struct { > > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > > index 5b72c20..089ac2d 100644 > > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c > > @@ -30,6 +30,23 @@ > > > > As I said, I was going to get stricter about adding function > description comments - please add one here. (And clone it to > ArmadaSoCDescLib.h, for the obious place to go look for a template > when implementing a variant for a new platform.) Sure, I will add it. > > > EFI_STATUS > > EFIAPI > > +ArmadaSoCAp8xxBaseGet ( > > + IN OUT UINT64 *ApBase, > > + IN UINTN ApIndex > > + ) > > +{ > > + if (ApIndex != 0) { > > This test should be using ARMADA7K8K_AP806_INDEX, and that definition > should be added to this patch. > Ok. Thanks, Marcin > > + DEBUG ((DEBUG_ERROR, "%a: Only one AP806 in A7K/A8K SoC\n", __FUNCTION__)); > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + *ApBase = MV_SOC_AP806_BASE; > > + > > + return EFI_SUCCESS; > > +} > > + > > +EFI_STATUS > > +EFIAPI > > ArmadaSoCDescComPhyGet ( > > IN OUT MV_SOC_COMPHY_DESC **ComPhyDesc, > > IN OUT UINTN *DescCount > > -- > > 2.7.4 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [platforms: PATCH 3/3] Marvell/Armada7k8k: Read DRAM settings from ARM-TF 2019-01-21 10:52 [platforms: PATCH 0/3] Armada7k8k memory handling update Marcin Wojtas 2019-01-21 10:52 ` [platforms: PATCH 1/3] Marvell: Armada7k8k: Shift PEI stack base Marcin Wojtas 2019-01-21 10:52 ` [platforms: PATCH 2/3] Marvell/Library: ArmadaSoCDescLib: Add North Bridge description Marcin Wojtas @ 2019-01-21 10:52 ` Marcin Wojtas 2019-01-21 11:51 ` Leif Lindholm 2 siblings, 1 reply; 12+ messages in thread From: Marcin Wojtas @ 2019-01-21 10:52 UTC (permalink / raw) To: edk2-devel; +Cc: leif.lindholm, ard.biesheuvel, nadavh, mw, jsd, jaz, kostap From: Grzegorz Jaszczyk <jaz@semihalf.com> The memory controller registers are marked as secure in the latest ARM-TF for Armada SoCs. It is available however get the DRAM information via SiP services in the EL3, so use it instead of accessing the registers directly. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Marcin Wojtas <mw@semihalf.com> --- Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf | 3 ++ Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h | 27 ++-------- Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c | 55 +++++--------------- 3 files changed, 21 insertions(+), 64 deletions(-) diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf index e888566..0c7f320 100644 --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf @@ -41,12 +41,15 @@ [Packages] ArmPkg/ArmPkg.dec ArmPlatformPkg/ArmPlatformPkg.dec + EmbeddedPkg/EmbeddedPkg.dec MdeModulePkg/MdeModulePkg.dec MdePkg/MdePkg.dec Silicon/Marvell/Marvell.dec [LibraryClasses] + ArmadaSoCDescLib ArmLib + ArmSmcLib DebugLib MemoryAllocationLib MppLib diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h index cc30e4a..8a46df6 100644 --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h @@ -47,27 +47,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #define DRAM_REMAP_TARGET \ (MmioRead32 (CCU_MC_RTBR_REG) << TARGET_BASE_OFFS) -#define DRAM_CH0_MMAP_LOW_REG(cs) (0xf0020200 + (cs) * 0x8) -#define DRAM_CS_VALID_ENABLED_MASK 0x1 -#define DRAM_AREA_LENGTH_OFFS 16 -#define DRAM_AREA_LENGTH_MASK (0x1f << DRAM_AREA_LENGTH_OFFS) -#define DRAM_START_ADDRESS_L_OFFS 23 -#define DRAM_START_ADDRESS_L_MASK (0x1ff << DRAM_START_ADDRESS_L_OFFS) -#define DRAM_CH0_MMAP_HIGH_REG(cs) (0xf0020204 + (cs) * 0x8) -#define DRAM_START_ADDR_HTOL_OFFS 32 +/* Armada7k8k North Bridge index */ +#define ARMADA7K8K_AP806_INDEX 0 -#define DRAM_MAX_CS_NUM 8 - -#define DRAM_CS_ENABLED(Cs) \ - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & DRAM_CS_VALID_ENABLED_MASK) -#define GET_DRAM_REGION_BASE(Cs) \ - ((UINT64)MmioRead32 (DRAM_CH0_MMAP_HIGH_REG ((Cs))) << \ - DRAM_START_ADDR_HTOL_OFFS) | \ - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & DRAM_START_ADDRESS_L_MASK); -#define GET_DRAM_REGION_SIZE_CODE(Cs) \ - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG ((Cs))) & \ - DRAM_AREA_LENGTH_MASK) >> DRAM_AREA_LENGTH_OFFS -#define DRAM_REGION_SIZE_EVEN(C) (((C) >= 7) && ((C) <= 26)) -#define GET_DRAM_REGION_SIZE_EVEN(C) ((UINT64)1 << ((C) + 16)) -#define DRAM_REGION_SIZE_ODD(C) ((C) <= 4) -#define GET_DRAM_REGION_SIZE_ODD(C) ((UINT64)0x18000000 << (C)) +/* Firmware related definition used for SMC calls */ +#define MV_SIP_DRAM_SIZE 0x82000010 diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c index 2a4f5ad..62e8467 100644 --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c @@ -33,11 +33,14 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. *******************************************************************************/ #include <Base.h> +#include <IndustryStandard/ArmStdSmc.h> #include <Library/ArmPlatformLib.h> +#include <Library/ArmSmcLib.h> #include <Library/DebugLib.h> #include <Library/HobLib.h> #include <Library/IoLib.h> #include <Library/MemoryAllocationLib.h> +#include <Protocol/BoardDesc.h> #include "Armada7k8kLibMem.h" @@ -57,49 +60,19 @@ GetDramSize ( IN OUT UINT64 *MemSize ) { - UINT64 BaseAddr; - UINT8 RegionCode; - UINT8 Cs; - - *MemSize = 0; - - for (Cs = 0; Cs < DRAM_MAX_CS_NUM; Cs++) { - - /* Exit loop on first disabled DRAM CS */ - if (!DRAM_CS_ENABLED (Cs)) { - break; - } - - /* - * Sanity check for base address of next DRAM block. - * Only continuous space will be used. - */ - BaseAddr = GET_DRAM_REGION_BASE (Cs); - if (BaseAddr != *MemSize) { - DEBUG ((DEBUG_ERROR, - "%a: DRAM blocks are not contiguous, limit size to 0x%llx\n", - __FUNCTION__, - *MemSize)); - return EFI_SUCCESS; - } - - /* Decode area length for current CS from register value */ - RegionCode = GET_DRAM_REGION_SIZE_CODE (Cs); - - if (DRAM_REGION_SIZE_EVEN (RegionCode)) { - *MemSize += GET_DRAM_REGION_SIZE_EVEN (RegionCode); - } else if (DRAM_REGION_SIZE_ODD (RegionCode)) { - *MemSize += GET_DRAM_REGION_SIZE_ODD (RegionCode); - } else { - DEBUG ((DEBUG_ERROR, - "%a: Invalid memory region code (0x%x) for CS#%d\n", - __FUNCTION__, - RegionCode, - Cs)); - return EFI_INVALID_PARAMETER; - } + ARM_SMC_ARGS SmcRegs = {0}; + EFI_STATUS Status; + + SmcRegs.Arg0 = MV_SIP_DRAM_SIZE; + Status = ArmadaSoCAp8xxBaseGet (&SmcRegs.Arg1, ARMADA7K8K_AP806_INDEX); + if (EFI_ERROR (Status)) { + return Status; } + ArmCallSmc (&SmcRegs); + + *MemSize = SmcRegs.Arg0; + return EFI_SUCCESS; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [platforms: PATCH 3/3] Marvell/Armada7k8k: Read DRAM settings from ARM-TF 2019-01-21 10:52 ` [platforms: PATCH 3/3] Marvell/Armada7k8k: Read DRAM settings from ARM-TF Marcin Wojtas @ 2019-01-21 11:51 ` Leif Lindholm 2019-01-21 15:47 ` Marcin Wojtas 0 siblings, 1 reply; 12+ messages in thread From: Leif Lindholm @ 2019-01-21 11:51 UTC (permalink / raw) To: Marcin Wojtas; +Cc: edk2-devel, ard.biesheuvel, nadavh, jsd, jaz, kostap On Mon, Jan 21, 2019 at 11:52:11AM +0100, Marcin Wojtas wrote: > From: Grzegorz Jaszczyk <jaz@semihalf.com> > > The memory controller registers are marked as secure in the latest > ARM-TF for Armada SoCs. It is available however get the DRAM > information via SiP services in the EL3, so use it instead > of accessing the registers directly. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > --- > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf | 3 ++ > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h | 27 ++-------- > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c | 55 +++++--------------- > 3 files changed, 21 insertions(+), 64 deletions(-) > > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf > index e888566..0c7f320 100644 > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf > @@ -41,12 +41,15 @@ > [Packages] > ArmPkg/ArmPkg.dec > ArmPlatformPkg/ArmPlatformPkg.dec > + EmbeddedPkg/EmbeddedPkg.dec > MdeModulePkg/MdeModulePkg.dec > MdePkg/MdePkg.dec > Silicon/Marvell/Marvell.dec > > [LibraryClasses] > + ArmadaSoCDescLib > ArmLib > + ArmSmcLib > DebugLib > MemoryAllocationLib > MppLib > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h > index cc30e4a..8a46df6 100644 > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h > @@ -47,27 +47,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > #define DRAM_REMAP_TARGET \ > (MmioRead32 (CCU_MC_RTBR_REG) << TARGET_BASE_OFFS) > > -#define DRAM_CH0_MMAP_LOW_REG(cs) (0xf0020200 + (cs) * 0x8) > -#define DRAM_CS_VALID_ENABLED_MASK 0x1 > -#define DRAM_AREA_LENGTH_OFFS 16 > -#define DRAM_AREA_LENGTH_MASK (0x1f << DRAM_AREA_LENGTH_OFFS) > -#define DRAM_START_ADDRESS_L_OFFS 23 > -#define DRAM_START_ADDRESS_L_MASK (0x1ff << DRAM_START_ADDRESS_L_OFFS) > -#define DRAM_CH0_MMAP_HIGH_REG(cs) (0xf0020204 + (cs) * 0x8) > -#define DRAM_START_ADDR_HTOL_OFFS 32 > +/* Armada7k8k North Bridge index */ > +#define ARMADA7K8K_AP806_INDEX 0 > > -#define DRAM_MAX_CS_NUM 8 > - > -#define DRAM_CS_ENABLED(Cs) \ > - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & DRAM_CS_VALID_ENABLED_MASK) > -#define GET_DRAM_REGION_BASE(Cs) \ > - ((UINT64)MmioRead32 (DRAM_CH0_MMAP_HIGH_REG ((Cs))) << \ > - DRAM_START_ADDR_HTOL_OFFS) | \ > - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & DRAM_START_ADDRESS_L_MASK); > -#define GET_DRAM_REGION_SIZE_CODE(Cs) \ > - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG ((Cs))) & \ > - DRAM_AREA_LENGTH_MASK) >> DRAM_AREA_LENGTH_OFFS > -#define DRAM_REGION_SIZE_EVEN(C) (((C) >= 7) && ((C) <= 26)) > -#define GET_DRAM_REGION_SIZE_EVEN(C) ((UINT64)1 << ((C) + 16)) > -#define DRAM_REGION_SIZE_ODD(C) ((C) <= 4) > -#define GET_DRAM_REGION_SIZE_ODD(C) ((UINT64)0x18000000 << (C)) > +/* Firmware related definition used for SMC calls */ > +#define MV_SIP_DRAM_SIZE 0x82000010 Hmm... This would end us up with having Marvell SMC calls spread across multiple files. Could you insert an intermediate patch which breaks out the ones from Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h into a separate (MarvellSmc.h?) include file? If you could further add _SMC_ID to the #defines (like in edk2 ArmPkg/Include/IndustryStandard/ArmStdSmc.h), that would make me very happy. (I'd be happy for you to drop _SIP, or keep it either side of the addition, as per your preference - we don't seem to have precedent here yet.) Regards, Leif > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > index 2a4f5ad..62e8467 100644 > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > @@ -33,11 +33,14 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > *******************************************************************************/ > > #include <Base.h> > +#include <IndustryStandard/ArmStdSmc.h> > #include <Library/ArmPlatformLib.h> > +#include <Library/ArmSmcLib.h> > #include <Library/DebugLib.h> > #include <Library/HobLib.h> > #include <Library/IoLib.h> > #include <Library/MemoryAllocationLib.h> > +#include <Protocol/BoardDesc.h> > > #include "Armada7k8kLibMem.h" > > @@ -57,49 +60,19 @@ GetDramSize ( > IN OUT UINT64 *MemSize > ) > { > - UINT64 BaseAddr; > - UINT8 RegionCode; > - UINT8 Cs; > - > - *MemSize = 0; > - > - for (Cs = 0; Cs < DRAM_MAX_CS_NUM; Cs++) { > - > - /* Exit loop on first disabled DRAM CS */ > - if (!DRAM_CS_ENABLED (Cs)) { > - break; > - } > - > - /* > - * Sanity check for base address of next DRAM block. > - * Only continuous space will be used. > - */ > - BaseAddr = GET_DRAM_REGION_BASE (Cs); > - if (BaseAddr != *MemSize) { > - DEBUG ((DEBUG_ERROR, > - "%a: DRAM blocks are not contiguous, limit size to 0x%llx\n", > - __FUNCTION__, > - *MemSize)); > - return EFI_SUCCESS; > - } > - > - /* Decode area length for current CS from register value */ > - RegionCode = GET_DRAM_REGION_SIZE_CODE (Cs); > - > - if (DRAM_REGION_SIZE_EVEN (RegionCode)) { > - *MemSize += GET_DRAM_REGION_SIZE_EVEN (RegionCode); > - } else if (DRAM_REGION_SIZE_ODD (RegionCode)) { > - *MemSize += GET_DRAM_REGION_SIZE_ODD (RegionCode); > - } else { > - DEBUG ((DEBUG_ERROR, > - "%a: Invalid memory region code (0x%x) for CS#%d\n", > - __FUNCTION__, > - RegionCode, > - Cs)); > - return EFI_INVALID_PARAMETER; > - } > + ARM_SMC_ARGS SmcRegs = {0}; > + EFI_STATUS Status; > + > + SmcRegs.Arg0 = MV_SIP_DRAM_SIZE; > + Status = ArmadaSoCAp8xxBaseGet (&SmcRegs.Arg1, ARMADA7K8K_AP806_INDEX); > + if (EFI_ERROR (Status)) { > + return Status; > } > > + ArmCallSmc (&SmcRegs); > + > + *MemSize = SmcRegs.Arg0; > + > return EFI_SUCCESS; > } > > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [platforms: PATCH 3/3] Marvell/Armada7k8k: Read DRAM settings from ARM-TF 2019-01-21 11:51 ` Leif Lindholm @ 2019-01-21 15:47 ` Marcin Wojtas 2019-01-21 16:05 ` Leif Lindholm 0 siblings, 1 reply; 12+ messages in thread From: Marcin Wojtas @ 2019-01-21 15:47 UTC (permalink / raw) To: Leif Lindholm Cc: edk2-devel-01, Ard Biesheuvel, nadavh, jsd@semihalf.com, Grzegorz Jaszczyk, Kostya Porotchkin Hi Leif, pon., 21 sty 2019 o 12:51 Leif Lindholm <leif.lindholm@linaro.org> napisał(a): > > On Mon, Jan 21, 2019 at 11:52:11AM +0100, Marcin Wojtas wrote: > > From: Grzegorz Jaszczyk <jaz@semihalf.com> > > > > The memory controller registers are marked as secure in the latest > > ARM-TF for Armada SoCs. It is available however get the DRAM > > information via SiP services in the EL3, so use it instead > > of accessing the registers directly. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > > --- > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf | 3 ++ > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h | 27 ++-------- > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c | 55 +++++--------------- > > 3 files changed, 21 insertions(+), 64 deletions(-) > > > > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf > > index e888566..0c7f320 100644 > > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf > > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf > > @@ -41,12 +41,15 @@ > > [Packages] > > ArmPkg/ArmPkg.dec > > ArmPlatformPkg/ArmPlatformPkg.dec > > + EmbeddedPkg/EmbeddedPkg.dec > > MdeModulePkg/MdeModulePkg.dec > > MdePkg/MdePkg.dec > > Silicon/Marvell/Marvell.dec > > > > [LibraryClasses] > > + ArmadaSoCDescLib > > ArmLib > > + ArmSmcLib > > DebugLib > > MemoryAllocationLib > > MppLib > > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h > > index cc30e4a..8a46df6 100644 > > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h > > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.h > > @@ -47,27 +47,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > > #define DRAM_REMAP_TARGET \ > > (MmioRead32 (CCU_MC_RTBR_REG) << TARGET_BASE_OFFS) > > > > -#define DRAM_CH0_MMAP_LOW_REG(cs) (0xf0020200 + (cs) * 0x8) > > -#define DRAM_CS_VALID_ENABLED_MASK 0x1 > > -#define DRAM_AREA_LENGTH_OFFS 16 > > -#define DRAM_AREA_LENGTH_MASK (0x1f << DRAM_AREA_LENGTH_OFFS) > > -#define DRAM_START_ADDRESS_L_OFFS 23 > > -#define DRAM_START_ADDRESS_L_MASK (0x1ff << DRAM_START_ADDRESS_L_OFFS) > > -#define DRAM_CH0_MMAP_HIGH_REG(cs) (0xf0020204 + (cs) * 0x8) > > -#define DRAM_START_ADDR_HTOL_OFFS 32 > > +/* Armada7k8k North Bridge index */ > > +#define ARMADA7K8K_AP806_INDEX 0 > > > > -#define DRAM_MAX_CS_NUM 8 > > - > > -#define DRAM_CS_ENABLED(Cs) \ > > - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & DRAM_CS_VALID_ENABLED_MASK) > > -#define GET_DRAM_REGION_BASE(Cs) \ > > - ((UINT64)MmioRead32 (DRAM_CH0_MMAP_HIGH_REG ((Cs))) << \ > > - DRAM_START_ADDR_HTOL_OFFS) | \ > > - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG (Cs)) & DRAM_START_ADDRESS_L_MASK); > > -#define GET_DRAM_REGION_SIZE_CODE(Cs) \ > > - (MmioRead32 (DRAM_CH0_MMAP_LOW_REG ((Cs))) & \ > > - DRAM_AREA_LENGTH_MASK) >> DRAM_AREA_LENGTH_OFFS > > -#define DRAM_REGION_SIZE_EVEN(C) (((C) >= 7) && ((C) <= 26)) > > -#define GET_DRAM_REGION_SIZE_EVEN(C) ((UINT64)1 << ((C) + 16)) > > -#define DRAM_REGION_SIZE_ODD(C) ((C) <= 4) > > -#define GET_DRAM_REGION_SIZE_ODD(C) ((UINT64)0x18000000 << (C)) > > +/* Firmware related definition used for SMC calls */ > > +#define MV_SIP_DRAM_SIZE 0x82000010 > > Hmm... > This would end us up with having Marvell SMC calls spread across > multiple files. Could you insert an intermediate patch which breaks out > the ones from Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h into a > separate (MarvellSmc.h?) include file? How about MvSmc.h ? I try to use 'Mv' prefix consistently, especially when adding new code. > > If you could further add _SMC_ID to the #defines (like in edk2 > ArmPkg/Include/IndustryStandard/ArmStdSmc.h), that would make me very > happy. (I'd be happy for you to drop _SIP, or keep it either side of > the addition, as per your preference - we don't seem to have precedent > here yet.) > How about below: #define MV_SMC_ID_COMPHY_POWER_ON 0x82000001 #define MV_SMC_ID_COMPHY_POWER_OFF 0x82000002 #define MV_SMC_ID_COMPHY_PLL_LOCK 0x82000003 ? Thanks, Marcin > Regards, > > Leif > > > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > > index 2a4f5ad..62e8467 100644 > > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > > @@ -33,11 +33,14 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > > *******************************************************************************/ > > > > #include <Base.h> > > +#include <IndustryStandard/ArmStdSmc.h> > > #include <Library/ArmPlatformLib.h> > > +#include <Library/ArmSmcLib.h> > > #include <Library/DebugLib.h> > > #include <Library/HobLib.h> > > #include <Library/IoLib.h> > > #include <Library/MemoryAllocationLib.h> > > +#include <Protocol/BoardDesc.h> > > > > #include "Armada7k8kLibMem.h" > > > > @@ -57,49 +60,19 @@ GetDramSize ( > > IN OUT UINT64 *MemSize > > ) > > { > > - UINT64 BaseAddr; > > - UINT8 RegionCode; > > - UINT8 Cs; > > - > > - *MemSize = 0; > > - > > - for (Cs = 0; Cs < DRAM_MAX_CS_NUM; Cs++) { > > - > > - /* Exit loop on first disabled DRAM CS */ > > - if (!DRAM_CS_ENABLED (Cs)) { > > - break; > > - } > > - > > - /* > > - * Sanity check for base address of next DRAM block. > > - * Only continuous space will be used. > > - */ > > - BaseAddr = GET_DRAM_REGION_BASE (Cs); > > - if (BaseAddr != *MemSize) { > > - DEBUG ((DEBUG_ERROR, > > - "%a: DRAM blocks are not contiguous, limit size to 0x%llx\n", > > - __FUNCTION__, > > - *MemSize)); > > - return EFI_SUCCESS; > > - } > > - > > - /* Decode area length for current CS from register value */ > > - RegionCode = GET_DRAM_REGION_SIZE_CODE (Cs); > > - > > - if (DRAM_REGION_SIZE_EVEN (RegionCode)) { > > - *MemSize += GET_DRAM_REGION_SIZE_EVEN (RegionCode); > > - } else if (DRAM_REGION_SIZE_ODD (RegionCode)) { > > - *MemSize += GET_DRAM_REGION_SIZE_ODD (RegionCode); > > - } else { > > - DEBUG ((DEBUG_ERROR, > > - "%a: Invalid memory region code (0x%x) for CS#%d\n", > > - __FUNCTION__, > > - RegionCode, > > - Cs)); > > - return EFI_INVALID_PARAMETER; > > - } > > + ARM_SMC_ARGS SmcRegs = {0}; > > + EFI_STATUS Status; > > + > > + SmcRegs.Arg0 = MV_SIP_DRAM_SIZE; > > + Status = ArmadaSoCAp8xxBaseGet (&SmcRegs.Arg1, ARMADA7K8K_AP806_INDEX); > > + if (EFI_ERROR (Status)) { > > + return Status; > > } > > > > + ArmCallSmc (&SmcRegs); > > + > > + *MemSize = SmcRegs.Arg0; > > + > > return EFI_SUCCESS; > > } > > > > -- > > 2.7.4 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [platforms: PATCH 3/3] Marvell/Armada7k8k: Read DRAM settings from ARM-TF 2019-01-21 15:47 ` Marcin Wojtas @ 2019-01-21 16:05 ` Leif Lindholm 0 siblings, 0 replies; 12+ messages in thread From: Leif Lindholm @ 2019-01-21 16:05 UTC (permalink / raw) To: Marcin Wojtas Cc: edk2-devel-01, Ard Biesheuvel, nadavh, jsd@semihalf.com, Grzegorz Jaszczyk, Kostya Porotchkin On Mon, Jan 21, 2019 at 04:47:31PM +0100, Marcin Wojtas wrote: > > > -#define DRAM_REGION_SIZE_EVEN(C) (((C) >= 7) && ((C) <= 26)) > > > -#define GET_DRAM_REGION_SIZE_EVEN(C) ((UINT64)1 << ((C) + 16)) > > > -#define DRAM_REGION_SIZE_ODD(C) ((C) <= 4) > > > -#define GET_DRAM_REGION_SIZE_ODD(C) ((UINT64)0x18000000 << (C)) > > > +/* Firmware related definition used for SMC calls */ > > > +#define MV_SIP_DRAM_SIZE 0x82000010 > > > > Hmm... > > This would end us up with having Marvell SMC calls spread across > > multiple files. Could you insert an intermediate patch which breaks out > > the ones from Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h into a > > separate (MarvellSmc.h?) include file? > > How about MvSmc.h ? I try to use 'Mv' prefix consistently, especially > when adding new code. Sure. > > > > If you could further add _SMC_ID to the #defines (like in edk2 > > ArmPkg/Include/IndustryStandard/ArmStdSmc.h), that would make me very > > happy. (I'd be happy for you to drop _SIP, or keep it either side of > > the addition, as per your preference - we don't seem to have precedent > > here yet.) > > > > How about below: > #define MV_SMC_ID_COMPHY_POWER_ON 0x82000001 > #define MV_SMC_ID_COMPHY_POWER_OFF 0x82000002 > #define MV_SMC_ID_COMPHY_PLL_LOCK 0x82000003 > ? Yeah, that works. Regards, Leif > Thanks, > Marcin > > > > Regards, > > > > Leif > > > > > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > > > index 2a4f5ad..62e8467 100644 > > > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > > > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLibMem.c > > > @@ -33,11 +33,14 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > > > *******************************************************************************/ > > > > > > #include <Base.h> > > > +#include <IndustryStandard/ArmStdSmc.h> > > > #include <Library/ArmPlatformLib.h> > > > +#include <Library/ArmSmcLib.h> > > > #include <Library/DebugLib.h> > > > #include <Library/HobLib.h> > > > #include <Library/IoLib.h> > > > #include <Library/MemoryAllocationLib.h> > > > +#include <Protocol/BoardDesc.h> > > > > > > #include "Armada7k8kLibMem.h" > > > > > > @@ -57,49 +60,19 @@ GetDramSize ( > > > IN OUT UINT64 *MemSize > > > ) > > > { > > > - UINT64 BaseAddr; > > > - UINT8 RegionCode; > > > - UINT8 Cs; > > > - > > > - *MemSize = 0; > > > - > > > - for (Cs = 0; Cs < DRAM_MAX_CS_NUM; Cs++) { > > > - > > > - /* Exit loop on first disabled DRAM CS */ > > > - if (!DRAM_CS_ENABLED (Cs)) { > > > - break; > > > - } > > > - > > > - /* > > > - * Sanity check for base address of next DRAM block. > > > - * Only continuous space will be used. > > > - */ > > > - BaseAddr = GET_DRAM_REGION_BASE (Cs); > > > - if (BaseAddr != *MemSize) { > > > - DEBUG ((DEBUG_ERROR, > > > - "%a: DRAM blocks are not contiguous, limit size to 0x%llx\n", > > > - __FUNCTION__, > > > - *MemSize)); > > > - return EFI_SUCCESS; > > > - } > > > - > > > - /* Decode area length for current CS from register value */ > > > - RegionCode = GET_DRAM_REGION_SIZE_CODE (Cs); > > > - > > > - if (DRAM_REGION_SIZE_EVEN (RegionCode)) { > > > - *MemSize += GET_DRAM_REGION_SIZE_EVEN (RegionCode); > > > - } else if (DRAM_REGION_SIZE_ODD (RegionCode)) { > > > - *MemSize += GET_DRAM_REGION_SIZE_ODD (RegionCode); > > > - } else { > > > - DEBUG ((DEBUG_ERROR, > > > - "%a: Invalid memory region code (0x%x) for CS#%d\n", > > > - __FUNCTION__, > > > - RegionCode, > > > - Cs)); > > > - return EFI_INVALID_PARAMETER; > > > - } > > > + ARM_SMC_ARGS SmcRegs = {0}; > > > + EFI_STATUS Status; > > > + > > > + SmcRegs.Arg0 = MV_SIP_DRAM_SIZE; > > > + Status = ArmadaSoCAp8xxBaseGet (&SmcRegs.Arg1, ARMADA7K8K_AP806_INDEX); > > > + if (EFI_ERROR (Status)) { > > > + return Status; > > > } > > > > > > + ArmCallSmc (&SmcRegs); > > > + > > > + *MemSize = SmcRegs.Arg0; > > > + > > > return EFI_SUCCESS; > > > } > > > > > > -- > > > 2.7.4 > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-01-21 16:05 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-01-21 10:52 [platforms: PATCH 0/3] Armada7k8k memory handling update Marcin Wojtas 2019-01-21 10:52 ` [platforms: PATCH 1/3] Marvell: Armada7k8k: Shift PEI stack base Marcin Wojtas 2019-01-21 11:26 ` Leif Lindholm 2019-01-21 15:34 ` Marcin Wojtas 2019-01-21 15:44 ` Leif Lindholm 2019-01-21 10:52 ` [platforms: PATCH 2/3] Marvell/Library: ArmadaSoCDescLib: Add North Bridge description Marcin Wojtas 2019-01-21 11:32 ` Leif Lindholm 2019-01-21 15:35 ` Marcin Wojtas 2019-01-21 10:52 ` [platforms: PATCH 3/3] Marvell/Armada7k8k: Read DRAM settings from ARM-TF Marcin Wojtas 2019-01-21 11:51 ` Leif Lindholm 2019-01-21 15:47 ` Marcin Wojtas 2019-01-21 16:05 ` Leif Lindholm
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox