Date   

Re: [PATCH V4 2/3] OvmfPkg/Sec: Update the check logic in SevEsIsEnabled

Brijesh Singh
 

Hi Min,

On 8/2/21 8:18 PM, Min Xu wrote:
RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
SevEsIsEnabled return TRUE if SevEsWorkArea->SevEsEnabled is non-zero.
It is correct when SevEsWorkArea is only used by SEV. After Intel TDX
is enabled in Ovmf, the SevEsWorkArea is shared by TDX and SEV. (This
is to avoid the waist of memory region in MEMFD). The value of
SevEsWorkArea->SevEsEnabled now is :
0 if in Legacy guest
1 if in SEV
2 if in Tdx guest
That's why the changes is made.
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Brijesh Singh <brijesh.singh@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: James Bottomley <jejb@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Signed-off-by: Min Xu <min.m.xu@...>
---
OvmfPkg/Sec/SecMain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index 9db67e17b2aa..e166a9389a1a 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -828,7 +828,7 @@ SevEsIsEnabled (
SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 (PcdSevEsWorkAreaBase);
- return ((SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled != 0));
+ return ((SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled == 1));
}
This is wrong, we need to check the SevEs sub type and not the global Sev enable. This also need to be broken into at least two commits

1. introduce the updated CcWorkArea structure
2. update the existing code to use the CcWorkArea layout

If you are okay then I can rework and send the patch so that you can add the TDX on top of it.

thanks

VOID


Re: [PATCH v2 1/1] MdePkg: add definition of LINUX_EFI_INITRD_MEDIA_GUID

Jeff Brasen
 

Mike, do the comments in this patch work for you? Would be nice to get this in prior to the 8/9 freeze if possible.

Thanks,

Jeff


From: Ard Biesheuvel <ardb@...>
Sent: Tuesday, July 27, 2021 10:48 AM
To: Jeff Brasen <jbrasen@...>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Ard Biesheuvel <ardb+tianocore@...>; Michael Kinney <michael.d.kinney@...>; Jordan Justen <jordan.l.justen@...>; Liming Gao (Byosoft address) <gaoliming@...>; Zhiguang Liu <zhiguang.liu@...>
Subject: Re: [PATCH v2 1/1] MdePkg: add definition of LINUX_EFI_INITRD_MEDIA_GUID
 
External email: Use caution opening links or attachments


On Tue, 27 Jul 2021 at 18:45, Jeff Brasen <jbrasen@...> wrote:
>
> Add LINUX_EFI_INITRD_MEDIA_GUID to our collection of GUID definitions,
> it can be used in a media device path to specify a Linux style initrd
> that can be loaded by the OS using the LoadFile2 protocol.
>
> Move these defines to MdePkg from OvmfPkg as these are relevant to
> non-OVMF targets as well.
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2564
> Signed-off-by: Jeff Brasen <jbrasen@...>

Acked-by: Ard Biesheuvel <ardb@...>

> ---
>  MdePkg/MdePkg.dec                          |  5 ++++
>  OvmfPkg/OvmfPkg.dec                        |  1 -
>  MdePkg/Include/Guid/LinuxEfiInitrdMedia.h  | 31 ++++++++++++++++++++++
>  OvmfPkg/Include/Guid/LinuxEfiInitrdMedia.h | 17 ------------
>  4 files changed, 36 insertions(+), 18 deletions(-)
>  create mode 100644 MdePkg/Include/Guid/LinuxEfiInitrdMedia.h
>  delete mode 100644 OvmfPkg/Include/Guid/LinuxEfiInitrdMedia.h
>
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index c5319fdd71ca..a28a2daaffa8 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -818,6 +818,11 @@ [Guids]
>    #
>    gTianoCustomDecompressGuid     = { 0xA31280AD, 0x481E, 0x41B6, { 0x95, 0xE8, 0x12, 0x7F, 0x4C, 0x98, 0x47, 0x79 }}
>
> +  #
> +  # GUID used to provide initrd to linux via LoadFile2 protocol
> +  #
> +  gLinuxEfiInitrdMediaGuid       = {0x5568e427, 0x68fc, 0x4f3d, {0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68}}
> +
>  [Guids.IA32, Guids.X64]
>    ## Include/Guid/Cper.h
>    gEfiIa32X64ErrorTypeCacheCheckGuid = { 0xA55701F5, 0xE3EF, 0x43de, { 0xAC, 0x72, 0x24, 0x9B, 0x57, 0x3F, 0xAD, 0x2C }}
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index 6ae733f6e39f..3153f5ae4540 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -118,7 +118,6 @@ [Guids]
>    gMicrosoftVendorGuid                  = {0x77fa9abd, 0x0359, 0x4d32, {0xbd, 0x60, 0x28, 0xf4, 0xe7, 0x8f, 0x78, 0x4b}}
>    gEfiLegacyBiosGuid                    = {0x2E3044AC, 0x879F, 0x490F, {0x97, 0x60, 0xBB, 0xDF, 0xAF, 0x69, 0x5F, 0x50}}
>    gEfiLegacyDevOrderVariableGuid        = {0xa56074db, 0x65fe, 0x45f7, {0xbd, 0x21, 0x2d, 0x2b, 0xdd, 0x8e, 0x96, 0x52}}
> -  gLinuxEfiInitrdMediaGuid              = {0x5568e427, 0x68fc, 0x4f3d, {0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68}}
>    gQemuKernelLoaderFsMediaGuid          = {0x1428f772, 0xb64a, 0x441e, {0xb8, 0xc3, 0x9e, 0xbd, 0xd7, 0xf8, 0x93, 0xc7}}
>    gGrubFileGuid                         = {0xb5ae312c, 0xbc8a, 0x43b1, {0x9c, 0x62, 0xeb, 0xb8, 0x26, 0xdd, 0x5d, 0x07}}
>    gConfidentialComputingSecretGuid      = {0xadf956ad, 0xe98c, 0x484c, {0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47}}
> diff --git a/MdePkg/Include/Guid/LinuxEfiInitrdMedia.h b/MdePkg/Include/Guid/LinuxEfiInitrdMedia.h
> new file mode 100644
> index 000000000000..0e7db8bd8140
> --- /dev/null
> +++ b/MdePkg/Include/Guid/LinuxEfiInitrdMedia.h
> @@ -0,0 +1,31 @@
> +/** @file
> +  GUID definition for the Linux Initrd media device path
> +
> +  Linux distro boot generally relies on an initial ramdisk (initrd)
> +  which is provided by the loader, and which contains additional kernel
> +  modules (for storage and network, for instance), and the initial user
> +  space startup code, i.e., the code which brings up the user space side
> +  of the entire OS.
> +
> +  In order to provide a standard method to locate this file,
> +  the GUID defined in this file is used to describe the device path
> +  for a LoadFile2 Protocol instance that is responsible for loading the initrd file.
> +
> +  The kernel EFI Stub will locate and use this instance to load the initrd,
> +  therefore the firmware/loader should install an instance of this to load the
> +  relevant initrd.
> +
> +  Copyright (c) 2020, Arm, Ltd. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef LINUX_EFI_INITRD_MEDIA_GUID_H__
> +#define LINUX_EFI_INITRD_MEDIA_GUID_H__
> +
> +#define LINUX_EFI_INITRD_MEDIA_GUID \
> +  {0x5568e427, 0x68fc, 0x4f3d, {0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68}}
> +
> +extern EFI_GUID gLinuxEfiInitrdMediaGuid;
> +
> +#endif
> diff --git a/OvmfPkg/Include/Guid/LinuxEfiInitrdMedia.h b/OvmfPkg/Include/Guid/LinuxEfiInitrdMedia.h
> deleted file mode 100644
> index 83fc3fc79aa6..000000000000
> --- a/OvmfPkg/Include/Guid/LinuxEfiInitrdMedia.h
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -/** @file
> -  GUID definition for the Linux Initrd media device path
> -
> -  Copyright (c) 2020, Arm, Ltd. All rights reserved.<BR>
> -
> -  SPDX-License-Identifier: BSD-2-Clause-Patent
> -**/
> -
> -#ifndef LINUX_EFI_INITRD_MEDIA_GUID_H__
> -#define LINUX_EFI_INITRD_MEDIA_GUID_H__
> -
> -#define LINUX_EFI_INITRD_MEDIA_GUID \
> -  {0x5568e427, 0x68fc, 0x4f3d, {0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68}}
> -
> -extern EFI_GUID gLinuxEfiInitrdMediaGuid;
> -
> -#endif
> --
> 2.25.1
>


Re: [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to populate StMM boot data from device tree

Sayanta Pattanayak
 

Hi,

Please find my response inline.

Regards,
Sayanta

-----Original Message-----
From: Ard Biesheuvel <ardb@...>
Sent: Sunday, August 1, 2021 10:08 PM
To: Sayanta Pattanayak <Sayanta.Pattanayak@...>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Sami Mujawar
<Sami.Mujawar@...>; Achin Gupta <Achin.Gupta@...>; Bret
Barkelew <Bret.Barkelew@...>; Jiewen Yao
<jiewen.yao@...>; Andrew Fish <afish@...>
Subject: Re: [edk2][PATCH v1 1/1] StandaloneMmPkg: add support to
populate StMM boot data from device tree

(correct Achin's email address, cc other replyers)

On Sun, 1 Aug 2021 at 18:36, Ard Biesheuvel <ardb@...> wrote:

On Fri, 30 Jul 2021 at 19:35, Sayanta Pattanayak
<sayanta.pattanayak@...> wrote:

Introduce support to populate StMM boot data via DTS parsing.
Why? Don't we have FF-A manifests for this? I would expect the secure
partition manager to marshal this data into the appropriate format
when necessary.
I may not have presented this patch properly.
The key objective of this patch is that in a FF-A Secure partition manager which has StandaloneMM as S-EL0 partition, the StMM boot data could be passed by partition manager through DT and StMM prepares the boot data by parsing the DT.
In existing solution, secure partition manager has StMM specific logic to fetch the StMM boot data and pass it on to StMM through sharedbuffer. Having FF-A manifest for secure partition, DT in this case, will allow Secure partition manager to not have StMM or any other secure partition specific logic to consolidate boot data and pass on.
In the context of this patch, secure partition manager(from EL3) passes the manifest or DT address as booting argument to StMM. StMM will just have the logic to parse the DT and prepare boot data structure, so the manifest is actually part of partition manager codebase.
StMM as secure partition can be used across various types of Secure partition manager, so some amount of uniformity is needed to follow same DT properties for StMM across all kind of Secure partition managers.

The DTB is
passed as a boot argument by a binary of higer exception level.
Previously it was achieved by placing the boot data structure in a
shared buffer and the address of this shared buffer was passed by
the binary of higher exception level. Now either of the option can
be used for populating StMM boot info.

StMM boot information structure binding in device tree can be of
following prototype. Property values are not mentioned here.

bootarg {
compatible = "bootargs";
h_type = <..>;
h_version = <..>;
h_size = <..>;
h_attr = <..>;
sp_mem_base = <..>;
sp_mem_limit = <..>;
sp_image_base = <..>;
sp_stack_base = <..>;
sp_heap_base = <..>;
sp_ns_comm_buf_base = <..>;
sp_shared_buf_base = <..>;
sp_image_size = <..>;
sp_pcpu_stack_size = <..>;
sp_heap_size = <..>;
sp_ns_comm_buf_size = <..>;
sp_shared_buf_size = <..>;
num_sp_mem_regions = <..>;
num_cpus = <..>;
};

Addition of DTS supoort involves a dependency on FdtLib from
EmbeddedPkg.

Signed-off-by: Sayanta Pattanayak <sayanta.pattanayak@...>
I don't think we should apply this change. DT is not part of the
original SPM or current FF-A spec, right? So please fix this in the
S-EL1 component instead.


---
Link to github branch with this patch -
https://github.com/SayantaP-arm/edk2/tree/stmm-dts

StandaloneMmPkg/StandaloneMmPkg.dsc | 1
+
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCo
reEntryPoint.inf | 3 +

StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalo
n
eMmCoreEntryPoint.c | 153 ++++++++++++++++++--
3 files changed, 143 insertions(+), 14 deletions(-)

diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc
b/StandaloneMmPkg/StandaloneMmPkg.dsc
index 0c45df95e2dd..e3a3a6ee3ba1 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dsc
+++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
@@ -49,6 +49,7 @@
HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHo
bLib.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf

MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMm
MemLib
.inf
+ FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmCoreMemor
yAllocationLib/StandaloneMmCoreMemoryAllocationLib.inf
MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Stand
aloneMmServicesTableLib.inf
PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
diff --git
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
Cor
eEntryPoint.inf
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
Cor
eEntryPoint.inf index 4fa426f58ef4..0a2e519dd664 100644
---
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMm
Cor
eEntryPoint.inf
+++
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneM
+++ mCoreEntryPoint.inf
@@ -30,6 +30,7 @@
X64/StandaloneMmCoreEntryPoint.c

[Packages]
+ EmbeddedPkg/EmbeddedPkg.dec
MdePkg/MdePkg.dec
MdeModulePkg/MdeModulePkg.dec
StandaloneMmPkg/StandaloneMmPkg.dec
@@ -40,10 +41,12 @@
[LibraryClasses]
BaseLib
DebugLib
+ FdtLib

[LibraryClasses.AARCH64]
StandaloneMmMmuLib
ArmSvcLib
+ FdtLib

[Guids]
gMpInformationHobGuid
diff --git
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standa
l
oneMmCoreEntryPoint.c
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standa
l
oneMmCoreEntryPoint.c index 6c50f470aa35..cc09d75dac36 100644
---
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standa
l
oneMmCoreEntryPoint.c
+++
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Sta
+++ ndaloneMmCoreEntryPoint.c
@@ -16,6 +16,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Guid/MmramMemoryReserve.h> #include
<Guid/MpInformation.h>

+#include <libfdt.h>
#include <Library/ArmMmuLib.h>
#include <Library/ArmSvcLib.h>
#include <Library/DebugLib.h>
@@ -45,33 +46,31 @@ STATIC CONST UINT32 mSpmMinorVerFfa =
SPM_MINOR_VERSION_FFA;
PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT CpuDriverEntryPoint =
NULL;

/**
- Retrieve a pointer to and print the boot information passed by
privileged
- secure firmware.
+ Prints boot information.

- @param [in] SharedBufAddress The pointer memory shared with
privileged
- firmware.
+ This function prints the boot information, which is passed by
+ privileged secure firmware through shared buffer or other mechanism.

+ @param [in] PayloadBootInfo Pointer to StandaloneMM Boot Info
structure.
**/
-EFI_SECURE_PARTITION_BOOT_INFO *
-GetAndPrintBootinformation (
- IN VOID *SharedBufAddress
+VOID
+PrintBootinformation (
+ IN EFI_SECURE_PARTITION_BOOT_INFO *PayloadBootInfo
)
{
- EFI_SECURE_PARTITION_BOOT_INFO *PayloadBootInfo;
EFI_SECURE_PARTITION_CPU_INFO *PayloadCpuInfo;
UINTN Index;

- PayloadBootInfo = (EFI_SECURE_PARTITION_BOOT_INFO *)
SharedBufAddress;

if (PayloadBootInfo == NULL) {
DEBUG ((DEBUG_ERROR, "PayloadBootInfo NULL\n"));
- return NULL;
+ return;
}

if (PayloadBootInfo->Header.Version != BOOT_PAYLOAD_VERSION) {
DEBUG ((DEBUG_ERROR, "Boot Information Version Mismatch.
Current=0x%x, Expected=0x%x.\n",
PayloadBootInfo->Header.Version, BOOT_PAYLOAD_VERSION));
- return NULL;
+ return;
}

DEBUG ((DEBUG_INFO, "NumSpMemRegions - 0x%x\n",
PayloadBootInfo->NumSpMemRegions));
@@ -96,7 +95,7 @@ GetAndPrintBootinformation (

if (PayloadCpuInfo == NULL) {
DEBUG ((DEBUG_ERROR, "PayloadCpuInfo NULL\n"));
- return NULL;
+ return;
}

for (Index = 0; Index < PayloadBootInfo->NumCpus; Index++) { @@
-105,7 +104,7 @@ GetAndPrintBootinformation (
DEBUG ((DEBUG_INFO, "Flags - 0x%x\n",
PayloadCpuInfo[Index].Flags));
}

- return PayloadBootInfo;
+ return;
}

/**
@@ -194,6 +193,119 @@ DelegatedEventLoop (
}
}

+/**
+ Populates StandAloneMM boot information structure.
+
+ This function receives dtb Address, where StMM Boot information
+ specific properties will be looked out to form the booting
+ structure of type EFI_SECURE_PARTITION_BOOT_INFO. At first, the
+ properties for StandAloneMM ConfigSize and Memory limit will be
+ checked out. Boot information will be stored at address (Memory
+ Limit - ConfigSize). Thereafter all boot information specific
+ properties will be parsed and corresponding values will be obtained.
+
+ @param [out] BootInfo Pointer, where Boot Info structure will be
populated.
+ @param [in] DtbAddress Address of the Device tree from where Boot
+ information will be fetched.
+**/
+VOID
+PopulateBootinformation (
+ OUT EFI_SECURE_PARTITION_BOOT_INFO **BootInfo,
+ IN VOID *DtbAddress
+)
+{
+ INT32 Offset;
+ CONST UINT32 *Property;
+ CONST UINT64 *Property64;
+ UINT32 ConfigSize;
+ UINT64 SpMemLimit;
+ EFI_SECURE_PARTITION_BOOT_INFO *PayloadBootInfo;
+
+ Offset = fdt_node_offset_by_compatible (DtbAddress, -1,
+ "config-size"); if (Offset < 0) {
+ DEBUG ((DEBUG_WARN, "Total Config Size is not defined\n")); }
+ else {
+ Property = fdt_getprop (DtbAddress, Offset, "size", NULL);
+ if (Property) {
+ ConfigSize = fdt32_to_cpu (*Property);
+ DEBUG ((DEBUG_INFO, "stmm dtb config-size = 0x%x \n",
ConfigSize));
+ }
+ }
+
+ Offset = fdt_node_offset_by_compatible (DtbAddress, -1,
+ "bootargs"); if (Offset >= 0) {
+ Property64 = fdt_getprop (DtbAddress, Offset, "sp_mem_limit",
NULL);
+ SpMemLimit = fdt64_to_cpu (*Property64); }
+
+ if (SpMemLimit && ConfigSize)
+ PayloadBootInfo =
+ (EFI_SECURE_PARTITION_BOOT_INFO *)(SpMemLimit - ConfigSize);
+
+ if (PayloadBootInfo) {
+ PayloadBootInfo->SpMemLimit = SpMemLimit;
+
+ Property = fdt_getprop (DtbAddress, Offset, "h_type", NULL);
+ PayloadBootInfo->Header.Type = (UINT8) fdt32_to_cpu(*Property);
+
+ Property = fdt_getprop (DtbAddress, Offset, "h_version", NULL);
+ PayloadBootInfo->Header.Version = (UINT8)
+ fdt32_to_cpu(*Property);
+
+ Property = fdt_getprop (DtbAddress, Offset, "h_size", NULL);
+ PayloadBootInfo->Header.Size = (UINT8) fdt32_to_cpu(*Property);
+
+ Property = fdt_getprop (DtbAddress, Offset, "h_attr", NULL);
+ PayloadBootInfo->Header.Attr = fdt32_to_cpu(*Property);
+
+ Property64 = fdt_getprop (DtbAddress, Offset, "sp_mem_base",
NULL);
+ PayloadBootInfo->SpMemBase = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset, "sp_image_base",
NULL);
+ PayloadBootInfo->SpImageBase = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset, "sp_stack_base",
NULL);
+ PayloadBootInfo->SpStackBase = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset, "sp_heap_base",
NULL);
+ PayloadBootInfo->SpHeapBase = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset,
"sp_ns_comm_buf_base", NULL);
+ PayloadBootInfo->SpNsCommBufBase = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset,
"sp_shared_buf_base", NULL);
+ PayloadBootInfo->SpSharedBufBase = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset, "sp_image_size",
NULL);
+ PayloadBootInfo->SpImageSize = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset, "sp_pcpu_stack_size",
NULL);
+ PayloadBootInfo->SpPcpuStackSize = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset, "sp_heap_size", NULL);
+ PayloadBootInfo->SpHeapSize = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset,
"sp_ns_comm_buf_size", NULL);
+ PayloadBootInfo->SpNsCommBufSize = fdt64_to_cpu(*Property64);
+
+ Property64 = fdt_getprop (DtbAddress, Offset, "sp_shared_buf_size",
NULL);
+ PayloadBootInfo->SpPcpuSharedBufSize =
+ fdt64_to_cpu(*Property64);
+
+ Property = fdt_getprop (DtbAddress, Offset, "num_sp_mem_regions",
NULL);
+ PayloadBootInfo->NumSpMemRegions = fdt32_to_cpu(*Property);
+
+ Property = fdt_getprop (DtbAddress, Offset, "num_cpus", NULL);
+ PayloadBootInfo->NumCpus = fdt32_to_cpu(*Property);
+
+ PayloadBootInfo->CpuInfo =
+ (EFI_SECURE_PARTITION_CPU_INFO *)((UINT64)PayloadBootInfo +
+
+ sizeof(EFI_SECURE_PARTITION_BOOT_INFO));
+ }
+
+ *BootInfo = PayloadBootInfo;
+
+ return;
+}
+
/**
Query the SPM version, check compatibility and return success if
compatible.

@@ -313,6 +425,7 @@ _ModuleEntryPoint (
VOID *TeData;
UINTN TeDataSize;
EFI_PHYSICAL_ADDRESS ImageBase;
+ VOID *DtbAddress;

// Get Secure Partition Manager Version Information
Status = GetSpmVersion ();
@@ -320,12 +433,24 @@ _ModuleEntryPoint (
goto finish;
}

- PayloadBootInfo = GetAndPrintBootinformation (SharedBufAddress);
+ // In cookie1 the DTB address is passed. With reference to DTB,
+ Boot // info structure can be populated.
+ // If cookie1 doesn't have any value, then Boot info is copied
+ from // Sharedbuffer.
+ if (cookie1) {
+ DtbAddress = (void *)cookie1;
+ PopulateBootinformation (&PayloadBootInfo, DtbAddress); } else
+ {
+ PayloadBootInfo = (EFI_SECURE_PARTITION_BOOT_INFO
+ *)SharedBufAddress; }
+
if (PayloadBootInfo == NULL) {
Status = EFI_UNSUPPORTED;
goto finish;
}

+ PrintBootinformation (PayloadBootInfo);
+
// Locate PE/COFF File information for the Standalone MM core module
Status = LocateStandaloneMmCorePeCoffData (
(EFI_FIRMWARE_VOLUME_HEADER *)
PayloadBootInfo->SpImageBase,
--
2.17.1
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: [edk2-platforms PATCH v5 1/2] Platform/RaspberryPi: Enable Boot Discovery Policy.

Ard Biesheuvel
 

On Tue, 3 Aug 2021 at 15:10, Ard Biesheuvel <ardb@...> wrote:

On Tue, 3 Aug 2021 at 15:08, Pete Batard <pete@...> wrote:

Hi Ard,

I thought the R-b from Sunny was enough.

For what is worth, I briefly tested these changes for v4.
Thus:

On 2021.08.03 14:03, Ard Biesheuvel wrote:
On Tue, 3 Aug 2021 at 15:00, Samer El-Haj-Mahmoud
<Samer.El-Haj-Mahmoud@...> wrote:

Ard,

Now that the EDK2 changes are merged (aaecef38b9440a65809cbdaf9d97029f4eeb), I think these RPi specific changes are ready to be merged as well.
I only see acks from Sunny though. This is why I asked Pete and Andrei
to chime in as well.
...

Reviewed-by: Pete Batard <pete@...>
Tested-by: Pete Batard <pete@...>
Thanks Pete. I did not want to assume that stakeholders like yourself
are ok with such changes if they did not get involved in the
discussion.

I will merge these now.
Pushed as 3ccc21d918c..2f0188b56ef4

Thanks to Grzegorz and others involved for finding a way to address
this boot protocol issue elegantly!

--
Ard.


Re: Proposing a new area of the edk2-test repository

Bret Barkelew <bret.barkelew@...>
 

Yeah, go for the patch.

 

- Bret

 

From: Samer El-Haj-Mahmoud via groups.io
Sent: Tuesday, August 3, 2021 6:10 AM
To: Nelson, Eric; Bret Barkelew; devel@edk2.groups.io; G Edhaya Chandran; gaojie@...; Kinney, Michael D
Cc: Samer El-Haj-Mahmoud
Subject: [EXTERNAL] Re: [edk2-devel] Proposing a new area of the edk2-test repository

 

I would think just sending the code contribution patch is sufficient.

 

 

From: Nelson, Eric <eric.nelson@...>
Sent: Wednesday, July 28, 2021 3:05 PM
To: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>; Bret Barkelew <Bret.Barkelew@...>; devel@edk2.groups.io; G Edhaya Chandran <Edhaya.Chandran@...>; gaojie@...; Kinney, Michael D <michael.d.kinney@...>
Subject: RE: Proposing a new area of the edk2-test repository

 

 

Adding ResumeOK.efi tool under /edk2-test/test-tools/TestToolsPkg would be great.

 

Should I propose this in the RFC and DEVEL mailing lists as a next step?

 

Thanks,

__e

 

 

From: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Sent: Friday, July 9, 2021 1:12 PM
To: Bret Barkelew <Bret.Barkelew@...>; devel@edk2.groups.io; Nelson, Eric <eric.nelson@...>; G Edhaya Chandran <Edhaya.Chandran@...>; gaojie@...; Kinney, Michael D <michael.d.kinney@...>
Subject: RE: Proposing a new area of the edk2-test repository

 

Interesting, thanks for sharing Bret. Some of those tests seem to be x64 specific (SMM tests), and some can be more generic like MorLockTestApp

 

Like I said earlier, I am not against adding test tools to edk2-test. That in fact is welcomed, especially if their usefulness in validating the solutions extend beyond specific implementations.

 

What would a good tree structure look like to accommodate misc tools? Today we have

 

/edk2-test/uefi-sct/SctPkg

 

How about something like this?

/edk2-test/test-tools/TestToolsPkg

or /edk2-test/ TestToolsPkg

 

The “ResumeOK” can be placed there

 

Any other ideas?

 

 

From: Bret Barkelew <Bret.Barkelew@...>
Sent: Thursday, June 24, 2021 12:25 AM
To: devel@edk2.groups.io; eric.nelson@...; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>; G Edhaya Chandran <Edhaya.Chandran@...>; gaojie@...; Kinney, Michael D <michael.d.kinney@...>
Subject: RE: Proposing a new area of the edk2-test repository

 

Fun fact! Mu also has a number of apps and things that we could work on moving to EDK2 if there were a suitable location. Right now, many of them are here:

mu_plus/UefiTestingPkg at release/202102 · microsoft/mu_plus (github.com)

 

- Bret

 

From: Nelson, Eric via groups.io
Sent: Wednesday, June 23, 2021 3:38 PM
To: Samer El-Haj-Mahmoud; G Edhaya Chandran; gaojie@...; devel@edk2.groups.io; Kinney, Michael D
Subject: [EXTERNAL] Re: [edk2-devel] Proposing a new area of the edk2-test repository

 

 

I have created a few other internal apps that build under WinTestPkg, although ResumeOK.efi is the only one I have received permissions to release sources for at this time.

And yes, they are primarily intended for validating Windows requirements.

I had some issues with my apps, needing to use different libraries than MdeModulePkg, and found it easier to create my own package, and use the libs I want.

 

__e

 

 

From: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Sent: Wednesday, June 23, 2021 1:56 PM
To: Nelson, Eric <eric.nelson@...>; G Edhaya Chandran <Edhaya.Chandran@...>; gaojie@...; devel@edk2.groups.io
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Subject: RE: Proposing a new area of the edk2-test repository

 

+edk2 list

 

I am not against adding additional test tools to edk2-test. Just feel like there is a need to organize and have a strategy, rather than just use edk2-test as a dumping group of miscellaneous tools.

 

There is already a place for apps under https://github.com/tianocore/edk2/tree/master/MdeModulePkg/Application

 

We also have a number of EDK2 misc applications that use edk2-libc in https://github.com/tianocore/edk2-libc/tree/master/AppPkg/Applications

 

A couple of questions:

  • Do you expect more apps from WinTestPkg to be contributed to TianoCore? And are they all around testing specific Windows requirements? If so, then having an edk2-test/WinTestPkg makes sense to me, as you will have a collection of useful testing app targeting specific area.
    • But what about other OSes?
  • If this is a one-off test app and other WinTestPkg apps are not going to be contributed, then does it make sense to put this under MdeModulePkg/Application ?

 

 

 

From: Nelson, Eric <eric.nelson@...>
Sent: Wednesday, June 23, 2021 3:10 PM
To: G Edhaya Chandran <Edhaya.Chandran@...>; gaojie@...
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Subject: RE: Proposing a new area of the edk2-test repository

 

 

Hi Edhay,

 

Do you have any more questions?

What do you think of creating another directory in edk2-test, for other test apps, in addition to uefi-sct, such as ResumeOK.efi?

 

Thanks,

__e

 

 

From: Nelson, Eric
Sent: Tuesday, June 15, 2021 12:00 PM
To: G Edhaya Chandran <Edhaya.Chandran@...>; gaojie@...
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Subject: RE: Proposing a new area of the edk2-test repository

 

 

Hi Edhay,

 

ResumeOK.efi is a tool I wrote from the HelloWorld example, that validates Windows resume from S4 requirements, specifically that the memory-map run-time memory regions don’t change, and secondly that PCI devices don’t disappear from the system, both conditions would cause Windows to fail to resume from S4.

 

You install the tool to the root of the ESP, and set it as the default/top entry in the boot manager, and launch it.  (Disable Secure Boot.)

 

It runs warm, cold, and 60s ACPI RTC wake cycles, infinitely looking for errors.

 

ResumeOK.efi writes a file to the root of the ESP, ResumeOK.map, which contains the ACPI Facs->HardwareSignature, a list of the PCI devices in the system, and a copy of its memory map, from the first time it runs.

 

During each test pass, it runs a barrage of tests:

 

  1. Free memory test – does the available memory match the memory map saved in ResumeOK.map
  2. HW signature check – does the system still have the same HW signature as saved in the ResumeOK.map
  3. Allocation test – all the available memory is allocated, and then the memory map is checked if the run-time regions match ResumeOK.map.

 

If any of the tests fail, then the new/missing PCI devices are listed (HW signature fail case), or the memory descriptor that changed, it’s location, and current and previous type and size.

 

I have received permission from Intel to *try* to release the source under Edk2-test.

 

I’ve included a 64-bit binary, if you want to give it a test drive.

 

Make sure Secure Boot is off.

Also, it is required to manually delete any ResumeOK.map on the ESP, before beginning a new test pass.

 

The tool also supports a host of EFI Shell commands:

 

Resumeok.efi MEMMAP – displays Windows coalesced view of the current memory map

ResumeOK.efi ROKMAP – displays Windows coalesced view of the memory saved in ResumeOK.map

ResumeOK.efi RTDATA – displays an analysis of RT_Data pool usage

ResumeOK.efi NORESET – run one test pass, but suppress automatic SX cycling

 

These are the files that build it:

 

Edk2\WinTestPkg\Application

Edk2\WinTestPkg\WinTestPkg.dec

Edk2\WinTestPkg\WinTestPkg.dsc

Edk2\WinTestPkg\Application\ResumeOK

Edk2\WinTestPkg\Application\ResumeOK\AcpiTbl.c

Edk2\WinTestPkg\Application\ResumeOK\AcpiTbl.h

Edk2\WinTestPkg\Application\ResumeOK\AppSupport.c

Edk2\WinTestPkg\Application\ResumeOK\BitMap.c

Edk2\WinTestPkg\Application\ResumeOK\BitMap.h

Edk2\WinTestPkg\Application\ResumeOK\EfiFileLib.c

Edk2\WinTestPkg\Application\ResumeOK\EfiFileLib.h

Edk2\WinTestPkg\Application\ResumeOK\pci.c

Edk2\WinTestPkg\Application\ResumeOK\Pci.h

Edk2\WinTestPkg\Application\ResumeOK\ResumeOK.c

Edk2\WinTestPkg\Application\ResumeOK\ResumeOK.h

Edk2\WinTestPkg\Application\ResumeOK\ResumeOK.inf

Edk2\WinTestPkg\Application\ResumeOK\ResumeOK.uni

Edk2\WinTestPkg\Application\ResumeOK\ResumeOKExtra.uni

Edk2\WinTestPkg\Application\ResumeOK\RtData.c

Edk2\WinTestPkg\Application\ResumeOK\TimeBaseLib.c

 

Thanks,

__e

 

 

From: G Edhaya Chandran <Edhaya.Chandran@...>
Sent: Monday, June 14, 2021 9:36 PM
To: Nelson, Eric <eric.nelson@...>; gaojie@...
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Subject: RE: Proposing a new area of the edk2-test repository

 

Hi Eric,

 

    Thanks for reaching out to us.

Can we get more details of the tool?

 

Is this tool already open sourced or could you send us the basic documentation pertaining to it.

 

With Warm Regards,
Edhay

 

 

From: Nelson, Eric <eric.nelson@...>
Sent: 15 June 2021 04:23
To: gaojie@...; G Edhaya Chandran <Edhaya.Chandran@...>
Subject: Proposing a new area of the edk2-test repository

 

 

Hello SCT maintainers,

 

I’m looking to release source to a UEFI validation tool that has been a big hit with platform BIOS validation teams, so it can help other PC vendors.

 

My coworker Michael Kinney suggested I reach out to you directly about the idea of creating a new top level directory in the edk2-test repro for other test apps, and I could be maintainer.

 

What do you think of creating another directory in edk2-test, for other test apps, in addition to uefi-sct?

 

Thanks!

__e

 

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.

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.

 

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.

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 edk2-test 1/1] uefi-sct/SctPkg: correct print code for EFI_MEMORY_TYPE

Samer El-Haj-Mahmoud
 

Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Heinrich
Schuchardt via groups.io
Sent: Friday, April 30, 2021 3:40 PM
To: EDK II Development <devel@edk2.groups.io>
Cc: Eric Jin <eric.jin@...>; G Edhaya Chandran
<Edhaya.Chandran@...>; Barton Gao <gaojie@...>; Arvin
Chen <arvinx.chen@...>; Samer El-Haj-Mahmoud <Samer.El-Haj-
Mahmoud@...>; Heinrich Schuchardt <xypron.glpk@...>
Subject: [edk2-devel] [PATCH edk2-test 1/1] uefi-sct/SctPkg: correct print code
for EFI_MEMORY_TYPE

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

EFI_MEMORY_TYPE is an enum. SctPrint expects an UINTN when printing with
%d. Add missing conversions in MemoryAllocationServicesBBTestFunction.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@...>
---
.../MemoryAllocationServicesBBTestFunction.c | 98 +++++++++----------
1 file changed, 49 insertions(+), 49 deletions(-)

diff --git a/uefi-
sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackB
oxTest/MemoryAllocationServicesBBTestFunction.c b/uefi-
sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackB
oxTest/MemoryAllocationServicesBBTestFunction.c
index bf8cd3b3afa4..e545b3cfc5b8 100644
--- a/uefi-
sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackB
oxTest/MemoryAllocationServicesBBTestFunction.c
+++ b/uefi-
sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackB
oxTest/MemoryAllocationServicesBBTestFunction.c
@@ -417,7 +417,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -437,7 +437,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory != 0) {
Status = gtBS->FreePages (
@@ -455,7 +455,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -478,7 +478,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
} else {
PageNum = (UINTN)Descriptor.NumberOfPages;
@@ -512,7 +512,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -532,7 +532,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory <= Descriptor.PhysicalStart +
SctLShiftU64 (Descriptor.NumberOfPages, EFI_PAGE_SHIFT) -
@@ -554,7 +554,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex],
+ (UINTN)AllocatePagesMemoryType[TypeIndex],
Descriptor.PhysicalStart,
Descriptor.NumberOfPages,
Memory
@@ -589,7 +589,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory2 & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -609,7 +609,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if ( Memory2 <= Descriptor.PhysicalStart +
SctLShiftU64 (Descriptor.NumberOfPages, EFI_PAGE_SHIFT) -
@@ -631,7 +631,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex],
+ (UINTN)AllocatePagesMemoryType[TypeIndex],
Memory2
);
if (Memory != 0) {
@@ -650,7 +650,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -670,7 +670,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -694,7 +694,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
} else {
PageNum = (UINTN)Descriptor.NumberOfPages;
@@ -739,7 +739,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -759,7 +759,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory == Start) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -779,7 +779,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory != 0) {
Status = gtBS->FreePages (
@@ -797,7 +797,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -824,7 +824,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
} else {
PageNum = (UINTN)Descriptor.NumberOfPages;
@@ -869,7 +869,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -889,7 +889,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory == Start) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -909,7 +909,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory != 0) {
Status = gtBS->FreePages (
@@ -927,7 +927,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -947,7 +947,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
} else {
PageNum = (UINTN)Descriptor.NumberOfPages;
@@ -992,7 +992,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1012,7 +1012,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory == Start + (SctLShiftU64 (PageNum/3, EFI_PAGE_SHIFT) &
0xFFFFFFFFFFFF0000)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1032,7 +1032,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory != 0) {
Status = gtBS->FreePages (
@@ -1050,7 +1050,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -1070,7 +1070,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
} else {
PageNum = (UINTN)Descriptor.NumberOfPages;
@@ -1115,7 +1115,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1135,7 +1135,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory == Start + (SctLShiftU64 (PageNum * 2 / 3, EFI_PAGE_SHIFT) &
0xFFFFFFFFFFFF0000)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1155,7 +1155,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory != 0) {
Status = gtBS->FreePages (
@@ -1173,7 +1173,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -1200,7 +1200,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
} else {
PageNum = (UINTN)Descriptor.NumberOfPages;
@@ -1245,7 +1245,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1265,7 +1265,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory == Start) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1285,7 +1285,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory != 0) {
Status = gtBS->FreePages (
@@ -1303,7 +1303,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -1323,7 +1323,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
} else {
PageNum = (UINTN)Descriptor.NumberOfPages;
@@ -1377,7 +1377,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1397,7 +1397,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory == Start) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1417,7 +1417,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory != 0) {
if (PageNum != 1) {
@@ -1442,7 +1442,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -1462,7 +1462,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
} else {
PageNum = (UINTN)Descriptor.NumberOfPages;
@@ -1507,7 +1507,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (!(Memory & EFI_PAGE_MASK)) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1527,7 +1527,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory == Start) {
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -1547,7 +1547,7 @@ BBTestAllocatePagesInterfaceTest (
__FILE__,
(UINTN)__LINE__,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
if (Memory != 0) {
Status = gtBS->FreePages (
@@ -1565,7 +1565,7 @@ BBTestAllocatePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}
}
@@ -1656,7 +1656,7 @@ BBTestFreePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
continue;
}
@@ -1685,7 +1685,7 @@ BBTestFreePagesInterfaceTest (
(UINTN)__LINE__,
Status,
TplArray[Index],
- AllocatePagesMemoryType[TypeIndex]
+ (UINTN)AllocatePagesMemoryType[TypeIndex]
);
}

--
2.30.2



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74678): https://edk2.groups.io/g/devel/message/74678
Mute This Topic: https://groups.io/mt/82490304/1945644
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [samer.el-haj-
mahmoud@...]
-=-=-=-=-=-=
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 v2 0/4] ArmPlatformPkg: Add support to generate HEST ACPI table

Sami Mujawar
 

Hi Omkar,

How did you check that the HEST table is populated correctly?

There is no HEST parser in Acpiview at the moment. Do you plan to add an HEST parser to Acpiview?

Regards,

Sami Mujawar

On 02/08/2021 01:49 PM, Sami Mujawar wrote:
Hi Omkar,

Thank you for this patch series and for the clear explaination below.

The explaination below is very useful for anyone who is trying to understand the code.

Since the cover letter will not be part of the patch commit messages, would it be possible to include this explanation:

1. as part of a commit message for one of the patches in this series (patch 2/4 or 3/4).

Or

2. in a Readme.md file

Regards,

Sami Mujawar


On 10/07/2021 05:18 PM, Omkar Anand Kulkarni wrote:
Changes since v1:
- Helper added for HEST ACPI table generation.
- Rebased to the latest upstream code.

Hardware Error Source Table (HEST)[1] and Software Delegated Exception Interface
(SDEI)[2] ACPI tables are used to acomplish firmware first error handling.This
patch series introduces a framework to build and install the HEST ACPI table
dynamically.

The following figure illustrates the possible usage of the dyanamic
generation of HEST ACPI table.

NS | S
+--------------------------------------+--------------------------------------+
| | |
|+-------------------------------------+---------------------+ |
|| +---------------------+--------------------+| |
|| | | || |
|| +-----------+ |+------------------+ | +-----------------+|| +-------------+|
|| |HestTable | || HestErrorSource | | | HestErrorSource ||| | DMC-620 ||
|| | DXE | || DXE | | | StandaloneMM ||| |Standalone MM||
|| +-----------+ |+------------------+ | +-----------------+|| +-------------+|
|| |GHESv2 | || |
|| +---------------------+--------------------+| |
|| +--------------------+ | | |
|| |PlatformErrorHandler| | | |
|| | DXE | | | |
|| +--------------------+ | | |
||FF FWK | | |
|+-------------------------------------+---------------------+ |
| | |
+--------------------------------------+--------------------------------------+
|
Figure: Firmware First Error Handling approach.

All the hardware error sources are added to HEST table as GHESv2[3] error source
descriptors. The framework comprises of following DXE and MM drivers:

- HestTableDxe:
Builds HEST table header and allows appending error source descriptors to the
HEST table. Also provides protocol interface to install the built HEST table.

- HestErrorSourceDxe & HestErrorSourceStandaloneMM:
These two drivers together retrieve all possible error source descriptors of
type GHESv2 from the MM drivers implementing HEST Error Source Descriptor
protocol. Once all the descriptors are collected HestErrorSourceDxe appends
it to HEST table using HestTableDxe driver.
- PlatformErrorHandlerDxe:
Builds and installs SDEI ACPI table. This driver does not initialize(load)
until HestErrorSourceDxe driver has finished appending all possible GHESv2
error source descriptors to the HEST table. Once that is complete using the
HestTableDxe driver it installs the HEST table.

This patch series provides reference implementation for DMC-620 Dynamic Memory
Controller[4] that has RAS feature enabled. This is platform code
implemented as Standalone MM driver in edk2-platforms.

References:
[1] : ACPI 6.3, Table 18-382, Hardware Error Source Table
[2] : SDEI Platform Design Document, revision b, 10 Appendix C, ACPI table
definitions for SDEI
[3] : ACPI Reference Specification 6.3, Table 18-393 GHESv2 Structure
[4] : DMC620 Dynamic Memory Controller, revision r1p0
[5] : UEFI Reference Specification 2.8, Appendix N - Common Platform Error
Record
[6] : UEFI Reference Specification 2.8, Section N.2.5 Memory Error Section

Link to github branch with the patches in this series -
https://github.com/omkkul01/edk2/tree/ras_firmware_first_edk2

Omkar Anand Kulkarni (4):
ArmPlatformPkg: Allow dynamic generation of HEST ACPI table
ArmPlatformPkg: add definition for MM_HEST_ERROR_SOURCE_DESC_PROTOCOL
ArmPlatformPkg: retreive error source descriptors from MM
ArmPlatformPkg: Add helpers for HEST table generation

ArmPlatformPkg/ArmPlatformPkg.dec | 12 +
.../Drivers/Apei/HestDxe/HestDxe.inf | 49 +++
.../HestMmErrorSources/HestErrorSourceDxe.inf | 44 +++
.../HestErrorSourceStandaloneMm.inf | 51 +++
.../HestMmErrorSourceCommon.h | 37 ++
ArmPlatformPkg/Include/HestAcpiHeader.h | 49 +++
.../Include/Protocol/HestErrorSourceInfo.h | 64 ++++
ArmPlatformPkg/Include/Protocol/HestTable.h | 71 ++++
ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.c | 354 ++++++++++++++++++
.../HestMmErrorSources/HestErrorSourceDxe.c | 308 +++++++++++++++
.../HestErrorSourceStandaloneMm.c | 312 +++++++++++++++
11 files changed, 1351 insertions(+)
create mode 100644 ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.inf
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h
create mode 100644 ArmPlatformPkg/Include/HestAcpiHeader.h
create mode 100644 ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h
create mode 100644 ArmPlatformPkg/Include/Protocol/HestTable.h
create mode 100644 ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.c
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c


Re: [RFC PATCH v5 07/28] OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase

Brijesh Singh
 

On 7/31/21 3:44 AM, Erdem Aktas wrote:
On Wed, Jun 30, 2021 at 5:54 AM Brijesh Singh <brijesh.singh@...> wrote:

a) Enhance the OVMF reset vector code to validate the pages as described
above (go through step 2 - 3).
OR
b) Validate the pages during the guest creation time. The SEV firmware
provides a command which can be used by the VMM to validate the pages
without affecting the measurement of the launch.
Are you referring to the PAGE_TYPE_UNMEASURED? Does it not affect the
measurement , PAGE_INFO will be still measured, right?
Yes. The unmeasured here means the contents of the page is not measured but the PAGE_INFO is measured for all the pages added before the VM launch.


Approach #b seems much simpler; it does not require any changes to the
OVMF reset vector code.
I am worried about verifying the measurement. I understand the secret
page and cpuid page being part of measurement because both of them are
mentioned in the AMD SNP SPEC but now we are introducing a new
parameters (all the 4KB page addresses between SNP_HV_VALIDATED_START
and SNP_HV_VALIDATED_END) that VM owner needs to know to calculate the
measurement and verify the attestation.
The page info of both the secrets and cpuid page also need to be measured. In order to calculate the expected measurement, a caller need to know the page_info for the secrets and cpuid. To get the page_info for the CPUID and Secrets they must read the OVMF reset GUID. While at it, they can also get the the range of the unmeasured pages. I don't see that being a big issue. Having said so, as I described in the patch, its not only option. It was easier for implementation without compromising the security.


Sorry if I am overthinking or missing something here.
-Erdem


Re: [PATCH v2 4/4] ArmPlatformPkg: Add helpers for HEST table generation

Sami Mujawar
 

Hi Omkar,

Thank you for this patch.

I have a minor suggestion marked inline as [SAMI], other than that this patch looks good to me.

Reviewed-by: Sami Mujawar<sami.mujawar@...>

Regards,

Sami Mujawar


On 10/07/2021 05:18 PM, Omkar Anand Kulkarni wrote:
Add helper macros for the generation of the HEST ACPI table. Macros to
initialize the HEST GHESv2 Notification Structure and Error Status
Structure are introduced.

Signed-off-by: Omkar Anand Kulkarni <omkar.kulkarni@...>
---
ArmPlatformPkg/Include/HestAcpiHeader.h | 49 ++++++++++++++++++++
1 file changed, 49 insertions(+)

diff --git a/ArmPlatformPkg/Include/HestAcpiHeader.h b/ArmPlatformPkg/Include/HestAcpiHeader.h
new file mode 100644
index 000000000000..5112ee5b22c5
--- /dev/null
+++ b/ArmPlatformPkg/Include/HestAcpiHeader.h
@@ -0,0 +1,49 @@
+/** @file
+ HEST table helper macros.
+
+ Macro definitions to initialize the HEST ACPI table specific structures.
+
+ Copyright (c) 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+ - ACPI Reference Specification 6.3
+ - UEFI Reference Specification 2.8
+**/
+
+#ifndef HEST_ACPI_HEADER_
+#define HEST_ACPI_HEADER_
+
+#include <IndustryStandard/Acpi.h>
+
+//
+// HEST table GHESv2 type related structures.
+//
+// Helper Macro to initialize the HEST GHESv2 Notification Structure.
+// Refer Table 18-394 in ACPI Specification, Version 6.3.
+#define EFI_ACPI_6_3_HARDWARE_ERROR_NOTIFICATION_STRUCTURE_INIT(Type, \
+ PollInterval, EventId) \
+ { \
+ Type, \
+ sizeof (EFI_ACPI_6_3_HARDWARE_ERROR_NOTIFICATION_STRUCTURE), \
+ {0, 0, 0, 0, 0, 0, 0}, /* ConfigurationWriteEnable */ \
+ PollInterval, \
+ EventId, \
+ 0, /* Poll Interval Threshold Value */ \
+ 0, /* Poll Interval Threshold Window */ \
+ 0, /* Error Threshold Value */ \
+ 0 /* Error Threshold Window */ \
+ }
+
+// Helper Macro to initialize the HEST GHESv2 Error Status Structure.
+// Refer Section 5.2.3.2 in ACPI Specification, Version 6.3.
+#define EFI_ACPI_6_3_GENERIC_ERROR_STATUS_STRUCTURE_INIT(Address) \
[SAMI] Would it be possible to define ARM_GAS64() in EmbeddedPkg\Include\Library\AcpiLib.h instead of this macro?
Similarly, can EFI_ACPI_6_3_HARDWARE_ERROR_NOTIFICATION_STRUCTURE_INIT() macro also be placed in EmbeddedPkg\Include\Library\AcpiLib.h
[/SAMI]
+ { \
+ 0, /* UINT8 Address Space ID */ \
+ 64, /* Register Bit Width */ \
+ 0, /* Register Bit Offset */ \
+ 4, /* Access Size */ \
+ Address /* CPER/Read Ack Addr */ \
+ }
+
+#endif /* HEST_ACPI_HEADER_ */


Re: [edk2-platforms][PATCH v5 21/46] KabylakeOpenBoardPkg: Update SpiFvbService & SpiFlashCommonLib

Michael Kubacki
 

Yes, patches were missing because the mailing list chose to unsort and retain non-consecutive patches (they were sent in order) after they exceeded the autoresponder mail loop guard of 40 messages.

The following were missing:
0, 10, 11, 15, 23, 32, 43

I sent the missing patches a few minutes ago.

Thanks,
Michael

On 8/3/2021 3:38 AM, Chiu, Chasel wrote:
Please see my comments below inline.
Thanks,
Chasel

-----Original Message-----
From: mikuback@... <mikuback@...>
Sent: Tuesday, August 3, 2021 10:39 AM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L
<nathaniel.l.desimone@...>; Jeremy Soller <jeremy@...>
Subject: [edk2-platforms][PATCH v5 21/46] KabylakeOpenBoardPkg: Update
SpiFvbService & SpiFlashCommonLib

From: Michael Kubacki <michael.kubacki@...>

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

Updates KabylakeOpenBoardPkg to use the SmmSpiFlashCommonLib instance in
IntelSiliconPkg and the SpiFvbServiceSmm driver in IntelSiliconPkg.

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Jeremy Soller <jeremy@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@...>
Reviewed-by: Chasel Chiu <chasel.chiu@...>
---
Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc | 7
+++++--
Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.fdf | 2 +-
Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc | 7
+++++--
Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.fdf | 2 +-
4 files changed, 12 insertions(+), 6 deletions(-)

diff --git
a/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc
b/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc
index 302cb679b5eb..89be744a9038 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc
+++ b/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc
@@ -228,7 +228,7 @@ [LibraryClasses.X64.DXE_SMM_DRIVER]
#######################################
# Silicon Initialization Package
#######################################
-
SpiFlashCommonLib|$(PLATFORM_SI_PACKAGE)/Pch/Library/SmmSpiFlashCom
monLib/SmmSpiFlashCommonLib.inf
+
+
SpiFlashCommonLib|IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SmmSpiF
+ lashCommonLib.inf

#######################################
# Platform Package
@@ -377,6 +377,10 @@ [Components.X64]
IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
$(PLATFORM_SI_BIN_PACKAGE)/Microcode/MicrocodeUpdates.inf

+!if gMinPlatformPkgTokenSpaceGuid.PcdBootToShellOnly == FALSE
+ IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
+!endif
+
I encountered build failure because this file not found, did I miss any prerequisite patch?

#######################################
# Platform Package
#######################################
@@ -393,7 +397,6 @@ [Components.X64]

!if gMinPlatformPkgTokenSpaceGuid.PcdBootToShellOnly == FALSE

- $(PLATFORM_PACKAGE)/Flash/SpiFvbService/SpiFvbServiceSmm.inf
$(PLATFORM_PACKAGE)/PlatformInit/PlatformInitSmm/PlatformInitSmm.inf

$(PLATFORM_PACKAGE)/Acpi/AcpiSmm/AcpiSmm.inf { diff --git
a/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.fdf
b/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.fdf
index 39432d21b8b5..239b6b720a6a 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.fdf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.fdf
@@ -401,7 +401,7 @@ [FV.FvOsBootUncompact] !if
gMinPlatformPkgTokenSpaceGuid.PcdBootToShellOnly == FALSE INF
$(PLATFORM_PACKAGE)/PlatformInit/SiliconPolicyDxe/SiliconPolicyDxe.inf
INF
$(PLATFORM_PACKAGE)/PlatformInit/PlatformInitSmm/PlatformInitSmm.inf
-INF $(PLATFORM_PACKAGE)/Flash/SpiFvbService/SpiFvbServiceSmm.inf
+INF IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf

INF $(PLATFORM_PACKAGE)/Acpi/AcpiTables/AcpiPlatform.inf
INF $(PLATFORM_PACKAGE)/Acpi/AcpiSmm/AcpiSmm.inf
diff --git
a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc
b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc
index 8523ab3f4fc1..f29393579c06 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc
+++ b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc
@@ -268,7 +268,7 @@ [LibraryClasses.X64.DXE_SMM_DRIVER]
#######################################
# Silicon Initialization Package
#######################################
-
SpiFlashCommonLib|$(PLATFORM_SI_PACKAGE)/Pch/Library/SmmSpiFlashCom
monLib/SmmSpiFlashCommonLib.inf
+
+
SpiFlashCommonLib|IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SmmSpiF
+ lashCommonLib.inf

#######################################
# Platform Package
@@ -456,6 +456,10 @@ [Components.X64]
IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
$(PLATFORM_SI_BIN_PACKAGE)/Microcode/MicrocodeUpdates.inf

+!if gMinPlatformPkgTokenSpaceGuid.PcdBootToShellOnly == FALSE
+ IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
+!endif
+
#######################################
# Platform Package
#######################################
@@ -472,7 +476,6 @@ [Components.X64]

!if gMinPlatformPkgTokenSpaceGuid.PcdBootToShellOnly == FALSE

- $(PLATFORM_PACKAGE)/Flash/SpiFvbService/SpiFvbServiceSmm.inf
$(PLATFORM_PACKAGE)/PlatformInit/PlatformInitSmm/PlatformInitSmm.inf

$(PLATFORM_PACKAGE)/Acpi/AcpiSmm/AcpiSmm.inf { diff --git
a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.fdf
b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.fdf
index f003dda0ddfc..23f9be5cf2a2 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.fdf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.fdf
@@ -408,7 +408,7 @@ [FV.FvOsBootUncompact] !if
gMinPlatformPkgTokenSpaceGuid.PcdBootToShellOnly == FALSE INF
$(PLATFORM_PACKAGE)/PlatformInit/SiliconPolicyDxe/SiliconPolicyDxe.inf
INF
$(PLATFORM_PACKAGE)/PlatformInit/PlatformInitSmm/PlatformInitSmm.inf
-INF $(PLATFORM_PACKAGE)/Flash/SpiFvbService/SpiFvbServiceSmm.inf
+INF IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf

INF $(PLATFORM_PACKAGE)/Acpi/AcpiTables/AcpiPlatform.inf
INF $(PLATFORM_PACKAGE)/Acpi/AcpiSmm/AcpiSmm.inf
--
2.28.0.windows.1


[edk2-platforms][PATCH v5 43/46] KabylakeOpenBoardPkg/PeiSerialPortLibSpiFlash: Update for new SPI PPI API

Michael Kubacki
 

From: Michael Kubacki <michael.kubacki@...>

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

Updates usage of gPchSpiPpiGuid to use the new interface that
identifies SPI flash regions by GUID.

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/Pei=
SerialPortLibSpiFlash.c | 4 ++--
Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/Pei=
SerialPortLibSpiFlash.inf | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLib=
SpiFlash/PeiSerialPortLibSpiFlash.c b/Platform/Intel/KabylakeOpenBoardPkg=
/Library/PeiSerialPortLibSpiFlash/PeiSerialPortLibSpiFlash.c
index fc48bdc6fccb..fe8883a8af29 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlas=
h/PeiSerialPortLibSpiFlash.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlas=
h/PeiSerialPortLibSpiFlash.c
@@ -98,7 +98,7 @@ SerialPortWrite (
LinearOffset =3D (UINT32) (FixedPcdGet32 (PcdFlashNvDebugMessageBase=
) - FixedPcdGet32 (PcdFlashAreaBaseAddress));
Status =3D PchSpiPpi->FlashErase (
PchSpiPpi,
- FlashRegionBios,
+ &gFlashRegionBiosGuid,
LinearOffset,
NvMessageAreaSize
);
@@ -118,7 +118,7 @@ SerialPortWrite (
=20
Status =3D PchSpiPpi->FlashWrite (
PchSpiPpi,
- FlashRegionBios,
+ &gFlashRegionBiosGuid,
LinearOffset,
BytesWritten,
(UINT8 *) &Buffer[SourceBufferOffset]
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLib=
SpiFlash/PeiSerialPortLibSpiFlash.inf b/Platform/Intel/KabylakeOpenBoardP=
kg/Library/PeiSerialPortLibSpiFlash/PeiSerialPortLibSpiFlash.inf
index b959cd1f4612..b8ae214f0920 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlas=
h/PeiSerialPortLibSpiFlash.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlas=
h/PeiSerialPortLibSpiFlash.inf
@@ -43,6 +43,7 @@ [Ppis]
gPchSpiPpiGuid
=20
[Guids]
+ gFlashRegionBiosGuid
gSpiFlashDebugHobGuid
=20
[Pcd]
--=20
2.28.0.windows.1


[edk2-platforms][PATCH v5 32/46] MinPlatformPkg: Remove SpiFlashCommonLibNull

Michael Kubacki
 

From: Michael Kubacki <michael.kubacki@...>

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

The library instance has moved to IntelSiliconPkg.

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Liming Gao <gaoliming@...>
Cc: Eric Dong <eric.dong@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@...>
Reviewed-by: Chasel Chiu <chasel.chiu@...>
---
Platform/Intel/MinPlatformPkg/Flash/Library/SpiFlashCommonLibNull/SpiFla=
shCommonLibNull.c | 101 --------------------
Platform/Intel/MinPlatformPkg/Flash/Library/SpiFlashCommonLibNull/SpiFla=
shCommonLibNull.inf | 29 ------
Platform/Intel/MinPlatformPkg/Include/Library/SpiFlashCommonLib.h =
| 98 -------------------
Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec =
| 2 -
Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc =
| 4 -
5 files changed, 234 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Flash/Library/SpiFlashCommonLi=
bNull/SpiFlashCommonLibNull.c b/Platform/Intel/MinPlatformPkg/Flash/Libra=
ry/SpiFlashCommonLibNull/SpiFlashCommonLibNull.c
deleted file mode 100644
index 403b16a1b421..000000000000
--- a/Platform/Intel/MinPlatformPkg/Flash/Library/SpiFlashCommonLibNull/S=
piFlashCommonLibNull.c
+++ /dev/null
@@ -1,101 +0,0 @@
-/** @file
- Null Library instance of SPI Flash Common Library Class
-
-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
-SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#include <Uefi.h>
-#include <Library/DebugLib.h>
-
-/**
- Enable block protection on the Serial Flash device.
-
- @retval EFI_SUCCESS Opertion is successful.
- @retval EFI_DEVICE_ERROR If there is any device errors.
-
-**/
-EFI_STATUS
-EFIAPI
-SpiFlashLock (
- VOID
- )
-{
- return EFI_SUCCESS;
-}
-
-/**
- Read NumBytes bytes of data from the address specified by
- PAddress into Buffer.
-
- @param[in] Address The starting physical address of the rea=
d.
- @param[in,out] NumBytes On input, the number of bytes to read. O=
n output, the number
- of bytes actually read.
- @param[out] Buffer The destination data buffer for the read=
.
-
- @retval EFI_SUCCESS Opertion is successful.
- @retval EFI_DEVICE_ERROR If there is any device errors.
-
-**/
-EFI_STATUS
-EFIAPI
-SpiFlashRead (
- IN UINTN Address,
- IN OUT UINT32 *NumBytes,
- OUT UINT8 *Buffer
- )
-{
- ASSERT(FALSE);
- return EFI_SUCCESS;
-}
-
-/**
- Write NumBytes bytes of data from Buffer to the address specified by
- PAddresss.
-
- @param[in] Address The starting physical address of the w=
rite.
- @param[in,out] NumBytes On input, the number of bytes to write=
. On output,
- the actual number of bytes written.
- @param[in] Buffer The source data buffer for the write.
-
- @retval EFI_SUCCESS Opertion is successful.
- @retval EFI_DEVICE_ERROR If there is any device errors.
-
-**/
-EFI_STATUS
-EFIAPI
-SpiFlashWrite (
- IN UINTN Address,
- IN OUT UINT32 *NumBytes,
- IN UINT8 *Buffer
- )
-{
- ASSERT(FALSE);
- return EFI_SUCCESS;
-}
-
-/**
- Erase the block starting at Address.
-
- @param[in] Address The starting physical address of the block=
to be erased.
- This library assume that caller garantee t=
hat the PAddress
- is at the starting address of this block.
- @param[in] NumBytes On input, the number of bytes of the logic=
al block to be erased.
- On output, the actual number of bytes eras=
ed.
-
- @retval EFI_SUCCESS. Opertion is successful.
- @retval EFI_DEVICE_ERROR If there is any device errors.
-
-**/
-EFI_STATUS
-EFIAPI
-SpiFlashBlockErase (
- IN UINTN Address,
- IN UINTN *NumBytes
- )
-{
- ASSERT(FALSE);
- return EFI_SUCCESS;
-}
-
diff --git a/Platform/Intel/MinPlatformPkg/Flash/Library/SpiFlashCommonLi=
bNull/SpiFlashCommonLibNull.inf b/Platform/Intel/MinPlatformPkg/Flash/Lib=
rary/SpiFlashCommonLibNull/SpiFlashCommonLibNull.inf
deleted file mode 100644
index 75ef1cb921df..000000000000
--- a/Platform/Intel/MinPlatformPkg/Flash/Library/SpiFlashCommonLibNull/S=
piFlashCommonLibNull.inf
+++ /dev/null
@@ -1,29 +0,0 @@
-### @file
-# NULL instance of Spi Flash Common Library Class
-#
-# Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
-#
-# SPDX-License-Identifier: BSD-2-Clause-Patent
-#
-###
-
-[Defines]
- INF_VERSION =3D 0x00010017
- BASE_NAME =3D SpiFlashCommonLibNull
- FILE_GUID =3D F35BBEE7-A681-443E-BB15-07AF9FABBDE=
D
- VERSION_STRING =3D 1.0
- MODULE_TYPE =3D BASE
- LIBRARY_CLASS =3D SpiFlashCommonLib
-#
-# The following information is for reference only and not required by th=
e build tools.
-#
-# VALID_ARCHITECTURES =3D IA32 X64
-#
-
-[LibraryClasses]
-
-[Packages]
- MdePkg/MdePkg.dec
-
-[Sources]
- SpiFlashCommonLibNull.c
diff --git a/Platform/Intel/MinPlatformPkg/Include/Library/SpiFlashCommon=
Lib.h b/Platform/Intel/MinPlatformPkg/Include/Library/SpiFlashCommonLib.h
deleted file mode 100644
index 0c5e72258c2d..000000000000
--- a/Platform/Intel/MinPlatformPkg/Include/Library/SpiFlashCommonLib.h
+++ /dev/null
@@ -1,98 +0,0 @@
-/** @file
- The header file includes the common header files, defines
- internal structure and functions used by SpiFlashCommonLib.
-
-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
-SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#ifndef __SPI_FLASH_COMMON_LIB_H__
-#define __SPI_FLASH_COMMON_LIB_H__
-
-#include <Uefi.h>
-#include <Library/BaseLib.h>
-#include <Library/PcdLib.h>
-#include <Library/DebugLib.h>
-#include <Library/BaseMemoryLib.h>
-#include <Library/MemoryAllocationLib.h>
-#include <Library/UefiDriverEntryPoint.h>
-#include <Library/UefiBootServicesTableLib.h>
-
-#define SECTOR_SIZE_4KB 0x1000 // Common 4kBytes sector size
-/**
- Enable block protection on the Serial Flash device.
-
- @retval EFI_SUCCESS Opertion is successful.
- @retval EFI_DEVICE_ERROR If there is any device errors.
-
-**/
-EFI_STATUS
-EFIAPI
-SpiFlashLock (
- VOID
- );
-
-/**
- Read NumBytes bytes of data from the address specified by
- PAddress into Buffer.
-
- @param[in] Address The starting physical address of the rea=
d.
- @param[in,out] NumBytes On input, the number of bytes to read. O=
n output, the number
- of bytes actually read.
- @param[out] Buffer The destination data buffer for the read=
.
-
- @retval EFI_SUCCESS Opertion is successful.
- @retval EFI_DEVICE_ERROR If there is any device errors.
-
-**/
-EFI_STATUS
-EFIAPI
-SpiFlashRead (
- IN UINTN Address,
- IN OUT UINT32 *NumBytes,
- OUT UINT8 *Buffer
- );
-
-/**
- Write NumBytes bytes of data from Buffer to the address specified by
- PAddresss.
-
- @param[in] Address The starting physical address of the w=
rite.
- @param[in,out] NumBytes On input, the number of bytes to write=
. On output,
- the actual number of bytes written.
- @param[in] Buffer The source data buffer for the write.
-
- @retval EFI_SUCCESS Opertion is successful.
- @retval EFI_DEVICE_ERROR If there is any device errors.
-
-**/
-EFI_STATUS
-EFIAPI
-SpiFlashWrite (
- IN UINTN Address,
- IN OUT UINT32 *NumBytes,
- IN UINT8 *Buffer
- );
-
-/**
- Erase the block starting at Address.
-
- @param[in] Address The starting physical address of the block=
to be erased.
- This library assume that caller garantee t=
hat the PAddress
- is at the starting address of this block.
- @param[in] NumBytes On input, the number of bytes of the logic=
al block to be erased.
- On output, the actual number of bytes eras=
ed.
-
- @retval EFI_SUCCESS. Opertion is successful.
- @retval EFI_DEVICE_ERROR If there is any device errors.
-
-**/
-EFI_STATUS
-EFIAPI
-SpiFlashBlockErase (
- IN UINTN Address,
- IN UINTN *NumBytes
- );
-
-#endif
diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec b/Platform/=
Intel/MinPlatformPkg/MinPlatformPkg.dec
index bcb42f0ef9e6..3bcd7c854369 100644
--- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
+++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
@@ -62,8 +62,6 @@ [LibraryClasses]
SiliconPolicyInitLib|Include/Library/SiliconPolicyInitLib.h
SiliconPolicyUpdateLib|Include/Library/SiliconPolicyUpdateLib.h
=20
- SpiFlashCommonLib|Include/Library/SpiFlashCommonLib.h
-
BoardInitLib|Include/Library/BoardInitLib.h
MultiBoardInitSupportLib|Include/Library/MultiBoardInitSupportLib.h
SecBoardInitLib|Include/Library/SecBoardInitLib.h
diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc b/Platform/=
Intel/MinPlatformPkg/MinPlatformPkg.dsc
index d43131bf9cec..cf7870a66140 100644
--- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc
+++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc
@@ -110,7 +110,6 @@ [LibraryClasses.common.DXE_DRIVER]
TpmPlatformHierarchyLib|MinPlatformPkg/Tcg/Library/PeiDxeTpmPlatformHi=
erarchyLib/PeiDxeTpmPlatformHierarchyLib.inf
=20
[LibraryClasses.common.DXE_SMM_DRIVER]
- SpiFlashCommonLib|MinPlatformPkg/Flash/Library/SpiFlashCommonLibNull/S=
piFlashCommonLibNull.inf
TestPointCheckLib|MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTes=
tPointCheckLib.inf
TestPointLib|MinPlatformPkg/Test/Library/TestPointLib/SmmTestPointLib.=
inf
=20
@@ -119,7 +118,6 @@ [LibraryClasses.common.MM_STANDALONE]
MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocati=
onLib/StandaloneMmMemoryAllocationLib.inf
MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Standal=
oneMmServicesTableLib.inf
PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
- SpiFlashCommonLib|MinPlatformPkg/Flash/Library/SpiFlashCommonLibNull/S=
piFlashCommonLibNull.inf
StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoi=
nt/StandaloneMmDriverEntryPoint.inf
VariableReadLib|MinPlatformPkg/Library/SmmVariableReadLib/StandaloneMm=
VariableReadLib.inf
VariableWriteLib|MinPlatformPkg/Library/SmmVariableWriteLib/Standalone=
MmVariableWriteLib.inf
@@ -162,8 +160,6 @@ [Components]
MinPlatformPkg/Bds/Library/BoardBootManagerLibNull/BoardBootManagerLib=
Null.inf
MinPlatformPkg/Bds/Library/DxePlatformBootManagerLib/DxePlatformBootMa=
nagerLib.inf
=20
- MinPlatformPkg/Flash/Library/SpiFlashCommonLibNull/SpiFlashCommonLibNu=
ll.inf
-
MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf
MinPlatformPkg/FspWrapper/Library/PeiFspWrapperHobProcessLib/PeiFspWra=
pperHobProcessLib.inf
MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWr=
apperPlatformSecLib.inf
--=20
2.28.0.windows.1


[edk2-platforms][PATCH v5 23/46] TigerlakeOpenBoardPkg: Update SpiFvbService & SpiFlashCommonLib

Michael Kubacki
 

From: Michael Kubacki <michael.kubacki@...>

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

Updates TigerlakeOpenBoardPkg to use the SmmSpiFlashCommonLib
instance in IntelSiliconPkg and the SpiFvbServiceSmm driver in
IntelSiliconPkg.

Cc: Sai Chaganty <rangasai.v.chaganty@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Heng Luo <heng.luo@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@...>
---
Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.dsc | 7 =
+++++--
Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.fdf | 2 =
+-
2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoard=
Pkg.dsc b/Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg=
.dsc
index 1adf63403450..758b966fee81 100644
--- a/Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.dsc
+++ b/Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.dsc
@@ -173,7 +173,7 @@ [LibraryClasses.X64]
!include $(PLATFORM_SI_PACKAGE)/SiPkgDxeLib.dsc
=20
[LibraryClasses.X64.DXE_SMM_DRIVER]
- SpiFlashCommonLib|$(PLATFORM_BOARD_PACKAGE)/Library/SmmSpiFlashCommonL=
ib/SmmSpiFlashCommonLib.inf
+ SpiFlashCommonLib|IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SmmSpiF=
lashCommonLib.inf
!if $(TARGET) =3D=3D DEBUG
TestPointCheckLib|$(PLATFORM_PACKAGE)/Test/Library/TestPointCheckLib/S=
mmTestPointCheckLib.inf
!endif
@@ -297,6 +297,10 @@ [Components.X64]
!include $(PLATFORM_SI_PACKAGE)/SiPkgDxe.dsc
$(PLATFORM_SI_BIN_PACKAGE)/Microcode/MicrocodeUpdates.inf
=20
+!if gMinPlatformPkgTokenSpaceGuid.PcdBootToShellOnly =3D=3D FALSE
+ IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
+!endif
+
#
# SmmAccess
#
@@ -326,7 +330,6 @@ [Components.X64]
NULL|$(PROJECT)/Library/BoardAcpiLib/SmmMultiBoardAcpiSupportLib.i=
nf
}
=20
- $(PLATFORM_PACKAGE)/Flash/SpiFvbService/SpiFvbServiceSmm.inf
$(PLATFORM_PACKAGE)/PlatformInit/PlatformInitSmm/PlatformInitSmm.inf
=20
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf {
diff --git a/Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoard=
Pkg.fdf b/Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg=
.fdf
index e3b2f048524c..b802c2167d06 100644
--- a/Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.fdf
+++ b/Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.fdf
@@ -434,7 +434,7 @@ [FV.FvOsBootUncompact]
!if gMinPlatformPkgTokenSpaceGuid.PcdBootToShellOnly =3D=3D FALSE
INF $(PLATFORM_PACKAGE)/PlatformInit/SiliconPolicyDxe/SiliconPolicyDxe.=
inf
INF $(PLATFORM_PACKAGE)/PlatformInit/PlatformInitSmm/PlatformInitSmm.in=
f
-INF $(PLATFORM_PACKAGE)/Flash/SpiFvbService/SpiFvbServiceSmm.inf
+INF IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
=20
INF UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
INF $(PLATFORM_PACKAGE)/Acpi/AcpiTables/AcpiPlatform.inf
--=20
2.28.0.windows.1


Re: [RFC PATCH v4 00/27] Add AMD Secure Nested Paging (SEV-SNP) support

Brijesh Singh
 

On 7/28/21 9:22 PM, Yao, Jiewen wrote:
Hi Brijesh
Thanks for the patient.
Most of my comment focus on the *common* part, and *interface* between SEV and common code.
I will leave you to decide the detailed SEV specific implementation.
Thank you Jiewen for your feedback. I will try to address the comments in next version.

Patch-04:
Can we use consistent naming conversion?
We have PcdOvmfSecGhcbPageTableBase, PcdOvmfSecGhcbBase, PcdSevLaunchSecretBase. Now we are adding PcdOvmfSnpSecretsBase.
Can we change PcdOvmfSnpSecretsBase to PcdSevSnpSecretsBase?
Or we change PcdSevLaunchSecretBase to PcdOvmfSevLaunchSecretBase?
I don't know why we choose "Ovmf" from the LaunchSecretsBase PCD. I thought PCD's specific the Uefi typically contains the Ovmf name. Maybe we can fix the LaunchSecretsBase to match with the name. I will do that as a separate patch.

Patch-05:
Ditto. Naming convention.
Patch-06:
I have recommendation to Min, to separate SEV stuff to a standalone file from ResetVectorVtf0.asm.
Intel can add TDX stuff to a standalone file, and make it included by ResetVectorVtf0.asm.
I am not sure if you want to do it, or you leave Min to do it.
For the SEV stuff, I will do it myself so that I can test it as well :)

Patch-07:
Same naming convention issue. See #04 and #05.
Patch-08:
I hope we can move all below code to AmdSev.asm, such as PostPageTableHookSev().
Then the PageTable64.asm can be SEV/TDX agnostic.
I am not sure if you want to do it, or you leave Min to do it.
==============
;
; Clear the encryption bit from the GHCB entry
;
mov ecx, (GHCB_BASE & 0x1F_FFFF) >> 12
mov [ecx * 8 + GHCB_PT_ADDR + 4], strict dword 0
mov ecx, GHCB_SIZE / 4
xor eax, eax
clearGhcbMemoryLoop:
mov dword[ecx * 4 + GHCB_BASE - 4], eax
loop clearGhcbMemoryLoop
;
; The page table built above cleared the memory encryption mask from the
; GHCB_BASE (aka made it shared). When SEV-SNP is enabled, to maintain
; the security guarantees, the page state transition from private to
; shared must go through the page invalidation steps. Invalidate the
; memory range before loading the page table below.
;
; NOTE: the invalidation must happen after zeroing the GHCB memory. This
; is because, in the 32-bit mode all the access are considered private.
; The invalidation before the zero'ing will cause a #VC.
;
OneTimeCall InvalidateGHCBPage
==============
I will try to see if I can move that out as well.

Patch-10:
I am not UEFI CPU package maintainer. But I do have a little concern to add more PcdXxxIsEnable style PCD, especially when they are mutual exclusive (like TDX v.s SEV).
If we follow this pattern, we will have PcdSevEsIsEnabled, PcdSevSnpIsEnabled, PcdSevFutureIsEnabled, PcdTdxIsEnabled, PcdTdxFutureIsEnabled, ... that will be an endless list.
If possible, I suggest define one PcdConfidentialComputingType - indicate Legacy, SEV, TDX.
There are certain things which are applicable to SEV-ES and not for the SEV-SNP and vice versa. I am not oppose to create a generic helper e.g

enum {
AmdSev,
AmdSevEs,
AmdSevSnp,
IntelTdx,
IntelSgx,
..
..
};

bool EncryptedGuestFeatureEnabled(enum type);

But I think some of this can be done later as well.

Patch-12:
Can we move all SEV stuff to a standalone file, such as AmdSev.c?
I am not sure if you want to do it, or you leave Min to do it.
Yes, I can do it.

Patch-18:
If we have a standalone AmdSev.c (#12), then we can move the function to that file, and only leave a hook call to SEV.
I will try to consolidate it in AmdSev.c

Patch-23:
This is UEFI CPU package update. I am thinking if we can follow same patter to move all SEV stuff to a standalone file, such as AmdSev.c, AmdSev.asm.
In the future, we may add TDX stuff as well.
Patch-26:
Same comment as #23.
Patch-27:
Can we move that function to a standalone AmdSev.c ?
Patch-28:
Would you please describe more on what is ConfidentialComputingBlob ?
While launching the SEV-SNP guests, the hypervisor may need to provide
some additional information during the guest boot. When booting under the EFI based BIOS, the EFI configuration table contains an entry for the confidential computing blob that contains the required information. The Linux kernel will lookup for this EFI table during the boot to locate the secrets and cpuid page.


Is that generic concept? Or SEV specific thing?
Its designed as a generic and the current only SEV-SNP provides it.
Who is consumer?
Any guest kernel (window or Linux)

What is difference between ConfidentialComputingSecret and ConfidentialComputingBlob ? When to use which?
The confidentialComputingSecrets contains the secrets keys where the CCBlob contains the information which maybe used during the boot.

You can see some more about it on my kernel patches:

https://lore.kernel.org/lkml/20210707181506.30489-26-brijesh.singh@amd.com/

I can understand how TDX use ConfidentialComputingSecret, but how do you expect TDX use ConfidentialComputingBlob (if it is a generic concept) ?
I think in the case of TDX , the information needed during the boot is provided through the ACPI tables but in SEV-SNP those are provided throught the CCBlob. In the contianer environement there will be no EFI so in that case the Blob will be passed to the boot loader setup data. If required then TDX can use it to pass the boot information.

thanks


[edk2-platforms][PATCH v5 15/46] WhiskeylakeOpenBoardPkg: Use IntelSiliconPkg BIOS area and ucode PCDs

Michael Kubacki
 

From: Michael Kubacki <michael.kubacki@...>

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

Updates PCDs to use the IntelSiliconPkg PCD tokenspace now that the
PCDs are declared in IntelSiliconPkg.dec.

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@...>
Reviewed-by: Chasel Chiu <chasel.chiu@...>
---
Platform/Intel/WhiskeylakeOpenBoardPkg/BiosInfo/BiosInfo.inf =
| 4 +--
Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/PolicyInitDxe/PolicyInitDx=
e.inf | 4 +--
Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/Include/Fdf/FlashMapIncl=
ude.fdf | 4 +--
Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/Library/BoardInitLib/Pei=
MultiBoardInitPreMemLib.inf | 2 +-
Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf =
| 36 ++++++++++----------
Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/Include/Fdf/Flash=
MapInclude.fdf | 4 +--
Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.fdf =
| 36 ++++++++++----------
7 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/Platform/Intel/WhiskeylakeOpenBoardPkg/BiosInfo/BiosInfo.inf=
b/Platform/Intel/WhiskeylakeOpenBoardPkg/BiosInfo/BiosInfo.inf
index a9687d93dee1..0a807ad84f4d 100644
--- a/Platform/Intel/WhiskeylakeOpenBoardPkg/BiosInfo/BiosInfo.inf
+++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/BiosInfo/BiosInfo.inf
@@ -36,8 +36,8 @@ [Packages]
MinPlatformPkg/MinPlatformPkg.dec
=20
[Pcd]
- gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CON=
SUMES
- gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CON=
SUMES
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =
## CONSUMES
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =
## CONSUMES
=20
[Sources]
BiosInfo.c
diff --git a/Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/PolicyInitDxe/=
PolicyInitDxe.inf b/Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/PolicyI=
nitDxe/PolicyInitDxe.inf
index 3233375d6568..537d507ed7d6 100644
--- a/Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/PolicyInitDxe/PolicyI=
nitDxe.inf
+++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/PolicyInitDxe/PolicyI=
nitDxe.inf
@@ -47,8 +47,8 @@ [Packages]
=20
[Pcd]
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress =
## CONSUMES
- gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =
## CONSUMES
- gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =
## CONSUMES
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =
## CONSUMES
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =
## CONSUMES
gWhiskeylakeOpenBoardPkgTokenSpaceGuid.PcdIntelGopEnable
gWhiskeylakeOpenBoardPkgTokenSpaceGuid.PcdPlatformFlavor
gWhiskeylakeOpenBoardPkgTokenSpaceGuid.PcdPlatformType
diff --git a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/Include/Fdf/=
FlashMapInclude.fdf b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/Inc=
lude/Fdf/FlashMapInclude.fdf
index f7aa730ae7d2..5895eebc5a79 100644
--- a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/Include/Fdf/FlashMa=
pInclude.fdf
+++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/Include/Fdf/FlashMa=
pInclude.fdf
@@ -38,8 +38,8 @@
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPostMemorySize =3D =
0x00170000 #
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspSOffset =3D =
0x00490000 # Flash addr (0xFFDE0000)
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspSSize =3D =
0x00070000 #
-SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset =3D =
0x00500000 # Flash addr (0xFFE50000)
-SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =3D =
0x00050000 #
+SET gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset =3D =
0x00500000 # Flash addr (0xFFE50000)
+SET gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =3D =
0x00050000 #
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspMOffset =3D =
0x00550000 # Flash addr (0xFFEA0000)
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspMSize =3D =
0x000EA000 #
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspTOffset =3D =
0x0063A000 # Flash addr (0xFFF8A000)
diff --git a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/Library/Boar=
dInitLib/PeiMultiBoardInitPreMemLib.inf b/Platform/Intel/WhiskeylakeOpenB=
oardPkg/UpXtreme/Library/BoardInitLib/PeiMultiBoardInitPreMemLib.inf
index 2903bdacaebd..091d2118c7b3 100644
--- a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/Library/BoardInitLi=
b/PeiMultiBoardInitPreMemLib.inf
+++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/Library/BoardInitLi=
b/PeiMultiBoardInitPreMemLib.inf
@@ -293,7 +293,7 @@ [Pcd]
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspTSize
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspMSize
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspSSize
- gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize
=20
[FixedPcd]
gSiPkgTokenSpaceGuid.PcdMchBaseAddress ## CONSUMES
diff --git a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg=
.fdf b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf
index 22fbfc99f0f0..8aea5aa475a0 100644
--- a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf
+++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf
@@ -31,8 +31,8 @@ [FD.UpXtreme]
# assigned with PCD values. Instead, it uses the definitions for its var=
iety, which
# are FLASH_BASE, FLASH_SIZE, FLASH_BLOCK_SIZE and FLASH_NUM_BLOCKS.
#
-BaseAddress =3D $(FLASH_BASE) | gSiPkgTokenSpaceGuid.PcdBiosAreaBaseAd=
dress #The base address of the FLASH Device.
-Size =3D $(FLASH_SIZE) | gSiPkgTokenSpaceGuid.PcdBiosSize =
#The size in bytes of the FLASH Device
+BaseAddress =3D $(FLASH_BASE) | gIntelSiliconPkgTokenSpaceGuid.PcdBios=
AreaBaseAddress #The base address of the FLASH Device.
+Size =3D $(FLASH_SIZE) | gIntelSiliconPkgTokenSpaceGuid.PcdBios=
Size #The size in bytes of the FLASH Device
ErasePolarity =3D 1
BlockSize =3D $(FLASH_BLOCK_SIZE)
NumBlocks =3D $(FLASH_NUM_BLOCKS)
@@ -43,21 +43,21 @@ [FD.UpXtreme]
# Set FLASH_REGION_FV_RECOVERY_OFFSET to PcdNemCodeCacheBase, because ma=
cro expression is not supported.
# So, PlatformSecLib uses PcdBiosAreaBaseAddress + PcdNemCodeCacheBase t=
o get the real CodeCache base address.
SET gSiPkgTokenSpaceGuid.PcdNemCodeCacheBase =3D $(gMinPlatformPkgTokenS=
paceGuid.PcdFlashFvPreMemoryOffset)
-SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =3D $(gSiPkgTokenSpaceG=
uid.PcdBiosAreaBaseAddress) + $(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvO=
ffset)
-SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =3D $(gSiPkgTokenSpaceG=
uid.PcdFlashMicrocodeFvSize)
-SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress =3D $(gSiPkgTo=
kenSpaceGuid.PcdFlashMicrocodeFvBase) + 0x60
-SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =3D $(gSiPk=
gTokenSpaceGuid.PcdFlashMicrocodeFvSize) - 0x60
+SET gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =3D $(gIntelS=
iliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gIntelSiliconPkgToken=
SpaceGuid.PcdFlashMicrocodeFvOffset)
+SET gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =3D $(gIntelS=
iliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize)
+SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress =3D $(gIntelSi=
liconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase) + 0x60
+SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =3D $(gInte=
lSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize) - 0x60
SET gMinPlatformPkgTokenSpaceGuid.PcdMicrocodeOffsetInFv =3D 0x60
-SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase =3D gSiPkgT=
okenSpaceGuid.PcdFlashMicrocodeFvBase
-SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize =3D gSiPkgT=
okenSpaceGuid.PcdFlashMicrocodeFvSize
-SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset =3D gSiPkgT=
okenSpaceGuid.PcdFlashMicrocodeFvOffset
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheAddress =3D gSiPkgT=
okenSpaceGuid.PcdBiosAreaBaseAddress
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheSize =3D gSiPkgT=
okenSpaceGuid.PcdBiosSize
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress =3D $(gSiPk=
gTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgTokenSpaceGuid=
.PcdFlashFvFspTOffset)
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress =3D $(gSiPk=
gTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgTokenSpaceGuid=
.PcdFlashFvFspMOffset)
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress =3D $(gSiPk=
gTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgTokenSpaceGuid=
.PcdFlashFvFspSOffset)
-SET gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaBaseAddress =3D gSiPkgT=
okenSpaceGuid.PcdBiosAreaBaseAddress
-SET gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize =3D gSiPkgT=
okenSpaceGuid.PcdBiosSize
+SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase
+SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize
+SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset
+SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheAddress =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress
+SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheSize =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdBiosSize
+SET gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress =3D $(gInte=
lSiliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgToke=
nSpaceGuid.PcdFlashFvFspTOffset)
+SET gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress =3D $(gInte=
lSiliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgToke=
nSpaceGuid.PcdFlashFvFspMOffset)
+SET gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress =3D $(gInte=
lSiliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgToke=
nSpaceGuid.PcdFlashFvFspSOffset)
+SET gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaBaseAddress =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress
+SET gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdBiosSize
########################################################################=
########
#
# Following are lists of FD Region layout which correspond to the locati=
ons of different
@@ -158,8 +158,8 @@ [FD.UpXtreme]
# FSP_S Section
FILE =3D $(PLATFORM_FSP_BIN_PACKAGE)/Fsp_Rebased_S.fd
=20
-gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset|gSiPkgTokenSpaceGuid.PcdF=
lashMicrocodeFvSize
-gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase|gSiPkgTokenSpaceGuid.PcdFla=
shMicrocodeFvSize
+gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset|gIntelSiliconPk=
gTokenSpaceGuid.PcdFlashMicrocodeFvSize
+gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase|gIntelSiliconPkgT=
okenSpaceGuid.PcdFlashMicrocodeFvSize
#Microcode
FV =3D FvMicrocode
=20
diff --git a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/Inclu=
de/Fdf/FlashMapInclude.fdf b/Platform/Intel/WhiskeylakeOpenBoardPkg/Whisk=
eylakeURvp/Include/Fdf/FlashMapInclude.fdf
index e0db38194211..586e3488c2a7 100644
--- a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/Include/Fdf/=
FlashMapInclude.fdf
+++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/Include/Fdf/=
FlashMapInclude.fdf
@@ -34,8 +34,8 @@
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvUefiBootSize =3D =
0x00190000 #
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPostMemoryOffset =3D =
0x00320000 # Flash addr (0xFFB20000)
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPostMemorySize =3D =
0x00170000 #
-SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset =3D =
0x00490000 # Flash addr (0xFFC90000)
-SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =3D =
0x000B0000 #
+SET gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset =3D =
0x00490000 # Flash addr (0xFFC90000)
+SET gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =3D =
0x000B0000 #
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspSOffset =3D =
0x00540000 # Flash addr (0xFFD40000)
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspSSize =3D =
0x00070000 #
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspMOffset =3D =
0x005B0000 # Flash addr (0xFFDB0000)
diff --git a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenB=
oardPkg.fdf b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/Open=
BoardPkg.fdf
index 1ab8c137924e..f0601984338c 100644
--- a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg=
.fdf
+++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg=
.fdf
@@ -31,8 +31,8 @@ [FD.WhiskeylakeURvp]
# assigned with PCD values. Instead, it uses the definitions for its var=
iety, which
# are FLASH_BASE, FLASH_SIZE, FLASH_BLOCK_SIZE and FLASH_NUM_BLOCKS.
#
-BaseAddress =3D $(FLASH_BASE) | gSiPkgTokenSpaceGuid.PcdBiosAreaBaseAd=
dress #The base address of the FLASH Device.
-Size =3D $(FLASH_SIZE) | gSiPkgTokenSpaceGuid.PcdBiosSize =
#The size in bytes of the FLASH Device
+BaseAddress =3D $(FLASH_BASE) | gIntelSiliconPkgTokenSpaceGuid.PcdBios=
AreaBaseAddress #The base address of the FLASH Device.
+Size =3D $(FLASH_SIZE) | gIntelSiliconPkgTokenSpaceGuid.PcdBios=
Size #The size in bytes of the FLASH Device
ErasePolarity =3D 1
BlockSize =3D $(FLASH_BLOCK_SIZE)
NumBlocks =3D $(FLASH_NUM_BLOCKS)
@@ -43,21 +43,21 @@ [FD.WhiskeylakeURvp]
# Set FLASH_REGION_FV_RECOVERY_OFFSET to PcdNemCodeCacheBase, because ma=
cro expression is not supported.
# So, PlatformSecLib uses PcdBiosAreaBaseAddress + PcdNemCodeCacheBase t=
o get the real CodeCache base address.
SET gSiPkgTokenSpaceGuid.PcdNemCodeCacheBase =3D $(gMinPlatformPkgTokenS=
paceGuid.PcdFlashFvPreMemoryOffset)
-SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =3D $(gSiPkgTokenSpaceG=
uid.PcdBiosAreaBaseAddress) + $(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvO=
ffset)
-SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =3D $(gSiPkgTokenSpaceG=
uid.PcdFlashMicrocodeFvSize)
-SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress =3D $(gSiPkgTo=
kenSpaceGuid.PcdFlashMicrocodeFvBase) + 0x60
-SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =3D $(gSiPk=
gTokenSpaceGuid.PcdFlashMicrocodeFvSize) - 0x60
+SET gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =3D $(gIntelS=
iliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gIntelSiliconPkgToken=
SpaceGuid.PcdFlashMicrocodeFvOffset)
+SET gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =3D $(gIntelS=
iliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize)
+SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress =3D $(gIntelSi=
liconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase) + 0x60
+SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =3D $(gInte=
lSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize) - 0x60
SET gMinPlatformPkgTokenSpaceGuid.PcdMicrocodeOffsetInFv =3D 0x60
-SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase =3D gSiPkgT=
okenSpaceGuid.PcdFlashMicrocodeFvBase
-SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize =3D gSiPkgT=
okenSpaceGuid.PcdFlashMicrocodeFvSize
-SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset =3D gSiPkgT=
okenSpaceGuid.PcdFlashMicrocodeFvOffset
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheAddress =3D gSiPkgT=
okenSpaceGuid.PcdBiosAreaBaseAddress
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheSize =3D gSiPkgT=
okenSpaceGuid.PcdBiosSize
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress =3D $(gSiPk=
gTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgTokenSpaceGuid=
.PcdFlashFvFspTOffset)
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress =3D $(gSiPk=
gTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgTokenSpaceGuid=
.PcdFlashFvFspMOffset)
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress =3D $(gSiPk=
gTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgTokenSpaceGuid=
.PcdFlashFvFspSOffset)
-SET gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaBaseAddress =3D gSiPkgT=
okenSpaceGuid.PcdBiosAreaBaseAddress
-SET gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize =3D gSiPkgT=
okenSpaceGuid.PcdBiosSize
+SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase
+SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize
+SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset
+SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheAddress =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress
+SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheSize =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdBiosSize
+SET gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress =3D $(gInte=
lSiliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgToke=
nSpaceGuid.PcdFlashFvFspTOffset)
+SET gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress =3D $(gInte=
lSiliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgToke=
nSpaceGuid.PcdFlashFvFspMOffset)
+SET gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress =3D $(gInte=
lSiliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgToke=
nSpaceGuid.PcdFlashFvFspSOffset)
+SET gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaBaseAddress =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress
+SET gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdBiosSize
########################################################################=
########
#
# Following are lists of FD Region layout which correspond to the locati=
ons of different
@@ -153,8 +153,8 @@ [FD.WhiskeylakeURvp]
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPostMemoryBase|gMinPlatformPkgTo=
kenSpaceGuid.PcdFlashFvPostMemorySize
FV =3D FvPostMemory
=20
-gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset|gSiPkgTokenSpaceGuid.PcdF=
lashMicrocodeFvSize
-gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase|gSiPkgTokenSpaceGuid.PcdFla=
shMicrocodeFvSize
+gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset|gIntelSiliconPk=
gTokenSpaceGuid.PcdFlashMicrocodeFvSize
+gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase|gIntelSiliconPkgT=
okenSpaceGuid.PcdFlashMicrocodeFvSize
#Microcode
FV =3D FvMicrocode
=20
--=20
2.28.0.windows.1


[edk2-platforms][PATCH v5 11/46] CometlakeOpenBoardPkg: Use IntelSiliconPkg BIOS area and ucode PCDs

Michael Kubacki
 

From: Michael Kubacki <michael.kubacki@...>

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

Updates PCDs to use the IntelSiliconPkg PCD tokenspace now that the
PCDs are declared in IntelSiliconPkg.dec.

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Cc: Deepika Kethi Reddy <deepika.kethi.reddy@...>
Cc: Kathappan Esakkithevar <kathappan.esakkithevar@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@...>
Reviewed-by: Chasel Chiu <chasel.chiu@...>
---
Platform/Intel/CometlakeOpenBoardPkg/BiosInfo/BiosInfo.inf =
| 4 +--
Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/Include/Fdf/FlashMapI=
nclude.fdf | 4 +--
Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.fdf =
| 36 ++++++++++----------
Platform/Intel/CometlakeOpenBoardPkg/Policy/PolicyInitDxe/PolicyInitDxe.=
inf | 4 +--
4 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/Platform/Intel/CometlakeOpenBoardPkg/BiosInfo/BiosInfo.inf b=
/Platform/Intel/CometlakeOpenBoardPkg/BiosInfo/BiosInfo.inf
index 9208aeda5d2a..6ca0ada751f6 100644
--- a/Platform/Intel/CometlakeOpenBoardPkg/BiosInfo/BiosInfo.inf
+++ b/Platform/Intel/CometlakeOpenBoardPkg/BiosInfo/BiosInfo.inf
@@ -36,8 +36,8 @@ [Packages]
MinPlatformPkg/MinPlatformPkg.dec
=20
[Pcd]
- gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CON=
SUMES
- gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CON=
SUMES
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CON=
SUMES
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CON=
SUMES
=20
[Sources]
BiosInfo.c
diff --git a/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/Include/F=
df/FlashMapInclude.fdf b/Platform/Intel/CometlakeOpenBoardPkg/CometlakeUR=
vp/Include/Fdf/FlashMapInclude.fdf
index d9959a79d0bb..7d2f4b2c0cb2 100644
--- a/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/Include/Fdf/Flas=
hMapInclude.fdf
+++ b/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/Include/Fdf/Flas=
hMapInclude.fdf
@@ -34,8 +34,8 @@
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvUefiBootSize =3D =
0x00190000 #
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPostMemoryOffset =3D =
0x00320000 # Flash addr (0xFFB20000)
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPostMemorySize =3D =
0x00170000 #
-SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset =3D =
0x00490000 # Flash addr (0xFFC90000)
-SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =3D =
0x000B0000 #
+SET gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset =3D =
0x00490000 # Flash addr (0xFFC90000)
+SET gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =3D =
0x000B0000 #
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspSOffset =3D =
0x00540000 # Flash addr (0xFFD40000)
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspSSize =3D =
0x00070000 #
SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvFspMOffset =3D =
0x005B0000 # Flash addr (0xFFDB0000)
diff --git a/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoard=
Pkg.fdf b/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg=
.fdf
index 795cc0da75d8..6397d80d3895 100644
--- a/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.fdf
+++ b/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.fdf
@@ -31,8 +31,8 @@ [FD.CometlakeURvp]
# assigned with PCD values. Instead, it uses the definitions for its var=
iety, which
# are FLASH_BASE, FLASH_SIZE, FLASH_BLOCK_SIZE and FLASH_NUM_BLOCKS.
#
-BaseAddress =3D $(FLASH_BASE) | gSiPkgTokenSpaceGuid.PcdBiosAreaBaseAd=
dress #The base address of the FLASH Device.
-Size =3D $(FLASH_SIZE) | gSiPkgTokenSpaceGuid.PcdBiosSize =
#The size in bytes of the FLASH Device
+BaseAddress =3D $(FLASH_BASE) | gIntelSiliconPkgTokenSpaceGuid.PcdBios=
AreaBaseAddress #The base address of the FLASH Device.
+Size =3D $(FLASH_SIZE) | gIntelSiliconPkgTokenSpaceGuid.PcdBios=
Size #The size in bytes of the FLASH Device
ErasePolarity =3D 1
BlockSize =3D $(FLASH_BLOCK_SIZE)
NumBlocks =3D $(FLASH_NUM_BLOCKS)
@@ -43,21 +43,21 @@ [FD.CometlakeURvp]
# Set FLASH_REGION_FV_RECOVERY_OFFSET to PcdNemCodeCacheBase, because ma=
cro expression is not supported.
# So, PlatformSecLib uses PcdBiosAreaBaseAddress + PcdNemCodeCacheBase t=
o get the real CodeCache base address.
SET gSiPkgTokenSpaceGuid.PcdNemCodeCacheBase =3D $(gMinPlatformPkgTokenS=
paceGuid.PcdFlashFvPreMemoryOffset)
-SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =3D $(gSiPkgTokenSpaceG=
uid.PcdBiosAreaBaseAddress) + $(gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvO=
ffset)
-SET gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =3D $(gSiPkgTokenSpaceG=
uid.PcdFlashMicrocodeFvSize)
-SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress =3D $(gSiPkgTo=
kenSpaceGuid.PcdFlashMicrocodeFvBase) + 0x60
-SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =3D $(gSiPk=
gTokenSpaceGuid.PcdFlashMicrocodeFvSize) - 0x60
+SET gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =3D $(gIntelS=
iliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gIntelSiliconPkgToken=
SpaceGuid.PcdFlashMicrocodeFvOffset)
+SET gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =3D $(gIntelS=
iliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize)
+SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress =3D $(gIntelSi=
liconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase) + 0x60
+SET gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize =3D $(gInte=
lSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize) - 0x60
SET gMinPlatformPkgTokenSpaceGuid.PcdMicrocodeOffsetInFv =3D 0x60
-SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase =3D gSiPkgT=
okenSpaceGuid.PcdFlashMicrocodeFvBase
-SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize =3D gSiPkgT=
okenSpaceGuid.PcdFlashMicrocodeFvSize
-SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset =3D gSiPkgT=
okenSpaceGuid.PcdFlashMicrocodeFvOffset
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheAddress =3D gSiPkgT=
okenSpaceGuid.PcdBiosAreaBaseAddress
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheSize =3D gSiPkgT=
okenSpaceGuid.PcdBiosSize
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress =3D $(gSiPk=
gTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgTokenSpaceGuid=
.PcdFlashFvFspTOffset)
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress =3D $(gSiPk=
gTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgTokenSpaceGuid=
.PcdFlashFvFspMOffset)
-SET gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress =3D $(gSiPk=
gTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgTokenSpaceGuid=
.PcdFlashFvFspSOffset)
-SET gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaBaseAddress =3D gSiPkgT=
okenSpaceGuid.PcdBiosAreaBaseAddress
-SET gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize =3D gSiPkgT=
okenSpaceGuid.PcdBiosSize
+SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase
+SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize
+SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset
+SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheAddress =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress
+SET gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheSize =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdBiosSize
+SET gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress =3D $(gInte=
lSiliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgToke=
nSpaceGuid.PcdFlashFvFspTOffset)
+SET gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress =3D $(gInte=
lSiliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgToke=
nSpaceGuid.PcdFlashFvFspMOffset)
+SET gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress =3D $(gInte=
lSiliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress) + $(gMinPlatformPkgToke=
nSpaceGuid.PcdFlashFvFspSOffset)
+SET gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaBaseAddress =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress
+SET gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize =3D gIntelS=
iliconPkgTokenSpaceGuid.PcdBiosSize
########################################################################=
########
#
# Following are lists of FD Region layout which correspond to the locati=
ons of different
@@ -153,8 +153,8 @@ [FD.CometlakeURvp]
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPostMemoryBase|gMinPlatformPkgTo=
kenSpaceGuid.PcdFlashFvPostMemorySize
FV =3D FvPostMemory
=20
-gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset|gSiPkgTokenSpaceGuid.PcdF=
lashMicrocodeFvSize
-gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase|gSiPkgTokenSpaceGuid.PcdFla=
shMicrocodeFvSize
+gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvOffset|gIntelSiliconPk=
gTokenSpaceGuid.PcdFlashMicrocodeFvSize
+gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase|gIntelSiliconPkgT=
okenSpaceGuid.PcdFlashMicrocodeFvSize
#Microcode
FV =3D FvMicrocode
=20
diff --git a/Platform/Intel/CometlakeOpenBoardPkg/Policy/PolicyInitDxe/Po=
licyInitDxe.inf b/Platform/Intel/CometlakeOpenBoardPkg/Policy/PolicyInitD=
xe/PolicyInitDxe.inf
index 1d09b990b163..abb79c111e0b 100644
--- a/Platform/Intel/CometlakeOpenBoardPkg/Policy/PolicyInitDxe/PolicyIni=
tDxe.inf
+++ b/Platform/Intel/CometlakeOpenBoardPkg/Policy/PolicyInitDxe/PolicyIni=
tDxe.inf
@@ -47,8 +47,8 @@ [Packages]
=20
[Pcd]
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress =
## CONSUMES
- gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =
## CONSUMES
- gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =
## CONSUMES
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase =
## CONSUMES
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize =
## CONSUMES
gCometlakeOpenBoardPkgTokenSpaceGuid.PcdIntelGopEnable
gCometlakeOpenBoardPkgTokenSpaceGuid.PcdPlatformFlavor
gCometlakeOpenBoardPkgTokenSpaceGuid.PcdPlatformType
--=20
2.28.0.windows.1


[edk2-platforms][PATCH v5 10/46] IntelSiliconPkg: Add MM SPI FVB services

Michael Kubacki
 

From: Michael Kubacki <michael.kubacki@...>

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

Adds a Traditional MM and Standalone MM SPI FVB Service driver to
IntelSiliconPkg. These drivers produce the firmware volume block
protocol for SPI flash devices compliant with the Intel Serial
Flash Interface Compatibility Specification.

Cc: Ray Ni <ray.ni@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@...>
---
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c =
| 94 ++
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceC=
ommon.c | 903 ++++++++++++++++++++
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceM=
m.c | 271 ++++++
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceS=
tandaloneMm.c | 32 +
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceT=
raditionalMm.c | 32 +
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceC=
ommon.h | 158 ++++
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceM=
m.h | 22 +
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceS=
mm.inf | 68 ++
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceS=
tandaloneMm.inf | 67 ++
Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc =
| 11 +
10 files changed, 1658 insertions(+)

diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/Fv=
bInfo.c b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbIn=
fo.c
new file mode 100644
index 000000000000..7f2678fa9e5a
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
@@ -0,0 +1,94 @@
+/**@file
+ Defines data structure that is the volume header found.
+ These data is intent to decouple FVB driver with FV header.
+
+Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "SpiFvbServiceCommon.h"
+
+#define FIRMWARE_BLOCK_SIZE 0x10000
+#define FVB_MEDIA_BLOCK_SIZE FIRMWARE_BLOCK_SIZE
+
+#define NV_STORAGE_BASE_ADDRESS FixedPcdGet32(PcdFlashNvStorageVaria=
bleBase)
+#define SYSTEM_NV_BLOCK_NUM ((FixedPcdGet32(PcdFlashNvStorageVar=
iableSize)+ FixedPcdGet32(PcdFlashNvStorageFtwWorkingSize) + FixedPcdGet3=
2(PcdFlashNvStorageFtwSpareSize))/ FVB_MEDIA_BLOCK_SIZE)
+
+typedef struct {
+ EFI_PHYSICAL_ADDRESS BaseAddress;
+ EFI_FIRMWARE_VOLUME_HEADER FvbInfo;
+ EFI_FV_BLOCK_MAP_ENTRY End[1];
+} EFI_FVB2_MEDIA_INFO;
+
+//
+// This data structure contains a template of all correct FV headers, wh=
ich is used to restore
+// Fv header if it's corrupted.
+//
+EFI_FVB2_MEDIA_INFO mPlatformFvbMediaInfo[] =3D {
+ //
+ // Systen NvStorage FVB
+ //
+ {
+ NV_STORAGE_BASE_ADDRESS,
+ {
+ {0,}, //ZeroVector[16]
+ EFI_SYSTEM_NV_DATA_FV_GUID,
+ FVB_MEDIA_BLOCK_SIZE * SYSTEM_NV_BLOCK_NUM,
+ EFI_FVH_SIGNATURE,
+ 0x0004feff, // check MdePkg/Include/Pi/PiFirmwareVolume.h for deta=
ils on EFI_FVB_ATTRIBUTES_2
+ sizeof (EFI_FIRMWARE_VOLUME_HEADER) + sizeof (EFI_FV_BLOCK_MAP_ENT=
RY),
+ 0, //CheckSum which will be calucated dynamically.
+ 0, //ExtHeaderOffset
+ {0,}, //Reserved[1]
+ 2, //Revision
+ {
+ {
+ SYSTEM_NV_BLOCK_NUM,
+ FVB_MEDIA_BLOCK_SIZE,
+ }
+ }
+ },
+ {
+ {
+ 0,
+ 0
+ }
+ }
+ }
+};
+
+EFI_STATUS
+GetFvbInfo (
+ IN EFI_PHYSICAL_ADDRESS FvBaseAddress,
+ OUT EFI_FIRMWARE_VOLUME_HEADER **FvbInfo
+ )
+{
+ UINTN Index;
+ EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
+
+ for (Index =3D 0; Index < sizeof (mPlatformFvbMediaInfo) / sizeof (EFI=
_FVB2_MEDIA_INFO); Index++) {
+ if (mPlatformFvbMediaInfo[Index].BaseAddress =3D=3D FvBaseAddress) {
+ FvHeader =3D &mPlatformFvbMediaInfo[Index].FvbInfo;
+
+ //
+ // Update the checksum value of FV header.
+ //
+ FvHeader->Checksum =3D CalculateCheckSum16 ( (UINT16 *) FvHeader, =
FvHeader->HeaderLength);
+
+ *FvbInfo =3D FvHeader;
+
+ DEBUG ((DEBUG_INFO, "BaseAddr: 0x%lx \n", FvBaseAddress));
+ DEBUG ((DEBUG_INFO, "FvLength: 0x%lx \n", (*FvbInfo)->FvLength));
+ DEBUG ((DEBUG_INFO, "HeaderLength: 0x%x \n", (*FvbInfo)->HeaderLen=
gth));
+ DEBUG ((DEBUG_INFO, "Header Checksum: 0x%X\n", (*FvbInfo)->Checksu=
m));
+ DEBUG ((DEBUG_INFO, "FvBlockMap[0].NumBlocks: 0x%x \n", (*FvbInfo)=
->BlockMap[0].NumBlocks));
+ DEBUG ((DEBUG_INFO, "FvBlockMap[0].BlockLength: 0x%x \n", (*FvbInf=
o)->BlockMap[0].Length));
+ DEBUG ((DEBUG_INFO, "FvBlockMap[1].NumBlocks: 0x%x \n", (*FvbInfo)=
->BlockMap[1].NumBlocks));
+ DEBUG ((DEBUG_INFO, "FvBlockMap[1].BlockLength: 0x%x \n\n", (*FvbI=
nfo)->BlockMap[1].Length));
+
+ return EFI_SUCCESS;
+ }
+ }
+ return EFI_NOT_FOUND;
+}
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/Sp=
iFvbServiceCommon.c b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbS=
ervice/SpiFvbServiceCommon.c
new file mode 100644
index 000000000000..dab818e98087
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSer=
viceCommon.c
@@ -0,0 +1,903 @@
+/** @file
+ Common driver source for several Serial Flash devices
+ which are compliant with the Intel(R) Serial Flash Interface Compatibi=
lity Specification.
+
+Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "SpiFvbServiceCommon.h"
+
+//
+// Global variable for this FVB driver which contains
+// the private data of all firmware volume block instances
+//
+FVB_GLOBAL mFvbModuleGlobal;
+
+//
+// This platform driver knows there are multiple FVs on FD.
+// Now we only provide FVs on Variable region and MicorCode region for p=
erformance issue.
+//
+FV_INFO mPlatformFvBaseAddress[] =3D {
+ {0, 0}, // {FixedPcdGet32(PcdFlashNvStorageVariableBase), FixedPcdGet3=
2(PcdFlashNvStorageVariableSize)},
+ {0, 0}, // {FixedPcdGet32(PcdFlashMicrocodeFvBase), FixedPcdGet32(PcdF=
lashMicrocodeFvSize)},
+ {0, 0}
+};
+
+FV_INFO mPlatformDefaultBaseAddress[] =3D {
+ {0, 0}, // {FixedPcdGet32(PcdFlashNvStorageVariableBase), FixedPcdGet3=
2(PcdFlashNvStorageVariableSize)},
+ {0, 0}, // {FixedPcdGet32(PcdFlashMicrocodeFvBase), FixedPcdGet32(PcdF=
lashMicrocodeFvSize)},
+ {0, 0}
+};
+
+FV_MEMMAP_DEVICE_PATH mFvMemmapDevicePathTemplate =3D {
+ {
+ {
+ HARDWARE_DEVICE_PATH,
+ HW_MEMMAP_DP,
+ {
+ (UINT8)(sizeof (MEMMAP_DEVICE_PATH)),
+ (UINT8)(sizeof (MEMMAP_DEVICE_PATH) >> 8)
+ }
+ },
+ EfiMemoryMappedIO,
+ (EFI_PHYSICAL_ADDRESS) 0,
+ (EFI_PHYSICAL_ADDRESS) 0,
+ },
+ {
+ END_DEVICE_PATH_TYPE,
+ END_ENTIRE_DEVICE_PATH_SUBTYPE,
+ {
+ END_DEVICE_PATH_LENGTH,
+ 0
+ }
+ }
+};
+
+FV_PIWG_DEVICE_PATH mFvPIWGDevicePathTemplate =3D {
+ {
+ {
+ MEDIA_DEVICE_PATH,
+ MEDIA_PIWG_FW_VOL_DP,
+ {
+ (UINT8)(sizeof (MEDIA_FW_VOL_DEVICE_PATH)),
+ (UINT8)(sizeof (MEDIA_FW_VOL_DEVICE_PATH) >> 8)
+ }
+ },
+ { 0 }
+ },
+ {
+ END_DEVICE_PATH_TYPE,
+ END_ENTIRE_DEVICE_PATH_SUBTYPE,
+ {
+ END_DEVICE_PATH_LENGTH,
+ 0
+ }
+ }
+};
+
+//
+// Template structure used when installing FVB protocol
+//
+EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL mFvbProtocolTemplate =3D {
+ FvbProtocolGetAttributes,
+ FvbProtocolSetAttributes,
+ FvbProtocolGetPhysicalAddress,
+ FvbProtocolGetBlockSize,
+ FvbProtocolRead,
+ FvbProtocolWrite,
+ FvbProtocolEraseBlocks,
+ NULL
+};
+
+/**
+ Get the EFI_FVB_ATTRIBUTES_2 of a FV.
+
+ @param[in] FvbInstance The pointer to the EFI_FVB_INSTANCE.
+
+ @return Attributes of the FV identified by FvbInstance.
+
+**/
+EFI_FVB_ATTRIBUTES_2
+FvbGetVolumeAttributes (
+ IN EFI_FVB_INSTANCE *FvbInstance
+ )
+{
+ return FvbInstance->FvHeader.Attributes;
+}
+
+/**
+ Retrieves the starting address of an LBA in an FV. It also
+ return a few other attribut of the FV.
+
+ @param[in] FvbInstance The pointer to the EFI_FVB_INSTANCE.
+ @param[in] Lba The logical block address
+ @param[out] LbaAddress On output, contains the physical starting =
address
+ of the Lba
+ @param[out] LbaLength On output, contains the length of the bloc=
k
+ @param[out] NumOfBlocks A pointer to a caller allocated UINTN in w=
hich the
+ number of consecutive blocks starting with=
Lba is
+ returned. All blocks in this range have a =
size of
+ BlockSize
+
+ @retval EFI_SUCCESS Successfully returns
+ @retval EFI_INVALID_PARAMETER Instance not found
+
+**/
+EFI_STATUS
+FvbGetLbaAddress (
+ IN EFI_FVB_INSTANCE *FvbInstance,
+ IN EFI_LBA Lba,
+ OUT UINTN *LbaAddress,
+ OUT UINTN *LbaLength,
+ OUT UINTN *NumOfBlocks
+ )
+{
+ UINT32 NumBlocks;
+ UINT32 BlockLength;
+ UINTN Offset;
+ EFI_LBA StartLba;
+ EFI_LBA NextLba;
+ EFI_FV_BLOCK_MAP_ENTRY *BlockMap;
+
+ StartLba =3D 0;
+ Offset =3D 0;
+ BlockMap =3D &(FvbInstance->FvHeader.BlockMap[0]);
+
+ //
+ // Parse the blockmap of the FV to find which map entry the Lba belong=
s to
+ //
+ while (TRUE) {
+ NumBlocks =3D BlockMap->NumBlocks;
+ BlockLength =3D BlockMap->Length;
+
+ if ( NumBlocks =3D=3D 0 || BlockLength =3D=3D 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ NextLba =3D StartLba + NumBlocks;
+
+ //
+ // The map entry found
+ //
+ if (Lba >=3D StartLba && Lba < NextLba) {
+ Offset =3D Offset + (UINTN)MultU64x32((Lba - StartLba), BlockLengt=
h);
+ if (LbaAddress ) {
+ *LbaAddress =3D FvbInstance->FvBase + Offset;
+ }
+
+ if (LbaLength ) {
+ *LbaLength =3D BlockLength;
+ }
+
+ if (NumOfBlocks ) {
+ *NumOfBlocks =3D (UINTN)(NextLba - Lba);
+ }
+ return EFI_SUCCESS;
+ }
+
+ StartLba =3D NextLba;
+ Offset =3D Offset + NumBlocks * BlockLength;
+ BlockMap++;
+ }
+}
+
+/**
+ Reads specified number of bytes into a buffer from the specified block=
.
+
+ @param[in] FvbInstance The pointer to the EFI_FVB_INSTA=
NCE
+ @param[in] Lba The logical block address to be =
read from
+ @param[in] BlockOffset Offset into the block at which t=
o begin reading
+ @param[in] NumBytes Pointer that on input contains t=
he total size of
+ the buffer. On output, it contai=
ns the total number
+ of bytes read
+ @param[in] Buffer Pointer to a caller allocated bu=
ffer that will be
+ used to hold the data read
+
+
+ @retval EFI_SUCCESS The firmware volume was read suc=
cessfully and
+ contents are in Buffer
+ @retval EFI_BAD_BUFFER_SIZE Read attempted across a LBA boun=
dary. On output,
+ NumBytes contains the total numb=
er of bytes returned
+ in Buffer
+ @retval EFI_ACCESS_DENIED The firmware volume is in the Re=
adDisabled state
+ @retval EFI_DEVICE_ERROR The block device is not function=
ing correctly and
+ could not be read
+ @retval EFI_INVALID_PARAMETER Instance not found, or NumBytes,=
Buffer are NULL
+
+**/
+EFI_STATUS
+FvbReadBlock (
+ IN EFI_FVB_INSTANCE *FvbInstance,
+ IN EFI_LBA Lba,
+ IN UINTN BlockOffset,
+ IN OUT UINTN *NumBytes,
+ IN UINT8 *Buffer
+ )
+{
+ EFI_FVB_ATTRIBUTES_2 Attributes;
+ UINTN LbaAddress;
+ UINTN LbaLength;
+ EFI_STATUS Status;
+ BOOLEAN BadBufferSize =3D FALSE;
+
+ if ((NumBytes =3D=3D NULL) || (Buffer =3D=3D NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+ if (*NumBytes =3D=3D 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Status =3D FvbGetLbaAddress (FvbInstance, Lba, &LbaAddress, &LbaLength=
, NULL);
+ if (EFI_ERROR(Status)) {
+ return Status;
+ }
+
+ Attributes =3D FvbGetVolumeAttributes (FvbInstance);
+
+ if ((Attributes & EFI_FVB2_READ_STATUS) =3D=3D 0) {
+ return EFI_ACCESS_DENIED;
+ }
+
+ if (BlockOffset > LbaLength) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (LbaLength < (*NumBytes + BlockOffset)) {
+ DEBUG ((DEBUG_INFO,
+ "FvReadBlock: Reducing Numbytes from 0x%x to 0x%x\n",
+ *NumBytes,
+ (UINT32)(LbaLength - BlockOffset))
+ );
+ *NumBytes =3D (UINT32) (LbaLength - BlockOffset);
+ BadBufferSize =3D TRUE;
+ }
+
+ Status =3D SpiFlashRead (LbaAddress + BlockOffset, (UINT32 *)NumBytes,=
Buffer);
+
+ if (!EFI_ERROR (Status) && BadBufferSize) {
+ return EFI_BAD_BUFFER_SIZE;
+ } else {
+ return Status;
+ }
+}
+
+/**
+ Writes specified number of bytes from the input buffer to the block.
+
+ @param[in] FvbInstance The pointer to the EFI_FVB_INSTANCE
+ @param[in] Lba The starting logical block index to =
write to
+ @param[in] BlockOffset Offset into the block at which to be=
gin writing
+ @param[in] NumBytes Pointer that on input contains the t=
otal size of
+ the buffer. On output, it contains t=
he total number
+ of bytes actually written
+ @param[in] Buffer Pointer to a caller allocated buffer=
that contains
+ the source for the write
+ @retval EFI_SUCCESS The firmware volume was written succ=
essfully
+ @retval EFI_BAD_BUFFER_SIZE Write attempted across a LBA boundar=
y. On output,
+ NumBytes contains the total number o=
f bytes
+ actually written
+ @retval EFI_ACCESS_DENIED The firmware volume is in the WriteD=
isabled state
+ @retval EFI_DEVICE_ERROR The block device is not functioning =
correctly and
+ could not be written
+ @retval EFI_INVALID_PARAMETER Instance not found, or NumBytes, Buf=
fer are NULL
+
+**/
+EFI_STATUS
+FvbWriteBlock (
+ IN EFI_FVB_INSTANCE *FvbInstance,
+ IN EFI_LBA Lba,
+ IN UINTN BlockOffset,
+ IN OUT UINTN *NumBytes,
+ IN UINT8 *Buffer
+ )
+{
+ EFI_FVB_ATTRIBUTES_2 Attributes;
+ UINTN LbaAddress;
+ UINTN LbaLength;
+ EFI_STATUS Status;
+ BOOLEAN BadBufferSize =3D FALSE;
+
+ if ((NumBytes =3D=3D NULL) || (Buffer =3D=3D NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+ if (*NumBytes =3D=3D 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Status =3D FvbGetLbaAddress (FvbInstance, Lba, &LbaAddress, &LbaLength=
, NULL);
+ if (EFI_ERROR(Status)) {
+ return Status;
+ }
+
+ //
+ // Check if the FV is write enabled
+ //
+ Attributes =3D FvbGetVolumeAttributes (FvbInstance);
+ if ((Attributes & EFI_FVB2_WRITE_STATUS) =3D=3D 0) {
+ return EFI_ACCESS_DENIED;
+ }
+
+ //
+ // Perform boundary checks and adjust NumBytes
+ //
+ if (BlockOffset > LbaLength) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (LbaLength < (*NumBytes + BlockOffset)) {
+ DEBUG ((DEBUG_INFO,
+ "FvWriteBlock: Reducing Numbytes from 0x%x to 0x%x\n",
+ *NumBytes,
+ (UINT32)(LbaLength - BlockOffset))
+ );
+ *NumBytes =3D (UINT32) (LbaLength - BlockOffset);
+ BadBufferSize =3D TRUE;
+ }
+
+ Status =3D SpiFlashWrite (LbaAddress + BlockOffset, (UINT32 *)NumBytes=
, Buffer);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status =3D SpiFlashLock ();
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ WriteBackInvalidateDataCacheRange ((VOID *) (LbaAddress + BlockOffset)=
, *NumBytes);
+
+ if (!EFI_ERROR (Status) && BadBufferSize) {
+ return EFI_BAD_BUFFER_SIZE;
+ } else {
+ return Status;
+ }
+}
+
+
+
+/**
+ Erases and initializes a firmware volume block.
+
+ @param[in] FvbInstance The pointer to the EFI_FVB_INSTANCE
+ @param[in] Lba The logical block index to be erased
+
+ @retval EFI_SUCCESS The erase request was successfully com=
pleted
+ @retval EFI_ACCESS_DENIED The firmware volume is in the WriteDis=
abled state
+ @retval EFI_DEVICE_ERROR The block device is not functioning co=
rrectly and
+ could not be written. Firmware device =
may have been
+ partially erased
+ @retval EFI_INVALID_PARAMETER Instance not found
+
+**/
+EFI_STATUS
+FvbEraseBlock (
+ IN EFI_FVB_INSTANCE *FvbInstance,
+ IN EFI_LBA Lba
+ )
+{
+
+ EFI_FVB_ATTRIBUTES_2 Attributes;
+ UINTN LbaAddress;
+ UINTN LbaLength;
+ EFI_STATUS Status;
+
+ //
+ // Check if the FV is write enabled
+ //
+ Attributes =3D FvbGetVolumeAttributes (FvbInstance);
+
+ if( (Attributes & EFI_FVB2_WRITE_STATUS) =3D=3D 0) {
+ return EFI_ACCESS_DENIED;
+ }
+
+ //
+ // Get the starting address of the block for erase.
+ //
+ Status =3D FvbGetLbaAddress (FvbInstance, Lba, &LbaAddress, &LbaLength=
, NULL);
+ if (EFI_ERROR(Status)) {
+ return Status;
+ }
+
+ Status =3D SpiFlashBlockErase (LbaAddress, &LbaLength);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status =3D SpiFlashLock ();
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ WriteBackInvalidateDataCacheRange ((VOID *) LbaAddress, LbaLength);
+
+ return Status;
+}
+
+/**
+ Modifies the current settings of the firmware volume according to the
+ input parameter, and returns the new setting of the volume
+
+ @param[in] FvbInstance The pointer to the EFI_FVB_INSTANCE.
+ @param[in] Attributes On input, it is a pointer to EFI_FVB=
_ATTRIBUTES_2
+ containing the desired firmware volu=
me settings.
+ On successful return, it contains th=
e new settings
+ of the firmware volume
+
+ @retval EFI_SUCCESS Successfully returns
+ @retval EFI_ACCESS_DENIED The volume setting is locked and can=
not be modified
+ @retval EFI_INVALID_PARAMETER Instance not found, or The attribute=
s requested are
+ in conflict with the capabilities as=
declared in the
+ firmware volume header
+
+**/
+EFI_STATUS
+FvbSetVolumeAttributes (
+ IN EFI_FVB_INSTANCE *FvbInstance,
+ IN OUT EFI_FVB_ATTRIBUTES_2 *Attributes
+ )
+{
+ EFI_FVB_ATTRIBUTES_2 OldAttributes;
+ EFI_FVB_ATTRIBUTES_2 *AttribPtr;
+ EFI_FVB_ATTRIBUTES_2 UnchangedAttributes;
+ UINT32 Capabilities;
+ UINT32 OldStatus, NewStatus;
+
+ AttribPtr =3D (EFI_FVB_ATTRIBUTES_2 *) &(FvbInstance->FvHeader.Att=
ributes);
+ OldAttributes =3D *AttribPtr;
+ Capabilities =3D OldAttributes & EFI_FVB2_CAPABILITIES;
+ OldStatus =3D OldAttributes & EFI_FVB2_STATUS;
+ NewStatus =3D *Attributes & EFI_FVB2_STATUS;
+
+ UnchangedAttributes =3D EFI_FVB2_READ_DISABLED_CAP | \
+ EFI_FVB2_READ_ENABLED_CAP | \
+ EFI_FVB2_WRITE_DISABLED_CAP | \
+ EFI_FVB2_WRITE_ENABLED_CAP | \
+ EFI_FVB2_LOCK_CAP | \
+ EFI_FVB2_STICKY_WRITE | \
+ EFI_FVB2_MEMORY_MAPPED | \
+ EFI_FVB2_ERASE_POLARITY | \
+ EFI_FVB2_READ_LOCK_CAP | \
+ EFI_FVB2_WRITE_LOCK_CAP | \
+ EFI_FVB2_ALIGNMENT;
+
+ //
+ // Some attributes of FV is read only can *not* be set
+ //
+ if ((OldAttributes & UnchangedAttributes) ^ (*Attributes & UnchangedAt=
tributes)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // If firmware volume is locked, no status bit can be updated
+ //
+ if ( OldAttributes & EFI_FVB2_LOCK_STATUS ) {
+ if ( OldStatus ^ NewStatus ) {
+ return EFI_ACCESS_DENIED;
+ }
+ }
+
+ //
+ // Test read disable
+ //
+ if ((Capabilities & EFI_FVB2_READ_DISABLED_CAP) =3D=3D 0) {
+ if ((NewStatus & EFI_FVB2_READ_STATUS) =3D=3D 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+ }
+
+ //
+ // Test read enable
+ //
+ if ((Capabilities & EFI_FVB2_READ_ENABLED_CAP) =3D=3D 0) {
+ if (NewStatus & EFI_FVB2_READ_STATUS) {
+ return EFI_INVALID_PARAMETER;
+ }
+ }
+
+ //
+ // Test write disable
+ //
+ if ((Capabilities & EFI_FVB2_WRITE_DISABLED_CAP) =3D=3D 0) {
+ if ((NewStatus & EFI_FVB2_WRITE_STATUS) =3D=3D 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+ }
+
+ //
+ // Test write enable
+ //
+ if ((Capabilities & EFI_FVB2_WRITE_ENABLED_CAP) =3D=3D 0) {
+ if (NewStatus & EFI_FVB2_WRITE_STATUS) {
+ return EFI_INVALID_PARAMETER;
+ }
+ }
+
+ //
+ // Test lock
+ //
+ if ((Capabilities & EFI_FVB2_LOCK_CAP) =3D=3D 0) {
+ if (NewStatus & EFI_FVB2_LOCK_STATUS) {
+ return EFI_INVALID_PARAMETER;
+ }
+ }
+
+ *AttribPtr =3D (*AttribPtr) & (0xFFFFFFFF & (~EFI_FVB2_STATUS));
+ *AttribPtr =3D (*AttribPtr) | NewStatus;
+ *Attributes =3D *AttribPtr;
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Check the integrity of firmware volume header
+
+ @param[in] FvHeader A pointer to a firmware volume header
+
+ @retval TRUE The firmware volume is consistent
+ @retval FALSE The firmware volume has corrupted.
+
+**/
+BOOLEAN
+IsFvHeaderValid (
+ IN EFI_PHYSICAL_ADDRESS FvBase,
+ IN CONST EFI_FIRMWARE_VOLUME_HEADER *FvHeader
+ )
+{
+ if (FvBase =3D=3D PcdGet32(PcdFlashNvStorageVariableBase)) {
+ if (CompareMem (&FvHeader->FileSystemGuid, &gEfiSystemNvDataFvGuid, =
sizeof(EFI_GUID)) !=3D 0 ) {
+ return FALSE;
+ }
+ } else {
+ if (CompareMem (&FvHeader->FileSystemGuid, &gEfiFirmwareFileSystem2G=
uid, sizeof(EFI_GUID)) !=3D 0 ) {
+ return FALSE;
+ }
+ }
+ if ( (FvHeader->Revision !=3D EFI_FVH_REVISION) ||
+ (FvHeader->Signature !=3D EFI_FVH_SIGNATURE) ||
+ (FvHeader->FvLength =3D=3D ((UINTN) -1)) ||
+ ((FvHeader->HeaderLength & 0x01 ) !=3D0) ) {
+ return FALSE;
+ }
+
+ if (CalculateCheckSum16 ((UINT16 *) FvHeader, FvHeader->HeaderLength) =
!=3D 0) {
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
+//
+// FVB protocol APIs
+//
+
+/**
+ Retrieves the physical address of the device.
+
+ @param[in] This A pointer to EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL.
+ @param[out] Address Output buffer containing the address.
+
+ retval EFI_SUCCESS The function always return successfully.
+
+**/
+EFI_STATUS
+EFIAPI
+FvbProtocolGetPhysicalAddress (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ OUT EFI_PHYSICAL_ADDRESS *Address
+ )
+{
+ EFI_FVB_INSTANCE *FvbInstance;
+
+ FvbInstance =3D FVB_INSTANCE_FROM_THIS (This);
+
+ *Address =3D FvbInstance->FvBase;
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Retrieve the size of a logical block
+
+ @param[in] This Calling context
+ @param[in] Lba Indicates which block to return the size for.
+ @param[out] BlockSize A pointer to a caller allocated UINTN in which
+ the size of the block is returned
+ @param[out] NumOfBlocks A pointer to a caller allocated UINTN in which=
the
+ number of consecutive blocks starting with Lba=
is
+ returned. All blocks in this range have a size=
of
+ BlockSize
+
+ @retval EFI_SUCCESS The function always return successfully.
+
+**/
+EFI_STATUS
+EFIAPI
+FvbProtocolGetBlockSize (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ IN EFI_LBA Lba,
+ OUT UINTN *BlockSize,
+ OUT UINTN *NumOfBlocks
+ )
+{
+ EFI_FVB_INSTANCE *FvbInstance;
+
+ FvbInstance =3D FVB_INSTANCE_FROM_THIS (This);
+
+ DEBUG((DEBUG_INFO,
+ "FvbProtocolGetBlockSize: Lba: 0x%lx BlockSize: 0x%x NumOfBlocks: 0x=
%x\n",
+ Lba,
+ BlockSize,
+ NumOfBlocks)
+ );
+
+ return FvbGetLbaAddress (
+ FvbInstance,
+ Lba,
+ NULL,
+ BlockSize,
+ NumOfBlocks
+ );
+}
+
+/**
+ Retrieves Volume attributes. No polarity translations are done.
+
+ @param[in] This Calling context
+ @param[out] Attributes Output buffer which contains attributes
+
+ @retval EFI_SUCCESS The function always return successfully.
+
+**/
+EFI_STATUS
+EFIAPI
+FvbProtocolGetAttributes (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ OUT EFI_FVB_ATTRIBUTES_2 *Attributes
+ )
+{
+ EFI_FVB_INSTANCE *FvbInstance;
+
+ FvbInstance =3D FVB_INSTANCE_FROM_THIS (This);
+
+ *Attributes =3D FvbGetVolumeAttributes (FvbInstance);
+
+ DEBUG ((DEBUG_INFO,
+ "FvbProtocolGetAttributes: This: 0x%x Attributes: 0x%x\n",
+ This,
+ *Attributes)
+ );
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Sets Volume attributes. No polarity translations are done.
+
+ @param[in] This Calling context
+ @param[out] Attributes Output buffer which contains attributes
+
+ @retval EFI_SUCCESS The function always return successfully.
+
+**/
+EFI_STATUS
+EFIAPI
+FvbProtocolSetAttributes (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ IN OUT EFI_FVB_ATTRIBUTES_2 *Attributes
+ )
+{
+ EFI_STATUS Status;
+ EFI_FVB_INSTANCE *FvbInstance;
+
+ DEBUG((DEBUG_INFO,
+ "FvbProtocolSetAttributes: Before SET - This: 0x%x Attributes: 0x%x=
\n",
+ This,
+ *Attributes)
+ );
+
+ FvbInstance =3D FVB_INSTANCE_FROM_THIS (This);
+
+ Status =3D FvbSetVolumeAttributes (FvbInstance, Attributes);
+
+ DEBUG((DEBUG_INFO,
+ "FvbProtocolSetAttributes: After SET - This: 0x%x Attributes: 0x%x\=
n",
+ This,
+ *Attributes)
+ );
+
+ return Status;
+}
+
+/**
+ The EraseBlock() function erases one or more blocks as denoted by the
+ variable argument list. The entire parameter list of blocks must be ve=
rified
+ prior to erasing any blocks. If a block is requested that does not ex=
ist
+ within the associated firmware volume (it has a larger index than the =
last
+ block of the firmware volume), the EraseBlock() function must return
+ EFI_INVALID_PARAMETER without modifying the contents of the firmware v=
olume.
+
+ @param[in] This Calling context
+ @param[in] ... Starting LBA followed by Number of Lba to eras=
e.
+ a -1 to terminate the list.
+
+ @retval EFI_SUCCESS The erase request was successfully completed
+ @retval EFI_ACCESS_DENIED The firmware volume is in the WriteDisabled =
state
+ @retval EFI_DEVICE_ERROR The block device is not functioning correctl=
y and
+ could not be written. Firmware device may ha=
ve been
+ partially erased
+
+**/
+EFI_STATUS
+EFIAPI
+FvbProtocolEraseBlocks (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ ...
+ )
+{
+ EFI_FVB_INSTANCE *FvbInstance;
+ UINTN NumOfBlocks;
+ VA_LIST Args;
+ EFI_LBA StartingLba;
+ UINTN NumOfLba;
+ EFI_STATUS Status;
+
+ DEBUG((DEBUG_INFO, "FvbProtocolEraseBlocks: \n"));
+
+ FvbInstance =3D FVB_INSTANCE_FROM_THIS (This);
+
+ NumOfBlocks =3D FvbInstance->NumOfBlocks;
+
+ VA_START (Args, This);
+
+ do {
+ StartingLba =3D VA_ARG (Args, EFI_LBA);
+ if ( StartingLba =3D=3D EFI_LBA_LIST_TERMINATOR ) {
+ break;
+ }
+
+ NumOfLba =3D VA_ARG (Args, UINT32);
+
+ //
+ // Check input parameters
+ //
+ if (NumOfLba =3D=3D 0) {
+ VA_END (Args);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if ( ( StartingLba + NumOfLba ) > NumOfBlocks ) {
+ return EFI_INVALID_PARAMETER;
+ }
+ } while ( 1 );
+
+ VA_END (Args);
+
+ VA_START (Args, This);
+ do {
+ StartingLba =3D VA_ARG (Args, EFI_LBA);
+ if (StartingLba =3D=3D EFI_LBA_LIST_TERMINATOR) {
+ break;
+ }
+
+ NumOfLba =3D VA_ARG (Args, UINT32);
+
+ while ( NumOfLba > 0 ) {
+ Status =3D FvbEraseBlock (FvbInstance, StartingLba);
+ if ( EFI_ERROR(Status)) {
+ VA_END (Args);
+ return Status;
+ }
+ StartingLba ++;
+ NumOfLba --;
+ }
+
+ } while ( 1 );
+
+ VA_END (Args);
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Writes data beginning at Lba:Offset from FV. The write terminates eith=
er
+ when *NumBytes of data have been written, or when a block boundary is
+ reached. *NumBytes is updated to reflect the actual number of bytes
+ written. The write opertion does not include erase. This routine will
+ attempt to write only the specified bytes. If the writes do not stick,
+ it will return an error.
+
+ @param[in] This Calling context
+ @param[in] Lba Block in which to begin write
+ @param[in] Offset Offset in the block at which to begin write
+ @param[in,out] NumBytes On input, indicates the requested write size=
. On
+ output, indicates the actual number of bytes=
written
+ @param[in] Buffer Buffer containing source data for the write.
+
+ @retval EFI_SUCCESS The firmware volume was written successf=
ully
+ @retval EFI_BAD_BUFFER_SIZE Write attempted across a LBA boundary. O=
n output,
+ NumBytes contains the total number of by=
tes
+ actually written
+ @retval EFI_ACCESS_DENIED The firmware volume is in the WriteDisab=
led state
+ @retval EFI_DEVICE_ERROR The block device is not functioning corr=
ectly and
+ could not be written
+ @retval EFI_INVALID_PARAMETER NumBytes or Buffer are NULL
+
+**/
+EFI_STATUS
+EFIAPI
+FvbProtocolWrite (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ IN EFI_LBA Lba,
+ IN UINTN Offset,
+ IN OUT UINTN *NumBytes,
+ IN UINT8 *Buffer
+ )
+{
+ EFI_FVB_INSTANCE *FvbInstance;
+
+ FvbInstance =3D FVB_INSTANCE_FROM_THIS (This);
+
+ DEBUG((DEBUG_INFO,
+ "FvbProtocolWrite: Lba: 0x%lx Offset: 0x%x NumBytes: 0x%x, Buffer: 0=
x%x\n",
+ Lba,
+ Offset,
+ *NumBytes,
+ Buffer)
+ );
+
+ return FvbWriteBlock (FvbInstance, Lba, Offset, NumBytes, Buffer);
+}
+
+/**
+ Reads data beginning at Lba:Offset from FV. The Read terminates either
+ when *NumBytes of data have been read, or when a block boundary is
+ reached. *NumBytes is updated to reflect the actual number of bytes
+ written. The write opertion does not include erase. This routine will
+ attempt to write only the specified bytes. If the writes do not stick,
+ it will return an error.
+
+ @param[in] This Calling context
+ @param[in] Lba Block in which to begin write
+ @param[in] Offset Offset in the block at which to begin write
+ @param[in,out] NumBytes On input, indicates the requested write size=
. On
+ output, indicates the actual number of bytes=
written
+ @param[in] Buffer Buffer containing source data for the write.
+
+ @retval EFI_SUCCESS The firmware volume was read successfull=
y and
+ contents are in Buffer
+ @retval EFI_BAD_BUFFER_SIZE Read attempted across a LBA boundary. On=
output,
+ NumBytes contains the total number of by=
tes returned
+ in Buffer
+ @retval EFI_ACCESS_DENIED The firmware volume is in the ReadDisabl=
ed state
+ @retval EFI_DEVICE_ERROR The block device is not functioning corr=
ectly and
+ could not be read
+ @retval EFI_INVALID_PARAMETER NumBytes or Buffer are NULL
+
+**/
+EFI_STATUS
+EFIAPI
+FvbProtocolRead (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ IN EFI_LBA Lba,
+ IN UINTN Offset,
+ IN OUT UINTN *NumBytes,
+ OUT UINT8 *Buffer
+ )
+{
+ EFI_FVB_INSTANCE *FvbInstance;
+ EFI_STATUS Status;
+
+ FvbInstance =3D FVB_INSTANCE_FROM_THIS (This);
+ Status =3D FvbReadBlock (FvbInstance, Lba, Offset, NumBytes, Buffer);
+ DEBUG((DEBUG_INFO,
+ "FvbProtocolRead: Lba: 0x%lx Offset: 0x%x NumBytes: 0x%x, Buffer: 0x=
%x\n",
+ Lba,
+ Offset,
+ *NumBytes,
+ Buffer)
+ );
+
+ return Status;
+}
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/Sp=
iFvbServiceMm.c b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbServi=
ce/SpiFvbServiceMm.c
new file mode 100644
index 000000000000..42a0828c6fae
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSer=
viceMm.c
@@ -0,0 +1,271 @@
+/** @file
+ MM driver source for several Serial Flash devices
+ which are compliant with the Intel(R) Serial Flash Interface Compatibi=
lity Specification.
+
+ Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) Microsoft Corporation.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "SpiFvbServiceCommon.h"
+#include <Library/MmServicesTableLib.h>
+#include <Library/UefiDriverEntryPoint.h>
+#include <Protocol/SmmFirmwareVolumeBlock.h>
+
+/**
+ The function installs EFI_FIRMWARE_VOLUME_BLOCK protocol
+ for each FV in the system.
+
+ @param[in] FvbInstance The pointer to a FW volume instance structur=
e,
+ which contains the information about one FV.
+
+ @retval VOID
+
+**/
+VOID
+InstallFvbProtocol (
+ IN EFI_FVB_INSTANCE *FvbInstance
+ )
+{
+ EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
+ EFI_STATUS Status;
+ EFI_HANDLE FvbHandle;
+
+ ASSERT (FvbInstance !=3D NULL);
+ if (FvbInstance =3D=3D NULL) {
+ return;
+ }
+
+ CopyMem (&FvbInstance->FvbProtocol, &mFvbProtocolTemplate, sizeof (EFI=
_FIRMWARE_VOLUME_BLOCK_PROTOCOL));
+
+ FvHeader =3D &FvbInstance->FvHeader;
+ if (FvHeader =3D=3D NULL) {
+ return;
+ }
+
+ //
+ // Set up the devicepath
+ //
+ DEBUG ((DEBUG_INFO, "FwBlockService.c: Setting up DevicePath for 0x%lx=
:\n", FvbInstance->FvBase));
+ if (FvHeader->ExtHeaderOffset =3D=3D 0) {
+ //
+ // FV does not contains extension header, then produce MEMMAP_DEVICE=
_PATH
+ //
+ FvbInstance->DevicePath =3D (EFI_DEVICE_PATH_PROTOCOL *) AllocateRun=
timeCopyPool (sizeof (FV_MEMMAP_DEVICE_PATH), &mFvMemmapDevicePathTemplat=
e);
+ if (FvbInstance->DevicePath =3D=3D NULL) {
+ DEBUG ((DEBUG_INFO, "SpiFvbServiceSmm.c: Memory allocation for MEM=
MAP_DEVICE_PATH failed\n"));
+ return;
+ }
+ ((FV_MEMMAP_DEVICE_PATH *) FvbInstance->DevicePath)->MemMapDevPath.S=
tartingAddress =3D FvbInstance->FvBase;
+ ((FV_MEMMAP_DEVICE_PATH *) FvbInstance->DevicePath)->MemMapDevPath.E=
ndingAddress =3D FvbInstance->FvBase + FvHeader->FvLength - 1;
+ } else {
+ FvbInstance->DevicePath =3D (EFI_DEVICE_PATH_PROTOCOL *) AllocateRun=
timeCopyPool (sizeof (FV_PIWG_DEVICE_PATH), &mFvPIWGDevicePathTemplate);
+ if (FvbInstance->DevicePath =3D=3D NULL) {
+ DEBUG ((DEBUG_INFO, "SpiFvbServiceSmm.c: Memory allocation for FV_=
PIWG_DEVICE_PATH failed\n"));
+ return;
+ }
+ CopyGuid (
+ &((FV_PIWG_DEVICE_PATH *)FvbInstance->DevicePath)->FvDevPath.FvNam=
e,
+ (GUID *)(UINTN)(FvbInstance->FvBase + FvHeader->ExtHeaderOffset)
+ );
+ }
+
+ //
+ // LocateDevicePath fails so install a new interface and device path
+ //
+ FvbHandle =3D NULL;
+
+ Status =3D gMmst->MmInstallProtocolInterface (
+ &FvbHandle,
+ &gEfiSmmFirmwareVolumeBlockProtocolGuid,
+ EFI_NATIVE_INTERFACE,
+ &(FvbInstance->FvbProtocol)
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ Status =3D gMmst->MmInstallProtocolInterface (
+ &FvbHandle,
+ &gEfiDevicePathProtocolGuid,
+ EFI_NATIVE_INTERFACE,
+ &(FvbInstance->DevicePath)
+ );
+ ASSERT_EFI_ERROR (Status);
+}
+
+/**
+ The function does the necessary initialization work for
+ Firmware Volume Block Driver.
+
+**/
+VOID
+FvbInitialize (
+ VOID
+ )
+{
+ EFI_FVB_INSTANCE *FvbInstance;
+ EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
+ EFI_FV_BLOCK_MAP_ENTRY *PtrBlockMapEntry;
+ EFI_PHYSICAL_ADDRESS BaseAddress;
+ EFI_STATUS Status;
+ UINTN BufferSize;
+ UINTN Idx;
+ UINT32 MaxLbaSize;
+ UINT32 BytesWritten;
+ UINTN BytesErased;
+
+ mPlatformFvBaseAddress[0].FvBase =3D PcdGet32(PcdFlashNvStorageVariabl=
eBase);
+ mPlatformFvBaseAddress[0].FvSize =3D PcdGet32(PcdFlashNvStorageVariabl=
eSize);
+ mPlatformFvBaseAddress[1].FvBase =3D PcdGet32(PcdFlashMicrocodeFvBase)=
;
+ mPlatformFvBaseAddress[1].FvSize =3D PcdGet32(PcdFlashMicrocodeFvSize)=
;
+ mPlatformDefaultBaseAddress[0].FvBase =3D PcdGet32(PcdFlashNvStorageVa=
riableBase);
+ mPlatformDefaultBaseAddress[0].FvSize =3D PcdGet32(PcdFlashNvStorageVa=
riableSize);
+ mPlatformDefaultBaseAddress[1].FvBase =3D PcdGet32(PcdFlashMicrocodeFv=
Base);
+ mPlatformDefaultBaseAddress[1].FvSize =3D PcdGet32(PcdFlashMicrocodeFv=
Size);
+
+ //
+ // We will only continue with FVB installation if the
+ // SPI is the active BIOS state
+ //
+ {
+ //
+ // Make sure all FVB are valid and/or fix if possible
+ //
+ for (Idx =3D 0;; Idx++) {
+ if (mPlatformFvBaseAddress[Idx].FvSize =3D=3D 0 && mPlatformFvBase=
Address[Idx].FvBase =3D=3D 0) {
+ break;
+ }
+
+ BaseAddress =3D mPlatformFvBaseAddress[Idx].FvBase;
+ FvHeader =3D (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) BaseAddress;
+
+ if (!IsFvHeaderValid (BaseAddress, FvHeader)) {
+ BytesWritten =3D 0;
+ BytesErased =3D 0;
+ DEBUG ((DEBUG_ERROR, "ERROR - The FV in 0x%x is invalid!\n", FvH=
eader));
+ Status =3D GetFvbInfo (BaseAddress, &FvHeader);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN, "ERROR - Can't recovery FV header at 0x%x.=
GetFvbInfo Status %r\n", BaseAddress, Status));
+ continue;
+ }
+ DEBUG ((DEBUG_INFO, "Rewriting FV header at 0x%X with static dat=
a\n", BaseAddress));
+ //
+ // Spi erase
+ //
+ BytesErased =3D (UINTN) FvHeader->BlockMap->Length;
+ Status =3D SpiFlashBlockErase( (UINTN) BaseAddress, &BytesErased=
);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN, "ERROR - SpiFlashBlockErase Error %r\n", =
Status));
+ continue;
+ }
+ if (BytesErased !=3D FvHeader->BlockMap->Length) {
+ DEBUG ((DEBUG_WARN, "ERROR - BytesErased !=3D FvHeader->BlockM=
ap->Length\n"));
+ DEBUG ((DEBUG_INFO, " BytesErased =3D 0x%X\n Length =3D 0x%X\n=
", BytesErased, FvHeader->BlockMap->Length));
+ continue;
+ }
+ BytesWritten =3D FvHeader->HeaderLength;
+ Status =3D SpiFlashWrite ((UINTN)BaseAddress, &BytesWritten, (UI=
NT8*)FvHeader);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN, "ERROR - SpiFlashWrite Error %r\n", Statu=
s));
+ continue;
+ }
+ if (BytesWritten !=3D FvHeader->HeaderLength) {
+ DEBUG ((DEBUG_WARN, "ERROR - BytesWritten !=3D HeaderLength\n"=
));
+ DEBUG ((DEBUG_INFO, " BytesWritten =3D 0x%X\n HeaderLength =3D=
0x%X\n", BytesWritten, FvHeader->HeaderLength));
+ continue;
+ }
+ Status =3D SpiFlashLock ();
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN, "ERROR - SpiFlashLock Error %r\n", Status=
));
+ continue;
+ }
+ DEBUG ((DEBUG_INFO, "FV Header @ 0x%X restored with static data\=
n", BaseAddress));
+ //
+ // Clear cache for this range.
+ //
+ WriteBackInvalidateDataCacheRange ( (VOID *) (UINTN) BaseAddress=
, FvHeader->BlockMap->Length);
+ }
+ }
+
+ //
+ // Calculate the total size for all firmware volume block instances
+ //
+ BufferSize =3D 0;
+ for (Idx =3D 0; ; Idx++) {
+ if (mPlatformFvBaseAddress[Idx].FvSize =3D=3D 0 && mPlatformFvBase=
Address[Idx].FvBase =3D=3D 0) {
+ break;
+ }
+ BaseAddress =3D mPlatformFvBaseAddress[Idx].FvBase;
+ FvHeader =3D (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) BaseAddress;
+
+ if (!IsFvHeaderValid (BaseAddress, FvHeader)) {
+ DEBUG ((DEBUG_WARN, "ERROR - The FV in 0x%x is invalid!\n", FvHe=
ader));
+ continue;
+ }
+
+ BufferSize +=3D (FvHeader->HeaderLength +
+ sizeof (EFI_FVB_INSTANCE) -
+ sizeof (EFI_FIRMWARE_VOLUME_HEADER)
+ );
+ }
+
+ mFvbModuleGlobal.FvbInstance =3D (EFI_FVB_INSTANCE *) AllocateRunti=
meZeroPool (BufferSize);
+ if (mFvbModuleGlobal.FvbInstance =3D=3D NULL) {
+ ASSERT (FALSE);
+ return;
+ }
+
+ MaxLbaSize =3D 0;
+ FvbInstance =3D mFvbModuleGlobal.FvbInstance;
+ mFvbModuleGlobal.NumFv =3D 0;
+
+ for (Idx =3D 0; ; Idx++) {
+ if (mPlatformFvBaseAddress[Idx].FvSize =3D=3D 0 && mPlatformFvBase=
Address[Idx].FvBase =3D=3D 0) {
+ break;
+ }
+ BaseAddress =3D mPlatformFvBaseAddress[Idx].FvBase;
+ FvHeader =3D (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) BaseAddress;
+
+ if (!IsFvHeaderValid (BaseAddress, FvHeader)) {
+ DEBUG ((DEBUG_WARN, "ERROR - The FV in 0x%x is invalid!\n", FvHe=
ader));
+ continue;
+ }
+
+ FvbInstance->Signature =3D FVB_INSTANCE_SIGNATURE;
+ CopyMem (&(FvbInstance->FvHeader), FvHeader, FvHeader->HeaderLengt=
h);
+
+ FvHeader =3D &(FvbInstance->FvHeader);
+ FvbInstance->FvBase =3D (UINTN)BaseAddress;
+
+ //
+ // Process the block map for each FV
+ //
+ FvbInstance->NumOfBlocks =3D 0;
+ for (PtrBlockMapEntry =3D FvHeader->BlockMap;
+ PtrBlockMapEntry->NumBlocks !=3D 0;
+ PtrBlockMapEntry++) {
+ //
+ // Get the maximum size of a block.
+ //
+ if (MaxLbaSize < PtrBlockMapEntry->Length) {
+ MaxLbaSize =3D PtrBlockMapEntry->Length;
+ }
+ FvbInstance->NumOfBlocks +=3D PtrBlockMapEntry->NumBlocks;
+ }
+
+ //
+ // Add a FVB Protocol Instance
+ //
+ InstallFvbProtocol (FvbInstance);
+ mFvbModuleGlobal.NumFv++;
+
+ //
+ // Move on to the next FvbInstance
+ //
+ FvbInstance =3D (EFI_FVB_INSTANCE *) ((UINTN)((UINT8 *)FvbInstance=
) +
+ FvHeader->HeaderLength +
+ (sizeof (EFI_FVB_INSTANCE) -=
sizeof (EFI_FIRMWARE_VOLUME_HEADER)));
+
+ }
+ }
+}
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/Sp=
iFvbServiceStandaloneMm.c b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/S=
piFvbService/SpiFvbServiceStandaloneMm.c
new file mode 100644
index 000000000000..252c818d6551
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSer=
viceStandaloneMm.c
@@ -0,0 +1,32 @@
+/** @file
+ MM driver source for several Serial Flash devices
+ which are compliant with the Intel(R) Serial Flash Interface Compatibi=
lity Specification.
+
+ Copyright (c) Microsoft Corporation.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "SpiFvbServiceCommon.h"
+#include "SpiFvbServiceMm.h"
+
+/**
+ The driver Standalone MM entry point.
+
+ @param[in] ImageHandle Image handle of this driver.
+ @param[in] MmSystemTable A pointer to the MM system table.
+
+ @retval EFI_SUCCESS This function always returns EFI_SUCCE=
SS.
+
+**/
+EFI_STATUS
+EFIAPI
+SpiFvbStandaloneMmInitialize (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_MM_SYSTEM_TABLE *MmSystemTable
+ )
+{
+ FvbInitialize ();
+
+ return EFI_SUCCESS;
+}
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/Sp=
iFvbServiceTraditionalMm.c b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/=
SpiFvbService/SpiFvbServiceTraditionalMm.c
new file mode 100644
index 000000000000..1c2dac70e3c6
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSer=
viceTraditionalMm.c
@@ -0,0 +1,32 @@
+/** @file
+ MM driver source for several Serial Flash devices
+ which are compliant with the Intel(R) Serial Flash Interface Compatibi=
lity Specification.
+
+ Copyright (c) Microsoft Corporation.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "SpiFvbServiceCommon.h"
+#include "SpiFvbServiceMm.h"
+
+/**
+ The driver Traditional MM entry point.
+
+ @param[in] ImageHandle Image handle of this driver.
+ @param[in] SystemTable A pointer to the EFI system table.
+
+ @retval EFI_SUCCESS This function always returns EFI_SUCCE=
SS.
+
+**/
+EFI_STATUS
+EFIAPI
+SpiFvbTraditionalMmInitialize (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ FvbInitialize ();
+
+ return EFI_SUCCESS;
+}
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/Sp=
iFvbServiceCommon.h b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbS=
ervice/SpiFvbServiceCommon.h
new file mode 100644
index 000000000000..e9d69e985814
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSer=
viceCommon.h
@@ -0,0 +1,158 @@
+/** @file
+ Common source definitions used in serial flash drivers
+
+Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _SPI_FVB_SERVICE_COMMON_H
+#define _SPI_FVB_SERVICE_COMMON_H
+
+#include <Guid/EventGroup.h>
+#include <Guid/FirmwareFileSystem2.h>
+#include <Guid/SystemNvDataGuid.h>
+#include <Protocol/DevicePath.h>
+#include <Protocol/FirmwareVolumeBlock.h>
+
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/IoLib.h>
+#include <Library/CacheMaintenanceLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PcdLib.h>
+#include <Library/DevicePathLib.h>
+#include <Library/HobLib.h>
+
+#include <Library/SpiFlashCommonLib.h>
+
+//
+// Define two helper macro to extract the Capability field or Status fie=
ld in FVB
+// bit fields
+//
+#define EFI_FVB2_CAPABILITIES (EFI_FVB2_READ_DISABLED_CAP | \
+ EFI_FVB2_READ_ENABLED_CAP | \
+ EFI_FVB2_WRITE_DISABLED_CAP | \
+ EFI_FVB2_WRITE_ENABLED_CAP | \
+ EFI_FVB2_LOCK_CAP \
+ )
+
+#define EFI_FVB2_STATUS (EFI_FVB2_READ_STATUS | EFI_FVB2_WRITE_STATUS | =
EFI_FVB2_LOCK_STATUS)
+
+#define FVB_INSTANCE_SIGNATURE SIGNATURE_32('F','V','B','I')
+
+typedef struct {
+ UINT32 Signature;
+ UINTN FvBase;
+ UINTN NumOfBlocks;
+ EFI_DEVICE_PATH_PROTOCOL *DevicePath;
+ EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL FvbProtocol;
+ EFI_FIRMWARE_VOLUME_HEADER FvHeader;
+} EFI_FVB_INSTANCE;
+
+typedef struct {
+ EFI_FVB_INSTANCE *FvbInstance;
+ UINT32 NumFv;
+} FVB_GLOBAL;
+
+//
+// Fvb Protocol instance data
+//
+#define FVB_INSTANCE_FROM_THIS(a) CR(a, EFI_FVB_INSTANCE, FvbProtocol, F=
VB_INSTANCE_SIGNATURE)
+
+typedef struct {
+ MEDIA_FW_VOL_DEVICE_PATH FvDevPath;
+ EFI_DEVICE_PATH_PROTOCOL EndDevPath;
+} FV_PIWG_DEVICE_PATH;
+
+typedef struct {
+ MEMMAP_DEVICE_PATH MemMapDevPath;
+ EFI_DEVICE_PATH_PROTOCOL EndDevPath;
+} FV_MEMMAP_DEVICE_PATH;
+
+typedef struct {
+ UINT32 FvBase;
+ UINT32 FvSize;
+} FV_INFO;
+
+//
+// Protocol APIs
+//
+EFI_STATUS
+EFIAPI
+FvbProtocolGetAttributes (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ OUT EFI_FVB_ATTRIBUTES_2 *Attributes
+ );
+
+EFI_STATUS
+EFIAPI
+FvbProtocolSetAttributes (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ IN OUT EFI_FVB_ATTRIBUTES_2 *Attributes
+ );
+
+EFI_STATUS
+EFIAPI
+FvbProtocolGetPhysicalAddress (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ OUT EFI_PHYSICAL_ADDRESS *Address
+ );
+
+EFI_STATUS
+EFIAPI
+FvbProtocolGetBlockSize (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ IN EFI_LBA Lba,
+ OUT UINTN *BlockSize,
+ OUT UINTN *NumOfBlocks
+ );
+
+EFI_STATUS
+EFIAPI
+FvbProtocolRead (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ IN EFI_LBA Lba,
+ IN UINTN Offset,
+ IN OUT UINTN *NumBytes,
+ OUT UINT8 *Buffer
+ );
+
+EFI_STATUS
+EFIAPI
+FvbProtocolWrite (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ IN EFI_LBA Lba,
+ IN UINTN Offset,
+ IN OUT UINTN *NumBytes,
+ IN UINT8 *Buffer
+ );
+
+EFI_STATUS
+EFIAPI
+FvbProtocolEraseBlocks (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ ...
+ );
+
+BOOLEAN
+IsFvHeaderValid (
+ IN EFI_PHYSICAL_ADDRESS FvBase,
+ IN CONST EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader
+ );
+
+EFI_STATUS
+GetFvbInfo (
+ IN EFI_PHYSICAL_ADDRESS FvBaseAddress,
+ OUT EFI_FIRMWARE_VOLUME_HEADER **FvbInfo
+ );
+
+extern FVB_GLOBAL mFvbModuleGlobal;
+extern FV_MEMMAP_DEVICE_PATH mFvMemmapDevicePathTemplate;
+extern FV_PIWG_DEVICE_PATH mFvPIWGDevicePathTemplate;
+extern EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL mFvbProtocolTemplate;
+extern FV_INFO mPlatformFvBaseAddress[];
+extern FV_INFO mPlatformDefaultBaseAddress[];
+
+#endif
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/Sp=
iFvbServiceMm.h b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbServi=
ce/SpiFvbServiceMm.h
new file mode 100644
index 000000000000..36af1130c8ee
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSer=
viceMm.h
@@ -0,0 +1,22 @@
+/** @file
+ Definitions common to MM implementation in this driver.
+
+ Copyright (c) Microsoft Corporation.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _SPI_FVB_SERVICE_MM_H_
+#define _SPI_FVB_SERVICE_MM_H_
+
+/**
+ The function does the necessary initialization work for
+ Firmware Volume Block Driver.
+
+**/
+VOID
+FvbInitialize (
+ VOID
+ );
+
+#endif
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/Sp=
iFvbServiceSmm.inf b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbSe=
rvice/SpiFvbServiceSmm.inf
new file mode 100644
index 000000000000..bf1306f00201
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSer=
viceSmm.inf
@@ -0,0 +1,68 @@
+### @file
+# Component description file for the Serial Flash device Runtime driver.
+#
+# Copyright (c) 2017-2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) Microsoft Corporation.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+###
+
+[Defines]
+ INF_VERSION =3D 0x00010017
+ BASE_NAME =3D SpiFvbServiceSmm
+ FILE_GUID =3D 68A10D85-6858-4402-B070-028B3EA2174=
7
+ VERSION_STRING =3D 1.0
+ MODULE_TYPE =3D DXE_SMM_DRIVER
+ PI_SPECIFICATION_VERSION =3D 1.10
+ ENTRY_POINT =3D SpiFvbTraditionalMmInitialize
+
+#
+# The following information is for reference only and not required by th=
e build tools.
+#
+# VALID_ARCHITECTURES =3D IA32 X64
+#
+
+[LibraryClasses]
+ PcdLib
+ MemoryAllocationLib
+ CacheMaintenanceLib
+ BaseMemoryLib
+ DebugLib
+ BaseLib
+ UefiBootServicesTableLib
+ UefiDriverEntryPoint
+ SpiFlashCommonLib
+ MmServicesTableLib
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ IntelSiliconPkg/IntelSiliconPkg.dec
+
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase ## CONS=
UMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ## CONS=
UMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize ## CONS=
UMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize ## CONS=
UMES
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CONS=
UMES
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CONS=
UMES
+
+[Sources]
+ FvbInfo.c
+ SpiFvbServiceCommon.h
+ SpiFvbServiceCommon.c
+ SpiFvbServiceMm.h
+ SpiFvbServiceMm.c
+ SpiFvbServiceTraditionalMm.c
+
+[Protocols]
+ gEfiDevicePathProtocolGuid ## PRODUCES
+ gEfiSmmFirmwareVolumeBlockProtocolGuid ## PRODUCES
+
+[Guids]
+ gEfiFirmwareFileSystem2Guid ## CONSUMES
+ gEfiSystemNvDataFvGuid ## CONSUMES
+
+[Depex]
+ TRUE
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/Sp=
iFvbServiceStandaloneMm.inf b/Silicon/Intel/IntelSiliconPkg/Feature/Flash=
/SpiFvbService/SpiFvbServiceStandaloneMm.inf
new file mode 100644
index 000000000000..b66233968247
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSer=
viceStandaloneMm.inf
@@ -0,0 +1,67 @@
+### @file
+# Component description file for the Serial Flash device Standalone MM d=
river.
+#
+# Copyright (c) 2017-2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) Microsoft Corporation.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+###
+
+[Defines]
+ INF_VERSION =3D 0x0001001B
+ BASE_NAME =3D SpiFvbServiceStandaloneMm
+ FILE_GUID =3D E6313655-8BD0-4EAB-B319-AD5E212CE6A=
B
+ VERSION_STRING =3D 1.0
+ MODULE_TYPE =3D MM_STANDALONE
+ PI_SPECIFICATION_VERSION =3D 0x00010032
+ ENTRY_POINT =3D SpiFvbStandaloneMmInitialize
+
+#
+# The following information is for reference only and not required by th=
e build tools.
+#
+# VALID_ARCHITECTURES =3D IA32 X64
+#
+
+[LibraryClasses]
+ BaseLib
+ BaseMemoryLib
+ CacheMaintenanceLib
+ DebugLib
+ MemoryAllocationLib
+ PcdLib
+ MmServicesTableLib
+ SpiFlashCommonLib
+ StandaloneMmDriverEntryPoint
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ IntelSiliconPkg/IntelSiliconPkg.dec
+
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase ## CONS=
UMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ## CONS=
UMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize ## CONS=
UMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize ## CONS=
UMES
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CONS=
UMES
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CONS=
UMES
+
+[Sources]
+ FvbInfo.c
+ SpiFvbServiceCommon.h
+ SpiFvbServiceCommon.c
+ SpiFvbServiceMm.h
+ SpiFvbServiceMm.c
+ SpiFvbServiceStandaloneMm.c
+
+[Protocols]
+ gEfiDevicePathProtocolGuid ## PRODUCES
+ gEfiSmmFirmwareVolumeBlockProtocolGuid ## PRODUCES
+
+[Guids]
+ gEfiFirmwareFileSystem2Guid ## CONSUMES
+ gEfiSystemNvDataFvGuid ## CONSUMES
+
+[Depex]
+ TRUE
diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc b/Silicon/=
Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
index d4e15100bfde..1e826a080f28 100644
--- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
+++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
@@ -40,6 +40,9 @@ [LibraryClasses]
PeiGetVtdPmrAlignmentLib|IntelSiliconPkg/Library/PeiGetVtdPmrAlignment=
Lib/PeiGetVtdPmrAlignmentLib.inf
TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasur=
ementLibNull.inf
MicrocodeLib|UefiCpuPkg/Library/MicrocodeLib/MicrocodeLib.inf
+ SpiFlashCommonLib|IntelSiliconPkg/Library/SpiFlashCommonLibNull/SpiFla=
shCommonLibNull.inf
+ UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiB=
ootServicesTableLib.inf
+ UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEnt=
ryPoint.inf
=20
[LibraryClasses.common.PEIM]
PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
@@ -61,8 +64,14 @@ [LibraryClasses.common.DXE_DRIVER]
=20
[LibraryClasses.common.DXE_SMM_DRIVER]
MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAll=
ocationLib.inf
+ MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLi=
b.inf
SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTabl=
eLib.inf
=20
+[LibraryClasses.common.MM_STANDALONE]
+ MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocati=
onLib/StandaloneMmMemoryAllocationLib.inf
+ MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Standal=
oneMmServicesTableLib.inf
+ StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoi=
nt/StandaloneMmDriverEntryPoint.inf
+
########################################################################=
###########################
#
# Components Section - list of the modules and components that will be p=
rocessed by compilation
@@ -86,6 +95,8 @@ [Components]
IntelSiliconPkg/Library/DxeSmbiosDataHobLib/DxeSmbiosDataHobLib.inf
IntelSiliconPkg/Feature/PcieSecurity/IntelPciDeviceSecurityDxe/IntelPc=
iDeviceSecurityDxe.inf
IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformDevicePolicyDxe/Sam=
plePlatformDevicePolicyDxe.inf
+ IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
+ IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceStandaloneMm.=
inf
IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmarPei.inf
IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.inf
--=20
2.28.0.windows.1


[edk2-platforms][PATCH v5 00/46] Consolidate SpiFlashCommonLib instances

Michael Kubacki
 

From: Michael Kubacki <michael.kubacki@...>

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

SpiFlashCommonLib is duplicated in multiple places across the MinPlatform
design in edk2-platforms. I'm planning to build some additional
functionality on top of SpiFlashCommonLib and, ideally, this duplication
will be consolidated into a single instance usable across all current lib=
rary
consumers.

This patch series focuses on consolidating the various SpiFlashCommonLib
instances as agreed upon in https://edk2.groups.io/g/devel/message/71701.

Read the BZ for more general background around this series.

I only have an UpXtreme board on hand so maintainers/reviewers of other
board packages should test these changes on those boards.

V5 changes:
- Added build support for PurleyOpenBoardPkg and WhitleyOpenBoardPkg
(added to edk2-platforms during the lifetime of this patch series).
- Updated KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash to
use the new SPI PPI interface that identifies SPI flash regions
by GUID.
- Added new Reviewed-by replies that came in to existing patches
during v4.

V4 changes:
- Assigned new GUID values to the PCH SPI PPI and Protocols to
differentiate from previous instances. This was done because
the interface changed to identify SPI flash regions by GUID.

V3 changes:
- Added support to IntelSiliconPkg to identify flash regions by GUID as
requested in v2 review feedback.
V2 changes:
- Rebased patch series on current edk2-platforms master (32183bdaa91)

Note: Previous patch series only received a couple review comments after
being on the mailing list for over 2 months. Please be respectful of
contributors time and efforts and review in a timely manner.

Cc: Agyeman Prince <prince.agyeman@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Deepika Kethi Reddy <deepika.kethi.reddy@...>
Cc: Eric Dong <eric.dong@...>
Cc: Heng Luo <heng.luo@...>
Cc: Jeremy Soller <jeremy@...>
Cc: Kathappan Esakkithevar <kathappan.esakkithevar@...>
Cc: Liming Gao <gaoliming@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>

Michael Kubacki (46):
CometlakeOpenBoardPkg: Remove redundant IntelSiliconPkg.dec entry
WhiskeylakeOpenBoardPkg: Remove redundant IntelSiliconPkg.dec entry
CometlakeOpenBoardPkg/PeiPolicyUpdateLib: Add missing GUID to INF
IntelSiliconPkg: Add BIOS area base address and size PCDs
IntelSiliconPkg: Add microcode FV PCDs
IntelSiliconPkg: Add PCH SPI PPI
IntelSiliconPkg: Add PCH SPI Protocol
IntelSiliconPkg: Add SpiFlashCommonLib
IntelSiliconPkg: Add SmmSpiFlashCommonLib
IntelSiliconPkg: Add MM SPI FVB services
CometlakeOpenBoardPkg: Use IntelSiliconPkg BIOS area and ucode PCDs
KabylakeOpenBoardPkg: Use IntelSiliconPkg BIOS area and ucode PCDs
SimicsOpenBoardPkg: Use IntelSiliconPkg BIOS area and ucode PCDs
TigerlakeOpenBoardPkg: Use IntelSiliconPkg BIOS area and ucode PCDs
WhiskeylakeOpenBoardPkg: Use IntelSiliconPkg BIOS area and ucode PCDs
CoffeelakeSiliconPkg: Use IntelSiliconPkg BIOS area and ucode PCDs
KabylakeSiliconPkg: Use IntelSiliconPkg BIOS area and ucode PCDs
SimicsIch10Pkg: Use IntelSiliconPkg BIOS area and ucode PCDs
TigerlakeSiliconPkg: Use IntelSiliconPkg BIOS area and ucode PCDs
CometlakeOpenBoardPkg: Update SpiFvbService & SpiFlashCommonLib
KabylakeOpenBoardPkg: Update SpiFvbService & SpiFlashCommonLib
SimicsOpenBoardPkg: Update SpiFvbService & SpiFlashCommonLib
TigerlakeOpenBoardPkg: Update SpiFvbService & SpiFlashCommonLib
WhiskeylakeOpenBoardPkg: Update SpiFvbService & SpiFlashCommonLib
PurleyOpenBoardPkg: Use IntelSiliconPkg SpiFvbServiceSmm
WhitleyOpenBoardPkg: UseIntelSiliconPkg SpiFvbServiceSmm
MinPlatformPkg: Remove SpiFvbService modules
CoffeelakeSiliconPkg: Remove SmmSpiFlashCommonLib
KabylakeSiliconPkg: Remove SmmSpiFlashCommonLib
SimicsIch10Pkg: Remove SmmSpiFlashCommonLib
TigerlakeOpenBoardPkg: Remove SmmSpiFlashCommonLib
MinPlatformPkg: Remove SpiFlashCommonLibNull
PurleyOpenBoardPkg: Add SpiFlashCommonLib.h
KabylakeOpenBoardPkg/PeiSerialPortLibSpiFlash: Add IntelSiliconPkg.dec
CoffeelakeSiliconPkg: Remove PCH SPI PPI and Protocol from package
KabylakeSiliconPkg: Remove PCH SPI PPI and Protocol from package
SimicsIch10Pkg: Remove PCH SPI SMM Protocol from package
TigerlakeSiliconPkg: Remove PCH SPI PPI and Protocol from package
IntelSiliconPkg: Add flash region GUIDs
IntelSiliconPkg: Identify flash regions by GUID
CoffeelakeSiliconPkg/BasePchSpiCommonLib: Identify flash regions by
GUID
KabylakeSiliconPkg: Identify flash regions by GUID
KabylakeOpenBoardPkg/PeiSerialPortLibSpiFlash: Update for new SPI PPI
API
KabylakeOpenBoardPkg/KabylakeRvp3: Add PeiSerialPortlibSpiFlash to
build
SimicsIch10Pkg/BasePchSpiCommonLib: Identify flash regions by GUID
TigerlakeSiliconPkg/BasePchSpiCommonLib: Identify flash regions by
GUID

Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/Pei=
SerialPortLibSpiFlash.c =
| 4 +-
Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCommonL=
ib/SpiCommon.c =
| 144 ++++++++--
Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiF=
lashCommon.c =
| 196 -------------
Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiF=
lashCommonSmmLib.c =
| 54 ----
{Platform/Intel/MinPlatformPkg =3D> Silicon/Intel/IntelSiliconPkg/Featur=
e}/Flash/SpiFvbService/FvbInfo.c =
| 0
{Platform/Intel/MinPlatformPkg =3D> Silicon/Intel/IntelSiliconPkg/Featur=
e}/Flash/SpiFvbService/SpiFvbServiceCommon.c =
| 4 +-
{Platform/Intel/MinPlatformPkg =3D> Silicon/Intel/IntelSiliconPkg/Featur=
e}/Flash/SpiFvbService/SpiFvbServiceMm.c =
| 8 +-
{Platform/Intel/MinPlatformPkg =3D> Silicon/Intel/IntelSiliconPkg/Featur=
e}/Flash/SpiFvbService/SpiFvbServiceStandaloneMm.c =
| 0
{Platform/Intel/MinPlatformPkg =3D> Silicon/Intel/IntelSiliconPkg/Featur=
e}/Flash/SpiFvbService/SpiFvbServiceTraditionalMm.c =
| 0
Platform/Intel/TigerlakeOpenBoardPkg/Library/SmmSpiFlashCommonLib/SpiFla=
shCommonSmmLib.c =3D> Silicon/Intel/IntelSiliconPkg/Library/SmmSpiFlashCo=
mmonLib/SmmSpiFlashCommonLib.c | 2 +-
{Platform/Intel/TigerlakeOpenBoardPkg =3D> Silicon/Intel/IntelSiliconPkg=
}/Library/SmmSpiFlashCommonLib/SpiFlashCommon.c =
| 7 +-
{Platform/Intel/MinPlatformPkg/Flash =3D> Silicon/Intel/IntelSiliconPkg}=
/Library/SpiFlashCommonLibNull/SpiFlashCommonLibNull.c =
| 12 +-
Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/SecureMemoryMapConfiguration.c=
=
| 106 ++++++-
Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiFla=
shCommon.c =
| 196 -------------
Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiFla=
shCommonSmmLib.c =
| 54 ----
Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommonLib/=
SpiCommon.c =
| 140 +++++++--
Silicon/Intel/SimicsIch10Pkg/Library/SmmSpiFlashCommonLib/SpiFlashCommon=
.c =
| 194 -------------
Silicon/Intel/SimicsIch10Pkg/Library/SmmSpiFlashCommonLib/SpiFlashCommon=
SmmLib.c =
| 54 ----
Silicon/Intel/SimicsIch10Pkg/LibraryPrivate/BasePchSpiCommonLib/SpiCommo=
n.c =
| 165 ++++++++---
Silicon/Intel/SimicsIch10Pkg/Spi/Smm/PchSpi.c =
=
| 4 +-
Silicon/Intel/TigerlakeSiliconPkg/IpBlock/Spi/LibraryPrivate/BaseSpiComm=
onLib/SpiCommon.c =
| 176 ++++++++++--
Platform/Intel/CometlakeOpenBoardPkg/BiosInfo/BiosInfo.inf =
=
| 4 +-
Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/Include/Fdf/FlashMapI=
nclude.fdf =
| 4 +-
Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.dsc =
=
| 7 +-
Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.fdf =
=
| 38 +--
Platform/Intel/CometlakeOpenBoardPkg/Policy/Library/PeiPolicyUpdateLib/P=
eiPolicyUpdateLib.inf =
| 2 +-
Platform/Intel/CometlakeOpenBoardPkg/Policy/PolicyInitDxe/PolicyInitDxe.=
inf =
| 4 +-
Platform/Intel/KabylakeOpenBoardPkg/BiosInfo/BiosInfo.inf =
=
| 4 +-
Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Include/Fdf/FlashMapInclu=
de.fdf =
| 4 +-
Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc =
=
| 7 +-
Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.fdf =
=
| 40 +--
Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Include/Fdf/FlashMapInc=
lude.fdf =
| 4 +-
Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc =
=
| 9 +-
Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.fdf =
=
| 40 +--
Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Policy/Library/PeiSilic=
onPolicyUpdateLib/PeiSiliconPolicyUpdateLib.inf =
| 4 +-
Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/Pei=
SerialPortLibSpiFlash.inf =
| 2 +
Platform/Intel/MinPlatformPkg/Include/Library/SpiFlashCommonLib.h =
=
| 98 -------
Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec =
=
| 2 -
Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc =
=
| 6 -
Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/OpenBoardPkg.dsc =
=
| 2 +-
Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/OpenBoardPkg.fdf =
=
| 2 +-
Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/OpenBoardPkg.dsc =
=
| 4 +-
Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/OpenBoardPkg.fdf =
=
| 5 +-
Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc =
=
| 6 +-
Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf =
=
| 2 +-
Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf.inc =
=
| 8 +-
Platform/Intel/TigerlakeOpenBoardPkg/BiosInfo/BiosInfo.inf =
=
| 8 +-
Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/Include/Fdf/FlashMapI=
nclude.fdf =
| 4 +-
Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.dsc =
=
| 7 +-
Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.fdf =
=
| 40 +--
Platform/Intel/WhiskeylakeOpenBoardPkg/BiosInfo/BiosInfo.inf =
=
| 4 +-
Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/Library/PeiPolicyUpdateLib=
/PeiPolicyUpdateLib.inf =
| 1 -
Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/PolicyInitDxe/PolicyInitDx=
e.inf =
| 4 +-
Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/Include/Fdf/FlashMapIncl=
ude.fdf =
| 4 +-
Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/Library/BoardInitLib/Pei=
MultiBoardInitPreMemLib.inf =
| 2 +-
Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc =
=
| 7 +-
Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf =
=
| 38 +--
Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/Include/Fdf/Flash=
MapInclude.fdf =
| 4 +-
Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.dsc =
=
| 7 +-
Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.fdf =
=
| 38 +--
{Silicon/Intel/CoffeelakeSiliconPkg/Pch =3D> Platform/Intel/WhitleyOpenB=
oardPkg}/Include/Library/SpiFlashCommonLib.h =
| 2 +-
Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc =
=
| 2 +-
Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.fdf =
=
| 5 +-
Silicon/Intel/CoffeelakeSiliconPkg/Cpu/Library/PeiCpuPolicyLib/PeiCpuPol=
icyLib.inf =
| 4 +-
Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Private/Library/PchSpiCom=
monLib.h =
| 16 +-
Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.inf =
=
| 1 +
Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCommonL=
ib/BasePchSpiCommonLib.inf =
| 13 +
Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SmmS=
piFlashCommonLib.inf =
| 51 ----
Silicon/Intel/CoffeelakeSiliconPkg/Pch/Spi/Smm/PchSpiSmm.inf =
=
| 1 +
Silicon/Intel/CoffeelakeSiliconPkg/SiPkg.dec =
=
| 8 -
{Platform/Intel/MinPlatformPkg =3D> Silicon/Intel/IntelSiliconPkg/Featur=
e}/Flash/SpiFvbService/SpiFvbServiceCommon.h =
| 0
{Platform/Intel/MinPlatformPkg =3D> Silicon/Intel/IntelSiliconPkg/Featur=
e}/Flash/SpiFvbService/SpiFvbServiceMm.h =
| 0
{Platform/Intel/MinPlatformPkg =3D> Silicon/Intel/IntelSiliconPkg/Featur=
e}/Flash/SpiFvbService/SpiFvbServiceSmm.inf =
| 6 +-
{Platform/Intel/MinPlatformPkg =3D> Silicon/Intel/IntelSiliconPkg/Featur=
e}/Flash/SpiFvbService/SpiFvbServiceStandaloneMm.inf =
| 6 +-
Silicon/Intel/IntelSiliconPkg/Include/Guid/FlashRegion.h =
=
| 45 +++
Silicon/Intel/{SimicsIch10Pkg =3D> IntelSiliconPkg}/Include/Library/SpiF=
lashCommonLib.h =
| 2 +-
Silicon/Intel/{CoffeelakeSiliconPkg/Pch =3D> IntelSiliconPkg}/Include/Pp=
i/Spi.h =
| 4 +-
Silicon/Intel/{CoffeelakeSiliconPkg/Pch =3D> IntelSiliconPkg}/Include/Pr=
otocol/Spi.h =
| 39 +--
Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec =
=
| 37 +++
Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc =
=
| 17 ++
{Platform/Intel/TigerlakeOpenBoardPkg =3D> Silicon/Intel/IntelSiliconPkg=
}/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.inf =
| 24 +-
{Platform/Intel/MinPlatformPkg/Flash =3D> Silicon/Intel/IntelSiliconPkg}=
/Library/SpiFlashCommonLibNull/SpiFlashCommonLibNull.inf =
| 3 +-
Silicon/Intel/KabylakeSiliconPkg/Cpu/Library/PeiCpuPolicyLib/PeiCpuPolic=
yLib.inf =
| 4 +-
Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/HstiSiliconDxe.inf =
=
| 12 +-
Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Library/SpiFlashCommonLib.h=
=
| 98 -------
Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Ppi/Spi.h =
=
| 26 --
Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Protocol/Spi.h =
=
| 293 -------------------
Silicon/Intel/KabylakeSiliconPkg/Pch/IncludePrivate/Library/PchSpiCommon=
Lib.h =
| 20 +-
Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.inf =
=
| 1 +
Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SmmSpi=
FlashCommonLib.inf =
| 53 ----
Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommonLib/=
BasePchSpiCommonLib.inf =
| 11 +
Silicon/Intel/KabylakeSiliconPkg/Pch/Spi/Smm/PchSpiSmm.inf =
=
| 1 +
Silicon/Intel/KabylakeSiliconPkg/SiPkg.dec =
=
| 13 +-
Silicon/Intel/SimicsIch10Pkg/Ich10Pkg.dec =
=
| 11 -
Silicon/Intel/SimicsIch10Pkg/Include/Protocol/Spi.h =
=
| 295 -------------------
Silicon/Intel/SimicsIch10Pkg/IncludePrivate/Library/PchSpiCommonLib.h =
=
| 46 +--
Silicon/Intel/SimicsIch10Pkg/Library/SmmSpiFlashCommonLib/SmmSpiFlashCom=
monLib.inf =
| 50 ----
Silicon/Intel/SimicsIch10Pkg/LibraryPrivate/BasePchSpiCommonLib/BasePchS=
piCommonLib.inf =
| 16 +-
Silicon/Intel/SimicsIch10Pkg/Spi/Smm/PchSpiSmm.inf =
=
| 3 +-
Silicon/Intel/TigerlakeSiliconPkg/Include/Protocol/Spi.h =
=
| 301 --------------------
Silicon/Intel/TigerlakeSiliconPkg/IpBlock/Spi/IncludePrivate/Library/Spi=
CommonLib.h =
| 16 +-
Silicon/Intel/TigerlakeSiliconPkg/IpBlock/Spi/LibraryPrivate/BaseSpiComm=
onLib/BaseSpiCommonLib.inf =
| 19 +-
Silicon/Intel/TigerlakeSiliconPkg/IpBlock/Spi/Smm/SpiSmm.inf =
=
| 1 +
Silicon/Intel/TigerlakeSiliconPkg/Pch/PchInit/Dxe/PchInitDxeTgl.inf =
=
| 1 +
Silicon/Intel/TigerlakeSiliconPkg/SiPkg.dec =
=
| 8 -
105 files changed, 1101 insertions(+), 2480 deletions(-)
delete mode 100644 Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/SmmSpi=
FlashCommonLib/SpiFlashCommon.c
delete mode 100644 Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/SmmSpi=
FlashCommonLib/SpiFlashCommonSmmLib.c
rename {Platform/Intel/MinPlatformPkg =3D> Silicon/Intel/IntelSiliconPkg=
/Feature}/Flash/SpiFvbService/FvbInfo.c (100%)
rename {Platform/Intel/MinPlatformPkg =3D> Silicon/Intel/IntelSiliconPkg=
/Feature}/Flash/SpiFvbService/SpiFvbServiceCommon.c (96%)
rename {Platform/Intel/MinPlatformPkg =3D> Silicon/Intel/IntelSiliconPkg=
/Feature}/Flash/SpiFvbService/SpiFvbServiceMm.c (94%)
rename {Platform/Intel/MinPlatformPkg =3D> Silicon/Intel/IntelSiliconPkg=
/Feature}/Flash/SpiFvbService/SpiFvbServiceStandaloneMm.c (100%)
rename {Platform/Intel/MinPlatformPkg =3D> Silicon/Intel/IntelSiliconPkg=
/Feature}/Flash/SpiFvbService/SpiFvbServiceTraditionalMm.c (100%)
rename Platform/Intel/TigerlakeOpenBoardPkg/Library/SmmSpiFlashCommonLib=
/SpiFlashCommonSmmLib.c =3D> Silicon/Intel/IntelSiliconPkg/Library/SmmSpi=
FlashCommonLib/SmmSpiFlashCommonLib.c (90%)
rename {Platform/Intel/TigerlakeOpenBoardPkg =3D> Silicon/Intel/IntelSil=
iconPkg}/Library/SmmSpiFlashCommonLib/SpiFlashCommon.c (93%)
rename {Platform/Intel/MinPlatformPkg/Flash =3D> Silicon/Intel/IntelSili=
conPkg}/Library/SpiFlashCommonLibNull/SpiFlashCommonLibNull.c (83%)
delete mode 100644 Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFl=
ashCommonLib/SpiFlashCommon.c
delete mode 100644 Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFl=
ashCommonLib/SpiFlashCommonSmmLib.c
delete mode 100644 Silicon/Intel/SimicsIch10Pkg/Library/SmmSpiFlashCommo=
nLib/SpiFlashCommon.c
delete mode 100644 Silicon/Intel/SimicsIch10Pkg/Library/SmmSpiFlashCommo=
nLib/SpiFlashCommonSmmLib.c
delete mode 100644 Platform/Intel/MinPlatformPkg/Include/Library/SpiFlas=
hCommonLib.h
rename {Silicon/Intel/CoffeelakeSiliconPkg/Pch =3D> Platform/Intel/Whitl=
eyOpenBoardPkg}/Include/Library/SpiFlashCommonLib.h (96%)
delete mode 100644 Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/SmmSpi=
FlashCommonLib/SmmSpiFlashCommonLib.inf
rename {Platform/Intel/MinPlatformPkg =3D> Silicon/Intel/IntelSiliconPkg=
/Feature}/Flash/SpiFvbService/SpiFvbServiceCommon.h (100%)
rename {Platform/Intel/MinPlatformPkg =3D> Silicon/Intel/IntelSiliconPkg=
/Feature}/Flash/SpiFvbService/SpiFvbServiceMm.h (100%)
rename {Platform/Intel/MinPlatformPkg =3D> Silicon/Intel/IntelSiliconPkg=
/Feature}/Flash/SpiFvbService/SpiFvbServiceSmm.inf (88%)
rename {Platform/Intel/MinPlatformPkg =3D> Silicon/Intel/IntelSiliconPkg=
/Feature}/Flash/SpiFvbService/SpiFvbServiceStandaloneMm.inf (88%)
create mode 100644 Silicon/Intel/IntelSiliconPkg/Include/Guid/FlashRegio=
n.h
rename Silicon/Intel/{SimicsIch10Pkg =3D> IntelSiliconPkg}/Include/Libra=
ry/SpiFlashCommonLib.h (96%)
rename Silicon/Intel/{CoffeelakeSiliconPkg/Pch =3D> IntelSiliconPkg}/Inc=
lude/Ppi/Spi.h (85%)
rename Silicon/Intel/{CoffeelakeSiliconPkg/Pch =3D> IntelSiliconPkg}/Inc=
lude/Protocol/Spi.h (89%)
rename {Platform/Intel/TigerlakeOpenBoardPkg =3D> Silicon/Intel/IntelSil=
iconPkg}/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.inf (67%)
rename {Platform/Intel/MinPlatformPkg/Flash =3D> Silicon/Intel/IntelSili=
conPkg}/Library/SpiFlashCommonLibNull/SpiFlashCommonLibNull.inf (91%)
delete mode 100644 Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Library/=
SpiFlashCommonLib.h
delete mode 100644 Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Ppi/Spi.=
h
delete mode 100644 Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Protocol=
/Spi.h
delete mode 100644 Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFl=
ashCommonLib/SmmSpiFlashCommonLib.inf
delete mode 100644 Silicon/Intel/SimicsIch10Pkg/Include/Protocol/Spi.h
delete mode 100644 Silicon/Intel/SimicsIch10Pkg/Library/SmmSpiFlashCommo=
nLib/SmmSpiFlashCommonLib.inf
delete mode 100644 Silicon/Intel/TigerlakeSiliconPkg/Include/Protocol/Sp=
i.h

--=20
2.28.0.windows.1


Re: [PATCH v2 3/4] ArmPlatformPkg: retreive error source descriptors from MM

Sami Mujawar
 

Hi Omkar,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar


On 10/07/2021 05:18 PM, Omkar Anand Kulkarni wrote:
Add a driver that retreives error source descriptors from MM and
populates those into the HEST ACPI table. The error source descriptors
that are available from the MM side are retreived using MM Communicate 2
protocol.

The first call into the MM returns the size of MM Communicate buffer
required to hold all error source descriptor info. The communication
buffer of that size is then allocated and the second call into MM
returns the error source descriptors in the communication buffer.
The retreived error source descriptors are then appended to the HEST
table.

Co-authored-by: Thomas Abraham <thomas.abraham@...>
Signed-off-by: Omkar Anand Kulkarni <omkar.kulkarni@...>
---
 ArmPlatformPkg/ArmPlatformPkg.dec                                         |   7 +
 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf          |  44 +++
 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf |  51 ++++
 ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h       |  37 +++
 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c            | 308 +++++++++++++++++++
 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c   | 312 ++++++++++++++++++++
[SAMI] Should this patch be split into 2?
 6 files changed, 759 insertions(+)

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
index 4f062292663b..846b3e863aa9 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -52,6 +52,8 @@
 
 [Guids.common]
   gArmPlatformTokenSpaceGuid   = { 0x9c0aaed4, 0x74c5, 0x4043, { 0xb4, 0x17, 0xa3, 0x22, 0x38, 0x14, 0xce, 0x76 } }
+  gArmPlatformHestErrorSourcesGuid = { 0x76b8ab43, 0x822d, 0x4b00, { 0x9f, 0xd0, 0xf4, 0xa5, 0x35, 0x82, 0x47, 0x0a } }
+  gMmHestGetErrorSourceInfoGuid = { 0x7d602951, 0x678e, 0x4cc4, { 0x98, 0xd9, 0xe3, 0x76, 0x04, 0xf6, 0x93, 0x0d } }
 
 [PcdsFeatureFlag.common]
   gArmPlatformTokenSpaceGuid.PcdSendSgiToBringUpSecondaryCores|FALSE|BOOLEAN|0x00000004
@@ -128,6 +130,11 @@
 
   gArmPlatformTokenSpaceGuid.PcdWatchdogCount|0x0|UINT32|0x00000033
 
+[PcdsFixedAtBuild, PcdsPatchableInModule]
+  ## ACPI CPER memory space
+  gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferBase|0x00000000|UINT64|0x00000046
+  gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferSize|0x00000000|UINT64|0x00000047
+
 [Protocols.common]
   ## Arm Platform HEST table generation protocol
   gHestTableProtocolGuid = { 0x705bdcd9, 0x8c47, 0x457e, { 0xad, 0x0d, 0xf7, 0x86, 0xf3, 0x4a, 0x0d, 0x63 } }
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf
new file mode 100644
index 000000000000..5227dea91630
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf
@@ -0,0 +1,44 @@
+## @file
+#  DXE driver to get secure error sources.
+#
+#  DXE driver to retrieve the error source descriptors from Standalone MM and
+#  append those to the HEST table.
+#
+#  Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x0001001A
+  BASE_NAME                      = HestMmErrorSourceDxe
+  FILE_GUID                      = 76b8ab43-822d-4b00-9fd0-f4a53582470a
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = HestErrorSourceInitialize
+
+[Sources.common]
+  HestErrorSourceDxe.c
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  MdePkg/MdePkg.dec
+  StandaloneMmPkg/StandaloneMmPkg.dec
+
+[LibraryClasses]
+  BaseMemoryLib
+  DebugLib
+  DxeServicesTableLib
+  UefiDriverEntryPoint
+  UefiLib
+
+[Guids]
+  gMmHestGetErrorSourceInfoGuid                  ## PRODUCES
+
+[Protocols]
+  gHestTableProtocolGuid                         ## CONSUMES
+  gEfiMmCommunication2ProtocolGuid
+
+[Depex]
+  gHestTableProtocolGuid AND gEfiMmCommunication2ProtocolGuid
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf
new file mode 100644
index 000000000000..9d566de9bec3
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf
@@ -0,0 +1,51 @@
+## @file
+#  HEST error source gateway Standalone MM driver.
+#
+#  Collects HEST error source descriptors,by communicating with all the MM
+#  drivers implementing the HEST error source descriptor protocol.
+#
+#  Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x0001001A
+  BASE_NAME                      = HestErrorSourceStandaloneMm
+  FILE_GUID                      = 3ddbebcc-9841-4ef8-87fa-305843c1922d
+  MODULE_TYPE                    = MM_STANDALONE
+  VERSION_STRING                 = 1.0
+  PI_SPECIFICATION_VERSION       = 0x00010032
+  ENTRY_POINT                    = StandaloneMmHestErrorSourceInitialize
+
+[Sources]
+  HestErrorSourceStandaloneMm.c
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  StandaloneMmPkg/StandaloneMmPkg.dec
+
+[LibraryClasses]
+  ArmLib
+  ArmSvcLib
+  BaseMemoryLib
+  DebugLib
+  MemoryAllocationLib
+  StandaloneMmDriverEntryPoint
+
+[Protocols]
+  gMmHestErrorSourceDescProtocolGuid
+
+[Guids]
+  gMmHestGetErrorSourceInfoGuid               ##PRODUCES
+  gEfiStandaloneMmNonSecureBufferGuid
+
+[FixedPcd]
+  gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferBase
+  gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferSize
+
+[Depex]
+  TRUE
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h
new file mode 100644
index 000000000000..6ddc6bd21922
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h
@@ -0,0 +1,37 @@
+/** @file
+  Data structures for error source descriptor information.
+
+  This data structure forms the CommBuffer part of the MM Communication
+  protocol used for communicating the Hardware Error sources form MM to
+  Non-MM environment.
+
+  Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef HEST_ERROR_SOURCE_DESCRIPTOR_H_
+#define HEST_ERROR_SOURCE_DESCRIPTOR_H_
+
+#define HEST_ERROR_SOURCE_DESC_INFO_SIZE \
+  (OFFSET_OF (HEST_ERROR_SOURCE_DESC_INFO, ErrSourceDescList))
[SAMI] I feel there can be a simple way to do this, see the comments below.
+
+//
+// Data Structure to communicate the error source descriptor information from
+// Standalone MM.
+//
+typedef struct {
+  //
+  // Total count of error source descriptors.
+  //
+  UINTN ErrSourceDescCount;
+  //
+  // Total size of all the error source descriptors.
+  //
[SAMI] Does the Total size also include the size of ErrSourceDescCount and ErrSourceDescSize? 
+  UINTN ErrSourceDescSize;
[SAMI] Can the first 2 fields of this structure be moved to a structure called HEST_ERROR_SOURCE_DESC_HEADER? I think it may simplify computing of the size of HEST_ERROR_SOURCE_DESC_INFO.
[/SAMI]
+  //
+  // Array of error source descriptors that is ErrSourceDescSize in size.
+  //
+  UINT8 ErrSourceDescList[1];
+} HEST_ERROR_SOURCE_DESC_INFO;
+
+#endif // HEST_ERROR_SOURCE_DESCRIPTOR_H_
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c
new file mode 100644
index 000000000000..acfb0fc9e838
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c
@@ -0,0 +1,308 @@
+/** @file
+  Collects and appends the HEST error source descriptors from the MM drivers.
+
+  The drivers entry point locates the MM Communication protocol and calls into
+  Standalone MM to get the HEST error sources length and count. It also
+  retrieves descriptor information. The information is then used to build the
+  HEST table using the HEST table generation protocol.
+
+  This driver collects the secure error source descriptor information from the
+  MM drviers that implement HEST error source protocol. Instead of directly
+  communicating with the individual MM drivers, it calls into
+  HestErrorSourceStandaloneMM driver which is a gatway MM driver. This MM driver
+  in-turn communicates with individual MM drivers collecting the error source
+  descriptor information.
+
+  Once all the error source descriptor information is retrieved the driver
+  appends the descriptors to HEST table using the HestDxe driver.
+
+  Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <IndustryStandard/Acpi.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DxeServicesTableLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/MmCommunication2.h>
+#include <Protocol/HestTable.h>
+#include "HestMmErrorSourceCommon.h"
+
+#define MM_COMMUNICATE_HEADER_SIZE (OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data))
[SAMI] Can this definition be moved to MdePkg\Include\Protocol\MmCommunication.h, please ?
+
+STATIC HEST_TABLE_PROTOCOL *mHestProtocol;
+STATIC EFI_MM_COMMUNICATION2_PROTOCOL *mMmCommunication2;
+
+/**
+  Retrieve the error source descriptors from Standalone MM.
+
+  Initialize the MM comminication buffer by assigning the MM service to
+  invoke as gMmHestGetErrorSourceInfoGuid. Use the MM communication
+  protocol to retrieve the error source descriptors.
+
+  @param[in]       CommBuffSize  Size of communicate buffer.
+  @param[in, out]  CommBuffer    The communicate buffer.
+
+  @retval  EFI_SUCCESS  MM Communicate protocol call successful.
+  @retval  Other        MM Communicate protocol call failed.
+**/
+STATIC
+EFI_STATUS
+GetErrorSourceDescriptors (
+  IN     UINTN                     CommBuffSize,
+  IN OUT EFI_MM_COMMUNICATE_HEADER **CommBuffer
+  )
+{
+  EFI_STATUS Status;
+
+  //
+  // Initialize the CommBuffer with MM Communicate metadata.
+  //
+  CopyGuid (&(*CommBuffer)->HeaderGuid, &gMmHestGetErrorSourceInfoGuid);
+  (*CommBuffer)->MessageLength =
+    CommBuffSize -
+    sizeof ((*CommBuffer)->HeaderGuid) -
+    sizeof ((*CommBuffer)->MessageLength);
+
+  //
+  // Call into the Standalone MM using the MM Communicate protocol.
+  //
+  Status = mMmCommunication2->Communicate (
+                                mMmCommunication2,
+                                (VOID *)*CommBuffer,
+                                (VOID *)*CommBuffer,
[SAMI] Can you check if the third parameter to Communicate() is correct, please?
+                                NULL
+                                );
+
+  return Status;
+}
+
+/**
+  Collect HEST error source descriptors from all Standalone MM drivers and
+  append them to the HEST table.
+
+  Use MM Communication Protocol to communicate and collect the error source
+  descriptor information from Standalone MM. Check for the required buffer size
+  returned by the MM driver. Allocate buffer of adequate size and call again
+  into MM.
+
+  @retval  EFI_SUCCESS           Successful to collect and append the error
+                                 source.
+                                 descriptors to HEST table.
+  @retval  EFI_OUT_OF_RESOURCES  Memory allocation failure.
+  @retval  Other                 For any other error.
+**/
+STATIC
+EFI_STATUS
+AppendMmErrorSources (VOID)
[SAMI] VOID and ) should be on a separate line. Can you check the other patches in this series as well, please?
+{
+  EFI_MM_COMMUNICATE_HEADER   *CommunicationHeader = NULL;
+  HEST_ERROR_SOURCE_DESC_INFO *ErrorSourceDescInfo;
+  EFI_STATUS                  Status;
+  UINTN                       CommBufferSize;
+
+  //
+  // Retrieve the count, length and the actual eror source descriptors from
+  // the MM drivers. Do this by performing two MM Communicate calls, in the
+  // first call pass CommBuffer which is atleast of the size of error source
+  // descriptor info structure. Followed by another communicate call with
+  // CommBuffer allocated to required buffer size to hold all descriptors.
+  //
+  // Allocate CommBuffer atleast the size of error source descriptor info
+  // structure.
+  CommBufferSize =
+    MM_COMMUNICATE_HEADER_SIZE + HEST_ERROR_SOURCE_DESC_INFO_SIZE;
+  CommunicationHeader = AllocatePool (CommBufferSize);
[SAMI] Would it be better to use AllocateZeroPool() ?
+  if (CommunicationHeader == NULL) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Failed to allocate memory for CommunicationHeader\n",
+      __FUNCTION__
+      ));
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  //
+  // Make the first MM Communicate call to HestErrorSourceStandaloneMM gateway
+  // driver, which returns the required buffer size adequate to hold all the
+  // desctriptor information.
+  //
+  Status = GetErrorSourceDescriptors (CommBufferSize, &CommunicationHeader);
+  if ((EFI_ERROR (Status)) &&
+      (Status != EFI_BAD_BUFFER_SIZE)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: MM Communicate protocol call failed, status: %r\n",
+      __FUNCTION__,
+      Status
+      ));
+    FreePool (CommunicationHeader);
+    return Status;
+  }
+
+  // Check for the length of Error Source descriptors.
+  ErrorSourceDescInfo =
+    (HEST_ERROR_SOURCE_DESC_INFO *)(CommunicationHeader->Data);
+  if ((ErrorSourceDescInfo->ErrSourceDescSize == 0) ||
+      (ErrorSourceDescInfo->ErrSourceDescCount == 0)) {
+    DEBUG ((
+      DEBUG_INFO,
+      "HesErrorSourceDxe: HEST error source(s) not found\n"
+      ));
+    FreePool (CommunicationHeader);
+    return EFI_SUCCESS;
[SAMI] return EFI_NOT_FOUND ?
+  }
+
+  //
+  // Allocate CommBuffer of required size to accomodate all the error source
+  // descriptors. Required size of communication buffer =
+  // MM communicate metadata. + (error source desc info struct + error source
+  // descriptor size).
+  //
+  CommBufferSize =
+    MM_COMMUNICATE_HEADER_SIZE +
+    HEST_ERROR_SOURCE_DESC_INFO_SIZE +
+    ErrorSourceDescInfo->ErrSourceDescSize;
+
+  // Free old MM Communicate buffer and allocate a new buffer of required size.
+  FreePool (CommunicationHeader);
+  CommunicationHeader = AllocatePool (CommBufferSize);
+  if (CommunicationHeader == NULL) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Failed to allocate memory for CommunicationHeader\n",
+      __FUNCTION__
+      ));
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  //
+  // Make second MM Communicate call to HestErrorSourceStandaloneMM driver to
+  // get the error source descriptors from the MM drivers.
+  //
+  Status = GetErrorSourceDescriptors (CommBufferSize, &CommunicationHeader);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: MM Communicate protocol failed, status: %r\n",
+      __FUNCTION__,
+      Status
+      ));
+    FreePool (CommunicationHeader);
+    return Status;
+  }
+
+  //
+  // Retrieve the HEST error source descriptor information. Ensure that there
+  // is a valid list of error source descriptors.
+  //
+  ErrorSourceDescInfo =
+    (HEST_ERROR_SOURCE_DESC_INFO *)(CommunicationHeader->Data);
+  if (ErrorSourceDescInfo->ErrSourceDescList == NULL) {
+    DEBUG ((
+      DEBUG_INFO,
+      "HestErrorSourceDxe: Error source descriptor list is empty"
+      ));
+    FreePool (CommunicationHeader);
+    return EFI_SUCCESS;
[SAMI] Can EFI_NOT_FOUND be returned here?
+  }
+
+  DEBUG ((
+    DEBUG_INFO,
+    "HestErrorSourceDxe: ErrorSources: TotalCount = %d TotalLength = %d \n",
+    ErrorSourceDescInfo->ErrSourceDescCount,
+    ErrorSourceDescInfo->ErrSourceDescSize
+    ));
+
+  //
+  // Append the error source descriptors to HEST table using the HEST table
+  // generation protocol.
+  //
+  Status = mHestProtocol->AppendErrorSourceDescriptors (
+                            ErrorSourceDescInfo->ErrSourceDescList,
+                            ErrorSourceDescInfo->ErrSourceDescSize,
+                            ErrorSourceDescInfo->ErrSourceDescCount
+                            );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Failed to append error source(s), status: %r\n",
+      __FUNCTION__,
+      Status
+      ));
+  }
+
+  FreePool (CommunicationHeader);
+  return Status;
+}
+
+/**
+  The Entry Point for HEST Error Source Dxe driver.
+
+  Locates the HEST Table generation and MM Communication2 protocols. Using the
+  MM Communication2, the driver collects the Error Source Descriptor(s) from
+  Standalone MM. It then appends those Error Source Descriptor(s) to the Hest
+  table using the HEST Table generation protocol.
+
+  @param[in]  ImageHandle  The firmware allocated handle for the Efi image.
+  @param[in]  SystemTable  A pointer to the Efi System Table.
+
+  @retval  EFI_SUCCESS  The entry point is executed successfully.
+  @retval  Other        Some error occurred when executing this entry point.
+**/
+EFI_STATUS
+EFIAPI
+HestErrorSourceInitialize (
+  IN EFI_HANDLE       ImageHandle,
+  IN EFI_SYSTEM_TABLE *SystemTable
+  )
+{
+  EFI_STATUS Status;
+
+  Status = gBS->LocateProtocol (
+                  &gHestTableProtocolGuid,
+                  NULL,
+                  (VOID **)&mHestProtocol
+                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Failed to locate HEST table generation protocol, status:%r\n",
+      __FUNCTION__,
+      Status
+      ));
+    return Status;
+  }
+
+  Status = gBS->LocateProtocol (
+                  &gEfiMmCommunication2ProtocolGuid,
+                  NULL,
+                  (VOID **)&mMmCommunication2
+                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Failed to locate MMCommunication2 driver protocol, status:%r\n",
+      __FUNCTION__,
+      Status
+      ));
+    return Status;
+  }
+
+  //
+  // Append HEST error sources retrieved from StandaloneMM, if any, into the
+  // HEST ACPI table.
+  //
+  Status = AppendMmErrorSources ();
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Failed appending error source desc to HEST table, status:%r\n",
+      __FUNCTION__,
+      Status
+      ));
+  }
+  return EFI_SUCCESS;
+}
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c
new file mode 100644
index 000000000000..c7b2304fc494
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c
@@ -0,0 +1,312 @@
+/** @file
+  MM HEST error source gateway driver.
+
+  This MM driver installs a handler which can be used to retrieve the error
+  source descriptors from the all MM drivers implementing the HEST error source
+  descriptor protocol.
+
+  The MM driver acts as a single point of contact to collect secure hardware
+  error sources from the MM drivers. It loops over all the MM drivers that
+  implement HEST error source descriptor protocol and collects error source
+  descriptor information along with the error source count and length.
+
+  Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Base.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Protocol/HestErrorSourceInfo.h>
+
+#include "HestMmErrorSourceCommon.h"
+
+STATIC EFI_MM_SYSTEM_TABLE *mMmst = NULL;
+
+/**
+  Returns an array of handles that implement the HEST error source descriptor
+  protocol.
+
+  Passing HandleBuffer as NULL will return the actual size of the buffer
+  required to hold the array of handles implementing the protocol.
+
+  @param[in, out]  HandleBufferSize  The size of the HandleBuffer.
+  @param[out]      HandleBuffer      A pointer to the buffer containing the list
+                                    of handles.
+
+  @retval  EFI_SUCCESS    The array of handles returned in HandleBuffer.
+  @retval  EFI_NOT_FOUND  No implementation present for the protocol.
+  @retval  Other          For any other error.
+**/
+STATIC
+EFI_STATUS
+GetHestErrorSourceProtocolHandles (
+  IN OUT UINTN      *HandleBufferSize,
+  OUT    EFI_HANDLE **HandleBuffer
+  )
+{
+  EFI_STATUS Status;
+
+  Status = mMmst->MmLocateHandle (
+                    ByProtocol,
+                    &gMmHestErrorSourceDescProtocolGuid,
+                    NULL,
+                    HandleBufferSize,
+                    *HandleBuffer
+                    );
+  if ((EFI_ERROR (Status)) &&
+      (Status != EFI_BUFFER_TOO_SMALL))
+  {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: No implementation of MmHestErrorSourceDescProtocol found, \
+       Status:%r\n",
+      __FUNCTION__,
+      Status
+      ));
+    return EFI_NOT_FOUND;
+  }
+
+  return Status;
+}
+
+/**
+  Mmi handler to retrieve HEST error source descriptor information.
+
+  Handler for Mmi service that returns the supported HEST error source
+  descriptors in MM. This handler populates the CommBuffer with the
+  list of all error source descriptors, prepended with the length and
+  the number of descriptors populated into CommBuffer.
+
+  @param[in]       DispatchHandle  The unique handle assigned to this handler by
+                                   MmiHandlerRegister().
+  @param[in]       Context         Points to an optional handler context that
+                                   is specified when the handler was registered.
+  @param[in, out]  CommBuffer      Buffer used for communication of HEST error
+                                   source descriptors.
+  @param[in, out]  CommBufferSize  The size of the CommBuffer.
+
+  @retval  EFI_SUCCESS            CommBuffer has valid data.
+  @retval  EFI_BAD_BUFFER_SIZE    CommBufferSize not adequate.
+  @retval  EFI_OUT_OF_RESOURCES   System out of memory resources.
+  @retval  EFI_INVALID_PARAMETER  Invalid CommBufferSize recieved.
+  @retval  Other                  For any other error.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+HestErrorSourcesInfoMmiHandler (
+  IN     EFI_HANDLE DispatchHandle,
+  IN     CONST VOID *Context,       OPTIONAL
+  IN OUT VOID       *CommBuffer,    OPTIONAL
+  IN OUT UINTN      *CommBufferSize OPTIONAL
+  )
+{
+  MM_HEST_ERROR_SOURCE_DESC_PROTOCOL *HestErrSourceDescProtocolHandle;
+  HEST_ERROR_SOURCE_DESC_INFO        *ErrorSourceInfoList;
+  EFI_HANDLE                         *HandleBuffer;
+  EFI_STATUS                         Status;
+  UINTN                              HandleCount;
+  UINTN                              HandleBufferSize;
+  UINTN                              Index;
+  UINTN                              SourceCount = 0;
+  UINTN                              SourceLength = 0;
+  VOID                               *ErrorSourcePtr;
+  UINTN                              TotalSourceLength = 0;
+  UINTN                              TotalSourceCount = 0;
+
+  if (*CommBufferSize < HEST_ERROR_SOURCE_DESC_INFO_SIZE) {
+    //
+    // Ensures that the communication buffer has enough space to atleast hold
+    // the ErrSourceDescCount and ErrSourceDescSize elements of the
+    // HEST_ERROR_SOURCE_DESC_INFO structure.
+    //
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Invalid CommBufferSize parameter\n",
+      __FUNCTION__
+      ));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  //
+  // Get all handles that implement the HEST error source descriptor protocol.
+  // Get the buffer size required to store list of handles for the protocol.
+  //
+  HandleBuffer = NULL;
+  HandleBufferSize = 0;
+  Status = GetHestErrorSourceProtocolHandles (&HandleBufferSize, &HandleBuffer);
+  if ((Status == EFI_NOT_FOUND) ||
+      (HandleBufferSize == 0))
+  {
+    return Status;
+  }
+
+  // Allocate memory for HandleBuffer of size HandleBufferSize.
+  HandleBuffer = AllocatePool (HandleBufferSize);
[SAMI] AllocateZeroPool() ?
+  if (HandleBuffer == NULL) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Failed to allocate memory for HandleBuffer\n",
+      __FUNCTION__
+      ));
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  // Get the list of handles.
+  Status = GetHestErrorSourceProtocolHandles (&HandleBufferSize, &HandleBuffer);
+  if ((EFI_ERROR (Status)) ||
+      (HandleBuffer == NULL))
[SAMI] Is check for HandleBuffer == NULL right here?
+  {
+    FreePool (HandleBuffer);
+    return Status;
+  }
+
+  // Count of handles for the protocol.
+  HandleCount = HandleBufferSize / sizeof (EFI_HANDLE);
+
+  //
+  // Loop to get the count and length of the error source descriptors.
+  //
+  // This loop collects and adds the length of error source descriptors and
+  // its count from all the the MM drivers implementing HEST error source.
+  // descriptor protocol. The total length and count values retrieved help
+  // to determine weather the CommBuffer is big enough to hold the descriptor
+  // information.
+  // As mentioned in the HEST error source descriptor protocol definition,
+  // Buffer parameter set to NULL ensures only length and the count values
+  // are returned from the driver and no error source information is copied to
+  // Buffer.
+  //
+  for (Index = 0; Index < HandleCount; ++Index) {
+    Status = mMmst->MmHandleProtocol (
+                      HandleBuffer[Index],
+                      &gMmHestErrorSourceDescProtocolGuid,
+                      (VOID **)&HestErrSourceDescProtocolHandle
+                      );
+    if (EFI_ERROR (Status)) {
+      continue;
+    }
+
+    //
+    // Protocol called with Buffer parameter passed as NULL, must return
+    // error source length and error count for that driver.
+    //
+    Status = HestErrSourceDescProtocolHandle->GetHestErrorSourceDescriptors (
+                                                HestErrSourceDescProtocolHandle,
+                                                NULL,
+                                                &SourceLength,
+                                                &SourceCount
+                                                );
+    if (Status == EFI_INVALID_PARAMETER) {
[SAMI] I think the error handling in this function and the error return implementation in GetHestErrorSourceDescriptors() could be improved.
e.g. GetHestErrorSourceDescriptors() could first check for the SourceLength & SourceCount and if it is less than what is required, it returns EFI_BUFFER_TOO_SMALL.
The next check would be to check ErrorSourcePtr and return EFI_INVALID_PARAMETER if it is NULL.
 [/SAMI]
+      TotalSourceLength += SourceLength;
+      TotalSourceCount += SourceCount;
+    }
+  }
+
+  // Set the count and length in the error source descriptor.
+  ErrorSourceInfoList = (HEST_ERROR_SOURCE_DESC_INFO *)(CommBuffer);
+  ErrorSourceInfoList->ErrSourceDescCount = TotalSourceCount;
+  ErrorSourceInfoList->ErrSourceDescSize = TotalSourceLength;
+
+  //
+  // Check the size of CommBuffer, it should atleast be of size
+  // TotalSourceLength + HEST_ERROR_SOURCE_DESC_INFO_SIZE.
+  //
+  TotalSourceLength = TotalSourceLength + HEST_ERROR_SOURCE_DESC_INFO_SIZE;
+  if ((*CommBufferSize) < TotalSourceLength) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Invalid CommBufferSize parameter\n",
+      __FUNCTION__
+      ));
+    FreePool (HandleBuffer);
+    return EFI_BAD_BUFFER_SIZE;
[SAMI] Should the return code be EFI_BUFFER_TOO_SMALL? The difference being, the caller can attempt to call again with a larger buffer if EFI_BUFFER_TOO_SMALL is returned.
CommBufferSize is declared as an OUT paramter, was the intent to return the required buffer size?
[/SAMI]
+  }
+
+  //
+  // CommBuffer size is adequate to return all the error source descriptors.
+  // So go ahead and populate it with the error source descriptor information.
+  //
+
+  // Buffer pointer to append the Error Descriptors data.
+  ErrorSourcePtr =  ErrorSourceInfoList->ErrSourceDescList;
+
+  //
+  // Loop to retrieve error source descriptors information.
+  //
+  // Calls into each MM driver that implement the HEST error source descriptor
+  // protocol. Here the Buffer parameter passed to the protocol service is
+  // valid. So the MM driver when called copies the descriptor information.
+  //
+  for (Index = 0; Index < HandleCount; ++Index) {
+    Status = mMmst->MmHandleProtocol (
+                      HandleBuffer[Index],
+                      &gMmHestErrorSourceDescProtocolGuid,
+                      (VOID **)&HestErrSourceDescProtocolHandle
+                      );
+    if (EFI_ERROR (Status)) {
+      continue;
+    }
+
+    Status = HestErrSourceDescProtocolHandle->GetHestErrorSourceDescriptors (
+                                                HestErrSourceDescProtocolHandle,
+                                                (VOID **)&ErrorSourcePtr,
+                                                &SourceLength,
+                                                &SourceCount
+                                                );
+    if (Status == EFI_SUCCESS) {
+      ErrorSourcePtr += SourceLength;
+    }
+  }
+
+  // Free the buffer holding all the protocol handles.
+  FreePool (HandleBuffer);
+
+  return EFI_SUCCESS;
[SAMI] return Status of last operation.
+}
+
+/**
+  Entry point for this Stanalone MM driver.
+
+  Registers an Mmi handler that retrieves the error source descriptors from all
+  the MM drivers implementing the MM_HEST_ERROR_SOURCE_DESC_PROTOCOL.
+
+  @param[in]  ImageHandle  The firmware allocated handle for the EFI image.
+  @param[in]  SystemTable  A pointer to the EFI System Table.
+
+  @retval  EFI_SUCCESS  The entry point registered handler successfully.
+  @retval  Other        Some error occurred when executing this entry point.
+**/
+EFI_STATUS
+EFIAPI
+StandaloneMmHestErrorSourceInitialize (
+  IN EFI_HANDLE          ImageHandle,
+  IN EFI_MM_SYSTEM_TABLE *SystemTable
+  )
+{
+  EFI_HANDLE DispatchHandle;
+  EFI_STATUS Status;
+
+  ASSERT (SystemTable != NULL);
+  mMmst = SystemTable;
+
+  Status = mMmst->MmiHandlerRegister (
+                    HestErrorSourcesInfoMmiHandler,
+                    &gMmHestGetErrorSourceInfoGuid,
+                    &DispatchHandle
+                    );
+  if (EFI_ERROR(Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Mmi handler registration failed with status : %r\n",
+      __FUNCTION__,
+      Status
+      ));
+    return Status;
+  }
+
+  return EFI_SUCCESS;
[SAMI] return Status of last operation.
+}


Re: [PATCH v2 2/4] ArmPlatformPkg: add definition for MM_HEST_ERROR_SOURCE_DESC_PROTOCOL

Sami Mujawar
 

Hi Omkar,

Please find my response below marked [SAMI]

Regards,

Sami Mujawar
On 10/07/2021 05:18 PM, Omkar Anand Kulkarni wrote:
Add the protocol definition of the MM_HEST_ERROR_SOURCE_DESC_PROTOCOL
protocol. This protocol can be implemented by MM drivers to publish
error source descriptors that have to be populated into HEST table.

Co-authored-by: Thomas Abraham <thomas.abraham@...>
Signed-off-by: Omkar Anand Kulkarni <omkar.kulkarni@...>
---
 ArmPlatformPkg/ArmPlatformPkg.dec                     |  1 +
 ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h | 64 ++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
index e4afe5da8e11..4f062292663b 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -131,3 +131,4 @@
 [Protocols.common]
   ## Arm Platform HEST table generation protocol
   gHestTableProtocolGuid = { 0x705bdcd9, 0x8c47, 0x457e, { 0xad, 0x0d, 0xf7, 0x86, 0xf3, 0x4a, 0x0d, 0x63 } }
+  gMmHestErrorSourceDescProtocolGuid = { 0x560bf236, 0xa4a8, 0x4d69, { 0xbc, 0xf6, 0xc2, 0x97, 0x24, 0x10, 0x9d, 0x91 } }
diff --git a/ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h b/ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h
new file mode 100644
index 000000000000..95afd4dffe9c
--- /dev/null
+++ b/ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h
@@ -0,0 +1,64 @@
+/** @file
+  MM protocol to get the secure error source descriptor information.
+
+  MM Drivers must implement this protocol in order to publish secure side
+  error source descriptor information to OSPM through the HEST ACPI table.
+
+  Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef MM_HEST_ERROR_SOURCE_DESC_
+#define MM_HEST_ERROR_SOURCE_DESC_
+
+#define MM_HEST_ERROR_SOURCE_DESC_PROTOCOL_GUID \
+  { \
+    0x560bf236, 0xa4a8, 0x4d69, { 0xbc, 0xf6, 0xc2, 0x97, 0x24, 0x10, 0x9d, 0x91 } \
+  }
+
+typedef struct MM_HEST_ERROR_SOURCE_DESC_PROTOCOL_
[SAMI] Not sure if a trailing underscore would be right to use for the name tag. Can MmHestErrorSourceDescProtocol be used as the name tag?
Also see https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/56_declarations_and_types#5-6-3-2-structure-declaration-with-forward-reference-or-self-reference
+                 MM_HEST_ERROR_SOURCE_DESC_PROTOCOL;
+
+/**
+  Get HEST Secure Error Source Descriptors.
+
+  The MM drivers implementing this protocol must convey the total count and
+  total length of the error sources the driver has along with the actual error
+  source descriptor(s).
+
+  Passing NULL as Buffer parameter shall return EFI_INVALID_PARAMETR with the
+  total length and count of the error source descriptor(s) it supports.
+
+  @param[in]   This                MM_HEST_ERROR_SOURCE_DESC_PROTOCOL instance.
+  @param[out]  Buffer              Buffer to be appended with the error
+                                   source descriptors information.
+  @param[out]  ErrorSourcesLength  Total length of all the error source
+                                   descriptors.
+  @param[out]  ErrorSourceCount    Count of total error source descriptors
+                                   supported by the driver.
+
+  retval  EFI_SUCCESS           If the Buffer is valid and is filled with valid
+                                Error Source descriptor data.
+  retval  EFI_INVALID_PARAMTER  Buffer is NULL.
+  retval  Other                 If no error source descriptor information is
+                                available.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *MM_HEST_GET_ERROR_SOURCE_DESCRIPTORS) (
+  IN  MM_HEST_ERROR_SOURCE_DESC_PROTOCOL *This,
+  OUT VOID                               **Buffer,
+  OUT UINTN                              *ErrorSourcesLength,
+  OUT UINTN                              *ErrorSourcesCount
+  );
+
+//
+// Protocol declaration
+//
+struct MM_HEST_ERROR_SOURCE_DESC_PROTOCOL_ {
+  MM_HEST_GET_ERROR_SOURCE_DESCRIPTORS GetHestErrorSourceDescriptors;
+};
+
+extern EFI_GUID gMmHestErrorSourceDescProtocolGuid;
+
+#endif // MM_HEST_ERROR_SOURCE_DESC_


16061 - 16080 of 94602