[edk2][PATCH V4 1/1] ArmPlatformPkg/PrePeiCore: Invoke constructors for SEC phase

[edk2][PATCH V4 1/1] ArmPlatformPkg/PrePeiCore: Invoke constructors for SEC phase

[Rohit Mathew]

On platforms that do not have the serial console port pre-initialized
prior to the SEC phase and due to the absence of a call to
"SerialPortInitialize", this results in missing debug logs. So, call
the auto-generated "ProcessLibraryConstructorList" function from SEC
phase to have all the dependent library constructors called
(this includes a call to "SerialPortInitialize").

Signed-off-by: Rohit Mathew <rohit.mathew@arm.com>
---
 ArmPlatformPkg/PrePeiCore/PrePeiCore.h | 12 +++++++++++-
 ArmPlatformPkg/PrePeiCore/PrePeiCore.c |  6 +++++-
 2 files changed, 16 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.

Changes since V3:
- Addressed comments from Sami.

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..c1e13e23e11e 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,14 @@ PeiCommonExceptionEntry (
   IN UINTN   LR
   );
 
+/*
+ * Autogenerated function that calls the library constructors for all of the
+ * module's dependent libraries.
+ */
+VOID
+EFIAPI
+ProcessLibraryConstructorList (
+  VOID
+  );
+
 #endif
diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
index 6dd9bcdea24f..9c4b25df953d 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
 
@@ -90,6 +90,10 @@ CEntryPoint (
 
   // If not primary Jump to Secondary Main
   if (ArmPlatformIsPrimaryCore (MpId)) {
+    // Invoke "ProcessLibraryConstructorList" to have all library constructors
+    // called.
+    ProcessLibraryConstructorList ();
+
     // Initialize the Debug Agent for Source Level Debugging
     InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
     SaveAndSetDebugTimerInterrupt (TRUE);
-- 
2.25.1

Re: [edk2][PATCH V4 1/1] ArmPlatformPkg/PrePeiCore: Invoke constructors for SEC phase

[Sami Mujawar]

Hi Rohit,

This patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 22/07/2022 06:02 pm, Rohit Mathew wrote:
> On platforms that do not have the serial console port pre-initialized
> prior to the SEC phase and due to the absence of a call to
> "SerialPortInitialize", this results in missing debug logs. So, call
> the auto-generated "ProcessLibraryConstructorList" function from SEC
> phase to have all the dependent library constructors called
> (this includes a call to "SerialPortInitialize").
>
> Signed-off-by: Rohit Mathew <rohit.mathew@arm.com>
> ---
>   ArmPlatformPkg/PrePeiCore/PrePeiCore.h | 12 +++++++++++-
>   ArmPlatformPkg/PrePeiCore/PrePeiCore.c |  6 +++++-
>   2 files changed, 16 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.
>
> Changes since V3:
> - Addressed comments from Sami.
>
> 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..c1e13e23e11e 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,14 @@ PeiCommonExceptionEntry (
>     IN UINTN   LR
>     );
>   
> +/*
> + * Autogenerated function that calls the library constructors for all of the
> + * module's dependent libraries.
> + */
> +VOID
> +EFIAPI
> +ProcessLibraryConstructorList (
> +  VOID
> +  );
> +
>   #endif
> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> index 6dd9bcdea24f..9c4b25df953d 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
>   
> @@ -90,6 +90,10 @@ CEntryPoint (
>   
>     // If not primary Jump to Secondary Main
>     if (ArmPlatformIsPrimaryCore (MpId)) {
> +    // Invoke "ProcessLibraryConstructorList" to have all library constructors
> +    // called.
> +    ProcessLibraryConstructorList ();
> +
>       // Initialize the Debug Agent for Source Level Debugging
>       InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
>       SaveAndSetDebugTimerInterrupt (TRUE);

Re: [edk2][PATCH V4 1/1] ArmPlatformPkg/PrePeiCore: Invoke constructors for SEC phase

[Rohit Mathew]

Hi,

Could anyone please let me know if there are any more comments on this patch? 
Saw that we have moved into soft freeze for edk2 and wanted to add that we had one round of review on the patch.

Regards,
Rohit

> -----Original Message-----
> From: Sami Mujawar <Sami.Mujawar@arm.com>
> Sent: 25 July 2022 12:12
> To: Rohit Mathew <Rohit.Mathew@arm.com>; devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Rebecca Cran
> <rebecca@bsdio.com>; Pierre Gondois <Pierre.Gondois@arm.com>; Thomas
> Abraham <thomas.abraham@arm.com>; nd <nd@arm.com>
> Subject: Re: [edk2][PATCH V4 1/1] ArmPlatformPkg/PrePeiCore: Invoke
> constructors for SEC phase
> 
> Hi Rohit,
> 
> This patch looks good to me.
> 
> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> 
> Regards,
> 
> Sami Mujawar
> 
> On 22/07/2022 06:02 pm, Rohit Mathew wrote:
> > On platforms that do not have the serial console port pre-initialized
> > prior to the SEC phase and due to the absence of a call to
> > "SerialPortInitialize", this results in missing debug logs. So, call
> > the auto-generated "ProcessLibraryConstructorList" function from SEC
> > phase to have all the dependent library constructors called (this
> > includes a call to "SerialPortInitialize").
> >
> > Signed-off-by: Rohit Mathew <rohit.mathew@arm.com>
> > ---
> >   ArmPlatformPkg/PrePeiCore/PrePeiCore.h | 12 +++++++++++-
> >   ArmPlatformPkg/PrePeiCore/PrePeiCore.c |  6 +++++-
> >   2 files changed, 16 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.
> >
> > Changes since V3:
> > - Addressed comments from Sami.
> >
> > 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..c1e13e23e11e 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,14 @@ PeiCommonExceptionEntry (
> >     IN UINTN   LR
> >     );
> >
> > +/*
> > + * Autogenerated function that calls the library constructors for all
> > +of the
> > + * module's dependent libraries.
> > + */
> > +VOID
> > +EFIAPI
> > +ProcessLibraryConstructorList (
> > +  VOID
> > +  );
> > +
> >   #endif
> > diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> > b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> > index 6dd9bcdea24f..9c4b25df953d 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
> >
> > @@ -90,6 +90,10 @@ CEntryPoint (
> >
> >     // If not primary Jump to Secondary Main
> >     if (ArmPlatformIsPrimaryCore (MpId)) {
> > +    // Invoke "ProcessLibraryConstructorList" to have all library constructors
> > +    // called.
> > +    ProcessLibraryConstructorList ();
> > +
> >       // Initialize the Debug Agent for Source Level Debugging
> >       InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
> >       SaveAndSetDebugTimerInterrupt (TRUE);

Re: [edk2-devel] [edk2][PATCH V4 1/1] ArmPlatformPkg/PrePeiCore: Invoke constructors for SEC phase

[Rebecca Cran]

It would be good if we could get this committed now the stable tag has 
been created.

-- 
Rebecca Cran

On 8/9/22 03:39, Rohit Mathew wrote:
> Hi,
>
> Could anyone please let me know if there are any more comments on this patch?
> Saw that we have moved into soft freeze for edk2 and wanted to add that we had one round of review on the patch.
>
> Regards,
> Rohit
>
>> -----Original Message-----
>> From: Sami Mujawar <Sami.Mujawar@arm.com>
>> Sent: 25 July 2022 12:12
>> To: Rohit Mathew <Rohit.Mathew@arm.com>; devel@edk2.groups.io
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Rebecca Cran
>> <rebecca@bsdio.com>; Pierre Gondois <Pierre.Gondois@arm.com>; Thomas
>> Abraham <thomas.abraham@arm.com>; nd <nd@arm.com>
>> Subject: Re: [edk2][PATCH V4 1/1] ArmPlatformPkg/PrePeiCore: Invoke
>> constructors for SEC phase
>>
>> Hi Rohit,
>>
>> This patch looks good to me.
>>
>> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
>>
>> Regards,
>>
>> Sami Mujawar
>>
>> On 22/07/2022 06:02 pm, Rohit Mathew wrote:
>>> On platforms that do not have the serial console port pre-initialized
>>> prior to the SEC phase and due to the absence of a call to
>>> "SerialPortInitialize", this results in missing debug logs. So, call
>>> the auto-generated "ProcessLibraryConstructorList" function from SEC
>>> phase to have all the dependent library constructors called (this
>>> includes a call to "SerialPortInitialize").
>>>
>>> Signed-off-by: Rohit Mathew <rohit.mathew@arm.com>
>>> ---
>>>    ArmPlatformPkg/PrePeiCore/PrePeiCore.h | 12 +++++++++++-
>>>    ArmPlatformPkg/PrePeiCore/PrePeiCore.c |  6 +++++-
>>>    2 files changed, 16 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.
>>>
>>> Changes since V3:
>>> - Addressed comments from Sami.
>>>
>>> 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..c1e13e23e11e 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,14 @@ PeiCommonExceptionEntry (
>>>      IN UINTN   LR
>>>      );
>>>
>>> +/*
>>> + * Autogenerated function that calls the library constructors for all
>>> +of the
>>> + * module's dependent libraries.
>>> + */
>>> +VOID
>>> +EFIAPI
>>> +ProcessLibraryConstructorList (
>>> +  VOID
>>> +  );
>>> +
>>>    #endif
>>> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
>>> b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
>>> index 6dd9bcdea24f..9c4b25df953d 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
>>>
>>> @@ -90,6 +90,10 @@ CEntryPoint (
>>>
>>>      // If not primary Jump to Secondary Main
>>>      if (ArmPlatformIsPrimaryCore (MpId)) {
>>> +    // Invoke "ProcessLibraryConstructorList" to have all library constructors
>>> +    // called.
>>> +    ProcessLibraryConstructorList ();
>>> +
>>>        // Initialize the Debug Agent for Source Level Debugging
>>>        InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
>>>        SaveAndSetDebugTimerInterrupt (TRUE);
>
> 
>
>

Re: [edk2-devel] [edk2][PATCH V4 1/1] ArmPlatformPkg/PrePeiCore: Invoke constructors for SEC phase

[Ard Biesheuvel]

On Tue, 30 Aug 2022 at 00:18, Rebecca Cran <rebecca@quicinc.com> wrote:
>
> It would be good if we could get this committed now the stable tag has
> been created.
>

Merged as #3287

>

> On 8/9/22 03:39, Rohit Mathew wrote:
> > Hi,
> >
> > Could anyone please let me know if there are any more comments on this patch?
> > Saw that we have moved into soft freeze for edk2 and wanted to add that we had one round of review on the patch.
> >
> > Regards,
> > Rohit
> >
> >> -----Original Message-----
> >> From: Sami Mujawar <Sami.Mujawar@arm.com>
> >> Sent: 25 July 2022 12:12
> >> To: Rohit Mathew <Rohit.Mathew@arm.com>; devel@edk2.groups.io
> >> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Rebecca Cran
> >> <rebecca@bsdio.com>; Pierre Gondois <Pierre.Gondois@arm.com>; Thomas
> >> Abraham <thomas.abraham@arm.com>; nd <nd@arm.com>
> >> Subject: Re: [edk2][PATCH V4 1/1] ArmPlatformPkg/PrePeiCore: Invoke
> >> constructors for SEC phase
> >>
> >> Hi Rohit,
> >>
> >> This patch looks good to me.
> >>
> >> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> >>
> >> Regards,
> >>
> >> Sami Mujawar
> >>
> >> On 22/07/2022 06:02 pm, Rohit Mathew wrote:
> >>> On platforms that do not have the serial console port pre-initialized
> >>> prior to the SEC phase and due to the absence of a call to
> >>> "SerialPortInitialize", this results in missing debug logs. So, call
> >>> the auto-generated "ProcessLibraryConstructorList" function from SEC
> >>> phase to have all the dependent library constructors called (this
> >>> includes a call to "SerialPortInitialize").
> >>>
> >>> Signed-off-by: Rohit Mathew <rohit.mathew@arm.com>
> >>> ---
> >>>    ArmPlatformPkg/PrePeiCore/PrePeiCore.h | 12 +++++++++++-
> >>>    ArmPlatformPkg/PrePeiCore/PrePeiCore.c |  6 +++++-
> >>>    2 files changed, 16 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.
> >>>
> >>> Changes since V3:
> >>> - Addressed comments from Sami.
> >>>
> >>> 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..c1e13e23e11e 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,14 @@ PeiCommonExceptionEntry (
> >>>      IN UINTN   LR
> >>>      );
> >>>
> >>> +/*
> >>> + * Autogenerated function that calls the library constructors for all
> >>> +of the
> >>> + * module's dependent libraries.
> >>> + */
> >>> +VOID
> >>> +EFIAPI
> >>> +ProcessLibraryConstructorList (
> >>> +  VOID
> >>> +  );
> >>> +
> >>>    #endif
> >>> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> >>> b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> >>> index 6dd9bcdea24f..9c4b25df953d 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
> >>>
> >>> @@ -90,6 +90,10 @@ CEntryPoint (
> >>>
> >>>      // If not primary Jump to Secondary Main
> >>>      if (ArmPlatformIsPrimaryCore (MpId)) {
> >>> +    // Invoke "ProcessLibraryConstructorList" to have all library constructors
> >>> +    // called.
> >>> +    ProcessLibraryConstructorList ();
> >>> +
> >>>        // Initialize the Debug Agent for Source Level Debugging
> >>>        InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
> >>>        SaveAndSetDebugTimerInterrupt (TRUE);
> >
> > 
> >
> >
>