Date   

Re: [edk2-platforms][PATCH v3 27/41] KabylakeSiliconPkg: Remove SmmSpiFlashCommonLib

Chiu, Chasel
 

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

-----Original Message-----
From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
Sent: Friday, June 18, 2021 10:07 AM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Chaganty, Rangasai V
<rangasai.v.chaganty@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>
Subject: [edk2-platforms][PATCH v3 27/41] KabylakeSiliconPkg: Remove
SmmSpiFlashCommonLib

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

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

The library has been consolidated with instances in other Intel silicon packages
as a single instance in IntelSiliconPkg

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
---

Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiFlashC
ommon.c | 196 --------------------

Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiFlashC
ommonSmmLib.c | 54 ------
Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Library/SpiFlashCommonLib.h
| 98 ----------

Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SmmSpiFl
ashCommonLib.inf | 53 ------
4 files changed, 401 deletions(-)

diff --git
a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiFlas
hCommon.c
b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiFlas
hCommon.c
deleted file mode 100644
index 7ee7ffab5001..000000000000
---
a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiFlas
hCommon.c
+++ /dev/null
@@ -1,196 +0,0 @@
-/** @file
- Wrap EFI_SPI_PROTOCOL to provide some library level interfaces
- for module use.
-
-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
-SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#include <Library/SpiFlashCommonLib.h>
-#include <Library/IoLib.h>
-#include <Library/PciLib.h>
-#include <PchAccess.h>
-#include <Library/MmPciLib.h>
-#include <Protocol/Spi.h>
-
-
-PCH_SPI_PROTOCOL *mSpiProtocol;
-
-//
-// FlashAreaBaseAddress and Size for boottime and runtime usage.
-//
-UINTN mFlashAreaBaseAddress = 0;
-UINTN mFlashAreaSize = 0;
-
-/**
- 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 read.
- @param[in,out] NumBytes On input, the number of bytes to read. On
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 ((NumBytes != NULL) && (Buffer != NULL));
- if ((NumBytes == NULL) || (Buffer == NULL)) {
- return EFI_INVALID_PARAMETER;
- }
-
- //
- // This function is implemented specifically for those platforms
- // at which the SPI device is memory mapped for read. So this
- // function just do a memory copy for Spi Flash Read.
- //
- CopyMem (Buffer, (VOID *) Address, *NumBytes);
-
- 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 write.
- @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
- )
-{
- EFI_STATUS Status;
- UINTN Offset;
- UINT32 Length;
- UINT32 RemainingBytes;
-
- ASSERT ((NumBytes != NULL) && (Buffer != NULL));
- if ((NumBytes == NULL) || (Buffer == NULL)) {
- return EFI_INVALID_PARAMETER;
- }
-
- ASSERT (Address >= mFlashAreaBaseAddress);
-
- Offset = Address - mFlashAreaBaseAddress;
-
- ASSERT ((*NumBytes + Offset) <= mFlashAreaSize);
-
- Status = EFI_SUCCESS;
- RemainingBytes = *NumBytes;
-
-
- while (RemainingBytes > 0) {
- if (RemainingBytes > SECTOR_SIZE_4KB) {
- Length = SECTOR_SIZE_4KB;
- } else {
- Length = RemainingBytes;
- }
- Status = mSpiProtocol->FlashWrite (
- mSpiProtocol,
- FlashRegionBios,
- (UINT32) Offset,
- Length,
- Buffer
- );
- if (EFI_ERROR (Status)) {
- break;
- }
- RemainingBytes -= Length;
- Offset += Length;
- Buffer += Length;
- }
-
- //
- // Actual number of bytes written
- //
- *NumBytes -= RemainingBytes;
-
- return Status;
-}
-
-/**
- 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 that the PAddress
- is at the starting address of this block.
- @param[in] NumBytes On input, the number of bytes of the logical block
to be erased.
- On output, the actual number of bytes erased.
-
- @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
- )
-{
- EFI_STATUS Status;
- UINTN Offset;
- UINTN RemainingBytes;
-
- ASSERT (NumBytes != NULL);
- if (NumBytes == NULL) {
- return EFI_INVALID_PARAMETER;
- }
-
- ASSERT (Address >= mFlashAreaBaseAddress);
-
- Offset = Address - mFlashAreaBaseAddress;
-
- ASSERT ((*NumBytes % SECTOR_SIZE_4KB) == 0);
- ASSERT ((*NumBytes + Offset) <= mFlashAreaSize);
-
- Status = EFI_SUCCESS;
- RemainingBytes = *NumBytes;
-
-
- Status = mSpiProtocol->FlashErase (
- mSpiProtocol,
- FlashRegionBios,
- (UINT32) Offset,
- (UINT32) RemainingBytes
- );
- return Status;
-}
-
diff --git
a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiFlas
hCommonSmmLib.c
b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiFlas
hCommonSmmLib.c
deleted file mode 100644
index 11133163d2d4..000000000000
---
a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiFlas
hCommonSmmLib.c
+++ /dev/null
@@ -1,54 +0,0 @@
-/** @file
- SMM 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 <Library/SpiFlashCommonLib.h>
-#include <Library/SmmServicesTableLib.h> -#include <Protocol/Spi.h>
-
-extern PCH_SPI_PROTOCOL *mSpiProtocol;
-
-extern UINTN mFlashAreaBaseAddress;
-extern UINTN mFlashAreaSize;
-
-/**
- The library constructuor.
-
- The function does the necessary initialization work for this library
- instance.
-
- @param[in] ImageHandle The firmware allocated handle for the UEFI
image.
- @param[in] SystemTable A pointer to the EFI system table.
-
- @retval EFI_SUCCESS The function always return EFI_SUCCESS for now.
- It will ASSERT on error for debug version.
- @retval EFI_ERROR Please reference LocateProtocol for error code
details.
-**/
-EFI_STATUS
-EFIAPI
-SmmSpiFlashCommonLibConstructor (
- IN EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE *SystemTable
- )
-{
- EFI_STATUS Status;
-
- mFlashAreaBaseAddress = (UINTN)PcdGet32 (PcdFlashAreaBaseAddress);
- mFlashAreaSize = (UINTN)PcdGet32 (PcdFlashAreaSize);
-
- //
- // Locate the SMM SPI protocol.
- //
- Status = gSmst->SmmLocateProtocol (
- &gPchSmmSpiProtocolGuid,
- NULL,
- (VOID **) &mSpiProtocol
- );
- ASSERT_EFI_ERROR (Status);
-
- return Status;
-}
diff --git
a/Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Library/SpiFlashCommonLib.h
b/Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Library/SpiFlashCommonLib.h
deleted file mode 100644
index 0c5e72258c2d..000000000000
--- a/Silicon/Intel/KabylakeSiliconPkg/Pch/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 read.
- @param[in,out] NumBytes On input, the number of bytes to read. On
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 write.
- @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 that the PAddress
- is at the starting address of this block.
- @param[in] NumBytes On input, the number of bytes of the logical block
to be erased.
- On output, the actual number of bytes erased.
-
- @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/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SmmSp
iFlashCommonLib.inf
b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SmmSp
iFlashCommonLib.inf
deleted file mode 100644
index d712b9e5f769..000000000000
---
a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SmmSp
iFlashCommonLib.inf
+++ /dev/null
@@ -1,53 +0,0 @@
-### @file
-# SMM Library 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 = 0x00010017
- BASE_NAME = SmmSpiFlashCommonLib
- FILE_GUID = 9632D96E-E849-4217-9217-DC500B8AAE47
- VERSION_STRING = 1.0
- MODULE_TYPE = DXE_SMM_DRIVER
- LIBRARY_CLASS = SpiFlashCommonLib|DXE_SMM_DRIVER
- CONSTRUCTOR = SmmSpiFlashCommonLibConstructor
-#
-# The following information is for reference only and not required by the build
tools.
-#
-# VALID_ARCHITECTURES = IA32 X64
-#
-
-[LibraryClasses]
- PciLib
- IoLib
- MemoryAllocationLib
- BaseLib
- UefiLib
- SmmServicesTableLib
- BaseMemoryLib
- DebugLib
- MmPciLib
-
-[Packages]
- MdePkg/MdePkg.dec
- KabylakeSiliconPkg/SiPkg.dec
-
-[Pcd]
- gSiPkgTokenSpaceGuid.PcdFlashAreaBaseAddress ## CONSUMES
- gSiPkgTokenSpaceGuid.PcdFlashAreaSize ## CONSUMES
- gSiPkgTokenSpaceGuid.PcdBiosGuardEnable ## CONSUMES
-
-[Sources]
- SpiFlashCommonSmmLib.c
- SpiFlashCommon.c
-
-[Protocols]
- gPchSmmSpiProtocolGuid ## CONSUMES
- gSmmBiosGuardProtocolGuid ## CONSUMES
-
-[Depex.X64.DXE_SMM_DRIVER]
- gPchSmmSpiProtocolGuid
--
2.28.0.windows.1


[PATCH v1 2/2] MdePkg: MmConfiguration: Added definition of MM Configuration PPI

Kun Qin
 

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

MM Configuration PPI was defined in PI Specification since v1.5. This
change added definition of such PPI and related GUIDs into MdePkg.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>

Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
MdePkg/Include/Ppi/MmConfiguration.h | 62 ++++++++++++++++++++
MdePkg/MdePkg.dec | 3 +
2 files changed, 65 insertions(+)

diff --git a/MdePkg/Include/Ppi/MmConfiguration.h b/MdePkg/Include/Ppi/MmConfiguration.h
new file mode 100644
index 000000000000..f950322b3877
--- /dev/null
+++ b/MdePkg/Include/Ppi/MmConfiguration.h
@@ -0,0 +1,62 @@
+/** @file
+ EFI MM Configuration PPI as defined in PI 1.5 specification.
+
+ This PPI is used to:
+ 1) report the portions of MMRAM regions which cannot be used for the MMRAM heap.
+ 2) register the MM Foundation entry point with the processor code. The entry
+ point will be invoked by the MM processor entry code.
+
+ Copyright (c) Microsoft Corporation.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef MM_CONFIGURATION_PPI_H_
+#define MM_CONFIGURATION_PPI_H_
+
+#include <Pi/PiMmCis.h>
+
+#define EFI_PEI_MM_CONFIGURATION_PPI_GUID \
+ { \
+ 0xc109319, 0xc149, 0x450e, { 0xa3, 0xe3, 0xb9, 0xba, 0xdd, 0x9d, 0xc3, 0xa4 } \
+ }
+
+typedef struct _EFI_PEI_MM_CONFIGURATION_PPI EFI_PEI_MM_CONFIGURATION_PPI;
+
+/**
+ This function registers the MM Foundation entry point with the processor code. This entry point will be
+ invoked by the MM Processor entry code as defined in PI specification.
+
+ @param[in] This The EFI_PEI_MM_CONFIGURATION_PPI instance.
+ @param[in] MmEntryPoint MM Foundation entry point.
+
+ @retval EFI_SUCCESS The entry-point was successfully registered.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EFI_PEI_MM_REGISTER_MM_ENTRY) (
+ IN CONST EFI_PEI_MM_CONFIGURATION_PPI *This,
+ IN EFI_MM_ENTRY_POINT MmEntryPoint
+ );
+
+///
+/// This PPI is a PPI published by a CPU PEIM to indicate which areas within MMRAM are reserved for use by
+/// the CPU for any purpose, such as stack, save state or MM entry point. If a platform chooses to let a CPU
+/// PEIM do MMRAM relocation, this PPI must be produced by this CPU PEIM.
+///
+/// The MmramReservedRegions points to an array of one or more EFI_MM_RESERVED_MMRAM_REGION structures, with
+/// the last structure having the MmramReservedSize set to 0. An empty array would contain only the last
+/// structure.
+///
+/// The RegisterMmEntry() function allows the MM IPL PEIM to register the MM Foundation entry point with the
+/// MM entry vector code.
+///
+struct _EFI_PEI_MM_CONFIGURATION_PPI {
+ EFI_MM_RESERVED_MMRAM_REGION *MmramReservedRegions;
+ EFI_PEI_MM_REGISTER_MM_ENTRY RegisterMmEntry;
+};
+
+extern EFI_GUID gEfiPeiMmConfigurationPpi;
+
+#endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index b49f88d8e18f..c5319fdd71ca 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -983,6 +983,9 @@ [Ppis]
## Include/Ppi/MmControl.h
gEfiPeiMmControlPpiGuid = { 0x61c68702, 0x4d7e, 0x4f43, { 0x8d, 0xef, 0xa7, 0x43, 0x5, 0xce, 0x74, 0xc5 }}

+ ## Include/Ppi/MmConfiguration.h
+ gEfiPeiMmConfigurationPpi = { 0xc109319, 0xc149, 0x450e, { 0xa3, 0xe3, 0xb9, 0xba, 0xdd, 0x9d, 0xc3, 0xa4 } }
+
#
# PPIs defined in PI 1.7.
#
--
2.31.1.windows.1


[PATCH v1 1/2] MdePkg: MmConfiguration: Moved EFI_MM_RESERVED_MMRAM_REGION to PiMmCis.h

Kun Qin
 

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

The definition of EFI_MM_RESERVED_MMRAM_REGION, according to PI Spec 1.5
is also referenced in EFI_PEI_MM_CONFIGURATION_PPI. Defining this
structure as is will enforce any potential usage of MM Configuration PPI
interface to include <Protocol/MmConfiguration.h>.

This change moves EFI_MM_RESERVED_MMRAM_REGION definition into PiMmCis.h,
which is already included in Protocol/MmConfiguration.h. It also paves
way for introducing Ppi/MmConfiguration.h with proper dependency.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>

Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
MdePkg/Include/Pi/PiMmCis.h | 16 ++++++++++++++++
MdePkg/Include/Protocol/MmConfiguration.h | 16 ----------------
2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/MdePkg/Include/Pi/PiMmCis.h b/MdePkg/Include/Pi/PiMmCis.h
index fdf0591a03d6..422a3ea6c2bb 100644
--- a/MdePkg/Include/Pi/PiMmCis.h
+++ b/MdePkg/Include/Pi/PiMmCis.h
@@ -242,6 +242,22 @@ VOID
IN CONST EFI_MM_ENTRY_CONTEXT *MmEntryContext
);

+///
+/// Structure describing a MMRAM region which cannot be used for the MMRAM heap.
+///
+typedef struct _EFI_MM_RESERVED_MMRAM_REGION {
+ ///
+ /// Starting address of the reserved MMRAM area, as it appears while MMRAM is open.
+ /// Ignored if MmramReservedSize is 0.
+ ///
+ EFI_PHYSICAL_ADDRESS MmramReservedStart;
+ ///
+ /// Number of bytes occupied by the reserved MMRAM area. A size of zero indicates the
+ /// last MMRAM area.
+ ///
+ UINT64 MmramReservedSize;
+} EFI_MM_RESERVED_MMRAM_REGION;
+
///
/// Management Mode System Table (MMST)
///
diff --git a/MdePkg/Include/Protocol/MmConfiguration.h b/MdePkg/Include/Protocol/MmConfiguration.h
index eeb94f64bdf7..d2fb6a13d4af 100644
--- a/MdePkg/Include/Protocol/MmConfiguration.h
+++ b/MdePkg/Include/Protocol/MmConfiguration.h
@@ -21,22 +21,6 @@
0x26eeb3de, 0xb689, 0x492e, {0x80, 0xf0, 0xbe, 0x8b, 0xd7, 0xda, 0x4b, 0xa7 } \
}

-///
-/// Structure describing a MMRAM region which cannot be used for the MMRAM heap.
-///
-typedef struct _EFI_MM_RESERVED_MMRAM_REGION {
- ///
- /// Starting address of the reserved MMRAM area, as it appears while MMRAM is open.
- /// Ignored if MmramReservedSize is 0.
- ///
- EFI_PHYSICAL_ADDRESS MmramReservedStart;
- ///
- /// Number of bytes occupied by the reserved MMRAM area. A size of zero indicates the
- /// last MMRAM area.
- ///
- UINT64 MmramReservedSize;
-} EFI_MM_RESERVED_MMRAM_REGION;
-
typedef struct _EFI_MM_CONFIGURATION_PROTOCOL EFI_MM_CONFIGURATION_PROTOCOL;

/**
--
2.31.1.windows.1


[PATCH v1 0/2] Add MM Configuration PPI definition to MdePkg

Kun Qin
 

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

EFI_PEI_MM_CONFIGURATION_PPI is defined since PI spec v1.5. This patch
series added the interface definition and related GUIDs into MdePkg.

On the other hand, EFI_MM_RESERVED_MMRAM_REGION is referenced by both
MM Configuration PPI and Protocol definitions, but currently defined in
Protocol/MmConfiguration.h. To avoid data strcuture entanglement during
usage, the EFI_MM_RESERVED_MMRAM_REGION definition is moved to PiMmCis.h
for common access for both MM Configuration PPI and Protocol.

Patch v1 branch: https://github.com/kuqin12/edk2/tree/mm_config_ppi_v1

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>

Kun Qin (2):
MdePkg: MmConfiguration: Moved EFI_MM_RESERVED_MMRAM_REGION to
PiMmCis.h
MdePkg: MmConfiguration: Added definition of MM Configuration PPI

MdePkg/Include/Pi/PiMmCis.h | 16 +++++
MdePkg/Include/Ppi/MmConfiguration.h | 62 ++++++++++++++++++++
MdePkg/Include/Protocol/MmConfiguration.h | 16 -----
MdePkg/MdePkg.dec | 3 +
4 files changed, 81 insertions(+), 16 deletions(-)
create mode 100644 MdePkg/Include/Ppi/MmConfiguration.h

--
2.31.1.windows.1


Re: [edk2-test][PATCH 1/1] SctPkg: Consume MdeLibs.dsc.inc for RegisterFilterLib

Sunny Wang
 

Looks good to me.
Reviewed-by: Sunny Wang <sunny.wang@arm.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gao Jie via groups.io
Sent: Thursday, June 10, 2021 3:39 PM
To: devel@edk2.groups.io
Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Eric Jin <eric.jin@intel.com>; Arvin Chen <arvinx.chen@intel.com>
Subject: [edk2-devel] [edk2-test][PATCH 1/1] SctPkg: Consume MdeLibs.dsc.inc for RegisterFilterLib

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

MdeLibs.dsc.inc was added for some basic/default library
instances provided by MdePkg, RegisterFilterLibNull which will be
consumed by IoLib and BaseLib was added in MdeLibs.dsc.inc.

To build UefiSct with edk2-stable202105 and later, this file
must be included in dsc file.

Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Cc: Eric Jin <eric.jin@intel.com>
Cc: Arvin Chen <arvinx.chen@intel.com>

Signed-off-by: Barton Gao <gaojie@byosoft.com.cn>
---
uefi-sct/SctPkg/UEFI/IHV_SCT.dsc | 2 ++
uefi-sct/SctPkg/UEFI/UEFI_SCT.dsc | 2 ++
2 files changed, 4 insertions(+)

diff --git a/uefi-sct/SctPkg/UEFI/IHV_SCT.dsc b/uefi-sct/SctPkg/UEFI/IHV_SCT.dsc
index 3371141f..7a4393e0 100644
--- a/uefi-sct/SctPkg/UEFI/IHV_SCT.dsc
+++ b/uefi-sct/SctPkg/UEFI/IHV_SCT.dsc
@@ -136,6 +136,8 @@

[Libraries.IA32,Libraries.X64]

+!include MdePkg/MdeLibs.dsc.inc
+
[LibraryClasses.common]
UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
diff --git a/uefi-sct/SctPkg/UEFI/UEFI_SCT.dsc b/uefi-sct/SctPkg/UEFI/UEFI_SCT.dsc
index 2b4f415f..5b3e5307 100644
--- a/uefi-sct/SctPkg/UEFI/UEFI_SCT.dsc
+++ b/uefi-sct/SctPkg/UEFI/UEFI_SCT.dsc
@@ -138,6 +138,8 @@
[Libraries.RISCV64]
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf

+!include MdePkg/MdeLibs.dsc.inc
+
[LibraryClasses.common]
UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
--
2.31.0.windows.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: [PATCH 1/6] NetworkPkg/IScsiDxe: re-set session-level authentication state before login

Philippe Mathieu-Daudé
 

On 6/8/21 3:06 PM, Laszlo Ersek wrote:
RFC 7143 explains that a single iSCSI session may use multiple TCP
connections. The first connection established is called the leading
connection. The login performed on the leading connection is called the
leading login. Before the session is considered full-featured, the leading
login must succeed. Further (non-leading) connections can be associated
with the session later.

(It's unclear to me from RFC 7143 whether the non-leading connections
require individual (non-leading) logins as well, but that particular
question is irrelevant from the perspective of this patch; see below.)

The data model in IScsiDxe exhibits some confusion, regarding connection /
session association:

- On one hand, the "ISCSI_SESSION.Conns" field is a *set* (it has type
LIST_ENTRY), and accordingly, connections can be added to, and removed
from, a session, with the IScsiAttatchConnection() and
IScsiDetatchConnection() functions.

- On the other hand, ISCSI_MAX_CONNS_PER_SESSION has value 1, therefore no
session will ever use more than 1 connection at a time (refer to
instances of "Session->MaxConnections" in
"NetworkPkg/IScsiDxe/IScsiProto.c").

This one-to-many confusion between ISCSI_SESSION and ISCSI_CONNECTION is
very visible in the CHAP logic, where the progress of the authentication
is maintained *per connection*, in the "ISCSI_CONNECTION.AuthStep" field
(with values such as ISCSI_AUTH_INITIAL, ISCSI_CHAP_STEP_ONE, etc), but
the *data* for the authentication are maintained *per session*, in the
"AuthType" and "AuthData" fields of ISCSI_SESSION. Clearly, this makes no
sense if multiple connections are eligible for logging in.

Knowing that IScsiDxe uses only one connection per session (put
differently: knowing that any connection is a leading connection, and any
login is a leading login), there is no functionality bug. But the data
model is still broken: "AuthType", "AuthData", and "AuthStep" should be
maintained at the *same* level -- be it "session-level" or "(leading)
connection-level".

Fixing this data model bug is more than what I'm signing up for. However,
I do need to add one function, in preparation for multi-hash support:
whenever a new login is attempted (put differently: whenever the leading
login is re-attempted), which always happens with a fresh connection, the
session-level authentication data needs to be rewound to a sane initial
state.

Introduce the IScsiSessionResetAuthData() function. Call it from the
central -- session-level -- IScsiSessionLogin() function, just before the
latter calls the -- connection-level -- IScsiConnLogin() function.

Right now, do nothing in IScsiSessionResetAuthData(); so functionally
speaking, the patch is a no-op.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3355
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
NetworkPkg/IScsiDxe/IScsiProto.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>


Re: [PATCH v1 0/5] EDK2 Code First: PI Specification: Update EFI_MM_COMMUNICATE_HEADER

Marvin Häuser
 

Hey Kun,

On 16.06.21 22:58, Kun Qin wrote:
Hi Marvin,

Thanks for reaching out. Please see my comment inline.

Regards,
Kun

On 06/16/2021 00:02, Marvin Häuser wrote:
Good day,

May I ask about two small things?

1) Was there any rationale as to e.g. code compatibility with choosing UINT64 for the data length? I understand that this is the maximum of the two as of currently, but I wonder whether a message length that exceeds the UINT32 range (4 GB!) can possibly be considered sane or a good idea.
I agree that >4GB communication buffer may not be practical as of today. That is why we modified the SMM communication routine in PiSmmIpl to use SafeInt functions in Patch 2 of this series. With that change, at least for 32bit MM, larger than 4GB will be considered insane. But in the meantime, I do not want to rule out the >4GB communication capability that a 64bit MM currently already have.

Please let me know if I missed anything in regards to adding SafeInt functions to SmmCommunication routine.
I didn't see anything missed, but that is part of the point. If it was UINT32, no such validation would be required. Also I feel like in high-security scenarios, you would have a cap (that is well below 4 GB I imagine) anyway past which you reject transmission for looking insane. I totally understand it's a tough choice to reduce the capabilities and to go with a capacity less than what is possible, but I do not think current transmission realistically exceed a very few megabytes? This would still allow for incredible headroom.


2) Is it feasible yet with the current set of supported compilers to support flexible arrays?
My impression is that flexible arrays are already supported (as seen in UnitTestFrameworkPkg/PrivateInclude/UnitTestFrameworkTypes.h). Please correct me if I am wrong.

Would you mind letting me know why this is applicable here? We are trying to seek ideas on how to catch developer mistakes caused by this change. So any input is appreciated.
Huh, interesting. Last time I tried I was told about incompatibilities with MSVC, but I know some have been dropped since then (2005 and 2008 if I recall correctly?), so that'd be great to allow globally. I feel like if the structure is modified anyway, it should probably get a trailing flexible array over the 1-sized hack. What do you think?

Best regards,
Marvin


Thank you for your work!

Best regards,
Marvin

On 10.06.21 03:42, Kun Qin wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430

In PI Spec v1.7 Errata A, Vol.4, Sec 5.7 MM Communication Protocol, the
MessageLength field of EFI_MM_COMMUNICATE_HEADER (also derived as
EFI_SMM_COMMUNICATE_HEADER) is currently defined as type UINTN.

But this structure, as a generic definition, could be used for both PEI
and DXE MM communication. Thus for a system that supports PEI MM launch,
but operates PEI in 32bit mode and MM foundation in 64bit, the current
EFI_MM_COMMUNICATE_HEADER definition will cause structure parse error due
to UINTN being used.

The suggested change is to make the MessageLength field defined with
definitive size as below:
```
typedef struct {
EFI_GUID  HeaderGuid;
UINT64    MessageLength;
UINT8     Data[ANYSIZE_ARRAY];
} EFI_MM_COMMUNICATE_HEADER;
```

Patch v1 branch: https://github.com/kuqin12/edk2/tree/BZ3398-MmCommunicate-Length

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif@nuviainc.com>

Kun Qin (5):
   EDK2 Code First: PI Specification: EFI_MM_COMMUNICATE_HEADER Update
   MdeModulePkg: PiSmmIpl: Update MessageLength calculation for
     MmCommunicate
   MdeModulePkg: MemoryProfileInfo: Updated MessageLength calculation
   MdeModulePkg: SmiHandlerProfileInfo: Updated MessageLength calculation
   MdePkg: MmCommunication: Extend MessageLength field size to UINT64

MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c | 20 +++--
MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c |  8 +-
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 13 ++-
BZ3430-SpecChange.md | 88 ++++++++++++++++++++
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf |  1 +
MdePkg/Include/Protocol/MmCommunication.h |  3 +-
  6 files changed, 124 insertions(+), 9 deletions(-)
  create mode 100644 BZ3430-SpecChange.md



[PATCH v2 6/6] MdePkg: MmCommunication: Extend MessageLength field size to UINT64

Kun Qin
 

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

The MessageLength field of EFI_MM_COMMUNICATE_HEADER, as a generic
definition, could be used for both PEI and DXE MM communication. On a
system that supports PEI MM launch, but operates PEI in 32bit mode and MM
foundation in 64bit, the current EFI_MM_COMMUNICATE_HEADER definition
will cause structure parse error due to UINTN used.

This change removes the architecture dependent field by extending this
field definition as UINT64.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>

Signed-off-by: Kun Qin <kuqin12@gmail.com>
---

Notes:
v2:
- Removed comments with "BZ"

MdePkg/Include/Protocol/MmCommunication.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdePkg/Include/Protocol/MmCommunication.h b/MdePkg/Include/Protocol/MmCommunication.h
index 34c3e2b5a9e3..d42b00bbeb7c 100644
--- a/MdePkg/Include/Protocol/MmCommunication.h
+++ b/MdePkg/Include/Protocol/MmCommunication.h
@@ -26,7 +26,7 @@ typedef struct {
///
/// Describes the size of Data (in bytes) and does not include the size of the header.
///
- UINTN MessageLength;
+ UINT64 MessageLength;
///
/// Designates an array of bytes that is MessageLength in size.
///
--
2.31.1.windows.1


[PATCH v2 5/6] MdeModulePkg: SmmLockBoxDxeLib: Updated MessageLength calculation

Kun Qin
 

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

This change replaced the calculation of communication buffer size from
explicitly adding the size of each member with the OFFSET macro function.
This will make the structure field defition change transparent to
consumers.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>

Signed-off-by: Kun Qin <kuqin12@gmail.com>
---

Notes:
v2:
- Newly added in v2

MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c | 23 ++++++++++----------
1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
index 2cbffe889e1f..66fbe4dd961c 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
@@ -79,8 +79,7 @@ LockBoxGetSmmCommBuffer (
return mLockBoxSmmCommBuffer;
}

- MinimalSizeNeeded = sizeof (EFI_GUID) +
- sizeof (UINTN) +
+ MinimalSizeNeeded = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) +
MAX (sizeof (EFI_SMM_LOCK_BOX_PARAMETER_SAVE),
MAX (sizeof (EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES),
MAX (sizeof (EFI_SMM_LOCK_BOX_PARAMETER_UPDATE),
@@ -142,7 +141,7 @@ SaveLockBox (
EFI_SMM_COMMUNICATION_PROTOCOL *SmmCommunication;
EFI_SMM_LOCK_BOX_PARAMETER_SAVE *LockBoxParameterSave;
EFI_SMM_COMMUNICATE_HEADER *CommHeader;
- UINT8 TempCommBuffer[sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE)];
+ UINT8 TempCommBuffer[OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE)];
UINT8 *CommBuffer;
UINTN CommSize;

@@ -182,7 +181,7 @@ SaveLockBox (
//
// Send command
//
- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE);
+ CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE);
Status = SmmCommunication->Communicate (
SmmCommunication,
&CommBuffer[0],
@@ -224,7 +223,7 @@ SetLockBoxAttributes (
EFI_SMM_COMMUNICATION_PROTOCOL *SmmCommunication;
EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES *LockBoxParameterSetAttributes;
EFI_SMM_COMMUNICATE_HEADER *CommHeader;
- UINT8 TempCommBuffer[sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES)];
+ UINT8 TempCommBuffer[OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES)];
UINT8 *CommBuffer;
UINTN CommSize;

@@ -264,7 +263,7 @@ SetLockBoxAttributes (
//
// Send command
//
- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES);
+ CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES);
Status = SmmCommunication->Communicate (
SmmCommunication,
&CommBuffer[0],
@@ -314,7 +313,7 @@ UpdateLockBox (
EFI_SMM_COMMUNICATION_PROTOCOL *SmmCommunication;
EFI_SMM_LOCK_BOX_PARAMETER_UPDATE *LockBoxParameterUpdate;
EFI_SMM_COMMUNICATE_HEADER *CommHeader;
- UINT8 TempCommBuffer[sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE)];
+ UINT8 TempCommBuffer[OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE)];
UINT8 *CommBuffer;
UINTN CommSize;

@@ -355,7 +354,7 @@ UpdateLockBox (
//
// Send command
//
- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE);
+ CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_UPDATE);
Status = SmmCommunication->Communicate (
SmmCommunication,
&CommBuffer[0],
@@ -403,7 +402,7 @@ RestoreLockBox (
EFI_SMM_COMMUNICATION_PROTOCOL *SmmCommunication;
EFI_SMM_LOCK_BOX_PARAMETER_RESTORE *LockBoxParameterRestore;
EFI_SMM_COMMUNICATE_HEADER *CommHeader;
- UINT8 TempCommBuffer[sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE)];
+ UINT8 TempCommBuffer[OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE)];
UINT8 *CommBuffer;
UINTN CommSize;

@@ -449,7 +448,7 @@ RestoreLockBox (
//
// Send command
//
- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE);
+ CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE);
Status = SmmCommunication->Communicate (
SmmCommunication,
&CommBuffer[0],
@@ -488,7 +487,7 @@ RestoreAllLockBoxInPlace (
EFI_SMM_COMMUNICATION_PROTOCOL *SmmCommunication;
EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE *LockBoxParameterRestoreAllInPlace;
EFI_SMM_COMMUNICATE_HEADER *CommHeader;
- UINT8 TempCommBuffer[sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)];
+ UINT8 TempCommBuffer[OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)];
UINT8 *CommBuffer;
UINTN CommSize;

@@ -518,7 +517,7 @@ RestoreAllLockBoxInPlace (
//
// Send command
//
- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE);
+ CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE);
Status = SmmCommunication->Communicate (
SmmCommunication,
&CommBuffer[0],
--
2.31.1.windows.1


[PATCH v2 4/6] MdeModulePkg: SmiHandlerProfileInfo: Updated MessageLength calculation

Kun Qin
 

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

This change replaced the calculation of communication buffer size from
explicitly adding the size of each member with the OFFSET macro function.
This will make the structure field defition change transparent to
consumers.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>

Signed-off-by: Kun Qin <kuqin12@gmail.com>
---

Notes:
v2:
- Updated comments by removing "BZ" tags [Hao]

MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
index 4153074b7a80..4bfd5946caba 100644
--- a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
+++ b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
@@ -116,7 +116,10 @@ GetSmiHandlerProfileDatabase(
CommGetInfo->Header.ReturnStatus = (UINT64)-1;
CommGetInfo->DataSize = 0;

- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader->MessageLength;
+ //
+ // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here.
+ //
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
Status = SmmCommunication->Communicate(SmmCommunication, CommBuffer, &CommSize);
if (EFI_ERROR(Status)) {
Print(L"SmiHandlerProfile: SmmCommunication - %r\n", Status);
@@ -149,7 +152,10 @@ GetSmiHandlerProfileDatabase(
CommGetData->Header.DataLength = sizeof(*CommGetData);
CommGetData->Header.ReturnStatus = (UINT64)-1;

- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader->MessageLength;
+ //
+ // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here.
+ //
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
Buffer = (UINT8 *)CommHeader + CommSize;
Size -= CommSize;

--
2.31.1.windows.1


[PATCH v2 3/6] MdeModulePkg: MemoryProfileInfo: Updated MessageLength calculation

Kun Qin
 

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

This change replaced the calculation of communication buffer size from
explicitly adding the size of each member with the OFFSET macro function.
This will make the structure field defition change transparent to
consumers.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>

Signed-off-by: Kun Qin <kuqin12@gmail.com>
---

Notes:
v2:
- Added a missed case this change should cover [Hao]
- Removed "BZ" tags from comments [Hao]

MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c | 28 +++++++++++++++-----
1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c b/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
index 191c31068545..69f78c090e7c 100644
--- a/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
+++ b/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
@@ -1140,8 +1140,7 @@ GetSmramProfileData (
return Status;
}

- MinimalSizeNeeded = sizeof (EFI_GUID) +
- sizeof (UINTN) +
+ MinimalSizeNeeded = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
MAX (sizeof (SMRAM_PROFILE_PARAMETER_GET_PROFILE_INFO),
MAX (sizeof (SMRAM_PROFILE_PARAMETER_GET_PROFILE_DATA_BY_OFFSET),
sizeof (SMRAM_PROFILE_PARAMETER_RECORDING_STATE)));
@@ -1190,7 +1189,10 @@ GetSmramProfileData (
CommRecordingState->Header.ReturnStatus = (UINT64)-1;
CommRecordingState->RecordingState = MEMORY_PROFILE_RECORDING_DISABLE;

- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength;
+ //
+ // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here.
+ //
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
Status = SmmCommunication->Communicate (SmmCommunication, CommBuffer, &CommSize);
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "SmramProfile: SmmCommunication - %r\n", Status));
@@ -1213,7 +1215,10 @@ GetSmramProfileData (
CommRecordingState->Header.ReturnStatus = (UINT64)-1;
CommRecordingState->RecordingState = MEMORY_PROFILE_RECORDING_DISABLE;

- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength;
+ //
+ // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here.
+ //
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
SmmCommunication->Communicate (SmmCommunication, CommBuffer, &CommSize);
}

@@ -1230,7 +1235,10 @@ GetSmramProfileData (
CommGetProfileInfo->Header.ReturnStatus = (UINT64)-1;
CommGetProfileInfo->ProfileSize = 0;

- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength;
+ //
+ // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here.
+ //
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
Status = SmmCommunication->Communicate (SmmCommunication, CommBuffer, &CommSize);
ASSERT_EFI_ERROR (Status);

@@ -1261,7 +1269,10 @@ GetSmramProfileData (
CommGetProfileData->Header.DataLength = sizeof (*CommGetProfileData);
CommGetProfileData->Header.ReturnStatus = (UINT64)-1;

- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength;
+ //
+ // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here.
+ //
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
Buffer = (UINT8 *) CommHeader + CommSize;
Size -= CommSize;

@@ -1320,7 +1331,10 @@ GetSmramProfileData (
CommRecordingState->Header.ReturnStatus = (UINT64)-1;
CommRecordingState->RecordingState = MEMORY_PROFILE_RECORDING_ENABLE;

- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength;
+ //
+ // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here.
+ //
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
SmmCommunication->Communicate (SmmCommunication, CommBuffer, &CommSize);
}

--
2.31.1.windows.1


[PATCH v2 2/6] MdeModulePkg: PiSmmIpl: Update MessageLength calculation for MmCommunicate

Kun Qin
 

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

This change updated calculation routine for MM communication in PiSmmIpl.
It removes ambiguity brought in by UINTN variables from this routine and
paves way for updating definition of field MessageLength in
EFI_MM_COMMUNICATE_HEADER to definitive size.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>

Signed-off-by: Kun Qin <kuqin12@gmail.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
---

Notes:
v2:
- Removed "BZ" tags from comments and variables [Hao]
- Added "Reviewed-by" tag [Hao]

MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 11 ++++++++++-
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf | 1 +
2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
index 599a0cd01d80..01cde6cfc3e4 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
@@ -34,6 +34,7 @@
#include <Library/UefiRuntimeLib.h>
#include <Library/PcdLib.h>
#include <Library/ReportStatusCodeLib.h>
+#include <Library/SafeIntLib.h>

#include "PiSmmCorePrivateData.h"

@@ -515,6 +516,7 @@ SmmCommunicationCommunicate (
EFI_STATUS Status;
EFI_SMM_COMMUNICATE_HEADER *CommunicateHeader;
BOOLEAN OldInSmm;
+ UINT64 LongCommSize;
UINTN TempCommSize;

//
@@ -527,7 +529,14 @@ SmmCommunicationCommunicate (
CommunicateHeader = (EFI_SMM_COMMUNICATE_HEADER *) CommBuffer;

if (CommSize == NULL) {
- TempCommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + CommunicateHeader->MessageLength;
+ Status = SafeUint64Add (OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data), CommunicateHeader->MessageLength, &LongCommSize);
+ if (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
+ }
+ Status = SafeUint64ToUintn (LongCommSize, &TempCommSize);
+ if (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
+ }
} else {
TempCommSize = *CommSize;
//
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
index 6109d6b5449c..ddeb39cee266 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
@@ -46,6 +46,7 @@ [LibraryClasses]
DxeServicesLib
PcdLib
ReportStatusCodeLib
+ SafeIntLib

[Protocols]
gEfiSmmBase2ProtocolGuid ## PRODUCES
--
2.31.1.windows.1


[PATCH v2 1/6] EDK2 Code First: PI Specification: EFI_MM_COMMUNICATE_HEADER Update

Kun Qin
 

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

This change includes specification update markdown file that describes
the proposed PI Specification v1.7 Errata A in detail and potential
impact to the existing codebase.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Hao A Wu <hao.a.wu@intel.com>

Signed-off-by: Kun Qin <kuqin12@gmail.com>
---

Notes:
v2:
- Updated change impact analysis regarding SmmLockBoxDxeLib [Hao]

BZ3430-SpecChange.md | 90 ++++++++++++++++++++
1 file changed, 90 insertions(+)

diff --git a/BZ3430-SpecChange.md b/BZ3430-SpecChange.md
new file mode 100644
index 000000000000..a4f8b397e822
--- /dev/null
+++ b/BZ3430-SpecChange.md
@@ -0,0 +1,90 @@
+# Title: Change MessageLength Field of EFI_MM_COMMUNICATE_HEADER to UINT64
+
+## Status: Draft
+
+## Document: UEFI Platform Initialization Specification Version 1.7 Errata A
+
+## License
+
+SPDX-License-Identifier: CC-BY-4.0
+
+## Submitter: [TianoCore Community](https://www.tianocore.org)
+
+## Summary of the change
+
+Change the `MessageLength` Field of `EFI_MM_COMMUNICATE_HEADER` from UINTN to UINT64 to remove architecture dependency:
+
+```c
+typedef struct {
+ EFI_GUID HeaderGuid;
+ UINT64 MessageLength;
+ UINT8 Data[ANYSIZE_ARRAY];
+} EFI_MM_COMMUNICATE_HEADER;
+```
+
+## Benefits of the change
+
+In PI Spec v1.7 Errata A, Vol.4, Sec 5.7 MM Communication Protocol, the MessageLength field of `EFI_MM_COMMUNICATE_HEADER` (also defined as `EFI_SMM_COMMUNICATE_HEADER`) is defined as type UINTN.
+
+But this structure, as a generic definition, could be used for both PEI and DXE MM communication. Thus for a system that supports PEI MM launch, but operates PEI in 32bit mode and MM foundation in 64bit, the current `EFI_MM_COMMUNICATE_HEADER` definition will cause structure parse error due to UINTN used.
+
+## Impact of the change
+
+This change will impact the known structure consumers including:
+
+```bash
+MdeModulePkg/Core/PiSmmCore/PiSmmIpl
+MdeModulePkg/Application/SmiHandlerProfileInfo
+MdeModulePkg/Application/MemoryProfileInfo
+MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib
+```
+
+For consumers that are not using `OFFSET_OF(EFI_MM_COMMUNICATE_HEADER, Data)`, but performing explicit addition such as the existing MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c, one will need to change code implementation to match new structure definition. Otherwise, the code compiled on IA32 architecture will experience structure field dereference error.
+
+User who currently uses UINTN local variables as place holder of MessageLength will need to use caution to make cast from UINTN to UINT64 and vice versa. It is recommended to use `SafeUint64ToUintn` for such operations when the value is indeterministic.
+
+Note: MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib is also consuming this structure, but it handled this size discrepancy internally. If this potential spec change is not applied, all applicable PEI MM communicate callers will need to use the same routine as that of SmmLockBoxPeiLib to invoke a properly populated EFI_MM_COMMUNICATE_HEADER to be used in X64 MM foundation.
+
+## Detailed description of the change [normative updates]
+
+### Specification Changes
+
+1. In PI Specification v1.7 Errata A: Vol. 4 Page-91, the definition of `EFI_MM_COMMUNICATE_HEADER` should be changed from current:
+
+```c
+typedef struct {
+ EFI_GUID HeaderGuid;
+ UINTN MessageLength;
+ UINT8 Data[ANYSIZE_ARRAY];
+} EFI_MM_COMMUNICATE_HEADER;
+```
+
+to:
+
+```c
+typedef struct {
+ EFI_GUID HeaderGuid;
+ UINT64 MessageLength;
+ UINT8 Data[ANYSIZE_ARRAY];
+} EFI_MM_COMMUNICATE_HEADER;
+```
+
+### Code Changes
+
+1. Remove the explicit calculation of the offset of `Data` in `EFI_MM_COMMUNICATE_HEADER`. Thus applicable calculations of `sizeof(EFI_GUID) + sizeof(UINTN)` should be replaced with `OFFSET_OF(EFI_MM_COMMUNICATE_HEADER, Data)` or similar alternatives. These calculations are identified in:
+
+```bash
+MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
+MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
+MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
+```
+
+1. Resolve potentially mismatched type between `UINTN` and `UINT64`. This would occur when `MessageLength` or its derivitive are used for local calculation with existing `UINTN` typed variables. Code change regarding this perspective is per case evaluation: if the variables involved are all deterministic values, and there is no overflow or underflow risk, a cast operation (from `UINTN` to `UINT64`) can be safely used. Otherwise, the calculation will be performed in `UINT64` bitwidth and then convert to `UINTN` using `SafeUint64*` and `SafeUint64ToUintn`, respectively. These operations are identified in:
+
+```bash
+MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
+MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
+MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
+```
+
+1. After all above changes applied and specification updated, `MdePkg/Include/Protocol/MmCommunication.h` will need to be updated to match new definition that includes the field type update.
--
2.31.1.windows.1


[PATCH v2 0/6] EDK2 Code First: PI Specification: Update EFI_MM_COMMUNICATE_HEADER

Kun Qin
 

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

This patch series is a follow up of previous submission:
https://edk2.groups.io/g/devel/message/76303

v2 patch changes include feedback for v1 series:
a. Added "Reviewed-by" tag for applicable patch;
b. Removed "BZ3398" tags for applicable patches;
c. Added a patch that covers changes needed for SmmLockBoxDxeLib;

There is also concern raised from v1 patch review:
https://edk2.groups.io/g/devel/message/76570

Please advise if any tool/checks could help catching errors as such.

Patch v2 branch: https://github.com/kuqin12/edk2/tree/BZ3398-MmCommunicate-Length-v2

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif@nuviainc.com>

Kun Qin (6):
EDK2 Code First: PI Specification: EFI_MM_COMMUNICATE_HEADER Update
MdeModulePkg: PiSmmIpl: Update MessageLength calculation for
MmCommunicate
MdeModulePkg: MemoryProfileInfo: Updated MessageLength calculation
MdeModulePkg: SmiHandlerProfileInfo: Updated MessageLength calculation
MdeModulePkg: SmmLockBoxDxeLib: Updated MessageLength calculation
MdePkg: MmCommunication: Extend MessageLength field size to UINT64

MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c | 28 ++++--
MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c | 10 ++-
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 11 ++-
MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c | 23 +++--
BZ3430-SpecChange.md | 90 ++++++++++++++++++++
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf | 1 +
MdePkg/Include/Protocol/MmCommunication.h | 2 +-
7 files changed, 142 insertions(+), 23 deletions(-)
create mode 100644 BZ3430-SpecChange.md

--
2.31.1.windows.1


Re: [PATCH v3 8/8] MdeModulePkg: Use SecureBootVariableLib in PlatformVarCleanupLib.

Grzegorz Bernacki
 

Hi,

Thanks for the comment, I will move that function to AuthVariableLib.
greg

czw., 17 cze 2021 o 04:35 gaoliming <gaoliming@byosoft.com.cn> napisał(a):


Grzegorz:
MdeModulePkg is generic base package. It should not depend on SecurityPkg.

I agree CreateTimeBasedPayload() is the generic API. It can be shared in
the different modules.
I propose to add it into MdeModulePkg AuthVariableLib.

Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Grzegorz
Bernacki
发送时间: 2021年6月14日 17:43
收件人: devel@edk2.groups.io
抄送: leif@nuviainc.com; ardb+tianocore@kernel.org;
Samer.El-Haj-Mahmoud@arm.com; sunny.Wang@arm.com;
mw@semihalf.com; upstream@semihalf.com; jiewen.yao@intel.com;
jian.j.wang@intel.com; min.m.xu@intel.com; lersek@redhat.com;
sami.mujawar@arm.com; afish@apple.com; ray.ni@intel.com;
jordan.l.justen@intel.com; rebecca@bsdio.com; grehan@freebsd.org;
thomas.abraham@arm.com; chasel.chiu@intel.com;
nathaniel.l.desimone@intel.com; gaoliming@byosoft.com.cn;
eric.dong@intel.com; michael.d.kinney@intel.com; zailiang.sun@intel.com;
yi.qian@intel.com; graeme@nuviainc.com; rad@semihalf.com; pete@akeo.ie;
Grzegorz Bernacki <gjb@semihalf.com>
主题: [edk2-devel] [PATCH v3 8/8] MdeModulePkg: Use
SecureBootVariableLib in PlatformVarCleanupLib.

This commits removes CreateTimeBasedPayload() function from
PlatformVarCleanupLib and uses exactly the same function from
SecureBootVariableLib.

Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
---
MdeModulePkg/Library/PlatformVarCleanupLib/PlatformVarCleanupLib.inf |
2 +
MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanup.h
| 1 +
MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
| 84 --------------------
3 files changed, 3 insertions(+), 84 deletions(-)

diff --git
a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatformVarCleanupLib.inf
b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatformVarCleanupLib.inf
index 8d5db826a0..493d03e1d8 100644
---
a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatformVarCleanupLib.inf
+++
b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatformVarCleanupLib.inf
@@ -34,6 +34,7 @@
[Packages]
MdePkg/MdePkg.dec
MdeModulePkg/MdeModulePkg.dec
+ SecurityPkg/SecurityPkg.dec

[LibraryClasses]
UefiBootServicesTableLib
@@ -44,6 +45,7 @@
PrintLib
MemoryAllocationLib
HiiLib
+ SecureBootVariableLib

[Guids]
gEfiIfrTianoGuid ## SOMETIMES_PRODUCES ##
GUID
diff --git a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanup.h
b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanup.h
index c809a7086b..94fbc7d2a4 100644
--- a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanup.h
+++ b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanup.h
@@ -18,6 +18,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/MemoryAllocationLib.h>
#include <Library/HiiLib.h>
#include <Library/PlatformVarCleanupLib.h>
+#include <Library/SecureBootVariableLib.h>

#include <Protocol/Variable.h>
#include <Protocol/VarCheck.h>
diff --git
a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
index 3875d614bb..204f1e00ad 100644
--- a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
+++ b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
@@ -319,90 +319,6 @@ DestroyUserVariableNode (
}
}

-/**
- Create a time based data payload by concatenating the
EFI_VARIABLE_AUTHENTICATION_2
- descriptor with the input data. NO authentication is required in this
function.
-
- @param[in, out] DataSize On input, the size of Data buffer in
bytes.
- On output, the size of data
returned in Data
- buffer in bytes.
- @param[in, out] Data On input, Pointer to data buffer to
be wrapped or
- pointer to NULL to wrap an
empty payload.
- On output, Pointer to the new
payload date buffer allocated from pool,
- it's caller's responsibility to free
the memory after using it.
-
- @retval EFI_SUCCESS Create time based payload
successfully.
- @retval EFI_OUT_OF_RESOURCES There are not enough memory
resourses to create time based payload.
- @retval EFI_INVALID_PARAMETER The parameter is invalid.
- @retval Others Unexpected error happens.
-
-**/
-EFI_STATUS
-CreateTimeBasedPayload (
- IN OUT UINTN *DataSize,
- IN OUT UINT8 **Data
- )
-{
- EFI_STATUS Status;
- UINT8 *NewData;
- UINT8 *Payload;
- UINTN PayloadSize;
- EFI_VARIABLE_AUTHENTICATION_2 *DescriptorData;
- UINTN DescriptorSize;
- EFI_TIME Time;
-
- if (Data == NULL || DataSize == NULL) {
- return EFI_INVALID_PARAMETER;
- }
-
- //
- // At user physical presence, the variable does not need to be signed
but
the
- // parameters to the SetVariable() call still need to be prepared as
authenticated
- // variable. So we create EFI_VARIABLE_AUTHENTICATED_2 descriptor
without certificate
- // data in it.
- //
- Payload = *Data;
- PayloadSize = *DataSize;
-
- DescriptorSize = OFFSET_OF (EFI_VARIABLE_AUTHENTICATION_2,
AuthInfo) + OFFSET_OF (WIN_CERTIFICATE_UEFI_GUID, CertData);
- NewData = (UINT8 *) AllocateZeroPool (DescriptorSize + PayloadSize);
- if (NewData == NULL) {
- return EFI_OUT_OF_RESOURCES;
- }
-
- if ((Payload != NULL) && (PayloadSize != 0)) {
- CopyMem (NewData + DescriptorSize, Payload, PayloadSize);
- }
-
- DescriptorData = (EFI_VARIABLE_AUTHENTICATION_2 *) (NewData);
-
- ZeroMem (&Time, sizeof (EFI_TIME));
- Status = gRT->GetTime (&Time, NULL);
- if (EFI_ERROR (Status)) {
- FreePool (NewData);
- return Status;
- }
- Time.Pad1 = 0;
- Time.Nanosecond = 0;
- Time.TimeZone = 0;
- Time.Daylight = 0;
- Time.Pad2 = 0;
- CopyMem (&DescriptorData->TimeStamp, &Time, sizeof (EFI_TIME));
-
- DescriptorData->AuthInfo.Hdr.dwLength = OFFSET_OF
(WIN_CERTIFICATE_UEFI_GUID, CertData);
- DescriptorData->AuthInfo.Hdr.wRevision = 0x0200;
- DescriptorData->AuthInfo.Hdr.wCertificateType =
WIN_CERT_TYPE_EFI_GUID;
- CopyGuid (&DescriptorData->AuthInfo.CertType, &gEfiCertPkcs7Guid);
-
- if (Payload != NULL) {
- FreePool (Payload);
- }
-
- *DataSize = DescriptorSize + PayloadSize;
- *Data = NewData;
- return EFI_SUCCESS;
-}
-
/**
Create a counter based data payload by concatenating the
EFI_VARIABLE_AUTHENTICATION
descriptor with the input data. NO authentication is required in this
function.
--
2.25.1











Re: [EXTERNAL] 回复: [edk2-devel] [edk2-platforms] [PATCH V1 0/2] Support for TiogaPass Platform and Override generic PciBus Driver with

Ni, Ray
 

To skip loading an option rom for certain devices, can you use IncompatiblePciDevice->CheckDevice()?

Table

Description automatically generated

 

For skipping enumerating a certain device, we could change PciBus to skip enumerating if gPciPlatformProtocol->PlatformPrepController() returns error status for that device. Do you think this solution is feasible to you?

 

Thanks,

Ray

 

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of manickavasakam karpagavinayagam
Sent: Friday, June 18, 2021 12:41 AM
To: Kinney, Michael D <michael.d.kinney@...>; devel@edk2.groups.io; gaoliming <gaoliming@...>; Ni, Ray <ray.ni@...>
Cc: DOPPALAPUDI, HARIKRISHNA <harikrishnad@...>
Subject: Re: [EXTERNAL]
回复: [edk2-devel] [edk2-platforms] [PATCH V1 0/2] Support for TiogaPass Platform and Override generic PciBus Driver with

 

Mike :

 

During PCI Bus enumeration, we need to skip SPI Controller (because of a silicon sighting) or else any SET variable asserts.

Also, need to skip a specific MLX card UEFI OPROM or else will see CPU exception.

 

We checked the PCIBUS driver code flow and there is no generic hooks to skip enumerating a device and to override the OPROM contents.

 

To avoid overriding the PCIBUS driver with platform instance, we can have PciBus Hooks at various places in PCIBUS driver to skip the device from enumeration, overriding the OPROM contents etc..

 

Ex : MdeModulePkg\Bus\Pci\PciBusDxe\PciLib.c PciScanBus()

 

        for (Device = 0; Device <= PCI_MAX_DEVICE; Device++) {

    TempReservedBusNum = 0;

    for (Func = 0; Func <= PCI_MAX_FUNC; Func++) {

 

      //

      // Check to see whether a pci device is present

      //

      Status = PciDevicePresent (

                PciRootBridgeIo,

                &Pci,

                StartBusNumber,

                Device,

                Func

                );

 

      if (EFI_ERROR (Status) && Func == 0) {

        //

        // go to next device if there is no Function 0

        //

        break;

      }

 

      if (EFI_ERROR (Status)) {

        continue;

      }

 

     

      Status = PciOemPlatformHooks(&Pci, isPciSkipDevice, &Pci, &StartBusNumber, &Device, &Func);

      if(EFI_ERROR(Status))

      {

          if(Status==EFI_UNSUPPORTED){

              Status=EFI_SUCCESS;

          } else ASSERT_EFI_ERROR(Status);

      }

      else

      {

          DEBUG((DEBUG_INFO,"Device @ [B%X|D%X|F%X], VID=%X, DID=%X SKIPPED from enumeration.\n\n",

                  StartBusNumber, Device, Func,

                  Pci.Hdr.VendorId,Pci.Hdr.DeviceId));

          continue;

      }

     

      //

      // Get the PCI device information

      //

      Status = PciSearchDevice (

                Bridge,

                &Pci,

                StartBusNumber,

                Device,

                Func,

                &PciDevice

                );

 

      if (EFI_ERROR (Status)) {

        continue;

      }

 

Thank you

 

-Manic

 

From: Kinney, Michael D <michael.d.kinney@...>
Sent: Thursday, June 17, 2021 11:19 AM
To: devel@edk2.groups.io; Manickavasakam Karpagavinayagam <manickavasakamk@...>; gaoliming <gaoliming@...>; Ni, Ray <ray.ni@...>; Kinney, Michael D <michael.d.kinney@...>
Cc: Harikrishna Doppalapudi <Harikrishnad@...>
Subject: RE: [EXTERNAL]
回复: [edk2-devel] [edk2-platforms] [PATCH V1 0/2] Support for TiogaPass Platform and Override generic PciBus Driver with

 

Has the reason for the PciBusDxe override been discussed with the PciBusDxe maintainer?

 

What feature would need to be added to PciBusDxe to accommodate the use case?

 

Mike

 

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of manickavasakam karpagavinayagam
Sent: Thursday, June 17, 2021 7:56 AM
To: gaoliming <gaoliming@...>; devel@edk2.groups.io
Cc: DOPPALAPUDI, HARIKRISHNA <harikrishnad@...>
Subject: Re: [EXTERNAL]
回复: [edk2-devel] [edk2-platforms] [PATCH V1 0/2] Support for TiogaPass Platform and Override generic PciBus Driver with

 

Liming :

 

Below email is the cover letter and this patch series has two changes. Sure next time, will add more comments in the cover letter also.

Please refer the attached email and it has information about the PCIBUS override changes. PCIBUS override is done based on the platform sighting and it can’t be generic.

 

Thank you

 

-Manic

 

From: gaoliming <gaoliming@...>
Sent: Wednesday, June 16, 2021 10:56 PM
To: devel@edk2.groups.io; Manickavasakam Karpagavinayagam <manickavasakamk@...>
Subject: [EXTERNAL]
回复: [edk2-devel] [edk2-platforms] [PATCH V1 0/2] Support for TiogaPass Platform and Override generic PciBus Driver with

 

 

**CAUTION: The e-mail below is from an external source. Please exercise caution before opening attachments, clicking links, or following guidance.**

 

Please follow https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format to update the commit message format.

 

And, for the override PciBus module, can you give more detail why need to override PciBus? Is it possible to update Edk2 MdeModulePkg PciBus to meet the platform requirement?

 

Thanks

Liming

发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 manickavasakam karpagavinayagam
发送时间: 2021617 7:05
收件人: devel@edk2.groups.io
主题: [edk2-devel] [edk2-platforms] [PATCH V1 0/2] Support for TiogaPass Platform and Override generic PciBus Driver with

 

Add BoardTiogaPass packages to support TiogaPass Platform Enabled Network, ISCSI,IPMI, SMBIOS, Performance Measurement
Remove AST2500 UEFI option ROM driver from PurleyOpenBoardPkg

AST2500 UEFI option ROM move to edk2-non-osi ASpeedGopBinPkg Update copyright headers

 

manickavasakam karpagavinayagam (2):

  PurleyOpenBoardPkg : Support for TiogaPass Platform

  PurleyOpenBoardPkg : Override generic PciBus Driver with Platform

    specific instance of PciBus driver.

 

.../IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c |    8 +-

.../Acpi/BoardAcpiDxe/AmlOffsetTable.c        |  453 +-

.../Acpi/BoardAcpiDxe/BoardAcpiDxeDsdt.c      |    3 +

.../BoardTiogaPass/CoreDxeInclude.dsc         |  168 +

.../BoardTiogaPass/CoreUefiBootInclude.fdf    |   82 +

.../BoardTiogaPass/GitEdk2MinTiogaPass.bat    |   93 +

.../BasePlatformHookLib/BasePlatformHookLib.c |  389 +

.../BasePlatformHookLib.inf                   |   36 +

.../BoardAcpiLib/DxeBoardAcpiTableLib.c       |   36 +

.../BoardAcpiLib/DxeBoardAcpiTableLib.inf     |   40 +

.../BoardAcpiLib/DxeTiogaPassAcpiTableLib.c   |   53 +

.../BoardAcpiLib/SmmBoardAcpiEnableLib.c      |   62 +

.../BoardAcpiLib/SmmBoardAcpiEnableLib.inf    |   41 +

.../BoardAcpiLib/SmmSiliconAcpiEnableLib.c    |  120 +

.../BoardAcpiLib/SmmTiogaPassAcpiEnableLib.c  |   37 +

.../Library/BoardInitLib/AllLanesEparam.c     |   44 +

.../Library/BoardInitLib/GpioTable.c          |  296 +

.../Library/BoardInitLib/IioBifur.c           |   70 +

.../BoardInitLib/PeiBoardInitPostMemLib.c     |   46 +

.../BoardInitLib/PeiBoardInitPostMemLib.inf   |   37 +

.../BoardInitLib/PeiBoardInitPreMemLib.c      |  112 +

.../BoardInitLib/PeiBoardInitPreMemLib.inf    |   69 +

.../Library/BoardInitLib/PeiTiogaPassDetect.c |   28 +

.../BoardInitLib/PeiTiogaPassInitLib.h        |   18 +

.../BoardInitLib/PeiTiogaPassInitPostMemLib.c |   86 +

.../BoardInitLib/PeiTiogaPassInitPreMemLib.c  |  638 ++

.../Library/BoardInitLib/UsbOC.c              |   46 +

.../Library/PeiReportFvLib/PeiReportFvLib.c   |  138 +

.../Library/PeiReportFvLib/PeiReportFvLib.inf |   51 +

.../BoardTiogaPass/OpenBoardPkg.dsc           |  245 +

.../BoardTiogaPass/OpenBoardPkg.fdf           |  600 ++

.../BoardTiogaPass/PlatformPkgBuildOption.dsc |   84 +

.../BoardTiogaPass/PlatformPkgConfig.dsc      |   58 +

.../BoardTiogaPass/PlatformPkgPcd.dsc         |  392 ++

.../BoardTiogaPass/StructureConfig.dsc        | 6236 +++++++++++++++++

.../BoardTiogaPass/__init__.py                |    0

.../PurleyOpenBoardPkg/BoardTiogaPass/bld.bat |  139 +

.../BoardTiogaPass/build_board.py             |  195 +

.../BoardTiogaPass/build_config.cfg           |   34 +

.../BoardTiogaPass/logo.txt                   |   10 +

.../BoardTiogaPass/postbuild.bat              |   96 +

.../BoardTiogaPass/prebuild.bat               |  213 +

.../Ipmi/Library/IpmiLibKcs/IpmiLibKcs.inf    |   10 +-

.../IpmiPlatformHookLib.inf                   |    6 +-

.../Include/Guid/PchRcVariable.h              |    6 +

.../Include/Guid/SetupVariable.h              |   15 +-

.../Intel/PurleyOpenBoardPkg/OpenBoardPkg.dec |    1 +

.../Bus/Pci/PciBusDxe/ComponentName.c         |  170 +

.../Bus/Pci/PciBusDxe/ComponentName.h         |  146 +

.../MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c   |  460 ++

.../MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h   |  396 ++

.../Bus/Pci/PciBusDxe/PciBusDxe.inf           |  112 +

.../Bus/Pci/PciBusDxe/PciBusDxe.uni           |   16 +

.../Bus/Pci/PciBusDxe/PciBusDxeExtra.uni      |   14 +

.../Bus/Pci/PciBusDxe/PciCommand.c            |  267 +

.../Bus/Pci/PciBusDxe/PciCommand.h            |  232 +

.../Bus/Pci/PciBusDxe/PciDeviceSupport.c      | 1056 +++

.../Bus/Pci/PciBusDxe/PciDeviceSupport.h      |  266 +

.../Bus/Pci/PciBusDxe/PciDriverOverride.c     |  188 +

.../Bus/Pci/PciBusDxe/PciDriverOverride.h     |   83 +

.../Bus/Pci/PciBusDxe/PciEnumerator.c         | 2210 ++++++

.../Bus/Pci/PciBusDxe/PciEnumerator.h         |  515 ++

.../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c  | 2885 ++++++++  .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.h  |  480 ++

.../Bus/Pci/PciBusDxe/PciHotPlugSupport.c     |  484 ++

.../Bus/Pci/PciBusDxe/PciHotPlugSupport.h     |  205 +

.../MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c    | 2087 ++++++

.../MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.h    |  660 ++

.../MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c   | 1809 +++++

.../MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.h   |  179 +

.../Bus/Pci/PciBusDxe/PciOptionRomSupport.c   |  776 ++

.../Bus/Pci/PciBusDxe/PciOptionRomSupport.h   |  136 +

.../Bus/Pci/PciBusDxe/PciPowerManagement.c    |   82 +

.../Bus/Pci/PciBusDxe/PciPowerManagement.h    |   28 +

.../Bus/Pci/PciBusDxe/PciResourceSupport.c    | 2292 ++++++

.../Bus/Pci/PciBusDxe/PciResourceSupport.h    |  456 ++

.../Bus/Pci/PciBusDxe/PciRomTable.c           |  135 +

.../Bus/Pci/PciBusDxe/PciRomTable.h           |   48 +

Platform/Intel/build.cfg                      |    2 +

Platform/Intel/build_bios.py                  |    3 +-

80 files changed, 30278 insertions(+), 240 deletions(-)  create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/CoreDxeInclude.dsc

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/CoreUefiBootInclude.fdf

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/GitEdk2MinTiogaPass.bat

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BasePlatformHookLib/BasePlatformHookLib.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BasePlatformHookLib/BasePlatformHookLib.inf

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardAcpiLib/DxeBoardAcpiTableLib.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardAcpiLib/DxeTiogaPassAcpiTableLib.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardAcpiLib/SmmSiliconAcpiEnableLib.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardAcpiLib/SmmTiogaPassAcpiEnableLib.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardInitLib/AllLanesEparam.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardInitLib/GpioTable.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardInitLib/IioBifur.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardInitLib/PeiBoardInitPostMemLib.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardInitLib/PeiBoardInitPostMemLib.inf

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardInitLib/PeiBoardInitPreMemLib.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardInitLib/PeiBoardInitPreMemLib.inf

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardInitLib/PeiTiogaPassDetect.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardInitLib/PeiTiogaPassInitLib.h

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardInitLib/PeiTiogaPassInitPostMemLib.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardInitLib/PeiTiogaPassInitPreMemLib.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/BoardInitLib/UsbOC.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/PeiReportFvLib/PeiReportFvLib.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/Library/PeiReportFvLib/PeiReportFvLib.inf

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/OpenBoardPkg.dsc

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/OpenBoardPkg.fdf

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/PlatformPkgBuildOption.dsc

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/PlatformPkgConfig.dsc

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/PlatformPkgPcd.dsc

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/StructureConfig.dsc

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/__init__.py

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/bld.bat

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/build_board.py

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/build_config.cfg

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/logo.txt

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/postbuild.bat

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/prebuild.bat

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/ComponentName.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/ComponentName.h

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.uni

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxeExtra.uni

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciCommand.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciCommand.h

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.h

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciDriverOverride.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciDriverOverride.h

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.h

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.h

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.h

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.h

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.h

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciPowerManagement.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciPowerManagement.h

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.h

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.c

create mode 100644 Platform/Intel/PurleyOpenBoardPkg/Override/MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.h

-The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.

-The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.


Re: [PATCH] OvmfPkg/Bhyve: clean up TPM_ENABLE remnants

Michael D Kinney
 

Hi Sean,

Here is the proposed fix to PatchCheck.py to not flag a Mergify merge commit as an error:

https://github.com/tianocore/edk2-codereview/commit/1b264739387ae940b42d70b5cd25b7c68ed21318

I have re-enabled PatchCheck in GitHub branch protection rules, and I did 2 PRs at same time and
they were auto rebased by Mergify and passed PatchCheck test.

Best regards,

Mike

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Thursday, June 17, 2021 2:55 PM
To: devel@edk2.groups.io; spbrogan@outlook.com; ardb@kernel.org; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Peter Grehan <grehan@freebsd.org>; Laszlo Ersek <lersek@redhat.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
Justen, Jordan L <jordan.l.justen@intel.com>; Sean Brogan <sean.brogan@microsoft.com>; Rebecca Cran <rebecca@bsdio.com>
Subject: RE: [edk2-devel] [PATCH] OvmfPkg/Bhyve: clean up TPM_ENABLE remnants

Link to Mergify queue feature documentation

https://docs.mergify.io/actions/queue/#merge-queue-problem

Mike

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Thursday, June 17, 2021 2:53 PM
To: devel@edk2.groups.io; spbrogan@outlook.com; ardb@kernel.org; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Peter Grehan <grehan@freebsd.org>; Laszlo Ersek <lersek@redhat.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
Justen, Jordan L <jordan.l.justen@intel.com>; Sean Brogan <sean.brogan@microsoft.com>; Rebecca Cran <rebecca@bsdio.com>
Subject: RE: [edk2-devel] [PATCH] OvmfPkg/Bhyve: clean up TPM_ENABLE remnants

Hi Sean,

Mergify had added a queue feature to handle the rebases automatically and make sure
CI passes in the order that the PRs are being applied to the base branch.

I have setup the edk2-codereview repo with a smaller set of CI tests to skip the
compiles to provide a way to experiment with these features quickly. I have
also simplified the Azure Pipelines templates to remove the FINISHED and FAILED
jobs and the 10 second delay. I have also removed all references to status
check names from the Mergify config file, so the status checks are only
configured using the GitHub protected branches feature.

Here is the Mergify config that is working:

https://github.com/tianocore/edk2-codereview/blob/master/.mergify/config.yml

Here is the Azure Pipelines file that has been simplified removing FINISHED and FAILED jobs
(also removes DEBUG, RELEASE, NOOPT builds for fast testing):

https://github.com/tianocore/edk2-codereview/blob/master/.azurepipelines/templates/pr-gate-build-job.yml

I then did an experiment sending a PR that is out of date with the base branch.
Mergify auto rebased and ran CI tests and added commits from the PR to the base
branch.

I did a 2nd experiment sending 2 PRs at the same time. The first PR finishes its
CI tests and was merged. The 2nd PR was auto rebased and CI tests run and was merged.
No developer interaction required on either one after submitting the PR. This
resolves a common issue seen by Maintainers that process a number of unrelated
patch sets at the same time. They can all be submitted on top of each other without
having to wait for each one to complete and rebase the next one before submitting.

The only open issue remaining is that Mergify auto rebase adds a merge commit
to the set of commits in the PR. The merge commit is not added to the base branch
when it is done. Since the merge commit is present in the PR, the patch check fails.
We need to update patch check to ignore merge commits by the Mergify agent. I have
temporarily disabled the patch check CI status check in edk2-codereview to
work around this issue. Once I have a fix for patch check, I will re-enable the
patch check status check.

I also think this approach will fix the issue that has been seen where Mergify merged
before all the platform tests were completed. We just need to make sure the
platform checks are in the list of enabled status checks in the GitHub protected
branch configuration. I will verify this issue is resolved using the edk2-codereview
repo.

Best regards,

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean
Sent: Wednesday, June 16, 2021 12:00 PM
To: ardb@kernel.org; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Peter Grehan <grehan@freebsd.org>; Laszlo Ersek <lersek@redhat.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
Justen, Jordan L <jordan.l.justen@intel.com>; Sean Brogan <sean.brogan@microsoft.com>; Rebecca Cran
<rebecca@bsdio.com>;
devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH] OvmfPkg/Bhyve: clean up TPM_ENABLE remnants

Ard,

The PR you are trying to "push" has a mergify merge in it which is
causing patchcheck to fail.

https://github.com/tianocore/edk2/pull/1727/commits



Mike,

I think github has better features now around things like auto complete
PR when "checks pass" and allow rebase merging and finally protections
to only allow linear history. Might be time to talk about changes to
mergify/github. I know you are busy so maybe opening up to more of the
edk2 community or actively looking for someone willing to provide best
practices for github usage (rust-lang and nodejs both do a lot with
github).


Thanks
Sean





On 6/16/2021 8:58 AM, Ard Biesheuvel wrote:
(+ Sean, Mike)

On Sat, 12 Jun 2021 at 22:43, Rebecca Cran <rebecca@bsdio.com> wrote:

TPM support hasn't been tested and any lines in the .dsc and .fdf files
that appear to show support are bogus. Remove them.

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

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
Strangely enough, this patch gets rejected by PatchCheck for lack of a
Signed-off-by line

https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=24198&view=results

The patch itself looks good to me

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

so if anyone else manages to fix the CI issue, feel free to push the
patch with my R-b (and Peter's, given in reply to this message)

(I will go offline for 3 weeks after Friday)

---
OvmfPkg/Bhyve/BhyveX64.dsc | 64 --------------------
OvmfPkg/Bhyve/BhyveX64.fdf | 15 -----
2 files changed, 79 deletions(-)

diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
index d8792812ab..cbf896e89b 100644
--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -31,8 +31,6 @@
DEFINE SECURE_BOOT_ENABLE = FALSE
DEFINE SMM_REQUIRE = FALSE
DEFINE SOURCE_DEBUG_ENABLE = FALSE
- DEFINE TPM_ENABLE = FALSE
- DEFINE TPM_CONFIG_ENABLE = FALSE

#
# Network definition
@@ -221,16 +219,8 @@
OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
XenPlatformLib|OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf

-
-!if $(TPM_ENABLE) == TRUE
- Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
- Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
- Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
- TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
-!else
Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
-!endif

[LibraryClasses.common]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -292,11 +282,6 @@
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf

-!if $(TPM_ENABLE) == TRUE
- BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
- Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
-!endif
-
MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf

[LibraryClasses.common.DXE_CORE]
@@ -366,9 +351,6 @@
!endif
PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
MpInitLib|UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.inf
-!if $(TPM_ENABLE) == TRUE
- Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf
-!endif

[LibraryClasses.common.UEFI_APPLICATION]
PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
@@ -563,22 +545,12 @@

gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00

-!if $(TPM_ENABLE) == TRUE
- gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00,
0x00, 0x00, 0x00, 0x00, 0x00}
-!endif
-
# MdeModulePkg resolution sets up the system display resolution
gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|0
gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|0
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutRow|0
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutColumn|0

-[PcdsDynamicHii]
-!if $(TPM_ENABLE) == TRUE && $(TPM_CONFIG_ENABLE) == TRUE
-
gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
- gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS
-!endif
-
################################################################################
#
# Components Section - list of all EDK II Modules needed by this Platform.
@@ -618,19 +590,6 @@
<LibraryClasses>
}

-!if $(TPM_ENABLE) == TRUE
- OvmfPkg/Bhyve/Tcg/Tcg2Config/Tcg2ConfigPei.inf
- SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {
- <LibraryClasses>
- HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf
- NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
- NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
- NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
- NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
- NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
- }
-!endif
-
#
# DXE Phase modules
#
@@ -653,9 +612,6 @@
<LibraryClasses>
!if $(SECURE_BOOT_ENABLE) == TRUE
NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
-!endif
-!if $(TPM_ENABLE) == TRUE
- NULL|SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
!endif
}

@@ -841,23 +797,3 @@
NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
}

-
- #
- # TPM support
- #
-!if $(TPM_ENABLE) == TRUE
- SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
- <LibraryClasses>
- Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRouterDxe.inf
- NULL|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
- HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.inf
- NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
- NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
- NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
- NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
- NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
- }
-!if $(TPM_CONFIG_ENABLE) == TRUE
- SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
-!endif
-!endif
diff --git a/OvmfPkg/Bhyve/BhyveX64.fdf b/OvmfPkg/Bhyve/BhyveX64.fdf
index 3eff36dac1..fbd63a395a 100644
--- a/OvmfPkg/Bhyve/BhyveX64.fdf
+++ b/OvmfPkg/Bhyve/BhyveX64.fdf
@@ -158,11 +158,6 @@ INF UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
INF OvmfPkg/Bhyve/SmmAccess/SmmAccessPei.inf
!endif

-!if $(TPM_ENABLE) == TRUE
-INF OvmfPkg/Bhyve/Tcg/Tcg2Config/Tcg2ConfigPei.inf
-INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
-!endif
-
################################################################################

[FV.DXEFV]
@@ -333,16 +328,6 @@ INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
!endif

-#
-# TPM support
-#
-!if $(TPM_ENABLE) == TRUE
-INF SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
-!if $(TPM_CONFIG_ENABLE) == TRUE
-INF SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
-!endif
-!endif
-
################################################################################

[FV.FVMAIN_COMPACT]
--
2.32.0







Re: [Patch] StandaloneMmPkg: Fixed communicating from TF-A failed issue

Ming Huang
 

On 6/16/21 10:10 PM, Ard Biesheuvel wrote:
On Wed, 16 Jun 2021 at 07:30, Omkar Kulkarni <Omkar.Kulkarni@arm.com> wrote:


On 6/10/21 6:44 AM, Ming Huang via groups.io wrote:
On 6/9/21 3:10 PM, Ard Biesheuvel wrote:
On Tue, 8 Jun 2021 at 16:21, Ming Huang <huangming@linux.alibaba.com>
wrote:

TF-A: TrustedFirmware-a
SPM: Secure Partition Manager(MM)

For AArch64, when SPM enable in TF-A, TF-A may communicate to MM
with
buffer address (PLAT_SPM_BUF_BASE). The address is different from
PcdMmBufferBase which use in edk2.
Then why do we have PcdMmBufferBase?
ArmPkg use this Pcd for the base address of non-secure communication
buffer.


Is it possible to set PcdMmBufferBase to the correct value?
The secure communication may interrupt the non-secure communication. if
we use the same address (PcdMmBufferBase and PLAT_SPM_BUF_BASE), the
date in communication buffer may be corrupted.

Best Regards,
Ming
In case where an interrupt handler executing from EL3 makes a call into StandaloneMM, the handler in EL3 makes an spm call into StandaloneMM using PLAT_SPM_BUF_BASE buffer base address. This PLAT_SPM_BUF_BASE is a shared buffer between EL3 and S-EL0. This is where the following check fails and leads to spm call failure. So this change would help resolve this issue.
But is it the right fix? Why would EDK2 even be aware of how EL3 and
S-EL0 communicate with each other, and where the buffer is located?
The root cause for this problem is that the StandaloneMmPkg and TF-A
are not full cooperative.
PLAT_SPM_BUF_BASE is not dynamic buffer just like PcdMmBufferBase.

Please refer PcdMmBufferBase comments in edk2-platforms/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc:
#
# Set the base address and size of the buffer used
# for communication between the Normal world edk2
# with StandaloneMm image at S-EL0 through MM_COMMUNICATE.
# This buffer gets allocated in ATF and since we do not have
# a mechanism currently to communicate the base address and
# size of this buffer from ATF, hard-code it here
#
## MM Communicate
gArmTokenSpaceGuid.PcdMmBufferBase|0xFF600000
gArmTokenSpaceGuid.PcdMmBufferSize|0x10000


Best Regards,
Ming





Checking address will let TF-A communicate failed to MM. So remove
below checking code:
if (NsCommBufferAddr < mNsCommBuffer.PhysicalStart) {
return EFI_ACCESS_DENIED;
}

Signed-off-by: Ming Huang <huangming@linux.alibaba.com>
---
StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c |
4
----
1 file changed, 4 deletions(-)

diff --git
a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
index 63fbe26642..fe98d3181d 100644
---
a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
+++
b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
@@ -103,10 +103,6 @@ PiMmStandaloneArmTfCpuDriverEntry (
return EFI_INVALID_PARAMETER;
}

- if (NsCommBufferAddr < mNsCommBuffer.PhysicalStart) {
- return EFI_ACCESS_DENIED;
- }
-
if ((NsCommBufferAddr + sizeof (EFI_MM_COMMUNICATE_HEADER)) >=
(mNsCommBuffer.PhysicalStart + mNsCommBuffer.PhysicalSize)) {
return EFI_INVALID_PARAMETER;
--
2.17.1



[edk2-platforms][PATCH v3 39/41] KabylakeSiliconPkg: Identify flash regions by GUID

Michael Kubacki
 

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

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

Updates the code to identify flash regions by GUID and internally
map the GUID entries to values specific to KabylakeSiliconPkg.

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/SecureMemoryMapConfiguration.c=
| 106 ++++++++++++++-
Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommonLib/=
SpiCommon.c | 140 ++++++++++++++++----
Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/HstiSiliconDxe.inf =
| 9 ++
Silicon/Intel/KabylakeSiliconPkg/Pch/IncludePrivate/Library/PchSpiCommon=
Lib.h | 20 +--
Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommonLib/=
BasePchSpiCommonLib.inf | 11 ++
5 files changed, 247 insertions(+), 39 deletions(-)

diff --git a/Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/SecureMemoryMapCon=
figuration.c b/Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/SecureMemoryMapC=
onfiguration.c
index a3c9bbebeaa9..ccf63b216f70 100644
--- a/Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/SecureMemoryMapConfigurat=
ion.c
+++ b/Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/SecureMemoryMapConfigurat=
ion.c
@@ -2,11 +2,14 @@
This file contains the tests for the SecureMemoryMapConfiguration bit
=20
Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) Microsoft Corporation.<BR>
+
SPDX-License-Identifier: BSD-2-Clause-Patent
=20
**/
=20
#include "HstiSiliconDxe.h"
+#include <Guid/FlashRegion.h>
=20
typedef struct {
UINT64 Base;
@@ -100,6 +103,90 @@ MEMORY_RANGE mNonLockableMemoryRange[NonLockableMem=
oryRangeMax] =3D {
// 14. SPI_BAR0 (BDF 0:31:5 + 0x10)
};
=20
+typedef enum {
+ FlashRegionDescriptor,
+ FlashRegionBios,
+ FlashRegionMe,
+ FlashRegionGbe,
+ FlashRegionPlatformData,
+ FlashRegionDer,
+ FlashRegionAll,
+ FlashRegionMax
+} FLASH_REGION_TYPE;
+
+typedef struct {
+ EFI_GUID *Guid;
+ FLASH_REGION_TYPE Type;
+} FLASH_REGION_MAPPING;
+
+FLASH_REGION_MAPPING mFlashRegionTypes[] =3D {
+ {
+ &gFlashRegionDescriptorGuid,
+ FlashRegionDescriptor
+ },
+ {
+ &gFlashRegionBiosGuid,
+ FlashRegionBios
+ },
+ {
+ &gFlashRegionMeGuid,
+ FlashRegionMe
+ },
+ {
+ &gFlashRegionGbeGuid,
+ FlashRegionGbe
+ },
+ {
+ &gFlashRegionPlatformDataGuid,
+ FlashRegionPlatformData
+ },
+ {
+ &gFlashRegionDerGuid,
+ FlashRegionDer
+ },
+ {
+ &gFlashRegionAllGuid,
+ FlashRegionAll
+ },
+ {
+ &gFlashRegionMaxGuid,
+ FlashRegionMax
+ }
+};
+
+/**
+ Returns the type of a flash region given its GUID.
+
+ @param[in] FlashRegionGuid Pointer to the flash region GUID.
+ @param[out] FlashRegionType Pointer to a buffer that will be set t=
o the flash region type value.
+
+ @retval EFI_SUCCESS The flash region type was found =
for the given flash region GUID.
+ @retval EFI_INVALID_PARAMETER A pointer argument passed to the=
function is NULL.
+ @retval EFI_NOT_FOUND The flash region type was not fo=
und for the given flash region GUID.
+
+**/
+EFI_STATUS
+GetFlashRegionType (
+ IN EFI_GUID *FlashRegionGuid,
+ OUT FLASH_REGION_TYPE *FlashRegionType
+ )
+{
+ UINTN Index;
+
+ if (FlashRegionGuid =3D=3D NULL || FlashRegionType =3D=3D NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ for (Index =3D 0; Index < ARRAY_SIZE (mFlashRegionTypes); Index++) {
+ if (CompareGuid (mFlashRegionTypes[Index].Guid, FlashRegionGuid)) {
+ *FlashRegionType =3D mFlashRegionTypes[Index].Type;
+ return EFI_SUCCESS;
+ }
+ }
+
+ return EFI_NOT_FOUND;
+}
+
/**
Check for overlaps in single range array
=20
@@ -224,7 +311,7 @@ AcquireSpiBar0 (
{
UINT32 SpiBar0;
UINTN PchSpiBase;
- =20
+
//
// Init PCH spi reserved MMIO address.
//
@@ -270,7 +357,7 @@ ReleaseSpiBar0 (
Get the SPI region base and size, based on the enum type
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for for the base=
address which is listed in the Descriptor.
+ @param[in] FlashRegionGuid The Flash Region GUID for the base add=
ress which corresponds to the type in the descriptor.
@param[out] BaseAddress The Flash Linear Address for the Regio=
n 'n' Base
@param[out] RegionSize The size for the Region 'n'
=20
@@ -281,13 +368,20 @@ ReleaseSpiBar0 (
EFI_STATUS
EFIAPI
GetRegionAddress (
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
OUT UINT32 *BaseAddress,
OUT UINT32 *RegionSize
)
{
- UINTN PchSpiBar0;
- UINT32 ReadValue;
+ EFI_STATUS Status;
+ FLASH_REGION_TYPE FlashRegionType;
+ UINTN PchSpiBar0;
+ UINT32 ReadValue;
+
+ Status =3D GetFlashRegionType (FlashRegionGuid, &FlashRegionType);
+ if (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
+ }
=20
if (FlashRegionType >=3D FlashRegionMax) {
return EFI_INVALID_PARAMETER;
@@ -484,7 +578,7 @@ CheckSecureMemoryMapConfiguration (
//
// Locate BIOS region size to update High bios base address
//
- GetRegionAddress (FlashRegionBios, &BaseAddress, &RegionSize);
+ GetRegionAddress (&gFlashRegionBiosGuid, &BaseAddress, &RegionSize);
DEBUG ((DEBUG_INFO, "Bios Region Size %x:\n", RegionSize));
mLockableMemoryRange[LockableMemoryRangeHighBios].Base =3D SIZE_4GB=
- RegionSize;
mLockableMemoryRange[LockableMemoryRangeLowDram].End =3D (MmioRead3=
2 (MmPciBase (0,SA_MC_DEV,SA_MC_FUN) + R_SA_TOLUD) & B_SA_TOLUD_TOLUD_MAS=
K) - 1;
diff --git a/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchS=
piCommonLib/SpiCommon.c b/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPri=
vate/BasePchSpiCommonLib/SpiCommon.c
index 58757a8cba39..d2eb8324bf58 100644
--- a/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommo=
nLib/SpiCommon.c
+++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommo=
nLib/SpiCommon.c
@@ -2,10 +2,13 @@
PCH SPI Common Driver implements the SPI Host Controller Compatibility=
Interface.
=20
Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) Microsoft Corporation.<BR>
+
SPDX-License-Identifier: BSD-2-Clause-Patent
=20
**/
#include <Uefi/UefiBaseType.h>
+#include <Guid/FlashRegion.h>
#include <Library/IoLib.h>
#include <Library/DebugLib.h>
#include <Library/BaseMemoryLib.h>
@@ -16,6 +19,90 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Protocol/Spi.h>
#include <Library/PchSpiCommonLib.h>
=20
+typedef enum {
+ FlashRegionDescriptor,
+ FlashRegionBios,
+ FlashRegionMe,
+ FlashRegionGbe,
+ FlashRegionPlatformData,
+ FlashRegionDer,
+ FlashRegionAll,
+ FlashRegionMax
+} FLASH_REGION_TYPE;
+
+typedef struct {
+ EFI_GUID *Guid;
+ FLASH_REGION_TYPE Type;
+} FLASH_REGION_MAPPING;
+
+FLASH_REGION_MAPPING mFlashRegionTypes[] =3D {
+ {
+ &gFlashRegionDescriptorGuid,
+ FlashRegionDescriptor
+ },
+ {
+ &gFlashRegionBiosGuid,
+ FlashRegionBios
+ },
+ {
+ &gFlashRegionMeGuid,
+ FlashRegionMe
+ },
+ {
+ &gFlashRegionGbeGuid,
+ FlashRegionGbe
+ },
+ {
+ &gFlashRegionPlatformDataGuid,
+ FlashRegionPlatformData
+ },
+ {
+ &gFlashRegionDerGuid,
+ FlashRegionDer
+ },
+ {
+ &gFlashRegionAllGuid,
+ FlashRegionAll
+ },
+ {
+ &gFlashRegionMaxGuid,
+ FlashRegionMax
+ }
+};
+
+/**
+ Returns the type of a flash region given its GUID.
+
+ @param[in] FlashRegionGuid Pointer to the flash region GUID.
+ @param[out] FlashRegionType Pointer to a buffer that will be set t=
o the flash region type value.
+
+ @retval EFI_SUCCESS The flash region type was found =
for the given flash region GUID.
+ @retval EFI_INVALID_PARAMETER A pointer argument passed to the=
function is NULL.
+ @retval EFI_NOT_FOUND The flash region type was not fo=
und for the given flash region GUID.
+
+**/
+EFI_STATUS
+GetFlashRegionType (
+ IN EFI_GUID *FlashRegionGuid,
+ OUT FLASH_REGION_TYPE *FlashRegionType
+ )
+{
+ UINTN Index;
+
+ if (FlashRegionGuid =3D=3D NULL || FlashRegionType =3D=3D NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ for (Index =3D 0; Index < ARRAY_SIZE (mFlashRegionTypes); Index++) {
+ if (CompareGuid (mFlashRegionTypes[Index].Guid, FlashRegionGuid)) {
+ *FlashRegionType =3D mFlashRegionTypes[Index].Type;
+ return EFI_SUCCESS;
+ }
+ }
+
+ return EFI_NOT_FOUND;
+}
+
/**
Initialize an SPI protocol instance.
=20
@@ -249,7 +336,7 @@ PchPmTimerStallRuntimeSafe (
Read data from the flash part.
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for flash cycle =
which is listed in the Descriptor.
+ @param[in] FlashRegionGuid The Flash Region GUID for flash cycle =
which corresponds to the type in the descriptor.
@param[in] Address The Flash Linear Address must fall wit=
hin a region for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of=
the SPI cycle.
@param[out] Buffer The Pointer to caller-allocated buffer=
containing the dada received.
@@ -263,7 +350,7 @@ EFI_STATUS
EFIAPI
SpiProtocolFlashRead (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
IN UINT32 Address,
IN UINT32 ByteCount,
OUT UINT8 *Buffer
@@ -276,7 +363,7 @@ SpiProtocolFlashRead (
//
Status =3D SendSpiCmd (
This,
- FlashRegionType,
+ FlashRegionGuid,
FlashCycleRead,
Address,
ByteCount,
@@ -289,7 +376,7 @@ SpiProtocolFlashRead (
Write data to the flash part.
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for flash cycle =
which is listed in the Descriptor.
+ @param[in] FlashRegionGuid The Flash Region GUID for flash cycle =
which corresponds to the type in the descriptor.
@param[in] Address The Flash Linear Address must fall wit=
hin a region for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of=
the SPI cycle.
@param[in] Buffer Pointer to caller-allocated buffer con=
taining the data sent during the SPI cycle.
@@ -302,7 +389,7 @@ EFI_STATUS
EFIAPI
SpiProtocolFlashWrite (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
IN UINT32 Address,
IN UINT32 ByteCount,
IN UINT8 *Buffer
@@ -315,7 +402,7 @@ SpiProtocolFlashWrite (
//
Status =3D SendSpiCmd (
This,
- FlashRegionType,
+ FlashRegionGuid,
FlashCycleWrite,
Address,
ByteCount,
@@ -328,7 +415,7 @@ SpiProtocolFlashWrite (
Erase some area on the flash part.
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for flash cycle =
which is listed in the Descriptor.
+ @param[in] FlashRegionGuid The Flash Region GUID for flash cycle =
which corresponds to the type in the descriptor.
@param[in] Address The Flash Linear Address must fall wit=
hin a region for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of=
the SPI cycle.
=20
@@ -340,7 +427,7 @@ EFI_STATUS
EFIAPI
SpiProtocolFlashErase (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
IN UINT32 Address,
IN UINT32 ByteCount
)
@@ -352,7 +439,7 @@ SpiProtocolFlashErase (
//
Status =3D SendSpiCmd (
This,
- FlashRegionType,
+ FlashRegionGuid,
FlashCycleErase,
Address,
ByteCount,
@@ -407,7 +494,7 @@ SpiProtocolFlashReadSfdp (
//
Status =3D SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleReadSfdp,
FlashAddress,
ByteCount,
@@ -460,7 +547,7 @@ SpiProtocolFlashReadJedecId (
//
Status =3D SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleReadJedecId,
Address,
ByteCount,
@@ -495,7 +582,7 @@ SpiProtocolFlashWriteStatus (
//
Status =3D SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleWriteStatus,
0,
ByteCount,
@@ -530,7 +617,7 @@ SpiProtocolFlashReadStatus (
//
Status =3D SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleReadStatus,
0,
ByteCount,
@@ -543,7 +630,7 @@ SpiProtocolFlashReadStatus (
Get the SPI region base and size, based on the enum type
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for for the base=
address which is listed in the Descriptor.
+ @param[in] FlashRegionGuid The Flash Region GUID for the base add=
ress which corresponds to the type in the descriptor.
@param[out] BaseAddress The Flash Linear Address for the Regio=
n 'n' Base
@param[out] RegionSize The size for the Region 'n'
=20
@@ -555,17 +642,24 @@ EFI_STATUS
EFIAPI
SpiProtocolGetRegionAddress (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
OUT UINT32 *BaseAddress,
OUT UINT32 *RegionSize
)
{
- SPI_INSTANCE *SpiInstance;
- UINTN PchSpiBar0;
- UINT32 ReadValue;
+ EFI_STATUS Status;
+ FLASH_REGION_TYPE FlashRegionType;
+ SPI_INSTANCE *SpiInstance;
+ UINTN PchSpiBar0;
+ UINT32 ReadValue;
=20
SpiInstance =3D SPI_INSTANCE_FROM_SPIPROTOCOL (This);
=20
+ Status =3D GetFlashRegionType (FlashRegionGuid, &FlashRegionType);
+ if (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
if (FlashRegionType >=3D FlashRegionMax) {
return EFI_INVALID_PARAMETER;
}
@@ -646,7 +740,7 @@ SpiProtocolReadPchSoftStrap (
//
Status =3D SendSpiCmd (
This,
- FlashRegionDescriptor,
+ &gFlashRegionDescriptorGuid,
FlashCycleRead,
StrapFlashAddr,
ByteCount,
@@ -704,7 +798,7 @@ SpiProtocolReadCpuSoftStrap (
//
Status =3D SendSpiCmd (
This,
- FlashRegionDescriptor,
+ &gFlashRegionDescriptorGuid,
FlashCycleRead,
StrapFlashAddr,
ByteCount,
@@ -717,7 +811,7 @@ SpiProtocolReadCpuSoftStrap (
This function sends the programmed SPI command to the slave device.
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] SpiRegionType The SPI Region type for flash cycle wh=
ich is listed in the Descriptor
+ @param[in] FlashRegionGuid The Flash Region GUID for flash cycle =
which corresponds to the type in the descriptor.
@param[in] FlashCycleType The Flash SPI cycle type list in HSFC =
(Hardware Sequencing Flash Control Register) register
@param[in] Address The Flash Linear Address must fall wit=
hin a region for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of=
the SPI cycle.
@@ -731,7 +825,7 @@ SpiProtocolReadCpuSoftStrap (
EFI_STATUS
SendSpiCmd (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
IN FLASH_CYCLE_TYPE FlashCycleType,
IN UINT32 Address,
IN UINT32 ByteCount,
@@ -795,7 +889,7 @@ SendSpiCmd (
goto SendSpiCmdEnd;
}
=20
- Status =3D SpiProtocolGetRegionAddress (This, FlashRegionType, &Hardwa=
reSpiAddr, &FlashRegionSize);
+ Status =3D SpiProtocolGetRegionAddress (This, FlashRegionGuid, &Hardwa=
reSpiAddr, &FlashRegionSize);
if (EFI_ERROR (Status)) {
goto SendSpiCmdEnd;
}
diff --git a/Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/HstiSiliconDxe.inf=
b/Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/HstiSiliconDxe.inf
index bd12fa691d40..09826cdfdf39 100644
--- a/Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/HstiSiliconDxe.inf
+++ b/Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/HstiSiliconDxe.inf
@@ -2,6 +2,7 @@
# Component description file for Hsti Silicon Driver
#
# Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) Microsoft Corporation.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -86,6 +87,14 @@ [LibraryClasses]
[Guids]
gEfiEndOfDxeEventGroupGuid
gSiMemoryPlatformDataGuid ## CONSUMES
+ gFlashRegionDescriptorGuid
+ gFlashRegionBiosGuid
+ gFlashRegionMeGuid
+ gFlashRegionGbeGuid
+ gFlashRegionPlatformDataGuid
+ gFlashRegionDerGuid
+ gFlashRegionAllGuid
+ gFlashRegionMaxGuid
=20
[Protocols]
gEfiDxeSmmReadyToLockProtocolGuid ## CONSUMES
diff --git a/Silicon/Intel/KabylakeSiliconPkg/Pch/IncludePrivate/Library/=
PchSpiCommonLib.h b/Silicon/Intel/KabylakeSiliconPkg/Pch/IncludePrivate/L=
ibrary/PchSpiCommonLib.h
index d408289ea253..fd991de96016 100644
--- a/Silicon/Intel/KabylakeSiliconPkg/Pch/IncludePrivate/Library/PchSpiC=
ommonLib.h
+++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/IncludePrivate/Library/PchSpiC=
ommonLib.h
@@ -134,7 +134,7 @@ ReleaseSpiBar0 (
Read data from the flash part.
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for flash cycle =
which is listed in the Descriptor.
+ @param[in] FlashRegionGuid The Flash Region GUID for flash cycle =
which corresponds to the type in the descriptor.
@param[in] Address The Flash Linear Address must fall wit=
hin a region for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of=
the SPI cycle.
@param[out] Buffer The Pointer to caller-allocated buffer=
containing the dada received.
@@ -148,7 +148,7 @@ EFI_STATUS
EFIAPI
SpiProtocolFlashRead (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
IN UINT32 Address,
IN UINT32 ByteCount,
OUT UINT8 *Buffer
@@ -158,7 +158,7 @@ SpiProtocolFlashRead (
Write data to the flash part.
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for flash cycle =
which is listed in the Descriptor.
+ @param[in] FlashRegionGuid The Flash Region GUID for flash cycle =
which corresponds to the type in the descriptor.
@param[in] Address The Flash Linear Address must fall wit=
hin a region for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of=
the SPI cycle.
@param[in] Buffer Pointer to caller-allocated buffer con=
taining the data sent during the SPI cycle.
@@ -171,7 +171,7 @@ EFI_STATUS
EFIAPI
SpiProtocolFlashWrite (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
IN UINT32 Address,
IN UINT32 ByteCount,
IN UINT8 *Buffer
@@ -181,7 +181,7 @@ SpiProtocolFlashWrite (
Erase some area on the flash part.
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for flash cycle =
which is listed in the Descriptor.
+ @param[in] FlashRegionGuid The Flash Region GUID for flash cycle =
which corresponds to the type in the descriptor.
@param[in] Address The Flash Linear Address must fall wit=
hin a region for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of=
the SPI cycle.
=20
@@ -193,7 +193,7 @@ EFI_STATUS
EFIAPI
SpiProtocolFlashErase (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
IN UINT32 Address,
IN UINT32 ByteCount
);
@@ -286,7 +286,7 @@ SpiProtocolFlashReadStatus (
Get the SPI region base and size, based on the enum type
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for for the base=
address which is listed in the Descriptor.
+ @param[in] FlashRegionGuid The Flash Region GUID for flash cycle =
which corresponds to the type in the descriptor.
@param[out] BaseAddress The Flash Linear Address for the Regio=
n 'n' Base
@param[out] RegionSize The size for the Region 'n'
=20
@@ -298,7 +298,7 @@ EFI_STATUS
EFIAPI
SpiProtocolGetRegionAddress (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
OUT UINT32 *BaseAddress,
OUT UINT32 *RegionSize
);
@@ -353,7 +353,7 @@ SpiProtocolReadCpuSoftStrap (
This function sends the programmed SPI command to the slave device.
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] SpiRegionType The SPI Region type for flash cycle wh=
ich is listed in the Descriptor
+ @param[in] FlashRegionGuid The Flash Region GUID for flash cycle =
which corresponds to the type in the descriptor.
@param[in] FlashCycleType The Flash SPI cycle type list in HSFC =
(Hardware Sequencing Flash Control Register) register
@param[in] Address The Flash Linear Address must fall wit=
hin a region for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of=
the SPI cycle.
@@ -367,7 +367,7 @@ SpiProtocolReadCpuSoftStrap (
EFI_STATUS
SendSpiCmd (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
IN FLASH_CYCLE_TYPE FlashCycleType,
IN UINT32 Address,
IN UINT32 ByteCount,
diff --git a/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchS=
piCommonLib/BasePchSpiCommonLib.inf b/Silicon/Intel/KabylakeSiliconPkg/Pc=
h/LibraryPrivate/BasePchSpiCommonLib/BasePchSpiCommonLib.inf
index 51e2d25a7f8b..67176c879de5 100644
--- a/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommo=
nLib/BasePchSpiCommonLib.inf
+++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommo=
nLib/BasePchSpiCommonLib.inf
@@ -2,6 +2,7 @@
# Component description file for the PchSpiCommonLib
#
# Copyright (c) 2017 - 2020 Intel Corporation. All rights reserved.<BR>
+# Copyright (c) Microsoft Corporation.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -28,3 +29,13 @@ [LibraryClasses]
IoLib
DebugLib
PchCycleDecodingLib
+
+[Guids]
+ gFlashRegionDescriptorGuid
+ gFlashRegionBiosGuid
+ gFlashRegionMeGuid
+ gFlashRegionGbeGuid
+ gFlashRegionPlatformDataGuid
+ gFlashRegionDerGuid
+ gFlashRegionAllGuid
+ gFlashRegionMaxGuid
--=20
2.28.0.windows.1


[edk2-platforms][PATCH v3 38/41] CoffeelakeSiliconPkg/BasePchSpiCommonLib: Identify flash regions by GUID

Michael Kubacki
 

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

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

Updates the library to identify flash regions by GUID and internally
map the GUID entries to values specific to CoffeelakeSiliconPkg.

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCommonL=
ib/SpiCommon.c | 144 ++++++++++++++++----
Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Private/Library/PchSpiCom=
monLib.h | 16 +--
Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCommonL=
ib/BasePchSpiCommonLib.inf | 12 ++
3 files changed, 141 insertions(+), 31 deletions(-)

diff --git a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BaseP=
chSpiCommonLib/SpiCommon.c b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Libra=
ry/Private/BasePchSpiCommonLib/SpiCommon.c
index bc84a4f27f1a..26a3d0e7db31 100644
--- a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCo=
mmonLib/SpiCommon.c
+++ b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCo=
mmonLib/SpiCommon.c
@@ -2,11 +2,13 @@
PCH SPI Common Driver implements the SPI Host Controller Compatibility=
Interface.
=20
Copyright (c) 2019 Intel Corporation. All rights reserved. <BR>
+ Copyright (c) Microsoft Corporation.<BR>
=20
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
=20
#include <Uefi/UefiBaseType.h>
+#include <Guid/FlashRegion.h>
#include <Library/BaseLib.h>
#include <Library/IoLib.h>
#include <Library/DebugLib.h>
@@ -20,6 +22,95 @@
#include <Register/PchRegsSpi.h>
#include <Register/PchRegsPmc.h>
=20
+typedef enum {
+ FlashRegionDescriptor,
+ FlashRegionBios,
+ FlashRegionMe,
+ FlashRegionGbe,
+ FlashRegionPlatformData,
+ FlashRegionDer,
+ FlashRegionEc =3D 8,
+ FlashRegionAll,
+ FlashRegionMax
+} FLASH_REGION_TYPE;
+
+typedef struct {
+ EFI_GUID *Guid;
+ FLASH_REGION_TYPE Type;
+} FLASH_REGION_MAPPING;
+
+FLASH_REGION_MAPPING mFlashRegionTypes[] =3D {
+ {
+ &gFlashRegionDescriptorGuid,
+ FlashRegionDescriptor
+ },
+ {
+ &gFlashRegionBiosGuid,
+ FlashRegionBios
+ },
+ {
+ &gFlashRegionMeGuid,
+ FlashRegionMe
+ },
+ {
+ &gFlashRegionGbeGuid,
+ FlashRegionGbe
+ },
+ {
+ &gFlashRegionPlatformDataGuid,
+ FlashRegionPlatformData
+ },
+ {
+ &gFlashRegionDerGuid,
+ FlashRegionDer
+ },
+ {
+ &gFlashRegionEcGuid,
+ FlashRegionEc
+ },
+ {
+ &gFlashRegionAllGuid,
+ FlashRegionAll
+ },
+ {
+ &gFlashRegionMaxGuid,
+ FlashRegionMax
+ }
+};
+
+/**
+ Returns the type of a flash region given its GUID.
+
+ @param[in] FlashRegionGuid Pointer to the flash region GUID.
+ @param[out] FlashRegionType Pointer to a buffer that will be set t=
o the flash region type value.
+
+ @retval EFI_SUCCESS The flash region type was found =
for the given flash region GUID.
+ @retval EFI_INVALID_PARAMETER A pointer argument passed to the=
function is NULL.
+ @retval EFI_NOT_FOUND The flash region type was not fo=
und for the given flash region GUID.
+
+**/
+EFI_STATUS
+GetFlashRegionType (
+ IN EFI_GUID *FlashRegionGuid,
+ OUT FLASH_REGION_TYPE *FlashRegionType
+ )
+{
+ UINTN Index;
+
+ if (FlashRegionGuid =3D=3D NULL || FlashRegionType =3D=3D NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ for (Index =3D 0; Index < ARRAY_SIZE (mFlashRegionTypes); Index++) {
+ if (CompareGuid (mFlashRegionTypes[Index].Guid, FlashRegionGuid)) {
+ *FlashRegionType =3D mFlashRegionTypes[Index].Type;
+ return EFI_SUCCESS;
+ }
+ }
+
+ return EFI_NOT_FOUND;
+}
+
/**
Initialize an SPI protocol instance.
=20
@@ -303,7 +394,7 @@ WaitForSpiCycleComplete (
This function sends the programmed SPI command to the slave device.
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] SpiRegionType The SPI Region type for flash cycle wh=
ich is listed in the Descriptor
+ @param[in] FlashRegionGuid The Flash Region GUID for flash cycle =
which corresponds to the type in the descriptor.
@param[in] FlashCycleType The Flash SPI cycle type list in HSFC =
(Hardware Sequencing Flash Control Register) register
@param[in] Address The Flash Linear Address must fall wit=
hin a region for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of=
the SPI cycle.
@@ -318,7 +409,7 @@ STATIC
EFI_STATUS
SendSpiCmd (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
IN FLASH_CYCLE_TYPE FlashCycleType,
IN UINT32 Address,
IN UINT32 ByteCount,
@@ -404,7 +495,7 @@ SendSpiCmd (
}
}
=20
- Status =3D SpiProtocolGetRegionAddress (This, FlashRegionType, &Hardwa=
reSpiAddr, &FlashRegionSize);
+ Status =3D SpiProtocolGetRegionAddress (This, FlashRegionGuid, &Hardwa=
reSpiAddr, &FlashRegionSize);
if (EFI_ERROR (Status)) {
goto SendSpiCmdEnd;
}
@@ -616,7 +707,7 @@ SendSpiCmd (
Read data from the flash part.
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for flash cycle =
which is listed in the Descriptor.
+ @param[in] FlashRegionGuid The Flash Region GUID for flash cycle =
which corresponds to the type in the descriptor.
@param[in] Address The Flash Linear Address must fall wit=
hin a region for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of=
the SPI cycle.
@param[out] Buffer The Pointer to caller-allocated buffer=
containing the dada received.
@@ -630,7 +721,7 @@ EFI_STATUS
EFIAPI
SpiProtocolFlashRead (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
IN UINT32 Address,
IN UINT32 ByteCount,
OUT UINT8 *Buffer
@@ -643,7 +734,7 @@ SpiProtocolFlashRead (
//
Status =3D SendSpiCmd (
This,
- FlashRegionType,
+ FlashRegionGuid,
FlashCycleRead,
Address,
ByteCount,
@@ -656,7 +747,7 @@ SpiProtocolFlashRead (
Write data to the flash part.
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for flash cycle =
which is listed in the Descriptor.
+ @param[in] FlashRegionGuid The Flash Region GUID for flash cycle =
which corresponds to the type in the descriptor.
@param[in] Address The Flash Linear Address must fall wit=
hin a region for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of=
the SPI cycle.
@param[in] Buffer Pointer to caller-allocated buffer con=
taining the data sent during the SPI cycle.
@@ -669,7 +760,7 @@ EFI_STATUS
EFIAPI
SpiProtocolFlashWrite (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
IN UINT32 Address,
IN UINT32 ByteCount,
IN UINT8 *Buffer
@@ -682,7 +773,7 @@ SpiProtocolFlashWrite (
//
Status =3D SendSpiCmd (
This,
- FlashRegionType,
+ FlashRegionGuid,
FlashCycleWrite,
Address,
ByteCount,
@@ -695,7 +786,7 @@ SpiProtocolFlashWrite (
Erase some area on the flash part.
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for flash cycle =
which is listed in the Descriptor.
+ @param[in] FlashRegionGuid The Flash Region GUID for flash cycle =
which corresponds to the type in the descriptor.
@param[in] Address The Flash Linear Address must fall wit=
hin a region for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of=
the SPI cycle.
=20
@@ -707,7 +798,7 @@ EFI_STATUS
EFIAPI
SpiProtocolFlashErase (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
IN UINT32 Address,
IN UINT32 ByteCount
)
@@ -719,7 +810,7 @@ SpiProtocolFlashErase (
//
Status =3D SendSpiCmd (
This,
- FlashRegionType,
+ FlashRegionGuid,
FlashCycleErase,
Address,
ByteCount,
@@ -774,7 +865,7 @@ SpiProtocolFlashReadSfdp (
//
Status =3D SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleReadSfdp,
FlashAddress,
ByteCount,
@@ -827,7 +918,7 @@ SpiProtocolFlashReadJedecId (
//
Status =3D SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleReadJedecId,
Address,
ByteCount,
@@ -862,7 +953,7 @@ SpiProtocolFlashWriteStatus (
//
Status =3D SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleWriteStatus,
0,
ByteCount,
@@ -897,7 +988,7 @@ SpiProtocolFlashReadStatus (
//
Status =3D SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleReadStatus,
0,
ByteCount,
@@ -910,7 +1001,7 @@ SpiProtocolFlashReadStatus (
Get the SPI region base and size, based on the enum type
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for for the base=
address which is listed in the Descriptor.
+ @param[in] FlashRegionGuid The Flash Region GUID for the base add=
ress which corresponds to the type in the descriptor.
@param[out] BaseAddress The Flash Linear Address for the Regio=
n 'n' Base
@param[out] RegionSize The size for the Region 'n'
=20
@@ -922,17 +1013,24 @@ EFI_STATUS
EFIAPI
SpiProtocolGetRegionAddress (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
OUT UINT32 *BaseAddress,
OUT UINT32 *RegionSize
)
{
- SPI_INSTANCE *SpiInstance;
- UINTN PchSpiBar0;
- UINT32 ReadValue;
+ EFI_STATUS Status;
+ FLASH_REGION_TYPE FlashRegionType;
+ SPI_INSTANCE *SpiInstance;
+ UINTN PchSpiBar0;
+ UINT32 ReadValue;
=20
SpiInstance =3D SPI_INSTANCE_FROM_SPIPROTOCOL (This);
=20
+ Status =3D GetFlashRegionType (FlashRegionGuid, &FlashRegionType);
+ if (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
if (FlashRegionType >=3D FlashRegionMax) {
return EFI_INVALID_PARAMETER;
}
@@ -1013,7 +1111,7 @@ SpiProtocolReadPchSoftStrap (
//
Status =3D SendSpiCmd (
This,
- FlashRegionDescriptor,
+ &gFlashRegionDescriptorGuid,
FlashCycleRead,
StrapFlashAddr,
ByteCount,
@@ -1071,7 +1169,7 @@ SpiProtocolReadCpuSoftStrap (
//
Status =3D SendSpiCmd (
This,
- FlashRegionDescriptor,
+ &gFlashRegionDescriptorGuid,
FlashCycleRead,
StrapFlashAddr,
ByteCount,
diff --git a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Private/Libra=
ry/PchSpiCommonLib.h b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Pri=
vate/Library/PchSpiCommonLib.h
index 0a973a77a381..e69e2f1e456c 100644
--- a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Private/Library/PchS=
piCommonLib.h
+++ b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Private/Library/PchS=
piCommonLib.h
@@ -148,7 +148,7 @@ IsSpiFlashWriteGranted (
Read data from the flash part.
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for flash cycle =
which is listed in the Descriptor.
+ @param[in] FlashRegionGuid The Flash Region GUID for flash cycle =
which corresponds to the type in the descriptor.
@param[in] Address The Flash Linear Address must fall wit=
hin a region for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of=
the SPI cycle.
@param[out] Buffer The Pointer to caller-allocated buffer=
containing the dada received.
@@ -162,7 +162,7 @@ EFI_STATUS
EFIAPI
SpiProtocolFlashRead (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
IN UINT32 Address,
IN UINT32 ByteCount,
OUT UINT8 *Buffer
@@ -172,7 +172,7 @@ SpiProtocolFlashRead (
Write data to the flash part.
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for flash cycle =
which is listed in the Descriptor.
+ @param[in] FlashRegionGuid The Flash Region GUID for flash cycle =
which corresponds to the type in the descriptor.
@param[in] Address The Flash Linear Address must fall wit=
hin a region for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of=
the SPI cycle.
@param[in] Buffer Pointer to caller-allocated buffer con=
taining the data sent during the SPI cycle.
@@ -185,7 +185,7 @@ EFI_STATUS
EFIAPI
SpiProtocolFlashWrite (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
IN UINT32 Address,
IN UINT32 ByteCount,
IN UINT8 *Buffer
@@ -195,7 +195,7 @@ SpiProtocolFlashWrite (
Erase some area on the flash part.
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for flash cycle =
which is listed in the Descriptor.
+ @param[in] FlashRegionGuid The Flash Region GUID for flash cycle =
which corresponds to the type in the descriptor.
@param[in] Address The Flash Linear Address must fall wit=
hin a region for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of=
the SPI cycle.
=20
@@ -207,7 +207,7 @@ EFI_STATUS
EFIAPI
SpiProtocolFlashErase (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
IN UINT32 Address,
IN UINT32 ByteCount
);
@@ -300,7 +300,7 @@ SpiProtocolFlashReadStatus (
Get the SPI region base and size, based on the enum type
=20
@param[in] This Pointer to the PCH_SPI_PROTOCOL instan=
ce.
- @param[in] FlashRegionType The Flash Region type for for the base=
address which is listed in the Descriptor.
+ @param[in] FlashRegionGuid The Flash Region GUID for the base add=
ress which corresponds to the type in the descriptor.
@param[out] BaseAddress The Flash Linear Address for the Regio=
n 'n' Base
@param[out] RegionSize The size for the Region 'n'
=20
@@ -312,7 +312,7 @@ EFI_STATUS
EFIAPI
SpiProtocolGetRegionAddress (
IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
+ IN EFI_GUID *FlashRegionGuid,
OUT UINT32 *BaseAddress,
OUT UINT32 *RegionSize
);
diff --git a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BaseP=
chSpiCommonLib/BasePchSpiCommonLib.inf b/Silicon/Intel/CoffeelakeSiliconP=
kg/Pch/Library/Private/BasePchSpiCommonLib/BasePchSpiCommonLib.inf
index f5dc4ee0bfef..b152d2278839 100644
--- a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCo=
mmonLib/BasePchSpiCommonLib.inf
+++ b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCo=
mmonLib/BasePchSpiCommonLib.inf
@@ -2,6 +2,7 @@
# Component description file for the PchSpiCommonLib
#
# Copyright (c) 2019 Intel Corporation. All rights reserved. <BR>
+# Copyright (c) Microsoft Corporation.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -27,3 +28,14 @@ [LibraryClasses]
IoLib
DebugLib
PmcLib
+
+[Guids]
+ gFlashRegionDescriptorGuid
+ gFlashRegionBiosGuid
+ gFlashRegionMeGuid
+ gFlashRegionGbeGuid
+ gFlashRegionPlatformDataGuid
+ gFlashRegionDerGuid
+ gFlashRegionEcGuid
+ gFlashRegionAllGuid
+ gFlashRegionMaxGuid
--=20
2.28.0.windows.1

5641 - 5660 of 82317