Topics

[PATCH] MinPlatformPkg: Add PcdFlashMicrocodeOffset


Ni, Ray
 

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/SecFspWrapper=
PlatformSecLib/SecFspWrapperPlatformSecLib.inf b/Platform/Intel/MinPlatform=
Pkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSec=
Lib.inf
index 4f3fa9fa34..68ce5d81cd 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor=
mSecLib/SecFspWrapperPlatformSecLib.inf
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor=
mSecLib/SecFspWrapperPlatformSecLib.inf
@@ -1,7 +1,7 @@
## @file=0D
# Provide FSP wrapper platform sec related function.=0D
#=0D
-# Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>=
=0D
+# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>=
=0D
#=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
@@ -88,9 +88,9 @@
gMinPlatformPkgTokenSpaceGuid.PcdSecSerialPortDebugEnable ## C=
ONSUMES=0D
=0D
[FixedPcd]=0D
- gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress ## C=
ONSUMES=0D
- gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize ## C=
ONSUMES=0D
- gIntelFsp2WrapperTokenSpaceGuid.PcdFlashMicrocodeOffset ## C=
ONSUMES=0D
+ gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase ## C=
ONSUMES=0D
+ gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize ## C=
ONSUMES=0D
+ gMinPlatformPkgTokenSpaceGuid.PcdFlashMicrocodeOffset ## C=
ONSUMES=0D
gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheAddress ## C=
ONSUMES=0D
gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheSize ## C=
ONSUMES=0D
gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress ## C=
ONSUMES=0D
diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapper=
PlatformSecLib/SecRamInitData.c b/Platform/Intel/MinPlatformPkg/FspWrapper/=
Library/SecFspWrapperPlatformSecLib/SecRamInitData.c
index b356327b4c..b4e10cca1f 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor=
mSecLib/SecRamInitData.c
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor=
mSecLib/SecRamInitData.c
@@ -1,7 +1,7 @@
/** @file=0D
Provide TempRamInitParams data.=0D
=0D
-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>=0D
+Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>=0D
SPDX-License-Identifier: BSD-2-Clause-Patent=0D
=0D
**/=0D
@@ -24,8 +24,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED CONST FSPT_UPD_CORE_DATA Fs=
ptUpdDataPtr =3D {
}=0D
},=0D
{=0D
- ((UINT32)FixedPcdGet64 (PcdCpuMicrocodePatchAddress) + FixedPcdGet32 (=
PcdFlashMicrocodeOffset)),=0D
- ((UINT32)FixedPcdGet64 (PcdCpuMicrocodePatchRegionSize) - FixedPcdGet3=
2 (PcdFlashMicrocodeOffset)),=0D
+ FixedPcdGet32 (PcdFlashFvMicrocodeBase) + FixedPcdGet32 (PcdFlashMicro=
codeOffset),=0D
+ FixedPcdGet32 (PcdFlashFvMicrocodeSize) - FixedPcdGet32 (PcdFlashMicro=
codeOffset),=0D
0, // Set CodeRegionBase as 0, so that caching will be 4GB-(C=
odeRegionSize > LLCSize ? LLCSize : CodeRegionSize) will be used.=0D
FixedPcdGet32 (PcdFlashCodeCacheSize),=0D
{ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,=0D
diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec b/Platform/In=
tel/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=0D
# for the build infrastructure.=0D
#=0D
-# Copyright (c) 2017 - 2020, Intel Corporation. All rights reserved.<BR>=0D
+# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>=0D
#=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
@@ -165,6 +165,7 @@
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase|0xFFE60000|UINT32|=
0x30000004=0D
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize|0x000A0000|UINT32|=
0x30000005=0D
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset|0x00660000|UINT3=
2|0x30000006=0D
+ gMinPlatformPkgTokenSpaceGuid.PcdFlashMicrocodeOffset|0x60|UINT32|0x3000=
0007=0D
=0D
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPreMemoryBase|0x00000000|UINT32|=
0x20000004=0D
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPreMemorySize|0x00000000|UINT32|=
0x20000005=0D
--=20
2.27.0.windows.1


Chiu, Chasel
 

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


Ni, Ray
 

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