| Message ID | 20190305133248.4828-6-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | 4b771927c801a873c8b8169b07e77e64d7726198 |
| Headers | show |
| Series | StandaloneMmPkg, ArmPkg: cleanups and improvements | expand |
Reviewed-by: Jiewen.yao@intel.com > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Tuesday, March 5, 2019 5:33 AM > To: edk2-devel@lists.01.org > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta > <achin.gupta@arm.com>; Supreeth Venkatesh > <supreeth.venkatesh@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>; > Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja > <jagadeesh.ujja@arm.com> > 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 <ard.biesheuvel@linaro.org> > --- > > 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
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 <ard.biesheuvel@linaro.org> > --- > 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
On Wed, 6 Mar 2019 at 17:35, Achin Gupta <Achin.Gupta@arm.com> 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 <ard.biesheuvel@linaro.org> > > --- > > 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
On Wed, Mar 06, 2019 at 05:41:30PM +0100, Ard Biesheuvel wrote: > On Wed, 6 Mar 2019 at 17:35, Achin Gupta <Achin.Gupta@arm.com> 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 <ard.biesheuvel@linaro.org> > > > --- > > > 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.gupta@arm.com _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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)) {
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 <ard.biesheuvel@linaro.org> --- StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c | 3 --- 1 file changed, 3 deletions(-) -- 2.20.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel