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

["Chiu, Chasel" <chasel.chiu@intel.com>]

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