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

[Chiu, Chasel]

Please see my comments below inline.

Thanks,
Chasel


> -----Original Message-----
> From: Ni, Ray <ray.ni at intel.com>
> Sent: Thursday, April 8, 2021 4:07 PM
> To: devel at edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu at intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone at intel.com>; Liming Gao <gaoliming at byosoft.com.cn>;
> Dong, Eric <eric.dong at 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 at intel.com>
> Cc: Chasel Chiu <chasel.chiu at intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone at intel.com>
> Cc: Liming Gao <gaoliming at byosoft.com.cn>
> Cc: Eric Dong <eric.dong at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-