[edk2-devel] [PATCH] MinPlatformPkg: Add PcdFlashMicrocodeOffset

Ni, Ray posted 1 patch 2 years, 11 months ago
Failed in applying to current master (apply log)
.../SecFspWrapperPlatformSecLib.inf                       | 8 ++++----
.../Library/SecFspWrapperPlatformSecLib/SecRamInitData.c  | 6 +++---
Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec          | 3 ++-
3 files changed, 9 insertions(+), 8 deletions(-)
[edk2-devel] [PATCH] MinPlatformPkg: Add PcdFlashMicrocodeOffset
Posted by Ni, Ray 2 years, 11 months ago
Add PcdFlashMicrocodeOffset in MinPlatformPkg.dec and update
SecFspWrapperPlatformSecLib library to use the microcode location
PCDs defined in MinPlatformPkg.

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
---
 .../SecFspWrapperPlatformSecLib.inf                       | 8 ++++----
 .../Library/SecFspWrapperPlatformSecLib/SecRamInitData.c  | 6 +++---
 Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec          | 3 ++-
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf
index 4f3fa9fa34..68ce5d81cd 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf
@@ -1,7 +1,7 @@
 ## @file
 #  Provide FSP wrapper platform sec related function.
 #
-#  Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -88,9 +88,9 @@
   gMinPlatformPkgTokenSpaceGuid.PcdSecSerialPortDebugEnable           ## CONSUMES
 
 [FixedPcd]
-  gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress         ## CONSUMES
-  gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize      ## CONSUMES
-  gIntelFsp2WrapperTokenSpaceGuid.PcdFlashMicrocodeOffset             ## CONSUMES
+  gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase               ## CONSUMES
+  gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize               ## CONSUMES
+  gMinPlatformPkgTokenSpaceGuid.PcdFlashMicrocodeOffset               ## CONSUMES
   gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheAddress            ## CONSUMES
   gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheSize               ## CONSUMES
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress                  ## CONSUMES
diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecRamInitData.c b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecRamInitData.c
index b356327b4c..b4e10cca1f 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecRamInitData.c
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecRamInitData.c
@@ -1,7 +1,7 @@
 /** @file
   Provide TempRamInitParams data.
 
-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -24,8 +24,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED CONST FSPT_UPD_CORE_DATA FsptUpdDataPtr = {
     }
   },
   {
-    ((UINT32)FixedPcdGet64 (PcdCpuMicrocodePatchAddress) + FixedPcdGet32 (PcdFlashMicrocodeOffset)),
-    ((UINT32)FixedPcdGet64 (PcdCpuMicrocodePatchRegionSize) - FixedPcdGet32 (PcdFlashMicrocodeOffset)),
+    FixedPcdGet32 (PcdFlashFvMicrocodeBase) + FixedPcdGet32 (PcdFlashMicrocodeOffset),
+    FixedPcdGet32 (PcdFlashFvMicrocodeSize) - FixedPcdGet32 (PcdFlashMicrocodeOffset),
     0,          // Set CodeRegionBase as 0, so that caching will be 4GB-(CodeRegionSize > LLCSize ? LLCSize : CodeRegionSize) will be used.
     FixedPcdGet32 (PcdFlashCodeCacheSize),
     { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
index 2b246cf0ac..1c9ae665a6 100644
--- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
+++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
@@ -6,7 +6,7 @@
 # INF files to generate AutoGen.c and AutoGen.h files
 # for the build infrastructure.
 #
-# Copyright (c) 2017 - 2020, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -165,6 +165,7 @@
   gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase|0xFFE60000|UINT32|0x30000004
   gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize|0x000A0000|UINT32|0x30000005
   gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset|0x00660000|UINT32|0x30000006
+  gMinPlatformPkgTokenSpaceGuid.PcdFlashMicrocodeOffset|0x60|UINT32|0x30000007
 
   gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPreMemoryBase|0x00000000|UINT32|0x20000004
   gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPreMemorySize|0x00000000|UINT32|0x20000005
-- 
2.27.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73825): https://edk2.groups.io/g/devel/message/73825
Mute This Topic: https://groups.io/mt/81936936/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] MinPlatformPkg: Add PcdFlashMicrocodeOffset
Posted by Chiu, Chasel 2 years, 11 months ago
Please see my comments below inline.

Thanks,
Chasel


> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Thursday, April 8, 2021 4:07 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
> Dong, Eric <eric.dong@intel.com>
> Subject: [PATCH] MinPlatformPkg: Add PcdFlashMicrocodeOffset
> 
> Add PcdFlashMicrocodeOffset in MinPlatformPkg.dec and update
> SecFspWrapperPlatformSecLib library to use the microcode location PCDs
> defined in MinPlatformPkg.
> 
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Eric Dong <eric.dong@intel.com>
> ---
>  .../SecFspWrapperPlatformSecLib.inf                       | 8 ++++----
>  .../Library/SecFspWrapperPlatformSecLib/SecRamInitData.c  | 6 +++---
>  Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec          | 3 ++-
>  3 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatform
> SecLib/SecFspWrapperPlatformSecLib.inf
> b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatform
> SecLib/SecFspWrapperPlatformSecLib.inf
> index 4f3fa9fa34..68ce5d81cd 100644
> ---
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatform
> SecLib/SecFspWrapperPlatformSecLib.inf
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
> +++ formSecLib/SecFspWrapperPlatformSecLib.inf
> @@ -1,7 +1,7 @@
>  ## @file #  Provide FSP wrapper platform sec related function. #-#  Copyright (c)
> 2017 - 2019, Intel Corporation. All rights reserved.<BR>+#  Copyright (c) 2017 -
> 2021, Intel Corporation. All rights reserved.<BR> # # SPDX-License-Identifier:
> BSD-2-Clause-Patent #@@ -88,9 +88,9 @@
>    gMinPlatformPkgTokenSpaceGuid.PcdSecSerialPortDebugEnable           ##
> CONSUMES  [FixedPcd]-
> gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress         ##
> CONSUMES-
> gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize      ##
> CONSUMES-  gIntelFsp2WrapperTokenSpaceGuid.PcdFlashMicrocodeOffset
> ## CONSUMES+  gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase
> ## CONSUMES+  gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize
> ## CONSUMES+  gMinPlatformPkgTokenSpaceGuid.PcdFlashMicrocodeOffset
> ## CONSUMES   gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheAddress
> ## CONSUMES   gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheSize
> ## CONSUMES   gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress
> ## CONSUMESdiff --git
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatform
> SecLib/SecRamInitData.c
> b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatform
> SecLib/SecRamInitData.c
> index b356327b4c..b4e10cca1f 100644
> ---
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatform
> SecLib/SecRamInitData.c
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
> +++ formSecLib/SecRamInitData.c
> @@ -1,7 +1,7 @@
>  /** @file   Provide TempRamInitParams data. -Copyright (c) 2017, Intel
> Corporation. All rights reserved.<BR>+Copyright (c) 2017 - 2021, Intel
> Corporation. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-
> Patent  **/@@ -24,8 +24,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED CONST
> FSPT_UPD_CORE_DATA FsptUpdDataPtr = {
>      }   },   {-    ((UINT32)FixedPcdGet64 (PcdCpuMicrocodePatchAddress) +
> FixedPcdGet32 (PcdFlashMicrocodeOffset)),-    ((UINT32)FixedPcdGet64
> (PcdCpuMicrocodePatchRegionSize) - FixedPcdGet32
> (PcdFlashMicrocodeOffset)),+    FixedPcdGet32 (PcdFlashFvMicrocodeBase) +
> FixedPcdGet32 (PcdFlashMicrocodeOffset),+    FixedPcdGet32
> (PcdFlashFvMicrocodeSize) - FixedPcdGet32 (PcdFlashMicrocodeOffset),     0,
> // Set CodeRegionBase as 0, so that caching will be 4GB-(CodeRegionSize >
> LLCSize ? LLCSize : CodeRegionSize) will be used.     FixedPcdGet32
> (PcdFlashCodeCacheSize),     { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> 0x00, 0x00,diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> index 2b246cf0ac..1c9ae665a6 100644
> --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> @@ -6,7 +6,7 @@
>  # INF files to generate AutoGen.c and AutoGen.h files # for the build
> infrastructure. #-# Copyright (c) 2017 - 2020, Intel Corporation. All rights
> reserved.<BR>+# Copyright (c) 2017 - 2021, Intel Corporation. All rights
> reserved.<BR> # # SPDX-License-Identifier: BSD-2-Clause-Patent #@@ -165,6
> +165,7 @@
> 
> gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase|0xFFE60000|UINT
> 32|0x30000004
> gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize|0x000A0000|UINT
> 32|0x30000005
> gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset|0x00660000|UI
> NT32|0x30000006+
> gMinPlatformPkgTokenSpaceGuid.PcdFlashMicrocodeOffset|0x60|UINT32|0x3
> 0000007

1. Maybe we can align with IntelFsp2WrapperPkg which have this PCD default value as 0x90 for typical cases?
2. Since we already have MicrocodeOffset PCD there, to prevent from confusing, how about renaming it to something like PcdFlashMicrocodeBypassBytes?
3. Please add comments to describe this particular PCD meaning, because it is different from others (flash map definition PCD)

> gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPreMemoryBase|0x00000000|UIN
> T32|0x20000004
> gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPreMemorySize|0x00000000|UIN
> T32|0x20000005--
> 2.27.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73826): https://edk2.groups.io/g/devel/message/73826
Mute This Topic: https://groups.io/mt/81936936/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] MinPlatformPkg: Add PcdFlashMicrocodeOffset
Posted by Ni, Ray 2 years, 11 months ago
> 1. Maybe we can align with IntelFsp2WrapperPkg which have this PCD
> default value as 0x90 for typical cases?
Sure. I will update to 0x90.

> 2. Since we already have MicrocodeOffset PCD there, to prevent from
> confusing, how about renaming it to something like
> PcdFlashMicrocodeBypassBytes?

BypassBytes might not be a clear name for indicating the PCD meaning.
How about "PcdMicrcocodeOffsetInFv"?

> 3. Please add comments to describe this particular PCD meaning, because it is
> different from others (flash map definition PCD)

I will.

> 
> >
> gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPreMemoryBase|0x00000000|
> UIN
> > T32|0x20000004
> >
> gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPreMemorySize|0x00000000|
> UIN
> > T32|0x20000005--
> > 2.27.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73828): https://edk2.groups.io/g/devel/message/73828
Mute This Topic: https://groups.io/mt/81936936/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-