Re: [edk2-devel] [edk2-platforms][PATCH v1 26/35] CoffeelakeSiliconPkg: Remove SmmSpiFlashCommonLib

["Michael Kubacki" <mikuback@linux.microsoft.com>]

Thanks for pointing this out. I'm not sure how, but it appears locally 
that patch 27 had an "ENQ" character introduced at the beginning of the 
patch file. This resulted in:

1. Actual patch 26 (correct): https://edk2.groups.io/g/devel/message/74179
2. This version of patch 27 (incorrect): 
https://edk2.groups.io/g/devel/message/74180 which retains the subject 
line from actual patch 26 ("CoffeelakeSiliconPkg...") but the contents 
from actual patch 27 (KabylakeSiliconPkg files modified).

Patch 27 should have had the subject "KabylakeSiliconPkg: Remove 
SmmSpiFlashCommonLib" (as indicated in the cover letter).

I am still collecting feedback on v1. I will check for this in the v2 
patch files.

- Michael

On 4/19/2021 2:07 AM, Chiu, Chasel wrote:
> 
> Hi Michael,
> 
> Please rename subject to "KabylakeSiliconPkg".
> With above change, Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
> 
> Thanks,
> Chasel
> 
> 
>> -----Original Message-----
>> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
>> Sent: Friday, April 16, 2021 10:32 AM
>> To: devel@edk2.groups.io
>> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Chaganty, Rangasai V
>> <rangasai.v.chaganty@intel.com>
>> Subject: [edk2-platforms][PATCH v1 26/35] CoffeelakeSiliconPkg: Remove
>> SmmSpiFlashCommonLib
>>
>> 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>
>> ---
>>
>> 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
> 
> 
> 
> 
>