Hi Sami, Thank you for the review. Please find my response inline -
On Thu, Jul 21, 2022 at 04:55 PM, Sami Mujawar wrote: > > > > Hi Rohit, > > > > Please find my response inline marked [SAMI]. > > > > Regards, > > > > Sami Mujawar > > On 06/07/2022 02:42 pm, Rohit Mathew wrote: > >> Invoke the constructor in the SEC phase to call into initialization >> functions associated with libraries linked with this particular module. >> For instance, PrePeiCore's CEntryPoint function calls DebugLib library's >> print API before the library is initialized. > > [SAMI] Can you rephrase the commit message to be clearer, please? [Rohit] Sure. Done. > > >> This change is essential >> to initialize uart for SEC phase. >> >> Signed-off-by: >> Rohit Mathew <rohit.mat...@arm.com> ( rohit.mat...@arm.com ) >> --- >> >> ArmPlatformPkg/PrePeiCore/PrePeiCore.h | 11 ++++++++++- >> >> ArmPlatformPkg/PrePeiCore/PrePeiCore.c | 6 +++++- >> 2 files changed, 15 >> insertions(+), 2 deletions(-) >> >> Changes since V1: >> - Rebased on top of >> latest master branch. >> - Addressed comments from Ard. >> >> Changes since V2: >> - >> Rebased on top of latest master branch. >> >> Link to github branch for the >> patch - >> https://github.com/rohit-arm/edk2/tree/sec_constructor_issue >> >> diff --git >> a/ArmPlatformPkg/PrePeiCore/PrePeiCore.h >> b/ArmPlatformPkg/PrePeiCore/PrePeiCore.h >> index 0345dd7bdd2a..b752c4b9e617 >> 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 >> >> - Copyright (c) 2011, ARM Limited. All rights >> reserved. >> + Copyright (c) 2011 - 2022, ARM Limited. All rights reserved. >> >> SPDX-License-Identifier: BSD-2-Clause-Patent >> >> @@ -73,4 +73,13 @@ >> PeiCommonExceptionEntry ( >> IN UINTN LR >> ); >> >> +/* >> + * Constructor for >> SEC phase > > > > [SAMI] The above description does not appear to be correct. > > > > Would the following test be more appropriate? > > Autogenerated function that calls the library constructors for all of the > module's > dependent libraries. > [/SAMI] > > [Rohit]. Thanks for correcting this. Done. > > > >> + */ >> +VOID >> +EFIAPI >> +ProcessLibraryConstructorList ( >> + VOID >> + ); >> + >> >> #endif >> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c >> b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c >> index 6dd9bcdea24f..9654866f0c13 >> 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 >> >> - 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,6 +86,10 @@ >> CEntryPoint ( >> ArmEnableVFP (); >> } >> >> + // Explicitly invoke the >> library constructor to resolve any library >> + // dependency. >> + >> ProcessLibraryConstructorList(); > > [SAMI] I don't think this will pass the edk2 CI. Can you check, please? [Rohit] Checked CI locally. Fixed spacing issue for parenthesis. Snippet of CI run after fixing issues - " PROGRESS - --->Test Success: Uncrustify Coding Standard Test NO-TARGET PROGRESS - --Running ArmPlatformPkg: Host Unit Test Dsc Complete Check Test NO-TARGET -- PROGRESS - --->Test Success: Host Unit Test Dsc Complete Check Test NO-TARGET PROGRESS - --Running ArmPlatformPkg: Compiler Plugin DEBUG -- PROGRESS - Start time: 2022-07-22 17:53:34.737944 PROGRESS - Setting up the Environment PROGRESS - Running Pre Build PROGRESS - Running Build DEBUG PROGRESS - Running Post Build PROGRESS - End time: 2022-07-22 17:53:39.417824 Total time Elapsed: 0:00:04 PROGRESS - --->Test Success: Compiler Plugin DEBUG " [/Rohit] > > >> + >> // Note: The MMU will be enabled by MemoryPeim. Only the primary core >> will have the MMU on. >> >> > > Regards, Rohit > > >> // If not primary Jump to Secondary Main >> > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#91726): https://edk2.groups.io/g/devel/message/91726 Mute This Topic: https://groups.io/mt/92206586/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
