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:c0b::242; helo=mail-it0-x242.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x242.google.com (mail-it0-x242.google.com [IPv6:2607:f8b0:4001:c0b::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 91643202E60EF for ; Fri, 20 Oct 2017 09:35:41 -0700 (PDT) Received: by mail-it0-x242.google.com with SMTP id p138so14669045itp.2 for ; Fri, 20 Oct 2017 09:39:20 -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=ins0hGQyG6g9aMHCZN2zYduExRs/QkhMoPJcxudYang=; b=Q+MuqbiFrNt/foGqH5SzUHVqYuFCYheIDKMc8rRn4P/8f3nB0v+qf3nLe6TKgaCa4U jpyxdt1eemRmZ02ieavzUY9NE1/Bmh3+VIjOX7hgNzVSf1zrrk3Bh/LnZywQyf+ElOA1 VrxcuUxj4fYFNnWafGuVBvWijin3BMPGLZ6AE= 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=ins0hGQyG6g9aMHCZN2zYduExRs/QkhMoPJcxudYang=; b=P+asW+CrkXrx9IoYF6DU0a1utmQtOgG+bupcIe3SKpD84TXHTiKDr69v5OmXUKUGtc H5TXZh6PmIVkMJ81pGpjWBsOhmSOoUPGGCoF56ko9+xSdOOMetp7sI1QY776LbQN9Lio eZDeiqP6LhszeAdTY0ks9zH831f8Yhn1l/CspH38wJkcxJhSQ3j6SD3Qs4wTsNCUYBD2 j56UDPRWu/9rQxvXVewvDSPbqzTGM/vI8SqvySmkQRRgdbmKW1sx4I2+u9OgONYUiR6/ ctskkLcuBAPfj0JpDcoZYfqNCmS/O409BedHC6u9PZObcboZjcypyLNokfyr8Moxa3WW tLmg== X-Gm-Message-State: AMCzsaXD4096NfBA9lNvNBRhJnsr9UF8asZRqt2U4wywHBosC+HuGpWU jTimLsH7xkUip7oAkTL0nV+IvPZy/58bDw5xg1YYXw== X-Google-Smtp-Source: ABhQp+TKJ1q98gNArELZX0PcUKoQsGygKTnfOhwBjjuWyGZG+YwhBTjDZkBq6N0d/OtJgs7RVq9XMwtiXP4VVvnvZBA= X-Received: by 10.36.141.70 with SMTP id w67mr3412601itd.58.1508517560380; Fri, 20 Oct 2017 09:39:20 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.131.167 with HTTP; Fri, 20 Oct 2017 09:39:19 -0700 (PDT) In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A14E16EABD@SHSMSX104.ccr.corp.intel.com> References: <20171020112325.10814-1-ard.biesheuvel@linaro.org> <20171020130024.l73uww7cxsjnwbsv@bivouac.eciton.net> <51d0ef33-022f-7153-9acd-9bc4a26cdd59@redhat.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E16EABD@SHSMSX104.ccr.corp.intel.com> From: Ard Biesheuvel Date: Fri, 20 Oct 2017 17:39:19 +0100 Message-ID: To: "Gao, Liming" Cc: Laszlo Ersek , "edk2-devel@lists.01.org" , Leif Lindholm Subject: Re: [PATCH] ArmPlatformPkg/PrePeiCore: seed temporary stack before entering PEI core X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Oct 2017 16:35:41 -0000 Content-Type: text/plain; charset="UTF-8" On 20 October 2017 at 17:37, Gao, Liming wrote: > Ard: > This case is to share the same value between PeiCore and SecCore. I also think it will be better to define one fixed PCD in MdeModulePkg.dec for this value. Could you submit bugzillar to catch this issue first? > Certainly! >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel >> Sent: Saturday, October 21, 2017 12:10 AM >> To: Laszlo Ersek >> Cc: edk2-devel@lists.01.org; Leif Lindholm >> Subject: Re: [edk2] [PATCH] ArmPlatformPkg/PrePeiCore: seed temporary stack before entering PEI core >> >> On 20 October 2017 at 16:43, Laszlo Ersek wrote: >> > On 10/20/17 15:00, Leif Lindholm wrote: >> >> On Fri, Oct 20, 2017 at 12:23:25PM +0100, Ard Biesheuvel wrote: >> >>> DEBUG builds of PEI code will print a diagnostic message regarding >> >>> the utilization of temporary RAM before switching to permanent RAM. >> >>> For example, >> >>> >> >>> Total temporary memory: 16352 bytes. >> >>> temporary memory stack ever used: 4820 bytes. >> >>> temporary memory heap used for HobList: 4720 bytes. >> >>> >> >>> Tracking stack utilization like this requires the stack to be seeded >> >>> with a known magic value, and this needs to occur before entering C >> >>> code, given that it uses the stack. Currently, only Nt32Pkg appears >> >>> to implement this feature, but it is useful nonetheless, so let's >> >>> wire it up for PrePeiCore. >> >>> >> >>> Contributed-under: TianoCore Contribution Agreement 1.1 >> >>> Signed-off-by: Ard Biesheuvel >> >>> --- >> >>> ArmPlatformPkg/PrePeiCore/AArch64/PrePeiCoreEntryPoint.S | 7 +++++++ >> >>> ArmPlatformPkg/PrePeiCore/Arm/PrePeiCoreEntryPoint.S | 10 ++++++++++ >> >>> ArmPlatformPkg/PrePeiCore/Arm/PrePeiCoreEntryPoint.asm | 10 ++++++++++ >> >>> 3 files changed, 27 insertions(+) >> >>> >> >>> diff --git a/ArmPlatformPkg/PrePeiCore/AArch64/PrePeiCoreEntryPoint.S >> b/ArmPlatformPkg/PrePeiCore/AArch64/PrePeiCoreEntryPoint.S >> >>> index aab5edab0c42..7a33e2754869 100644 >> >>> --- a/ArmPlatformPkg/PrePeiCore/AArch64/PrePeiCoreEntryPoint.S >> >>> +++ b/ArmPlatformPkg/PrePeiCore/AArch64/PrePeiCoreEntryPoint.S >> >>> @@ -13,6 +13,8 @@ >> >>> >> >>> #include >> >>> >> >>> +#define INIT_CAR_VALUE 0x5AA55AA55AA55AA5 >> >>> + >> >>> ASM_FUNC(_ModuleEntryPoint) >> >>> // Do early platform specific actions >> >>> bl ASM_PFX(ArmPlatformPeiBootAction) >> >>> @@ -84,4 +86,9 @@ _PrepareArguments: >> >>> >> >>> _SetupPrimaryCoreStack: >> >>> mov sp, x1 >> >>> + MOV64 (x8, FixedPcdGet64(PcdCPUCoresStackBase)) >> >>> + MOV64 (x9, INIT_CAR_VALUE) >> >>> +0:stp x9, x9, [x8], #16 >> >>> + cmp x8, x1 >> >>> + b.lt 0b >> >>> b _PrepareArguments >> >>> diff --git a/ArmPlatformPkg/PrePeiCore/Arm/PrePeiCoreEntryPoint.S b/ArmPlatformPkg/PrePeiCore/Arm/PrePeiCoreEntryPoint.S >> >>> index 14344425ad4c..7342e49bea59 100644 >> >>> --- a/ArmPlatformPkg/PrePeiCore/Arm/PrePeiCoreEntryPoint.S >> >>> +++ b/ArmPlatformPkg/PrePeiCore/Arm/PrePeiCoreEntryPoint.S >> >>> @@ -13,6 +13,8 @@ >> >>> >> >>> #include >> >>> >> >>> +#define INIT_CAR_VALUE 0x5AA55AA5 >> >>> + >> >> >> >> Worth moving to a common header somewhere? >> >> >> >> Also defined/used in MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c. >> > >> > Furthermore, open-coded in: >> > >> > EmulatorPkg/Unix/Host/Host.c: *StackPointer = 0x5AA55AA5; >> > Nt32Pkg/Sec/SecMain.c: *StackPointer = 0x5AA55AA5; >> > >> > Honestly I think it should be a Fixed-At-Build PCD, in MdeModulePkg.dec, >> > similarly to >> > >> > gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue >> > >> > in MdePkg.dec. >> > >> >> Yes. And you both know how the MdeModulePkg maintainers are going to >> respond if I propose adding another PCD. >> >> > I'm unhappy that we have to annoy Ard with a request to "upstream" this >> > constant to MdeModulePkg in some form, but we'll need it yet again in >> > OVMF... >> > >> >> >> >> >> >> That file has an explicit comment saying "temporary memory is filled >> >> with this initial value during SEC phase". Should this end have a >> >> corresponding comment saying "checked for during PEI phase"? >> > >> > Thanks >> > Laszlo >> > >> >> >> >> / >> >> Leif >> >> >> >>> ASM_FUNC(_ModuleEntryPoint) >> >>> // Do early platform specific actions >> >>> bl ASM_PFX(ArmPlatformPeiBootAction) >> >>> @@ -65,6 +67,14 @@ _PrepareArguments: >> >>> >> >>> _SetupPrimaryCoreStack: >> >>> mov sp, r1 >> >>> + MOV32 (r8, FixedPcdGet64(PcdCPUCoresStackBase)) >> >>> + MOV32 (r9, INIT_CAR_VALUE) >> >>> + mov r10, r9 >> >>> + mov r11, r9 >> >>> + mov r12, r9 >> >>> +0:stm r8!, {r9-r12} >> >>> + cmp r8, r1 >> >>> + blt 0b >> >>> b _PrepareArguments >> >>> >> >>> _NeverReturn: >> >>> diff --git a/ArmPlatformPkg/PrePeiCore/Arm/PrePeiCoreEntryPoint.asm >> b/ArmPlatformPkg/PrePeiCore/Arm/PrePeiCoreEntryPoint.asm >> >>> index abea675828df..7455de8aa66e 100644 >> >>> --- a/ArmPlatformPkg/PrePeiCore/Arm/PrePeiCoreEntryPoint.asm >> >>> +++ b/ArmPlatformPkg/PrePeiCore/Arm/PrePeiCoreEntryPoint.asm >> >>> @@ -13,6 +13,8 @@ >> >>> >> >>> #include >> >>> >> >>> +#define INIT_CAR_VALUE 0x5AA55AA5 >> >>> + >> >>> INCLUDE AsmMacroIoLib.inc >> >>> >> >>> IMPORT CEntryPoint >> >>> @@ -79,6 +81,14 @@ _PrepareArguments >> >>> >> >>> _SetupPrimaryCoreStack >> >>> mov sp, r1 >> >>> + mov32 r8, FixedPcdGet64(PcdCPUCoresStackBase) >> >>> + mov32 r9, INIT_CAR_VALUE >> >>> + mov r10, r9 >> >>> + mov r11, r9 >> >>> + mov r12, r9 >> >>> +0:stm r8!, {r9-r12} >> >>> + cmp r8, r1 >> >>> + blt 0b >> >>> b _PrepareArguments >> >>> >> >>> _NeverReturn >> >>> -- >> >>> 2.11.0 >> >>> >> > >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel