From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::242; helo=mail-io0-x242.google.com; envelope-from=ard.biesheuvel@linaro.org; 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 77AD6207E36CB for ; Fri, 1 Jun 2018 09:57:15 -0700 (PDT) Received: by mail-io0-x242.google.com with SMTP id e15-v6so22062063iog.1 for ; Fri, 01 Jun 2018 09:57:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=RvCQO1uvrQFKaMPwGl01sAIAtmgFv+0CkbzR+9Klxfc=; b=ABPXYdlxdE30CVj3X8LoZZJfx0gpMBY81qfcj+sGeARkgAX+NJMWg/o/7e0ho7Pamp QZghfp6QtHnyYRqKs953EBD0SVSOHXNEpIZjKMvzz8v0F2J08MnItAw/JugHcgr0UkLR L6tgvSpABkgZpjBtjoRnEfPPggR+vKFDweQsg= 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=RvCQO1uvrQFKaMPwGl01sAIAtmgFv+0CkbzR+9Klxfc=; b=uN40IZWos7t4r2wbqMTbdhdTaSwii6aOeeM/O1zqcr0WiI2nElfyzq2F0Oq55Lws27 OSmpDzxM47YGoJEO/4xPtuLgb3fLs/Tgg37h8s4+tWLZzuEwQ7inBTzqW6SkH2i/J9vP iRIH6tvL8o/gxLv7fKIOT7Eon3y4GUVpO4ugVioUy2rLcZB2m2G7bFZ6YI+eyYtvsjap cOkW4kLfJx8+fdWWOAt++a1z8HKXdIWNVE9KBoPxspLtduBMqhyFkarw/n7g4++sf3RF K+VQJag+laSv88Jzm7Gh0zl6QrQJXN2ewSrRfmLOu8YzJQRgdV2LWmeAbmfLtvbQfxzR 4Mhw== X-Gm-Message-State: APt69E26rfrZydjsFDm0R+m66bygJYVntG/aofcREvPqcHeS68YWxVxV W9X6rnRLgtaXpu/eAZrKkBLBTtjXNKRCuA4TXSeaFg== X-Google-Smtp-Source: ADUXVKIrXkehh5HqdI3E69D7SJ0E4ST41K4RT7sR1/QS5qBPjaULGiINQOUab7pMdZRcrLIxmHVO87eIi60ZBYyDmms= X-Received: by 2002:a6b:6709:: with SMTP id b9-v6mr11066631ioc.170.1527872234510; Fri, 01 Jun 2018 09:57:14 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bb86:0:0:0:0:0 with HTTP; Fri, 1 Jun 2018 09:57:13 -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: Ard Biesheuvel Date: Fri, 1 Jun 2018 18:57:13 +0200 Message-ID: To: Marcin Wojtas 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 16:57:15 -0000 Content-Type: text/plain; charset="UTF-8" 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? >> >> 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.