2018-06-01 18:57 GMT+02:00 Ard Biesheuvel <[email protected]>: > On 1 June 2018 at 18:43, Marcin Wojtas <[email protected]> wrote: >> 2018-06-01 17:30 GMT+02:00 Ard Biesheuvel <[email protected]>: >>> On 1 June 2018 at 16:32, Marcin Wojtas <[email protected]> wrote: >>>> PEI phase will allow to use more robust platform initialization, >>>> with new features like the capsule support. Wire up all >>>> dependencies for that purpose. >>>> >>>> Contributed-under: TianoCore Contribution Agreement 1.1 >>>> Signed-off-by: Marcin Wojtas <[email protected]> >>>> --- >>>> Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf | 15 ++++++-- >>>> Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 40 ++++++++++++++++++-- >>>> 2 files changed, 48 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf >>>> b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf >>>> index 69cb4cd..bf04f4d 100644 >>>> --- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf >>>> +++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf >>>> @@ -63,7 +63,7 @@ DATA = { >>>> !endif >>>> } >>>> >>>> -0x00001000|0x000ff000 >>>> +0x00001000|0x001ff000 >>>> gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize >>>> FV = FVMAIN_COMPACT >>>> >>>> @@ -221,7 +221,14 @@ READ_STATUS = TRUE >>>> READ_LOCK_CAP = TRUE >>>> READ_LOCK_STATUS = TRUE >>>> >>>> - INF ArmPlatformPkg/PrePi/PeiUniCore.inf >>>> + INF ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf >>>> + INF MdeModulePkg/Core/Pei/PeiMain.inf >>>> + INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf >>>> + INF ArmPlatformPkg/PlatformPei/PlatformPeim.inf >>>> + INF ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf >>>> + INF ArmPkg/Drivers/CpuPei/CpuPei.inf >>>> + INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf >>>> + INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf >>>> >>>> FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 { >>>> SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF >>>> PROCESSING_REQUIRED = TRUE { >>>> @@ -264,14 +271,14 @@ READ_LOCK_STATUS = TRUE >>>> >>>> [Rule.Common.PEI_CORE] >>>> FILE PEI_CORE = $(NAMED_GUID) { >>>> - TE TE $(INF_OUTPUT)/$(MODULE_NAME).efi >>>> + TE TE Align = Auto $(INF_OUTPUT)/$(MODULE_NAME).efi >>>> UI STRING ="$(MODULE_NAME)" Optional >>>> } >>>> >>>> [Rule.Common.PEIM] >>>> FILE PEIM = $(NAMED_GUID) { >>>> PEI_DEPEX PEI_DEPEX Optional $(INF_OUTPUT)/$(MODULE_NAME).depex >>>> - TE TE $(INF_OUTPUT)/$(MODULE_NAME).efi >>>> + TE TE Align = Auto $(INF_OUTPUT)/$(MODULE_NAME).efi >>>> UI STRING="$(MODULE_NAME)" Optional >>>> } >>>> >>>> diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc >>>> b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc >>>> index 4129742..195b6b7 100644 >>>> --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc >>>> +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc >>>> @@ -145,13 +145,28 @@ >>>> >>>> MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf >>>> HobLib|EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf >>>> >>>> PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf >>>> - PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf >>>> ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf >>>> PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf >>>> >>>> [LibraryClasses.common.SEC, LibraryClasses.common.PEIM] >>>> - >>>> MemoryInitPeiLib|Silicon/Marvell/Armada7k8k/Library/Armada7k8kMemoryInitPeiLib/Armada7k8kMemoryInitPeiLib.inf >>>> BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf >>>> + PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf >>>> + >>>> +[LibraryClasses.common.PEI_CORE, LibraryClasses.common.PEIM] >>>> + PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf >>>> + BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf >>>> + HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf >>>> + PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf >>>> + >>>> MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf >>>> + >>>> ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtractGuidedSectionLib.inf >>>> + >>>> PeiServicesTablePointerLib|ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf >>>> + >>>> +[LibraryClasses.common.PEI_CORE] >>>> + PeiCoreEntryPoint|MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf >>>> + >>>> +[LibraryClasses.common.PEIM] >>>> + PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf >>>> + >>>> MemoryInitPeiLib|Silicon/Marvell/Armada7k8k/Library/Armada7k8kMemoryInitPeiLib/Armada7k8kMemoryInitPeiLib.inf >>>> >>>> [LibraryClasses.common.DXE_CORE] >>>> HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf >>>> @@ -336,8 +351,13 @@ >>>> # ARM Pcds >>>> gArmTokenSpaceGuid.PcdSystemMemoryBase|0 >>>> gArmTokenSpaceGuid.PcdSystemMemorySize|0x40000000 >>>> + >>>> + gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|36 >>>> gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36 >>>> >>>> + gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x2E000000 >>>> + gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0xFFC0 >>>> + >>> >>> These were copy/pasted from SynQuacer, I suppose? >>> >> >> SynQuacer and ARM VExpress use this value, I also checked ones used in >> HiSilicon Platforms - I didn't notice any issue with neither of them. >> > > SynQuacer and ARM VExpress are both platforms designed by ARM Ltd, and > both have non-secure SRAM at this offset. > >>> Ideally, these should point to some memory that is not exposed to the >>> OS, so that the PEI phase cannot corrupt a capsule image that has been >>> left in DRAM by the OS. >>> >>> This only becomes relevant once we implement support for PSCI warm >>> reboot, otherwise DRAM will be cleared anyway. However, pointing these >>> into a random slice of main memory feels a little risky. >> >> >> Currently we have also a hole between 0xc0000000 - 4GB for the config >> space, so how about squeezing those 64kB there? >> >> Alternatively would you recommend to use value outside the DRAM space >> (e.g. above max DRAM size)? >> > > No, this region should point to usable memory, and ideally memory that > the OS does not use. So the options are (in order of preference) > - put the PEI stack and heap in non-secure SRAM > - put the PEI stack and heap in a slice of DRAM that is marked as > 'reserved' in the EFI memory map. > - put the PEI stack and heap in a slice of DRAM that is otherwise > protected from allocation until DxeCore launches. > > When the OS calls UpdateCapsule(), it will put the capsule at an > arbitrary location in memory, and if this happens to be the same > region PEI uses for its stack and heap, you will corrupt the capsule > before the capsule PEIM gets a chance to preserve it. > > At the moment, this does not make any difference given that we don't > implement warm reboot yet (but PSCI defines it now). Also, fwupdate in > Linux does not rely on UpdateCapsule() at OS runtime. However, M$ > Windows does, and I assume you don't want to make your UEFI > implementation Linux-only? >
Thanks for explanation. Not sure if I will be able to use the SRAM, otherwise I will extend the hole in DRAM, which is reserved for the PSCI and use it as the PEI stack and heap. > >>> >>> Do you have non-secure SRAM on this SoC? >> >> There are a couple of SRAMs (for offload CM3 CPUs), one configured as >> secure. Not sure about others. Why do you ask? >> > > Because that is the preferred place for the PEI stack and heap. _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
