From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-x233.google.com (mail-lf0-x233.google.com [IPv6:2a00:1450:4010:c07::233]) (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 A64851A1E3A for ; Mon, 24 Oct 2016 05:05:52 -0700 (PDT) Received: by mail-lf0-x233.google.com with SMTP id f134so29489007lfg.2 for ; Mon, 24 Oct 2016 05:05:52 -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=F6WR3nfrZ2SFKYh4OgRdEy8Se2J6JMVY17bww5KmzEA=; b=KlloTCbYo6wGd1xBwgM2QPdrQjc1efVrgW7Eq+m/qY8+OisMmxA1RqvXFLfQwKBPOY Ql5HEWpqihoA8N/8sP2gANiacdGmiuH7+wZPoYWcD6DwFPJ+W6srbn7ptvg08acDTMY9 aR7L6LjS+EPrqUC6MztrUQrDs0HaTlk5h0/0M= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=F6WR3nfrZ2SFKYh4OgRdEy8Se2J6JMVY17bww5KmzEA=; b=gPJUKRopFPy73SzscbGrHNaosstFZFNqlw2SSPlTsPmTAXrl6yznSll7n3yKYenU4H oLy9hvXZDoVtiVOLTsAjpaB65jy0AAra+1AOA01qCwmWeYLH0xNZiLhtxvAAbGnzEL7o NbCyYWoaeUA9YDwW7tRhU/Tx+w8PMYz6H5IBWxpYq4HJ8D2M4qNbCtHFCHnWkwrV04wr JXrPvHABBXbJiIv8fQoGhKjIVhDzrPMjBoSjZ/Zvaz3ajINa2TR+J3hpKwpIyHxIG4Pm T7JWqKNKqRxKtcT6cLoUiEoBR72QqrQD+JcEGbkYdeT8pZ/b9W3ItDjL9Z/taR3W/TYo 4Inw== X-Gm-Message-State: ABUngvdrAmZmbY9eVi01GBKLr35EhH2SCzKc5ZeMULqR1LtTv7H/iB1pxODIHCAmGcf8H2Vd2m3Z8IxKjQXcq/1l X-Received: by 10.25.153.75 with SMTP id b72mr5314648lfe.112.1477310750674; Mon, 24 Oct 2016 05:05:50 -0700 (PDT) MIME-Version: 1.0 Received: by 10.25.28.78 with HTTP; Mon, 24 Oct 2016 05:05:49 -0700 (PDT) In-Reply-To: <1477299443-9324-2-git-send-email-ard.biesheuvel@linaro.org> References: <1477299443-9324-1-git-send-email-ard.biesheuvel@linaro.org> <1477299443-9324-2-git-send-email-ard.biesheuvel@linaro.org> From: Ryan Harkin Date: Mon, 24 Oct 2016 13:05:49 +0100 Message-ID: To: Ard Biesheuvel Cc: edk2-devel-01 , Leif Lindholm , Laszlo Ersek , Heyi Guo Subject: Re: [PATCH 2/2] ArmPlatformPkg/PrePi: avoid global variable write to mSystemMemoryEnd X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 24 Oct 2016 12:05:53 -0000 Content-Type: text/plain; charset=UTF-8 On 24 October 2016 at 09:57, Ard Biesheuvel wrote: > The global variable mSystemMemoryEnd is initialized by PrePi only if > it has not been initialized by ArmPlatformPeiBootAction(). This allows > platforms executing under, e.g., ARM Trusted Firmware to dynamically > reserve a window at the top of memory that will be used by the secure > firmware. > > However, PrePi is a SEC module, and writing to a global variable > violates the SEC constraints, since SEC and PEI may execute from NOR > flash. > > So instead, initialize mSystemMemoryEnd statically. This will ensure > it holds the correct value for all implementations where the value > is not overridden, but still allows it to be overridden during the > call to ArmPlatformPeiBootAction(). > > Note that this patch also fixes a latent bug on 32-bit platforms where > a value of mSystemMemoryEnd exceeding 4 GB would be truncated to 32-bits > rather than limited to (4 GB - 1) > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel I tested this successfully on FVP Foundation & AEMv8 models, Juno R0/1/2 and TC2. Tested-by: Ryan Harkin > --- > > Build tested only. > > ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S | 14 ------------- > ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S | 20 +++++-------------- > ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.asm | 21 ++++++-------------- > ArmPlatformPkg/PrePi/PrePi.c | 3 +++ > 4 files changed, 14 insertions(+), 44 deletions(-) > > diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S > index d0530a874726..a81709d5d12d 100644 > --- a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S > +++ b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S > @@ -13,8 +13,6 @@ > > #include > > -ASM_GLOBAL ASM_PFX(mSystemMemoryEnd) > - > ASM_FUNC(_ModuleEntryPoint) > // Do early platform specific actions > bl ASM_PFX(ArmPlatformPeiBootAction) > @@ -31,16 +29,6 @@ _SetSVCMode: > _SystemMemoryEndInit: > ldr x1, mSystemMemoryEnd > > - // Is mSystemMemoryEnd initialized? > - cmp x1, #0 > - bne _SetupStackPosition > - > - MOV64 (x1, FixedPcdGet64(PcdSystemMemoryBase) + FixedPcdGet64(PcdSystemMemorySize) - 1) > - > - // Update the global variable > - adr x2, mSystemMemoryEnd > - str x1, [x2] > - > _SetupStackPosition: > // r1 = SystemMemoryTop > > @@ -129,5 +117,3 @@ _PrepareArguments: > > _NeverReturn: > b _NeverReturn > - > -ASM_PFX(mSystemMemoryEnd): .8byte 0 > diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S > index 39030da5f2c3..212cab62d44b 100644 > --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S > +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S > @@ -15,8 +15,6 @@ > > #include > > -GCC_ASM_EXPORT(mSystemMemoryEnd) > - > ASM_FUNC(_ModuleEntryPoint) > // Do early platform specific actions > bl ASM_PFX(ArmPlatformPeiBootAction) > @@ -35,17 +33,11 @@ _SetSVCMode: > // to install the stacks at the bottom of the Firmware Device (case the FD is located > // at the top of the DRAM) > _SystemMemoryEndInit: > - ldr r1, mSystemMemoryEnd > - > - // Is mSystemMemoryEnd initialized? > - cmp r1, #0 > - bne _SetupStackPosition > - > - MOV32 (r1, FixedPcdGet32(PcdSystemMemoryBase) + FixedPcdGet32(PcdSystemMemorySize) - 1) > - > - // Update the global variable > - adr r2, mSystemMemoryEnd > - str r1, [r2] > + ADRL (r1, mSystemMemoryEnd) > + ldrd r2, r3, [r1] > + teq r3, #0 > + moveq r1, r2 > + mvnne r1, #0 > > _SetupStackPosition: > // r1 = SystemMemoryTop > @@ -136,5 +128,3 @@ _PrepareArguments: > > _NeverReturn: > b _NeverReturn > - > -ASM_PFX(mSystemMemoryEnd): .8byte 0 > diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.asm b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.asm > index 023339841f75..1e9daf563bb6 100644 > --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.asm > +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.asm > @@ -21,15 +21,14 @@ > IMPORT ArmReadMpidr > IMPORT ArmPlatformPeiBootAction > IMPORT ArmPlatformStackSet > + IMPORT mSystemMemoryEnd > > EXPORT _ModuleEntryPoint > - EXPORT mSystemMemoryEnd > > PRESERVE8 > AREA PrePiCoreEntryPoint, CODE, READONLY > > StartupAddr DCD CEntryPoint > -mSystemMemoryEnd DCQ 0 > > _ModuleEntryPoint > // Do early platform specific actions > @@ -49,19 +48,11 @@ _SetSVCMode > // to install the stacks at the bottom of the Firmware Device (case the FD is located > // at the top of the DRAM) > _SystemMemoryEndInit > - ldr r1, mSystemMemoryEnd > - > - // Is mSystemMemoryEnd initialized? > - cmp r1, #0 > - bne _SetupStackPosition > - > - mov32 r1, FixedPcdGet32(PcdSystemMemoryBase) > - mov32 r2, FixedPcdGet32(PcdSystemMemorySize) > - sub r2, r2, #1 > - add r1, r1, r2 > - // Update the global variable > - adr r2, mSystemMemoryEnd > - str r1, [r2] > + mov32 r1, mSystemMemoryEnd > + ldrd r2, r3, [r1] > + teq r3, #0 > + moveq r1, r2 > + mvnne r1, #0 > > _SetupStackPosition > // r1 = SystemMemoryTop > diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c > index 36928c65a73b..e548ccace097 100644 > --- a/ArmPlatformPkg/PrePi/PrePi.c > +++ b/ArmPlatformPkg/PrePi/PrePi.c > @@ -32,6 +32,9 @@ > #define IS_XIP() (((UINT64)FixedPcdGet64 (PcdFdBaseAddress) > mSystemMemoryEnd) || \ > ((FixedPcdGet64 (PcdFdBaseAddress) + FixedPcdGet32 (PcdFdSize)) < FixedPcdGet64 (PcdSystemMemoryBase))) > > +UINT64 mSystemMemoryEnd = FixedPcdGet64(PcdSystemMemoryBase) + > + FixedPcdGet64(PcdSystemMemorySize) - 1; > + > EFI_STATUS > EFIAPI > ExtractGuidedSectionLibConstructor ( > -- > 2.7.4 >