From mboxrd@z Thu Jan 1 00:00:00 1970 Subject: Re: [edk2-devel] [edk2][PATCH V1 1/1] ArmPlatformPkg/PrePeiCore: Explicitly invoke constructor for SEC phase To: Ard Biesheuvel ,devel@edk2.groups.io From: "Rohit Mathew" X-Originating-Location: Cambridge, England, GB (217.140.106.13) X-Originating-Platform: Windows Chrome 102 User-Agent: GROUPS.IO Web Poster MIME-Version: 1.0 Date: Mon, 20 Jun 2022 06:49:53 -0700 References: In-Reply-To: Message-ID: <10348.1655732993084547107@groups.io> Content-Type: multipart/alternative; boundary="y5hbZlcSpiBpVoAyX5bB" --y5hbZlcSpiBpVoAyX5bB Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi Ard, Apologies, I missed your response. On Tue, May 3, 2022 at 09:43 AM, Ard Biesheuvel wrote: >=20 > (+ Rebecca) >=20 > On Tue, 8 Mar 2022 at 12:55, Rohit Mathew wrote: >=20 >> PrePeiCore's CEntry function calls DebugLib library's print API before >> the library is initialized. So, invoke the constructor in the SEC phase >> to call into initialization functions associated with libraries linked >> with this particular module. This change is essential to initialize >> uart for SEC. >>=20 >> Signed-off-by: Rohit Mathew >> --- >> ArmPlatformPkg/PrePeiCore/PrePeiCore.h | 11 ++++++++++- >> ArmPlatformPkg/PrePeiCore/PrePeiCore.c | 9 +++++++-- >> 2 files changed, 17 insertions(+), 3 deletions(-) >>=20 >> Link to github branch with the patches in this series - >> https://github.com/rohit-arm/edk2/tree/sec_constructor_issue >>=20 >> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.h >> b/ArmPlatformPkg/PrePeiCore/PrePeiCore.h >> index 0345dd7bdd2a..d2491aa586ee 100644 >> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.h >> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCore.h >> @@ -1,7 +1,7 @@ >> /** @file >> Main file supporting the transition to PEI Core in Normal World for >> Versatile Express >>=20 >> - Copyright (c) 2011, ARM Limited. All rights reserved. >> + Copyright (c) 2011-2022, ARM Limited. All rights reserved. >>=20 >> SPDX-License-Identifier: BSD-2-Clause-Patent >>=20 >> @@ -73,4 +73,13 @@ PeiCommonExceptionEntry ( >> IN UINTN LR >> ); >>=20 >> +/* >> + * Constructor for SEC phase >> + */ >> +VOID >> +EFIAPI >> +ProcessLibraryConstructorList ( >> + VOID >> + ); >> + >=20 > Why do we need this? Can't we just include PiPei.h or something like that= ? PiPei.h has already been included in ArmPlatformPkg/PrePeiCore/PrePeiCore.h= . I tried including=C2=A0ArmPlatformPkg/PrePi/PrePi.h which had ProcessLibr= aryConstructorList=C2=A0prototype, but both PrePeicore.h and PrePi.h have t= here own versions for "VOID PrimaryMain(*)" which resulted in conflict. >=20 >=20 >> #endif >> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c >> b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c >> index 6dd9bcdea24f..b5f7d2f05b1e 100644 >> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c >> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c >> @@ -1,7 +1,7 @@ >> /** @file >> Main file supporting the transition to PEI Core in Normal World for >> Versatile Express >>=20 >> - Copyright (c) 2011-2014, ARM Limited. All rights reserved. >> + Copyright (c) 2011-2022, ARM Limited. All rights reserved. >>=20 >> SPDX-License-Identifier: BSD-2-Clause-Patent >>=20 >> @@ -86,7 +86,12 @@ CEntryPoint ( >> ArmEnableVFP (); >> } >>=20 >> - // Note: The MMU will be enabled by MemoryPeim. Only the primary core >> will have the MMU on. >> + // In the Sec phase, explicitly invoke the library constructors. This >> helps >> + // the DebugPrint library to be initialized before it is used by >> subsequent >> + // code executed in the sec phase. >=20 > Please drop the reference to DebugLib - calling library constructors > could be needed for any library dependency. Sure. >=20 >=20 >> + ProcessLibraryConstructorList(); >> + >> + //Note: The MMU will be enabled by MemoryPeim. Only the primary core >> will have the MMU on. >=20 > Please leave this comment as before. Will do that. >=20 >=20 >> // If not primary Jump to Secondary Main >> if (ArmPlatformIsPrimaryCore (MpId)) { >> -- >> 2.25.1 >=20 >=20 Regards, Rohit --y5hbZlcSpiBpVoAyX5bB Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable

Hi Ard,

Apologies, I missed your response.

On Tue, May 3, 2022 at 09:43 AM, Ard Biesheuvel wrote:

(+ Rebecca)

On Tue, 8 Mar 2022 at 12:55, Rohit Mathe= w <rohit.mathew@arm.com> wrote:
PrePeiCore's CEntry function calls DebugLib library's print API= before
the library is initialized. So, invoke the constructor in the = SEC phase
to call into initialization functions associated with librar= ies linked
with this particular module. This change is essential to in= itialize
uart for SEC.

Signed-off-by: Rohit Mathew <rohi= t.mathew@arm.com>
---
ArmPlatformPkg/PrePeiCore/PrePeiCore.h |= 11 ++++++++++-
ArmPlatformPkg/PrePeiCore/PrePeiCore.c | 9 +++++++--2 files changed, 17 insertions(+), 3 deletions(-)

Link to gi= thub branch with the patches in this series -
https://github.com/rohit-arm/edk2/tree/sec_constructor_issue
diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.h b/ArmPlatform= Pkg/PrePeiCore/PrePeiCore.h
index 0345dd7bdd2a..d2491aa586ee 100644--- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.h
+++ b/ArmPlatformPkg/P= rePeiCore/PrePeiCore.h
@@ -1,7 +1,7 @@
/** @file
Main file s= upporting the transition to PEI Core in Normal World for Versatile Express<= br />
- Copyright (c) 2011, ARM Limited. All rights reserved.
+ C= opyright (c) 2011-2022, ARM Limited. All rights reserved.

SPDX-L= icense-Identifier: BSD-2-Clause-Patent

@@ -73,4 +73,13 @@ PeiCom= monExceptionEntry (
IN UINTN LR
);

+/*
+ * Constr= uctor for SEC phase
+ */
+VOID
+EFIAPI
+ProcessLibraryC= onstructorList (
+ VOID
+ );
+
Why do we need this? Can't we just include PiPei.h or something like that?<= /blockquote>

PiPei.h has already been included in ArmPlatformPkg/PrePeiCore/PrePeiCor= e.h. I tried including ArmPlatformPkg/PrePi/PrePi.h which had ProcessL= ibraryConstructorList prototype, but both PrePeicore.h and PrePi.h hav= e there own versions for "VOID PrimaryMain(*)" which resulted in conflict.<= /p>

 
#endif
diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c= b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
index 6dd9bcdea24f..b5f7d2f0= 5b1e 100644
--- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
+++ b/Ar= mPlatformPkg/PrePeiCore/PrePeiCore.c
@@ -1,7 +1,7 @@
/** @fileMain file supporting the transition to PEI Core in Normal World for Vers= atile Express

- Copyright (c) 2011-2014, ARM Limited. All rights= reserved.
+ Copyright (c) 2011-2022, ARM Limited. All rights reserved= .

SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -86= ,7 +86,12 @@ CEntryPoint (
ArmEnableVFP ();
}

- // Not= e: The MMU will be enabled by MemoryPeim. Only the primary core will have t= he MMU on.
+ // In the Sec phase, explicitly invoke the library constr= uctors. This helps
+ // the DebugPrint library to be initialized befor= e it is used by subsequent
+ // code executed in the sec phase. Please drop the reference to DebugLib - calling library constructors
c= ould be needed for any library dependency.
Sure.

+ ProcessLibraryConstructorList();
+
+ //Note: The MM= U will be enabled by MemoryPeim. Only the primary core will have the MMU on= .
Please leave this comment as before.
Will do that.

// If not primary Jump to Secondary Main
if (ArmPlatformIs= PrimaryCore (MpId)) {
--
2.25.1
Regards,
Rohit --y5hbZlcSpiBpVoAyX5bB--