public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] MdePkg DxeHobLib: Make GetHobList working before Constructor is called
@ 2017-01-16  9:07 Star Zeng
  2017-01-20  7:26 ` Gao, Liming
  0 siblings, 1 reply; 2+ messages in thread
From: Star Zeng @ 2017-01-16  9:07 UTC (permalink / raw)
  To: edk2-devel; +Cc: Star Zeng, Jiewen Yao, Liming Gao, Michael Kinney, Amy Chan

The latest PiSmmCore driver added several debug messages in the
function SmmAddMemoryRegion in Page.c. The function SmmAddMemoryRegion
is called by the library constructor
PiSmmCoreMemoryAllocationLibConstructor.

When PiSmmCoreMemoryAllocationLibConstructor is executed, the
constructor of DxeHobLib (HobLibConstructor in HobLib.c) is not
executed yet. But platform instance of DebugLib may need get hob
before printing any message. As a result, an ASSERT happens in the
function GetHobList.

The patch is to update GetHobList to get HOB list from system
configuration table when the HOB list is not retrieved and not cached
yet, and HobLibConstructor is also to be updated to just call
GetHobList.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Amy Chan <amy.chan@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 MdePkg/Library/DxeHobLib/HobLib.c | 67 ++++++++++++++++++++-------------------
 1 file changed, 34 insertions(+), 33 deletions(-)

diff --git a/MdePkg/Library/DxeHobLib/HobLib.c b/MdePkg/Library/DxeHobLib/HobLib.c
index c6c04e6a5924..bb65206b044a 100644
--- a/MdePkg/Library/DxeHobLib/HobLib.c
+++ b/MdePkg/Library/DxeHobLib/HobLib.c
@@ -1,7 +1,7 @@
 /** @file
   HOB Library implemenation for Dxe Phase.
 
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -24,35 +24,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 VOID  *mHobList = NULL;
 
 /**
-  The constructor function caches the pointer to HOB list.
-  
-  The constructor function gets the start address of HOB list from system configuration table.
-  It will ASSERT() if that operation fails and it will always return EFI_SUCCESS. 
-
-  @param  ImageHandle   The firmware allocated handle for the EFI image.
-  @param  SystemTable   A pointer to the EFI System Table.
-  
-  @retval EFI_SUCCESS   The constructor successfully gets HobList.
-  @retval Other value   The constructor can't get HobList.
-
-**/
-EFI_STATUS
-EFIAPI
-HobLibConstructor (
-  IN EFI_HANDLE        ImageHandle,
-  IN EFI_SYSTEM_TABLE  *SystemTable
-  )
-{
-  EFI_STATUS  Status;
-
-  Status = EfiGetSystemConfigurationTable (&gEfiHobListGuid, &mHobList);
-  ASSERT_EFI_ERROR (Status);
-  ASSERT (mHobList != NULL);
-
-  return Status;
-}
-
-/**
   Returns the pointer to the HOB list.
 
   This function returns the pointer to first HOB in the list.
@@ -62,9 +33,11 @@ HobLibConstructor (
   Since the System Configuration Table does not exist that the time the DXE Core is 
   launched, the DXE Core uses a global variable from the DXE Core Entry Point Library 
   to manage the pointer to the HOB list.
-  
+
   If the pointer to the HOB list is NULL, then ASSERT().
-  
+
+  This function also caches the pointer to the HOB list retrieved.
+
   @return The pointer to the HOB list.
 
 **/
@@ -74,11 +47,39 @@ GetHobList (
   VOID
   )
 {
-  ASSERT (mHobList != NULL);
+  EFI_STATUS  Status;
+
+  if (mHobList == NULL) {
+    Status = EfiGetSystemConfigurationTable (&gEfiHobListGuid, &mHobList);
+    ASSERT_EFI_ERROR (Status);
+    ASSERT (mHobList != NULL);
+  }
   return mHobList;
 }
 
 /**
+  The constructor function caches the pointer to HOB list by calling GetHobList()
+  and will always return EFI_SUCCESS. 
+
+  @param  ImageHandle   The firmware allocated handle for the EFI image.
+  @param  SystemTable   A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS   The constructor successfully gets HobList.
+
+**/
+EFI_STATUS
+EFIAPI
+HobLibConstructor (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  GetHobList ();
+
+  return EFI_SUCCESS;
+}
+
+/**
   Returns the next instance of a HOB type from the starting HOB.
 
   This function searches the first instance of a HOB type from the starting HOB pointer. 
-- 
2.7.0.windows.1



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] MdePkg DxeHobLib: Make GetHobList working before Constructor is called
  2017-01-16  9:07 [PATCH] MdePkg DxeHobLib: Make GetHobList working before Constructor is called Star Zeng
@ 2017-01-20  7:26 ` Gao, Liming
  0 siblings, 0 replies; 2+ messages in thread
From: Gao, Liming @ 2017-01-20  7:26 UTC (permalink / raw)
  To: Zeng, Star, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Chan, Amy, Yao, Jiewen, Zeng, Star

Reviewed-by: Liming Gao <liming.gao@intel.com>

>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Star
>Zeng
>Sent: Monday, January 16, 2017 5:07 PM
>To: edk2-devel@lists.01.org
>Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Chan, Amy
><amy.chan@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming
><liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
>Subject: [edk2] [PATCH] MdePkg DxeHobLib: Make GetHobList working
>before Constructor is called
>
>The latest PiSmmCore driver added several debug messages in the
>function SmmAddMemoryRegion in Page.c. The function
>SmmAddMemoryRegion
>is called by the library constructor
>PiSmmCoreMemoryAllocationLibConstructor.
>
>When PiSmmCoreMemoryAllocationLibConstructor is executed, the
>constructor of DxeHobLib (HobLibConstructor in HobLib.c) is not
>executed yet. But platform instance of DebugLib may need get hob
>before printing any message. As a result, an ASSERT happens in the
>function GetHobList.
>
>The patch is to update GetHobList to get HOB list from system
>configuration table when the HOB list is not retrieved and not cached
>yet, and HobLibConstructor is also to be updated to just call
>GetHobList.
>
>Cc: Jiewen Yao <jiewen.yao@intel.com>
>Cc: Liming Gao <liming.gao@intel.com>
>Cc: Michael Kinney <michael.d.kinney@intel.com>
>Cc: Amy Chan <amy.chan@intel.com>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Star Zeng <star.zeng@intel.com>
>---
> MdePkg/Library/DxeHobLib/HobLib.c | 67 ++++++++++++++++++++-----------
>--------
> 1 file changed, 34 insertions(+), 33 deletions(-)
>
>diff --git a/MdePkg/Library/DxeHobLib/HobLib.c
>b/MdePkg/Library/DxeHobLib/HobLib.c
>index c6c04e6a5924..bb65206b044a 100644
>--- a/MdePkg/Library/DxeHobLib/HobLib.c
>+++ b/MdePkg/Library/DxeHobLib/HobLib.c
>@@ -1,7 +1,7 @@
> /** @file
>   HOB Library implemenation for Dxe Phase.
>
>-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
>License
> which accompanies this distribution.  The full text of the license may be found
>at
>@@ -24,35 +24,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
>KIND, EITHER EXPRESS OR IMPLIED.
> VOID  *mHobList = NULL;
>
> /**
>-  The constructor function caches the pointer to HOB list.
>-
>-  The constructor function gets the start address of HOB list from system
>configuration table.
>-  It will ASSERT() if that operation fails and it will always return EFI_SUCCESS.
>-
>-  @param  ImageHandle   The firmware allocated handle for the EFI image.
>-  @param  SystemTable   A pointer to the EFI System Table.
>-
>-  @retval EFI_SUCCESS   The constructor successfully gets HobList.
>-  @retval Other value   The constructor can't get HobList.
>-
>-**/
>-EFI_STATUS
>-EFIAPI
>-HobLibConstructor (
>-  IN EFI_HANDLE        ImageHandle,
>-  IN EFI_SYSTEM_TABLE  *SystemTable
>-  )
>-{
>-  EFI_STATUS  Status;
>-
>-  Status = EfiGetSystemConfigurationTable (&gEfiHobListGuid, &mHobList);
>-  ASSERT_EFI_ERROR (Status);
>-  ASSERT (mHobList != NULL);
>-
>-  return Status;
>-}
>-
>-/**
>   Returns the pointer to the HOB list.
>
>   This function returns the pointer to first HOB in the list.
>@@ -62,9 +33,11 @@ HobLibConstructor (
>   Since the System Configuration Table does not exist that the time the DXE
>Core is
>   launched, the DXE Core uses a global variable from the DXE Core Entry Point
>Library
>   to manage the pointer to the HOB list.
>-
>+
>   If the pointer to the HOB list is NULL, then ASSERT().
>-
>+
>+  This function also caches the pointer to the HOB list retrieved.
>+
>   @return The pointer to the HOB list.
>
> **/
>@@ -74,11 +47,39 @@ GetHobList (
>   VOID
>   )
> {
>-  ASSERT (mHobList != NULL);
>+  EFI_STATUS  Status;
>+
>+  if (mHobList == NULL) {
>+    Status = EfiGetSystemConfigurationTable (&gEfiHobListGuid, &mHobList);
>+    ASSERT_EFI_ERROR (Status);
>+    ASSERT (mHobList != NULL);
>+  }
>   return mHobList;
> }
>
> /**
>+  The constructor function caches the pointer to HOB list by calling
>GetHobList()
>+  and will always return EFI_SUCCESS.
>+
>+  @param  ImageHandle   The firmware allocated handle for the EFI image.
>+  @param  SystemTable   A pointer to the EFI System Table.
>+
>+  @retval EFI_SUCCESS   The constructor successfully gets HobList.
>+
>+**/
>+EFI_STATUS
>+EFIAPI
>+HobLibConstructor (
>+  IN EFI_HANDLE        ImageHandle,
>+  IN EFI_SYSTEM_TABLE  *SystemTable
>+  )
>+{
>+  GetHobList ();
>+
>+  return EFI_SUCCESS;
>+}
>+
>+/**
>   Returns the next instance of a HOB type from the starting HOB.
>
>   This function searches the first instance of a HOB type from the starting HOB
>pointer.
>--
>2.7.0.windows.1
>
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-01-20  7:26 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-16  9:07 [PATCH] MdePkg DxeHobLib: Make GetHobList working before Constructor is called Star Zeng
2017-01-20  7:26 ` Gao, Liming

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox