Date   

[edk2-platforms][PATCH v3 40/41] SimicsIch10Pkg/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 SimicsIch10Pkg.

Cc: Agyeman Prince <prince.agyeman@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
Silicon/Intel/SimicsIch10Pkg/LibraryPrivate/BasePchSpiCommonLib/SpiCommo=
n.c | 139 ++++++++++++++++----
Silicon/Intel/SimicsIch10Pkg/IncludePrivate/Library/PchSpiCommonLib.h =
| 20 +--
Silicon/Intel/SimicsIch10Pkg/LibraryPrivate/BasePchSpiCommonLib/BasePchS=
piCommonLib.inf | 11 ++
3 files changed, 137 insertions(+), 33 deletions(-)

diff --git a/Silicon/Intel/SimicsIch10Pkg/LibraryPrivate/BasePchSpiCommon=
Lib/SpiCommon.c b/Silicon/Intel/SimicsIch10Pkg/LibraryPrivate/BasePchSpiC=
ommonLib/SpiCommon.c
index fc2a8be76b6a..04dbd921c091 100644
--- a/Silicon/Intel/SimicsIch10Pkg/LibraryPrivate/BasePchSpiCommonLib/Spi=
Common.c
+++ b/Silicon/Intel/SimicsIch10Pkg/LibraryPrivate/BasePchSpiCommonLib/Spi=
Common.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/IoLib.h>
#include <Library/DebugLib.h>
#include <Library/BaseMemoryLib.h>
@@ -16,6 +18,90 @@
#include <IncludePrivate/Library/PchSpiCommonLib.h>
#include <Register/X58Ich10.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
@@ -145,7 +231,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.
@@ -159,7 +245,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 +258,7 @@ SpiProtocolFlashRead (
//
Status =3D SendSpiCmd (
This,
- FlashRegionType,
+ FlashRegionGuid,
FlashCycleRead,
Address,
ByteCount,
@@ -185,7 +271,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.
@@ -198,7 +284,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
@@ -211,7 +297,7 @@ SpiProtocolFlashWrite (
//
Status =3D SendSpiCmd (
This,
- FlashRegionType,
+ FlashRegionGuid,
FlashCycleWrite,
Address,
ByteCount,
@@ -224,7 +310,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
@@ -236,7 +322,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
)
@@ -248,7 +334,7 @@ SpiProtocolFlashErase (
//
Status =3D SendSpiCmd (
This,
- FlashRegionType,
+ FlashRegionGuid,
FlashCycleErase,
Address,
ByteCount,
@@ -303,7 +389,7 @@ SpiProtocolFlashReadSfdp (
//
Status =3D SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleReadSfdp,
FlashAddress,
ByteCount,
@@ -356,7 +442,7 @@ SpiProtocolFlashReadJedecId (
//
Status =3D SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleReadJedecId,
Address,
ByteCount,
@@ -391,7 +477,7 @@ SpiProtocolFlashWriteStatus (
//
Status =3D SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleWriteStatus,
0,
ByteCount,
@@ -426,7 +512,7 @@ SpiProtocolFlashReadStatus (
//
Status =3D SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleReadStatus,
0,
ByteCount,
@@ -439,7 +525,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
@@ -451,17 +537,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;
}
@@ -542,7 +635,7 @@ SpiProtocolReadPchSoftStrap (
//
Status =3D SendSpiCmd (
This,
- FlashRegionDescriptor,
+ &gFlashRegionDescriptorGuid,
FlashCycleRead,
StrapFlashAddr,
ByteCount,
@@ -600,7 +693,7 @@ SpiProtocolReadCpuSoftStrap (
//
Status =3D SendSpiCmd (
This,
- FlashRegionDescriptor,
+ &gFlashRegionDescriptorGuid,
FlashCycleRead,
StrapFlashAddr,
ByteCount,
@@ -613,7 +706,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.
@@ -627,7 +720,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,
@@ -682,7 +775,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/SimicsIch10Pkg/IncludePrivate/Library/PchSpiCo=
mmonLib.h b/Silicon/Intel/SimicsIch10Pkg/IncludePrivate/Library/PchSpiCom=
monLib.h
index 2c8162ac8170..603e141e2058 100644
--- a/Silicon/Intel/SimicsIch10Pkg/IncludePrivate/Library/PchSpiCommonLib=
.h
+++ b/Silicon/Intel/SimicsIch10Pkg/IncludePrivate/Library/PchSpiCommonLib=
.h
@@ -135,7 +135,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.
@@ -149,7 +149,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
@@ -159,7 +159,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.
@@ -172,7 +172,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
@@ -182,7 +182,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
@@ -194,7 +194,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
);
@@ -287,7 +287,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
@@ -299,7 +299,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
);
@@ -354,7 +354,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.
@@ -368,7 +368,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/SimicsIch10Pkg/LibraryPrivate/BasePchSpiCommon=
Lib/BasePchSpiCommonLib.inf b/Silicon/Intel/SimicsIch10Pkg/LibraryPrivate=
/BasePchSpiCommonLib/BasePchSpiCommonLib.inf
index b5aa13c1c56d..3a64005b5690 100644
--- a/Silicon/Intel/SimicsIch10Pkg/LibraryPrivate/BasePchSpiCommonLib/Bas=
ePchSpiCommonLib.inf
+++ b/Silicon/Intel/SimicsIch10Pkg/LibraryPrivate/BasePchSpiCommonLib/Bas=
ePchSpiCommonLib.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
#
@@ -30,3 +31,13 @@ [LibraryClasses]
[Pcd]
gIntelSiliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress ## CONSUMES
gIntelSiliconPkgTokenSpaceGuid.PcdBiosSize ## CONSUMES
+
+[Guids]
+ gFlashRegionDescriptorGuid
+ gFlashRegionBiosGuid
+ gFlashRegionMeGuid
+ gFlashRegionGbeGuid
+ gFlashRegionPlatformDataGuid
+ gFlashRegionDerGuid
+ gFlashRegionAllGuid
+ gFlashRegionMaxGuid
--=20
2.28.0.windows.1


Re: [PATCH V1 4/4] Platform/NXP/LS1046aFrwyPkg: Add OEM specific DSDT generator

Vikas Singh
 

On Tue, Jun 15, 2021 at 3:07 AM Leif Lindholm <leif@nuviainc.com> wrote:

On Fri, Jun 11, 2021 at 21:22:00 +0530, Vikas Singh wrote:
This patch adds platform specific DSDT generator
and Clk dsdt properties for LS1046AFRWY platform.

Signed-off-by: Vikas Singh <vikas.singh@puresoftware.com>
---
Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/Dsdt/Clk.asl | 60 +++++++++
Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/Dsdt/Dsdt.asl | 15 +++
Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiDsdtLib.inf | 39 ++++++
Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiDsdtLib/RawDsdtGenerator.c | 138 ++++++++++++++++++++
Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiLib.h | 23 ++++
Platform/NXP/LS1046aFrwyPkg/Include/Platform.h | 6 +-
Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc | 1 +
7 files changed, 281 insertions(+), 1 deletion(-)

diff --git a/Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/Dsdt/Clk.asl b/Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/Dsdt/Clk.asl
new file mode 100644
index 0000000000..58541c3019
--- /dev/null
+++ b/Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/Dsdt/Clk.asl
@@ -0,0 +1,60 @@
+/** @file
+* DSDT : Dynamic Clock ACPI Information
+*
+* Copyright 2021 NXP
+* Copyright 2021 Puresoftware Ltd.
+*
+* SPDX-License-Identifier: BSD-2-Clause-Patent
+*
+**/
+
+Scope(_SB)
+{
+ Device(PCLK) {
+ Name(_HID, "NXP0017")
+ Name(CLK, 0) // Maximum Platform Clock (Hz)
+ Name(CCLK, 0) // Maximum CPU Core Clock (MHz)
+ Name(AVBL, 0)
+ OperationRegion(RCWS, SystemMemory, DCFG_BASE, DCFG_LEN)
+ Method(_REG,2) {
+ if (Arg0 == "RCWS") {
+ Store(Arg1, AVBL)
+ }
+ }
+ Field (RCWS, ByteAcc, NoLock, Preserve) {
+ /* The below table provides the func of diff bits in 512 bits RCW data:
+ SYS_PLL_CFG : 0-1 bits
+ SYS_PLL_RAT : 2-6 bits
+ SYSCLK_FREQ : 472-481 bits etc.
+ Refer LS1046ARM for more info.
+ For LS1046 RCWSRs are read as RCW[0:31] .
+ */
+ offset(0x100),
+ RESV, 1,
+ PRAT, 5,
+ PCFG, 2,
+ offset(0x103),
+ CPRT, 6, // Cluster Group PLL Multiplier ratio
+ offset(0x13B),
+ HFRQ, 8, // Higher 8 bits of SYSCLK_FREQ
+ RESX, 6,
+ LFRQ, 2 // Lower bits of SYSCLK_FREQ
+ }
+
+ Method(_INI, 0, NotSerialized) {
+ /* Calculating Platform Clock */
+ Local0 = (HFRQ<<2 | LFRQ) // Concatinating LFRQ at end of HFRQ
+ Multiply(Local0, 500000, Local0)
+ Multiply(Local0, PRAT, Local0)
+ Divide(Local0, 3, , Local0)
+ Store(Local0, CLK)
+
+ /* Calculating Maximum Core Clock */
+ Local0 = (HFRQ<<2 | LFRQ) // Concatinating LFRQ at end of HFRQ
+ Multiply(Local0, 500000, Local0)
+ Divide(Local0, 3, , Local0)
+ Divide(Local0, 1000000, , Local0) //Just the MHz part of SYSCLK.
+ Multiply(Local0, CPRT, CCLK) // PLL_Ratio * SYSCLK, Max freq of cluster
+ }
+ } // end of device PCLK
+}
diff --git a/Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/Dsdt/Dsdt.asl b/Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/Dsdt/Dsdt.asl
new file mode 100644
index 0000000000..19f3f1c0e8
--- /dev/null
+++ b/Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/Dsdt/Dsdt.asl
@@ -0,0 +1,15 @@
+/** @file
+ Differentiated System Description Table Fields (DSDT)
+
+ Copyright 2021 NXP
+ Copyright 2021 Puresoftware Ltd.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "Platform.h"
+
+DefinitionBlock("DsdtTable.aml", "DSDT", 2, "NXP ", "LS1046 ", EFI_ACPI_ARM_OEM_REVISION) {
+ include ("Clk.asl")
+}
diff --git a/Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiDsdtLib.inf b/Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiDsdtLib.inf
new file mode 100644
index 0000000000..ed5f9dd442
--- /dev/null
+++ b/Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiDsdtLib.inf
@@ -0,0 +1,39 @@
+## @file
+# Raw Table Generator
+#
+# Copyright 2021 NXP
+# Copyright 2021 Puresoftware Ltd
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+ INF_VERSION = 0x00010019
+ BASE_NAME = PlatformAcpiDsdtLib
+ FILE_GUID = A97F70AC-3BB4-4596-B4D2-9F948EC12D17
+ VERSION_STRING = 1.0
+ MODULE_TYPE = DXE_DRIVER
+ LIBRARY_CLASS = NULL|DXE_DRIVER
+ CONSTRUCTOR = AcpiDsdtLibConstructor
+ DESTRUCTOR = AcpiDsdtLibDestructor
+
+[Sources]
+ PlatformAcpiDsdtLib/RawDsdtGenerator.c
+ Dsdt/Dsdt.asl
+
+[Packages]
+ DynamicTablesPkg/DynamicTablesPkg.dec
+ EmbeddedPkg/EmbeddedPkg.dec
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dec
+ Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerPkg.dec
+
+[LibraryClasses]
+ BaseLib
+
+[Pcd]
+
+[Protocols]
+
+[Guids]
diff --git a/Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiDsdtLib/RawDsdtGenerator.c b/Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiDsdtLib/RawDsdtGenerator.c
new file mode 100644
index 0000000000..7d886396ca
--- /dev/null
+++ b/Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiDsdtLib/RawDsdtGenerator.c
@@ -0,0 +1,138 @@
+/** @file
+ Raw DSDT Table Generator
+
+ Copyright 2021 NXP
+ Copyright 2021 Puresoftware Ltd.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/AcpiLib.h>
+#include <Library/DebugLib.h>
+#include <Protocol/AcpiTable.h>
+
+// Module specific include files.
+#include <AcpiTableGenerator.h>
+#include <ConfigurationManagerObject.h>
+#include <ConfigurationManagerHelper.h>
+#include <Library/TableHelperLib.h>
+#include <Protocol/ConfigurationManagerProtocol.h>
+
+#include "PlatformAcpiLib.h"
+
+/** Construct the ACPI table using the ACPI table data provided.
+ This function invokes the Configuration Manager protocol interface
+ to get the required hardware information for generating the ACPI
+ table.
+ If this function allocates any resources then they must be freed
+ in the FreeXXXXTableResources function.
+ @param [in] This Pointer to the table generator.
+ @param [in] AcpiTableInfo Pointer to the ACPI Table Info.
+ @param [in] CfgMgrProtocol Pointer to the Configuration Manager
+ Protocol Interface.
+ @param [out] Table Pointer to the constructed ACPI Table.
+ @retval EFI_SUCCESS Table generated successfully.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+BuildRawDsdtTable (
+ IN CONST ACPI_TABLE_GENERATOR * CONST This,
+ IN CONST CM_STD_OBJ_ACPI_TABLE_INFO * CONST AcpiTableInfo,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL * CONST CfgMgrProtocol,
+ OUT EFI_ACPI_DESCRIPTION_HEADER ** CONST Table
+ )
+{
+ ASSERT (This != NULL);
+ ASSERT (AcpiTableInfo != NULL);
+ ASSERT (CfgMgrProtocol != NULL);
+ ASSERT (Table != NULL);
+ ASSERT (AcpiTableInfo->TableGeneratorId == This->GeneratorID);
+
+ if (AcpiTableInfo->AcpiTableData == NULL) {
+ // Add the dsdt aml code here.
+ *Table = (EFI_ACPI_DESCRIPTION_HEADER *)&dsdt_aml_code;
+ }
+
+ return EFI_SUCCESS;
+}
+
+/** This macro defines the Raw Generator revision.
+*/
+#define DSDT_GENERATOR_REVISION CREATE_REVISION (1, 0)
+
+/** The interface for the Raw Table Generator.
+*/
+STATIC
+CONST
+ACPI_TABLE_GENERATOR RawDsdtGenerator = {
+ // Generator ID
+ CREATE_OEM_ACPI_TABLE_GEN_ID (PlatAcpiTableIdDsdt),
+ // Generator Description
+ L"ACPI.OEM.RAW.DSDT.GENERATOR",
+ // ACPI Table Signature - Unused
+ 0,
+ // ACPI Table Revision - Unused
+ 0,
+ // Minimum ACPI Table Revision - Unused
+ 0,
+ // Creator ID
+ TABLE_GENERATOR_CREATOR_ID_ARM,
+ // Creator Revision
+ DSDT_GENERATOR_REVISION,
+ // Build Table function
+ BuildRawDsdtTable,
+ // No additional resources are allocated by the generator.
+ // Hence the Free Resource function is not required.
+ NULL,
+ // Extended build function not needed
+ NULL,
+ // Extended build function not implemented by the generator.
+ // Hence extended free resource function is not required.
+ NULL
+};
+
+/** Register the Generator with the ACPI Table Factory.
+ @param [in] ImageHandle The handle to the image.
+ @param [in] SystemTable Pointer to the System Table.
+ @retval EFI_SUCCESS The Generator is registered.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_ALREADY_STARTED The Generator for the Table ID
+ is already registered.
+**/
+EFI_STATUS
+EFIAPI
+AcpiDsdtLibConstructor (
+ IN CONST EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * CONST SystemTable
+ )
+{
+ EFI_STATUS Status;
+ Status = RegisterAcpiTableGenerator (&RawDsdtGenerator);
+ DEBUG ((DEBUG_INFO, "OEM: Register DSDT Generator. Status = %r\n", Status));
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+}
+
+/** Deregister the Generator from the ACPI Table Factory.
+ @param [in] ImageHandle The handle to the image.
+ @param [in] SystemTable Pointer to the System Table.
+ @retval EFI_SUCCESS The Generator is deregistered.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND The Generator is not registered.
+**/
+EFI_STATUS
+EFIAPI
+AcpiDsdtLibDestructor (
+ IN CONST EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * CONST SystemTable
+ )
+{
+ EFI_STATUS Status;
+ Status = DeregisterAcpiTableGenerator (&RawDsdtGenerator);
+ DEBUG ((DEBUG_INFO, "OEM: Deregister DSDT Generator. Status = %r\n", Status));
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+}
diff --git a/Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiLib.h b/Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiLib.h
new file mode 100644
index 0000000000..e5f907a7d4
--- /dev/null
+++ b/Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiLib.h
@@ -0,0 +1,23 @@
+/** @file
+ * Acpi lib headers
+ *
+ * Copyright 2021 NXP
+ * Copyright 2021 Puresoftware Ltd
+ *
+ * SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+**/
+
+
+#ifndef LS1046AFRWY_PLATFORM_ACPI_LIB_H
+#define LS1046AFRWY_PLATFORM_ACPI_LIB_H
+
+#include <PlatformAcpiTableGenerator.h>
+
+/** C array containing the compiled AML template.
+ These symbols are defined in the auto generated C file
+ containing the AML bytecode array.
+*/
+extern CHAR8 dsdt_aml_code[];
+
+#endif
diff --git a/Platform/NXP/LS1046aFrwyPkg/Include/Platform.h b/Platform/NXP/LS1046aFrwyPkg/Include/Platform.h
index 19e879ec6d..b21e875f20 100644
--- a/Platform/NXP/LS1046aFrwyPkg/Include/Platform.h
+++ b/Platform/NXP/LS1046aFrwyPkg/Include/Platform.h
@@ -20,6 +20,10 @@
#define SVR_MAJOR(svr) (((svr) >> 4) & 0xf)
#define SVR_MINOR(svr) (((svr) >> 0) & 0xf)

+// PCLK : Dynamic Clock
+#define DCFG_BASE 0x1EE0000 /* Device configuration data Base Address */
+#define DCFG_LEN 0xFFF /* Device configuration data length */
+

// Gic
#define GIC_VERSION 2
#define GICD_BASE 0x1410000
@@ -62,7 +66,7 @@
#define CFG_MGR_TABLE_ID SIGNATURE_64 ('L','S','1','0','4','6',' ',' ')

// Specify the OEM defined tables
-#define OEM_ACPI_TABLES 0
+#define OEM_ACPI_TABLES 1 // Added DSDT
Drop the comment.
With that:
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
Ok, I will add "Reviewed-by: Leif Lindholm <leif@nuviainc.com>" as
part of the commit body.



#define PLAT_PCI_SEG0 LS1046A_PCI_SEG0
#define PLAT_PCI_SEG1_CONFIG_BASE LS1046A_PCI_SEG1_CONFIG_BASE
diff --git a/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc b/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc
index 20111e6037..7041d15da5 100755
--- a/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc
+++ b/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc
@@ -65,6 +65,7 @@
NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/AcpiMadtLibArm.inf
NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLibArm.inf
NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.inf
+ NULL|Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiDsdtLib.inf
}
!endif

--
2.25.1


Re: [PATCH V1 3/4] Platform/NXP/LS1046aFrwyPkg: Extend Dynamic ACPI support

Vikas Singh
 

On Tue, Jun 15, 2021 at 2:58 AM Leif Lindholm <leif@nuviainc.com> wrote:

On Fri, Jun 11, 2021 at 21:21:59 +0530, Vikas Singh wrote:
This patch set extends Configuration Manager (CM) and
its services to leverage the Dynamic ACPI support for
NXP's LS1046aFrwy platform.
This patch does not touch ConfigurationManager.
Please describe what this patch does.

My guess is it's along the lines of:
This set enables use of the ConfigurationManager framework for the
LS1046aFrwy platform.

Refer-https://edk2.groups.io/g/devel/message/71710
That is a 1326-line patch set.
What is this reference supposed to tell me?
Leif,
Yes this patch will enable the usage of ConfigurationManager(CM) for LS1046aFrwy
so that Dynamic ACPI table framework can be reused to generate the
tables on this platform as well.

And https://edk2.groups.io/g/devel/message/71710 was added just to
mark these changes in continuation with the Dynamic ACPI
support that we have already added for layerscape platforms.

Signed-off-by: Vikas Singh <vikas.singh@puresoftware.com>
---
Platform/NXP/LS1046aFrwyPkg/Include/Platform.h | 155 ++++++++++++++++++++
Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc | 28 ++++
Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf | 13 ++
Silicon/NXP/LS1046A/LS1046A.dsc.inc | 10 ++
4 files changed, 206 insertions(+)

diff --git a/Platform/NXP/LS1046aFrwyPkg/Include/Platform.h b/Platform/NXP/LS1046aFrwyPkg/Include/Platform.h
new file mode 100644
index 0000000000..19e879ec6d
--- /dev/null
+++ b/Platform/NXP/LS1046aFrwyPkg/Include/Platform.h
@@ -0,0 +1,155 @@
+/** @file
+ * Platform headers
+ *
+ * Copyright 2021 NXP
+ * Copyright 2021 Puresoftware Ltd
+ *
+ * SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+**/
+
+
+#ifndef LS1046AFRWY_PLATFORM_H
+#define LS1046AFRWY_PLATFORM_H
+
+#define EFI_ACPI_ARM_OEM_REVISION 0x00000000
+
+// Soc defines
+#define PLAT_SOC_NAME "LS1046AFRWY"
+#define SVR_SOC_VER(svr) (((svr) >> 8) & 0xFFFFFE)
+#define SVR_MAJOR(svr) (((svr) >> 4) & 0xf)
+#define SVR_MINOR(svr) (((svr) >> 0) & 0xf)
We already have three identical copies of these three macros:
Platform/NXP/LX2160aRdbPkg/Include/Platform.h
Silicon/NXP/Chassis2/Include/Chassis.h
Silicon/NXP/Chassis3V2/Include/Chassis.h

Could they be defined once, in a single common header, rather than
adding a fourth one?
Sure, will avoid duplicates in platform headers and reuse the
definitions from chassis headers (SoC level)
Not sure if we can define this in a common file as these definitions
can be different based on the SoC versions.

+
+// Gic
+#define GIC_VERSION 2
+#define GICD_BASE 0x1410000
+#define GICC_BASE 0x142f000
+#define GICH_BASE 0x1440000
+#define GICV_BASE 0x1460000
+
+// UART
+#define UART0_BASE 0x21C0500
+#define UART0_IT 86
These (GIC and UART) definitions duplicate things already described in
Silicon/NXP/LS1046A/LS1046A.dsc.inc
I understand you concern lief, But ConfigurationManager is kind of a
dead entity which will always look for platform header for any
platform specific definitions
However, In my opinion as you have suggested, we should start
encouraging the pcd usage in platform headers instead of static
#defines where ever possible.
But this is not the intent of this patch series currently, hopefully
you can expect PCD changes in my upcoming patch series for sure.

/
Leif

+#define UART0_LENGTH 0x100
+#define SPCR_FLOW_CONTROL_NONE 0
+
+// Timer
+#define TIMER_BLOCK_COUNT 1
+#define TIMER_FRAME_COUNT 4
+#define TIMER_WATCHDOG_COUNT 1
+#define TIMER_BASE_ADDRESS 0x23E0000 // a.k.a CNTControlBase
+#define TIMER_READ_BASE_ADDRESS 0x23F0000 // a.k.a CNTReadBase
+#define TIMER_SEC_IT 29
+#define TIMER_NON_SEC_IT 30
+#define TIMER_VIRT_IT 27
+#define TIMER_HYP_IT 26
+#define TIMER_FRAME0_IT 78
+#define TIMER_FRAME1_IT 79
+#define TIMER_FRAME2_IT 92
+
+// Mcfg
+#define LS1046A_PCI_SEG0_CONFIG_BASE 0x4000000000
+#define LS1046A_PCI_SEG0 0x0
+#define LS1046A_PCI_SEG_BUSNUM_MIN 0x0
+#define LS1046A_PCI_SEG_BUSNUM_MAX 0xff
+#define LS1046A_PCI_SEG1_CONFIG_BASE 0x4800000000
+#define LS1046A_PCI_SEG2_CONFIG_BASE 0x5000000000
+#define LS1046A_PCI_SEG1 0x1
+#define LS1046A_PCI_SEG2 0x2
+
+// Platform specific info needed by Configuration Manager
+
+#define CFG_MGR_TABLE_ID SIGNATURE_64 ('L','S','1','0','4','6',' ',' ')
+
+// Specify the OEM defined tables
+#define OEM_ACPI_TABLES 0
+
+#define PLAT_PCI_SEG0 LS1046A_PCI_SEG0
+#define PLAT_PCI_SEG1_CONFIG_BASE LS1046A_PCI_SEG1_CONFIG_BASE
+#define PLAT_PCI_SEG1 LS1046A_PCI_SEG1
+#define PLAT_PCI_SEG_BUSNUM_MIN LS1046A_PCI_SEG_BUSNUM_MIN
+#define PLAT_PCI_SEG_BUSNUM_MAX LS1046A_PCI_SEG_BUSNUM_MAX
+#define PLAT_PCI_SEG2_CONFIG_BASE LS1046A_PCI_SEG2_CONFIG_BASE
+#define PLAT_PCI_SEG2 LS1046A_PCI_SEG2
+
+#define PLAT_GIC_VERSION GIC_VERSION
+#define PLAT_GICD_BASE GICD_BASE
+#define PLAT_GICI_BASE GICI_BASE
+#define PLAT_GICR_BASE GICR_BASE
+#define PLAT_GICR_LEN GICR_LEN
+#define PLAT_GICC_BASE GICC_BASE
+#define PLAT_GICH_BASE GICH_BASE
+#define PLAT_GICV_BASE GICV_BASE
+
+#define PLAT_CPU_COUNT 4
+#define PLAT_GTBLOCK_COUNT 0
+#define PLAT_GTFRAME_COUNT 0
+#define PLAT_PCI_CONFG_COUNT 2
+
+#define PLAT_WATCHDOG_COUNT 0
+#define PLAT_GIC_REDISTRIBUTOR_COUNT 0
+#define PLAT_GIC_ITS_COUNT 0
+
+/* GIC CPU Interface information
+ GIC_ENTRY (CPUInterfaceNumber, Mpidr, PmuIrq, VGicIrq, EnergyEfficiency)
+ */
+#define PLAT_GIC_CPU_INTERFACE { \
+ GICC_ENTRY (0, GET_MPID (0, 0), 138, 0x19, 0), \
+ GICC_ENTRY (1, GET_MPID (0, 1), 139, 0x19, 0), \
+ GICC_ENTRY (2, GET_MPID (0, 2), 127, 0x19, 0), \
+ GICC_ENTRY (3, GET_MPID (0, 3), 129, 0x19, 0), \
+}
+
+#define PLAT_WATCHDOG_INFO \
+ { \
+ } \
+
+#define PLAT_TIMER_BLOCK_INFO \
+ { \
+ } \
+
+#define PLAT_TIMER_FRAME_INFO \
+ { \
+ } \
+
+#define PLAT_GIC_DISTRIBUTOR_INFO \
+ { \
+ PLAT_GICD_BASE, /* UINT64 PhysicalBaseAddress */ \
+ 0, /* UINT32 SystemVectorBase */ \
+ PLAT_GIC_VERSION /* UINT8 GicVersion */ \
+ } \
+
+#define PLAT_GIC_REDISTRIBUTOR_INFO \
+ { \
+ } \
+
+#define PLAT_GIC_ITS_INFO \
+ { \
+ } \
+
+#define PLAT_MCFG_INFO \
+ { \
+ { \
+ PLAT_PCI_SEG1_CONFIG_BASE, \
+ PLAT_PCI_SEG1, \
+ PLAT_PCI_SEG_BUSNUM_MIN, \
+ PLAT_PCI_SEG_BUSNUM_MAX, \
+ }, \
+ { \
+ PLAT_PCI_SEG2_CONFIG_BASE, \
+ PLAT_PCI_SEG2, \
+ PLAT_PCI_SEG_BUSNUM_MIN, \
+ PLAT_PCI_SEG_BUSNUM_MAX, \
+ } \
+ } \
+
+#define PLAT_SPCR_INFO \
+ { \
+ UART0_BASE, \
+ UART0_IT, \
+ 115200, \
+ 0, \
+ EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_16550 \
+ } \
+
+#endif // LS1046AFRWY_PLATFORM_H
diff --git a/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc b/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc
index 67cf15cbe4..20111e6037 100755
--- a/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc
+++ b/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc
@@ -3,6 +3,7 @@
# LS1046AFRWY Board package.
#
# Copyright 2019-2020 NXP
+# Copyright 2021 Puresoftware Ltd
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -22,10 +23,18 @@
OUTPUT_DIRECTORY = Build/LS1046aFrwyPkg
FLASH_DEFINITION = Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf

+ # This flag controls the dynamic acpi generation
+ #
+ DEFINE DYNAMIC_ACPI_ENABLE = TRUE
+
!include Silicon/NXP/NxpQoriqLs.dsc.inc
!include MdePkg/MdeLibs.dsc.inc
!include Silicon/NXP/LS1046A/LS1046A.dsc.inc

+!if $(DYNAMIC_ACPI_ENABLE) == TRUE
+ !include DynamicTablesPkg/DynamicTables.dsc.inc
+!endif
+
[LibraryClasses.common]
ArmPlatformLib|Platform/NXP/LS1046aFrwyPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
RealTimeClockLib|EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.inf
@@ -46,4 +55,23 @@

Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf

+ #
+ # Dynamic Table Factory
+ !if $(DYNAMIC_ACPI_ENABLE) == TRUE
+ DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf {
+ <LibraryClasses>
+ NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/AcpiFadtLibArm.inf
+ NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/AcpiGtdtLibArm.inf
+ NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/AcpiMadtLibArm.inf
+ NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLibArm.inf
+ NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.inf
+ }
+ !endif
+
+ #
+ # Acpi Support
+ #
+ MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf
+ MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
+
##
diff --git a/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf b/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf
index 34c4e5a025..f3cac033bc 100755
--- a/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf
+++ b/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf
@@ -3,6 +3,7 @@
# FLASH layout file for LS1046a board.
#
# Copyright 2019-2020 NXP
+# Copyright 2021 Puresoftware Ltd
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -99,6 +100,18 @@ READ_LOCK_STATUS = TRUE
INF MdeModulePkg/Universal/Metronome/Metronome.inf
INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf

+
+ #
+ # Acpi Support
+ #
+ INF MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf
+ INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
+
+ !if $(DYNAMIC_ACPI_ENABLE) == TRUE
+ INF Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
+ !include DynamicTablesPkg/DynamicTables.fdf.inc
+ !endif
+
#
# Multiple Console IO support
#
diff --git a/Silicon/NXP/LS1046A/LS1046A.dsc.inc b/Silicon/NXP/LS1046A/LS1046A.dsc.inc
index 7004533ed5..98f999edfd 100644
--- a/Silicon/NXP/LS1046A/LS1046A.dsc.inc
+++ b/Silicon/NXP/LS1046A/LS1046A.dsc.inc
@@ -2,6 +2,7 @@
# LS1046A Soc package.
#
# Copyright 2017-2020 NXP
+# Copyright 2021 Puresoftware Ltd
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -48,4 +49,13 @@
[Components.common]
MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf

+#
+# Configuration Manager
+!if $(DYNAMIC_ACPI_ENABLE) == TRUE
+ Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManagerDxe.inf {
+ <BuildOptions>
+ *_*_*_PLATFORM_FLAGS = -I$(WORKSPACE)/Platform/NXP/LS1046aFrwyPkg/Include
+ }
+!endif
+
##
--
2.25.1


Re: [PATCH V1 2/4] Silicon/NXP: Add support of SVR handling for LS1046FRWY

Vikas Singh
 

On Tue, Jun 15, 2021 at 2:29 AM Leif Lindholm <leif@nuviainc.com> wrote:

On Fri, Jun 11, 2021 at 21:21:58 +0530, Vikas Singh wrote:
This change set intend to add a generic method to get
Does it intend to add, or does it add?

/
Leif
Leif, this patch adds a generic method to get the SVR details for LS1046A SoC.
I will do the suggested changes in subject and body of the commit.

I will share the updated V2 series shortly.

access to SoC's Silicon Version Register (SVR) and its
handling for LS1046aFrwy platform.

Signed-off-by: Vikas Singh <vikas.singh@puresoftware.com>
---
Silicon/NXP/LS1046A/Library/SocLib/SocLib.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/Silicon/NXP/LS1046A/Library/SocLib/SocLib.c b/Silicon/NXP/LS1046A/Library/SocLib/SocLib.c
index 8fa6a7dd00..003f5bd82f 100644
--- a/Silicon/NXP/LS1046A/Library/SocLib/SocLib.c
+++ b/Silicon/NXP/LS1046A/Library/SocLib/SocLib.c
@@ -2,6 +2,7 @@
SoC specific Library containg functions to initialize various SoC components

Copyright 2017-2020 NXP
+ Copyright 2021 Puresoftware Ltd

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

@@ -64,6 +65,21 @@ SocGetClock (
return ReturnValue;
}

+/**
+ Function to get SoC's System Version Register(SVR)
+ **/
+UINT32
+SocGetSvr (
+ VOID
+ )
+{
+ LS1046A_DEVICE_CONFIG *Dcfg;
+
+ Dcfg = (LS1046A_DEVICE_CONFIG *)LS1046A_DCFG_ADDRESS;
+
+ return DcfgRead32 ((UINTN)&Dcfg->Svr);
+}
+
/**
Function to select pins depending upon pcd using supplemental
configuration unit(SCFG) extended RCW controlled pinmux control
--
2.25.1


Re: [PATCH V1 1/4] Platform/NXP: Add generic log in CM to print SoC version

Vikas Singh
 

On Tue, Jun 15, 2021 at 2:28 AM Leif Lindholm <leif@nuviainc.com> wrote:

On Fri, Jun 11, 2021 at 21:21:57 +0530, Vikas Singh wrote:
Summary -
1.Configuration Manager(CM) is a common implementation
and should not evaluate the SoC version using macro's
However CM must directly consume SoC ver string from
platfrom who is extending CM services for ACPI table
generation.
This tells me nothing about what this patch does.

2.Platforms who extends CM services for themselves must
notify their SoC details to CM.
Neither does this.

3.This patch will update the lx2160ardb platform header
also with PLAT_SOC_NAME, this will be consumed by CM.
And this sound like it should be a separate patch.

*However* when I look at the code, this does look like a single
change. And what is descibed as point 3 is the actual change in the patch.
Moreover, this patch addresses a historic horror, in that SVR_LX2160A
was defined both in the platform header and in the SoC header (which
I'm not saying you had necessarily noticed, but I am suggesting it is
added to the message).

If I wrote this commit message, I would start with a slightly tweaked
subject line:

Platform/NXP: Make SoC version log in ConfigurationManager generic

(CM is not a recognised abbreviation, so should be printed expanded)

As for the body, I would say something like:

This patch replaces the logic in ConfigurationManager to print
platform name based on platform ID with a simple #define.
This also removes a duplication of the SVR_LX2160A definition between
SoC and platform headers.

No comments on the code itself.

/
Leif
Leif, I will do the suggested changes in subject and body of the commit.
Additionally, I will remove all the SVR_* duplicates from the platform headers.
Will try to reuse the SVR_* definitions from the SoC headers itself.

I will share the updated V2 series shortly.

Signed-off-by: Vikas Singh <vikas.singh@puresoftware.com>
---
Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c | 10 +++-------
Platform/NXP/LX2160aRdbPkg/Include/Platform.h | 5 ++---
2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c
index 80ce8412c4..dc1a7f5f85 100644
--- a/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c
+++ b/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c
@@ -2,7 +2,7 @@
Configuration Manager Dxe

Copyright 2020 NXP
- Copyright 2020 Puresoftware Ltd
+ Copyright 2020-2021 Puresoftware Ltd

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

@@ -170,12 +170,8 @@ InitializePlatformRepository (
PlatformRepo = This->PlatRepoInfo;

Svr = SocGetSvr ();
- if (SVR_SOC_VER(Svr) == SVR_LX2160A) {
- PlatformRepo->FslBoardRevision = SVR_MAJOR(Svr);
- DEBUG ((DEBUG_INFO, "Fsl : SoC LX2160A Rev = 0x%x\n", PlatformRepo->FslBoardRevision));
- } else {
- DEBUG ((DEBUG_INFO, "Fsl : SoC Unknown Rev = 0x%x\n", PlatformRepo->FslBoardRevision));
- }
+ PlatformRepo->FslBoardRevision = SVR_MAJOR(Svr);
+ DEBUG ((DEBUG_INFO, "Fsl : SoC = %s Rev = 0x%x\n", PLAT_SOC_NAME, PlatformRepo->FslBoardRevision));

return EFI_SUCCESS;
}
diff --git a/Platform/NXP/LX2160aRdbPkg/Include/Platform.h b/Platform/NXP/LX2160aRdbPkg/Include/Platform.h
index 76a41d4369..c18faf28cd 100644
--- a/Platform/NXP/LX2160aRdbPkg/Include/Platform.h
+++ b/Platform/NXP/LX2160aRdbPkg/Include/Platform.h
@@ -2,7 +2,7 @@
* Platform headers
*
* Copyright 2020 NXP
- * Copyright 2020 Puresoftware Ltd
+ * Copyright 2020-2021 Puresoftware Ltd
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*
@@ -15,12 +15,11 @@
#define EFI_ACPI_ARM_OEM_REVISION 0x00000000

// Soc defines
+#define PLAT_SOC_NAME "LX2160ARDB"
#define SVR_SOC_VER(svr) (((svr) >> 8) & 0xFFFFFE)
#define SVR_MAJOR(svr) (((svr) >> 4) & 0xf)
#define SVR_MINOR(svr) (((svr) >> 0) & 0xf)

-#define SVR_LX2160A 0x873600
-
// PCLK
#define DCFG_BASE 0x1E00000
#define DCFG_LEN 0x1FFFF
--
2.25.1


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

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 39/41] KabylakeSiliconPkg: Identify flash
regions by GUID

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

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

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/SpiC
ommon.c | 140 ++++++++++++++++----
Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/HstiSiliconDxe.inf
| 9 ++

Silicon/Intel/KabylakeSiliconPkg/Pch/IncludePrivate/Library/PchSpiCommonLib.
h | 20 +--

Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommonLib/Bas
ePchSpiCommonLib.inf | 11 ++
5 files changed, 247 insertions(+), 39 deletions(-)

diff --git
a/Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/SecureMemoryMapConfiguration.c
b/Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/SecureMemoryMapConfiguration.c
index a3c9bbebeaa9..ccf63b216f70 100644
---
a/Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/SecureMemoryMapConfiguration.c
+++ b/Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/SecureMemoryMapConfigura
+++ tion.c
@@ -2,11 +2,14 @@
This file contains the tests for the SecureMemoryMapConfiguration bit

Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) Microsoft Corporation.<BR>
+
SPDX-License-Identifier: BSD-2-Clause-Patent

**/

#include "HstiSiliconDxe.h"
+#include <Guid/FlashRegion.h>

typedef struct {
UINT64 Base;
@@ -100,6 +103,90 @@ MEMORY_RANGE
mNonLockableMemoryRange[NonLockableMemoryRangeMax] = { // 14.
SPI_BAR0 (BDF 0:31:5 + 0x10) };

+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[] = {
+ {
+ &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 to 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 found for
the given flash region GUID.
+
+**/
+EFI_STATUS
+GetFlashRegionType (
+ IN EFI_GUID *FlashRegionGuid,
+ OUT FLASH_REGION_TYPE *FlashRegionType
+ )
+{
+ UINTN Index;
+
+ if (FlashRegionGuid == NULL || FlashRegionType == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ for (Index = 0; Index < ARRAY_SIZE (mFlashRegionTypes); Index++) {
+ if (CompareGuid (mFlashRegionTypes[Index].Guid, FlashRegionGuid)) {
+ *FlashRegionType = mFlashRegionTypes[Index].Type;
+ return EFI_SUCCESS;
+ }
+ }
+
+ return EFI_NOT_FOUND;
+}
+
/**
Check for overlaps in single range array

@@ -224,7 +311,7 @@ AcquireSpiBar0 (
{
UINT32 SpiBar0;
UINTN PchSpiBase;
-
+
//
// Init PCH spi reserved MMIO address.
//
@@ -270,7 +357,7 @@ ReleaseSpiBar0 (
Get the SPI region base and size, based on the enum type

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @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 address
which corresponds to the type in the descriptor.
@param[out] BaseAddress The Flash Linear Address for the Region 'n'
Base
@param[out] RegionSize The size for the Region 'n'

@@ -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 = GetFlashRegionType (FlashRegionGuid, &FlashRegionType); if
+ (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
+ }

if (FlashRegionType >= 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 = SIZE_4GB
- RegionSize;
mLockableMemoryRange[LockableMemoryRangeLowDram].End =
(MmioRead32 (MmPciBase (0,SA_MC_DEV,SA_MC_FUN) + R_SA_TOLUD) &
B_SA_TOLUD_TOLUD_MASK) - 1; diff --git
a/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommonLib/S
piCommon.c
b/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommonLib/S
piCommon.c
index 58757a8cba39..d2eb8324bf58 100644
---
a/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommonLib/S
piCommon.c
+++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiComm
+++ onLib/SpiCommon.c
@@ -2,10 +2,13 @@
PCH SPI Common Driver implements the SPI Host Controller Compatibility
Interface.

Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) Microsoft Corporation.<BR>
+
SPDX-License-Identifier: BSD-2-Clause-Patent

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

+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[] = {
+ {
+ &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 to 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 found for
the given flash region GUID.
+
+**/
+EFI_STATUS
+GetFlashRegionType (
+ IN EFI_GUID *FlashRegionGuid,
+ OUT FLASH_REGION_TYPE *FlashRegionType
+ )
+{
+ UINTN Index;
+
+ if (FlashRegionGuid == NULL || FlashRegionType == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ for (Index = 0; Index < ARRAY_SIZE (mFlashRegionTypes); Index++) {
+ if (CompareGuid (mFlashRegionTypes[Index].Guid, FlashRegionGuid)) {
+ *FlashRegionType = mFlashRegionTypes[Index].Type;
+ return EFI_SUCCESS;
+ }
+ }
+
+ return EFI_NOT_FOUND;
+}
+
/**
Initialize an SPI protocol instance.

@@ -249,7 +336,7 @@ PchPmTimerStallRuntimeSafe (
Read data from the flash part.

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @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 within 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 = SendSpiCmd (
This,
- FlashRegionType,
+ FlashRegionGuid,
FlashCycleRead,
Address,
ByteCount,
@@ -289,7 +376,7 @@ SpiProtocolFlashRead (
Write data to the flash part.

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @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 within 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 containing 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 = SendSpiCmd (
This,
- FlashRegionType,
+ FlashRegionGuid,
FlashCycleWrite,
Address,
ByteCount,
@@ -328,7 +415,7 @@ SpiProtocolFlashWrite (
Erase some area on the flash part.

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @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 within a region
for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of the SPI
cycle.

@@ -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 = SendSpiCmd (
This,
- FlashRegionType,
+ FlashRegionGuid,
FlashCycleErase,
Address,
ByteCount,
@@ -407,7 +494,7 @@ SpiProtocolFlashReadSfdp (
//
Status = SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleReadSfdp,
FlashAddress,
ByteCount,
@@ -460,7 +547,7 @@ SpiProtocolFlashReadJedecId (
//
Status = SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleReadJedecId,
Address,
ByteCount,
@@ -495,7 +582,7 @@ SpiProtocolFlashWriteStatus (
//
Status = SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleWriteStatus,
0,
ByteCount,
@@ -530,7 +617,7 @@ SpiProtocolFlashReadStatus (
//
Status = SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleReadStatus,
0,
ByteCount,
@@ -543,7 +630,7 @@ SpiProtocolFlashReadStatus (
Get the SPI region base and size, based on the enum type

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @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 address
which corresponds to the type in the descriptor.
@param[out] BaseAddress The Flash Linear Address for the Region 'n'
Base
@param[out] RegionSize The size for the Region 'n'

@@ -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;

SpiInstance = SPI_INSTANCE_FROM_SPIPROTOCOL (This);

+ Status = GetFlashRegionType (FlashRegionGuid, &FlashRegionType); if
+ (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
if (FlashRegionType >= FlashRegionMax) {
return EFI_INVALID_PARAMETER;
}
@@ -646,7 +740,7 @@ SpiProtocolReadPchSoftStrap (
//
Status = SendSpiCmd (
This,
- FlashRegionDescriptor,
+ &gFlashRegionDescriptorGuid,
FlashCycleRead,
StrapFlashAddr,
ByteCount,
@@ -704,7 +798,7 @@ SpiProtocolReadCpuSoftStrap (
//
Status = SendSpiCmd (
This,
- FlashRegionDescriptor,
+ &gFlashRegionDescriptorGuid,
FlashCycleRead,
StrapFlashAddr,
ByteCount,
@@ -717,7 +811,7 @@ SpiProtocolReadCpuSoftStrap (
This function sends the programmed SPI command to the slave device.

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] SpiRegionType The SPI 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] FlashCycleType The Flash SPI cycle type list in HSFC (Hardware
Sequencing Flash Control Register) register
@param[in] Address The Flash Linear Address must fall within 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;
}

- Status = SpiProtocolGetRegionAddress (This, FlashRegionType,
&HardwareSpiAddr, &FlashRegionSize);
+ Status = SpiProtocolGetRegionAddress (This, FlashRegionGuid,
+ &HardwareSpiAddr, &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

[Protocols]
gEfiDxeSmmReadyToLockProtocolGuid ## CONSUMES diff --git
a/Silicon/Intel/KabylakeSiliconPkg/Pch/IncludePrivate/Library/PchSpiCommonLi
b.h
b/Silicon/Intel/KabylakeSiliconPkg/Pch/IncludePrivate/Library/PchSpiCommonLi
b.h
index d408289ea253..fd991de96016 100644
---
a/Silicon/Intel/KabylakeSiliconPkg/Pch/IncludePrivate/Library/PchSpiCommonLi
b.h
+++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/IncludePrivate/Library/PchSpi
+++ CommonLib.h
@@ -134,7 +134,7 @@ ReleaseSpiBar0 (
Read data from the flash part.

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @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 within 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.

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @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 within 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 containing 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.

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @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 within a region
for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of the SPI
cycle.

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

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @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 Region 'n'
Base
@param[out] RegionSize The size for the Region 'n'

@@ -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.

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] SpiRegionType The SPI 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] FlashCycleType The Flash SPI cycle type list in HSFC (Hardware
Sequencing Flash Control Register) register
@param[in] Address The Flash Linear Address must fall within 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/BasePchSpiCommonLib/B
asePchSpiCommonLib.inf
b/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommonLib/B
asePchSpiCommonLib.inf
index 51e2d25a7f8b..67176c879de5 100644
---
a/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommonLib/B
asePchSpiCommonLib.inf
+++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiComm
+++ onLib/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
--
2.28.0.windows.1


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

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 38/41]
CoffeelakeSiliconPkg/BasePchSpiCommonLib: Identify flash regions by GUID

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

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

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/BasePchSpiCommonLib/S
piCommon.c | 144 ++++++++++++++++----

Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Private/Library/PchSpiCommonLi
b.h | 16 +--

Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCommonLib/
BasePchSpiCommonLib.inf | 12 ++
3 files changed, 141 insertions(+), 31 deletions(-)

diff --git
a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCommonLib
/SpiCommon.c
b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCommonLib
/SpiCommon.c
index bc84a4f27f1a..26a3d0e7db31 100644
---
a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCommonLib
/SpiCommon.c
+++ b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiC
+++ ommonLib/SpiCommon.c
@@ -2,11 +2,13 @@
PCH SPI Common Driver implements the SPI Host Controller Compatibility
Interface.

Copyright (c) 2019 Intel Corporation. All rights reserved. <BR>
+ Copyright (c) Microsoft Corporation.<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent **/

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

+typedef enum {
+ FlashRegionDescriptor,
+ FlashRegionBios,
+ FlashRegionMe,
+ FlashRegionGbe,
+ FlashRegionPlatformData,
+ FlashRegionDer,
+ FlashRegionEc = 8,
+ FlashRegionAll,
+ FlashRegionMax
+} FLASH_REGION_TYPE;
+
+typedef struct {
+ EFI_GUID *Guid;
+ FLASH_REGION_TYPE Type;
+} FLASH_REGION_MAPPING;
+
+FLASH_REGION_MAPPING mFlashRegionTypes[] = {
+ {
+ &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 to 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 found for
the given flash region GUID.
+
+**/
+EFI_STATUS
+GetFlashRegionType (
+ IN EFI_GUID *FlashRegionGuid,
+ OUT FLASH_REGION_TYPE *FlashRegionType
+ )
+{
+ UINTN Index;
+
+ if (FlashRegionGuid == NULL || FlashRegionType == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ for (Index = 0; Index < ARRAY_SIZE (mFlashRegionTypes); Index++) {
+ if (CompareGuid (mFlashRegionTypes[Index].Guid, FlashRegionGuid)) {
+ *FlashRegionType = mFlashRegionTypes[Index].Type;
+ return EFI_SUCCESS;
+ }
+ }
+
+ return EFI_NOT_FOUND;
+}
+
/**
Initialize an SPI protocol instance.

@@ -303,7 +394,7 @@ WaitForSpiCycleComplete (
This function sends the programmed SPI command to the slave device.

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] SpiRegionType The SPI 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] FlashCycleType The Flash SPI cycle type list in HSFC (Hardware
Sequencing Flash Control Register) register
@param[in] Address The Flash Linear Address must fall within 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 (
}
}

- Status = SpiProtocolGetRegionAddress (This, FlashRegionType,
&HardwareSpiAddr, &FlashRegionSize);
+ Status = SpiProtocolGetRegionAddress (This, FlashRegionGuid,
+ &HardwareSpiAddr, &FlashRegionSize);
if (EFI_ERROR (Status)) {
goto SendSpiCmdEnd;
}
@@ -616,7 +707,7 @@ SendSpiCmd (
Read data from the flash part.

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @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 within 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 = SendSpiCmd (
This,
- FlashRegionType,
+ FlashRegionGuid,
FlashCycleRead,
Address,
ByteCount,
@@ -656,7 +747,7 @@ SpiProtocolFlashRead (
Write data to the flash part.

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @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 within 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 containing 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 = SendSpiCmd (
This,
- FlashRegionType,
+ FlashRegionGuid,
FlashCycleWrite,
Address,
ByteCount,
@@ -695,7 +786,7 @@ SpiProtocolFlashWrite (
Erase some area on the flash part.

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @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 within a region
for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of the SPI
cycle.

@@ -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 = SendSpiCmd (
This,
- FlashRegionType,
+ FlashRegionGuid,
FlashCycleErase,
Address,
ByteCount,
@@ -774,7 +865,7 @@ SpiProtocolFlashReadSfdp (
//
Status = SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleReadSfdp,
FlashAddress,
ByteCount,
@@ -827,7 +918,7 @@ SpiProtocolFlashReadJedecId (
//
Status = SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleReadJedecId,
Address,
ByteCount,
@@ -862,7 +953,7 @@ SpiProtocolFlashWriteStatus (
//
Status = SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleWriteStatus,
0,
ByteCount,
@@ -897,7 +988,7 @@ SpiProtocolFlashReadStatus (
//
Status = SendSpiCmd (
This,
- FlashRegionAll,
+ &gFlashRegionAllGuid,
FlashCycleReadStatus,
0,
ByteCount,
@@ -910,7 +1001,7 @@ SpiProtocolFlashReadStatus (
Get the SPI region base and size, based on the enum type

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @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 address
which corresponds to the type in the descriptor.
@param[out] BaseAddress The Flash Linear Address for the Region 'n'
Base
@param[out] RegionSize The size for the Region 'n'

@@ -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;

SpiInstance = SPI_INSTANCE_FROM_SPIPROTOCOL (This);

+ Status = GetFlashRegionType (FlashRegionGuid, &FlashRegionType); if
+ (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
if (FlashRegionType >= FlashRegionMax) {
return EFI_INVALID_PARAMETER;
}
@@ -1013,7 +1111,7 @@ SpiProtocolReadPchSoftStrap (
//
Status = SendSpiCmd (
This,
- FlashRegionDescriptor,
+ &gFlashRegionDescriptorGuid,
FlashCycleRead,
StrapFlashAddr,
ByteCount,
@@ -1071,7 +1169,7 @@ SpiProtocolReadCpuSoftStrap (
//
Status = SendSpiCmd (
This,
- FlashRegionDescriptor,
+ &gFlashRegionDescriptorGuid,
FlashCycleRead,
StrapFlashAddr,
ByteCount,
diff --git
a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Private/Library/PchSpiCommo
nLib.h
b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Private/Library/PchSpiCommo
nLib.h
index 0a973a77a381..e69e2f1e456c 100644
---
a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Private/Library/PchSpiCommo
nLib.h
+++ b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Private/Library/Pch
+++ SpiCommonLib.h
@@ -148,7 +148,7 @@ IsSpiFlashWriteGranted (
Read data from the flash part.

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @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 within 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.

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @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 within 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 containing 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.

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @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 within a region
for which BIOS has access permissions.
@param[in] ByteCount Number of bytes in the data portion of the SPI
cycle.

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

@param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @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 address
which corresponds to the type in the descriptor.
@param[out] BaseAddress The Flash Linear Address for the Region 'n'
Base
@param[out] RegionSize The size for the Region 'n'

@@ -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/BasePchSpiCommonLib
/BasePchSpiCommonLib.inf
b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCommonLib
/BasePchSpiCommonLib.inf
index f5dc4ee0bfef..b152d2278839 100644
---
a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCommonLib
/BasePchSpiCommonLib.inf
+++ b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiC
+++ ommonLib/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
--
2.28.0.windows.1


Re: [edk2-platforms][PATCH v3 33/41] KabylakeSiliconPkg: Remove PCH SPI PPI and Protocol from package

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 33/41] KabylakeSiliconPkg: Remove PCH SPI
PPI and Protocol from package

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

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

The following PPI and Protocols have moved to IntelSiliconPkg. The remaining
definitions in KabylakeSiliconPkg are removed and libs modules that need to
reference IntelSiliconPkg are updated.

1. gPchSpiProtocolGuid
2. gPchSmmSpiProtocolGuid
3. gPchSpiPpiGuid

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: Chasel Chiu <chasel.chiu@intel.com>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
---
Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/HstiSiliconDxe.inf | 3 +-
Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Ppi/Spi.h | 26 --
Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Protocol/Spi.h | 293 ----------
----------
Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.inf | 1 +
Silicon/Intel/KabylakeSiliconPkg/Pch/Spi/Smm/PchSpiSmm.inf | 1 +
Silicon/Intel/KabylakeSiliconPkg/SiPkg.dec | 3 -
6 files changed, 4 insertions(+), 323 deletions(-)

diff --git a/Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/HstiSiliconDxe.inf
b/Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/HstiSiliconDxe.inf
index 52e3b6ceba3e..bd12fa691d40 100644
--- a/Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/HstiSiliconDxe.inf
+++ b/Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/HstiSiliconDxe.inf
@@ -46,6 +46,7 @@ [Sources]
[Packages]
MdePkg/MdePkg.dec
UefiCpuPkg/UefiCpuPkg.dec
+ IntelSiliconPkg/IntelSiliconPkg.dec
KabylakeSiliconPkg/SiPkg.dec
SecurityPkg/SecurityPkg.dec

@@ -92,7 +93,7 @@ [Protocols]
gEfiMpServiceProtocolGuid ## CONSUMES
gDxeSiPolicyProtocolGuid ## CONSUMES
gHstiPublishCompleteProtocolGuid ## PRODUCES
-
+
[FixedPcd]
gSiPkgTokenSpaceGuid.PcdHstiIhvFeature1
gSiPkgTokenSpaceGuid.PcdHstiIhvFeature2
diff --git a/Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Ppi/Spi.h
b/Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Ppi/Spi.h
deleted file mode 100644
index e11f82edcaea..000000000000
--- a/Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Ppi/Spi.h
+++ /dev/null
@@ -1,26 +0,0 @@
-/** @file
- This file defines the PCH SPI PPI which implements the
- Intel(R) PCH SPI Host Controller Compatibility Interface.
-
-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
-SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-#ifndef _PCH_SPI_PPI_H_
-#define _PCH_SPI_PPI_H_
-
-#include <Protocol/Spi.h>
-
-//
-// Extern the GUID for PPI users.
-//
-extern EFI_GUID gPchSpiPpiGuid;
-
-/**
- Reuse the PCH_SPI_PROTOCOL definitions
- This is possible becaues the PPI implementation does not rely on a PeiService
pointer,
- as it uses EDKII Glue Lib to do IO accesses -**/ -typedef PCH_SPI_PROTOCOL
PCH_SPI_PPI;
-
-#endif
diff --git a/Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Protocol/Spi.h
b/Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Protocol/Spi.h
deleted file mode 100644
index 8c66e5063fa9..000000000000
--- a/Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Protocol/Spi.h
+++ /dev/null
@@ -1,293 +0,0 @@
-/** @file
- This file defines the PCH SPI Protocol which implements the
- Intel(R) PCH SPI Host Controller Compatibility Interface.
-
-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
-SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-#ifndef _PCH_SPI_PROTOCOL_H_
-#define _PCH_SPI_PROTOCOL_H_
-
-//
-// Extern the GUID for protocol users.
-//
-extern EFI_GUID gPchSpiProtocolGuid;
-extern EFI_GUID gPchSmmSpiProtocolGuid;
-
-//
-// Forward reference for ANSI C compatibility -// -typedef struct
_PCH_SPI_PROTOCOL PCH_SPI_PROTOCOL;
-
-//
-// SPI protocol data structures and definitions -//
-
-/**
- Flash Region Type
-**/
-typedef enum {
- FlashRegionDescriptor,
- FlashRegionBios,
- FlashRegionMe,
- FlashRegionGbE,
- FlashRegionPlatformData,
- FlashRegionDer,
- FlashRegionAll,
- FlashRegionMax
-} FLASH_REGION_TYPE;
-
-//
-// Protocol member functions
-//
-
-/**
- Read data from the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] FlashRegionType The Flash Region type for flash cycle which is
listed in the Descriptor.
- @param[in] Address The Flash Linear Address must fall within 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.
- It is the caller's responsibility to make sure Buffer is large
enough for the total number of bytes read.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormally.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_READ) (
- IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
- IN UINT32 Address,
- IN UINT32 ByteCount,
- OUT UINT8 *Buffer
- );
-
-/**
- Write data to the flash part. Remark: Erase may be needed before write to the
flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] FlashRegionType The Flash Region type for flash cycle which is
listed in the Descriptor.
- @param[in] Address The Flash Linear Address must fall within 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 containing the
data sent during the SPI cycle.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormally.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_WRITE) (
- IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
- IN UINT32 Address,
- IN UINT32 ByteCount,
- IN UINT8 *Buffer
- );
-
-/**
- Erase some area on the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] FlashRegionType The Flash Region type for flash cycle which is
listed in the Descriptor.
- @param[in] Address The Flash Linear Address must fall within a region
for which BIOS has access permissions.
- @param[in] ByteCount Number of bytes in the data portion of the SPI
cycle.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormally.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_ERASE) (
- IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
- IN UINT32 Address,
- IN UINT32 ByteCount
- );
-
-/**
- Read SFDP data from the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] ComponentNumber The Componen Number for chip select
- @param[in] Address The starting byte address for SFDP data read.
- @param[in] ByteCount Number of bytes in SFDP data portion of the SPI
cycle
- @param[out] SfdpData The Pointer to caller-allocated buffer containing
the SFDP data received
- It is the caller's responsibility to make sure Buffer is large
enough for the total number of bytes read
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormally.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_READ_SFDP) (
- IN PCH_SPI_PROTOCOL *This,
- IN UINT8 ComponentNumber,
- IN UINT32 Address,
- IN UINT32 ByteCount,
- OUT UINT8 *SfdpData
- );
-
-/**
- Read Jedec Id from the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] ComponentNumber The Componen Number for chip select
- @param[in] ByteCount Number of bytes in JedecId data portion of the
SPI cycle, the data size is 3 typically
- @param[out] JedecId The Pointer to caller-allocated buffer containing
JEDEC ID received
- It is the caller's responsibility to make sure Buffer is large
enough for the total number of bytes read.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormally.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_READ_JEDEC_ID) (
- IN PCH_SPI_PROTOCOL *This,
- IN UINT8 ComponentNumber,
- IN UINT32 ByteCount,
- OUT UINT8 *JedecId
- );
-
-/**
- Write the status register in the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] ByteCount Number of bytes in Status data portion of the SPI
cycle, the data size is 1 typically
- @param[in] StatusValue The Pointer to caller-allocated buffer containing
the value of Status register writing
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormally.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_WRITE_STATUS) (
- IN PCH_SPI_PROTOCOL *This,
- IN UINT32 ByteCount,
- IN UINT8 *StatusValue
- );
-
-/**
- Read status register in the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] ByteCount Number of bytes in Status data portion of the SPI
cycle, the data size is 1 typically
- @param[out] StatusValue The Pointer to caller-allocated buffer
containing the value of Status register received.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormally.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_READ_STATUS) (
- IN PCH_SPI_PROTOCOL *This,
- IN UINT32 ByteCount,
- OUT UINT8 *StatusValue
- );
-
-/**
- Get the SPI region base and size, based on the enum type
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] FlashRegionType The Flash Region type for for the base
address which is listed in the Descriptor.
- @param[out] BaseAddress The Flash Linear Address for the Region 'n'
Base
- @param[out] RegionSize The size for the Region 'n'
-
- @retval EFI_SUCCESS Read success
- @retval EFI_INVALID_PARAMETER Invalid region type given
- @retval EFI_DEVICE_ERROR The region is not used
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_GET_REGION_ADDRESS) (
- IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
- OUT UINT32 *BaseAddress,
- OUT UINT32 *RegionSize
- );
-
-/**
- Read PCH Soft Strap Values
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] SoftStrapAddr PCH Soft Strap address offset from FPSBA.
- @param[in] ByteCount Number of bytes in SoftStrap data portion of the
SPI cycle
- @param[out] SoftStrapValue The Pointer to caller-allocated buffer
containing PCH Soft Strap Value.
- If the value of ByteCount is 0, the data type of
SoftStrapValue should be UINT16 and SoftStrapValue will be PCH Soft Strap
Length
- It is the caller's responsibility to make sure Buffer is large
enough for the total number of bytes read.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormally.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_READ_PCH_SOFTSTRAP) (
- IN PCH_SPI_PROTOCOL *This,
- IN UINT32 SoftStrapAddr,
- IN UINT32 ByteCount,
- OUT VOID *SoftStrapValue
- );
-
-/**
- Read CPU Soft Strap Values
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] SoftStrapAddr CPU Soft Strap address offset from FCPUSBA.
- @param[in] ByteCount Number of bytes in SoftStrap data portion of the
SPI cycle.
- @param[out] SoftStrapValue The Pointer to caller-allocated buffer
containing CPU Soft Strap Value.
- If the value of ByteCount is 0, the data type of
SoftStrapValue should be UINT16 and SoftStrapValue will be PCH Soft Strap
Length
- It is the caller's responsibility to make sure Buffer is large
enough for the total number of bytes read.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormally.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_READ_CPU_SOFTSTRAP) (
- IN PCH_SPI_PROTOCOL *This,
- IN UINT32 SoftStrapAddr,
- IN UINT32 ByteCount,
- OUT VOID *SoftStrapValue
- );
-
-/**
- These protocols/PPI allows a platform module to perform SPI operations
through the
- Intel PCH SPI Host Controller Interface.
-**/
-struct _PCH_SPI_PROTOCOL {
- /**
- This member specifies the revision of this structure. This field is used to
- indicate backwards compatible changes to the protocol.
- **/
- UINT8 Revision;
- PCH_SPI_FLASH_READ FlashRead; ///< Read data from the flash
part.
- PCH_SPI_FLASH_WRITE FlashWrite; ///< Write data to the flash
part. Remark: Erase may be needed before write to the flash part.
- PCH_SPI_FLASH_ERASE FlashErase; ///< Erase some area on the
flash part.
- PCH_SPI_FLASH_READ_SFDP FlashReadSfdp; ///< Read SFDP data
from the flash part.
- PCH_SPI_FLASH_READ_JEDEC_ID FlashReadJedecId; ///< Read Jedec Id
from the flash part.
- PCH_SPI_FLASH_WRITE_STATUS FlashWriteStatus; ///< Write the status
register in the flash part.
- PCH_SPI_FLASH_READ_STATUS FlashReadStatus; ///< Read status
register in the flash part.
- PCH_SPI_GET_REGION_ADDRESS GetRegionAddress; ///< Get the SPI
region base and size
- PCH_SPI_READ_PCH_SOFTSTRAP ReadPchSoftStrap; ///< Read PCH Soft
Strap Values
- PCH_SPI_READ_CPU_SOFTSTRAP ReadCpuSoftStrap; ///< Read CPU Soft
Strap Values
-};
-
-/**
- PCH SPI PPI/PROTOCOL revision number
-
- Revision 1: Initial version
-**/
-#define PCH_SPI_SERVICES_REVISION 1
-
-#endif
diff --git a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.inf
b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.inf
index 31f4ffe43a23..c6bc1ad406c8 100644
--- a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.inf
+++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.i
+++ nf
@@ -32,6 +32,7 @@ [LibraryClasses]

[Packages]
MdePkg/MdePkg.dec
+ IntelSiliconPkg/IntelSiliconPkg.dec
KabylakeSiliconPkg/SiPkg.dec

[Sources]
diff --git a/Silicon/Intel/KabylakeSiliconPkg/Pch/Spi/Smm/PchSpiSmm.inf
b/Silicon/Intel/KabylakeSiliconPkg/Pch/Spi/Smm/PchSpiSmm.inf
index 964489064a74..819dc2439f30 100644
--- a/Silicon/Intel/KabylakeSiliconPkg/Pch/Spi/Smm/PchSpiSmm.inf
+++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Spi/Smm/PchSpiSmm.inf
@@ -30,6 +30,7 @@ [LibraryClasses]

[Packages]
MdePkg/MdePkg.dec
+IntelSiliconPkg/IntelSiliconPkg.dec
KabylakeSiliconPkg/SiPkg.dec
KabylakeSiliconPkg/KabylakeSiliconPrivate.dec

diff --git a/Silicon/Intel/KabylakeSiliconPkg/SiPkg.dec
b/Silicon/Intel/KabylakeSiliconPkg/SiPkg.dec
index 5ff7b39ca60e..d9ae9f6dfd91 100644
--- a/Silicon/Intel/KabylakeSiliconPkg/SiPkg.dec
+++ b/Silicon/Intel/KabylakeSiliconPkg/SiPkg.dec
@@ -294,13 +294,11 @@ [Protocols]
##
## PCH
##
-gPchSpiProtocolGuid = {0xc7d289, 0x1347, 0x4de0, {0xbf, 0x42, 0xe, 0x26,
0x9d, 0xe, 0xf3, 0x4a}} gPchSerialGpioProtocolGuid = {0xf52c3858, 0x5ef8,
0x4d41, {0x83, 0x4e, 0xc3, 0x9e, 0xef, 0x8a, 0x45, 0xa3}} gWdtProtocolGuid =
{0xb42b8d12, 0x2acb, 0x499a, {0xa9, 0x20, 0xdd, 0x5b, 0xe6, 0xcf, 0x09, 0xb1}}
gPchInfoProtocolGuid = {0x984eb4e9, 0x5a95, 0x41de, {0xaa, 0xd0, 0x53, 0x66,
0x8c, 0xa5, 0x13, 0xc0}} gPchSerialIoUartDebugInfoProtocolGuid =
{0x2fd2b1bd, 0x0387, 0x4ec6, {0x94, 0x1f, 0xf1, 0x4b, 0x7f, 0x1c, 0x94, 0xb6}}
gEfiSmmSmbusProtocolGuid = {0x72e40094, 0x2ee1, 0x497a, {0x8f, 0x33, 0x4c,
0x93, 0x4a, 0x9e, 0x9c, 0x0c}} -gPchSmmSpiProtocolGuid = {0x56521f06,
0xa62, 0x4822, {0x99, 0x63, 0xdf, 0x1, 0x9d, 0x72, 0xc7, 0xe1}}
gPchSmmIoTrapControlGuid = {0x514d2afd, 0x2096, 0x4283, {0x9d, 0xa6, 0x70,
0x0c, 0xd2, 0x7d, 0xc7, 0xa5}} gPchTcoSmiDispatchProtocolGuid =
{0x9e71d609, 0x6d24, 0x47fd, {0xb5, 0x72, 0x61, 0x40, 0xf8, 0xd9, 0xc2, 0xa4}}
gPchPcieSmiDispatchProtocolGuid = {0x3e7d2b56, 0x3f47, 0x42aa, {0x8f, 0x6b,
0x22, 0xf5, 0x19, 0x81, 0x8d, 0xab}} @@ -361,7 +359,6 @@ [Ppis] ## PCH ##
gWdtPpiGuid = {0xf38d1338, 0xaf7a, 0x4fb6, {0x91, 0xdb, 0x1a, 0x9c, 0x21,
0x83, 0x57, 0x0d}} -gPchSpiPpiGuid = {0xdade7ce3, 0x6971, 0x4b75, {0x82,
0x5e, 0xe, 0xe0, 0xeb, 0x17, 0x72, 0x2d}} gPeiSmbusPolicyPpiGuid =
{0x63b6e435, 0x32bc, 0x49c6, {0x81, 0xbd, 0xb7, 0xa1, 0xa0, 0xfe, 0x1a, 0x6c}}
gPchResetCallbackPpiGuid = {0x17865dc0, 0x0b8b, 0x4da8, {0x8b, 0x42, 0x7c,
0x46, 0xb8, 0x5c, 0xca, 0x4d}} gPchResetPpiGuid = {0x433e0f9f, 0x05ae,
0x410a, {0xa0, 0xc3, 0xbf, 0x29, 0x8e, 0xcb, 0x25, 0xac}}
--
2.28.0.windows.1


Re: [edk2-platforms][PATCH v3 32/41] CoffeelakeSiliconPkg: Remove PCH SPI PPI and Protocol from package

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 32/41] CoffeelakeSiliconPkg: Remove PCH
SPI PPI and Protocol from package

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

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

The following PPI and Protocols have moved to IntelSiliconPkg. The remaining
definitions in CoffeelakeSiliconPkg are removed and libs/ modules that need to
reference IntelSiliconPkg are updated.

1. gPchSpiProtocolGuid
2. gPchSmmSpiProtocolGuid
3. gPchSpiPpiGuid

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/CoffeelakeSiliconPkg/Pch/Include/Ppi/Spi.h
| 27 --
Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Protocol/Spi.h
| 295 --------------------
Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.inf
| 1 +

Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCommonLib/
BasePchSpiCommonLib.inf | 1 +
Silicon/Intel/CoffeelakeSiliconPkg/Pch/Spi/Smm/PchSpiSmm.inf
| 1 +
Silicon/Intel/CoffeelakeSiliconPkg/SiPkg.dec | 3 -
6 files changed, 3 insertions(+), 325 deletions(-)

diff --git a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Ppi/Spi.h
b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Ppi/Spi.h
deleted file mode 100644
index d3ff152742cf..000000000000
--- a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Ppi/Spi.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/** @file
- This file defines the PCH SPI PPI which implements the
- Intel(R) PCH SPI Host Controller Compatibility Interface.
-
- Copyright (c) 2019 Intel Corporation. All rights reserved. <BR>
-
- SPDX-License-Identifier: BSD-2-Clause-Patent -**/
-
-#ifndef _PCH_SPI_PPI_H_
-#define _PCH_SPI_PPI_H_
-
-#include <Protocol/Spi.h>
-
-//
-// Extern the GUID for PPI users.
-//
-extern EFI_GUID gPchSpiPpiGuid;
-
-/**
- Reuse the PCH_SPI_PROTOCOL definitions
- This is possible becaues the PPI implementation does not rely on a PeiService
pointer,
- as it uses EDKII Glue Lib to do IO accesses -**/ -typedef PCH_SPI_PROTOCOL
PCH_SPI_PPI;
-
-#endif
diff --git a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Protocol/Spi.h
b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Protocol/Spi.h
deleted file mode 100644
index 22df7fe35147..000000000000
--- a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Protocol/Spi.h
+++ /dev/null
@@ -1,295 +0,0 @@
-/** @file
- This file defines the PCH SPI Protocol which implements the
- Intel(R) PCH SPI Host Controller Compatibility Interface.
-
- Copyright (c) 2019 Intel Corporation. All rights reserved. <BR>
-
- SPDX-License-Identifier: BSD-2-Clause-Patent -**/
-
-#ifndef _PCH_SPI_PROTOCOL_H_
-#define _PCH_SPI_PROTOCOL_H_
-
-//
-// Extern the GUID for protocol users.
-//
-extern EFI_GUID gPchSpiProtocolGuid;
-extern EFI_GUID gPchSmmSpiProtocolGuid;
-
-//
-// Forward reference for ANSI C compatibility -// -typedef struct
_PCH_SPI_PROTOCOL PCH_SPI_PROTOCOL;
-
-//
-// SPI protocol data structures and definitions -//
-
-/**
- Flash Region Type
-**/
-typedef enum {
- FlashRegionDescriptor,
- FlashRegionBios,
- FlashRegionMe,
- FlashRegionGbE,
- FlashRegionPlatformData,
- FlashRegionDer,
- FlashRegionEC = 8,
- FlashRegionAll,
- FlashRegionMax
-} FLASH_REGION_TYPE;
-
-//
-// Protocol member functions
-//
-
-/**
- Read data from the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] FlashRegionType The Flash Region type for flash cycle which is
listed in the Descriptor.
- @param[in] Address The Flash Linear Address must fall within 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.
- It is the caller's responsibility to make sure Buffer is large
enough for the total number of bytes read.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormally.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_READ) (
- IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
- IN UINT32 Address,
- IN UINT32 ByteCount,
- OUT UINT8 *Buffer
- );
-
-/**
- Write data to the flash part. Remark: Erase may be needed before write to the
flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] FlashRegionType The Flash Region type for flash cycle which is
listed in the Descriptor.
- @param[in] Address The Flash Linear Address must fall within 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 containing the
data sent during the SPI cycle.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormally.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_WRITE) (
- IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
- IN UINT32 Address,
- IN UINT32 ByteCount,
- IN UINT8 *Buffer
- );
-
-/**
- Erase some area on the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] FlashRegionType The Flash Region type for flash cycle which is
listed in the Descriptor.
- @param[in] Address The Flash Linear Address must fall within a region
for which BIOS has access permissions.
- @param[in] ByteCount Number of bytes in the data portion of the SPI
cycle.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormally.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_ERASE) (
- IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
- IN UINT32 Address,
- IN UINT32 ByteCount
- );
-
-/**
- Read SFDP data from the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] ComponentNumber The Componen Number for chip select
- @param[in] Address The starting byte address for SFDP data read.
- @param[in] ByteCount Number of bytes in SFDP data portion of the SPI
cycle
- @param[out] SfdpData The Pointer to caller-allocated buffer containing
the SFDP data received
- It is the caller's responsibility to make sure Buffer is large
enough for the total number of bytes read
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormally.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_READ_SFDP) (
- IN PCH_SPI_PROTOCOL *This,
- IN UINT8 ComponentNumber,
- IN UINT32 Address,
- IN UINT32 ByteCount,
- OUT UINT8 *SfdpData
- );
-
-/**
- Read Jedec Id from the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] ComponentNumber The Componen Number for chip select
- @param[in] ByteCount Number of bytes in JedecId data portion of the
SPI cycle, the data size is 3 typically
- @param[out] JedecId The Pointer to caller-allocated buffer containing
JEDEC ID received
- It is the caller's responsibility to make sure Buffer is large
enough for the total number of bytes read.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormally.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_READ_JEDEC_ID) (
- IN PCH_SPI_PROTOCOL *This,
- IN UINT8 ComponentNumber,
- IN UINT32 ByteCount,
- OUT UINT8 *JedecId
- );
-
-/**
- Write the status register in the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] ByteCount Number of bytes in Status data portion of the SPI
cycle, the data size is 1 typically
- @param[in] StatusValue The Pointer to caller-allocated buffer containing
the value of Status register writing
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormally.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_WRITE_STATUS) (
- IN PCH_SPI_PROTOCOL *This,
- IN UINT32 ByteCount,
- IN UINT8 *StatusValue
- );
-
-/**
- Read status register in the flash part.
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] ByteCount Number of bytes in Status data portion of the SPI
cycle, the data size is 1 typically
- @param[out] StatusValue The Pointer to caller-allocated buffer
containing the value of Status register received.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormally.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_FLASH_READ_STATUS) (
- IN PCH_SPI_PROTOCOL *This,
- IN UINT32 ByteCount,
- OUT UINT8 *StatusValue
- );
-
-/**
- Get the SPI region base and size, based on the enum type
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] FlashRegionType The Flash Region type for for the base
address which is listed in the Descriptor.
- @param[out] BaseAddress The Flash Linear Address for the Region 'n'
Base
- @param[out] RegionSize The size for the Region 'n'
-
- @retval EFI_SUCCESS Read success
- @retval EFI_INVALID_PARAMETER Invalid region type given
- @retval EFI_DEVICE_ERROR The region is not used
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_GET_REGION_ADDRESS) (
- IN PCH_SPI_PROTOCOL *This,
- IN FLASH_REGION_TYPE FlashRegionType,
- OUT UINT32 *BaseAddress,
- OUT UINT32 *RegionSize
- );
-
-/**
- Read PCH Soft Strap Values
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] SoftStrapAddr PCH Soft Strap address offset from FPSBA.
- @param[in] ByteCount Number of bytes in SoftStrap data portion of the
SPI cycle
- @param[out] SoftStrapValue The Pointer to caller-allocated buffer
containing PCH Soft Strap Value.
- If the value of ByteCount is 0, the data type of
SoftStrapValue should be UINT16 and SoftStrapValue will be PCH Soft Strap
Length
- It is the caller's responsibility to make sure Buffer is large
enough for the total number of bytes read.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormally.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_READ_PCH_SOFTSTRAP) (
- IN PCH_SPI_PROTOCOL *This,
- IN UINT32 SoftStrapAddr,
- IN UINT32 ByteCount,
- OUT VOID *SoftStrapValue
- );
-
-/**
- Read CPU Soft Strap Values
-
- @param[in] This Pointer to the PCH_SPI_PROTOCOL instance.
- @param[in] SoftStrapAddr CPU Soft Strap address offset from FCPUSBA.
- @param[in] ByteCount Number of bytes in SoftStrap data portion of the
SPI cycle.
- @param[out] SoftStrapValue The Pointer to caller-allocated buffer
containing CPU Soft Strap Value.
- If the value of ByteCount is 0, the data type of
SoftStrapValue should be UINT16 and SoftStrapValue will be PCH Soft Strap
Length
- It is the caller's responsibility to make sure Buffer is large
enough for the total number of bytes read.
-
- @retval EFI_SUCCESS Command succeed.
- @retval EFI_INVALID_PARAMETER The parameters specified are not valid.
- @retval EFI_DEVICE_ERROR Device error, command aborts abnormally.
-**/
-typedef
-EFI_STATUS
-(EFIAPI *PCH_SPI_READ_CPU_SOFTSTRAP) (
- IN PCH_SPI_PROTOCOL *This,
- IN UINT32 SoftStrapAddr,
- IN UINT32 ByteCount,
- OUT VOID *SoftStrapValue
- );
-
-/**
- These protocols/PPI allows a platform module to perform SPI operations
through the
- Intel PCH SPI Host Controller Interface.
-**/
-struct _PCH_SPI_PROTOCOL {
- /**
- This member specifies the revision of this structure. This field is used to
- indicate backwards compatible changes to the protocol.
- **/
- UINT8 Revision;
- PCH_SPI_FLASH_READ FlashRead; ///< Read data from the flash
part.
- PCH_SPI_FLASH_WRITE FlashWrite; ///< Write data to the flash
part. Remark: Erase may be needed before write to the flash part.
- PCH_SPI_FLASH_ERASE FlashErase; ///< Erase some area on the
flash part.
- PCH_SPI_FLASH_READ_SFDP FlashReadSfdp; ///< Read SFDP data
from the flash part.
- PCH_SPI_FLASH_READ_JEDEC_ID FlashReadJedecId; ///< Read Jedec Id
from the flash part.
- PCH_SPI_FLASH_WRITE_STATUS FlashWriteStatus; ///< Write the status
register in the flash part.
- PCH_SPI_FLASH_READ_STATUS FlashReadStatus; ///< Read status
register in the flash part.
- PCH_SPI_GET_REGION_ADDRESS GetRegionAddress; ///< Get the SPI
region base and size
- PCH_SPI_READ_PCH_SOFTSTRAP ReadPchSoftStrap; ///< Read PCH Soft
Strap Values
- PCH_SPI_READ_CPU_SOFTSTRAP ReadCpuSoftStrap; ///< Read CPU Soft
Strap Values
-};
-
-/**
- PCH SPI PPI/PROTOCOL revision number
-
- Revision 1: Initial version
-**/
-#define PCH_SPI_SERVICES_REVISION 1
-
-#endif
diff --git a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.inf
b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.inf
index fb2fad78d39e..4e4b456574f0 100644
--- a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.inf
+++ b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib
+++ .inf
@@ -30,6 +30,7 @@ [LibraryClasses]

[Packages]
MdePkg/MdePkg.dec
+IntelSiliconPkg/IntelSiliconPkg.dec
CoffeelakeSiliconPkg/SiPkg.dec


diff --git
a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCommonLib
/BasePchSpiCommonLib.inf
b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCommonLib
/BasePchSpiCommonLib.inf
index ea23e628c80e..f5dc4ee0bfef 100644
---
a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCommonLib
/BasePchSpiCommonLib.inf
+++ b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiC
+++ ommonLib/BasePchSpiCommonLib.inf
@@ -20,6 +20,7 @@ [Sources]

[Packages]
MdePkg/MdePkg.dec
+ IntelSiliconPkg/IntelSiliconPkg.dec
CoffeelakeSiliconPkg/SiPkg.dec

[LibraryClasses]
diff --git a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Spi/Smm/PchSpiSmm.inf
b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Spi/Smm/PchSpiSmm.inf
index 77bd3ad72bff..231929151222 100644
--- a/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Spi/Smm/PchSpiSmm.inf
+++ b/Silicon/Intel/CoffeelakeSiliconPkg/Pch/Spi/Smm/PchSpiSmm.inf
@@ -29,6 +29,7 @@ [LibraryClasses]

[Packages]
MdePkg/MdePkg.dec
+IntelSiliconPkg/IntelSiliconPkg.dec
CoffeelakeSiliconPkg/SiPkg.dec


diff --git a/Silicon/Intel/CoffeelakeSiliconPkg/SiPkg.dec
b/Silicon/Intel/CoffeelakeSiliconPkg/SiPkg.dec
index 5ea6fbb28411..efc2d8788168 100644
--- a/Silicon/Intel/CoffeelakeSiliconPkg/SiPkg.dec
+++ b/Silicon/Intel/CoffeelakeSiliconPkg/SiPkg.dec
@@ -303,11 +303,9 @@ [Protocols]
##
## PCH
##
-gPchSpiProtocolGuid = {0xc7d289, 0x1347, 0x4de0, {0xbf, 0x42, 0xe, 0x26,
0x9d, 0xe, 0xf3, 0x4a}} gWdtProtocolGuid = {0xb42b8d12, 0x2acb, 0x499a,
{0xa9, 0x20, 0xdd, 0x5b, 0xe6, 0xcf, 0x09, 0xb1}}
gPchSerialIoUartDebugInfoProtocolGuid = {0x2fd2b1bd, 0x0387, 0x4ec6, {0x94,
0x1f, 0xf1, 0x4b, 0x7f, 0x1c, 0x94, 0xb6}} gEfiSmmSmbusProtocolGuid =
{0x72e40094, 0x2ee1, 0x497a, {0x8f, 0x33, 0x4c, 0x93, 0x4a, 0x9e, 0x9c, 0x0c}}
-gPchSmmSpiProtocolGuid = {0x56521f06, 0xa62, 0x4822, {0x99, 0x63, 0xdf,
0x1, 0x9d, 0x72, 0xc7, 0xe1}} gPchSmmIoTrapControlGuid = {0x514d2afd,
0x2096, 0x4283, {0x9d, 0xa6, 0x70, 0x0c, 0xd2, 0x7d, 0xc7, 0xa5}}
gPchTcoSmiDispatchProtocolGuid = {0x9e71d609, 0x6d24, 0x47fd, {0xb5, 0x72,
0x61, 0x40, 0xf8, 0xd9, 0xc2, 0xa4}} gPchPcieSmiDispatchProtocolGuid =
{0x3e7d2b56, 0x3f47, 0x42aa, {0x8f, 0x6b, 0x22, 0xf5, 0x19, 0x81, 0x8d, 0xab}}
@@ -382,7 +380,6 @@ [Ppis] ## PCH ## gWdtPpiGuid = {0xf38d1338, 0xaf7a,
0x4fb6, {0x91, 0xdb, 0x1a, 0x9c, 0x21, 0x83, 0x57, 0x0d}} -gPchSpiPpiGuid =
{0xdade7ce3, 0x6971, 0x4b75, {0x82, 0x5e, 0xe, 0xe0, 0xeb, 0x17, 0x72, 0x2d}}
gPeiSmbusPolicyPpiGuid = {0x63b6e435, 0x32bc, 0x49c6, {0x81, 0xbd, 0xb7,
0xa1, 0xa0, 0xfe, 0x1a, 0x6c}} gPchResetCallbackPpiGuid = {0x17865dc0,
0x0b8b, 0x4da8, {0x8b, 0x42, 0x7c, 0x46, 0xb8, 0x5c, 0xca, 0x4d}}

--
2.28.0.windows.1


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

4101 - 4120 of 80786