From patchwork Sat Oct 22 08:28:17 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ard Biesheuvel X-Patchwork-Id: 78763 Delivered-To: patch@linaro.org Received: by 10.140.97.247 with SMTP id m110csp1684194qge; Sat, 22 Oct 2016 01:28:24 -0700 (PDT) X-Received: by 10.200.47.61 with SMTP id j58mr5366657qta.106.1477124904836; Sat, 22 Oct 2016 01:28:24 -0700 (PDT) Return-Path: Received: from lists.linaro.org (lists.linaro.org. [54.225.227.206]) by mx.google.com with ESMTP id d10si4465746qkc.278.2016.10.22.01.28.24; Sat, 22 Oct 2016 01:28:24 -0700 (PDT) Received-SPF: pass (google.com: domain of linaro-uefi-bounces@lists.linaro.org designates 54.225.227.206 as permitted sender) client-ip=54.225.227.206; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linaro-uefi-bounces@lists.linaro.org designates 54.225.227.206 as permitted sender) smtp.mailfrom=linaro-uefi-bounces@lists.linaro.org; dmarc=pass (p=NONE dis=NONE) header.from=linaro.org Received: by lists.linaro.org (Postfix, from userid 109) id 5565560A1E; Sat, 22 Oct 2016 08:28:24 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on ip-10-142-244-252 X-Spam-Level: X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL autolearn=disabled version=3.4.0 Received: from [127.0.0.1] (localhost [127.0.0.1]) by lists.linaro.org (Postfix) with ESMTP id 84E1B606F3; Sat, 22 Oct 2016 08:28:20 +0000 (UTC) X-Original-To: Linaro-uefi@lists.linaro.org Delivered-To: Linaro-uefi@lists.linaro.org Received: by lists.linaro.org (Postfix, from userid 109) id 0909F60673; Sat, 22 Oct 2016 08:28:19 +0000 (UTC) Received: from mail-it0-f47.google.com (mail-it0-f47.google.com [209.85.214.47]) by lists.linaro.org (Postfix) with ESMTPS id 232C260673 for ; Sat, 22 Oct 2016 08:28:18 +0000 (UTC) Received: by mail-it0-f47.google.com with SMTP id 198so17255231itw.0 for ; Sat, 22 Oct 2016 01:28:18 -0700 (PDT) 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=XYle38EL+85x3atYD1qL8YHURWfjpsaLTTktdUyNpUA=; b=duQq/0P6miCip1Ca+CSZw7N/raK2d96YZyCbt2+M+LvjMOkESdR5ydDspO+Avj1VhJ Jk7FvdR9cxzSoCgyWO6FQO4wV4leyK9wX/lU7HomzqUZabjtmy28Oyig0Ji1TFrUUbEZ ke6OPTpVYoWIHfFGZU577MdK0BwHL9mzmNuwYRKJ+7fOwaorGI7ECSkRS5g7B4Wwpx05 K6MWKxGc+d6uRExLpCRNMSEc8F+oOhwBzpJHg/XBoOyjhUT7mZxxxmlYjwaZb9uLBtcc pJdcMhWe6ENWnvRSmjigsAGwQFjogMDd4hMqc4AUTyKZNPJs/LBudhE5nDnOPRQpbwPf GUJQ== X-Gm-Message-State: ABUngvck1jiZHwF3vnZA1HLhxmF8eqyFJ6dkkIx89HyOmjR6HumeybexFUwvgzjSZW1152/qp0RQU2zPPU+b5g5YKwc= X-Received: by 10.36.158.194 with SMTP id p185mr2191408itd.37.1477124897620; Sat, 22 Oct 2016 01:28:17 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.5.139 with HTTP; Sat, 22 Oct 2016 01:28:17 -0700 (PDT) In-Reply-To: References: From: Ard Biesheuvel Date: Sat, 22 Oct 2016 09:28:17 +0100 Message-ID: To: Heyi Guo Cc: Linaro UEFI Mailman List Subject: Re: [Linaro-uefi] Global variable write in PrePi X-BeenThere: linaro-uefi@lists.linaro.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linaro-uefi-bounces@lists.linaro.org Sender: "Linaro-uefi" On 22 October 2016 at 09:09, Ard Biesheuvel wrote: > On 22 October 2016 at 04:21, Heyi Guo wrote: >> Hi folks, >> >> We are still using PrePi on some of our platforms and the code is still >> running on Flash at this stage. We found there is global data write in >> ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S. The change was introduced >> by this commit: 5dbacdb21b59748e885c2eccae370b81271ab795: >> >> +mSystemMemoryEnd: .8byte 0^M >> >> ASM_PFX(_ModuleEntryPoint): >> // Do early platform specific actions >> @@ -40,12 +42,23 @@ _SetSVCMode: >> // Check if we can install the stack at the top of the System Memory or if >> we need >> // to install the stacks at the bottom of the Firmware Device (case the FD >> is located >> // at the top of the DRAM) >> -_SetupStackPosition: >> - // Compute Top of System Memory >> - LoadConstantToReg (FixedPcdGet64 (PcdSystemMemoryBase), x1) >> - LoadConstantToReg (FixedPcdGet64 (PcdSystemMemorySize), x2) >> +_SystemMemoryEndInit:^M >> + ldr x1, mSystemMemoryEnd^M >> +^M >> + // Is mSystemMemoryEnd initialized?^M >> + cmp x1, #0^M >> + bne _SetupStackPosition^M >> +^M >> + LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryBase), x1)^M >> + LoadConstantToReg (FixedPcdGet32(PcdSystemMemorySize), x2)^M >> sub x2, x2, #1 >> - add x1, x1, x2 // x1 = SystemMemoryTop = PcdSystemMemoryBase + >> PcdSystemMemorySize >> + add x1, x1, x2^M >> + // Update the global variable^M >> + adr x2, mSystemMemoryEnd^M >> + str x1, [x2]^M >> >> I think direct write to flash should be forbidden. This change may work well >> for platforms which use ATF and load PrePi into memory, but not for other >> platforms. >> >> Please let me know your comments about this. >> > > You are right. This should be removed. I think it was added for ATF on > Juno, but obviously, this is not the right way to do it, given that > SEC and PEIM modules may run from NOR. Does this work for you? _SetupStackPosition: // r1 = SystemMemoryTop @@ -130,4 +118,5 @@ _PrepareArguments: _NeverReturn: b _NeverReturn -ASM_PFX(mSystemMemoryEnd): .8byte 0 +ASM_PFX(mSystemMemoryEnd): + .8byte FixedPcdGet64(PcdSystemMemoryBase) + FixedPcdGet64(PcdSystemMemorySize) - 1 --- a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S +++ b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S @@ -28,18 +28,6 @@ _SetSVCMode: // Check if we can install the stack at the top of the System Memory or if we need // to install the stacks at the bottom of the Firmware Device (case the FD is located // at the top of the DRAM) -_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]