[edk2,3/6] MdeModulePkg/FaultTolerantWriteDxe: factor out boot service accesses

Message ID 20190103182825.32231-5-ard.biesheuvel@linaro.org
State New
Headers show
Series
  • implement standalone MM versions of the variable runtime drivers
Related show

Commit Message

Ard Biesheuvel Jan. 3, 2019, 6:28 p.m.
In preparation of providing a standalone MM based FTW driver, move
the existing SMM driver to the new MM services table, and factor out
some pieces that are specific to the traditional driver, mainly
related to the use of UEFI boot services, which are not accessible
to standalone MM drivers.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 MdeModulePkg/MdeModulePkg.dsc                                                  |  1 +
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h              | 22 ++++-
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c           | 31 +++++++
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c           | 54 +++++------
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf         |  5 +-
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmCommon.h     | 31 +++++++
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmDxe.c        |  1 +
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditionalMm.c | 94 ++++++++++++++++++++
 MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c              | 10 +--
 9 files changed, 205 insertions(+), 44 deletions(-)

-- 
2.17.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Wang, Jian J Jan. 10, 2019, 1:36 a.m. | #1
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>


> -----Original Message-----

> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> Sent: Friday, January 04, 2019 2:28 AM

> To: edk2-devel@lists.01.org

> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek

> <lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney,

> Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>;

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

> Jagadeesh Ujja <jagadeesh.ujja@arm.com>; Achin Gupta

> <Achin.Gupta@arm.com>; Thomas Panakamattam Abraham

> <thomas.abraham@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com>

> Subject: [PATCH 3/6] MdeModulePkg/FaultTolerantWriteDxe: factor out boot

> service accesses

> 

> In preparation of providing a standalone MM based FTW driver, move

> the existing SMM driver to the new MM services table, and factor out

> some pieces that are specific to the traditional driver, mainly

> related to the use of UEFI boot services, which are not accessible

> to standalone MM drivers.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  MdeModulePkg/MdeModulePkg.dsc                                                  |  1 +

>  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h

> | 22 ++++-

>  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c

> | 31 +++++++

>  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c

> | 54 +++++------

>  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf

> |  5 +-

> 

> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmCom

> mon.h     | 31 +++++++

> 

> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmDxe.c

> |  1 +

> 

> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditiona

> lMm.c | 94 ++++++++++++++++++++

>  MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c

> | 10 +--

>  9 files changed, 205 insertions(+), 44 deletions(-)

> 

> diff --git a/MdeModulePkg/MdeModulePkg.dsc

> b/MdeModulePkg/MdeModulePkg.dsc

> index 5d042be3a862..ef3c144ed524 100644

> --- a/MdeModulePkg/MdeModulePkg.dsc

> +++ b/MdeModulePkg/MdeModulePkg.dsc

> @@ -153,6 +153,7 @@ [LibraryClasses.common.DXE_SMM_DRIVER]

>    DebugLib|MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf

> 

> MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemory

> AllocationLib.inf

> 

> SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTable

> Lib.inf

> +

> MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib

> .inf

>    LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf

>    SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf

> 

> diff --git

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h

> index 844cf3bee04d..8d146264b129 100644

> --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h

> +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h

> @@ -31,7 +31,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY

> KIND, EITHER EXPRESS OR IMPLIED.

>  #include <Library/UefiDriverEntryPoint.h>

>  #include <Library/BaseMemoryLib.h>

>  #include <Library/MemoryAllocationLib.h>

> -#include <Library/UefiBootServicesTableLib.h>

>  #include <Library/ReportStatusCodeLib.h>

> 

>  //

> @@ -766,4 +765,25 @@ WriteWorkSpaceData (

>    IN UINT8                              *Buffer

>    );

> 

> +/**

> +  Internal implementation of CRC32. Depending on the execution context

> +  (traditional SMM or DXE vs standalone MM), this function is implemented

> +  via a call to the CalculateCrc32 () boot service, or via a library

> +  call.

> +

> +  If Buffer is NULL, then ASSERT().

> +  If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().

> +

> +  @param[in]  Buffer       A pointer to the buffer on which the 32-bit CRC is to be

> computed.

> +  @param[in]  Length       The number of bytes in the buffer Data.

> +

> +  @retval Crc32            The 32-bit CRC was computed for the data buffer.

> +

> +**/

> +UINT32

> +FtwCalculateCrc32 (

> +  IN  VOID                         *Buffer,

> +  IN  UINTN                        Length

> +  );

> +

>  #endif

> diff --git

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c

> index 094e40f9d86c..24e507104bbe 100644

> ---

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c

> +++

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c

> @@ -51,6 +51,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY

> KIND, EITHER EXPRESS OR IMPLIED.

> 

>  **/

> 

> +#include <Library/UefiBootServicesTableLib.h>

>  #include "FaultTolerantWrite.h"

>  EFI_EVENT                                 mFvbRegistration = NULL;

> 

> @@ -250,3 +251,33 @@ FaultTolerantWriteInitialize (

> 

>    return EFI_SUCCESS;

>  }

> +

> +/**

> +  Internal implementation of CRC32. Depending on the execution context

> +  (traditional SMM or DXE vs standalone MM), this function is implemented

> +  via a call to the CalculateCrc32 () boot service, or via a library

> +  call.

> +

> +  If Buffer is NULL, then ASSERT().

> +  If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().

> +

> +  @param[in]  Buffer       A pointer to the buffer on which the 32-bit CRC is to be

> computed.

> +  @param[in]  Length       The number of bytes in the buffer Data.

> +

> +  @retval Crc32            The 32-bit CRC was computed for the data buffer.

> +

> +**/

> +UINT32

> +FtwCalculateCrc32 (

> +  IN  VOID                         *Buffer,

> +  IN  UINTN                        Length

> +  )

> +{

> +  EFI_STATUS    Status;

> +  UINT32        ReturnValue;

> +

> +  Status = gBS->CalculateCrc32 (Buffer, Length, &ReturnValue);

> +  ASSERT_EFI_ERROR (Status);

> +

> +  return ReturnValue;

> +}

> diff --git

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c

> index 481fea3f1fdf..e91d04e56d7c 100644

> ---

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c

> +++

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c

> @@ -54,14 +54,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY

> KIND, EITHER EXPRESS OR IMPLIED.

> 

>  **/

> 

> -#include <PiSmm.h>

> -#include <Library/SmmServicesTableLib.h>

> -#include <Library/SmmMemLib.h>

> +#include <PiMm.h>

> +#include <Library/MmServicesTableLib.h>

>  #include <Library/BaseLib.h>

>  #include <Protocol/SmmSwapAddressRange.h>

>  #include "FaultTolerantWrite.h"

>  #include "FaultTolerantWriteSmmCommon.h"

> -#include <Protocol/SmmEndOfDxe.h>

> +#include <Protocol/MmEndOfDxe.h>

> 

>  EFI_EVENT                                 mFvbRegistration = NULL;

>  EFI_FTW_DEVICE                            *mFtwDevice      = NULL;

> @@ -92,7 +91,7 @@ FtwGetFvbByHandle (

>    //

>    // To get the SMM FVB protocol interface on the handle

>    //

> -  return gSmst->SmmHandleProtocol (

> +  return gMmst->MmHandleProtocol (

>                    FvBlockHandle,

>                    &gEfiSmmFirmwareVolumeBlockProtocolGuid,

>                    (VOID **) FvBlock

> @@ -119,7 +118,7 @@ FtwGetSarProtocol (

>    //

>    // Locate Smm Swap Address Range protocol

>    //

> -  Status = gSmst->SmmLocateProtocol (

> +  Status = gMmst->MmLocateProtocol (

>                      &gEfiSmmSwapAddressRangeProtocolGuid,

>                      NULL,

>                      SarProtocol

> @@ -158,7 +157,7 @@ GetFvbCountAndBuffer (

>    BufferSize     = 0;

>    *NumberHandles = 0;

>    *Buffer        = NULL;

> -  Status = gSmst->SmmLocateHandle (

> +  Status = gMmst->MmLocateHandle (

>                      ByProtocol,

>                      &gEfiSmmFirmwareVolumeBlockProtocolGuid,

>                      NULL,

> @@ -174,7 +173,7 @@ GetFvbCountAndBuffer (

>      return EFI_OUT_OF_RESOURCES;

>    }

> 

> -  Status = gSmst->SmmLocateHandle (

> +  Status = gMmst->MmLocateHandle (

>                      ByProtocol,

>                      &gEfiSmmFirmwareVolumeBlockProtocolGuid,

>                      NULL,

> @@ -336,8 +335,7 @@ SmmFaultTolerantWriteHandler (

>    }

>    CommBufferPayloadSize = TempCommBufferSize -

> SMM_FTW_COMMUNICATE_HEADER_SIZE;

> 

> -  if (!SmmIsBufferOutsideSmmValid ((UINTN)CommBuffer,

> TempCommBufferSize)) {

> -    DEBUG ((EFI_D_ERROR, "SmmFtwHandler: SMM communication buffer in

> SMRAM or overflow!\n"));

> +  if (!FtwSmmIsBufferOutsideSmmValid ((UINTN)CommBuffer,

> TempCommBufferSize)) {

>      return EFI_SUCCESS;

>    }

> 

> @@ -525,13 +523,12 @@ FvbNotificationEvent (

>    EFI_STATUS                              Status;

>    EFI_SMM_FAULT_TOLERANT_WRITE_PROTOCOL   *FtwProtocol;

>    EFI_HANDLE                              SmmFtwHandle;

> -  EFI_HANDLE                              FtwHandle;

> 

>    //

>    // Just return to avoid install SMM FaultTolerantWriteProtocol again

>    // if SMM Fault Tolerant Write protocol had been installed.

>    //

> -  Status = gSmst->SmmLocateProtocol (

> +  Status = gMmst->MmLocateProtocol (

>                      &gEfiSmmFaultTolerantWriteProtocolGuid,

>                      NULL,

>                      (VOID **) &FtwProtocol

> @@ -551,7 +548,7 @@ FvbNotificationEvent (

>    //

>    // Install protocol interface

>    //

> -  Status = gSmst->SmmInstallProtocolInterface (

> +  Status = gMmst->MmInstallProtocolInterface (

>                      &mFtwDevice->Handle,

>                      &gEfiSmmFaultTolerantWriteProtocolGuid,

>                      EFI_NATIVE_INTERFACE,

> @@ -562,20 +559,13 @@ FvbNotificationEvent (

>    ///

>    /// Register SMM FTW SMI handler

>    ///

> -  Status = gSmst->SmiHandlerRegister (SmmFaultTolerantWriteHandler,

> &gEfiSmmFaultTolerantWriteProtocolGuid, &SmmFtwHandle);

> +  Status = gMmst->MmiHandlerRegister (SmmFaultTolerantWriteHandler,

> &gEfiSmmFaultTolerantWriteProtocolGuid, &SmmFtwHandle);

>    ASSERT_EFI_ERROR (Status);

> 

>    //

>    // Notify the Ftw wrapper driver SMM Ftw is ready

>    //

> -  FtwHandle = NULL;

> -  Status = gBS->InstallProtocolInterface (

> -                  &FtwHandle,

> -                  &gEfiSmmFaultTolerantWriteProtocolGuid,

> -                  EFI_NATIVE_INTERFACE,

> -                  NULL

> -                  );

> -  ASSERT_EFI_ERROR (Status);

> +  FtwNotifySmmReady ();

> 

>    return EFI_SUCCESS;

>  }

> @@ -592,7 +582,7 @@ FvbNotificationEvent (

>  **/

>  EFI_STATUS

>  EFIAPI

> -SmmEndOfDxeCallback (

> +MmEndOfDxeCallback (

>    IN CONST EFI_GUID                       *Protocol,

>    IN VOID                                 *Interface,

>    IN EFI_HANDLE                           Handle

> @@ -614,14 +604,12 @@ SmmEndOfDxeCallback (

> 

>  **/

>  EFI_STATUS

> -EFIAPI

> -SmmFaultTolerantWriteInitialize (

> -  IN EFI_HANDLE                           ImageHandle,

> -  IN EFI_SYSTEM_TABLE                     *SystemTable

> +MmFaultTolerantWriteInitialize (

> +  VOID

>    )

>  {

>    EFI_STATUS                              Status;

> -  VOID                                    *SmmEndOfDxeRegistration;

> +  VOID                                    *MmEndOfDxeRegistration;

> 

>    //

>    // Allocate private data structure for SMM FTW protocol and do some

> initialization

> @@ -634,17 +622,17 @@ SmmFaultTolerantWriteInitialize (

>    //

>    // Register EFI_SMM_END_OF_DXE_PROTOCOL_GUID notify function.

>    //

> -  Status = gSmst->SmmRegisterProtocolNotify (

> -                    &gEfiSmmEndOfDxeProtocolGuid,

> -                    SmmEndOfDxeCallback,

> -                    &SmmEndOfDxeRegistration

> +  Status = gMmst->MmRegisterProtocolNotify (

> +                    &gEfiMmEndOfDxeProtocolGuid,

> +                    MmEndOfDxeCallback,

> +                    &MmEndOfDxeRegistration

>                      );

>    ASSERT_EFI_ERROR (Status);

> 

>    //

>    // Register FvbNotificationEvent () notify function.

>    //

> -  Status = gSmst->SmmRegisterProtocolNotify (

> +  Status = gMmst->MmRegisterProtocolNotify (

>                      &gEfiSmmFirmwareVolumeBlockProtocolGuid,

>                      FvbNotificationEvent,

>                      &mFvbRegistration

> diff --git

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf

> index 606cc2266bda..1653365bc247 100644

> ---

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf

> +++

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf

> @@ -37,6 +37,7 @@ [Sources]

>    FtwMisc.c

>    UpdateWorkingBlock.c

>    FaultTolerantWrite.c

> +  FaultTolerantWriteTraditionalMm.c

>    FaultTolerantWriteSmm.c

>    FaultTolerantWrite.h

>    FaultTolerantWriteSmmCommon.h

> @@ -46,7 +47,7 @@ [Packages]

>    MdeModulePkg/MdeModulePkg.dec

> 

>  [LibraryClasses]

> -  SmmServicesTableLib

> +  MmServicesTableLib

>    MemoryAllocationLib

>    BaseMemoryLib

>    UefiDriverEntryPoint

> @@ -73,7 +74,7 @@ [Protocols]

>    ## PRODUCES

>    ## UNDEFINED # SmiHandlerRegister

>    gEfiSmmFaultTolerantWriteProtocolGuid

> -  gEfiSmmEndOfDxeProtocolGuid                      ## CONSUMES

> +  gEfiMmEndOfDxeProtocolGuid                      ## CONSUMES

> 

>  [FeaturePcd]

>    gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable    ## CONSUMES

> diff --git

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmCo

> mmon.h

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmCo

> mmon.h

> index 8ad0015f3c9e..25b5f7c87326 100644

> ---

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmCo

> mmon.h

> +++

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmCo

> mmon.h

> @@ -77,4 +77,35 @@ typedef struct {

>    UINT8                                 Data[1];

>  } SMM_FTW_GET_LAST_WRITE_HEADER;

> 

> +/**

> +  Entry point of the module

> +**/

> +EFI_STATUS

> +MmFaultTolerantWriteInitialize (

> +  VOID

> +  );

> +

> +/**

> +  This function check if the buffer is valid per processor architecture and not

> overlap with SMRAM.

> +

> +  @param Buffer  The buffer start address to be checked.

> +  @param Length  The buffer length to be checked.

> +

> +  @retval TRUE  This buffer is valid per processor architecture and not overlap

> with SMRAM.

> +  @retval FALSE This buffer is not valid per processor architecture or overlap

> with SMRAM.

> +**/

> +BOOLEAN

> +FtwSmmIsBufferOutsideSmmValid (

> +  IN EFI_PHYSICAL_ADDRESS  Buffer,

> +  IN UINT64                Length

> +  );

> +

> +/**

> +  Notify the system that the SMM driver is ready

> +**/

> +VOID

> +FtwNotifySmmReady (

> +  VOID

> +  );

> +

>  #endif

> diff --git

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmDx

> e.c

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmDx

> e.c

> index 259e9365f483..8694b9254cde 100644

> ---

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmDx

> e.c

> +++

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmDx

> e.c

> @@ -14,6 +14,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY

> KIND, EITHER EXPRESS OR IMPLIED.

> 

>  **/

> 

> +#include <Library/UefiBootServicesTableLib.h>

>  #include "FaultTolerantWriteSmmDxe.h"

> 

>  EFI_HANDLE                         mHandle                   = NULL;

> diff --git

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditio

> nalMm.c

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditio

> nalMm.c

> new file mode 100644

> index 000000000000..440dced37bf8

> --- /dev/null

> +++

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditio

> nalMm.c

> @@ -0,0 +1,94 @@

> +/** @file

> +

> +  Parts of the SMM/MM implementation that are specific to traditional MM

> +

> +Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved. <BR>

> +Copyright (c) 2018, Linaro, Ltd. All rights reserved. <BR>

> +This program and the accompanying materials

> +are licensed and made available under the terms and conditions of the BSD

> License

> +which accompanies this distribution.  The full text of the license may be found

> at

> +http://opensource.org/licenses/bsd-license.php

> +

> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,

> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS

> OR IMPLIED.

> +

> +**/

> +

> +#include <Library/SmmMemLib.h>

> +#include <Library/UefiBootServicesTableLib.h>

> +#include "FaultTolerantWrite.h"

> +#include "FaultTolerantWriteSmmCommon.h"

> +

> +BOOLEAN

> +FtwSmmIsBufferOutsideSmmValid (

> +  IN EFI_PHYSICAL_ADDRESS  Buffer,

> +  IN UINT64                Length

> +  )

> +{

> +  if (!SmmIsBufferOutsideSmmValid (Buffer, Length)) {

> +    DEBUG ((EFI_D_ERROR, "SmmFtwHandler: SMM communication buffer in

> SMRAM or overflow!\n"));

> +    return FALSE;

> +  }

> +  return TRUE;

> +}

> +

> +/**

> +  Internal implementation of CRC32. Depending on the execution context

> +  (traditional SMM or DXE vs standalone MM), this function is implemented

> +  via a call to the CalculateCrc32 () boot service, or via a library

> +  call.

> +

> +  If Buffer is NULL, then ASSERT().

> +  If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().

> +

> +  @param[in]  Buffer       A pointer to the buffer on which the 32-bit CRC is to be

> computed.

> +  @param[in]  Length       The number of bytes in the buffer Data.

> +

> +  @retval Crc32            The 32-bit CRC was computed for the data buffer.

> +

> +**/

> +UINT32

> +FtwCalculateCrc32 (

> +  IN  VOID                         *Buffer,

> +  IN  UINTN                        Length

> +  )

> +{

> +  EFI_STATUS    Status;

> +  UINT32        ReturnValue;

> +

> +  Status = gBS->CalculateCrc32 (Buffer, Length, &ReturnValue);

> +  ASSERT_EFI_ERROR (Status);

> +

> +  return ReturnValue;

> +}

> +

> +/**

> +  Notify the system that the SMM driver is ready

> +**/

> +VOID

> +FtwNotifySmmReady (

> +  VOID

> +  )

> +{

> +  EFI_HANDLE          FtwHandle;

> +  EFI_STATUS          Status;

> +

> +  FtwHandle = NULL;

> +  Status = gBS->InstallProtocolInterface (

> +                  &FtwHandle,

> +                  &gEfiSmmFaultTolerantWriteProtocolGuid,

> +                  EFI_NATIVE_INTERFACE,

> +                  NULL

> +                  );

> +  ASSERT_EFI_ERROR (Status);

> +}

> +

> +EFI_STATUS

> +EFIAPI

> +SmmFaultTolerantWriteInitialize (

> +  IN EFI_HANDLE            ImageHandle,

> +  IN EFI_SYSTEM_TABLE      *SystemTable

> +  )

> +{

> +  return MmFaultTolerantWriteInitialize ();

> +}

> diff --git

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c

> index 50d3421b88bb..d09e9719cf05 100644

> --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c

> +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c

> @@ -29,8 +29,6 @@ InitializeLocalWorkSpaceHeader (

>    VOID

>    )

>  {

> -  EFI_STATUS                              Status;

> -

>    //

>    // Check signature with gEdkiiWorkingBlockSignatureGuid.

>    //

> @@ -64,12 +62,8 @@ InitializeLocalWorkSpaceHeader (

>    //

>    // Calculate the Crc of woking block header

>    //

> -  Status = gBS->CalculateCrc32 (

> -                  &mWorkingBlockHeader,

> -                  sizeof (EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER),

> -                  &mWorkingBlockHeader.Crc

> -                  );

> -  ASSERT_EFI_ERROR (Status);

> +  mWorkingBlockHeader.Crc = FtwCalculateCrc32 (&mWorkingBlockHeader,

> +                              sizeof (EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER));

> 

>    mWorkingBlockHeader.WorkingBlockValid    = FTW_VALID_STATE;

>    mWorkingBlockHeader.WorkingBlockInvalid  = FTW_INVALID_STATE;

> --

> 2.17.1


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Zeng, Star Jan. 10, 2019, 6:45 a.m. | #2
Hi Ard,

Some minor feedback added inline.

On 2019/1/4 2:28, Ard Biesheuvel wrote:
> In preparation of providing a standalone MM based FTW driver, move

> the existing SMM driver to the new MM services table, and factor out

> some pieces that are specific to the traditional driver, mainly

> related to the use of UEFI boot services, which are not accessible

> to standalone MM drivers.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>   MdeModulePkg/MdeModulePkg.dsc                                                  |  1 +

>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h              | 22 ++++-

>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c           | 31 +++++++

>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c           | 54 +++++------

>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf         |  5 +-

>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmCommon.h     | 31 +++++++

>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmDxe.c        |  1 +

>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditionalMm.c | 94 ++++++++++++++++++++

>   MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c              | 10 +--

>   9 files changed, 205 insertions(+), 44 deletions(-)

> 

> diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc

> index 5d042be3a862..ef3c144ed524 100644

> --- a/MdeModulePkg/MdeModulePkg.dsc

> +++ b/MdeModulePkg/MdeModulePkg.dsc

> @@ -153,6 +153,7 @@ [LibraryClasses.common.DXE_SMM_DRIVER]

>     DebugLib|MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf

>     MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.inf

>     SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf

> +  MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf

>     LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf

>     SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf

>   

> diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h

> index 844cf3bee04d..8d146264b129 100644

> --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h

> +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h

> @@ -31,7 +31,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.

>   #include <Library/UefiDriverEntryPoint.h>

>   #include <Library/BaseMemoryLib.h>

>   #include <Library/MemoryAllocationLib.h>

> -#include <Library/UefiBootServicesTableLib.h>

>   #include <Library/ReportStatusCodeLib.h>

>   

>   //

> @@ -766,4 +765,25 @@ WriteWorkSpaceData (

>     IN UINT8                              *Buffer

>     );

>   

> +/**

> +  Internal implementation of CRC32. Depending on the execution context

> +  (traditional SMM or DXE vs standalone MM), this function is implemented

> +  via a call to the CalculateCrc32 () boot service, or via a library

> +  call.

> +

> +  If Buffer is NULL, then ASSERT().

> +  If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().

> +

> +  @param[in]  Buffer       A pointer to the buffer on which the 32-bit CRC is to be computed.

> +  @param[in]  Length       The number of bytes in the buffer Data.

> +

> +  @retval Crc32            The 32-bit CRC was computed for the data buffer.

> +

> +**/

> +UINT32

> +FtwCalculateCrc32 (

> +  IN  VOID                         *Buffer,

> +  IN  UINTN                        Length

> +  );

> +

>   #endif

> diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c

> index 094e40f9d86c..24e507104bbe 100644

> --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c

> +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c

> @@ -51,6 +51,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.

>   

>   **/

>   

> +#include <Library/UefiBootServicesTableLib.h>

>   #include "FaultTolerantWrite.h"

>   EFI_EVENT                                 mFvbRegistration = NULL;

>   

> @@ -250,3 +251,33 @@ FaultTolerantWriteInitialize (

>   

>     return EFI_SUCCESS;

>   }

> +

> +/**

> +  Internal implementation of CRC32. Depending on the execution context

> +  (traditional SMM or DXE vs standalone MM), this function is implemented

> +  via a call to the CalculateCrc32 () boot service, or via a library

> +  call.

> +

> +  If Buffer is NULL, then ASSERT().

> +  If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().

> +

> +  @param[in]  Buffer       A pointer to the buffer on which the 32-bit CRC is to be computed.

> +  @param[in]  Length       The number of bytes in the buffer Data.

> +

> +  @retval Crc32            The 32-bit CRC was computed for the data buffer.

> +

> +**/

> +UINT32

> +FtwCalculateCrc32 (

> +  IN  VOID                         *Buffer,

> +  IN  UINTN                        Length

> +  )

> +{

> +  EFI_STATUS    Status;

> +  UINT32        ReturnValue;

> +

> +  Status = gBS->CalculateCrc32 (Buffer, Length, &ReturnValue);

> +  ASSERT_EFI_ERROR (Status);

> +

> +  return ReturnValue;

> +}

> diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c

> index 481fea3f1fdf..e91d04e56d7c 100644

> --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c

> +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c

> @@ -54,14 +54,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.

>   

>   **/

>   

> -#include <PiSmm.h>

> -#include <Library/SmmServicesTableLib.h>

> -#include <Library/SmmMemLib.h>

> +#include <PiMm.h>

> +#include <Library/MmServicesTableLib.h>

>   #include <Library/BaseLib.h>

>   #include <Protocol/SmmSwapAddressRange.h>

>   #include "FaultTolerantWrite.h"

>   #include "FaultTolerantWriteSmmCommon.h"

> -#include <Protocol/SmmEndOfDxe.h>

> +#include <Protocol/MmEndOfDxe.h>

>   

>   EFI_EVENT                                 mFvbRegistration = NULL;

>   EFI_FTW_DEVICE                            *mFtwDevice      = NULL;

> @@ -92,7 +91,7 @@ FtwGetFvbByHandle (

>     //

>     // To get the SMM FVB protocol interface on the handle

>     //

> -  return gSmst->SmmHandleProtocol (

> +  return gMmst->MmHandleProtocol (

>                     FvBlockHandle,

>                     &gEfiSmmFirmwareVolumeBlockProtocolGuid,

>                     (VOID **) FvBlock

> @@ -119,7 +118,7 @@ FtwGetSarProtocol (

>     //

>     // Locate Smm Swap Address Range protocol

>     //

> -  Status = gSmst->SmmLocateProtocol (

> +  Status = gMmst->MmLocateProtocol (

>                       &gEfiSmmSwapAddressRangeProtocolGuid,

>                       NULL,

>                       SarProtocol

> @@ -158,7 +157,7 @@ GetFvbCountAndBuffer (

>     BufferSize     = 0;

>     *NumberHandles = 0;

>     *Buffer        = NULL;

> -  Status = gSmst->SmmLocateHandle (

> +  Status = gMmst->MmLocateHandle (

>                       ByProtocol,

>                       &gEfiSmmFirmwareVolumeBlockProtocolGuid,

>                       NULL,

> @@ -174,7 +173,7 @@ GetFvbCountAndBuffer (

>       return EFI_OUT_OF_RESOURCES;

>     }

>   

> -  Status = gSmst->SmmLocateHandle (

> +  Status = gMmst->MmLocateHandle (

>                       ByProtocol,

>                       &gEfiSmmFirmwareVolumeBlockProtocolGuid,

>                       NULL,

> @@ -336,8 +335,7 @@ SmmFaultTolerantWriteHandler (

>     }

>     CommBufferPayloadSize = TempCommBufferSize - SMM_FTW_COMMUNICATE_HEADER_SIZE;

>   

> -  if (!SmmIsBufferOutsideSmmValid ((UINTN)CommBuffer, TempCommBufferSize)) {

> -    DEBUG ((EFI_D_ERROR, "SmmFtwHandler: SMM communication buffer in SMRAM or overflow!\n"));


How about keeping the debug message printing code here (but not be in 
FtwSmmIsBufferOutsideSmmValid) to align the change for variable drivers?

> +  if (!FtwSmmIsBufferOutsideSmmValid ((UINTN)CommBuffer, TempCommBufferSize)) {

>       return EFI_SUCCESS;

>     }

>   

> @@ -525,13 +523,12 @@ FvbNotificationEvent (

>     EFI_STATUS                              Status;

>     EFI_SMM_FAULT_TOLERANT_WRITE_PROTOCOL   *FtwProtocol;

>     EFI_HANDLE                              SmmFtwHandle;

> -  EFI_HANDLE                              FtwHandle;

>   

>     //

>     // Just return to avoid install SMM FaultTolerantWriteProtocol again

>     // if SMM Fault Tolerant Write protocol had been installed.

>     //

> -  Status = gSmst->SmmLocateProtocol (

> +  Status = gMmst->MmLocateProtocol (

>                       &gEfiSmmFaultTolerantWriteProtocolGuid,

>                       NULL,

>                       (VOID **) &FtwProtocol

> @@ -551,7 +548,7 @@ FvbNotificationEvent (

>     //

>     // Install protocol interface

>     //

> -  Status = gSmst->SmmInstallProtocolInterface (

> +  Status = gMmst->MmInstallProtocolInterface (

>                       &mFtwDevice->Handle,

>                       &gEfiSmmFaultTolerantWriteProtocolGuid,

>                       EFI_NATIVE_INTERFACE,

> @@ -562,20 +559,13 @@ FvbNotificationEvent (

>     ///

>     /// Register SMM FTW SMI handler

>     ///

> -  Status = gSmst->SmiHandlerRegister (SmmFaultTolerantWriteHandler, &gEfiSmmFaultTolerantWriteProtocolGuid, &SmmFtwHandle);

> +  Status = gMmst->MmiHandlerRegister (SmmFaultTolerantWriteHandler, &gEfiSmmFaultTolerantWriteProtocolGuid, &SmmFtwHandle);

>     ASSERT_EFI_ERROR (Status);

>   

>     //

>     // Notify the Ftw wrapper driver SMM Ftw is ready

>     //

> -  FtwHandle = NULL;

> -  Status = gBS->InstallProtocolInterface (

> -                  &FtwHandle,

> -                  &gEfiSmmFaultTolerantWriteProtocolGuid,

> -                  EFI_NATIVE_INTERFACE,

> -                  NULL

> -                  );

> -  ASSERT_EFI_ERROR (Status);

> +  FtwNotifySmmReady ();

>   

>     return EFI_SUCCESS;

>   }

> @@ -592,7 +582,7 @@ FvbNotificationEvent (

>   **/

>   EFI_STATUS

>   EFIAPI

> -SmmEndOfDxeCallback (

> +MmEndOfDxeCallback (

>     IN CONST EFI_GUID                       *Protocol,

>     IN VOID                                 *Interface,

>     IN EFI_HANDLE                           Handle

> @@ -614,14 +604,12 @@ SmmEndOfDxeCallback (

>   

>   **/

>   EFI_STATUS

> -EFIAPI

> -SmmFaultTolerantWriteInitialize (

> -  IN EFI_HANDLE                           ImageHandle,

> -  IN EFI_SYSTEM_TABLE                     *SystemTable

> +MmFaultTolerantWriteInitialize (

> +  VOID

>     )

>   {

>     EFI_STATUS                              Status;

> -  VOID                                    *SmmEndOfDxeRegistration;

> +  VOID                                    *MmEndOfDxeRegistration;

>   

>     //

>     // Allocate private data structure for SMM FTW protocol and do some initialization

> @@ -634,17 +622,17 @@ SmmFaultTolerantWriteInitialize (

>     //

>     // Register EFI_SMM_END_OF_DXE_PROTOCOL_GUID notify function.

>     //

> -  Status = gSmst->SmmRegisterProtocolNotify (

> -                    &gEfiSmmEndOfDxeProtocolGuid,

> -                    SmmEndOfDxeCallback,

> -                    &SmmEndOfDxeRegistration

> +  Status = gMmst->MmRegisterProtocolNotify (

> +                    &gEfiMmEndOfDxeProtocolGuid,

> +                    MmEndOfDxeCallback,

> +                    &MmEndOfDxeRegistration

>                       );

>     ASSERT_EFI_ERROR (Status);

>   

>     //

>     // Register FvbNotificationEvent () notify function.

>     //

> -  Status = gSmst->SmmRegisterProtocolNotify (

> +  Status = gMmst->MmRegisterProtocolNotify (

>                       &gEfiSmmFirmwareVolumeBlockProtocolGuid,

>                       FvbNotificationEvent,

>                       &mFvbRegistration

> diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf

> index 606cc2266bda..1653365bc247 100644

> --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf

> +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf

> @@ -37,6 +37,7 @@ [Sources]

>     FtwMisc.c

>     UpdateWorkingBlock.c

>     FaultTolerantWrite.c

> +  FaultTolerantWriteTraditionalMm.c

>     FaultTolerantWriteSmm.c

>     FaultTolerantWrite.h

>     FaultTolerantWriteSmmCommon.h

> @@ -46,7 +47,7 @@ [Packages]

>     MdeModulePkg/MdeModulePkg.dec

>   

>   [LibraryClasses]

> -  SmmServicesTableLib

> +  MmServicesTableLib

>     MemoryAllocationLib

>     BaseMemoryLib

>     UefiDriverEntryPoint

> @@ -73,7 +74,7 @@ [Protocols]

>     ## PRODUCES

>     ## UNDEFINED # SmiHandlerRegister

>     gEfiSmmFaultTolerantWriteProtocolGuid

> -  gEfiSmmEndOfDxeProtocolGuid                      ## CONSUMES

> +  gEfiMmEndOfDxeProtocolGuid                      ## CONSUMES

>   

>   [FeaturePcd]

>     gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable    ## CONSUMES

> diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmCommon.h b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmCommon.h

> index 8ad0015f3c9e..25b5f7c87326 100644

> --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmCommon.h

> +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmCommon.h

> @@ -77,4 +77,35 @@ typedef struct {

>     UINT8                                 Data[1];

>   } SMM_FTW_GET_LAST_WRITE_HEADER;

>   

> +/**

> +  Entry point of the module

> +**/

> +EFI_STATUS

> +MmFaultTolerantWriteInitialize (

> +  VOID

> +  );

> +

> +/**

> +  This function check if the buffer is valid per processor architecture and not overlap with SMRAM.

> +

> +  @param Buffer  The buffer start address to be checked.

> +  @param Length  The buffer length to be checked.

> +

> +  @retval TRUE  This buffer is valid per processor architecture and not overlap with SMRAM.

> +  @retval FALSE This buffer is not valid per processor architecture or overlap with SMRAM.

> +**/

> +BOOLEAN

> +FtwSmmIsBufferOutsideSmmValid (

> +  IN EFI_PHYSICAL_ADDRESS  Buffer,

> +  IN UINT64                Length

> +  );

> +

> +/**

> +  Notify the system that the SMM driver is ready


How about using "SMM FTW driver" instead of "SMM driver" here and also 
for the implementations?

> +**/

> +VOID

> +FtwNotifySmmReady (

> +  VOID

> +  );

> +

>   #endif

> diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmDxe.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmDxe.c

> index 259e9365f483..8694b9254cde 100644

> --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmDxe.c

> +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmDxe.c

> @@ -14,6 +14,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.

>   

>   **/

>   

> +#include <Library/UefiBootServicesTableLib.h>


It is not needed as FaultTolerantWriteSmmDxe.h has included it.

>   #include "FaultTolerantWriteSmmDxe.h"

>   

>   EFI_HANDLE                         mHandle                   = NULL;

> diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditionalMm.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditionalMm.c

> new file mode 100644

> index 000000000000..440dced37bf8

> --- /dev/null

> +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditionalMm.c

> @@ -0,0 +1,94 @@

> +/** @file

> +

> +  Parts of the SMM/MM implementation that are specific to traditional MM

> +

> +Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved. <BR>

> +Copyright (c) 2018, Linaro, Ltd. All rights reserved. <BR>

> +This program and the accompanying materials

> +are licensed and made available under the terms and conditions of the BSD License

> +which accompanies this distribution.  The full text of the license may be found at

> +http://opensource.org/licenses/bsd-license.php

> +

> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,

> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.

> +

> +**/

> +

> +#include <Library/SmmMemLib.h>

> +#include <Library/UefiBootServicesTableLib.h>

> +#include "FaultTolerantWrite.h"

> +#include "FaultTolerantWriteSmmCommon.h"

> +

> +BOOLEAN

> +FtwSmmIsBufferOutsideSmmValid (

> +  IN EFI_PHYSICAL_ADDRESS  Buffer,

> +  IN UINT64                Length

> +  )

> +{

> +  if (!SmmIsBufferOutsideSmmValid (Buffer, Length)) {

> +    DEBUG ((EFI_D_ERROR, "SmmFtwHandler: SMM communication buffer in SMRAM or overflow!\n"));


How about keeping the debug message printing code in 
FaultTolerantWriteSmm.c?

> +    return FALSE;

> +  }

> +  return TRUE;

> +}


Please add function comment header for it.

> +

> +/**

> +  Internal implementation of CRC32. Depending on the execution context

> +  (traditional SMM or DXE vs standalone MM), this function is implemented

> +  via a call to the CalculateCrc32 () boot service, or via a library

> +  call.

> +

> +  If Buffer is NULL, then ASSERT().

> +  If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().

> +

> +  @param[in]  Buffer       A pointer to the buffer on which the 32-bit CRC is to be computed.

> +  @param[in]  Length       The number of bytes in the buffer Data.

> +

> +  @retval Crc32            The 32-bit CRC was computed for the data buffer.

> +

> +**/

> +UINT32

> +FtwCalculateCrc32 (

> +  IN  VOID                         *Buffer,

> +  IN  UINTN                        Length

> +  )

> +{

> +  EFI_STATUS    Status;

> +  UINT32        ReturnValue;

> +

> +  Status = gBS->CalculateCrc32 (Buffer, Length, &ReturnValue);

> +  ASSERT_EFI_ERROR (Status);

> +

> +  return ReturnValue;

> +}

> +

> +/**

> +  Notify the system that the SMM driver is ready


How about using "SMM FTW driver" instead of "SMM driver" here?

> +**/

> +VOID

> +FtwNotifySmmReady (

> +  VOID

> +  )

> +{

> +  EFI_HANDLE          FtwHandle;

> +  EFI_STATUS          Status;

> +

> +  FtwHandle = NULL;

> +  Status = gBS->InstallProtocolInterface (

> +                  &FtwHandle,

> +                  &gEfiSmmFaultTolerantWriteProtocolGuid,

> +                  EFI_NATIVE_INTERFACE,

> +                  NULL

> +                  );

> +  ASSERT_EFI_ERROR (Status);

> +}

> +

> +EFI_STATUS

> +EFIAPI

> +SmmFaultTolerantWriteInitialize (

> +  IN EFI_HANDLE            ImageHandle,

> +  IN EFI_SYSTEM_TABLE      *SystemTable

> +  )

> +{

> +  return MmFaultTolerantWriteInitialize ();

> +}


Please add function comment header for it.

Thanks,
Star
> diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c

> index 50d3421b88bb..d09e9719cf05 100644

> --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c

> +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c

> @@ -29,8 +29,6 @@ InitializeLocalWorkSpaceHeader (

>     VOID

>     )

>   {

> -  EFI_STATUS                              Status;

> -

>     //

>     // Check signature with gEdkiiWorkingBlockSignatureGuid.

>     //

> @@ -64,12 +62,8 @@ InitializeLocalWorkSpaceHeader (

>     //

>     // Calculate the Crc of woking block header

>     //

> -  Status = gBS->CalculateCrc32 (

> -                  &mWorkingBlockHeader,

> -                  sizeof (EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER),

> -                  &mWorkingBlockHeader.Crc

> -                  );

> -  ASSERT_EFI_ERROR (Status);

> +  mWorkingBlockHeader.Crc = FtwCalculateCrc32 (&mWorkingBlockHeader,

> +                              sizeof (EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER));

>   

>     mWorkingBlockHeader.WorkingBlockValid    = FTW_VALID_STATE;

>     mWorkingBlockHeader.WorkingBlockInvalid  = FTW_INVALID_STATE;

> 


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch

diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index 5d042be3a862..ef3c144ed524 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -153,6 +153,7 @@  [LibraryClasses.common.DXE_SMM_DRIVER]
   DebugLib|MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf
   MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.inf
   SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf
+  MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf
   LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf
   SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf
 
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
index 844cf3bee04d..8d146264b129 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.h
@@ -31,7 +31,6 @@  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Library/UefiDriverEntryPoint.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/MemoryAllocationLib.h>
-#include <Library/UefiBootServicesTableLib.h>
 #include <Library/ReportStatusCodeLib.h>
 
 //
@@ -766,4 +765,25 @@  WriteWorkSpaceData (
   IN UINT8                              *Buffer
   );
 
+/**
+  Internal implementation of CRC32. Depending on the execution context
+  (traditional SMM or DXE vs standalone MM), this function is implemented
+  via a call to the CalculateCrc32 () boot service, or via a library
+  call.
+
+  If Buffer is NULL, then ASSERT().
+  If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+  @param[in]  Buffer       A pointer to the buffer on which the 32-bit CRC is to be computed.
+  @param[in]  Length       The number of bytes in the buffer Data.
+
+  @retval Crc32            The 32-bit CRC was computed for the data buffer.
+
+**/
+UINT32
+FtwCalculateCrc32 (
+  IN  VOID                         *Buffer,
+  IN  UINTN                        Length
+  );
+
 #endif
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c
index 094e40f9d86c..24e507104bbe 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c
@@ -51,6 +51,7 @@  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 **/
 
+#include <Library/UefiBootServicesTableLib.h>
 #include "FaultTolerantWrite.h"
 EFI_EVENT                                 mFvbRegistration = NULL;
 
@@ -250,3 +251,33 @@  FaultTolerantWriteInitialize (
 
   return EFI_SUCCESS;
 }
+
+/**
+  Internal implementation of CRC32. Depending on the execution context
+  (traditional SMM or DXE vs standalone MM), this function is implemented
+  via a call to the CalculateCrc32 () boot service, or via a library
+  call.
+
+  If Buffer is NULL, then ASSERT().
+  If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+  @param[in]  Buffer       A pointer to the buffer on which the 32-bit CRC is to be computed.
+  @param[in]  Length       The number of bytes in the buffer Data.
+
+  @retval Crc32            The 32-bit CRC was computed for the data buffer.
+
+**/
+UINT32
+FtwCalculateCrc32 (
+  IN  VOID                         *Buffer,
+  IN  UINTN                        Length
+  )
+{
+  EFI_STATUS    Status;
+  UINT32        ReturnValue;
+
+  Status = gBS->CalculateCrc32 (Buffer, Length, &ReturnValue);
+  ASSERT_EFI_ERROR (Status);
+
+  return ReturnValue;
+}
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
index 481fea3f1fdf..e91d04e56d7c 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
@@ -54,14 +54,13 @@  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 **/
 
-#include <PiSmm.h>
-#include <Library/SmmServicesTableLib.h>
-#include <Library/SmmMemLib.h>
+#include <PiMm.h>
+#include <Library/MmServicesTableLib.h>
 #include <Library/BaseLib.h>
 #include <Protocol/SmmSwapAddressRange.h>
 #include "FaultTolerantWrite.h"
 #include "FaultTolerantWriteSmmCommon.h"
-#include <Protocol/SmmEndOfDxe.h>
+#include <Protocol/MmEndOfDxe.h>
 
 EFI_EVENT                                 mFvbRegistration = NULL;
 EFI_FTW_DEVICE                            *mFtwDevice      = NULL;
@@ -92,7 +91,7 @@  FtwGetFvbByHandle (
   //
   // To get the SMM FVB protocol interface on the handle
   //
-  return gSmst->SmmHandleProtocol (
+  return gMmst->MmHandleProtocol (
                   FvBlockHandle,
                   &gEfiSmmFirmwareVolumeBlockProtocolGuid,
                   (VOID **) FvBlock
@@ -119,7 +118,7 @@  FtwGetSarProtocol (
   //
   // Locate Smm Swap Address Range protocol
   //
-  Status = gSmst->SmmLocateProtocol (
+  Status = gMmst->MmLocateProtocol (
                     &gEfiSmmSwapAddressRangeProtocolGuid,
                     NULL,
                     SarProtocol
@@ -158,7 +157,7 @@  GetFvbCountAndBuffer (
   BufferSize     = 0;
   *NumberHandles = 0;
   *Buffer        = NULL;
-  Status = gSmst->SmmLocateHandle (
+  Status = gMmst->MmLocateHandle (
                     ByProtocol,
                     &gEfiSmmFirmwareVolumeBlockProtocolGuid,
                     NULL,
@@ -174,7 +173,7 @@  GetFvbCountAndBuffer (
     return EFI_OUT_OF_RESOURCES;
   }
 
-  Status = gSmst->SmmLocateHandle (
+  Status = gMmst->MmLocateHandle (
                     ByProtocol,
                     &gEfiSmmFirmwareVolumeBlockProtocolGuid,
                     NULL,
@@ -336,8 +335,7 @@  SmmFaultTolerantWriteHandler (
   }
   CommBufferPayloadSize = TempCommBufferSize - SMM_FTW_COMMUNICATE_HEADER_SIZE;
 
-  if (!SmmIsBufferOutsideSmmValid ((UINTN)CommBuffer, TempCommBufferSize)) {
-    DEBUG ((EFI_D_ERROR, "SmmFtwHandler: SMM communication buffer in SMRAM or overflow!\n"));
+  if (!FtwSmmIsBufferOutsideSmmValid ((UINTN)CommBuffer, TempCommBufferSize)) {
     return EFI_SUCCESS;
   }
 
@@ -525,13 +523,12 @@  FvbNotificationEvent (
   EFI_STATUS                              Status;
   EFI_SMM_FAULT_TOLERANT_WRITE_PROTOCOL   *FtwProtocol;
   EFI_HANDLE                              SmmFtwHandle;
-  EFI_HANDLE                              FtwHandle;
 
   //
   // Just return to avoid install SMM FaultTolerantWriteProtocol again
   // if SMM Fault Tolerant Write protocol had been installed.
   //
-  Status = gSmst->SmmLocateProtocol (
+  Status = gMmst->MmLocateProtocol (
                     &gEfiSmmFaultTolerantWriteProtocolGuid,
                     NULL,
                     (VOID **) &FtwProtocol
@@ -551,7 +548,7 @@  FvbNotificationEvent (
   //
   // Install protocol interface
   //
-  Status = gSmst->SmmInstallProtocolInterface (
+  Status = gMmst->MmInstallProtocolInterface (
                     &mFtwDevice->Handle,
                     &gEfiSmmFaultTolerantWriteProtocolGuid,
                     EFI_NATIVE_INTERFACE,
@@ -562,20 +559,13 @@  FvbNotificationEvent (
   ///
   /// Register SMM FTW SMI handler
   ///
-  Status = gSmst->SmiHandlerRegister (SmmFaultTolerantWriteHandler, &gEfiSmmFaultTolerantWriteProtocolGuid, &SmmFtwHandle);
+  Status = gMmst->MmiHandlerRegister (SmmFaultTolerantWriteHandler, &gEfiSmmFaultTolerantWriteProtocolGuid, &SmmFtwHandle);
   ASSERT_EFI_ERROR (Status);
 
   //
   // Notify the Ftw wrapper driver SMM Ftw is ready
   //
-  FtwHandle = NULL;
-  Status = gBS->InstallProtocolInterface (
-                  &FtwHandle,
-                  &gEfiSmmFaultTolerantWriteProtocolGuid,
-                  EFI_NATIVE_INTERFACE,
-                  NULL
-                  );
-  ASSERT_EFI_ERROR (Status);
+  FtwNotifySmmReady ();
 
   return EFI_SUCCESS;
 }
@@ -592,7 +582,7 @@  FvbNotificationEvent (
 **/
 EFI_STATUS
 EFIAPI
-SmmEndOfDxeCallback (
+MmEndOfDxeCallback (
   IN CONST EFI_GUID                       *Protocol,
   IN VOID                                 *Interface,
   IN EFI_HANDLE                           Handle
@@ -614,14 +604,12 @@  SmmEndOfDxeCallback (
 
 **/
 EFI_STATUS
-EFIAPI
-SmmFaultTolerantWriteInitialize (
-  IN EFI_HANDLE                           ImageHandle,
-  IN EFI_SYSTEM_TABLE                     *SystemTable
+MmFaultTolerantWriteInitialize (
+  VOID
   )
 {
   EFI_STATUS                              Status;
-  VOID                                    *SmmEndOfDxeRegistration;
+  VOID                                    *MmEndOfDxeRegistration;
 
   //
   // Allocate private data structure for SMM FTW protocol and do some initialization
@@ -634,17 +622,17 @@  SmmFaultTolerantWriteInitialize (
   //
   // Register EFI_SMM_END_OF_DXE_PROTOCOL_GUID notify function.
   //
-  Status = gSmst->SmmRegisterProtocolNotify (
-                    &gEfiSmmEndOfDxeProtocolGuid,
-                    SmmEndOfDxeCallback,
-                    &SmmEndOfDxeRegistration
+  Status = gMmst->MmRegisterProtocolNotify (
+                    &gEfiMmEndOfDxeProtocolGuid,
+                    MmEndOfDxeCallback,
+                    &MmEndOfDxeRegistration
                     );
   ASSERT_EFI_ERROR (Status);
 
   //
   // Register FvbNotificationEvent () notify function.
   //
-  Status = gSmst->SmmRegisterProtocolNotify (
+  Status = gMmst->MmRegisterProtocolNotify (
                     &gEfiSmmFirmwareVolumeBlockProtocolGuid,
                     FvbNotificationEvent,
                     &mFvbRegistration
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
index 606cc2266bda..1653365bc247 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
@@ -37,6 +37,7 @@  [Sources]
   FtwMisc.c
   UpdateWorkingBlock.c
   FaultTolerantWrite.c
+  FaultTolerantWriteTraditionalMm.c
   FaultTolerantWriteSmm.c
   FaultTolerantWrite.h
   FaultTolerantWriteSmmCommon.h
@@ -46,7 +47,7 @@  [Packages]
   MdeModulePkg/MdeModulePkg.dec
 
 [LibraryClasses]
-  SmmServicesTableLib
+  MmServicesTableLib
   MemoryAllocationLib
   BaseMemoryLib
   UefiDriverEntryPoint
@@ -73,7 +74,7 @@  [Protocols]
   ## PRODUCES
   ## UNDEFINED # SmiHandlerRegister
   gEfiSmmFaultTolerantWriteProtocolGuid
-  gEfiSmmEndOfDxeProtocolGuid                      ## CONSUMES
+  gEfiMmEndOfDxeProtocolGuid                      ## CONSUMES
 
 [FeaturePcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable    ## CONSUMES
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmCommon.h b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmCommon.h
index 8ad0015f3c9e..25b5f7c87326 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmCommon.h
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmCommon.h
@@ -77,4 +77,35 @@  typedef struct {
   UINT8                                 Data[1];
 } SMM_FTW_GET_LAST_WRITE_HEADER;
 
+/**
+  Entry point of the module
+**/
+EFI_STATUS
+MmFaultTolerantWriteInitialize (
+  VOID
+  );
+
+/**
+  This function check if the buffer is valid per processor architecture and not overlap with SMRAM.
+
+  @param Buffer  The buffer start address to be checked.
+  @param Length  The buffer length to be checked.
+
+  @retval TRUE  This buffer is valid per processor architecture and not overlap with SMRAM.
+  @retval FALSE This buffer is not valid per processor architecture or overlap with SMRAM.
+**/
+BOOLEAN
+FtwSmmIsBufferOutsideSmmValid (
+  IN EFI_PHYSICAL_ADDRESS  Buffer,
+  IN UINT64                Length
+  );
+
+/**
+  Notify the system that the SMM driver is ready
+**/
+VOID
+FtwNotifySmmReady (
+  VOID
+  );
+
 #endif
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmDxe.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmDxe.c
index 259e9365f483..8694b9254cde 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmDxe.c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmmDxe.c
@@ -14,6 +14,7 @@  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 **/
 
+#include <Library/UefiBootServicesTableLib.h>
 #include "FaultTolerantWriteSmmDxe.h"
 
 EFI_HANDLE                         mHandle                   = NULL;
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditionalMm.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditionalMm.c
new file mode 100644
index 000000000000..440dced37bf8
--- /dev/null
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditionalMm.c
@@ -0,0 +1,94 @@ 
+/** @file
+
+  Parts of the SMM/MM implementation that are specific to traditional MM
+
+Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved. <BR>
+Copyright (c) 2018, Linaro, Ltd. All rights reserved. <BR>
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD License
+which accompanies this distribution.  The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Library/SmmMemLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include "FaultTolerantWrite.h"
+#include "FaultTolerantWriteSmmCommon.h"
+
+BOOLEAN
+FtwSmmIsBufferOutsideSmmValid (
+  IN EFI_PHYSICAL_ADDRESS  Buffer,
+  IN UINT64                Length
+  )
+{
+  if (!SmmIsBufferOutsideSmmValid (Buffer, Length)) {
+    DEBUG ((EFI_D_ERROR, "SmmFtwHandler: SMM communication buffer in SMRAM or overflow!\n"));
+    return FALSE;
+  }
+  return TRUE;
+}
+
+/**
+  Internal implementation of CRC32. Depending on the execution context
+  (traditional SMM or DXE vs standalone MM), this function is implemented
+  via a call to the CalculateCrc32 () boot service, or via a library
+  call.
+
+  If Buffer is NULL, then ASSERT().
+  If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+  @param[in]  Buffer       A pointer to the buffer on which the 32-bit CRC is to be computed.
+  @param[in]  Length       The number of bytes in the buffer Data.
+
+  @retval Crc32            The 32-bit CRC was computed for the data buffer.
+
+**/
+UINT32
+FtwCalculateCrc32 (
+  IN  VOID                         *Buffer,
+  IN  UINTN                        Length
+  )
+{
+  EFI_STATUS    Status;
+  UINT32        ReturnValue;
+
+  Status = gBS->CalculateCrc32 (Buffer, Length, &ReturnValue);
+  ASSERT_EFI_ERROR (Status);
+
+  return ReturnValue;
+}
+
+/**
+  Notify the system that the SMM driver is ready
+**/
+VOID
+FtwNotifySmmReady (
+  VOID
+  )
+{
+  EFI_HANDLE          FtwHandle;
+  EFI_STATUS          Status;
+
+  FtwHandle = NULL;
+  Status = gBS->InstallProtocolInterface (
+                  &FtwHandle,
+                  &gEfiSmmFaultTolerantWriteProtocolGuid,
+                  EFI_NATIVE_INTERFACE,
+                  NULL
+                  );
+  ASSERT_EFI_ERROR (Status);
+}
+
+EFI_STATUS
+EFIAPI
+SmmFaultTolerantWriteInitialize (
+  IN EFI_HANDLE            ImageHandle,
+  IN EFI_SYSTEM_TABLE      *SystemTable
+  )
+{
+  return MmFaultTolerantWriteInitialize ();
+}
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
index 50d3421b88bb..d09e9719cf05 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
@@ -29,8 +29,6 @@  InitializeLocalWorkSpaceHeader (
   VOID
   )
 {
-  EFI_STATUS                              Status;
-
   //
   // Check signature with gEdkiiWorkingBlockSignatureGuid.
   //
@@ -64,12 +62,8 @@  InitializeLocalWorkSpaceHeader (
   //
   // Calculate the Crc of woking block header
   //
-  Status = gBS->CalculateCrc32 (
-                  &mWorkingBlockHeader,
-                  sizeof (EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER),
-                  &mWorkingBlockHeader.Crc
-                  );
-  ASSERT_EFI_ERROR (Status);
+  mWorkingBlockHeader.Crc = FtwCalculateCrc32 (&mWorkingBlockHeader,
+                              sizeof (EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER));
 
   mWorkingBlockHeader.WorkingBlockValid    = FTW_VALID_STATE;
   mWorkingBlockHeader.WorkingBlockInvalid  = FTW_INVALID_STATE;