public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type
@ 2021-12-16  8:10 Ashraf Ali S
  2021-12-16 10:46 ` ted.kuo
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Ashraf Ali S @ 2021-12-16  8:10 UTC (permalink / raw)
  To: devel
  Cc: Ashraf Ali S, Chasel Chiu, Nate DeSimone, Star Zeng, Kuo Ted,
	Duggapu Chinni B, Rangasai V Chaganty, Digant H Solanki,
	Sangeetha V, Ray Ni

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3642
when the module is not building in IA32 mode which will lead to building
error. when a module built-in X64 function pointer will be the size of
64bit width which cannot be fit in 32bit address which will lead to
error. to overcome this issue introducing the 2 new PCD's for the 64bit
modules can consume it. based on the which pcd platform set, use that.

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Kuo Ted <ted.kuo@intel.com>
Cc: Duggapu Chinni B <chinni.b.duggapu@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Digant H Solanki <digant.h.solanki@intel.com>
Cc: Sangeetha V <sangeetha.v@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
---
 .../FspmWrapperPeim/FspmWrapperPeim.c         | 25 ++++++++++++++++---
 .../FspmWrapperPeim/FspmWrapperPeim.inf       |  3 ++-
 .../FspsWrapperPeim/FspsWrapperPeim.c         | 25 ++++++++++++++++---
 .../FspsWrapperPeim/FspsWrapperPeim.inf       |  3 ++-
 IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec   |  2 ++
 5 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
index 287e7f9159..49fbb27eca 100644
--- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
@@ -3,7 +3,7 @@
   register TemporaryRamDonePpi to call TempRamExit API, and register MemoryDiscoveredPpi
   notify to call FspSiliconInit API.
 
-  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -38,6 +38,25 @@
 
 extern EFI_GUID  gFspHobGuid;
 
+/**
+  Get the FSP M UPD Data address
+
+  @return FSP-M UPD Data Address
+**/
+
+UINTN
+EFIAPI
+GetFspmUpdDataAddress (
+  VOID
+  )
+{
+  if (PcdGet64 (PcdFspmUpdDataAddress64) != 0) {
+    return (UINTN) PcdGet64 (PcdFspmUpdDataAddress64);
+  } else {
+    return (UINTN) PcdGet32 (PcdFspmUpdDataAddress);
+  }
+}
+
 /**
   Call FspMemoryInit API.
 
@@ -67,7 +86,7 @@ PeiFspMemoryInit (
     return EFI_DEVICE_ERROR;
   }
 
-  if ((PcdGet32 (PcdFspmUpdDataAddress) == 0) && (FspmHeaderPtr->CfgRegionSize != 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
+  if ((GetFspmUpdDataAddress () == 0) && (FspmHeaderPtr->CfgRegionSize != 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
     //
     // Copy default FSP-M UPD data from Flash
     //
@@ -79,7 +98,7 @@ PeiFspMemoryInit (
     //
     // External UPD is ready, get the buffer from PCD pointer.
     //
-    FspmUpdDataPtr = (FSPM_UPD_COMMON *)PcdGet32 (PcdFspmUpdDataAddress);
+    FspmUpdDataPtr = (FSPM_UPD_COMMON *) GetFspmUpdDataAddress();
     ASSERT (FspmUpdDataPtr != NULL);
   }
 
diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
index 00166e56a0..5d0e021401 100644
--- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
@@ -6,7 +6,7 @@
 # register TemporaryRamDonePpi to call TempRamExit API, and register MemoryDiscoveredPpi
 # notify to call FspSiliconInit API.
 #
-#  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -60,6 +60,7 @@
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
   gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress       ## CONSUMES
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64  ## CONSUMES
 
 [Sources]
   FspmWrapperPeim.c
diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
index f7459a90b5..ddee9cd029 100644
--- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
+++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
@@ -3,7 +3,7 @@
   register TemporaryRamDonePpi to call TempRamExit API, and register MemoryDiscoveredPpi
   notify to call FspSiliconInit API.
 
-  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -181,6 +181,25 @@ FspSiliconInitDoneGetFspHobList (
   }
 }
 
+/**
+  Get the FSP S UPD Data address
+
+  @return FSP-S UPD Data Address
+**/
+
+UINTN
+EFIAPI
+GetFspsUpdDataAddress (
+  VOID
+  )
+{
+  if (PcdGet64 (PcdFspsUpdDataAddress64) != 0) {
+    return (UINTN) PcdGet64 (PcdFspsUpdDataAddress64);
+  } else {
+    return (UINTN) PcdGet32 (PcdFspsUpdDataAddress);
+  }
+}
+
 /**
   This function is for FSP dispatch mode to perform post FSP-S process.
 
@@ -283,7 +302,7 @@ PeiMemoryDiscoveredNotify (
     return EFI_DEVICE_ERROR;
   }
 
-  if ((PcdGet32 (PcdFspsUpdDataAddress) == 0) && (FspsHeaderPtr->CfgRegionSize != 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
+  if ((GetFspsUpdDataAddress () == 0) && (FspsHeaderPtr->CfgRegionSize != 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
     //
     // Copy default FSP-S UPD data from Flash
     //
@@ -292,7 +311,7 @@ PeiMemoryDiscoveredNotify (
     SourceData = (UINTN *)((UINTN)FspsHeaderPtr->ImageBase + (UINTN)FspsHeaderPtr->CfgRegionOffset);
     CopyMem (FspsUpdDataPtr, SourceData, (UINTN)FspsHeaderPtr->CfgRegionSize);
   } else {
-    FspsUpdDataPtr = (FSPS_UPD_COMMON *)PcdGet32 (PcdFspsUpdDataAddress);
+    FspsUpdDataPtr = (FSPS_UPD_COMMON *) GetFspsUpdDataAddress();
     ASSERT (FspsUpdDataPtr != NULL);
   }
 
diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
index aeeca58d6d..da0049a654 100644
--- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
+++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
@@ -6,7 +6,7 @@
 # register TemporaryRamDonePpi to call TempRamExit API, and register MemoryDiscoveredPpi
 # notify to call FspSiliconInit API.
 #
-#  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -68,6 +68,7 @@
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64  ## CONSUMES
 
 [Guids]
   gFspHobGuid                           ## CONSUMES ## HOB
diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
index b8dac1b574..a5a8b8a19e 100644
--- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
+++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
@@ -121,3 +121,5 @@
   #
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress|0x00000000|UINT32|0x50000000
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress|0x00000000|UINT32|0x50000001
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64|0x00000000|UINT64|0x50000002
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64|0x00000000|UINT64|0x50000003
-- 
2.30.2.windows.1


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

* Re: [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type
  2021-12-16  8:10 [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type Ashraf Ali S
@ 2021-12-16 10:46 ` ted.kuo
  2021-12-17  1:53   ` Chiu, Chasel
  2021-12-17  3:47 ` Chiu, Chasel
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: ted.kuo @ 2021-12-16 10:46 UTC (permalink / raw)
  To: S, Ashraf Ali, devel@edk2.groups.io
  Cc: Chiu, Chasel, Desimone, Nathaniel L, Zeng, Star,
	Duggapu, Chinni B, Chaganty, Rangasai V, Solanki, Digant H,
	V, Sangeetha, Ni, Ray

Can we use the size of UINTN instead of the value of PcdFspmUpdDataAddress64 to decide which PCD should be returned in GetFspmUpdDataAddress and GetFspsUpdDataAddress?

Thanks,
Ted

-----Original Message-----
From: S, Ashraf Ali <ashraf.ali.s@intel.com> 
Sent: Thursday, December 16, 2021 4:10 PM
To: devel@edk2.groups.io
Cc: S, Ashraf Ali <ashraf.ali.s@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Solanki, Digant H <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3642
when the module is not building in IA32 mode which will lead to building error. when a module built-in X64 function pointer will be the size of 64bit width which cannot be fit in 32bit address which will lead to error. to overcome this issue introducing the 2 new PCD's for the 64bit modules can consume it. based on the which pcd platform set, use that.

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Kuo Ted <ted.kuo@intel.com>
Cc: Duggapu Chinni B <chinni.b.duggapu@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Digant H Solanki <digant.h.solanki@intel.com>
Cc: Sangeetha V <sangeetha.v@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
---
 .../FspmWrapperPeim/FspmWrapperPeim.c         | 25 ++++++++++++++++---
 .../FspmWrapperPeim/FspmWrapperPeim.inf       |  3 ++-
 .../FspsWrapperPeim/FspsWrapperPeim.c         | 25 ++++++++++++++++---
 .../FspsWrapperPeim/FspsWrapperPeim.inf       |  3 ++-
 IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec   |  2 ++
 5 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
index 287e7f9159..49fbb27eca 100644
--- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
@@ -3,7 +3,7 @@
   register TemporaryRamDonePpi to call TempRamExit API, and register MemoryDiscoveredPpi
   notify to call FspSiliconInit API.
 
-  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
+ reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -38,6 +38,25 @@
 
 extern EFI_GUID  gFspHobGuid;
 
+/**
+  Get the FSP M UPD Data address
+
+  @return FSP-M UPD Data Address
+**/
+
+UINTN
+EFIAPI
+GetFspmUpdDataAddress (
+  VOID
+  )
+{
+  if (PcdGet64 (PcdFspmUpdDataAddress64) != 0) {
+    return (UINTN) PcdGet64 (PcdFspmUpdDataAddress64);
+  } else {
+    return (UINTN) PcdGet32 (PcdFspmUpdDataAddress);
+  }
+}
+
 /**
   Call FspMemoryInit API.
 
@@ -67,7 +86,7 @@ PeiFspMemoryInit (
     return EFI_DEVICE_ERROR;
   }
 
-  if ((PcdGet32 (PcdFspmUpdDataAddress) == 0) && (FspmHeaderPtr->CfgRegionSize != 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
+  if ((GetFspmUpdDataAddress () == 0) && (FspmHeaderPtr->CfgRegionSize 
+ != 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
     //
     // Copy default FSP-M UPD data from Flash
     //
@@ -79,7 +98,7 @@ PeiFspMemoryInit (
     //
     // External UPD is ready, get the buffer from PCD pointer.
     //
-    FspmUpdDataPtr = (FSPM_UPD_COMMON *)PcdGet32 (PcdFspmUpdDataAddress);
+    FspmUpdDataPtr = (FSPM_UPD_COMMON *) GetFspmUpdDataAddress();
     ASSERT (FspmUpdDataPtr != NULL);
   }
 
diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
index 00166e56a0..5d0e021401 100644
--- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
@@ -6,7 +6,7 @@
 # register TemporaryRamDonePpi to call TempRamExit API, and register MemoryDiscoveredPpi  # notify to call FspSiliconInit API.
 #
-#  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
+reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -60,6 +60,7 @@
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
   gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress       ## CONSUMES
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64  ## CONSUMES
 
 [Sources]
   FspmWrapperPeim.c
diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
index f7459a90b5..ddee9cd029 100644
--- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
+++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
@@ -3,7 +3,7 @@
   register TemporaryRamDonePpi to call TempRamExit API, and register MemoryDiscoveredPpi
   notify to call FspSiliconInit API.
 
-  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
+ reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -181,6 +181,25 @@ FspSiliconInitDoneGetFspHobList (
   }
 }
 
+/**
+  Get the FSP S UPD Data address
+
+  @return FSP-S UPD Data Address
+**/
+
+UINTN
+EFIAPI
+GetFspsUpdDataAddress (
+  VOID
+  )
+{
+  if (PcdGet64 (PcdFspsUpdDataAddress64) != 0) {
+    return (UINTN) PcdGet64 (PcdFspsUpdDataAddress64);
+  } else {
+    return (UINTN) PcdGet32 (PcdFspsUpdDataAddress);
+  }
+}
+
 /**
   This function is for FSP dispatch mode to perform post FSP-S process.
 
@@ -283,7 +302,7 @@ PeiMemoryDiscoveredNotify (
     return EFI_DEVICE_ERROR;
   }
 
-  if ((PcdGet32 (PcdFspsUpdDataAddress) == 0) && (FspsHeaderPtr->CfgRegionSize != 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
+  if ((GetFspsUpdDataAddress () == 0) && (FspsHeaderPtr->CfgRegionSize 
+ != 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
     //
     // Copy default FSP-S UPD data from Flash
     //
@@ -292,7 +311,7 @@ PeiMemoryDiscoveredNotify (
     SourceData = (UINTN *)((UINTN)FspsHeaderPtr->ImageBase + (UINTN)FspsHeaderPtr->CfgRegionOffset);
     CopyMem (FspsUpdDataPtr, SourceData, (UINTN)FspsHeaderPtr->CfgRegionSize);
   } else {
-    FspsUpdDataPtr = (FSPS_UPD_COMMON *)PcdGet32 (PcdFspsUpdDataAddress);
+    FspsUpdDataPtr = (FSPS_UPD_COMMON *) GetFspsUpdDataAddress();
     ASSERT (FspsUpdDataPtr != NULL);
   }
 
diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
index aeeca58d6d..da0049a654 100644
--- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
+++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
@@ -6,7 +6,7 @@
 # register TemporaryRamDonePpi to call TempRamExit API, and register MemoryDiscoveredPpi  # notify to call FspSiliconInit API.
 #
-#  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
+reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -68,6 +68,7 @@
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64  ## CONSUMES
 
 [Guids]
   gFspHobGuid                           ## CONSUMES ## HOB
diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
index b8dac1b574..a5a8b8a19e 100644
--- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
+++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
@@ -121,3 +121,5 @@
   #
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress|0x00000000|UINT32|0x50000000
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress|0x00000000|UINT32|0x50000001
+  
+ gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64|0x00000000|UIN
+ T64|0x50000002
+  
+ gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64|0x00000000|UIN
+ T64|0x50000003
--
2.30.2.windows.1


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

* Re: [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type
  2021-12-16 10:46 ` ted.kuo
@ 2021-12-17  1:53   ` Chiu, Chasel
  0 siblings, 0 replies; 8+ messages in thread
From: Chiu, Chasel @ 2021-12-17  1:53 UTC (permalink / raw)
  To: Kuo, Ted, S, Ashraf Ali, devel@edk2.groups.io
  Cc: Desimone, Nathaniel L, Zeng, Star, Duggapu, Chinni B,
	Chaganty, Rangasai V, Solanki, Digant H, V, Sangeetha, Ni, Ray


Hi Ted,

Some bootloaders may choose to always use PcdFspmUpdDataAddress64 regardless 32bit or 64bit builds, we do not need to block such usage cases by checking UINTN size.

Thanks,
Chasel


> -----Original Message-----
> From: Kuo, Ted <ted.kuo@intel.com>
> Sent: Thursday, December 16, 2021 6:47 PM
> To: S, Ashraf Ali <ashraf.ali.s@intel.com>; devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; Duggapu,
> Chinni B <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Solanki, Digant H
> <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: RE: [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based
> on Build Type
> 
> Can we use the size of UINTN instead of the value of
> PcdFspmUpdDataAddress64 to decide which PCD should be returned in
> GetFspmUpdDataAddress and GetFspsUpdDataAddress?
> 
> Thanks,
> Ted
> 
> -----Original Message-----
> From: S, Ashraf Ali <ashraf.ali.s@intel.com>
> Sent: Thursday, December 16, 2021 4:10 PM
> To: devel@edk2.groups.io
> Cc: S, Ashraf Ali <ashraf.ali.s@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>;
> Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B
> <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Solanki, Digant H
> <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on
> Build Type
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3642
> when the module is not building in IA32 mode which will lead to building error.
> when a module built-in X64 function pointer will be the size of 64bit width which
> cannot be fit in 32bit address which will lead to error. to overcome this issue
> introducing the 2 new PCD's for the 64bit modules can consume it. based on the
> which pcd platform set, use that.
> 
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Kuo Ted <ted.kuo@intel.com>
> Cc: Duggapu Chinni B <chinni.b.duggapu@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Digant H Solanki <digant.h.solanki@intel.com>
> Cc: Sangeetha V <sangeetha.v@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
> ---
>  .../FspmWrapperPeim/FspmWrapperPeim.c         | 25 ++++++++++++++++---
>  .../FspmWrapperPeim/FspmWrapperPeim.inf       |  3 ++-
>  .../FspsWrapperPeim/FspsWrapperPeim.c         | 25 ++++++++++++++++---
>  .../FspsWrapperPeim/FspsWrapperPeim.inf       |  3 ++-
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec   |  2 ++
>  5 files changed, 50 insertions(+), 8 deletions(-)
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> index 287e7f9159..49fbb27eca 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -38,6 +38,25 @@
> 
>  extern EFI_GUID  gFspHobGuid;
> 
> +/**
> +  Get the FSP M UPD Data address
> +
> +  @return FSP-M UPD Data Address
> +**/
> +
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  )
> +{
> +  if (PcdGet64 (PcdFspmUpdDataAddress64) != 0) {
> +    return (UINTN) PcdGet64 (PcdFspmUpdDataAddress64);
> +  } else {
> +    return (UINTN) PcdGet32 (PcdFspmUpdDataAddress);
> +  }
> +}
> +
>  /**
>    Call FspMemoryInit API.
> 
> @@ -67,7 +86,7 @@ PeiFspMemoryInit (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if ((PcdGet32 (PcdFspmUpdDataAddress) == 0) && (FspmHeaderPtr-
> >CfgRegionSize != 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
> +  if ((GetFspmUpdDataAddress () == 0) && (FspmHeaderPtr->CfgRegionSize
> + != 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
>      //
>      // Copy default FSP-M UPD data from Flash
>      //
> @@ -79,7 +98,7 @@ PeiFspMemoryInit (
>      //
>      // External UPD is ready, get the buffer from PCD pointer.
>      //
> -    FspmUpdDataPtr = (FSPM_UPD_COMMON *)PcdGet32
> (PcdFspmUpdDataAddress);
> +    FspmUpdDataPtr = (FSPM_UPD_COMMON *) GetFspmUpdDataAddress();
>      ASSERT (FspmUpdDataPtr != NULL);
>    }
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> index 00166e56a0..5d0e021401 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> @@ -6,7 +6,7 @@
>  # register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi  # notify to call FspSiliconInit API.
>  #
> -#  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2014 - 2021, Intel Corporation. All rights
> +reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -60,6 +60,7 @@
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress       ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ##
> CONSUMES
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64  ##
> CONSUMES
> 
>  [Sources]
>    FspmWrapperPeim.c
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> index f7459a90b5..ddee9cd029 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -181,6 +181,25 @@ FspSiliconInitDoneGetFspHobList (
>    }
>  }
> 
> +/**
> +  Get the FSP S UPD Data address
> +
> +  @return FSP-S UPD Data Address
> +**/
> +
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  )
> +{
> +  if (PcdGet64 (PcdFspsUpdDataAddress64) != 0) {
> +    return (UINTN) PcdGet64 (PcdFspsUpdDataAddress64);
> +  } else {
> +    return (UINTN) PcdGet32 (PcdFspsUpdDataAddress);
> +  }
> +}
> +
>  /**
>    This function is for FSP dispatch mode to perform post FSP-S process.
> 
> @@ -283,7 +302,7 @@ PeiMemoryDiscoveredNotify (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if ((PcdGet32 (PcdFspsUpdDataAddress) == 0) && (FspsHeaderPtr-
> >CfgRegionSize != 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
> +  if ((GetFspsUpdDataAddress () == 0) && (FspsHeaderPtr->CfgRegionSize
> + != 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
>      //
>      // Copy default FSP-S UPD data from Flash
>      //
> @@ -292,7 +311,7 @@ PeiMemoryDiscoveredNotify (
>      SourceData = (UINTN *)((UINTN)FspsHeaderPtr->ImageBase +
> (UINTN)FspsHeaderPtr->CfgRegionOffset);
>      CopyMem (FspsUpdDataPtr, SourceData, (UINTN)FspsHeaderPtr-
> >CfgRegionSize);
>    } else {
> -    FspsUpdDataPtr = (FSPS_UPD_COMMON *)PcdGet32
> (PcdFspsUpdDataAddress);
> +    FspsUpdDataPtr = (FSPS_UPD_COMMON *) GetFspsUpdDataAddress();
>      ASSERT (FspsUpdDataPtr != NULL);
>    }
> 
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> index aeeca58d6d..da0049a654 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> @@ -6,7 +6,7 @@
>  # register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi  # notify to call FspSiliconInit API.
>  #
> -#  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2014 - 2021, Intel Corporation. All rights
> +reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -68,6 +68,7 @@
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ##
> CONSUMES
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64  ##
> CONSUMES
> 
>  [Guids]
>    gFspHobGuid                           ## CONSUMES ## HOB
> diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> index b8dac1b574..a5a8b8a19e 100644
> --- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> +++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> @@ -121,3 +121,5 @@
>    #
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress|0x00000000|UIN
> T32|0x50000000
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress|0x00000000|UINT
> 32|0x50000001
> +
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64|0x00000000|U
> IN
> + T64|0x50000002
> +
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64|0x00000000|UI
> N
> + T64|0x50000003
> --
> 2.30.2.windows.1


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

* Re: [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type
  2021-12-16  8:10 [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type Ashraf Ali S
  2021-12-16 10:46 ` ted.kuo
@ 2021-12-17  3:47 ` Chiu, Chasel
  2021-12-17  4:02   ` Zeng, Star
  2021-12-20  3:45 ` Chiu, Chasel
  2021-12-20  5:15 ` Ni, Ray
  3 siblings, 1 reply; 8+ messages in thread
From: Chiu, Chasel @ 2021-12-17  3:47 UTC (permalink / raw)
  To: S, Ashraf Ali, devel@edk2.groups.io
  Cc: Desimone, Nathaniel L, Zeng, Star, Kuo, Ted, Duggapu, Chinni B,
	Chaganty, Rangasai V, Solanki, Digant H, V, Sangeetha, Ni, Ray


Thanks Ashraf!
Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>



> -----Original Message-----
> From: S, Ashraf Ali <ashraf.ali.s@intel.com>
> Sent: Thursday, December 16, 2021 4:10 PM
> To: devel@edk2.groups.io
> Cc: S, Ashraf Ali <ashraf.ali.s@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>;
> Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B
> <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Solanki, Digant H
> <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on
> Build Type
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3642
> when the module is not building in IA32 mode which will lead to building error.
> when a module built-in X64 function pointer will be the size of 64bit width which
> cannot be fit in 32bit address which will lead to error. to overcome this issue
> introducing the 2 new PCD's for the 64bit modules can consume it. based on the
> which pcd platform set, use that.
> 
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Kuo Ted <ted.kuo@intel.com>
> Cc: Duggapu Chinni B <chinni.b.duggapu@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Digant H Solanki <digant.h.solanki@intel.com>
> Cc: Sangeetha V <sangeetha.v@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
> ---
>  .../FspmWrapperPeim/FspmWrapperPeim.c         | 25 ++++++++++++++++---
>  .../FspmWrapperPeim/FspmWrapperPeim.inf       |  3 ++-
>  .../FspsWrapperPeim/FspsWrapperPeim.c         | 25 ++++++++++++++++---
>  .../FspsWrapperPeim/FspsWrapperPeim.inf       |  3 ++-
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec   |  2 ++
>  5 files changed, 50 insertions(+), 8 deletions(-)
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> index 287e7f9159..49fbb27eca 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -38,6 +38,25 @@
> 
>  extern EFI_GUID  gFspHobGuid;
> 
> +/**
> +  Get the FSP M UPD Data address
> +
> +  @return FSP-M UPD Data Address
> +**/
> +
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  )
> +{
> +  if (PcdGet64 (PcdFspmUpdDataAddress64) != 0) {
> +    return (UINTN) PcdGet64 (PcdFspmUpdDataAddress64);
> +  } else {
> +    return (UINTN) PcdGet32 (PcdFspmUpdDataAddress);
> +  }
> +}
> +
>  /**
>    Call FspMemoryInit API.
> 
> @@ -67,7 +86,7 @@ PeiFspMemoryInit (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if ((PcdGet32 (PcdFspmUpdDataAddress) == 0) && (FspmHeaderPtr-
> >CfgRegionSize != 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
> +  if ((GetFspmUpdDataAddress () == 0) && (FspmHeaderPtr->CfgRegionSize
> + != 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
>      //
>      // Copy default FSP-M UPD data from Flash
>      //
> @@ -79,7 +98,7 @@ PeiFspMemoryInit (
>      //
>      // External UPD is ready, get the buffer from PCD pointer.
>      //
> -    FspmUpdDataPtr = (FSPM_UPD_COMMON *)PcdGet32
> (PcdFspmUpdDataAddress);
> +    FspmUpdDataPtr = (FSPM_UPD_COMMON *) GetFspmUpdDataAddress();
>      ASSERT (FspmUpdDataPtr != NULL);
>    }
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> index 00166e56a0..5d0e021401 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> @@ -6,7 +6,7 @@
>  # register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi  # notify to call FspSiliconInit API.
>  #
> -#  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2014 - 2021, Intel Corporation. All rights
> +reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -60,6 +60,7 @@
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress       ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ##
> CONSUMES
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64  ##
> CONSUMES
> 
>  [Sources]
>    FspmWrapperPeim.c
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> index f7459a90b5..ddee9cd029 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -181,6 +181,25 @@ FspSiliconInitDoneGetFspHobList (
>    }
>  }
> 
> +/**
> +  Get the FSP S UPD Data address
> +
> +  @return FSP-S UPD Data Address
> +**/
> +
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  )
> +{
> +  if (PcdGet64 (PcdFspsUpdDataAddress64) != 0) {
> +    return (UINTN) PcdGet64 (PcdFspsUpdDataAddress64);
> +  } else {
> +    return (UINTN) PcdGet32 (PcdFspsUpdDataAddress);
> +  }
> +}
> +
>  /**
>    This function is for FSP dispatch mode to perform post FSP-S process.
> 
> @@ -283,7 +302,7 @@ PeiMemoryDiscoveredNotify (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if ((PcdGet32 (PcdFspsUpdDataAddress) == 0) && (FspsHeaderPtr-
> >CfgRegionSize != 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
> +  if ((GetFspsUpdDataAddress () == 0) && (FspsHeaderPtr->CfgRegionSize
> + != 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
>      //
>      // Copy default FSP-S UPD data from Flash
>      //
> @@ -292,7 +311,7 @@ PeiMemoryDiscoveredNotify (
>      SourceData = (UINTN *)((UINTN)FspsHeaderPtr->ImageBase +
> (UINTN)FspsHeaderPtr->CfgRegionOffset);
>      CopyMem (FspsUpdDataPtr, SourceData, (UINTN)FspsHeaderPtr-
> >CfgRegionSize);
>    } else {
> -    FspsUpdDataPtr = (FSPS_UPD_COMMON *)PcdGet32
> (PcdFspsUpdDataAddress);
> +    FspsUpdDataPtr = (FSPS_UPD_COMMON *) GetFspsUpdDataAddress();
>      ASSERT (FspsUpdDataPtr != NULL);
>    }
> 
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> index aeeca58d6d..da0049a654 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> @@ -6,7 +6,7 @@
>  # register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi  # notify to call FspSiliconInit API.
>  #
> -#  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2014 - 2021, Intel Corporation. All rights
> +reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -68,6 +68,7 @@
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ##
> CONSUMES
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64  ##
> CONSUMES
> 
>  [Guids]
>    gFspHobGuid                           ## CONSUMES ## HOB
> diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> index b8dac1b574..a5a8b8a19e 100644
> --- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> +++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> @@ -121,3 +121,5 @@
>    #
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress|0x00000000|UIN
> T32|0x50000000
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress|0x00000000|UINT
> 32|0x50000001
> +
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64|0x00000000|U
> IN
> + T64|0x50000002
> +
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64|0x00000000|UI
> N
> + T64|0x50000003
> --
> 2.30.2.windows.1


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

* Re: [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type
  2021-12-17  3:47 ` Chiu, Chasel
@ 2021-12-17  4:02   ` Zeng, Star
  0 siblings, 0 replies; 8+ messages in thread
From: Zeng, Star @ 2021-12-17  4:02 UTC (permalink / raw)
  To: Chiu, Chasel, S, Ashraf Ali, devel@edk2.groups.io
  Cc: Desimone, Nathaniel L, Kuo, Ted, Duggapu, Chinni B,
	Chaganty, Rangasai V, Solanki, Digant H, V, Sangeetha, Ni, Ray,
	Zeng, Star

How about adding the minor comments like below? 😊

#
# Non-0 means PcdFspmUpdDataAddress will be ignored, otherwise PcdFspmUpdDataAddress will be used.
#
gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64|0x00000000|UINT64|0x50000002
#
# Non-0 means PcdFspsUpdDataAddress will be ignored, otherwise PcdFspsUpdDataAddress will be used.
#
gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64|0x00000000|UINT64|0x50000003


Anyway, Reviewed-by: Star Zeng <star.zeng@intel.com>


Thanks,
Star
-----Original Message-----
From: Chiu, Chasel <chasel.chiu@intel.com> 
Sent: 2021年12月17日 11:48
To: S, Ashraf Ali <ashraf.ali.s@intel.com>; devel@edk2.groups.io
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Solanki, Digant H <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type


Thanks Ashraf!
Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>



> -----Original Message-----
> From: S, Ashraf Ali <ashraf.ali.s@intel.com>
> Sent: Thursday, December 16, 2021 4:10 PM
> To: devel@edk2.groups.io
> Cc: S, Ashraf Ali <ashraf.ali.s@intel.com>; Chiu, Chasel 
> <chasel.chiu@intel.com>; Desimone, Nathaniel L 
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; 
> Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B 
> <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V 
> <rangasai.v.chaganty@intel.com>; Solanki, Digant H 
> <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; 
> Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address 
> based on Build Type
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3642
> when the module is not building in IA32 mode which will lead to building error.
> when a module built-in X64 function pointer will be the size of 64bit 
> width which cannot be fit in 32bit address which will lead to error. 
> to overcome this issue introducing the 2 new PCD's for the 64bit 
> modules can consume it. based on the which pcd platform set, use that.
> 
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Kuo Ted <ted.kuo@intel.com>
> Cc: Duggapu Chinni B <chinni.b.duggapu@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Digant H Solanki <digant.h.solanki@intel.com>
> Cc: Sangeetha V <sangeetha.v@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
> ---
>  .../FspmWrapperPeim/FspmWrapperPeim.c         | 25 ++++++++++++++++---
>  .../FspmWrapperPeim/FspmWrapperPeim.inf       |  3 ++-
>  .../FspsWrapperPeim/FspsWrapperPeim.c         | 25 ++++++++++++++++---
>  .../FspsWrapperPeim/FspsWrapperPeim.inf       |  3 ++-
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec   |  2 ++
>  5 files changed, 50 insertions(+), 8 deletions(-)
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> index 287e7f9159..49fbb27eca 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register 
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -38,6 +38,25 @@
> 
>  extern EFI_GUID  gFspHobGuid;
> 
> +/**
> +  Get the FSP M UPD Data address
> +
> +  @return FSP-M UPD Data Address
> +**/
> +
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  )
> +{
> +  if (PcdGet64 (PcdFspmUpdDataAddress64) != 0) {
> +    return (UINTN) PcdGet64 (PcdFspmUpdDataAddress64);
> +  } else {
> +    return (UINTN) PcdGet32 (PcdFspmUpdDataAddress);
> +  }
> +}
> +
>  /**
>    Call FspMemoryInit API.
> 
> @@ -67,7 +86,7 @@ PeiFspMemoryInit (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if ((PcdGet32 (PcdFspmUpdDataAddress) == 0) && (FspmHeaderPtr-
> >CfgRegionSize != 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
> +  if ((GetFspmUpdDataAddress () == 0) && 
> + (FspmHeaderPtr->CfgRegionSize != 0) && 
> + (FspmHeaderPtr->CfgRegionOffset != 0)) {
>      //
>      // Copy default FSP-M UPD data from Flash
>      //
> @@ -79,7 +98,7 @@ PeiFspMemoryInit (
>      //
>      // External UPD is ready, get the buffer from PCD pointer.
>      //
> -    FspmUpdDataPtr = (FSPM_UPD_COMMON *)PcdGet32
> (PcdFspmUpdDataAddress);
> +    FspmUpdDataPtr = (FSPM_UPD_COMMON *) GetFspmUpdDataAddress();
>      ASSERT (FspmUpdDataPtr != NULL);
>    }
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> index 00166e56a0..5d0e021401 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> @@ -6,7 +6,7 @@
>  # register TemporaryRamDonePpi to call TempRamExit API, and register 
> MemoryDiscoveredPpi  # notify to call FspSiliconInit API.
>  #
> -#  Copyright (c) 2014 - 2020, Intel Corporation. All rights 
> reserved.<BR>
> +#  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
> +reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -60,6 +60,7 @@
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress       ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ## 
> CONSUMES
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64  ##
> CONSUMES
> 
>  [Sources]
>    FspmWrapperPeim.c
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> index f7459a90b5..ddee9cd029 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register 
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -181,6 +181,25 @@ FspSiliconInitDoneGetFspHobList (
>    }
>  }
> 
> +/**
> +  Get the FSP S UPD Data address
> +
> +  @return FSP-S UPD Data Address
> +**/
> +
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  )
> +{
> +  if (PcdGet64 (PcdFspsUpdDataAddress64) != 0) {
> +    return (UINTN) PcdGet64 (PcdFspsUpdDataAddress64);
> +  } else {
> +    return (UINTN) PcdGet32 (PcdFspsUpdDataAddress);
> +  }
> +}
> +
>  /**
>    This function is for FSP dispatch mode to perform post FSP-S process.
> 
> @@ -283,7 +302,7 @@ PeiMemoryDiscoveredNotify (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if ((PcdGet32 (PcdFspsUpdDataAddress) == 0) && (FspsHeaderPtr-
> >CfgRegionSize != 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
> +  if ((GetFspsUpdDataAddress () == 0) && 
> + (FspsHeaderPtr->CfgRegionSize != 0) && 
> + (FspsHeaderPtr->CfgRegionOffset != 0)) {
>      //
>      // Copy default FSP-S UPD data from Flash
>      //
> @@ -292,7 +311,7 @@ PeiMemoryDiscoveredNotify (
>      SourceData = (UINTN *)((UINTN)FspsHeaderPtr->ImageBase + 
> (UINTN)FspsHeaderPtr->CfgRegionOffset);
>      CopyMem (FspsUpdDataPtr, SourceData, (UINTN)FspsHeaderPtr-
> >CfgRegionSize);
>    } else {
> -    FspsUpdDataPtr = (FSPS_UPD_COMMON *)PcdGet32
> (PcdFspsUpdDataAddress);
> +    FspsUpdDataPtr = (FSPS_UPD_COMMON *) GetFspsUpdDataAddress();
>      ASSERT (FspsUpdDataPtr != NULL);
>    }
> 
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> index aeeca58d6d..da0049a654 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> @@ -6,7 +6,7 @@
>  # register TemporaryRamDonePpi to call TempRamExit API, and register 
> MemoryDiscoveredPpi  # notify to call FspSiliconInit API.
>  #
> -#  Copyright (c) 2014 - 2020, Intel Corporation. All rights 
> reserved.<BR>
> +#  Copyright (c) 2014 - 2021, Intel Corporation. All rights 
> +reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -68,6 +68,7 @@
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ## 
> CONSUMES
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64  ##
> CONSUMES
> 
>  [Guids]
>    gFspHobGuid                           ## CONSUMES ## HOB
> diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> index b8dac1b574..a5a8b8a19e 100644
> --- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> +++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> @@ -121,3 +121,5 @@
>    #
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress|0x00000000|UIN
> T32|0x50000000
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress|0x00000000|UINT
> 32|0x50000001
> +
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64|0x00000000|U
> IN
> + T64|0x50000002
> +
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64|0x00000000|UI
> N
> + T64|0x50000003
> --
> 2.30.2.windows.1


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

* Re: [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type
  2021-12-16  8:10 [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type Ashraf Ali S
  2021-12-16 10:46 ` ted.kuo
  2021-12-17  3:47 ` Chiu, Chasel
@ 2021-12-20  3:45 ` Chiu, Chasel
  2021-12-20  5:15 ` Ni, Ray
  3 siblings, 0 replies; 8+ messages in thread
From: Chiu, Chasel @ 2021-12-20  3:45 UTC (permalink / raw)
  To: S, Ashraf Ali, devel@edk2.groups.io
  Cc: Desimone, Nathaniel L, Zeng, Star, Kuo, Ted, Duggapu, Chinni B,
	Chaganty, Rangasai V, Solanki, Digant H, V, Sangeetha, Ni, Ray


Patch pushed:
https://github.com/tianocore/edk2/commit/de9e5b7dc721d4ca319c0455cf83577347e0abef

Thanks,
Chasel

> -----Original Message-----
> From: S, Ashraf Ali <ashraf.ali.s@intel.com>
> Sent: Thursday, December 16, 2021 4:10 PM
> To: devel@edk2.groups.io
> Cc: S, Ashraf Ali <ashraf.ali.s@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>;
> Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Kuo, Ted <ted.kuo@intel.com>; Duggapu, Chinni B
> <chinni.b.duggapu@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Solanki, Digant H
> <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on
> Build Type
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3642
> when the module is not building in IA32 mode which will lead to building error.
> when a module built-in X64 function pointer will be the size of 64bit width which
> cannot be fit in 32bit address which will lead to error. to overcome this issue
> introducing the 2 new PCD's for the 64bit modules can consume it. based on the
> which pcd platform set, use that.
> 
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Kuo Ted <ted.kuo@intel.com>
> Cc: Duggapu Chinni B <chinni.b.duggapu@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Digant H Solanki <digant.h.solanki@intel.com>
> Cc: Sangeetha V <sangeetha.v@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
> ---
>  .../FspmWrapperPeim/FspmWrapperPeim.c         | 25 ++++++++++++++++---
>  .../FspmWrapperPeim/FspmWrapperPeim.inf       |  3 ++-
>  .../FspsWrapperPeim/FspsWrapperPeim.c         | 25 ++++++++++++++++---
>  .../FspsWrapperPeim/FspsWrapperPeim.inf       |  3 ++-
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec   |  2 ++
>  5 files changed, 50 insertions(+), 8 deletions(-)
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> index 287e7f9159..49fbb27eca 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -38,6 +38,25 @@
> 
>  extern EFI_GUID  gFspHobGuid;
> 
> +/**
> +  Get the FSP M UPD Data address
> +
> +  @return FSP-M UPD Data Address
> +**/
> +
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  )
> +{
> +  if (PcdGet64 (PcdFspmUpdDataAddress64) != 0) {
> +    return (UINTN) PcdGet64 (PcdFspmUpdDataAddress64);
> +  } else {
> +    return (UINTN) PcdGet32 (PcdFspmUpdDataAddress);
> +  }
> +}
> +
>  /**
>    Call FspMemoryInit API.
> 
> @@ -67,7 +86,7 @@ PeiFspMemoryInit (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if ((PcdGet32 (PcdFspmUpdDataAddress) == 0) && (FspmHeaderPtr-
> >CfgRegionSize != 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
> +  if ((GetFspmUpdDataAddress () == 0) && (FspmHeaderPtr->CfgRegionSize
> + != 0) && (FspmHeaderPtr->CfgRegionOffset != 0)) {
>      //
>      // Copy default FSP-M UPD data from Flash
>      //
> @@ -79,7 +98,7 @@ PeiFspMemoryInit (
>      //
>      // External UPD is ready, get the buffer from PCD pointer.
>      //
> -    FspmUpdDataPtr = (FSPM_UPD_COMMON *)PcdGet32
> (PcdFspmUpdDataAddress);
> +    FspmUpdDataPtr = (FSPM_UPD_COMMON *) GetFspmUpdDataAddress();
>      ASSERT (FspmUpdDataPtr != NULL);
>    }
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> index 00166e56a0..5d0e021401 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
> @@ -6,7 +6,7 @@
>  # register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi  # notify to call FspSiliconInit API.
>  #
> -#  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2014 - 2021, Intel Corporation. All rights
> +reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -60,6 +60,7 @@
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress       ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ##
> CONSUMES
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64  ##
> CONSUMES
> 
>  [Sources]
>    FspmWrapperPeim.c
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> index f7459a90b5..ddee9cd029 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> @@ -3,7 +3,7 @@
>    register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi
>    notify to call FspSiliconInit API.
> 
> -  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2021, Intel Corporation. All rights
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -181,6 +181,25 @@ FspSiliconInitDoneGetFspHobList (
>    }
>  }
> 
> +/**
> +  Get the FSP S UPD Data address
> +
> +  @return FSP-S UPD Data Address
> +**/
> +
> +UINTN
> +EFIAPI
> +GetFspsUpdDataAddress (
> +  VOID
> +  )
> +{
> +  if (PcdGet64 (PcdFspsUpdDataAddress64) != 0) {
> +    return (UINTN) PcdGet64 (PcdFspsUpdDataAddress64);
> +  } else {
> +    return (UINTN) PcdGet32 (PcdFspsUpdDataAddress);
> +  }
> +}
> +
>  /**
>    This function is for FSP dispatch mode to perform post FSP-S process.
> 
> @@ -283,7 +302,7 @@ PeiMemoryDiscoveredNotify (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if ((PcdGet32 (PcdFspsUpdDataAddress) == 0) && (FspsHeaderPtr-
> >CfgRegionSize != 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
> +  if ((GetFspsUpdDataAddress () == 0) && (FspsHeaderPtr->CfgRegionSize
> + != 0) && (FspsHeaderPtr->CfgRegionOffset != 0)) {
>      //
>      // Copy default FSP-S UPD data from Flash
>      //
> @@ -292,7 +311,7 @@ PeiMemoryDiscoveredNotify (
>      SourceData = (UINTN *)((UINTN)FspsHeaderPtr->ImageBase +
> (UINTN)FspsHeaderPtr->CfgRegionOffset);
>      CopyMem (FspsUpdDataPtr, SourceData, (UINTN)FspsHeaderPtr-
> >CfgRegionSize);
>    } else {
> -    FspsUpdDataPtr = (FSPS_UPD_COMMON *)PcdGet32
> (PcdFspsUpdDataAddress);
> +    FspsUpdDataPtr = (FSPS_UPD_COMMON *) GetFspsUpdDataAddress();
>      ASSERT (FspsUpdDataPtr != NULL);
>    }
> 
> diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> index aeeca58d6d..da0049a654 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
> @@ -6,7 +6,7 @@
>  # register TemporaryRamDonePpi to call TempRamExit API, and register
> MemoryDiscoveredPpi  # notify to call FspSiliconInit API.
>  #
> -#  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2014 - 2021, Intel Corporation. All rights
> +reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -68,6 +68,7 @@
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ##
> CONSUMES
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64  ##
> CONSUMES
> 
>  [Guids]
>    gFspHobGuid                           ## CONSUMES ## HOB
> diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> index b8dac1b574..a5a8b8a19e 100644
> --- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> +++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> @@ -121,3 +121,5 @@
>    #
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress|0x00000000|UIN
> T32|0x50000000
> 
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress|0x00000000|UINT
> 32|0x50000001
> +
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress64|0x00000000|U
> IN
> + T64|0x50000002
> +
> +
> gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress64|0x00000000|UI
> N
> + T64|0x50000003
> --
> 2.30.2.windows.1


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

* Re: [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type
  2021-12-16  8:10 [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type Ashraf Ali S
                   ` (2 preceding siblings ...)
  2021-12-20  3:45 ` Chiu, Chasel
@ 2021-12-20  5:15 ` Ni, Ray
  2021-12-21  0:41   ` Chiu, Chasel
  3 siblings, 1 reply; 8+ messages in thread
From: Ni, Ray @ 2021-12-20  5:15 UTC (permalink / raw)
  To: S, Ashraf Ali, devel@edk2.groups.io
  Cc: Chiu, Chasel, Desimone, Nathaniel L, Zeng, Star, Kuo, Ted,
	Duggapu, Chinni B, Chaganty, Rangasai V, Solanki, Digant H,
	V, Sangeetha


+UINTN
+EFIAPI
+GetFspmUpdDataAddress (
+  VOID
+  )

This is internal function. Please remove "EFIAPI".

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

* Re: [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type
  2021-12-20  5:15 ` Ni, Ray
@ 2021-12-21  0:41   ` Chiu, Chasel
  0 siblings, 0 replies; 8+ messages in thread
From: Chiu, Chasel @ 2021-12-21  0:41 UTC (permalink / raw)
  To: Ni, Ray, S, Ashraf Ali, devel@edk2.groups.io
  Cc: Desimone, Nathaniel L, Zeng, Star, Kuo, Ted, Duggapu, Chinni B,
	Chaganty, Rangasai V, Solanki, Digant H, V, Sangeetha


Thanks Ray! I have sent a code review for removing EFIAPI, please help to review.

Thanks,
Chasel


> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Monday, December 20, 2021 1:15 PM
> To: S, Ashraf Ali <ashraf.ali.s@intel.com>; devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; Kuo, Ted
> <ted.kuo@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com>;
> Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Solanki, Digant H
> <digant.h.solanki@intel.com>; V, Sangeetha <sangeetha.v@intel.com>
> Subject: RE: [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based
> on Build Type
> 
> 
> +UINTN
> +EFIAPI
> +GetFspmUpdDataAddress (
> +  VOID
> +  )
> 
> This is internal function. Please remove "EFIAPI".

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

end of thread, other threads:[~2021-12-21  0:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-16  8:10 [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type Ashraf Ali S
2021-12-16 10:46 ` ted.kuo
2021-12-17  1:53   ` Chiu, Chasel
2021-12-17  3:47 ` Chiu, Chasel
2021-12-17  4:02   ` Zeng, Star
2021-12-20  3:45 ` Chiu, Chasel
2021-12-20  5:15 ` Ni, Ray
2021-12-21  0:41   ` Chiu, Chasel

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