Date   

Re: [PATCH v1 03/12] ArmPkg: Add missing library headers to ArmPkg.dec

PierreGondois
 

Hi Bret,
Ok I will do that in a V2.

Do these patches look ok to you ?

[PATCH v1 08/12] .pytool: Enable CI for ArmPkg
[PATCH v1 09/12] .pytool: Enable CI for ArmPlatformPkg
[PATCH v1 10/12] .pytool: Document LicenseCheck and EccCheck
[PATCH v1 11/12] AzurePipelines: Add support for ArmPkg
[PATCH v1 12/12] AzurePipelines: Add support for ArmPlatformPkg

Regards,
Pierre

On 4/21/21 8:13 PM, brbarkel via groups.io wrote:

1) To expedite the required reviews, you may want to add CC to the package maintainers for ArmPkg to this commit message and email. I know a lot of people filter based on direct mention vs mailing list.

2) Generally, other packages have a brief description of the lib in the DEC, as well. Example:
https://github.com/tianocore/edk2/blob/d3b0d007a135284981fa750612a47234b83976f9/MdeModulePkg/MdeModulePkg.dec#L55 <https://github.com/tianocore/edk2/blob/d3b0d007a135284981fa750612a47234b83976f9/MdeModulePkg/MdeModulePkg.dec#L55>
However, I see that this has not historically been maintained in this package, so I'm not going to make a big deal of it.

Reviewed-by: Bret Barkelew <bret.barkelew@microsoft.com>


Re: [PATCH v2] BaseTools: Add support for version 3 of FMP Image Header structure

Michael D Kinney
 

Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sughosh Ganu
Sent: Friday, April 23, 2021 4:29 AM
To: devel@edk2.groups.io
Cc: Michal Simek <michal.simek@xilinx.com>; Sughosh Ganu <sughosh.ganu@linaro.org>
Subject: [edk2-devel] [PATCH v2] BaseTools: Add support for version 3 of FMP Image Header structure

Add support for the ImageCapsuleSupport field, introduced in version 3
of the EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER structure. This
structure member is used to indicate if the corresponding payload has
support for authentication and dependency.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---

Changes since v1:
- Reword the patch header to get rid of the PatchCheck warning
- Make passing of ImageCapsuleSupport parameter to the AddPayload
function as an optional parameter to maintain backward compatibility
- Declare the values of CAPSULE_SUPPORT_DEPENDENCY and
CAPSULE_SUPPORT_AUTHENTICATION in the FmpCapsuleHeaderClass and use
those in the GenerateCapsule script

.../Source/Python/Capsule/GenerateCapsule.py | 5 +++-
.../Common/Uefi/Capsule/FmpCapsuleHeader.py | 28 +++++++++++++------
2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/BaseTools/Source/Python/Capsule/GenerateCapsule.py b/BaseTools/Source/Python/Capsule/GenerateCapsule.py
index a8de988253..b8039db878 100644
--- a/BaseTools/Source/Python/Capsule/GenerateCapsule.py
+++ b/BaseTools/Source/Python/Capsule/GenerateCapsule.py
@@ -561,6 +561,7 @@ if __name__ == '__main__':
print ('GenerateCapsule: error:' + str(Msg))
sys.exit (1)
for SinglePayloadDescriptor in PayloadDescriptorList:
+ ImageCapsuleSupport = 0x0000000000000000
Result = SinglePayloadDescriptor.Payload
try:
FmpPayloadHeader.FwVersion = SinglePayloadDescriptor.FwVersion
@@ -575,6 +576,7 @@ if __name__ == '__main__':
if SinglePayloadDescriptor.UseDependency:
CapsuleDependency.Payload = Result
CapsuleDependency.DepexExp = SinglePayloadDescriptor.DepexExp
+ ImageCapsuleSupport |= FmpCapsuleHeader.CAPSULE_SUPPORT_DEPENDENCY
Result = CapsuleDependency.Encode ()
if args.Verbose:
CapsuleDependency.DumpInfo ()
@@ -607,13 +609,14 @@ if __name__ == '__main__':
FmpAuthHeader.MonotonicCount = SinglePayloadDescriptor.MonotonicCount
FmpAuthHeader.CertData = CertData
FmpAuthHeader.Payload = Result
+ ImageCapsuleSupport |= FmpCapsuleHeader.CAPSULE_SUPPORT_AUTHENTICATION
Result = FmpAuthHeader.Encode ()
if args.Verbose:
FmpAuthHeader.DumpInfo ()
except:
print ('GenerateCapsule: error: can not encode FMP Auth Header')
sys.exit (1)
- FmpCapsuleHeader.AddPayload (SinglePayloadDescriptor.Guid, Result, HardwareInstance =
SinglePayloadDescriptor.HardwareInstance, UpdateImageIndex = SinglePayloadDescriptor.UpdateImageIndex)
+ FmpCapsuleHeader.AddPayload (SinglePayloadDescriptor.Guid, Result, HardwareInstance =
SinglePayloadDescriptor.HardwareInstance, UpdateImageIndex = SinglePayloadDescriptor.UpdateImageIndex, CapsuleSupport =
ImageCapsuleSupport)
try:
for EmbeddedDriver in EmbeddedDriverDescriptorList:
FmpCapsuleHeader.AddEmbeddedDriver(EmbeddedDriver)
diff --git a/BaseTools/Source/Python/Common/Uefi/Capsule/FmpCapsuleHeader.py
b/BaseTools/Source/Python/Common/Uefi/Capsule/FmpCapsuleHeader.py
index 91d24919c4..8abb449c6f 100644
--- a/BaseTools/Source/Python/Common/Uefi/Capsule/FmpCapsuleHeader.py
+++ b/BaseTools/Source/Python/Common/Uefi/Capsule/FmpCapsuleHeader.py
@@ -47,14 +47,19 @@ class FmpCapsuleImageHeaderClass (object):
# /// therefore can be modified without changing the Auth data.
# ///
# UINT64 UpdateHardwareInstance;
+ #
+ # ///
+ # /// Bits which indicate authentication and depex information for the image that follows this structure
+ # ///
+ # UINT64 ImageCapsuleSupport
# } EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER;
#
- # #define EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER_INIT_VERSION 0x00000002
+ # #define EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER_INIT_VERSION 0x00000003

- _StructFormat = '<I16sB3BIIQ'
+ _StructFormat = '<I16sB3BIIQQ'
_StructSize = struct.calcsize (_StructFormat)

- EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER_INIT_VERSION = 0x00000002
+ EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER_INIT_VERSION = 0x00000003

def __init__ (self):
self._Valid = False
@@ -64,6 +69,7 @@ class FmpCapsuleImageHeaderClass (object):
self.UpdateImageSize = 0
self.UpdateVendorCodeSize = 0
self.UpdateHardwareInstance = 0x0000000000000000
+ self.ImageCapsuleSupport = 0x0000000000000000
self.Payload = b''
self.VendorCodeBytes = b''

@@ -78,7 +84,8 @@ class FmpCapsuleImageHeaderClass (object):
0,0,0,
self.UpdateImageSize,
self.UpdateVendorCodeSize,
- self.UpdateHardwareInstance
+ self.UpdateHardwareInstance,
+ self.ImageCapsuleSupport
)
self._Valid = True
return FmpCapsuleImageHeader + self.Payload + self.VendorCodeBytes
@@ -86,7 +93,7 @@ class FmpCapsuleImageHeaderClass (object):
def Decode (self, Buffer):
if len (Buffer) < self._StructSize:
raise ValueError
- (Version, UpdateImageTypeId, UpdateImageIndex, r0, r1, r2, UpdateImageSize, UpdateVendorCodeSize,
UpdateHardwareInstance) = \
+ (Version, UpdateImageTypeId, UpdateImageIndex, r0, r1, r2, UpdateImageSize, UpdateVendorCodeSize,
UpdateHardwareInstance, ImageCapsuleSupport) = \
struct.unpack (
self._StructFormat,
Buffer[0:self._StructSize]
@@ -105,6 +112,7 @@ class FmpCapsuleImageHeaderClass (object):
self.UpdateImageSize = UpdateImageSize
self.UpdateVendorCodeSize = UpdateVendorCodeSize
self.UpdateHardwareInstance = UpdateHardwareInstance
+ self.ImageCapsuleSupport = ImageCapsuleSupport
self.Payload = Buffer[self._StructSize:self._StructSize + UpdateImageSize]
self.VendorCodeBytes = Buffer[self._StructSize + UpdateImageSize:]
self._Valid = True
@@ -119,6 +127,7 @@ class FmpCapsuleImageHeaderClass (object):
print ('EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER.UpdateImageSize = {UpdateImageSize:08X}'.format
(UpdateImageSize = self.UpdateImageSize))
print ('EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER.UpdateVendorCodeSize = {UpdateVendorCodeSize:08X}'.format
(UpdateVendorCodeSize = self.UpdateVendorCodeSize))
print ('EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER.UpdateHardwareInstance =
{UpdateHardwareInstance:016X}'.format (UpdateHardwareInstance = self.UpdateHardwareInstance))
+ print ('EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER.ImageCapsuleSupport = {ImageCapsuleSupport:016X}'.format
(ImageCapsuleSupport = self.ImageCapsuleSupport))
print ('sizeof (Payload) = {Size:08X}'.format (Size = len
(self.Payload)))
print ('sizeof (VendorCodeBytes) = {Size:08X}'.format (Size = len
(self.VendorCodeBytes)))

@@ -153,6 +162,8 @@ class FmpCapsuleHeaderClass (object):
_ItemOffsetSize = struct.calcsize (_ItemOffsetFormat)

EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER_INIT_VERSION = 0x00000001
+ CAPSULE_SUPPORT_AUTHENTICATION = 0x0000000000000001
+ CAPSULE_SUPPORT_DEPENDENCY = 0x0000000000000002

def __init__ (self):
self._Valid = False
@@ -172,8 +183,8 @@ class FmpCapsuleHeaderClass (object):
raise ValueError
return self._EmbeddedDriverList[Index]

- def AddPayload (self, UpdateImageTypeId, Payload = b'', VendorCodeBytes = b'', HardwareInstance = 0, UpdateImageIndex
= 1):
- self._PayloadList.append ((UpdateImageTypeId, Payload, VendorCodeBytes, HardwareInstance, UpdateImageIndex))
+ def AddPayload (self, UpdateImageTypeId, Payload = b'', VendorCodeBytes = b'', HardwareInstance = 0, UpdateImageIndex
= 1, CapsuleSupport = 0):
+ self._PayloadList.append ((UpdateImageTypeId, Payload, VendorCodeBytes, HardwareInstance, UpdateImageIndex,
CapsuleSupport))

def GetFmpCapsuleImageHeader (self, Index):
if Index >= len (self._FmpCapsuleImageHeaderList):
@@ -198,13 +209,14 @@ class FmpCapsuleHeaderClass (object):
self._ItemOffsetList.append (Offset)
Offset = Offset + len (EmbeddedDriver)
Index = 1
- for (UpdateImageTypeId, Payload, VendorCodeBytes, HardwareInstance, UpdateImageIndex) in self._PayloadList:
+ for (UpdateImageTypeId, Payload, VendorCodeBytes, HardwareInstance, UpdateImageIndex, CapsuleSupport) in
self._PayloadList:
FmpCapsuleImageHeader = FmpCapsuleImageHeaderClass ()
FmpCapsuleImageHeader.UpdateImageTypeId = UpdateImageTypeId
FmpCapsuleImageHeader.UpdateImageIndex = UpdateImageIndex
FmpCapsuleImageHeader.Payload = Payload
FmpCapsuleImageHeader.VendorCodeBytes = VendorCodeBytes
FmpCapsuleImageHeader.UpdateHardwareInstance = HardwareInstance
+ FmpCapsuleImageHeader.ImageCapsuleSupport = CapsuleSupport
FmpCapsuleImage = FmpCapsuleImageHeader.Encode ()
FmpCapsuleData = FmpCapsuleData + FmpCapsuleImage

--
2.17.1





Re: RFC: Adding support for ARM (RNDR etc.) to RngDxe

Rebecca Cran
 

Thanks. Yes, I'll work on implementing the RngLib|RNDR part.
I'll be using Qemu's sbsa-ref platform for testing. I'll also look into using the FVP_Base_AEMv8A-AEMv8A too.

--
Rebecca Cran

On 4/22/21 3:30 AM, Sami Mujawar wrote:
Hi Rebecca,
I have been working on the following modules (See slide 11 in “EDKII - Proposed update to RNG implementation.pdf <https://edk2.groups.io/g/devel/files/Designs/2021/0116/EDKII%20-%20Proposed%20update%20to%20RNG%20implementation.pdf>”):
1. TrngLib|FwTrnglib (Arm Firmware TRNG)
2. DrbgLib stack – with support for DrbgAlgorithmLib|CRT_DRBG &
AesLib|ArmAesInstructionLib.
I plan to post patches for (a) in the next fortnight. Following this I plan to update the proposal with the interface definitions for the various library interfaces in the DrbgLib Stack.
I have not looked at RngLib|RNDR as I believe you were interested in implementing the part. Kindly let me know if you plan to implement this and the platform you would be using for testing. It looks like the FVP_Base_AEMv8A-AEMv8A and the FVP-RevC models support RNDR, so these could be used for testing as well. Please feel free to get in touch should you need any help with the model parameters or if you face any issues.
Regards,
Sami Mujawar
*From: *Rebecca Cran <rebecca@nuviainc.com>
*Date: *Tuesday, 20 April 2021 at 21:04
*To: *Sami Mujawar <Sami.Mujawar@arm.com>, devel@edk2.groups.io <devel@edk2.groups.io>, Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>, Ard Biesheuvel <Ard.Biesheuvel@arm.com>, leif@nuviainc.com <leif@nuviainc.com>
*Cc: *rfc@edk2.groups.io <rfc@edk2.groups.io>, Jiewen Yao <jiewen.yao@intel.com>, Rahul Kumar <rahul1.kumar@intel.com>, nd <nd@arm.com>, Jose Marinho <Jose.Marinho@arm.com>
*Subject: *Re: [edk2-devel] RFC: Adding support for ARM (RNDR etc.) to RngDxe
Hi Sami,
I was wondering if you're still collecting feedback on the design, or if
you have a plan and schedule for the implementation?
--
Rebecca Cran
On 1/15/21 7:51 PM, Sami Mujawar wrote:
> Hi All,
>
> I have shared some initial thoughts on the RNG implementation updates
at https://edk2.groups.io/g/devel/files/Designs/2021/0116/EDKII%20-%20Proposed%20update%20to%20RNG%20implementation.pdf <https://edk2.groups.io/g/devel/files/Designs/2021/0116/EDKII%20-%20Proposed%20update%20to%20RNG%20implementation.pdf>
>
> Kindly let me know your feedback or if you have any queries.
>
> Regards,
>
> Sami Mujawar
>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
Rebecca Cran via groups.io
> Sent: 14 January 2021 09:05 PM
> To: Sami Mujawar <Sami.Mujawar@arm.com>; devel@edk2.groups.io; Samer
El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Ard Biesheuvel <Ard.Biesheuvel@arm.com>; leif@nuviainc.com
> Cc: rfc@edk2.groups.io; Jiewen Yao <jiewen.yao@intel.com>; Rahul
Kumar <rahul1.kumar@intel.com>; nd <nd@arm.com>
> Subject: Re: [edk2-devel] RFC: Adding support for ARM (RNDR etc.) to
RngDxe
>
> On 12/10/20 4:26 AM, Sami Mujawar wrote:
>
>> I am working on the TRNG FW API interface and will share more details
>> for the discussion soon.
>>
>> We had some thoughts about streamlining the RngDxe implementations and
>> would like to share some diagrams for the discussion.
>>
>> My diagrams are in Visio that I can export as JPG images. However, I am
>> open to switching to any other suggested tool.
>
> Hi Sami,
>
> I don't see any further discussions on this. Have you made any progress
> with sharing the design documents or scheduling a review?
>


Re: [PATCH 3/3] OvmfPkg/PlatformPei: Mark TPM MMIO range as unencrypted for SEV

Lendacky, Thomas
 

On 4/26/21 7:07 AM, Laszlo Ersek wrote:
On 04/23/21 22:02, Tom Lendacky wrote:
On 4/23/21 12:41 PM, Tom Lendacky wrote:
On 4/23/21 8:04 AM, Laszlo Ersek wrote:
On 04/23/21 12:26, Laszlo Ersek wrote:
review#2 from scratch:

On 04/21/21 00:54, Tom Lendacky wrote:
From: Tom Lendacky <thomas.lendacky@amd.com>

BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3345&;data=04%7C01%7Cthomas.lendacky%40amd.com%7Cc8a12e7c1e6a4282963508d908abe333%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637550356650588901%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=3SeLVc3SscAozGX62BSAyWseujfwxzm6qIB%2Fh8ALyq0%3D&amp;reserved=0

The TPM support in OVMF performs MMIO accesses during the PEI phase. At
this point, MMIO ranges have not been marked un-encyrpted, so an SEV-ES
guest will fail attempting to perform MMIO to an encrypted address.
(1) As discussed, please update the commit message, for more clarify
about SEV vs. SEV-ES.


Read the PcdTpmBaseAddress and mark the specification defined range
(0x5000 in length) as un-encrypted, to allow an SEV-ES guest to process
the MMIO requests.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Min Xu <min.m.xu@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
OvmfPkg/PlatformPei/PlatformPei.inf | 1 +
OvmfPkg/PlatformPei/AmdSev.c | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 6ef77ba7bb21..de60332e9390 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -113,6 +113,7 @@ [Pcd]

[FixedPcd]
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
+ gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType
diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index dddffdebda4b..d524929f9e10 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -141,6 +141,7 @@ AmdSevInitialize (
)
{
UINT64 EncryptionMask;
+ UINT64 TpmBaseAddress;
RETURN_STATUS PcdStatus;

//
@@ -206,6 +207,24 @@ AmdSevInitialize (
}
}

+ //
+ // PEI TPM support will perform MMIO accesses, be sure this range is not
+ // marked encrypted.
+ //
+ TpmBaseAddress = PcdGet64 (PcdTpmBaseAddress);
+ if (TpmBaseAddress != 0) {
It's OK to keep this as a sanity check, yes.

+ RETURN_STATUS DecryptStatus;
+
+ DecryptStatus = MemEncryptSevClearPageEncMask (
+ 0,
+ TpmBaseAddress,
+ EFI_SIZE_TO_PAGES (0x5000),
(2) Should be (UINTN)0x5000, as discussed earlier.

+ FALSE
+ );
+
+ ASSERT_RETURN_ERROR (DecryptStatus);
(3) So this is where the mess begins.

The idea is to delay the dispatch of Tcg2ConfigPei until after
PlatformPei determines if SEV is active, and (in case SEV is active)
PlatformPei decrypts the MMIO range of the TPM.

For this, we need to change the IA32 / X64 DEPEX of Tcg2ConfigPei, from
the current TRUE, to some PPI GUID.

There are two choices for that PPI:

(a) gEfiPeiMemoryDiscoveredPpiGuid

Advantages:

- no new PPI definition needed,

- no new PPI installation needed,

- OvmfPkg/Bhyve/PlatformPei needs no separate change

Disadvantages:

- total abuse of gEfiPeiMemoryDiscoveredPpiGuid


(b) gOvmfTpmMmioAccessiblePpiGuid

Disadvantages:

- this new GUID must be defined in "OvmfPkg.dec", in the [Ppis] section,
in a separate patch; its comment should say "this PPI signals that
accessing the MMIO range of the TPM is possible in the PEI phase,
regardless of memory encryption". The PPI definitions should be kept
alphabetically ordered.

- OvmfPkg/PlatformPei must install this new PPI, with a NULL interface.
(See "mPpiBootMode" as a technical example.) OvmfPkg/PlatformPei must
install this new PPI either when the SEV check at the top of
AmdSevInitialize() fails, or when MemEncryptSevClearPageEncMask() succeeds.

- OvmfPkg/Bhyve/PlatformPei must receive the same update, in a separate
patch. That's because "OvmfPkg/Bhyve/BhyveX64.fdf" includes the same
"Tcg2ConfigPei", but consumes "OvmfPkg/Bhyve/PlatformPei" rather than
"OvmfPkg/PlatformPei". Tcg2ConfigPei will receive the same
stricter-than-before depex, so something on the bhyve platform too must
produce the new PPI.

Advantages:

- more or less palatable as a concept, with the new PPI precisely
expressing the dependency we have.


In approach (b), the "OvmfPkg/Bhyve/PlatformPei" patch needs to be CC'd
to the Bhyve reviewers. If the Bhyve reviewers determine that such an
update is actually unnecessary, because on Bhyve, there is no TPM
support and/or no SEV support in fact, then *first* we have to create an
independent Bhyve cleanup series, that rips out the TPM and/or SEV
remnants from the OvmfPkg/Bhyve sub-tree.


I prefer approach (b). I'm sorry that it means extra work wrt. Bhyve,
but I strongly believe in keeping all platforms in the tree, and that
means we need to spend time on such changes.

I'm not CC'ing Rebecca and Peter on this message -- we're deep into this
patch review thread, and they'd have no useful context. I suggest simply
including the "OvmfPkg/Bhyve/PlatformPei" patch in the next version of
this series, with a proper explanation in the blurb (patch#0) and on the
"OvmfPkg/Bhyve/PlatformPei" patch. That should give them enough context
to evaluate whether the change is necessary, or whether we should purge
the TPM and/or the SEV bits from Bhyve. You could also ask them just
this question in advance, in a separate email on the list (with
distilled context). Personally I'm unsure if the TPM and SEV bits
survived into Bhyve because those bits are actually put to use there, or
because the initial platform creation / cloning wasn't as minimal as it
could have been.

Note that in case TPM makes sense on bhyve but SEV doesn't, then
"OvmfPkg/Bhyve/PlatformPei" will have to install the new PPI
unconditionally.
I've had a further idea on this.

You could add an entirely new PEIM just for this. The entry point
function of the PEIM would check for SEV, decrypt the TPM range if SEV
were active, and then install gOvmfTpmMmioAccessiblePpiGuid
(unconditionally). The exit status of the PEIM would always be
EFI_ABORTED, because there would be no need to keep the PEIM resident.

The new PEIM would have a DEPEX on gEfiPeiMemoryDiscoveredPpiGuid, to
make sure that potential page table splitting for the potential MMIO
range decryption could be satisfied from permanent PEI RAM.

The new PEIM would be included in the DSC and FDF files of the usual
three OVMF platforms, and in the Bhyve platform -- dependent on the
TPM_ENABLE build flag.

There are several advantages to such a separate PEIM:

- For Bhyve, the update is minimal. Just include one line in each of the
FDF and the DSC files. No need to customize an existent
platform-specific PEIM, no code duplication between two PlatformPei modules.

- The new PEIM would depend on the TPM_ENABLE build flag, so it would
only be included in the firmware binaries if and only if Tcg2ConfigPei
were. No useless PPI installation would occur in the absence of TPM_ENABLE.

- No need to check PcdTpmBaseAddress for nullity in the new PEIM, before
the decryption, as TPM_ENABLE guarantees (on IA32/X64) that the PCD
already has the right value.

- The new logic would be properly ordered between PlatformPei and
Tcg2ConfigPei, namely due to the use of two such PPI GUIDs in DEPEXes
that actually make sense. PlatformPei -> TPM MMIO decryptor PEIM ordered
via "memory discovered" (needed for potential page table splitting), TPM
MMIO decryptor PEIM -> Tcg2ConfigPei ordered via "TPM MMIO decrypted".

You could place the new PEIM at:

OvmfPkg/Tcg/TpmMmioSevDecryptPei

If you haven't lost your patience with me yet, I'd really appreciate if
you could investigate this!
So far, this appears to be working nicely. I'm new at the whole PEIM
thing, so hopefully I haven't missed anything. I should be submitting the
patches soon for review.
So one thing I failed to do before submitting my previous patch was to
complete my testing against the IA32 and X64 combination build. In this
build, PEI is built as Ia32, and MemEncryptSevClearPageEncMask() will
return UNSUPPORTED causing an ASSERT (since I check the return code). So
there are a few options:

1. SEV works with the current encrypted mapping, it is only the SEV-ES
support that fails because of the ValidateMmioMemory() check. I can do
the mapping change just for SEV-ES since it is X64 only. This works,
because MemEncryptSevClearPageEncMask() will not return UNSUPPORTED
when running in 64-bit.
Can we really say "SEV works" though? Because, even using an X64 PEI
phase, and enabling only SEV (not SEV-ES), TPM access will be broken in
the PEI phase. Is my understanding correct?
Because the memory range is marked as MMIO, we'll take a nested page fault
(NPF). The GPA passed as part of the NPF does not include the c-bit. So we
do in fact work properly with a TPM in SEV. SEV-ES would also work
properly if the mitigation for accessing an encrypted address was removed
from the #VC handler. It is only this added mitigation to protect MMIO
that results in an issue with the TPM in PEI.


I think the behavior you currently see is actually what we want, we
should double down on it -- if MemEncryptSevClearPageEncMask() fails,
report an explicit DEBUG_ERROR, and call CpuDeadLoop(). If the firmware
is built with TPM_ENABLE, and SEV is active, then an IA32 PEI phase is
simply unusable. Silently pretending that the TPM is not there, even
though it may have been configured on the QEMU command line, we just
failed to communicate with it, is not a good idea, IMO.
However, because the c-bit is not part of the NPF, we do communicate
successfully with the TPM.

So we could actually do following:
- For IA32:
- Remove the Depex on gOvmfTpmMmioAccessiblePpiGuid
- Do not add OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf

- For X64:
- Add the Depex on gOvmfTpmMmioAccessiblePpiGuid
- Add OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf

That might be confusing, though. So we could just do option #3 below.

Thanks,
Tom


This is somewhat similar IMO to the S3Verification() function in
"OvmfPkg/PlatformPei/Platform.c".

TPM_ENABLE, SEV, IA32 PEI phase: pick any two.

Thanks,
Laszlo


2. Call MemEncryptSevClearPageEncMask() for SEV or SEV-ES, but don't check
the return status.

3. Create Ia32 and X64 versions of internal functions, where the Ia32
version simply returns SUCCESS because it can't do anything and the X64
version calls MemEncryptSevClearPageEncMask(), allowing the main code
to ASSERT on any errors.

I'm leaning towards #1, because this is an SEV-ES only issue. Thoughts?

Thanks,
Tom


One thing I found is that the Bhyve package makes reference to the
OvmfPkg/Bhyve/Tcg directory, but that directory does not exist. So I don't
think that TPM enablement has been tested. I didn't update the Bhyve
support for that reason.

Thanks,
Tom

Thanks!
Laszlo


Re: [PATCH v1 1/4] ArmVirtPkg: Library: Memory initialization for Cloud Hypervisor

Jianyong Wu
 

Hi Laszlo,

-----Original Message-----
From: Laszlo Ersek <lersek@redhat.com>
Sent: Friday, April 23, 2021 8:00 PM
To: Jianyong Wu <Jianyong.Wu@arm.com>; edk2-devel-groups-io
<devel@edk2.groups.io>; Sami Mujawar <Sami.Mujawar@arm.com>
Cc: Justin He <Justin.He@arm.com>; Ard Biesheuvel
<ardb+tianocore@kernel.org>; Leif Lindholm <leif@nuviainc.com>
Subject: Re: [edk2-devel] [PATCH v1 1/4] ArmVirtPkg: Library: Memory
initialization for Cloud Hypervisor

Hi Jianyong,

On 04/22/21 15:56, Laszlo Ersek wrote:

(2) "Clh" is a catastrophically bad abbreviation. The whole point of
your work is to add Cloud Hypervisor support, so why trash the most
relevant information in the file names with an inane abbreviation?

(Not to mention that the name "Cloud Hypervisor" itself is as
nondescript as possible. :/)
In an attempt to approach this constructively, I've given it more thought.
Does "CloudHv" sound acceptable to the community? I've seen "hv" stand
for "hypervisor" frequently.

Yeah, CloudHv is better, as the original name is too long. I will take it as the abbreviation of Cloud Hypervisor.

I have another high-level note. I could delay it until after you post v2, but I
figure I could save you some time by sharing my observation with you right
now.

I think that the ACPI platform stuff, in patch#2, does not belong in
OvmfPkg/AcpiPlatformDxe. What's more, I don't think it belongs in OvmfPkg,
even.

The CloudHvAcpiPlatformDxe and CloudHvPlatformHasAcpiDtDxe drivers
should exist as stand-alone, self-contained drivers; they should be as minimal
as possible. This is already a given for "CloudHvPlatformHasAcpiDtDxe", but it
should also be possible for "CloudHvAcpiPlatformDxe".
OvmfPkg/AcpiPlatformDxe is a complex driver, and the overlap between
what OvmfPkg/AcpiPlatformDxe currently does, and what
CloudHvAcpiPlatformDxe actually *needs*, is virtually nil.

And so, the series shouldn't touch OvmfPkg at all.

Ultimately I suggest following the Xen pattern that can be seen under
ArmVirtPkg already. In detail, the following files and directories should
contain the new platform:

ArmVirtPkg/ArmVirtCloudHv.dsc
ArmVirtPkg/ArmVirtCloudHv.fdf
ArmVirtPkg/CloudHvAcpiPlatformDxe/
ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/
ArmVirtPkg/Library/CloudHvVirtMemInfoLib/
Ok , it seems more coherent. I will reorganize the files according to Acpi.

(And I don't really see the point of an FDF include file.)
Yeah, I can include them into the fdf file directly.

Thanks
Jianyong


Thanks!
Laszlo
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH v1 1/4] ArmVirtPkg: Library: Memory initialization for Cloud Hypervisor

Jianyong Wu
 

Hi Laszlo,

Thanks for your time of commenting on my "horrible" patch set. It's very helpful. I will refactor the patch set base on your comments and resend it later more carefully.

Thanks
Jianyong

-----Original Message-----
From: Laszlo Ersek <lersek@redhat.com>
Sent: Thursday, April 22, 2021 9:57 PM
To: Jianyong Wu <Jianyong.Wu@arm.com>; edk2-devel-groups-io
<devel@edk2.groups.io>
Cc: Justin He <Justin.He@arm.com>; Ard Biesheuvel
<ardb+tianocore@kernel.org>; Leif Lindholm <leif@nuviainc.com>; Sami
Mujawar <Sami.Mujawar@arm.com>
Subject: Re: [PATCH v1 1/4] ArmVirtPkg: Library: Memory initialization for
Cloud Hypervisor

Hi Jianyong,

On 04/22/21 10:24, Jianyong Wu wrote:
Cloud Hypervisor is kvm based VMM implemented in rust.

This library populates the system memory map for the Cloud Hypervisor
virtual platform.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <leif@nuviainc.com>
Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
---
ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoPeiLib.inf |
47 +++++++++
ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoLib.c | 94
++++++++++++++++++

ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoPeiLibConstructor.c
| 100 ++++++++++++++++++++
3 files changed, 241 insertions(+)
let's sort out the meta-problems first:

(1) you need a Feature Request BZ for this; <https://bugzilla.tianocore.org/>.
The commit messages should reference the specific bugzilla ticket URL.

(2) "Clh" is a catastrophically bad abbreviation. The whole point of your work
is to add Cloud Hypervisor support, so why trash the most relevant
information in the file names with an inane abbreviation?

(Not to mention that the name "Cloud Hypervisor" itself is as nondescript as
possible. :/)

(3) I have not received a cover letter (0/4). Not sure if you sent one.

(4) I don't see the messages in my edk2-devel folder, or in the mailing list
archives, or in the messages held for moderation at the groups.io WebUI.

(5) "Cloud Hypervisor" is not something that I can justifiably spend much time
on. I'm willing to review this series at the level at which I've reviewed (for
example) XenPVH or Bhyve in the past, mainly focusing on style and
potential regressions. However, that's not enough for the long term:
someone from ARM (or elsewhere) will have to step up for permanent
reviewership. Please add a patch for extending "Maintainers.txt"
appropriately. Example subsystems:

- ArmVirtPkg: modules used on Xen
- ArmVirtPkg: Kvmtool emulated platform support
- OvmfPkg: bhyve-related modules
- OvmfPkg: Xen-related modules

Please keep the subsystem titles alphabetically sorted in the file.

Please resend.

(I'm posting these comments at once because they are understandable to
the community even in the absence of your patches on the list.)

Thanks
Laszlo


diff --git
a/ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoPeiLib.inf
b/ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoPeiLib.inf
new file mode 100644
index 000000000000..04cb1f2a581a
--- /dev/null
+++ b/ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoPeiLib.inf
@@ -0,0 +1,47 @@
+#/* @file
+#
+# Copyright (c) 2011-2015, ARM Limited. All rights reserved.
+# Copyright (c) 2014-2017, Linaro Limited. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent # #*/
+
+[Defines]
+ INF_VERSION = 0x0001001A
+ BASE_NAME = ClhVirtMemInfoPeiLib
+ FILE_GUID = 3E29D940-0591-EE6A-CAD4-223A9CF55E75
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = ArmVirtMemInfoLib|PEIM
+ CONSTRUCTOR = ClhVirtMemInfoPeiLibConstructor
+
+[Sources]
+ ClhVirtMemInfoLib.c
+ ClhVirtMemInfoPeiLibConstructor.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ ArmVirtPkg/ArmVirtPkg.dec
+ EmbeddedPkg/EmbeddedPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ ArmLib
+ BaseMemoryLib
+ DebugLib
+ FdtLib
+ PcdLib
+ MemoryAllocationLib
+
+[Pcd]
+ gArmTokenSpaceGuid.PcdFdBaseAddress
+ gArmTokenSpaceGuid.PcdFvBaseAddress
+ gArmTokenSpaceGuid.PcdSystemMemoryBase
+ gArmTokenSpaceGuid.PcdSystemMemorySize
+
+[FixedPcd]
+ gArmTokenSpaceGuid.PcdFdSize
+ gArmTokenSpaceGuid.PcdFvSize
+ gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
diff --git a/ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoLib.c
b/ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoLib.c
new file mode 100644
index 000000000000..829d7d7aa259
--- /dev/null
+++ b/ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoLib.c
@@ -0,0 +1,94 @@
+/** @file
+
+ Copyright (c) 2014-2017, Linaro Limited. All rights reserved.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Base.h>
+#include <Library/ArmLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+
+// Number of Virtual Memory Map Descriptors
+#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 5
+
+//
+// mach-virt's core peripherals such as the UART, the GIC and the RTC
+are // all mapped in the 'miscellaneous device I/O' region, which we
+just map // in its entirety rather than device by device. Note that
+it does not // cover any of the NOR flash banks or PCI resource windows.
+//
+#define MACH_VIRT_PERIPH_BASE 0x08000000
+#define MACH_VIRT_PERIPH_SIZE SIZE_128MB
+
+//
+// in cloud-hypervisor, 0x0 ~ 0x8000000 is reserved as normal memory
+for UEFI //
+#define CLH_UEFI_MEM_BASE 0x0
+#define CLH_UEFI_MEM_SIZE 0x08000000
+
+/**
+ Return the Virtual Memory Map of your platform
+
+ This Virtual Memory Map is used by MemoryInitPei Module to
+ initialize the MMU on your platform.
+
+ @param[out] VirtualMemoryMap Array of
ARM_MEMORY_REGION_DESCRIPTOR
+ describing a Physical-to-Virtual Memory
+ mapping. This array must be ended by a
+ zero-filled entry. The allocated memory
+ will not be freed.
+
+**/
+VOID
+ArmVirtGetMemoryMap (
+ OUT ARM_MEMORY_REGION_DESCRIPTOR **VirtualMemoryMap
+ )
+{
+ ARM_MEMORY_REGION_DESCRIPTOR *VirtualMemoryTable;
+
+ ASSERT (VirtualMemoryMap != NULL);
+
+ VirtualMemoryTable = AllocatePool (sizeof
(ARM_MEMORY_REGION_DESCRIPTOR) *
+
+ MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS);
+
+ if (VirtualMemoryTable == NULL) {
+ DEBUG ((DEBUG_ERROR, "%a: Error: Failed AllocatePool()\n",
__FUNCTION__));
+ return;
+ }
+
+ // System DRAM
+ VirtualMemoryTable[0].PhysicalBase = PcdGet64
+ (PcdSystemMemoryBase); VirtualMemoryTable[0].VirtualBase =
VirtualMemoryTable[0].PhysicalBase;
+ VirtualMemoryTable[0].Length = PcdGet64 (PcdSystemMemorySize);
+ VirtualMemoryTable[0].Attributes =
ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
+
+ DEBUG ((DEBUG_INFO, "%a: Dumping System DRAM Memory Map:\n"
+ "\tPhysicalBase: 0x%lX\n"
+ "\tVirtualBase: 0x%lX\n"
+ "\tLength: 0x%lX\n",
+ __FUNCTION__,
+ VirtualMemoryTable[0].PhysicalBase,
+ VirtualMemoryTable[0].VirtualBase,
+ VirtualMemoryTable[0].Length));
+
+ // Memory mapped peripherals (UART, RTC, GIC, virtio-mmio, etc)
+ VirtualMemoryTable[1].PhysicalBase = MACH_VIRT_PERIPH_BASE;
+ VirtualMemoryTable[1].VirtualBase = MACH_VIRT_PERIPH_BASE;
+ VirtualMemoryTable[1].Length = MACH_VIRT_PERIPH_SIZE;
+ VirtualMemoryTable[1].Attributes =
ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+
+ // Map the FV region as normal executable memory
+ VirtualMemoryTable[2].PhysicalBase = PcdGet64 (PcdFvBaseAddress);
+ VirtualMemoryTable[2].VirtualBase =
VirtualMemoryTable[2].PhysicalBase;
+ VirtualMemoryTable[2].Length = FixedPcdGet32 (PcdFvSize);
+ VirtualMemoryTable[2].Attributes =
ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
+
+ // End of Table
+ ZeroMem (&VirtualMemoryTable[3], sizeof
+ (ARM_MEMORY_REGION_DESCRIPTOR));
+
+ *VirtualMemoryMap = VirtualMemoryTable; }
diff --git
a/ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoPeiLibConstructor
.c
b/ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoPeiLibConstructor
.c
new file mode 100644
index 000000000000..5f89b70df990
--- /dev/null
+++
b/ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoPeiLibConstru
+++ ctor.c
@@ -0,0 +1,100 @@
+/** @file
+
+ Copyright (c) 2014-2017, Linaro Limited. All rights reserved.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Base.h>
+#include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
+#include <libfdt.h>
+
+RETURN_STATUS
+EFIAPI
+ClhVirtMemInfoPeiLibConstructor (
+ VOID
+ )
+{
+ VOID *DeviceTreeBase;
+ INT32 Node, Prev;
+ UINT64 NewBase, CurBase;
+ UINT64 NewSize, CurSize;
+ CONST CHAR8 *Type;
+ INT32 Len;
+ CONST UINT64 *RegProp;
+ RETURN_STATUS PcdStatus;
+
+ NewBase = 0;
+ NewSize = 0;
+
+ DeviceTreeBase = (VOID *)(UINTN)PcdGet64
+ (PcdDeviceTreeInitialBaseAddress);
+ ASSERT (DeviceTreeBase != NULL);
+
+ //
+ // Make sure we have a valid device tree blob // ASSERT
+ (fdt_check_header (DeviceTreeBase) == 0);
+
+ //
+ // Look for the lowest memory node
+ //
+ for (Prev = 0;; Prev = Node) {
+ Node = fdt_next_node (DeviceTreeBase, Prev, NULL);
+ if (Node < 0) {
+ break;
+ }
+
+ //
+ // Check for memory node
+ //
+ Type = fdt_getprop (DeviceTreeBase, Node, "device_type", &Len);
+ if (Type && AsciiStrnCmp (Type, "memory", Len) == 0) {
+ //
+ // Get the 'reg' property of this node. For now, we will assume
+ // two 8 byte quantities for base and size, respectively.
+ //
+ RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
+ if (RegProp != 0 && Len == (2 * sizeof (UINT64))) {
+
+ CurBase = fdt64_to_cpu (ReadUnaligned64 (RegProp));
+ CurSize = fdt64_to_cpu (ReadUnaligned64 (RegProp + 1));
+
+ DEBUG ((DEBUG_INFO, "%a: System RAM @ 0x%lx - 0x%lx\n",
+ __FUNCTION__, CurBase, CurBase + CurSize - 1));
+
+ if (NewBase > CurBase || NewBase == 0) {
+ NewBase = CurBase;
+ NewSize = CurSize;
+ }
+ } else {
+ DEBUG ((DEBUG_ERROR, "%a: Failed to parse FDT memory node\n",
+ __FUNCTION__));
+ }
+ }
+ }
+
+ //
+ // Make sure the start of DRAM matches our expectation // ASSERT
+ (FixedPcdGet64 (PcdSystemMemoryBase) == NewBase); PcdStatus =
+ PcdSet64S (PcdSystemMemorySize, NewSize); ASSERT_RETURN_ERROR
+ (PcdStatus);
+
+ //
+ // We need to make sure that the machine we are running on has at
+ least // 128 MB of memory configured, and is currently executing
+ this binary from // NOR flash. This prevents a device tree image in
+ DRAM from getting // clobbered when our caller installs permanent
+ PEI RAM, before we have a // chance of marking its location as
+ reserved or copy it to a freshly // allocated block in the permanent PEI
RAM in the platform PEIM.
+ //
+ ASSERT (NewSize >= SIZE_128MB);
+ ASSERT (
+ (((UINT64)PcdGet64 (PcdFdBaseAddress) +
+ (UINT64)PcdGet32 (PcdFdSize)) <= NewBase) ||
+ ((UINT64)PcdGet64 (PcdFdBaseAddress) >= (NewBase + NewSize)));
+
+ return RETURN_SUCCESS;
+}
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH v1 1/4] ArmVirtPkg: Library: Memory initialization for Cloud Hypervisor

Jianyong Wu
 

Thanks Sami, I have registered the edk2 mail list and now I can receive the mails from it. Also, I thinks I am the member of edk2 group now.

 

From: Sami Mujawar <Sami.Mujawar@...>
Sent: Thursday, April 22, 2021 10:13 PM
To: Laszlo Ersek <lersek@...>; Jianyong Wu <Jianyong.Wu@...>; edk2-devel-groups-io <devel@edk2.groups.io>; edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Justin He <Justin.He@...>; Ard Biesheuvel <ardb+tianocore@...>; Leif Lindholm <leif@...>; nd <nd@...>
Subject: Re: [PATCH v1 1/4] ArmVirtPkg: Library: Memory initialization for Cloud Hypervisor

 

Hi Jianyong,

 

You need to check if you're subscribed to the EDK II development mailing list. Otherwise, your patch email will get rejected. You can subscribe here: https://edk2.groups.io/g/devel.

Make sure that you reply to the email with subscription confirmation sent from noreply@groups.io. Unless you do it, you won't become a member of the mailing list.

I would also recommend that you wait for a day after confirming, as I believe the edk2.groups.io admin will need to approve your membership.

 

Regards,

 

Sami Mujawar

 

From: Laszlo Ersek <lersek@...>
Date: Thursday, 22 April 2021 at 14:57
To: Jianyong Wu <Jianyong.Wu@...>, edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Justin He <Justin.He@...>, Ard Biesheuvel <ardb+tianocore@...>, Leif Lindholm <leif@...>, Sami Mujawar <Sami.Mujawar@...>
Subject: Re: [PATCH v1 1/4] ArmVirtPkg: Library: Memory initialization for Cloud Hypervisor

Hi Jianyong,

On 04/22/21 10:24, Jianyong Wu wrote:
> Cloud Hypervisor is kvm based VMM implemented in rust.
>
> This library populates the system memory map for the
> Cloud Hypervisor virtual platform.
>
> Cc: Laszlo Ersek <lersek@...>
> Cc: Ard Biesheuvel <ardb+tianocore@...>
> Cc: Leif Lindholm <leif@...>
> Signed-off-by: Jianyong Wu <jianyong.wu@...>
> ---
>  ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoPeiLib.inf          |  47 +++++++++
>  ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoLib.c               |  94 ++++++++++++++++++
>  ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoPeiLibConstructor.c | 100 ++++++++++++++++++++
>  3 files changed, 241 insertions(+)

let's sort out the meta-problems first:

(1) you need a Feature Request BZ for this;
<https://bugzilla.tianocore.org/>. The commit messages should reference
the specific bugzilla ticket URL.

(2) "Clh" is a catastrophically bad abbreviation. The whole point of
your work is to add Cloud Hypervisor support, so why trash the most
relevant information in the file names with an inane abbreviation?

(Not to mention that the name "Cloud Hypervisor" itself is as
nondescript as possible. :/)

(3) I have not received a cover letter (0/4). Not sure if you sent one.

(4) I don't see the messages in my edk2-devel folder, or in the mailing
list archives, or in the messages held for moderation at the groups.io
WebUI.

(5) "Cloud Hypervisor" is not something that I can justifiably spend
much time on. I'm willing to review this series at the level at which
I've reviewed (for example) XenPVH or Bhyve in the past, mainly focusing
on style and potential regressions. However, that's not enough for the
long term: someone from ARM (or elsewhere) will have to step up for
permanent reviewership. Please add a patch for extending
"Maintainers.txt" appropriately. Example subsystems:

- ArmVirtPkg: modules used on Xen
- ArmVirtPkg: Kvmtool emulated platform support
- OvmfPkg: bhyve-related modules
- OvmfPkg: Xen-related modules

Please keep the subsystem titles alphabetically sorted in the file.

Please resend.

(I'm posting these comments at once because they are understandable to
the community even in the absence of your patches on the list.)

Thanks
Laszlo

>
> diff --git a/ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoPeiLib.inf b/ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoPeiLib.inf
> new file mode 100644
> index 000000000000..04cb1f2a581a
> --- /dev/null
> +++ b/ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoPeiLib.inf
> @@ -0,0 +1,47 @@
> +#/* @file
> +#
> +#  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
> +#  Copyright (c) 2014-2017, Linaro Limited. All rights reserved.
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#*/
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001A
> +  BASE_NAME                      = ClhVirtMemInfoPeiLib
> +  FILE_GUID                      = 3E29D940-0591-EE6A-CAD4-223A9CF55E75
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = ArmVirtMemInfoLib|PEIM
> +  CONSTRUCTOR                    = ClhVirtMemInfoPeiLibConstructor
> +
> +[Sources]
> +  ClhVirtMemInfoLib.c
> +  ClhVirtMemInfoPeiLibConstructor.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  ArmVirtPkg/ArmVirtPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  ArmLib
> +  BaseMemoryLib
> +  DebugLib
> +  FdtLib
> +  PcdLib
> +  MemoryAllocationLib
> +
> +[Pcd]
> +  gArmTokenSpaceGuid.PcdFdBaseAddress
> +  gArmTokenSpaceGuid.PcdFvBaseAddress
> +  gArmTokenSpaceGuid.PcdSystemMemoryBase
> +  gArmTokenSpaceGuid.PcdSystemMemorySize
> +
> +[FixedPcd]
> +  gArmTokenSpaceGuid.PcdFdSize
> +  gArmTokenSpaceGuid.PcdFvSize
> +  gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
> diff --git a/ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoLib.c b/ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoLib.c
> new file mode 100644
> index 000000000000..829d7d7aa259
> --- /dev/null
> +++ b/ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoLib.c
> @@ -0,0 +1,94 @@
> +/** @file
> +
> +  Copyright (c) 2014-2017, Linaro Limited. All rights reserved.
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Base.h>
> +#include <Library/ArmLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +
> +// Number of Virtual Memory Map Descriptors
> +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          5
> +
> +//
> +// mach-virt's core peripherals such as the UART, the GIC and the RTC are
> +// all mapped in the 'miscellaneous device I/O' region, which we just map
> +// in its entirety rather than device by device. Note that it does not
> +// cover any of the NOR flash banks or PCI resource windows.
> +//
> +#define MACH_VIRT_PERIPH_BASE       0x08000000
> +#define MACH_VIRT_PERIPH_SIZE       SIZE_128MB
> +
> +//
> +// in cloud-hypervisor, 0x0 ~ 0x8000000 is reserved as normal memory for UEFI
> +//
> +#define CLH_UEFI_MEM_BASE       0x0
> +#define CLH_UEFI_MEM_SIZE       0x08000000
> +
> +/**
> +  Return the Virtual Memory Map of your platform
> +
> +  This Virtual Memory Map is used by MemoryInitPei Module to initialize the MMU
> +  on your platform.
> +
> +  @param[out]   VirtualMemoryMap    Array of ARM_MEMORY_REGION_DESCRIPTOR
> +                                    describing a Physical-to-Virtual Memory
> +                                    mapping. This array must be ended by a
> +                                    zero-filled entry. The allocated memory
> +                                    will not be freed.
> +
> +**/
> +VOID
> +ArmVirtGetMemoryMap (
> +  OUT ARM_MEMORY_REGION_DESCRIPTOR   **VirtualMemoryMap
> +  )
> +{
> +  ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
> +
> +  ASSERT (VirtualMemoryMap != NULL);
> +
> +  VirtualMemoryTable = AllocatePool (sizeof (ARM_MEMORY_REGION_DESCRIPTOR) *
> +                                     MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS);
> +
> +  if (VirtualMemoryTable == NULL) {
> +    DEBUG ((DEBUG_ERROR, "%a: Error: Failed AllocatePool()\n", __FUNCTION__));
> +    return;
> +  }
> +
> +  // System DRAM
> +  VirtualMemoryTable[0].PhysicalBase = PcdGet64 (PcdSystemMemoryBase);
> +  VirtualMemoryTable[0].VirtualBase  = VirtualMemoryTable[0].PhysicalBase;
> +  VirtualMemoryTable[0].Length       = PcdGet64 (PcdSystemMemorySize);
> +  VirtualMemoryTable[0].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> +
> +  DEBUG ((DEBUG_INFO, "%a: Dumping System DRAM Memory Map:\n"
> +      "\tPhysicalBase: 0x%lX\n"
> +      "\tVirtualBase: 0x%lX\n"
> +      "\tLength: 0x%lX\n",
> +      __FUNCTION__,
> +      VirtualMemoryTable[0].PhysicalBase,
> +      VirtualMemoryTable[0].VirtualBase,
> +      VirtualMemoryTable[0].Length));
> +
> +  // Memory mapped peripherals (UART, RTC, GIC, virtio-mmio, etc)
> +  VirtualMemoryTable[1].PhysicalBase = MACH_VIRT_PERIPH_BASE;
> +  VirtualMemoryTable[1].VirtualBase  = MACH_VIRT_PERIPH_BASE;
> +  VirtualMemoryTable[1].Length       = MACH_VIRT_PERIPH_SIZE;
> +  VirtualMemoryTable[1].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
> +  // Map the FV region as normal executable memory
> +  VirtualMemoryTable[2].PhysicalBase = PcdGet64 (PcdFvBaseAddress);
> +  VirtualMemoryTable[2].VirtualBase  = VirtualMemoryTable[2].PhysicalBase;
> +  VirtualMemoryTable[2].Length       = FixedPcdGet32 (PcdFvSize);
> +  VirtualMemoryTable[2].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> +
> +  // End of Table
> +  ZeroMem (&VirtualMemoryTable[3], sizeof (ARM_MEMORY_REGION_DESCRIPTOR));
> +
> +  *VirtualMemoryMap = VirtualMemoryTable;
> +}
> diff --git a/ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoPeiLibConstructor.c b/ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoPeiLibConstructor.c
> new file mode 100644
> index 000000000000..5f89b70df990
> --- /dev/null
> +++ b/ArmVirtPkg/Library/ClhVirtMemInfoLib/ClhVirtMemInfoPeiLibConstructor.c
> @@ -0,0 +1,100 @@
> +/** @file
> +
> +  Copyright (c) 2014-2017, Linaro Limited. All rights reserved.
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Base.h>
> +#include <Library/DebugLib.h>
> +#include <Library/PcdLib.h>
> +#include <libfdt.h>
> +
> +RETURN_STATUS
> +EFIAPI
> +ClhVirtMemInfoPeiLibConstructor (
> +  VOID
> +  )
> +{
> +  VOID          *DeviceTreeBase;
> +  INT32         Node, Prev;
> +  UINT64        NewBase, CurBase;
> +  UINT64        NewSize, CurSize;
> +  CONST CHAR8   *Type;
> +  INT32         Len;
> +  CONST UINT64  *RegProp;
> +  RETURN_STATUS PcdStatus;
> +
> +  NewBase = 0;
> +  NewSize = 0;
> +
> +  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
> +  ASSERT (DeviceTreeBase != NULL);
> +
> +  //
> +  // Make sure we have a valid device tree blob
> +  //
> +  ASSERT (fdt_check_header (DeviceTreeBase) == 0);
> +
> +  //
> +  // Look for the lowest memory node
> +  //
> +  for (Prev = 0;; Prev = Node) {
> +    Node = fdt_next_node (DeviceTreeBase, Prev, NULL);
> +    if (Node < 0) {
> +      break;
> +    }
> +
> +    //
> +    // Check for memory node
> +    //
> +    Type = fdt_getprop (DeviceTreeBase, Node, "device_type", &Len);
> +    if (Type && AsciiStrnCmp (Type, "memory", Len) == 0) {
> +      //
> +      // Get the 'reg' property of this node. For now, we will assume
> +      // two 8 byte quantities for base and size, respectively.
> +      //
> +      RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
> +      if (RegProp != 0 && Len == (2 * sizeof (UINT64))) {
> +
> +        CurBase = fdt64_to_cpu (ReadUnaligned64 (RegProp));
> +        CurSize = fdt64_to_cpu (ReadUnaligned64 (RegProp + 1));
> +
> +        DEBUG ((DEBUG_INFO, "%a: System RAM @ 0x%lx - 0x%lx\n",
> +          __FUNCTION__, CurBase, CurBase + CurSize - 1));
> +
> +        if (NewBase > CurBase || NewBase == 0) {
> +          NewBase = CurBase;
> +          NewSize = CurSize;
> +        }
> +      } else {
> +        DEBUG ((DEBUG_ERROR, "%a: Failed to parse FDT memory node\n",
> +          __FUNCTION__));
> +      }
> +    }
> +  }
> +
> +  //
> +  // Make sure the start of DRAM matches our expectation
> +  //
> +  ASSERT (FixedPcdGet64 (PcdSystemMemoryBase) == NewBase);
> +  PcdStatus = PcdSet64S (PcdSystemMemorySize, NewSize);
> +  ASSERT_RETURN_ERROR (PcdStatus);
> +
> +  //
> +  // We need to make sure that the machine we are running on has at least
> +  // 128 MB of memory configured, and is currently executing this binary from
> +  // NOR flash. This prevents a device tree image in DRAM from getting
> +  // clobbered when our caller installs permanent PEI RAM, before we have a
> +  // chance of marking its location as reserved or copy it to a freshly
> +  // allocated block in the permanent PEI RAM in the platform PEIM.
> +  //
> +  ASSERT (NewSize >= SIZE_128MB);
> +  ASSERT (
> +    (((UINT64)PcdGet64 (PcdFdBaseAddress) +
> +      (UINT64)PcdGet32 (PcdFdSize)) <= NewBase) ||
> +    ((UINT64)PcdGet64 (PcdFdBaseAddress) >= (NewBase + NewSize)));
> +
> +  return RETURN_SUCCESS;
> +}
>


Re: [PATCH 3/3] OvmfPkg/PlatformPei: Mark TPM MMIO range as unencrypted for SEV

Laszlo Ersek
 

On 04/23/21 22:02, Tom Lendacky wrote:
On 4/23/21 12:41 PM, Tom Lendacky wrote:
On 4/23/21 8:04 AM, Laszlo Ersek wrote:
On 04/23/21 12:26, Laszlo Ersek wrote:
review#2 from scratch:

On 04/21/21 00:54, Tom Lendacky wrote:
From: Tom Lendacky <thomas.lendacky@amd.com>

BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3345&;data=04%7C01%7Cthomas.lendacky%40amd.com%7Ca24cbb3f52404e466fe808d906585034%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637547798659640707%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=o2rAydWG7jenRhqbZx3oDffZhlCUimJL9f9Tpmy4etk%3D&amp;reserved=0

The TPM support in OVMF performs MMIO accesses during the PEI phase. At
this point, MMIO ranges have not been marked un-encyrpted, so an SEV-ES
guest will fail attempting to perform MMIO to an encrypted address.
(1) As discussed, please update the commit message, for more clarify
about SEV vs. SEV-ES.


Read the PcdTpmBaseAddress and mark the specification defined range
(0x5000 in length) as un-encrypted, to allow an SEV-ES guest to process
the MMIO requests.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Min Xu <min.m.xu@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
OvmfPkg/PlatformPei/PlatformPei.inf | 1 +
OvmfPkg/PlatformPei/AmdSev.c | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 6ef77ba7bb21..de60332e9390 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -113,6 +113,7 @@ [Pcd]

[FixedPcd]
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
+ gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType
diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index dddffdebda4b..d524929f9e10 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -141,6 +141,7 @@ AmdSevInitialize (
)
{
UINT64 EncryptionMask;
+ UINT64 TpmBaseAddress;
RETURN_STATUS PcdStatus;

//
@@ -206,6 +207,24 @@ AmdSevInitialize (
}
}

+ //
+ // PEI TPM support will perform MMIO accesses, be sure this range is not
+ // marked encrypted.
+ //
+ TpmBaseAddress = PcdGet64 (PcdTpmBaseAddress);
+ if (TpmBaseAddress != 0) {
It's OK to keep this as a sanity check, yes.

+ RETURN_STATUS DecryptStatus;
+
+ DecryptStatus = MemEncryptSevClearPageEncMask (
+ 0,
+ TpmBaseAddress,
+ EFI_SIZE_TO_PAGES (0x5000),
(2) Should be (UINTN)0x5000, as discussed earlier.

+ FALSE
+ );
+
+ ASSERT_RETURN_ERROR (DecryptStatus);
(3) So this is where the mess begins.

The idea is to delay the dispatch of Tcg2ConfigPei until after
PlatformPei determines if SEV is active, and (in case SEV is active)
PlatformPei decrypts the MMIO range of the TPM.

For this, we need to change the IA32 / X64 DEPEX of Tcg2ConfigPei, from
the current TRUE, to some PPI GUID.

There are two choices for that PPI:

(a) gEfiPeiMemoryDiscoveredPpiGuid

Advantages:

- no new PPI definition needed,

- no new PPI installation needed,

- OvmfPkg/Bhyve/PlatformPei needs no separate change

Disadvantages:

- total abuse of gEfiPeiMemoryDiscoveredPpiGuid


(b) gOvmfTpmMmioAccessiblePpiGuid

Disadvantages:

- this new GUID must be defined in "OvmfPkg.dec", in the [Ppis] section,
in a separate patch; its comment should say "this PPI signals that
accessing the MMIO range of the TPM is possible in the PEI phase,
regardless of memory encryption". The PPI definitions should be kept
alphabetically ordered.

- OvmfPkg/PlatformPei must install this new PPI, with a NULL interface.
(See "mPpiBootMode" as a technical example.) OvmfPkg/PlatformPei must
install this new PPI either when the SEV check at the top of
AmdSevInitialize() fails, or when MemEncryptSevClearPageEncMask() succeeds.

- OvmfPkg/Bhyve/PlatformPei must receive the same update, in a separate
patch. That's because "OvmfPkg/Bhyve/BhyveX64.fdf" includes the same
"Tcg2ConfigPei", but consumes "OvmfPkg/Bhyve/PlatformPei" rather than
"OvmfPkg/PlatformPei". Tcg2ConfigPei will receive the same
stricter-than-before depex, so something on the bhyve platform too must
produce the new PPI.

Advantages:

- more or less palatable as a concept, with the new PPI precisely
expressing the dependency we have.


In approach (b), the "OvmfPkg/Bhyve/PlatformPei" patch needs to be CC'd
to the Bhyve reviewers. If the Bhyve reviewers determine that such an
update is actually unnecessary, because on Bhyve, there is no TPM
support and/or no SEV support in fact, then *first* we have to create an
independent Bhyve cleanup series, that rips out the TPM and/or SEV
remnants from the OvmfPkg/Bhyve sub-tree.


I prefer approach (b). I'm sorry that it means extra work wrt. Bhyve,
but I strongly believe in keeping all platforms in the tree, and that
means we need to spend time on such changes.

I'm not CC'ing Rebecca and Peter on this message -- we're deep into this
patch review thread, and they'd have no useful context. I suggest simply
including the "OvmfPkg/Bhyve/PlatformPei" patch in the next version of
this series, with a proper explanation in the blurb (patch#0) and on the
"OvmfPkg/Bhyve/PlatformPei" patch. That should give them enough context
to evaluate whether the change is necessary, or whether we should purge
the TPM and/or the SEV bits from Bhyve. You could also ask them just
this question in advance, in a separate email on the list (with
distilled context). Personally I'm unsure if the TPM and SEV bits
survived into Bhyve because those bits are actually put to use there, or
because the initial platform creation / cloning wasn't as minimal as it
could have been.

Note that in case TPM makes sense on bhyve but SEV doesn't, then
"OvmfPkg/Bhyve/PlatformPei" will have to install the new PPI
unconditionally.
I've had a further idea on this.

You could add an entirely new PEIM just for this. The entry point
function of the PEIM would check for SEV, decrypt the TPM range if SEV
were active, and then install gOvmfTpmMmioAccessiblePpiGuid
(unconditionally). The exit status of the PEIM would always be
EFI_ABORTED, because there would be no need to keep the PEIM resident.

The new PEIM would have a DEPEX on gEfiPeiMemoryDiscoveredPpiGuid, to
make sure that potential page table splitting for the potential MMIO
range decryption could be satisfied from permanent PEI RAM.

The new PEIM would be included in the DSC and FDF files of the usual
three OVMF platforms, and in the Bhyve platform -- dependent on the
TPM_ENABLE build flag.

There are several advantages to such a separate PEIM:

- For Bhyve, the update is minimal. Just include one line in each of the
FDF and the DSC files. No need to customize an existent
platform-specific PEIM, no code duplication between two PlatformPei modules.

- The new PEIM would depend on the TPM_ENABLE build flag, so it would
only be included in the firmware binaries if and only if Tcg2ConfigPei
were. No useless PPI installation would occur in the absence of TPM_ENABLE.

- No need to check PcdTpmBaseAddress for nullity in the new PEIM, before
the decryption, as TPM_ENABLE guarantees (on IA32/X64) that the PCD
already has the right value.

- The new logic would be properly ordered between PlatformPei and
Tcg2ConfigPei, namely due to the use of two such PPI GUIDs in DEPEXes
that actually make sense. PlatformPei -> TPM MMIO decryptor PEIM ordered
via "memory discovered" (needed for potential page table splitting), TPM
MMIO decryptor PEIM -> Tcg2ConfigPei ordered via "TPM MMIO decrypted".

You could place the new PEIM at:

OvmfPkg/Tcg/TpmMmioSevDecryptPei

If you haven't lost your patience with me yet, I'd really appreciate if
you could investigate this!
So far, this appears to be working nicely. I'm new at the whole PEIM
thing, so hopefully I haven't missed anything. I should be submitting the
patches soon for review.
So one thing I failed to do before submitting my previous patch was to
complete my testing against the IA32 and X64 combination build. In this
build, PEI is built as Ia32, and MemEncryptSevClearPageEncMask() will
return UNSUPPORTED causing an ASSERT (since I check the return code). So
there are a few options:

1. SEV works with the current encrypted mapping, it is only the SEV-ES
support that fails because of the ValidateMmioMemory() check. I can do
the mapping change just for SEV-ES since it is X64 only. This works,
because MemEncryptSevClearPageEncMask() will not return UNSUPPORTED
when running in 64-bit.
Can we really say "SEV works" though? Because, even using an X64 PEI
phase, and enabling only SEV (not SEV-ES), TPM access will be broken in
the PEI phase. Is my understanding correct?

I think the behavior you currently see is actually what we want, we
should double down on it -- if MemEncryptSevClearPageEncMask() fails,
report an explicit DEBUG_ERROR, and call CpuDeadLoop(). If the firmware
is built with TPM_ENABLE, and SEV is active, then an IA32 PEI phase is
simply unusable. Silently pretending that the TPM is not there, even
though it may have been configured on the QEMU command line, we just
failed to communicate with it, is not a good idea, IMO.

This is somewhat similar IMO to the S3Verification() function in
"OvmfPkg/PlatformPei/Platform.c".

TPM_ENABLE, SEV, IA32 PEI phase: pick any two.

Thanks,
Laszlo


2. Call MemEncryptSevClearPageEncMask() for SEV or SEV-ES, but don't check
the return status.

3. Create Ia32 and X64 versions of internal functions, where the Ia32
version simply returns SUCCESS because it can't do anything and the X64
version calls MemEncryptSevClearPageEncMask(), allowing the main code
to ASSERT on any errors.

I'm leaning towards #1, because this is an SEV-ES only issue. Thoughts?

Thanks,
Tom


One thing I found is that the Bhyve package makes reference to the
OvmfPkg/Bhyve/Tcg directory, but that directory does not exist. So I don't
think that TPM enablement has been tested. I didn't update the Bhyve
support for that reason.

Thanks,
Tom

Thanks!
Laszlo


Re: [PATCH 3/3] OvmfPkg/PlatformPei: Mark TPM MMIO range as unencrypted for SEV

Laszlo Ersek
 

On 04/23/21 19:41, Tom Lendacky wrote:
On 4/23/21 8:04 AM, Laszlo Ersek wrote:
I've had a further idea on this.

You could add an entirely new PEIM just for this. The entry point
function of the PEIM would check for SEV, decrypt the TPM range if SEV
were active, and then install gOvmfTpmMmioAccessiblePpiGuid
(unconditionally). The exit status of the PEIM would always be
EFI_ABORTED, because there would be no need to keep the PEIM resident.

The new PEIM would have a DEPEX on gEfiPeiMemoryDiscoveredPpiGuid, to
make sure that potential page table splitting for the potential MMIO
range decryption could be satisfied from permanent PEI RAM.

The new PEIM would be included in the DSC and FDF files of the usual
three OVMF platforms, and in the Bhyve platform -- dependent on the
TPM_ENABLE build flag.

There are several advantages to such a separate PEIM:

- For Bhyve, the update is minimal. Just include one line in each of the
FDF and the DSC files. No need to customize an existent
platform-specific PEIM, no code duplication between two PlatformPei modules.

- The new PEIM would depend on the TPM_ENABLE build flag, so it would
only be included in the firmware binaries if and only if Tcg2ConfigPei
were. No useless PPI installation would occur in the absence of TPM_ENABLE.

- No need to check PcdTpmBaseAddress for nullity in the new PEIM, before
the decryption, as TPM_ENABLE guarantees (on IA32/X64) that the PCD
already has the right value.

- The new logic would be properly ordered between PlatformPei and
Tcg2ConfigPei, namely due to the use of two such PPI GUIDs in DEPEXes
that actually make sense. PlatformPei -> TPM MMIO decryptor PEIM ordered
via "memory discovered" (needed for potential page table splitting), TPM
MMIO decryptor PEIM -> Tcg2ConfigPei ordered via "TPM MMIO decrypted".

You could place the new PEIM at:

OvmfPkg/Tcg/TpmMmioSevDecryptPei

If you haven't lost your patience with me yet, I'd really appreciate if
you could investigate this!
So far, this appears to be working nicely. I'm new at the whole PEIM
thing, so hopefully I haven't missed anything. I should be submitting the
patches soon for review.
Thanks!


One thing I found is that the Bhyve package makes reference to the
OvmfPkg/Bhyve/Tcg directory, but that directory does not exist. So I don't
think that TPM enablement has been tested. I didn't update the Bhyve
support for that reason.
That's good to know; thanks for reporting this. I've turned it now into
a BZ ticket (assigned to Rebecca):

https://bugzilla.tianocore.org/show_bug.cgi?id=3354

Thanks!
Laszlo


Re: [PATCH 1/7] IntelSiliconPkg/ReportCpuHobLib: Add ReportCpuHobLib

Ni, Ray
 

Sofia,
Several comments:
1. Please don't use hardcode value 0x80000000. Use the macro in MdePkg/.../Cpuid.h
2. Please don't use hardcode value 0x80000008. Use the macro and data structure in MdePkg/.../Cpuid.h
3. Please add "EFIAPI"
+VOID
+ReportCpuHob (
+ VOID
+ );
4. Can you please try to remove the IntelSiliconPkg.dec from below?
+[Packages]
+ MdePkg/MdePkg.dec
+ IntelSiliconPkg/IntelSiliconPkg.dec

-----Original Message-----
From: Chuang, SofiaX <sofiax.chuang@intel.com>
Sent: Monday, April 19, 2021 4:44 PM
To: devel@edk2.groups.io
Cc: Chuang, SofiaX <sofiax.chuang@intel.com>; Ni, Ray <ray.ni@intel.com>;
Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>
Subject: [PATCH 1/7] IntelSiliconPkg/ReportCpuHobLib: Add
ReportCpuHobLib

From: SofiaX Chuang <sofiax.chuang@intel.com>

Add ReportCpuHobLib

Signed-off-by: SofiaX Chuang <sofiax.chuang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
---
.../Include/Library/ReportCpuHobLib.h | 23 +++++++++++++
.../Intel/IntelSiliconPkg/IntelSiliconPkg.dec | 6 +++-
.../Library/ReportCpuHobLib/ReportCpuHobLib.c | 33
+++++++++++++++++++
.../ReportCpuHobLib/ReportCpuHobLib.inf | 27 +++++++++++++++
4 files changed, 88 insertions(+), 1 deletion(-)
create mode 100644
Silicon/Intel/IntelSiliconPkg/Include/Library/ReportCpuHobLib.h
create mode 100644
Silicon/Intel/IntelSiliconPkg/Library/ReportCpuHobLib/ReportCpuHobLib.c
create mode 100644
Silicon/Intel/IntelSiliconPkg/Library/ReportCpuHobLib/ReportCpuHobLib.inf

diff --git a/Silicon/Intel/IntelSiliconPkg/Include/Library/ReportCpuHobLib.h
b/Silicon/Intel/IntelSiliconPkg/Include/Library/ReportCpuHobLib.h
new file mode 100644
index 0000000000..9ca18146ed
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Include/Library/ReportCpuHobLib.h
@@ -0,0 +1,23 @@
+/** @file

+

+ Report CPU HOB library

+

+ This library report the CPU HOB with Physical Address bits.

+

+Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>

+SPDX-License-Identifier: BSD-2-Clause-Patent

+

+**/

+

+#ifndef _REPORT_CPU_HOB_LIB_H_

+#define _REPORT_CPU_HOB_LIB_H_

+

+#include <PiPei.h>

+#include <Uefi.h>

+

+VOID

+ReportCpuHob (

+ VOID

+ );

+

+#endif

diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
index 4a2cbca5c1..2461ab8e06 100644
--- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
+++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
@@ -3,7 +3,7 @@
#

# This package provides common open source Intel silicon modules.

#

-# Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>

+# Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR>

# SPDX-License-Identifier: BSD-2-Clause-Patent

#

##

@@ -42,6 +42,10 @@
#

AslUpdateLib|Include/Library/AslUpdateLib.h



+ ## @libraryclass Provides services to report CPU hob

+ #

+ ReportCpuHobLib|Include/Library/ReportCpuHobLib.h

+

[Guids]

## GUID for Package token space

# {A9F8D54E-1107-4F0A-ADD0-4587E7A4A735}

diff --git
a/Silicon/Intel/IntelSiliconPkg/Library/ReportCpuHobLib/ReportCpuHobLib.c
b/Silicon/Intel/IntelSiliconPkg/Library/ReportCpuHobLib/ReportCpuHobLib.c
new file mode 100644
index 0000000000..1a3d60366d
--- /dev/null
+++
b/Silicon/Intel/IntelSiliconPkg/Library/ReportCpuHobLib/ReportCpuHobLib.c
@@ -0,0 +1,33 @@
+/** @file

+ Source code file for Report CPU HOB library.

+

+Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>

+SPDX-License-Identifier: BSD-2-Clause-Patent

+

+**/

+

+#include <PiPei.h>

+#include <Library/BaseLib.h>

+#include <Library/HobLib.h>

+

+VOID

+ReportCpuHob (

+ VOID

+ )

+{

+ UINT8 PhysicalAddressBits;

+ UINT32 RegEax;

+

+ AsmCpuid (0x80000000, &RegEax, NULL, NULL, NULL);

+ if (RegEax >= 0x80000008) {

+ AsmCpuid (0x80000008, &RegEax, NULL, NULL, NULL);

+ PhysicalAddressBits = (UINT8) RegEax;

+ } else {

+ PhysicalAddressBits = 36;

+ }

+

+ ///

+ /// Create a CPU hand-off information

+ ///

+ BuildCpuHob (PhysicalAddressBits, 16);

+}

diff --git
a/Silicon/Intel/IntelSiliconPkg/Library/ReportCpuHobLib/ReportCpuHobLib.in
f
b/Silicon/Intel/IntelSiliconPkg/Library/ReportCpuHobLib/ReportCpuHobLib.in
f
new file mode 100644
index 0000000000..ae7871ad4e
--- /dev/null
+++
b/Silicon/Intel/IntelSiliconPkg/Library/ReportCpuHobLib/ReportCpuHobLib.in
f
@@ -0,0 +1,27 @@
+### @file

+# Component information file for the Report CPU HOB library.

+#

+# Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>

+#

+# SPDX-License-Identifier: BSD-2-Clause-Patent

+#

+###

+

+[Defines]

+ INF_VERSION = 0x00010005

+ BASE_NAME = ReportCpuHobLib

+ FILE_GUID = 0A1C9D6B-44BE-4FD7-A4A2-D0E68D436848

+ VERSION_STRING = 1.0

+ MODULE_TYPE = PEIM

+ LIBRARY_CLASS = ReportCpuHobLib

+

+[LibraryClasses]

+ BaseLib

+ HobLib

+

+[Packages]

+ MdePkg/MdePkg.dec

+ IntelSiliconPkg/IntelSiliconPkg.dec

+

+[Sources]

+ ReportCpuHobLib.c

--
2.27.0


Re: [PATCH] ArmPkg/ArmGicLib: fix maximum interrupts supported

Sami Mujawar
 

Hi Adrian,

 

I believe there is already a similar patch on the mailing list at https://edk2.groups.io/g/devel/message/72596. This patch is pending review and tested-by.

Can you check if this patch covers the problems you describe, please?

 

Regards,

 

Sami Mujawar

 

From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Adrián Herrera via groups.io <adr.her.arc.95@...>
Date: Saturday, 24 April 2021 at 03:57
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Adrián Herrera <adr.her.arc.95@...>
Subject: [edk2-devel] [PATCH] ArmPkg/ArmGicLib: fix maximum interrupts supported

The maximum number of interrupts supported is determined as
32 * (GICD_TYPER.ITLinesNumber + 1).

When GICD_TYPER.ITLinesNumber = 0b11111, the maximum number of
interrupts supported is 1024. However, both GICv2 and GICv3 reserve
INTIDs 1020-1023 for special purposes.

This results in runtime crashes because:
  (1) ArmGicLib functions do not guard against special interrupts.
  (2) ArmGicGetMaxNumInterrupts number includes special interrupts.
  (2) ArmGicV*Dxe relies on ArmGicGetMaxNumInterrupts, and thus
      programs special interrupts through ArmGicLib.

ArmGicGetMaxNumInterrupts now does not include special interrupts, that
is, it reports 1020 instead of 1024 when GICD_TYPER.ITLinesNumber = 0b11111.
This avoids the overhead of guarding ArmGicLib functions while not
requiring to modify the drivers code.

Signed-off-by: Adrián Herrera <adr.her.arc.95@...>
---
 ArmPkg/Drivers/ArmGic/ArmGicLib.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
index 6b01c88206..dff1401e9c 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
+++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
@@ -120,7 +120,10 @@ ArmGicGetMaxNumInterrupts (
   IN  INTN          GicDistributorBase


   )


 {


-  return 32 * ((MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) & 0x1F) + 1);


+  UINT32 ITLinesNumber;


+


+  ITLinesNumber = MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) & 0x1F;


+  return MIN (32 * (ITLinesNumber+ 1), 1020);


 }


 


 VOID


--
2.30.0






Re: [Patch 1/1] BaseTools/GenMake: Sort generated makefile tool definitions

Yuwei Chen
 

Reviewed-by: Yuwei Chen<yuwei.chen@intel.com>

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Monday, April 26, 2021 8:50 AM
To: devel@edk2.groups.io
Cc: Feng, Bob C <bob.c.feng@intel.com>; Liming Gao
<gaoliming@byosoft.com.cn>; Chen, Christine <yuwei.chen@intel.com>
Subject: [Patch 1/1] BaseTools/GenMake: Sort generated makefile tool
definitions

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3353

Sort the tool definition content of generated makefiles to help verify that
makefile contents have not changed after BaseTools code changes.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Yuwei Chen <yuwei.chen@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
BaseTools/Source/Python/AutoGen/GenMake.py | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 1cfac1cd82ca..961b2ab1c399 100755
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -1,7 +1,7 @@
## @file
# Create makefile for MS nmake and GNU make # -# Copyright (c) 2007 -
2020, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2007 - 2021, Intel Corporation. All rights
+reserved.<BR>
# Copyright (c) 2020, ARM Limited. All rights reserved.<BR> # SPDX-License-
Identifier: BSD-2-Clause-Patent # @@ -519,13 +519,15 @@ cleanlib:
# tools definitions
ToolsDef = []
IncPrefix = self._INC_FLAG_[MyAgo.ToolChainFamily]
- for Tool in MyAgo.BuildOption:
- for Attr in MyAgo.BuildOption[Tool]:
+ for Tool in sorted(list(MyAgo.BuildOption)):
+ Appended = False
+ for Attr in sorted(list(MyAgo.BuildOption[Tool])):
Value = MyAgo.BuildOption[Tool][Attr]
if Attr == "FAMILY":
continue
elif Attr == "PATH":
ToolsDef.append("%s = %s" % (Tool, Value))
+ Appended = True
else:
# Don't generate MAKE_FLAGS in makefile. It's put in
environment variable.
if Tool == "MAKE":
@@ -542,7 +544,9 @@ cleanlib:
Value = ' '.join(ValueList)

ToolsDef.append("%s_%s = %s" % (Tool, Attr, Value))
- ToolsDef.append("")
+ Appended = True
+ if Appended:
+ ToolsDef.append("")

# generate the Response file and Response flag
RespDict = self.CommandExceedLimit()
--
2.31.1.windows.1


[Patch 1/1] BaseTools/GenMake: Sort generated makefile tool definitions

Michael D Kinney
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3353

Sort the tool definition content of generated makefiles to help
verify that makefile contents have not changed after BaseTools
code changes.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Yuwei Chen <yuwei.chen@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
BaseTools/Source/Python/AutoGen/GenMake.py | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py b/BaseTools/Source/Python/AutoGen/GenMake.py
index 1cfac1cd82ca..961b2ab1c399 100755
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -1,7 +1,7 @@
## @file
# Create makefile for MS nmake and GNU make
#
-# Copyright (c) 2007 - 2020, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
# Copyright (c) 2020, ARM Limited. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -519,13 +519,15 @@ cleanlib:
# tools definitions
ToolsDef = []
IncPrefix = self._INC_FLAG_[MyAgo.ToolChainFamily]
- for Tool in MyAgo.BuildOption:
- for Attr in MyAgo.BuildOption[Tool]:
+ for Tool in sorted(list(MyAgo.BuildOption)):
+ Appended = False
+ for Attr in sorted(list(MyAgo.BuildOption[Tool])):
Value = MyAgo.BuildOption[Tool][Attr]
if Attr == "FAMILY":
continue
elif Attr == "PATH":
ToolsDef.append("%s = %s" % (Tool, Value))
+ Appended = True
else:
# Don't generate MAKE_FLAGS in makefile. It's put in environment variable.
if Tool == "MAKE":
@@ -542,7 +544,9 @@ cleanlib:
Value = ' '.join(ValueList)

ToolsDef.append("%s_%s = %s" % (Tool, Attr, Value))
- ToolsDef.append("")
+ Appended = True
+ if Appended:
+ ToolsDef.append("")

# generate the Response file and Response flag
RespDict = self.CommandExceedLimit()
--
2.31.1.windows.1


Re: [PATCH v1] Intel/WhiskeylakeOpenBoardPkg: Simplify microcode related PCD usage

Chiu, Chasel
 

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

-----Original Message-----
From: Lou, Yun <yun.lou@intel.com>
Sent: Sunday, April 25, 2021 10:42 PM
To: devel@edk2.groups.io
Cc: Lou, Yun <yun.lou@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>;
Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star
<star.zeng@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH v1] Intel/WhiskeylakeOpenBoardPkg: Simplify microcode
related PCD usage

From: Jason Lou <yun.lou@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3334

There are following PCDs in IntelFsp2WrapperPkg for microcode location:

* IntelFsp2WrapperPkg:
PcdCpuMicrocodePatchAddress
PcdCpuMicrocodePatchRegionSize
PcdFlashMicrocodeOffset

The change simplify the platform code to use following PCDs instead:
* MinPlatformPkg
PcdFlashFvMicrocodeOffset
PcdFlashFvMicrocodeBase = $(BIOS_BASE) + PcdFlashFvMicrocodeOffset
PcdFlashFvMicrocodeSize
PcdMicrocodeOffsetInFv <NEW>

Signed-off-by: Jason Lou <yun.lou@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
---

Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/SecF
spWrapperPlatformSecLib/SecRamInitData.c | 6 +++---

Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/SecF
spWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf | 8 ++++----
Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf
| 6 ++----

Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.fd
f | 6 ++----
4 files changed, 11 insertions(+), 15 deletions(-)

diff --git
a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/Se
cFspWrapperPlatformSecLib/SecRamInitData.c
b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/Se
cFspWrapperPlatformSecLib/SecRamInitData.c
index 8442e5fbff..41a37f5da5 100644
---
a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/Se
cFspWrapperPlatformSecLib/SecRamInitData.c
+++
b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/Se
cFspWrapperPlatformSecLib/SecRamInitData.c
@@ -1,7 +1,7 @@
/** @file

Provide TempRamInitParams data.



-Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>

+Copyright (c) 2020 - 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
FsptUpdDataPtr = {
},

// FSPT_CORE_UPD

{

- ((UINT32) FixedPcdGet64 (PcdCpuMicrocodePatchAddress) + FixedPcdGet32
(PcdFlashMicrocodeOffset)),

- ((UINT32) FixedPcdGet64 (PcdCpuMicrocodePatchRegionSize) -
FixedPcdGet32 (PcdFlashMicrocodeOffset)),

+ FixedPcdGet32 (PcdFlashFvMicrocodeBase) + FixedPcdGet32
(PcdMicrocodeOffsetInFv),

+ FixedPcdGet32 (PcdFlashFvMicrocodeSize) - FixedPcdGet32
(PcdMicrocodeOffsetInFv),

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/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/Se
cFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf
b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/Se
cFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf
index b17226d43b..e7319cf9e7 100644
---
a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/Se
cFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf
+++
b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/Se
cFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf
@@ -1,7 +1,7 @@
## @file

# Provide FSP wrapper platform sec related function.

#

-# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>

+# Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>

#

# SPDX-License-Identifier: BSD-2-Clause-Patent

#

@@ -92,9 +92,9 @@
[FixedPcd]

gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress ##
CONSUMES

gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate ##
CONSUMES

- gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress ##
CONSUMES

- gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize ##
CONSUMES

- gIntelFsp2WrapperTokenSpaceGuid.PcdFlashMicrocodeOffset ##
CONSUMES

+ gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase ##
CONSUMES

+ gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize ##
CONSUMES

+ gMinPlatformPkgTokenSpaceGuid.PcdMicrocodeOffsetInFv ##
CONSUMES

gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheAddress ##
CONSUMES

gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheSize ##
CONSUMES

gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress ##
CONSUMES

diff --git
a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf
b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf
index 0d99114961..22fbfc99f0 100644
--- a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf
+++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf
@@ -2,7 +2,7 @@
# FDF file for the UpXtreme.

#

#

-# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>

+# Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>

#

# SPDX-License-Identifier: BSD-2-Clause-Patent

#

@@ -47,9 +47,7 @@ SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =
$(gSiPkgTokenSpaceGuid.PcdBio
SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize)

SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase) + 0x60

SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize) - 0x60

-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress =
$(gSiPkgTokenSpaceGuid.PcdBiosAreaBaseAddress) +
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset)

-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize)

-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashMicrocodeOffset = 0x60

+SET gMinPlatformPkgTokenSpaceGuid.PcdMicrocodeOffsetInFv = 0x60

SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase

SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize

SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset

diff --git
a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.
fdf
b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.
fdf
index ad32268a82..1ab8c13792 100644
---
a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.
fdf
+++
b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.
fdf
@@ -2,7 +2,7 @@
# FDF file of Platform.

#

#

-# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>

+# Copyright (c) 2019 - 2021, Intel Corporation. All rights reserved.<BR>

#

# SPDX-License-Identifier: BSD-2-Clause-Patent

#

@@ -47,9 +47,7 @@ SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =
$(gSiPkgTokenSpaceGuid.PcdBio
SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize)

SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase) + 0x60

SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize) - 0x60

-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress =
$(gSiPkgTokenSpaceGuid.PcdBiosAreaBaseAddress) +
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset)

-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize)

-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashMicrocodeOffset = 0x60

+SET gMinPlatformPkgTokenSpaceGuid.PcdMicrocodeOffsetInFv = 0x60

SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase

SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize

SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset

--
2.28.0.windows.1


Re: [PATCH v1] Intel/TigerlakeOpenBoardPkg: Simplify microcode related PCD usage

Chiu, Chasel
 

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

-----Original Message-----
From: Lou, Yun <yun.lou@intel.com>
Sent: Sunday, April 25, 2021 10:42 PM
To: devel@edk2.groups.io
Cc: Lou, Yun <yun.lou@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>;
Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star
<star.zeng@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH v1] Intel/TigerlakeOpenBoardPkg: Simplify microcode related
PCD usage

From: Jason Lou <yun.lou@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3334

There are following PCDs in IntelFsp2WrapperPkg for microcode location:

* IntelFsp2WrapperPkg:
PcdCpuMicrocodePatchAddress
PcdCpuMicrocodePatchRegionSize
PcdFlashMicrocodeOffset

The change simplify the platform code to use following PCDs instead:
* MinPlatformPkg
PcdFlashFvMicrocodeOffset
PcdFlashFvMicrocodeBase = $(BIOS_BASE) + PcdFlashFvMicrocodeOffset
PcdFlashFvMicrocodeSize
PcdMicrocodeOffsetInFv <NEW>

Signed-off-by: Jason Lou <yun.lou@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
---
Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.fdf | 4
+---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git
a/Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.fdf
b/Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.fdf
index 0f645ed63e..c1fd2be6af 100644
--- a/Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.fdf
+++
b/Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.fdf
@@ -47,14 +47,12 @@ SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =
$(gSiPkgTokenSpaceGuid.PcdBio
SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize)

SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase) +
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeOffset)

SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize) -
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeOffset)

-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress =
$(gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress)

-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =
$(gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize)

-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashMicrocodeOffset =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeOffset)

SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheAddress =
gSiPkgTokenSpaceGuid.PcdBiosAreaBaseAddress

SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheSize =
gSiPkgTokenSpaceGuid.PcdBiosSize

SET gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress =
$(gSiPkgTokenSpaceGuid.PcdBiosAreaBaseAddress) +
$(gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspTOffset)

SET gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress =
$(gSiPkgTokenSpaceGuid.PcdBiosAreaBaseAddress) +
$(gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspMOffset)

SET gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress =
$(gSiPkgTokenSpaceGuid.PcdBiosAreaBaseAddress) +
$(gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspSOffset)

+SET gMinPlatformPkgTokenSpaceGuid.PcdMicrocodeOffsetInFv =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeOffset

SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase

SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize

SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset

--
2.28.0.windows.1


Re: [PATCH v1] Intel/KabylakeOpenBoardPkg: Simplify microcode related PCD usage

Chiu, Chasel
 

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

-----Original Message-----
From: Lou, Yun <yun.lou@intel.com>
Sent: Sunday, April 25, 2021 10:42 PM
To: devel@edk2.groups.io
Cc: Lou, Yun <yun.lou@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>;
Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star
<star.zeng@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH v1] Intel/KabylakeOpenBoardPkg: Simplify microcode related
PCD usage

From: Jason Lou <yun.lou@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3334

There are following PCDs in IntelFsp2WrapperPkg for microcode location:

* IntelFsp2WrapperPkg:
PcdCpuMicrocodePatchAddress
PcdCpuMicrocodePatchRegionSize
PcdFlashMicrocodeOffset

The change simplify the platform code to use following PCDs instead:
* MinPlatformPkg
PcdFlashFvMicrocodeOffset
PcdFlashFvMicrocodeBase = $(BIOS_BASE) + PcdFlashFvMicrocodeOffset
PcdFlashFvMicrocodeSize
PcdMicrocodeOffsetInFv <NEW>

Signed-off-by: Jason Lou <yun.lou@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
---
Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.fdf | 6 ++-
---
Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.fdf | 6
++----
2 files changed, 4 insertions(+), 8 deletions(-)

diff --git
a/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.fdf
b/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.fdf
index 196f04c68d..bcd1ade72b 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.fdf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.fdf
@@ -1,7 +1,7 @@
## @file

# System 76 GalagoPro3 board flash file.

#

-# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>

+# Copyright (c) 2019 - 2021, Intel Corporation. All rights reserved.<BR>

#

# SPDX-License-Identifier: BSD-2-Clause-Patent

#

@@ -45,9 +45,7 @@ SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =
$(gSiPkgTokenSpaceGuid.PcdFla
SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize)

SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase) + 0x60

SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize) - 0x60

-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress =
$(gSiPkgTokenSpaceGuid.PcdFlashAreaBaseAddress) +
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset)

-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize)

-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashMicrocodeOffset = 0x60

+SET gMinPlatformPkgTokenSpaceGuid.PcdMicrocodeOffsetInFv = 0x60

SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase

SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize

SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset

diff --git
a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.fdf
b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.fdf
index f2cfff1c7b..6cdf4e2f9f 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.fdf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.fdf
@@ -1,7 +1,7 @@
## @file

# FDF file of Platform.

#

-# 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

#

@@ -45,9 +45,7 @@ SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =
$(gSiPkgTokenSpaceGuid.PcdFla
SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize)

SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase) + 0x60

SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize) - 0x60

-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress =
$(gSiPkgTokenSpaceGuid.PcdFlashAreaBaseAddress) +
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset)

-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize)

-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashMicrocodeOffset = 0x60

+SET gMinPlatformPkgTokenSpaceGuid.PcdMicrocodeOffsetInFv = 0x60

SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase

SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize

SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset

--
2.28.0.windows.1


Re: [PATCH v1] Intel/CometlakeOpenBoardPkg: Simplify microcode related PCD usage

Chiu, Chasel
 

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

-----Original Message-----
From: Lou, Yun <yun.lou@intel.com>
Sent: Sunday, April 25, 2021 10:42 PM
To: devel@edk2.groups.io
Cc: Lou, Yun <yun.lou@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>;
Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star
<star.zeng@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH v1] Intel/CometlakeOpenBoardPkg: Simplify microcode related
PCD usage

From: Jason Lou <yun.lou@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3334

There are following PCDs in IntelFsp2WrapperPkg for microcode location:

* IntelFsp2WrapperPkg:
PcdCpuMicrocodePatchAddress
PcdCpuMicrocodePatchRegionSize
PcdFlashMicrocodeOffset

The change simplify the platform code to use following PCDs instead:
* MinPlatformPkg
PcdFlashFvMicrocodeOffset
PcdFlashFvMicrocodeBase = $(BIOS_BASE) + PcdFlashFvMicrocodeOffset
PcdFlashFvMicrocodeSize
PcdMicrocodeOffsetInFv <NEW>

Signed-off-by: Jason Lou <yun.lou@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
---
Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.fdf |
6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git
a/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.fdf
b/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.fdf
index 31f4d22311..795cc0da75 100644
---
a/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.fdf
+++
b/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.fdf
@@ -2,7 +2,7 @@
# FDF file of Platform.

#

#

-# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>

+# Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>

#

# SPDX-License-Identifier: BSD-2-Clause-Patent

#

@@ -47,9 +47,7 @@ SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =
$(gSiPkgTokenSpaceGuid.PcdBio
SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize)

SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase) + 0x60

SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize) - 0x60

-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress =
$(gSiPkgTokenSpaceGuid.PcdBiosAreaBaseAddress) +
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset)

-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =
$(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize)

-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashMicrocodeOffset = 0x60

+SET gMinPlatformPkgTokenSpaceGuid.PcdMicrocodeOffsetInFv = 0x60

SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase

SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize

SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset =
gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset

--
2.28.0.windows.1


[PATCH v1] Intel/WhiskeylakeOpenBoardPkg: Simplify microcode related PCD usage

Jason Lou
 

From: Jason Lou <yun.lou@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3334

There are following PCDs in IntelFsp2WrapperPkg for microcode location:

* IntelFsp2WrapperPkg:
PcdCpuMicrocodePatchAddress
PcdCpuMicrocodePatchRegionSize
PcdFlashMicrocodeOffset

The change simplify the platform code to use following PCDs instead:
* MinPlatformPkg
PcdFlashFvMicrocodeOffset
PcdFlashFvMicrocodeBase =3D $(BIOS_BASE) + PcdFlashFvMicrocodeOffset
PcdFlashFvMicrocodeSize
PcdMicrocodeOffsetInFv <NEW>

Signed-off-by: Jason Lou <yun.lou@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
---
Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/SecFspW=
rapperPlatformSecLib/SecRamInitData.c | 6 +++---
Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/SecFspW=
rapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf | 8 ++++----
Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf =
| 6 ++----
Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.fdf =
| 6 ++----
4 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Lib=
rary/SecFspWrapperPlatformSecLib/SecRamInitData.c b/Platform/Intel/Whiskeyl=
akeOpenBoardPkg/UpXtreme/FspWrapper/Library/SecFspWrapperPlatformSecLib/Sec=
RamInitData.c
index 8442e5fbff..41a37f5da5 100644
--- a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/Se=
cFspWrapperPlatformSecLib/SecRamInitData.c
+++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/Se=
cFspWrapperPlatformSecLib/SecRamInitData.c
@@ -1,7 +1,7 @@
/** @file=0D
Provide TempRamInitParams data.=0D
=0D
-Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>=0D
+Copyright (c) 2020 - 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 FsptUpdDataP=
tr =3D {
},=0D
// FSPT_CORE_UPD=0D
{=0D
- ((UINT32) FixedPcdGet64 (PcdCpuMicrocodePatchAddress) + FixedPcdGet32 =
(PcdFlashMicrocodeOffset)),=0D
- ((UINT32) FixedPcdGet64 (PcdCpuMicrocodePatchRegionSize) - FixedPcdGet=
32 (PcdFlashMicrocodeOffset)),=0D
+ FixedPcdGet32 (PcdFlashFvMicrocodeBase) + FixedPcdGet32 (PcdMicrocodeO=
ffsetInFv),=0D
+ FixedPcdGet32 (PcdFlashFvMicrocodeSize) - FixedPcdGet32 (PcdMicrocodeO=
ffsetInFv),=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/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Lib=
rary/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf b/Platform=
/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/SecFspWrapperPla=
tformSecLib/SecFspWrapperPlatformSecLib.inf
index b17226d43b..e7319cf9e7 100644
--- a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/Se=
cFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf
+++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/FspWrapper/Library/Se=
cFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf
@@ -1,7 +1,7 @@
## @file=0D
# Provide FSP wrapper platform sec related function.=0D
#=0D
-# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>=0D
+# Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>=
=0D
#=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
@@ -92,9 +92,9 @@
[FixedPcd]=0D
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress ## C=
ONSUMES=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate ## C=
ONSUMES=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.PcdMicrocodeOffsetInFv ## C=
ONSUMES=0D
gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheAddress ## C=
ONSUMES=0D
gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheSize ## C=
ONSUMES=0D
gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress ## C=
ONSUMES=0D
diff --git a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.f=
df b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf
index 0d99114961..22fbfc99f0 100644
--- a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf
+++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf
@@ -2,7 +2,7 @@
# FDF file for the UpXtreme.=0D
#=0D
#=0D
-# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>=0D
+# Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>=
=0D
#=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
@@ -47,9 +47,7 @@ SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =3D $(gS=
iPkgTokenSpaceGuid.PcdBio
SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =3D $(gSiPkgTokenSpaceGui=
d.PcdFlashMicrocodeFvSize)=0D
SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress =3D $(gSiPkgToke=
nSpaceGuid.PcdFlashMicrocodeFvBase) + 0x60=0D
SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =3D $(gSiPkgT=
okenSpaceGuid.PcdFlashMicrocodeFvSize) - 0x60=0D
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress =3D $(gSiP=
kgTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gSiPkgTokenSpaceGuid.PcdFlashM=
icrocodeFvOffset)=0D
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =3D $(g=
SiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize)=0D
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashMicrocodeOffset =3D 0x60=0D
+SET gMinPlatformPkgTokenSpaceGuid.PcdMicrocodeOffsetInFv =3D 0x60=0D
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase =3D gSiPkgTok=
enSpaceGuid.PcdFlashMicrocodeFvBase=0D
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize =3D gSiPkgTok=
enSpaceGuid.PcdFlashMicrocodeFvSize=0D
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset =3D gSiPkgTok=
enSpaceGuid.PcdFlashMicrocodeFvOffset=0D
diff --git a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoa=
rdPkg.fdf b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoar=
dPkg.fdf
index ad32268a82..1ab8c13792 100644
--- a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.f=
df
+++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.f=
df
@@ -2,7 +2,7 @@
# FDF file of Platform.=0D
#=0D
#=0D
-# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>=0D
+# Copyright (c) 2019 - 2021, Intel Corporation. All rights reserved.<BR>=
=0D
#=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
@@ -47,9 +47,7 @@ SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =3D $(gS=
iPkgTokenSpaceGuid.PcdBio
SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =3D $(gSiPkgTokenSpaceGui=
d.PcdFlashMicrocodeFvSize)=0D
SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress =3D $(gSiPkgToke=
nSpaceGuid.PcdFlashMicrocodeFvBase) + 0x60=0D
SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =3D $(gSiPkgT=
okenSpaceGuid.PcdFlashMicrocodeFvSize) - 0x60=0D
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress =3D $(gSiP=
kgTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gSiPkgTokenSpaceGuid.PcdFlashM=
icrocodeFvOffset)=0D
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =3D $(g=
SiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize)=0D
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashMicrocodeOffset =3D 0x60=0D
+SET gMinPlatformPkgTokenSpaceGuid.PcdMicrocodeOffsetInFv =3D 0x60=0D
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase =3D gSiPkgTok=
enSpaceGuid.PcdFlashMicrocodeFvBase=0D
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize =3D gSiPkgTok=
enSpaceGuid.PcdFlashMicrocodeFvSize=0D
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset =3D gSiPkgTok=
enSpaceGuid.PcdFlashMicrocodeFvOffset=0D
--=20
2.28.0.windows.1


[PATCH v1] Intel/TigerlakeOpenBoardPkg: Simplify microcode related PCD usage

Jason Lou
 

From: Jason Lou <yun.lou@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3334

There are following PCDs in IntelFsp2WrapperPkg for microcode location:

* IntelFsp2WrapperPkg:
PcdCpuMicrocodePatchAddress
PcdCpuMicrocodePatchRegionSize
PcdFlashMicrocodeOffset

The change simplify the platform code to use following PCDs instead:
* MinPlatformPkg
PcdFlashFvMicrocodeOffset
PcdFlashFvMicrocodeBase =3D $(BIOS_BASE) + PcdFlashFvMicrocodeOffset
PcdFlashFvMicrocodeSize
PcdMicrocodeOffsetInFv <NEW>

Signed-off-by: Jason Lou <yun.lou@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
---
Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.fdf | 4 +-=
--
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPk=
g.fdf b/Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.fdf
index 0f645ed63e..c1fd2be6af 100644
--- a/Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.fdf
+++ b/Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.fdf
@@ -47,14 +47,12 @@ SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =3D $(=
gSiPkgTokenSpaceGuid.PcdBio
SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =3D $(gSiPkgTokenSpaceGui=
d.PcdFlashMicrocodeFvSize)=0D
SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress =3D $(gSiPkgToke=
nSpaceGuid.PcdFlashMicrocodeFvBase) + $(gSiPkgTokenSpaceGuid.PcdFlashMicroc=
odeOffset)=0D
SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =3D $(gSiPkgT=
okenSpaceGuid.PcdFlashMicrocodeFvSize) - $(gSiPkgTokenSpaceGuid.PcdFlashMic=
rocodeOffset)=0D
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress =3D $(gUef=
iCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress)=0D
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =3D $(g=
UefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize)=0D
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashMicrocodeOffset =3D $(gSiPkgTo=
kenSpaceGuid.PcdFlashMicrocodeOffset)=0D
SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheAddress =3D gSiPkgTok=
enSpaceGuid.PcdBiosAreaBaseAddress=0D
SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheSize =3D gSiPkgTok=
enSpaceGuid.PcdBiosSize=0D
SET gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress =3D $(gSiPkgTokenSp=
aceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgTokenSpaceGuid.PcdFlashF=
vFspTOffset)=0D
SET gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress =3D $(gSiPkgTokenSp=
aceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgTokenSpaceGuid.PcdFlashF=
vFspMOffset)=0D
SET gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress =3D $(gSiPkgTokenSp=
aceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgTokenSpaceGuid.PcdFlashF=
vFspSOffset)=0D
+SET gMinPlatformPkgTokenSpaceGuid.PcdMicrocodeOffsetInFv =3D gSiPkgTok=
enSpaceGuid.PcdFlashMicrocodeOffset=0D
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase =3D gSiPkgTok=
enSpaceGuid.PcdFlashMicrocodeFvBase=0D
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize =3D gSiPkgTok=
enSpaceGuid.PcdFlashMicrocodeFvSize=0D
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset =3D gSiPkgTok=
enSpaceGuid.PcdFlashMicrocodeFvOffset=0D
--=20
2.28.0.windows.1


[PATCH v1] Intel/CometlakeOpenBoardPkg: Simplify microcode related PCD usage

Jason Lou
 

From: Jason Lou <yun.lou@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3334

There are following PCDs in IntelFsp2WrapperPkg for microcode location:

* IntelFsp2WrapperPkg:
PcdCpuMicrocodePatchAddress
PcdCpuMicrocodePatchRegionSize
PcdFlashMicrocodeOffset

The change simplify the platform code to use following PCDs instead:
* MinPlatformPkg
PcdFlashFvMicrocodeOffset
PcdFlashFvMicrocodeBase =3D $(BIOS_BASE) + PcdFlashFvMicrocodeOffset
PcdFlashFvMicrocodeSize
PcdMicrocodeOffsetInFv <NEW>

Signed-off-by: Jason Lou <yun.lou@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
---
Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.fdf | 6 ++=
----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPk=
g.fdf b/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.fdf
index 31f4d22311..795cc0da75 100644
--- a/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.fdf
+++ b/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.fdf
@@ -2,7 +2,7 @@
# FDF file of Platform.=0D
#=0D
#=0D
-# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>=0D
+# Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>=
=0D
#=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
@@ -47,9 +47,7 @@ SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =3D $(gS=
iPkgTokenSpaceGuid.PcdBio
SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =3D $(gSiPkgTokenSpaceGui=
d.PcdFlashMicrocodeFvSize)=0D
SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress =3D $(gSiPkgToke=
nSpaceGuid.PcdFlashMicrocodeFvBase) + 0x60=0D
SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =3D $(gSiPkgT=
okenSpaceGuid.PcdFlashMicrocodeFvSize) - 0x60=0D
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress =3D $(gSiP=
kgTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gSiPkgTokenSpaceGuid.PcdFlashM=
icrocodeFvOffset)=0D
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =3D $(g=
SiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize)=0D
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashMicrocodeOffset =3D 0x60=0D
+SET gMinPlatformPkgTokenSpaceGuid.PcdMicrocodeOffsetInFv =3D 0x60=0D
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase =3D gSiPkgTok=
enSpaceGuid.PcdFlashMicrocodeFvBase=0D
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize =3D gSiPkgTok=
enSpaceGuid.PcdFlashMicrocodeFvSize=0D
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset =3D gSiPkgTok=
enSpaceGuid.PcdFlashMicrocodeFvOffset=0D
--=20
2.28.0.windows.1

7821 - 7840 of 82170