Re: [edk2-platforms][PATCH v4 37/41] IntelSiliconPkg: Identify flash regions by GUID

["Chaganty, Rangasai V" <rangasai.v.chaganty@intel.com>]

Reviewed-by: Sai Chaganty <rangasai.v.chaganty@intel.com>    

-----Original Message-----
From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com> 
Sent: Friday, June 25, 2021 2:21 PM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Subject: [edk2-platforms][PATCH v4 37/41] IntelSiliconPkg: Identify flash regions by GUID

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

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

Updates the SPI protocol to identify flash regions by GUID instead of fixed values defined in an enum. Packages consuming IntelSiliconPkg are able to associate a given GUID with their chosen values based on their SPI flash details as implemented in their PCH_SPI_PROTOCOL instance.

Cc: Ray Ni <ray.ni@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/IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.c   |  2 +-
 Silicon/Intel/IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SpiFlashCommon.c         |  4 +-
 Silicon/Intel/IntelSiliconPkg/Include/Protocol/Spi.h                                | 43 ++++----------------
 Silicon/Intel/IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.inf |  3 ++
 4 files changed, 14 insertions(+), 38 deletions(-)

diff --git a/Silicon/Intel/IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.c b/Silicon/Intel/IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.c
index 7941b8f8720c..2c9c889e7f48 100644
--- a/Silicon/Intel/IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.c
+++ b/Silicon/Intel/IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SmmSpiF
+++ lashCommonLib.c
@@ -52,7 +52,7 @@ SmmSpiFlashCommonLibConstructor (
                     );
   ASSERT_EFI_ERROR (Status);
 
-  mSpiProtocol->GetRegionAddress (mSpiProtocol, FlashRegionBios, &BaseAddr, &RegionSize);
+  mSpiProtocol->GetRegionAddress (mSpiProtocol, &gFlashRegionBiosGuid, 
+ &BaseAddr, &RegionSize);
   mBiosOffset = BaseAddr;
   return Status;
 }
diff --git a/Silicon/Intel/IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SpiFlashCommon.c b/Silicon/Intel/IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SpiFlashCommon.c
index daebaf8e5e33..62c1d099fc2c 100644
--- a/Silicon/Intel/IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SpiFlashCommon.c
+++ b/Silicon/Intel/IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SpiFlas
+++ hCommon.c
@@ -127,7 +127,7 @@ SpiFlashWrite (
     }
     Status = mSpiProtocol->FlashWrite (
                              mSpiProtocol,
-                             FlashRegionBios,
+                             &gFlashRegionBiosGuid,
                              (UINT32) Offset,
                              Length,
                              Buffer
@@ -201,7 +201,7 @@ SpiFlashBlockErase (
 
   Status = mSpiProtocol->FlashErase (
                            mSpiProtocol,
-                           FlashRegionBios,
+                           &gFlashRegionBiosGuid,
                            (UINT32) Offset,
                            (UINT32) RemainingBytes
                            );
diff --git a/Silicon/Intel/IntelSiliconPkg/Include/Protocol/Spi.h b/Silicon/Intel/IntelSiliconPkg/Include/Protocol/Spi.h
index c13dc5a5f5f5..2b09ca1faf02 100644
--- a/Silicon/Intel/IntelSiliconPkg/Include/Protocol/Spi.h
+++ b/Silicon/Intel/IntelSiliconPkg/Include/Protocol/Spi.h
@@ -19,33 +19,6 @@ extern EFI_GUID                   gPchSmmSpiProtocolGuid;
 //
 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,
-  FlashRegionSecondaryBios,
-  FlashRegionuCodePatch,
-  FlashRegionEC,
-  FlashRegionDeviceExpansion2,
-  FlashRegionIE,
-  FlashRegion10Gbe_A,
-  FlashRegion10Gbe_B,
-  FlashRegion13,
-  FlashRegion14,
-  FlashRegion15,
-  FlashRegionAll,
-  FlashRegionMax
-} FLASH_REGION_TYPE;
 //
 // Protocol member functions
 //
@@ -54,7 +27,7 @@ typedef enum {
   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.
@@ -68,7 +41,7 @@ typedef
 EFI_STATUS
 (EFIAPI *PCH_SPI_FLASH_READ) (
   IN     PCH_SPI_PROTOCOL   *This,
-  IN     FLASH_REGION_TYPE  FlashRegionType,
+  IN     EFI_GUID           *FlashRegionGuid,
   IN     UINT32             Address,
   IN     UINT32             ByteCount,
   OUT    UINT8              *Buffer
@@ -78,7 +51,7 @@ EFI_STATUS
   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] 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.
@@ -91,7 +64,7 @@ typedef
 EFI_STATUS
 (EFIAPI *PCH_SPI_FLASH_WRITE) (
   IN     PCH_SPI_PROTOCOL   *This,
-  IN     FLASH_REGION_TYPE  FlashRegionType,
+  IN     EFI_GUID           *FlashRegionGuid,
   IN     UINT32             Address,
   IN     UINT32             ByteCount,
   IN     UINT8              *Buffer
@@ -101,7 +74,7 @@ EFI_STATUS
   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.
 
@@ -113,7 +86,7 @@ typedef
 EFI_STATUS
 (EFIAPI *PCH_SPI_FLASH_ERASE) (
   IN     PCH_SPI_PROTOCOL   *This,
-  IN     FLASH_REGION_TYPE  FlashRegionType,
+  IN     EFI_GUID           *FlashRegionGuid,
   IN     UINT32             Address,
   IN     UINT32             ByteCount
   );
@@ -206,7 +179,7 @@ EFI_STATUS
   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'
 
@@ -218,7 +191,7 @@ typedef
 EFI_STATUS
 (EFIAPI *PCH_SPI_GET_REGION_ADDRESS) (
   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/IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.inf b/Silicon/Intel/IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.inf
index f6a06351ace5..b28ea3c2719d 100644
--- a/Silicon/Intel/IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.inf
+++ b/Silicon/Intel/IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SmmSpiF
+++ lashCommonLib.inf
@@ -37,6 +37,9 @@ [Pcd]
   gIntelSiliconPkgTokenSpaceGuid.PcdBiosAreaBaseAddress   ## CONSUMES
   gIntelSiliconPkgTokenSpaceGuid.PcdBiosSize              ## CONSUMES
 
+[Guids]
+  gFlashRegionBiosGuid
+
 [Sources]
   SmmSpiFlashCommonLib.c
   SpiFlashCommon.c
--
2.28.0.windows.1