From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-x232.google.com (mail-lf0-x232.google.com [IPv6:2a00:1450:4010:c07::232]) (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 60AE21A1EC8 for ; Wed, 7 Sep 2016 05:18:44 -0700 (PDT) Received: by mail-lf0-x232.google.com with SMTP id h127so9485857lfh.0 for ; Wed, 07 Sep 2016 05:18:44 -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=msL3OyMP04JTQo3AXtdo7Q5N0EARz7PaFD0RuOvYAZo=; b=Zxuwhj1ndJTf7sGHtlBAzPPXOpMyth9XC/JzDK9HJSg1l+LG5zZbmAo5L7U4kYGxAU jrb5N3wtuQ4NUyJ2u6mvdiQRW+uc2QCpEyRnPGnq8UMmuL70DQKOwRAEfwuwtwVOU35p UJMQdfKq60dauqZxb6lmEnOy2t/pY562nZNNg= 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=msL3OyMP04JTQo3AXtdo7Q5N0EARz7PaFD0RuOvYAZo=; b=QJ2ZryJqlbb7z91tp/y7+lyV+Sd51ndTj8V8+/c6L7+Zn8XYOOquwlJSdb/WT8h6eD Mjfvt4rfUBXO8FGGLvqrq/hwNOnXQVT9ynQ2pqvC8YhACVQ0C3y4/niU26FDABW2jt1d LnF4KLhrKRUSZGa5uNOrAr1ZmWrqWgDuJkLCjQDg3LT4lgk7TRmcAUXBzkpAo+B5dSjg FfX/GA1M2A75Eh/3zBhaE23mDfi3ui6rNCVjirEvkJSPuHWLiI0+uaisQ7OKsHurJP0m PvQhcO+ZtvqeBQO1J8dEKmz8OjaSaG33+NxcMQ49WNQ7SIqWZNvIAU206XBGLI59OGOV EWdw== X-Gm-Message-State: AE9vXwPy+lWEDWfyfIS/lFGIkEf4+P+hWbLt7GzSu8HDNvf13oOA8ZBSY8+qKRRq040UBEbcY8Z22zE2063bvFm4 X-Received: by 10.25.23.80 with SMTP id n77mr4452906lfi.114.1473250722751; Wed, 07 Sep 2016 05:18:42 -0700 (PDT) MIME-Version: 1.0 Received: by 10.25.19.106 with HTTP; Wed, 7 Sep 2016 05:18:41 -0700 (PDT) In-Reply-To: References: <1470842282-8415-1-git-send-email-ard.biesheuvel@linaro.org> <1470842282-8415-23-git-send-email-ard.biesheuvel@linaro.org> <20160811083817.GQ31760@bivouac.eciton.net> From: Ryan Harkin Date: Wed, 7 Sep 2016 13:18:41 +0100 Message-ID: To: Ard Biesheuvel Cc: Michael Zimmermann , Leif Lindholm , "edk2-devel@lists.01.org" , "Cohen, Eugene" , Laszlo Ersek Subject: Re: [PATCH 22/26] ArmPlatformPkg/PrePi: switch to ASM_FUNC() asm macro 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: Wed, 07 Sep 2016 12:18:45 -0000 Content-Type: text/plain; charset=UTF-8 On 7 September 2016 at 12:59, Ard Biesheuvel wrote: > On 7 September 2016 at 12:10, Ryan Harkin wrote: >> On 31 August 2016 at 05:33, Michael Zimmermann wrote: >>> reviewed should mean tested ;) >>> >>> took me some time to find out why my system currently doesn't boot anymore >> >> I have just bisected my TC2 boot failure down to this patch too. >> >>> but here's the fix for this commit: >>> >>> diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>> b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>> index b7127ce..39030da 100644 >>> --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>> +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>> @@ -101,7 +101,7 @@ _GetStackBase: >>> sub r10, r1, r2 >>> >>> // Stack for the secondary core = Number of Cores - 1 >>> - MOV32 (r0, (FixedPcdGet32(PcdCoreCount) - 1) * >>> FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) >>> + MOV32 (r1, (FixedPcdGet32(PcdCoreCount) - 1) * >>> FixedPcdGet32(PcdCPUCoreSecondaryStackSize))^M >>> sub r10, r10, r1 >>> >>> // r10 = The base of the MpCore Stacks (primary stack & secondary stacks) >> >> And this fix works for me on TC2 also. >> > > May we take that as a tested-by? > Oh yes! :-) You can even have some tested text too: Tested-by: Ryan Harkin But I'll be happy to report that if Michael (or someone else) posts a patch. >> Michael/Ard, has one of you already or does one of you intend to >> submit the fix for this? >> > > Michael: care to send it as a proper patch, please? > > >> >>> >>> On Thu, Aug 11, 2016 at 10:38 AM, Leif Lindholm >>> wrote: >>> >>>> On Wed, Aug 10, 2016 at 05:17:58PM +0200, Ard Biesheuvel wrote: >>>> > Annotate functions with ASM_FUNC() so that they are emitted into >>>> > separate sections. >>>> >>>> Also replacing LoadConstantToReg. Add that to commit message and: >>>> Reviewed-by: Leif Lindholm >>>> >>>> > Contributed-under: TianoCore Contribution Agreement 1.0 >>>> > Signed-off-by: Ard Biesheuvel >>>> > --- >>>> > ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S | 49 >>>> ++++++------------- >>>> > ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S | 50 >>>> ++++++-------------- >>>> > 2 files changed, 29 insertions(+), 70 deletions(-) >>>> > >>>> > diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >>>> b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >>>> > index 9538c70a237c..d0530a874726 100644 >>>> > --- a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >>>> > +++ b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S >>>> > @@ -12,24 +12,10 @@ >>>> > // >>>> > >>>> > #include >>>> > -#include >>>> > -#include >>>> > -#include >>>> > - >>>> > -.text >>>> > -.align 3 >>>> > - >>>> > -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore) >>>> > -GCC_ASM_IMPORT(ArmReadMpidr) >>>> > -GCC_ASM_IMPORT(ArmPlatformPeiBootAction) >>>> > -GCC_ASM_IMPORT(ArmPlatformStackSet) >>>> > -GCC_ASM_EXPORT(_ModuleEntryPoint) >>>> > -ASM_GLOBAL ASM_PFX(mSystemMemoryEnd) >>>> > >>>> > -StartupAddr: .8byte ASM_PFX(CEntryPoint) >>>> > -ASM_PFX(mSystemMemoryEnd): .8byte 0 >>>> > +ASM_GLOBAL ASM_PFX(mSystemMemoryEnd) >>>> > >>>> > -ASM_PFX(_ModuleEntryPoint): >>>> > +ASM_FUNC(_ModuleEntryPoint) >>>> > // Do early platform specific actions >>>> > bl ASM_PFX(ArmPlatformPeiBootAction) >>>> > >>>> > @@ -49,10 +35,8 @@ _SystemMemoryEndInit: >>>> > cmp x1, #0 >>>> > bne _SetupStackPosition >>>> > >>>> > - LoadConstantToReg (FixedPcdGet64(PcdSystemMemoryBase), x1) >>>> > - LoadConstantToReg (FixedPcdGet64(PcdSystemMemorySize), x2) >>>> > - sub x2, x2, #1 >>>> > - add x1, x1, x2 >>>> > + MOV64 (x1, FixedPcdGet64(PcdSystemMemoryBase) + FixedPcdGet64(PcdSystemMemorySize) >>>> - 1) >>>> > + >>>> > // Update the global variable >>>> > adr x2, mSystemMemoryEnd >>>> > str x1, [x2] >>>> > @@ -61,13 +45,13 @@ _SetupStackPosition: >>>> > // r1 = SystemMemoryTop >>>> > >>>> > // Calculate Top of the Firmware Device >>>> > - LoadConstantToReg (FixedPcdGet64(PcdFdBaseAddress), x2) >>>> > - LoadConstantToReg (FixedPcdGet32(PcdFdSize), x3) >>>> > + MOV64 (x2, FixedPcdGet64(PcdFdBaseAddress)) >>>> > + MOV32 (x3, FixedPcdGet32(PcdFdSize) - 1) >>>> > sub x3, x3, #1 >>>> > add x3, x3, x2 // x3 = FdTop = PcdFdBaseAddress + PcdFdSize >>>> > >>>> > // UEFI Memory Size (stacks are allocated in this region) >>>> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), x4) >>>> > + MOV32 (x4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize)) >>>> > >>>> > // >>>> > // Reserve the memory for the UEFI region (contain stacks on its top) >>>> > @@ -98,9 +82,7 @@ _SetupAlignedStack: >>>> > _SetupOverflowStack: >>>> > // Case memory at the top of the address space. Ensure the top of the >>>> stack is EFI_PAGE_SIZE >>>> > // aligned (4KB) >>>> > - LoadConstantToReg (EFI_PAGE_MASK, x11) >>>> > - and x11, x11, x1 >>>> > - sub x1, x1, x11 >>>> > + and x1, x1, ~EFI_PAGE_MASK >>>> > >>>> > _GetBaseUefiMemory: >>>> > // Calculate the Base of the UEFI Memory >>>> > @@ -109,22 +91,19 @@ _GetBaseUefiMemory: >>>> > _GetStackBase: >>>> > // r1 = The top of the Mpcore Stacks >>>> > // Stack for the primary core = PrimaryCoreStack >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) >>>> > + MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >>>> > sub x12, x1, x2 >>>> > >>>> > // Stack for the secondary core = Number of Cores - 1 >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCoreCount), x0) >>>> > - sub x0, x0, #1 >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x1) >>>> > - mul x1, x1, x0 >>>> > + MOV32 (x1, (FixedPcdGet32(PcdCoreCount) - 1) * FixedPcdGet32( >>>> PcdCPUCoreSecondaryStackSize)) >>>> > sub x12, x12, x1 >>>> > >>>> > // x12 = The base of the MpCore Stacks (primary stack & secondary >>>> stacks) >>>> > mov x0, x12 >>>> > mov x1, x10 >>>> > //ArmPlatformStackSet(StackBase, MpId, PrimaryStackSize, >>>> SecondaryStackSize) >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2) >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x3) >>>> > + MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >>>> > + MOV32 (x3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) >>>> > bl ASM_PFX(ArmPlatformStackSet) >>>> > >>>> > // Is it the Primary Core ? >>>> > @@ -140,7 +119,7 @@ _PrepareArguments: >>>> > >>>> > // Move sec startup address into a data register >>>> > // Ensure we're jumping to FV version of the code (not boot remapped >>>> alias) >>>> > - ldr x4, StartupAddr >>>> > + ldr x4, =ASM_PFX(CEntryPoint) >>>> > >>>> > // Jump to PrePiCore C code >>>> > // x0 = MpId >>>> > @@ -150,3 +129,5 @@ _PrepareArguments: >>>> > >>>> > _NeverReturn: >>>> > b _NeverReturn >>>> > + >>>> > +ASM_PFX(mSystemMemoryEnd): .8byte 0 >>>> > diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>>> b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>>> > index 1311efc5cb2c..b7127ce9fb4c 100644 >>>> > --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>>> > +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S >>>> > @@ -12,28 +12,12 @@ >>>> > // >>>> > >>>> > #include >>>> > -#include >>>> > -#include >>>> > -#include >>>> > >>>> > #include >>>> > >>>> > -.text >>>> > -.align 3 >>>> > - >>>> > -GCC_ASM_IMPORT(CEntryPoint) >>>> > -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore) >>>> > -GCC_ASM_IMPORT(ArmReadMpidr) >>>> > -GCC_ASM_IMPORT(ArmPlatformPeiBootAction) >>>> > -GCC_ASM_IMPORT(ArmPlatformStackSet) >>>> > -GCC_ASM_EXPORT(_ModuleEntryPoint) >>>> > GCC_ASM_EXPORT(mSystemMemoryEnd) >>>> > >>>> > -StartupAddr: .word CEntryPoint >>>> > -mSystemMemoryEnd: .8byte 0 >>>> > - >>>> > - >>>> > -ASM_PFX(_ModuleEntryPoint): >>>> > +ASM_FUNC(_ModuleEntryPoint) >>>> > // Do early platform specific actions >>>> > bl ASM_PFX(ArmPlatformPeiBootAction) >>>> > >>>> > @@ -57,10 +41,8 @@ _SystemMemoryEndInit: >>>> > cmp r1, #0 >>>> > bne _SetupStackPosition >>>> > >>>> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryBase), r1) >>>> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemorySize), r2) >>>> > - sub r2, r2, #1 >>>> > - add r1, r1, r2 >>>> > + MOV32 (r1, FixedPcdGet32(PcdSystemMemoryBase) + FixedPcdGet32(PcdSystemMemorySize) >>>> - 1) >>>> > + >>>> > // Update the global variable >>>> > adr r2, mSystemMemoryEnd >>>> > str r1, [r2] >>>> > @@ -69,13 +51,12 @@ _SetupStackPosition: >>>> > // r1 = SystemMemoryTop >>>> > >>>> > // Calculate Top of the Firmware Device >>>> > - LoadConstantToReg (FixedPcdGet32(PcdFdBaseAddress), r2) >>>> > - LoadConstantToReg (FixedPcdGet32(PcdFdSize), r3) >>>> > - sub r3, r3, #1 >>>> > + MOV32 (r2, FixedPcdGet32(PcdFdBaseAddress)) >>>> > + MOV32 (r3, FixedPcdGet32(PcdFdSize) - 1) >>>> > add r3, r3, r2 // r3 = FdTop = PcdFdBaseAddress + PcdFdSize >>>> > >>>> > // UEFI Memory Size (stacks are allocated in this region) >>>> > - LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), r4) >>>> > + MOV32 (r4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize)) >>>> > >>>> > // >>>> > // Reserve the memory for the UEFI region (contain stacks on its top) >>>> > @@ -106,9 +87,8 @@ _SetupAlignedStack: >>>> > _SetupOverflowStack: >>>> > // Case memory at the top of the address space. Ensure the top of the >>>> stack is EFI_PAGE_SIZE >>>> > // aligned (4KB) >>>> > - LoadConstantToReg (EFI_PAGE_MASK, r9) >>>> > - and r9, r9, r1 >>>> > - sub r1, r1, r9 >>>> > + MOV32 (r9, ~EFI_PAGE_MASK & 0xFFFFFFFF) >>>> > + and r1, r1, r9 >>>> > >>>> > _GetBaseUefiMemory: >>>> > // Calculate the Base of the UEFI Memory >>>> > @@ -117,22 +97,19 @@ _GetBaseUefiMemory: >>>> > _GetStackBase: >>>> > // r1 = The top of the Mpcore Stacks >>>> > // Stack for the primary core = PrimaryCoreStack >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) >>>> > + MOV32 (r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >>>> > sub r10, r1, r2 >>>> > >>>> > // Stack for the secondary core = Number of Cores - 1 >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCoreCount), r0) >>>> > - sub r0, r0, #1 >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), r1) >>>> > - mul r1, r1, r0 >>>> > + MOV32 (r0, (FixedPcdGet32(PcdCoreCount) - 1) * FixedPcdGet32( >>>> PcdCPUCoreSecondaryStackSize)) >>>> > sub r10, r10, r1 >>>> > >>>> > // r10 = The base of the MpCore Stacks (primary stack & secondary >>>> stacks) >>>> > mov r0, r10 >>>> > mov r1, r8 >>>> > //ArmPlatformStackSet(StackBase, MpId, PrimaryStackSize, >>>> SecondaryStackSize) >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2) >>>> > - LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), r3) >>>> > + MOV32 (r2, FixedPcdGet32(PcdCPUCorePrimaryStackSize)) >>>> > + MOV32 (r3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize)) >>>> > bl ASM_PFX(ArmPlatformStackSet) >>>> > >>>> > // Is it the Primary Core ? >>>> > @@ -149,7 +126,7 @@ _PrepareArguments: >>>> > >>>> > // Move sec startup address into a data register >>>> > // Ensure we're jumping to FV version of the code (not boot remapped >>>> alias) >>>> > - ldr r4, StartupAddr >>>> > + ldr r4, =ASM_PFX(CEntryPoint) >>>> > >>>> > // Jump to PrePiCore C code >>>> > // r0 = MpId >>>> > @@ -160,3 +137,4 @@ _PrepareArguments: >>>> > _NeverReturn: >>>> > b _NeverReturn >>>> > >>>> > +ASM_PFX(mSystemMemoryEnd): .8byte 0 >>>> > -- >>>> > 2.7.4 >>>> > >>>> _______________________________________________ >>>> edk2-devel mailing list >>>> edk2-devel@lists.01.org >>>> https://lists.01.org/mailman/listinfo/edk2-devel >>>> >>> _______________________________________________ >>> edk2-devel mailing list >>> edk2-devel@lists.01.org >>> https://lists.01.org/mailman/listinfo/edk2-devel