From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=2607:f8b0:4001:c06::242; helo=mail-io0-x242.google.com; envelope-from=mw@semihalf.com; receiver=edk2-devel@lists.01.org Received: from mail-io0-x242.google.com (mail-io0-x242.google.com [IPv6:2607:f8b0:4001:c06::242]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 936DD210D6CC6 for ; Fri, 1 Jun 2018 10:18:48 -0700 (PDT) Received: by mail-io0-x242.google.com with SMTP id t6-v6so11932017iob.10 for ; Fri, 01 Jun 2018 10:18:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=3leUMxBuawigSzC352P2HhxOL27A5sWVtUprXbgLje8=; b=ahLMMOVyMfsGXw1/DuDUPlWtl8/RzStWlAaeG1KqsFq5raRkizZO+PzYmpJNyqJaDH eIzLLKlOYqusB+2/X3YfWMf13wk2mRrln4RlPbsxVkiqI89MqE3QKlgsb8R26crBgNDI 4d6GmN2etS+hFqKLBMzsMrzT6pK2vOlJgpcCbUSA6T0EfMeEi4ZXRcMT3ekuA83mbxjS soH/vP+H5FH1E6WZ8if4vbeHUtgFYDvOmn7vnWj8u2Fo8dsA7aKbwwiAVpTTjbmvwMqu buf6YdMYyMtvgz8RThG91ziG0mIqd1jtgovgwUnsC3SNQtdxmLNGdXzeNwSEK7jLrJpU 94Fg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=3leUMxBuawigSzC352P2HhxOL27A5sWVtUprXbgLje8=; b=t0f+OKX6Zi3vZyvb2smflR/1x4LrChz66QRpg/bvzt8xeMsacVgDDBWnfuffLiMk55 fHjJrwchwX4vwQpEgv98PclrKMF43K7nZWNNsb5GMcpNMrqrukS3qexR090LAJp8Au0G tcXL0pf8Sifka+Ega8nH4E8oVP+aPPG8Cubi7IycLfp+RLvtHG1G03g5E+ACmufudJo/ mkNkerl+fORL23co/Ecku19mvsyYeU/u3nUYr5sQfP8oYfc+pMi+AIWF6rYeb/Hj7ePz mtOO5WNrVIVzbPLTKwqfafAXFuz7ttctRjn7PhUW0IYB+LXXcWSNOBImUoouGF9Y40WL F6gw== X-Gm-Message-State: APt69E0s7+pRAxhjub6TgUFofeWrC8/BTQSk0GyKMqDApcKhQaKi41hO U+CN3IM2pWNIM9u09OVB37Wiz7qhGxmHRA1FaNmMrg== X-Google-Smtp-Source: ADUXVKJfTibZ/PmlsmvCMk04sSCBmTNMTOt4Qi9dE16KEgdpoR4rtXI8uVBENslb1Di3J+83TqSw2WWEmBcpNaImd4c= X-Received: by 2002:a6b:1884:: with SMTP id 126-v6mr1304015ioy.183.1527873527498; Fri, 01 Jun 2018 10:18:47 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:c6cc:0:0:0:0:0 with HTTP; Fri, 1 Jun 2018 10:18:46 -0700 (PDT) In-Reply-To: References: <1527863526-5494-1-git-send-email-mw@semihalf.com> <1527863526-5494-3-git-send-email-mw@semihalf.com> From: Marcin Wojtas Date: Fri, 1 Jun 2018 19:18:46 +0200 Message-ID: To: Ard Biesheuvel Cc: "edk2-devel@lists.01.org" , Leif Lindholm , =?UTF-8?B?SmFuIETEhWJyb8Wb?= , Hua Jing , Grzegorz Jaszczyk , David Sniatkiwicz Subject: Re: [platforms PATCH 2/4] Marvell/Aramda7k8k: Enable PEI booting stage X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 01 Jun 2018 17:18:49 -0000 Content-Type: text/plain; charset="UTF-8" 2018-06-01 18:57 GMT+02:00 Ard Biesheuvel : > On 1 June 2018 at 18:43, Marcin Wojtas wrote: >> 2018-06-01 17:30 GMT+02:00 Ard Biesheuvel : >>> On 1 June 2018 at 16:32, Marcin Wojtas 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 >>>> --- >>>> 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.