Re: [edk2] [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call

2019-03-06 Thread Achin Gupta 
On Wed, Mar 06, 2019 at 05:41:30PM +0100, Ard Biesheuvel wrote:
> On Wed, 6 Mar 2019 at 17:35, Achin Gupta  wrote:
> >
> > Hi Ard,
> >
> > On Tue, Mar 05, 2019 at 02:32:43PM +0100, Ard Biesheuvel wrote:
> > > Sending DEBUG output to the serial port should only be done via
> > > DebugLib calls, which is in charge of initializing the serial
> > > port when appropriate. So drop the explicit SerialPortInitialize ()
> > > invocation, and rely on normal constructor ordering to get the
> > > serial port into the appropriate state at the right time.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Ard Biesheuvel 
> > > ---
> > >  
> > > StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> > >  | 3 ---
> > >  1 file changed, 3 deletions(-)
> > >
> > > diff --git 
> > > a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> > >  
> > > b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> > > index 5cca532456fd..c8e11a253d24 100644
> > > --- 
> > > a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> > > +++ 
> > > b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> > > @@ -232,9 +232,6 @@ _ModuleEntryPoint (
> > >VOID*TeData;
> > >UINTN   TeDataSize;
> > >
> > > -  Status = SerialPortInitialize ();
> > > -  ASSERT_EFI_ERROR (Status);
> >
> > This is done in the first few instructions after EL3 ERETs into S-EL0 to
> > initialise the StMM partition. The constructors will be called a bit later. 
> > I
> > agree with the change but does EDK2 provide a mechanism for early prints to 
> > the
> > console in case we need this in future.
> >
>
> If so, the correct way to achieve this would be to call the DebugLib
> constructor by hand, and that should call the SerialPortLib
> constructor. Unfortunately, EDK2 is not put together like that, and
> especially constructor ordering is slightly broken.

Thanks!

Reviewed-by: achin.gu...@arm.com
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call

2019-03-06 Thread Ard Biesheuvel 
On Wed, 6 Mar 2019 at 17:35, Achin Gupta  wrote:
>
> Hi Ard,
>
> On Tue, Mar 05, 2019 at 02:32:43PM +0100, Ard Biesheuvel wrote:
> > Sending DEBUG output to the serial port should only be done via
> > DebugLib calls, which is in charge of initializing the serial
> > port when appropriate. So drop the explicit SerialPortInitialize ()
> > invocation, and rely on normal constructor ordering to get the
> > serial port into the appropriate state at the right time.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >  
> > StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> >  | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git 
> > a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> >  
> > b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> > index 5cca532456fd..c8e11a253d24 100644
> > --- 
> > a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> > +++ 
> > b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> > @@ -232,9 +232,6 @@ _ModuleEntryPoint (
> >VOID*TeData;
> >UINTN   TeDataSize;
> >
> > -  Status = SerialPortInitialize ();
> > -  ASSERT_EFI_ERROR (Status);
>
> This is done in the first few instructions after EL3 ERETs into S-EL0 to
> initialise the StMM partition. The constructors will be called a bit later. I
> agree with the change but does EDK2 provide a mechanism for early prints to 
> the
> console in case we need this in future.
>

If so, the correct way to achieve this would be to call the DebugLib
constructor by hand, and that should call the SerialPortLib
constructor. Unfortunately, EDK2 is not put together like that, and
especially constructor ordering is slightly broken.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call

2019-03-06 Thread Achin Gupta 
Hi Ard,

On Tue, Mar 05, 2019 at 02:32:43PM +0100, Ard Biesheuvel wrote:
> Sending DEBUG output to the serial port should only be done via
> DebugLib calls, which is in charge of initializing the serial
> port when appropriate. So drop the explicit SerialPortInitialize ()
> invocation, and rely on normal constructor ordering to get the
> serial port into the appropriate state at the right time.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
>  | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git 
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
>  
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> index 5cca532456fd..c8e11a253d24 100644
> --- 
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> +++ 
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> @@ -232,9 +232,6 @@ _ModuleEntryPoint (
>VOID*TeData;
>UINTN   TeDataSize;
>
> -  Status = SerialPortInitialize ();
> -  ASSERT_EFI_ERROR (Status);

This is done in the first few instructions after EL3 ERETs into S-EL0 to
initialise the StMM partition. The constructors will be called a bit later. I
agree with the change but does EDK2 provide a mechanism for early prints to the
console in case we need this in future.

cheers,
Achin

> -
>// Get Secure Partition Manager Version Information
>Status = GetSpmVersion ();
>if (EFI_ERROR (Status)) {
> --
> 2.20.1
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call

2019-03-05 Thread Yao, Jiewen 
Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel ; Achin Gupta
> ; Supreeth Venkatesh
> ; Yao, Jiewen ;
> Leif Lindholm ; Jagadeesh Ujja
> 
> Subject: [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint:
> drop explicit SerialPortLib call
> 
> Sending DEBUG output to the serial port should only be done via
> DebugLib calls, which is in charge of initializing the serial
> port when appropriate. So drop the explicit SerialPortInitialize ()
> invocation, and rely on normal constructor ordering to get the
> serial port into the appropriate state at the right time.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
> 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standal
> oneMmCoreEntryPoint.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> aloneMmCoreEntryPoint.c
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> aloneMmCoreEntryPoint.c
> index 5cca532456fd..c8e11a253d24 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> aloneMmCoreEntryPoint.c
> +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> aloneMmCoreEntryPoint.c
> @@ -232,9 +232,6 @@ _ModuleEntryPoint (
>VOID*TeData;
>UINTN   TeDataSize;
> 
> -  Status = SerialPortInitialize ();
> -  ASSERT_EFI_ERROR (Status);
> -
>// Get Secure Partition Manager Version Information
>Status = GetSpmVersion ();
>if (EFI_ERROR (Status)) {
> --
> 2.20.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call

2019-03-05 Thread Ard Biesheuvel 
Sending DEBUG output to the serial port should only be done via
DebugLib calls, which is in charge of initializing the serial
port when appropriate. So drop the explicit SerialPortInitialize ()
invocation, and rely on normal constructor ordering to get the
serial port into the appropriate state at the right time.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
 | 3 ---
 1 file changed, 3 deletions(-)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
index 5cca532456fd..c8e11a253d24 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
@@ -232,9 +232,6 @@ _ModuleEntryPoint (
   VOID*TeData;
   UINTN   TeDataSize;
 
-  Status = SerialPortInitialize ();
-  ASSERT_EFI_ERROR (Status);
-
   // Get Secure Partition Manager Version Information
   Status = GetSpmVersion ();
   if (EFI_ERROR (Status)) {
-- 
2.20.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel