| Message ID | 20171020112325.10814-1-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [edk2] ArmPlatformPkg/PrePeiCore: seed temporary stack before entering PEI core | expand |
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 <ard.biesheuvel@linaro.org> > --- > 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 <AsmMacroIoLibV8.h> > > +#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 <AsmMacroIoLib.h> > > +#define INIT_CAR_VALUE 0x5AA55AA5 > + Worth moving to a common header somewhere? Also defined/used in MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c. 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"? / 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 <AutoGen.h> > > +#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
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 <ard.biesheuvel@linaro.org> >> --- >> 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 <AsmMacroIoLibV8.h> >> >> +#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 <AsmMacroIoLib.h> >> >> +#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. 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 <AutoGen.h> >> >> +#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
On 20 October 2017 at 16:43, Laszlo Ersek <lersek@redhat.com> 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 <ard.biesheuvel@linaro.org> >>> --- >>> 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 <AsmMacroIoLibV8.h> >>> >>> +#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 <AsmMacroIoLib.h> >>> >>> +#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 <AutoGen.h> >>> >>> +#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
On 10/20/17 18:10, Ard Biesheuvel wrote: > On 20 October 2017 at 16:43, Laszlo Ersek <lersek@redhat.com> 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 <ard.biesheuvel@linaro.org> >>>> --- >>>> 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 <AsmMacroIoLibV8.h> >>>> >>>> +#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 <AsmMacroIoLib.h> >>>> >>>> +#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. Yes, I can certainly see myself on the "wrong end" of that patch. :( Acked-by: Laszlo Ersek <lersek@redhat.com> Thanks Laszlo > >> 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 <AutoGen.h> >>>> >>>> +#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
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? Thanks Liming > -----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 <lersek@redhat.com> > Cc: edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org> > Subject: Re: [edk2] [PATCH] ArmPlatformPkg/PrePeiCore: seed temporary stack before entering PEI core > > On 20 October 2017 at 16:43, Laszlo Ersek <lersek@redhat.com> 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 <ard.biesheuvel@linaro.org> > >>> --- > >>> 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 <AsmMacroIoLibV8.h> > >>> > >>> +#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 <AsmMacroIoLib.h> > >>> > >>> +#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 <AutoGen.h> > >>> > >>> +#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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 20 October 2017 at 17:37, Gao, Liming <liming.gao@intel.com> 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 <lersek@redhat.com> >> Cc: edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org> >> Subject: Re: [edk2] [PATCH] ArmPlatformPkg/PrePeiCore: seed temporary stack before entering PEI core >> >> On 20 October 2017 at 16:43, Laszlo Ersek <lersek@redhat.com> 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 <ard.biesheuvel@linaro.org> >> >>> --- >> >>> 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 <AsmMacroIoLibV8.h> >> >>> >> >>> +#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 <AsmMacroIoLib.h> >> >>> >> >>> +#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 <AutoGen.h> >> >>> >> >>> +#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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 10/20/17 18:39, Ard Biesheuvel wrote: > On 20 October 2017 at 17:37, Gao, Liming <liming.gao@intel.com> 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! Would it be possible to define the PCD as UINT32, and task 64-bit SEC (and PEI_CORE) code to first construct the wider value manually (in a register or otherwise)? Just thinking out loud. Thanks! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 20 October 2017 at 17:51, Laszlo Ersek <lersek@redhat.com> wrote: > On 10/20/17 18:39, Ard Biesheuvel wrote: >> On 20 October 2017 at 17:37, Gao, Liming <liming.gao@intel.com> 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! > > Would it be possible to define the PCD as UINT32, and task 64-bit SEC > (and PEI_CORE) code to first construct the wider value manually (in a > register or otherwise)? > > Just thinking out loud. > Could you think the reasoning behind that out loud as well? _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 10/20/17 18:52, Ard Biesheuvel wrote: > On 20 October 2017 at 17:51, Laszlo Ersek <lersek@redhat.com> wrote: >> On 10/20/17 18:39, Ard Biesheuvel wrote: >>> On 20 October 2017 at 17:37, Gao, Liming <liming.gao@intel.com> 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! >> >> Would it be possible to define the PCD as UINT32, and task 64-bit SEC >> (and PEI_CORE) code to first construct the wider value manually (in a >> register or otherwise)? >> >> Just thinking out loud. >> > > Could you think the reasoning behind that out loud as well? Haha, good stab :) Sure. In your patch you have: +#define INIT_CAR_VALUE 0x5AA55AA55AA55AA5 for 64-bit, and +#define INIT_CAR_VALUE 0x5AA55AA5 for 32-bit. Both 64-bit assembly code in SEC, and 64-bit C-code in the PEI_CORE, can easily compose the large value from the small value, starting from FixedPcdGet32(). The alternatives are: - asking the 32-bit assembly code to truncate the 64-bit constant -- it won't compile, - defining *two* FixedAtBuild PCDs, one for 32-bit, another for 64-bit SEC -- an idea probably not universally liked. ... When I originally started writing my previous email, I even thought about introducing the PCD as UINT16 :) But then I realized, if any platform lacks *some* 32-bit mode when it sets up the stack in assembly, for C-language entry, then the platform won't be supported by edk2. Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
This is for debug purpose. I think UINT32 is OK if AARCH64 can operate 32bit value. Thanks Liming > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Saturday, October 21, 2017 1:19 AM > To: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org> > Subject: Re: [edk2] [PATCH] ArmPlatformPkg/PrePeiCore: seed temporary stack before entering PEI core > > On 10/20/17 18:52, Ard Biesheuvel wrote: > > On 20 October 2017 at 17:51, Laszlo Ersek <lersek@redhat.com> wrote: > >> On 10/20/17 18:39, Ard Biesheuvel wrote: > >>> On 20 October 2017 at 17:37, Gao, Liming <liming.gao@intel.com> 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! > >> > >> Would it be possible to define the PCD as UINT32, and task 64-bit SEC > >> (and PEI_CORE) code to first construct the wider value manually (in a > >> register or otherwise)? > >> > >> Just thinking out loud. > >> > > > > Could you think the reasoning behind that out loud as well? > > Haha, good stab :) Sure. > > In your patch you have: > > +#define INIT_CAR_VALUE 0x5AA55AA55AA55AA5 > > for 64-bit, and > > +#define INIT_CAR_VALUE 0x5AA55AA5 > > for 32-bit. > > Both 64-bit assembly code in SEC, and 64-bit C-code in the PEI_CORE, can > easily compose the large value from the small value, starting from > FixedPcdGet32(). The alternatives are: > > - asking the 32-bit assembly code to truncate the 64-bit constant -- it > won't compile, > > - defining *two* FixedAtBuild PCDs, one for 32-bit, another for 64-bit > SEC -- an idea probably not universally liked. > > ... When I originally started writing my previous email, I even thought > about introducing the PCD as UINT16 :) But then I realized, if any > platform lacks *some* 32-bit mode when it sets up the stack in assembly, > for C-language entry, then the platform won't be supported by edk2. > > Thanks > Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 23 October 2017 at 15:18, Gao, Liming <liming.gao@intel.com> wrote: > This is for debug purpose. I think UINT32 is OK if AARCH64 can operate 32bit value. > Both my ARM and AARCH64 implementations write 16 bytes at a time, and so whether the source value is UINT32 or UINT64 or UINT16 does not make any difference at all. >> -----Original Message----- >> From: Laszlo Ersek [mailto:lersek@redhat.com] >> Sent: Saturday, October 21, 2017 1:19 AM >> To: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> Cc: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org> >> Subject: Re: [edk2] [PATCH] ArmPlatformPkg/PrePeiCore: seed temporary stack before entering PEI core >> >> On 10/20/17 18:52, Ard Biesheuvel wrote: >> > On 20 October 2017 at 17:51, Laszlo Ersek <lersek@redhat.com> wrote: >> >> On 10/20/17 18:39, Ard Biesheuvel wrote: >> >>> On 20 October 2017 at 17:37, Gao, Liming <liming.gao@intel.com> 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! >> >> >> >> Would it be possible to define the PCD as UINT32, and task 64-bit SEC >> >> (and PEI_CORE) code to first construct the wider value manually (in a >> >> register or otherwise)? >> >> >> >> Just thinking out loud. >> >> >> > >> > Could you think the reasoning behind that out loud as well? >> >> Haha, good stab :) Sure. >> >> In your patch you have: >> >> +#define INIT_CAR_VALUE 0x5AA55AA55AA55AA5 >> >> for 64-bit, and >> >> +#define INIT_CAR_VALUE 0x5AA55AA5 >> >> for 32-bit. >> >> Both 64-bit assembly code in SEC, and 64-bit C-code in the PEI_CORE, can >> easily compose the large value from the small value, starting from >> FixedPcdGet32(). The alternatives are: >> >> - asking the 32-bit assembly code to truncate the 64-bit constant -- it >> won't compile, >> >> - defining *two* FixedAtBuild PCDs, one for 32-bit, another for 64-bit >> SEC -- an idea probably not universally liked. >> >> ... When I originally started writing my previous email, I even thought >> about introducing the PCD as UINT16 :) But then I realized, if any >> platform lacks *some* 32-bit mode when it sets up the stack in assembly, >> for C-language entry, then the platform won't be supported by edk2. >> >> Thanks >> Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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 <AsmMacroIoLibV8.h> +#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 <AsmMacroIoLib.h> +#define INIT_CAR_VALUE 0x5AA55AA5 + 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 <AutoGen.h> +#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
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 <ard.biesheuvel@linaro.org> --- ArmPlatformPkg/PrePeiCore/AArch64/PrePeiCoreEntryPoint.S | 7 +++++++ ArmPlatformPkg/PrePeiCore/Arm/PrePeiCoreEntryPoint.S | 10 ++++++++++ ArmPlatformPkg/PrePeiCore/Arm/PrePeiCoreEntryPoint.asm | 10 ++++++++++ 3 files changed, 27 insertions(+) -- 2.11.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel