diff mbox series

[edk2,4/6] MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM version

Message ID 20190103182825.32231-6-ard.biesheuvel@linaro.org
State Accepted
Commit d0def00d33fa5d4a70c427dfc9a36d826b42967d
Headers show
Series implement standalone MM versions of the variable runtime drivers | expand

Commit Message

Ard Biesheuvel Jan. 3, 2019, 6:28 p.m. UTC
Implement a new version of the fault tolerant write driver that can
be used in the context of a standalone MM implementation.

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

---
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c   | 70 +++++++++++++++
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf | 90 ++++++++++++++++++++
 2 files changed, 160 insertions(+)

-- 
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:41 a.m. UTC | #1
Ard,


Regards,
Jian


> -----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 4/6] MdeModulePkg/FaultTolerantWriteDxe: implement

> standalone MM version

> 

> Implement a new version of the fault tolerant write driver that can

> be used in the context of a standalone MM implementation.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

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

> ---

> 

> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandalon

> eMm.c   | 70 +++++++++++++++

> 

> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandalon

> eMm.inf | 90 ++++++++++++++++++++

>  2 files changed, 160 insertions(+)

> 

> diff --git

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

> oneMm.c

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

> oneMm.c

> new file mode 100644

> index 000000000000..b6fbf6c64f8a

> --- /dev/null

> +++

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

> oneMm.c

> @@ -0,0 +1,70 @@

> +/** @file

> +

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

> +

> +Copyright (c) 2010 - 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

> +  )

> +{

> +  return TRUE;

> +}

> +

> +/**

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

> +  (standalone 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

> +  )

> +{

> +  return CalculateCrc32 (Buffer, Length);

> +}

> +

> +VOID

> +FtwNotifySmmReady (

> +  VOID

> +  )

> +{

> +}

> +

> +EFI_STATUS

> +EFIAPI

> +StandaloneMmFaultTolerantWriteInitialize (

> +  IN EFI_HANDLE            ImageHandle,

> +  IN EFI_MM_SYSTEM_TABLE   *MmSystemTable

> +  )

> +{

> +  return MmFaultTolerantWriteInitialize ();

> +}

> diff --git

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

> oneMm.inf

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

> oneMm.inf

> new file mode 100644

> index 000000000000..99bd62ad5ceb

> --- /dev/null

> +++

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

> oneMm.inf

> @@ -0,0 +1,90 @@

> + ## @file

> +#   Fault Tolerant Write Smm Driver.

> +#

> +#   This driver installs SMM Fault Tolerant Write (FTW) protocol, which provides

> fault

> +#   tolerant write capability in SMM environment for block devices. Its

> implementation

> +#   depends on the full functionality SMM FVB protocol that support read,

> write/erase

> +#   flash access.

> +#

> +# Copyright (c) 2010 - 2018, Intel Corporation. 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.

> +#

> +##

> +

> +[Defines]

> +  INF_VERSION                    = 0x0001001A

> +  BASE_NAME                      = FaultTolerantWriteStandaloneMm

> +  FILE_GUID                      = 3aade4ec-63cc-4a48-a928-5a374dd463eb

> +  MODULE_TYPE                    = MM_STANDALONE

> +  VERSION_STRING                 = 1.0

> +  PI_SPECIFICATION_VERSION       = 0x00010032

> +  ENTRY_POINT                    = StandaloneMmFaultTolerantWriteInitialize

> +

> +#

> +# The following information is for reference only and not required by the build

> tools.

> +#

> +#  VALID_ARCHITECTURES           = AARCH64

> +#

> +

> +[Sources]

> +  FtwMisc.c

> +  UpdateWorkingBlock.c

> +  FaultTolerantWrite.c

> +  FaultTolerantWriteStandaloneMm.c

> +  FaultTolerantWriteSmm.c

> +  FaultTolerantWrite.h

> +  FaultTolerantWriteSmmCommon.h

> +

> +[Packages]

> +  MdePkg/MdePkg.dec

> +  MdeModulePkg/MdeModulePkg.dec

> +  StandaloneMmPkg/StandaloneMmPkg.dec

> +

> +[LibraryClasses]

> +  BaseLib

> +  BaseMemoryLib

> +  DebugLib

> +  MemoryAllocationLib

> +  MmServicesTableLib

> +  PcdLib

> +  ReportStatusCodeLib

> +  StandaloneMmDriverEntryPoint

> +

> +[Guids]

> +  #

> +  # Signature in EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER

> +  #

> +  ## CONSUMES           ## GUID

> +  ## PRODUCES           ## GUID

> +  gEdkiiWorkingBlockSignatureGuid

> +

> +[Protocols]

> +  gEfiSmmSwapAddressRangeProtocolGuid |

> gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable  ##

> SOMETIMES_CONSUMES

> +  ## NOTIFY

> +  ## CONSUMES

> +  gEfiSmmFirmwareVolumeBlockProtocolGuid

> +  ## PRODUCES

> +  ## UNDEFINED # SmiHandlerRegister

> +  gEfiSmmFaultTolerantWriteProtocolGuid


CONSUMES/PRODUCES is normally put in trailing comment.


> +  gEfiMmEndOfDxeProtocolGuid                       ## CONSUMES

> +

> +[FeaturePcd]

> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable    ##

> CONSUMES

> +

> +[Pcd]

> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase    ##

> SOMETIMES_CONSUMES

> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64

> ## CONSUMES

> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ##

> CONSUMES

> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase      ##

> SOMETIMES_CONSUMES

> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ##

> CONSUMES

> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ##

> CONSUMES

> +

> +[Depex]

> +  TRUE

> +

> --

> 2.17.1


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Wang, Jian J Jan. 10, 2019, 1:48 a.m. UTC | #2
Forget the previous comment, multiple comments for usage are allowed in INF.

Reviewed-by: Jian J Wang <jian.j.wang@intel.com>



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

> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Wang,

> Jian J

> Sent: Thursday, January 10, 2019 9:42 AM

> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org

> Cc: Wu, Hao A <hao.a.wu@intel.com>; Gao, Liming <liming.gao@intel.com>;

> Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek

> <lersek@redhat.com>

> Subject: Re: [edk2] [PATCH 4/6] MdeModulePkg/FaultTolerantWriteDxe:

> implement standalone MM version

> 

> Ard,

> 

> 

> Regards,

> Jian

> 

> 

> > -----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 4/6] MdeModulePkg/FaultTolerantWriteDxe: implement

> > standalone MM version

> >

> > Implement a new version of the fault tolerant write driver that can

> > be used in the context of a standalone MM implementation.

> >

> > Contributed-under: TianoCore Contribution Agreement 1.1

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

> > ---

> >

> >

> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandalon

> > eMm.c   | 70 +++++++++++++++

> >

> >

> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandalon

> > eMm.inf | 90 ++++++++++++++++++++

> >  2 files changed, 160 insertions(+)

> >

> > diff --git

> >

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

> > oneMm.c

> >

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

> > oneMm.c

> > new file mode 100644

> > index 000000000000..b6fbf6c64f8a

> > --- /dev/null

> > +++

> >

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

> > oneMm.c

> > @@ -0,0 +1,70 @@

> > +/** @file

> > +

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

> > +

> > +Copyright (c) 2010 - 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

> > +  )

> > +{

> > +  return TRUE;

> > +}

> > +

> > +/**

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

> > +  (standalone 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

> > +  )

> > +{

> > +  return CalculateCrc32 (Buffer, Length);

> > +}

> > +

> > +VOID

> > +FtwNotifySmmReady (

> > +  VOID

> > +  )

> > +{

> > +}

> > +

> > +EFI_STATUS

> > +EFIAPI

> > +StandaloneMmFaultTolerantWriteInitialize (

> > +  IN EFI_HANDLE            ImageHandle,

> > +  IN EFI_MM_SYSTEM_TABLE   *MmSystemTable

> > +  )

> > +{

> > +  return MmFaultTolerantWriteInitialize ();

> > +}

> > diff --git

> >

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

> > oneMm.inf

> >

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

> > oneMm.inf

> > new file mode 100644

> > index 000000000000..99bd62ad5ceb

> > --- /dev/null

> > +++

> >

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

> > oneMm.inf

> > @@ -0,0 +1,90 @@

> > + ## @file

> > +#   Fault Tolerant Write Smm Driver.

> > +#

> > +#   This driver installs SMM Fault Tolerant Write (FTW) protocol, which

> provides

> > fault

> > +#   tolerant write capability in SMM environment for block devices. Its

> > implementation

> > +#   depends on the full functionality SMM FVB protocol that support read,

> > write/erase

> > +#   flash access.

> > +#

> > +# Copyright (c) 2010 - 2018, Intel Corporation. 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.

> > +#

> > +##

> > +

> > +[Defines]

> > +  INF_VERSION                    = 0x0001001A

> > +  BASE_NAME                      = FaultTolerantWriteStandaloneMm

> > +  FILE_GUID                      = 3aade4ec-63cc-4a48-a928-5a374dd463eb

> > +  MODULE_TYPE                    = MM_STANDALONE

> > +  VERSION_STRING                 = 1.0

> > +  PI_SPECIFICATION_VERSION       = 0x00010032

> > +  ENTRY_POINT                    = StandaloneMmFaultTolerantWriteInitialize

> > +

> > +#

> > +# The following information is for reference only and not required by the

> build

> > tools.

> > +#

> > +#  VALID_ARCHITECTURES           = AARCH64

> > +#

> > +

> > +[Sources]

> > +  FtwMisc.c

> > +  UpdateWorkingBlock.c

> > +  FaultTolerantWrite.c

> > +  FaultTolerantWriteStandaloneMm.c

> > +  FaultTolerantWriteSmm.c

> > +  FaultTolerantWrite.h

> > +  FaultTolerantWriteSmmCommon.h

> > +

> > +[Packages]

> > +  MdePkg/MdePkg.dec

> > +  MdeModulePkg/MdeModulePkg.dec

> > +  StandaloneMmPkg/StandaloneMmPkg.dec

> > +

> > +[LibraryClasses]

> > +  BaseLib

> > +  BaseMemoryLib

> > +  DebugLib

> > +  MemoryAllocationLib

> > +  MmServicesTableLib

> > +  PcdLib

> > +  ReportStatusCodeLib

> > +  StandaloneMmDriverEntryPoint

> > +

> > +[Guids]

> > +  #

> > +  # Signature in EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER

> > +  #

> > +  ## CONSUMES           ## GUID

> > +  ## PRODUCES           ## GUID

> > +  gEdkiiWorkingBlockSignatureGuid

> > +

> > +[Protocols]

> > +  gEfiSmmSwapAddressRangeProtocolGuid |

> > gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable  ##

> > SOMETIMES_CONSUMES

> > +  ## NOTIFY

> > +  ## CONSUMES

> > +  gEfiSmmFirmwareVolumeBlockProtocolGuid

> > +  ## PRODUCES

> > +  ## UNDEFINED # SmiHandlerRegister

> > +  gEfiSmmFaultTolerantWriteProtocolGuid

> 

> CONSUMES/PRODUCES is normally put in trailing comment.

> 

> 

> > +  gEfiMmEndOfDxeProtocolGuid                       ## CONSUMES

> > +

> > +[FeaturePcd]

> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable    ##

> > CONSUMES

> > +

> > +[Pcd]

> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase

> ##

> > SOMETIMES_CONSUMES

> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64

> > ## CONSUMES

> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize

> ##

> > CONSUMES

> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase      ##

> > SOMETIMES_CONSUMES

> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64

> ##

> > CONSUMES

> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ##

> > CONSUMES

> > +

> > +[Depex]

> > +  TRUE

> > +

> > --

> > 2.17.1

> 

> _______________________________________________

> edk2-devel mailing list

> edk2-devel@lists.01.org

> https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Zeng, Star Jan. 10, 2019, 6:31 a.m. UTC | #3
On 2019/1/10 9:41, Wang, Jian J wrote:
> Ard,

> 

> 

> Regards,

> Jian

> 

> 

>> -----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 4/6] MdeModulePkg/FaultTolerantWriteDxe: implement

>> standalone MM version

>>

>> Implement a new version of the fault tolerant write driver that can

>> be used in the context of a standalone MM implementation.

>>

>> Contributed-under: TianoCore Contribution Agreement 1.1

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

>> ---

>>

>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandalon

>> eMm.c   | 70 +++++++++++++++

>>

>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandalon

>> eMm.inf | 90 ++++++++++++++++++++

>>   2 files changed, 160 insertions(+)

>>

>> diff --git

>> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

>> oneMm.c

>> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

>> oneMm.c

>> new file mode 100644

>> index 000000000000..b6fbf6c64f8a

>> --- /dev/null

>> +++

>> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

>> oneMm.c

>> @@ -0,0 +1,70 @@

>> +/** @file

>> +

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

>> +

>> +Copyright (c) 2010 - 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

>> +  )

>> +{

>> +  return TRUE;

>> +}

>> +

>> +/**

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

>> +  (standalone 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

>> +  )

>> +{

>> +  return CalculateCrc32 (Buffer, Length);

>> +}

>> +

>> +VOID

>> +FtwNotifySmmReady (

>> +  VOID

>> +  )

>> +{

>> +}

>> +

>> +EFI_STATUS

>> +EFIAPI

>> +StandaloneMmFaultTolerantWriteInitialize (

>> +  IN EFI_HANDLE            ImageHandle,

>> +  IN EFI_MM_SYSTEM_TABLE   *MmSystemTable

>> +  )

>> +{

>> +  return MmFaultTolerantWriteInitialize ();

>> +}

>> diff --git

>> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

>> oneMm.inf

>> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

>> oneMm.inf

>> new file mode 100644

>> index 000000000000..99bd62ad5ceb

>> --- /dev/null

>> +++

>> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

>> oneMm.inf

>> @@ -0,0 +1,90 @@

>> + ## @file

>> +#   Fault Tolerant Write Smm Driver.

>> +#

>> +#   This driver installs SMM Fault Tolerant Write (FTW) protocol, which provides

>> fault

>> +#   tolerant write capability in SMM environment for block devices. Its

>> implementation

>> +#   depends on the full functionality SMM FVB protocol that support read,

>> write/erase

>> +#   flash access.

>> +#

>> +# Copyright (c) 2010 - 2018, Intel Corporation. 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.

>> +#

>> +##

>> +

>> +[Defines]

>> +  INF_VERSION                    = 0x0001001A

>> +  BASE_NAME                      = FaultTolerantWriteStandaloneMm

>> +  FILE_GUID                      = 3aade4ec-63cc-4a48-a928-5a374dd463eb

>> +  MODULE_TYPE                    = MM_STANDALONE

>> +  VERSION_STRING                 = 1.0

>> +  PI_SPECIFICATION_VERSION       = 0x00010032

>> +  ENTRY_POINT                    = StandaloneMmFaultTolerantWriteInitialize

>> +

>> +#

>> +# The following information is for reference only and not required by the build

>> tools.

>> +#

>> +#  VALID_ARCHITECTURES           = AARCH64

>> +#

>> +

>> +[Sources]

>> +  FtwMisc.c

>> +  UpdateWorkingBlock.c

>> +  FaultTolerantWrite.c

>> +  FaultTolerantWriteStandaloneMm.c

>> +  FaultTolerantWriteSmm.c

>> +  FaultTolerantWrite.h

>> +  FaultTolerantWriteSmmCommon.h

>> +

>> +[Packages]

>> +  MdePkg/MdePkg.dec

>> +  MdeModulePkg/MdeModulePkg.dec

>> +  StandaloneMmPkg/StandaloneMmPkg.dec

>> +

>> +[LibraryClasses]

>> +  BaseLib

>> +  BaseMemoryLib

>> +  DebugLib

>> +  MemoryAllocationLib

>> +  MmServicesTableLib

>> +  PcdLib

>> +  ReportStatusCodeLib

>> +  StandaloneMmDriverEntryPoint

>> +

>> +[Guids]

>> +  #

>> +  # Signature in EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER

>> +  #

>> +  ## CONSUMES           ## GUID

>> +  ## PRODUCES           ## GUID

>> +  gEdkiiWorkingBlockSignatureGuid

>> +

>> +[Protocols]

>> +  gEfiSmmSwapAddressRangeProtocolGuid |

>> gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable  ##

>> SOMETIMES_CONSUMES

>> +  ## NOTIFY

>> +  ## CONSUMES

>> +  gEfiSmmFirmwareVolumeBlockProtocolGuid

>> +  ## PRODUCES

>> +  ## UNDEFINED # SmiHandlerRegister

>> +  gEfiSmmFaultTolerantWriteProtocolGuid

> 

> CONSUMES/PRODUCES is normally put in trailing comment.


Jian,

If there is only one line usage comment for the GUID, it is normally put 
in trailing comment.

But there are two lines usage comments for this GUID, the style is correct.

Thanks,
Star

> 

> 

>> +  gEfiMmEndOfDxeProtocolGuid                       ## CONSUMES

>> +

>> +[FeaturePcd]

>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable    ##

>> CONSUMES

>> +

>> +[Pcd]

>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase    ##

>> SOMETIMES_CONSUMES

>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64

>> ## CONSUMES

>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ##

>> CONSUMES

>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase      ##

>> SOMETIMES_CONSUMES

>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ##

>> CONSUMES

>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ##

>> CONSUMES

>> +

>> +[Depex]

>> +  TRUE

>> +

>> --

>> 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:47 a.m. UTC | #4
Hi Ard,

Some minor feedback added inline.

On 2019/1/4 2:28, Ard Biesheuvel wrote:
> Implement a new version of the fault tolerant write driver that can

> be used in the context of a standalone MM implementation.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

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

> ---

>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c   | 70 +++++++++++++++

>   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf | 90 ++++++++++++++++++++

>   2 files changed, 160 insertions(+)

> 

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

> new file mode 100644

> index 000000000000..b6fbf6c64f8a

> --- /dev/null

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

> @@ -0,0 +1,70 @@

> +/** @file

> +

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

> +

> +Copyright (c) 2010 - 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

> +  )

> +{

> +  return TRUE;

> +}


Please add function comment header for it, otherwise some coding style 
tool may report error.

> +

> +/**

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

> +  (standalone 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

> +  )

> +{

> +  return CalculateCrc32 (Buffer, Length);

> +}


Please add function comment header for it, otherwise some coding style 
tool may report error.

> +

> +VOID

> +FtwNotifySmmReady (

> +  VOID

> +  )

> +{

> +}


Please add function comment header for it, otherwise some coding style 
tool may report error.

Thanks,
Star

> +

> +EFI_STATUS

> +EFIAPI

> +StandaloneMmFaultTolerantWriteInitialize (

> +  IN EFI_HANDLE            ImageHandle,

> +  IN EFI_MM_SYSTEM_TABLE   *MmSystemTable

> +  )

> +{

> +  return MmFaultTolerantWriteInitialize ();

> +}

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

> new file mode 100644

> index 000000000000..99bd62ad5ceb

> --- /dev/null

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

> @@ -0,0 +1,90 @@

> + ## @file

> +#   Fault Tolerant Write Smm Driver.

> +#

> +#   This driver installs SMM Fault Tolerant Write (FTW) protocol, which provides fault

> +#   tolerant write capability in SMM environment for block devices. Its implementation

> +#   depends on the full functionality SMM FVB protocol that support read, write/erase

> +#   flash access.

> +#

> +# Copyright (c) 2010 - 2018, Intel Corporation. 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.

> +#

> +##

> +

> +[Defines]

> +  INF_VERSION                    = 0x0001001A

> +  BASE_NAME                      = FaultTolerantWriteStandaloneMm

> +  FILE_GUID                      = 3aade4ec-63cc-4a48-a928-5a374dd463eb

> +  MODULE_TYPE                    = MM_STANDALONE

> +  VERSION_STRING                 = 1.0

> +  PI_SPECIFICATION_VERSION       = 0x00010032

> +  ENTRY_POINT                    = StandaloneMmFaultTolerantWriteInitialize

> +

> +#

> +# The following information is for reference only and not required by the build tools.

> +#

> +#  VALID_ARCHITECTURES           = AARCH64

> +#

> +

> +[Sources]

> +  FtwMisc.c

> +  UpdateWorkingBlock.c

> +  FaultTolerantWrite.c

> +  FaultTolerantWriteStandaloneMm.c

> +  FaultTolerantWriteSmm.c

> +  FaultTolerantWrite.h

> +  FaultTolerantWriteSmmCommon.h

> +

> +[Packages]

> +  MdePkg/MdePkg.dec

> +  MdeModulePkg/MdeModulePkg.dec

> +  StandaloneMmPkg/StandaloneMmPkg.dec

> +

> +[LibraryClasses]

> +  BaseLib

> +  BaseMemoryLib

> +  DebugLib

> +  MemoryAllocationLib

> +  MmServicesTableLib

> +  PcdLib

> +  ReportStatusCodeLib

> +  StandaloneMmDriverEntryPoint

> +

> +[Guids]

> +  #

> +  # Signature in EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER

> +  #

> +  ## CONSUMES           ## GUID

> +  ## PRODUCES           ## GUID

> +  gEdkiiWorkingBlockSignatureGuid

> +

> +[Protocols]

> +  gEfiSmmSwapAddressRangeProtocolGuid | gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable  ## SOMETIMES_CONSUMES

> +  ## NOTIFY

> +  ## CONSUMES

> +  gEfiSmmFirmwareVolumeBlockProtocolGuid

> +  ## PRODUCES

> +  ## UNDEFINED # SmiHandlerRegister

> +  gEfiSmmFaultTolerantWriteProtocolGuid

> +  gEfiMmEndOfDxeProtocolGuid                       ## CONSUMES

> +

> +[FeaturePcd]

> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable    ## CONSUMES

> +

> +[Pcd]

> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase    ## SOMETIMES_CONSUMES

> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64  ## CONSUMES

> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ## CONSUMES

> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase      ## SOMETIMES_CONSUMES

> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ## CONSUMES

> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ## CONSUMES

> +

> +[Depex]

> +  TRUE

> +

> 


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

Another minor feedback.

On 2019/1/10 14:47, Zeng, Star wrote:
> Hi Ard,
> 
> Some minor feedback added inline.
> 
> On 2019/1/4 2:28, Ard Biesheuvel wrote:
>> Implement a new version of the fault tolerant write driver that can
>> be used in the context of a standalone MM implementation.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>   
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c   
>> | 70 +++++++++++++++
>>   
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf 
>> | 90 ++++++++++++++++++++
>>   2 files changed, 160 insertions(+)

Please add it into MdeModulePkg.dsc for package build verification.

Thanks,
Star

>>
>> diff --git 
>> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c 
>> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c 
>>
>> new file mode 100644
>> index 000000000000..b6fbf6c64f8a
>> --- /dev/null
>> +++ 
>> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c 
>>
>> @@ -0,0 +1,70 @@
>> +/** @file
>> +
>> +  Parts of the SMM/MM implementation that are specific to standalone MM
>> +
>> +Copyright (c) 2010 - 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
>> +  )
>> +{
>> +  return TRUE;
>> +}
> 
> Please add function comment header for it, otherwise some coding style 
> tool may report error.
> 
>> +
>> +/**
>> +  Internal implementation of CRC32. Depending on the execution context
>> +  (standalone 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
>> +  )
>> +{
>> +  return CalculateCrc32 (Buffer, Length);
>> +}
> 
> Please add function comment header for it, otherwise some coding style 
> tool may report error.
> 
>> +
>> +VOID
>> +FtwNotifySmmReady (
>> +  VOID
>> +  )
>> +{
>> +}
> 
> Please add function comment header for it, otherwise some coding style 
> tool may report error.
> 
> Thanks,
> Star
> 
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +StandaloneMmFaultTolerantWriteInitialize (
>> +  IN EFI_HANDLE            ImageHandle,
>> +  IN EFI_MM_SYSTEM_TABLE   *MmSystemTable
>> +  )
>> +{
>> +  return MmFaultTolerantWriteInitialize ();
>> +}
>> diff --git 
>> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf 
>> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf 
>>
>> new file mode 100644
>> index 000000000000..99bd62ad5ceb
>> --- /dev/null
>> +++ 
>> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf 
>>
>> @@ -0,0 +1,90 @@
>> + ## @file
>> +#   Fault Tolerant Write Smm Driver.
>> +#
>> +#   This driver installs SMM Fault Tolerant Write (FTW) protocol, 
>> which provides fault
>> +#   tolerant write capability in SMM environment for block devices. 
>> Its implementation
>> +#   depends on the full functionality SMM FVB protocol that support 
>> read, write/erase
>> +#   flash access.
>> +#
>> +# Copyright (c) 2010 - 2018, Intel Corporation. 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.
>> +#
>> +##
>> +
>> +[Defines]
>> +  INF_VERSION                    = 0x0001001A
>> +  BASE_NAME                      = FaultTolerantWriteStandaloneMm
>> +  FILE_GUID                      = 3aade4ec-63cc-4a48-a928-5a374dd463eb
>> +  MODULE_TYPE                    = MM_STANDALONE
>> +  VERSION_STRING                 = 1.0
>> +  PI_SPECIFICATION_VERSION       = 0x00010032
>> +  ENTRY_POINT                    = 
>> StandaloneMmFaultTolerantWriteInitialize
>> +
>> +#
>> +# The following information is for reference only and not required by 
>> the build tools.
>> +#
>> +#  VALID_ARCHITECTURES           = AARCH64
>> +#
>> +
>> +[Sources]
>> +  FtwMisc.c
>> +  UpdateWorkingBlock.c
>> +  FaultTolerantWrite.c
>> +  FaultTolerantWriteStandaloneMm.c
>> +  FaultTolerantWriteSmm.c
>> +  FaultTolerantWrite.h
>> +  FaultTolerantWriteSmmCommon.h
>> +
>> +[Packages]
>> +  MdePkg/MdePkg.dec
>> +  MdeModulePkg/MdeModulePkg.dec
>> +  StandaloneMmPkg/StandaloneMmPkg.dec
>> +
>> +[LibraryClasses]
>> +  BaseLib
>> +  BaseMemoryLib
>> +  DebugLib
>> +  MemoryAllocationLib
>> +  MmServicesTableLib
>> +  PcdLib
>> +  ReportStatusCodeLib
>> +  StandaloneMmDriverEntryPoint
>> +
>> +[Guids]
>> +  #
>> +  # Signature in EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER
>> +  #
>> +  ## CONSUMES           ## GUID
>> +  ## PRODUCES           ## GUID
>> +  gEdkiiWorkingBlockSignatureGuid
>> +
>> +[Protocols]
>> +  gEfiSmmSwapAddressRangeProtocolGuid | 
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable  ## 
>> SOMETIMES_CONSUMES
>> +  ## NOTIFY
>> +  ## CONSUMES
>> +  gEfiSmmFirmwareVolumeBlockProtocolGuid
>> +  ## PRODUCES
>> +  ## UNDEFINED # SmiHandlerRegister
>> +  gEfiSmmFaultTolerantWriteProtocolGuid
>> +  gEfiMmEndOfDxeProtocolGuid                       ## CONSUMES
>> +
>> +[FeaturePcd]
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable    ## CONSUMES
>> +
>> +[Pcd]
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase    
>> ## SOMETIMES_CONSUMES
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64  
>> ## CONSUMES
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    
>> ## CONSUMES
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase      
>> ## SOMETIMES_CONSUMES
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    
>> ## CONSUMES
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      
>> ## CONSUMES
>> +
>> +[Depex]
>> +  TRUE
>> +
>>
Ard Biesheuvel Jan. 10, 2019, 7:33 a.m. UTC | #6
On Thu, 10 Jan 2019 at 08:30, Zeng, Star <star.zeng@intel.com> wrote:
>

> Hi Ard,

>

> Another minor feedback.

>

> On 2019/1/10 14:47, Zeng, Star wrote:

> > Hi Ard,

> >

> > Some minor feedback added inline.

> >

> > On 2019/1/4 2:28, Ard Biesheuvel wrote:

> >> Implement a new version of the fault tolerant write driver that can

> >> be used in the context of a standalone MM implementation.

> >>

> >> Contributed-under: TianoCore Contribution Agreement 1.1

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

> >> ---

> >>

> >> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c

> >> | 70 +++++++++++++++

> >>

> >> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf

> >> | 90 ++++++++++++++++++++

> >>   2 files changed, 160 insertions(+)

>

> Please add it into MdeModulePkg.dsc for package build verification.

>


Hello Star,

Thanks for all the feedback. I will respond in more detail later.

However, to the point raised here: it is not possible to add these
drivers to MdeModulePkg.dsc unless we add a dummy implementation of
StandaloneMmDriverEntryPoint to MdeModulePkg. Do you think we should
do that?


> >>

> >> diff --git

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

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

> >>

> >> new file mode 100644

> >> index 000000000000..b6fbf6c64f8a

> >> --- /dev/null

> >> +++

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

> >>

> >> @@ -0,0 +1,70 @@

> >> +/** @file

> >> +

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

> >> +

> >> +Copyright (c) 2010 - 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

> >> +  )

> >> +{

> >> +  return TRUE;

> >> +}

> >

> > Please add function comment header for it, otherwise some coding style

> > tool may report error.

> >

> >> +

> >> +/**

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

> >> +  (standalone 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

> >> +  )

> >> +{

> >> +  return CalculateCrc32 (Buffer, Length);

> >> +}

> >

> > Please add function comment header for it, otherwise some coding style

> > tool may report error.

> >

> >> +

> >> +VOID

> >> +FtwNotifySmmReady (

> >> +  VOID

> >> +  )

> >> +{

> >> +}

> >

> > Please add function comment header for it, otherwise some coding style

> > tool may report error.

> >

> > Thanks,

> > Star

> >

> >> +

> >> +EFI_STATUS

> >> +EFIAPI

> >> +StandaloneMmFaultTolerantWriteInitialize (

> >> +  IN EFI_HANDLE            ImageHandle,

> >> +  IN EFI_MM_SYSTEM_TABLE   *MmSystemTable

> >> +  )

> >> +{

> >> +  return MmFaultTolerantWriteInitialize ();

> >> +}

> >> diff --git

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

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

> >>

> >> new file mode 100644

> >> index 000000000000..99bd62ad5ceb

> >> --- /dev/null

> >> +++

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

> >>

> >> @@ -0,0 +1,90 @@

> >> + ## @file

> >> +#   Fault Tolerant Write Smm Driver.

> >> +#

> >> +#   This driver installs SMM Fault Tolerant Write (FTW) protocol,

> >> which provides fault

> >> +#   tolerant write capability in SMM environment for block devices.

> >> Its implementation

> >> +#   depends on the full functionality SMM FVB protocol that support

> >> read, write/erase

> >> +#   flash access.

> >> +#

> >> +# Copyright (c) 2010 - 2018, Intel Corporation. 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.

> >> +#

> >> +##

> >> +

> >> +[Defines]

> >> +  INF_VERSION                    = 0x0001001A

> >> +  BASE_NAME                      = FaultTolerantWriteStandaloneMm

> >> +  FILE_GUID                      = 3aade4ec-63cc-4a48-a928-5a374dd463eb

> >> +  MODULE_TYPE                    = MM_STANDALONE

> >> +  VERSION_STRING                 = 1.0

> >> +  PI_SPECIFICATION_VERSION       = 0x00010032

> >> +  ENTRY_POINT                    =

> >> StandaloneMmFaultTolerantWriteInitialize

> >> +

> >> +#

> >> +# The following information is for reference only and not required by

> >> the build tools.

> >> +#

> >> +#  VALID_ARCHITECTURES           = AARCH64

> >> +#

> >> +

> >> +[Sources]

> >> +  FtwMisc.c

> >> +  UpdateWorkingBlock.c

> >> +  FaultTolerantWrite.c

> >> +  FaultTolerantWriteStandaloneMm.c

> >> +  FaultTolerantWriteSmm.c

> >> +  FaultTolerantWrite.h

> >> +  FaultTolerantWriteSmmCommon.h

> >> +

> >> +[Packages]

> >> +  MdePkg/MdePkg.dec

> >> +  MdeModulePkg/MdeModulePkg.dec

> >> +  StandaloneMmPkg/StandaloneMmPkg.dec

> >> +

> >> +[LibraryClasses]

> >> +  BaseLib

> >> +  BaseMemoryLib

> >> +  DebugLib

> >> +  MemoryAllocationLib

> >> +  MmServicesTableLib

> >> +  PcdLib

> >> +  ReportStatusCodeLib

> >> +  StandaloneMmDriverEntryPoint

> >> +

> >> +[Guids]

> >> +  #

> >> +  # Signature in EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER

> >> +  #

> >> +  ## CONSUMES           ## GUID

> >> +  ## PRODUCES           ## GUID

> >> +  gEdkiiWorkingBlockSignatureGuid

> >> +

> >> +[Protocols]

> >> +  gEfiSmmSwapAddressRangeProtocolGuid |

> >> gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable  ##

> >> SOMETIMES_CONSUMES

> >> +  ## NOTIFY

> >> +  ## CONSUMES

> >> +  gEfiSmmFirmwareVolumeBlockProtocolGuid

> >> +  ## PRODUCES

> >> +  ## UNDEFINED # SmiHandlerRegister

> >> +  gEfiSmmFaultTolerantWriteProtocolGuid

> >> +  gEfiMmEndOfDxeProtocolGuid                       ## CONSUMES

> >> +

> >> +[FeaturePcd]

> >> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable    ## CONSUMES

> >> +

> >> +[Pcd]

> >> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase

> >> ## SOMETIMES_CONSUMES

> >> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64

> >> ## CONSUMES

> >> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize

> >> ## CONSUMES

> >> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase

> >> ## SOMETIMES_CONSUMES

> >> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64

> >> ## CONSUMES

> >> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize

> >> ## CONSUMES

> >> +

> >> +[Depex]

> >> +  TRUE

> >> +

> >>

>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Zeng, Star Jan. 10, 2019, 7:59 a.m. UTC | #7
On 2019/1/10 15:33, Ard Biesheuvel wrote:
> On Thu, 10 Jan 2019 at 08:30, Zeng, Star <star.zeng@intel.com> wrote:

>>

>> Hi Ard,

>>

>> Another minor feedback.

>>

>> On 2019/1/10 14:47, Zeng, Star wrote:

>>> Hi Ard,

>>>

>>> Some minor feedback added inline.

>>>

>>> On 2019/1/4 2:28, Ard Biesheuvel wrote:

>>>> Implement a new version of the fault tolerant write driver that can

>>>> be used in the context of a standalone MM implementation.

>>>>

>>>> Contributed-under: TianoCore Contribution Agreement 1.1

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

>>>> ---

>>>>

>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c

>>>> | 70 +++++++++++++++

>>>>

>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf

>>>> | 90 ++++++++++++++++++++

>>>>    2 files changed, 160 insertions(+)

>>

>> Please add it into MdeModulePkg.dsc for package build verification.

>>

> 

> Hello Star,

> 

> Thanks for all the feedback. I will respond in more detail later.

> 

> However, to the point raised here: it is not possible to add these

> drivers to MdeModulePkg.dsc unless we add a dummy implementation of

> StandaloneMmDriverEntryPoint to MdeModulePkg. Do you think we should

> do that?


Oh, good information.
To have full code building coverage for the package, personally I think 
we can move StandaloneMmDriverEntryPoint library class and instance into 
MdePkg, and even the MmServicesTableLib for MM_STANDALONE, they should 
be generic enough.

I do not want to block this patch set because of this. So let's discuss 
this in parallel as separated topic.

Mike, Liming, Laszlo, Jian and Hao,\
What's your opinion?


Thanks,
Star

> 

> 

>>>>

>>>> diff --git

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

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

>>>>

>>>> new file mode 100644

>>>> index 000000000000..b6fbf6c64f8a

>>>> --- /dev/null

>>>> +++

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

>>>>

>>>> @@ -0,0 +1,70 @@

>>>> +/** @file

>>>> +

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

>>>> +

>>>> +Copyright (c) 2010 - 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

>>>> +  )

>>>> +{

>>>> +  return TRUE;

>>>> +}

>>>

>>> Please add function comment header for it, otherwise some coding style

>>> tool may report error.

>>>

>>>> +

>>>> +/**

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

>>>> +  (standalone 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

>>>> +  )

>>>> +{

>>>> +  return CalculateCrc32 (Buffer, Length);

>>>> +}

>>>

>>> Please add function comment header for it, otherwise some coding style

>>> tool may report error.

>>>

>>>> +

>>>> +VOID

>>>> +FtwNotifySmmReady (

>>>> +  VOID

>>>> +  )

>>>> +{

>>>> +}

>>>

>>> Please add function comment header for it, otherwise some coding style

>>> tool may report error.

>>>

>>> Thanks,

>>> Star

>>>

>>>> +

>>>> +EFI_STATUS

>>>> +EFIAPI

>>>> +StandaloneMmFaultTolerantWriteInitialize (

>>>> +  IN EFI_HANDLE            ImageHandle,

>>>> +  IN EFI_MM_SYSTEM_TABLE   *MmSystemTable

>>>> +  )

>>>> +{

>>>> +  return MmFaultTolerantWriteInitialize ();

>>>> +}

>>>> diff --git

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

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

>>>>

>>>> new file mode 100644

>>>> index 000000000000..99bd62ad5ceb

>>>> --- /dev/null

>>>> +++

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

>>>>

>>>> @@ -0,0 +1,90 @@

>>>> + ## @file

>>>> +#   Fault Tolerant Write Smm Driver.

>>>> +#

>>>> +#   This driver installs SMM Fault Tolerant Write (FTW) protocol,

>>>> which provides fault

>>>> +#   tolerant write capability in SMM environment for block devices.

>>>> Its implementation

>>>> +#   depends on the full functionality SMM FVB protocol that support

>>>> read, write/erase

>>>> +#   flash access.

>>>> +#

>>>> +# Copyright (c) 2010 - 2018, Intel Corporation. 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.

>>>> +#

>>>> +##

>>>> +

>>>> +[Defines]

>>>> +  INF_VERSION                    = 0x0001001A

>>>> +  BASE_NAME                      = FaultTolerantWriteStandaloneMm

>>>> +  FILE_GUID                      = 3aade4ec-63cc-4a48-a928-5a374dd463eb

>>>> +  MODULE_TYPE                    = MM_STANDALONE

>>>> +  VERSION_STRING                 = 1.0

>>>> +  PI_SPECIFICATION_VERSION       = 0x00010032

>>>> +  ENTRY_POINT                    =

>>>> StandaloneMmFaultTolerantWriteInitialize

>>>> +

>>>> +#

>>>> +# The following information is for reference only and not required by

>>>> the build tools.

>>>> +#

>>>> +#  VALID_ARCHITECTURES           = AARCH64

>>>> +#

>>>> +

>>>> +[Sources]

>>>> +  FtwMisc.c

>>>> +  UpdateWorkingBlock.c

>>>> +  FaultTolerantWrite.c

>>>> +  FaultTolerantWriteStandaloneMm.c

>>>> +  FaultTolerantWriteSmm.c

>>>> +  FaultTolerantWrite.h

>>>> +  FaultTolerantWriteSmmCommon.h

>>>> +

>>>> +[Packages]

>>>> +  MdePkg/MdePkg.dec

>>>> +  MdeModulePkg/MdeModulePkg.dec

>>>> +  StandaloneMmPkg/StandaloneMmPkg.dec

>>>> +

>>>> +[LibraryClasses]

>>>> +  BaseLib

>>>> +  BaseMemoryLib

>>>> +  DebugLib

>>>> +  MemoryAllocationLib

>>>> +  MmServicesTableLib

>>>> +  PcdLib

>>>> +  ReportStatusCodeLib

>>>> +  StandaloneMmDriverEntryPoint

>>>> +

>>>> +[Guids]

>>>> +  #

>>>> +  # Signature in EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER

>>>> +  #

>>>> +  ## CONSUMES           ## GUID

>>>> +  ## PRODUCES           ## GUID

>>>> +  gEdkiiWorkingBlockSignatureGuid

>>>> +

>>>> +[Protocols]

>>>> +  gEfiSmmSwapAddressRangeProtocolGuid |

>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable  ##

>>>> SOMETIMES_CONSUMES

>>>> +  ## NOTIFY

>>>> +  ## CONSUMES

>>>> +  gEfiSmmFirmwareVolumeBlockProtocolGuid

>>>> +  ## PRODUCES

>>>> +  ## UNDEFINED # SmiHandlerRegister

>>>> +  gEfiSmmFaultTolerantWriteProtocolGuid

>>>> +  gEfiMmEndOfDxeProtocolGuid                       ## CONSUMES

>>>> +

>>>> +[FeaturePcd]

>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable    ## CONSUMES

>>>> +

>>>> +[Pcd]

>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase

>>>> ## SOMETIMES_CONSUMES

>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64

>>>> ## CONSUMES

>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize

>>>> ## CONSUMES

>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase

>>>> ## SOMETIMES_CONSUMES

>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64

>>>> ## CONSUMES

>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize

>>>> ## CONSUMES

>>>> +

>>>> +[Depex]

>>>> +  TRUE

>>>> +

>>>>

>>


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Wang, Jian J Jan. 10, 2019, 12:28 p.m. UTC | #8
Star,

I think moving it to MdePkg would be better, just like UefiDriverEntryPoint. A dummy
one may be not necessary.

Regards,
Jian


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

> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Zeng,

> Star

> Sent: Thursday, January 10, 2019 4:00 PM

> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> Cc: Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org; Gao, Liming

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

> Laszlo Ersek <lersek@redhat.com>; Zeng, Star <star.zeng@intel.com>

> Subject: Re: [edk2] [PATCH 4/6] MdeModulePkg/FaultTolerantWriteDxe:

> implement standalone MM version

> 

> On 2019/1/10 15:33, Ard Biesheuvel wrote:

> > On Thu, 10 Jan 2019 at 08:30, Zeng, Star <star.zeng@intel.com> wrote:

> >>

> >> Hi Ard,

> >>

> >> Another minor feedback.

> >>

> >> On 2019/1/10 14:47, Zeng, Star wrote:

> >>> Hi Ard,

> >>>

> >>> Some minor feedback added inline.

> >>>

> >>> On 2019/1/4 2:28, Ard Biesheuvel wrote:

> >>>> Implement a new version of the fault tolerant write driver that can

> >>>> be used in the context of a standalone MM implementation.

> >>>>

> >>>> Contributed-under: TianoCore Contribution Agreement 1.1

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

> >>>> ---

> >>>>

> >>>>

> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandalon

> eMm.c

> >>>> | 70 +++++++++++++++

> >>>>

> >>>>

> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandalon

> eMm.inf

> >>>> | 90 ++++++++++++++++++++

> >>>>    2 files changed, 160 insertions(+)

> >>

> >> Please add it into MdeModulePkg.dsc for package build verification.

> >>

> >

> > Hello Star,

> >

> > Thanks for all the feedback. I will respond in more detail later.

> >

> > However, to the point raised here: it is not possible to add these

> > drivers to MdeModulePkg.dsc unless we add a dummy implementation of

> > StandaloneMmDriverEntryPoint to MdeModulePkg. Do you think we should

> > do that?

> 

> Oh, good information.

> To have full code building coverage for the package, personally I think

> we can move StandaloneMmDriverEntryPoint library class and instance into

> MdePkg, and even the MmServicesTableLib for MM_STANDALONE, they should

> be generic enough.

> 

> I do not want to block this patch set because of this. So let's discuss

> this in parallel as separated topic.

> 

> Mike, Liming, Laszlo, Jian and Hao,\

> What's your opinion?

> 

> 

> Thanks,

> Star

> 

> >

> >

> >>>>

> >>>> diff --git

> >>>>

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

> oneMm.c

> >>>>

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

> oneMm.c

> >>>>

> >>>> new file mode 100644

> >>>> index 000000000000..b6fbf6c64f8a

> >>>> --- /dev/null

> >>>> +++

> >>>>

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

> oneMm.c

> >>>>

> >>>> @@ -0,0 +1,70 @@

> >>>> +/** @file

> >>>> +

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

> MM

> >>>> +

> >>>> +Copyright (c) 2010 - 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

> >>>> +  )

> >>>> +{

> >>>> +  return TRUE;

> >>>> +}

> >>>

> >>> Please add function comment header for it, otherwise some coding style

> >>> tool may report error.

> >>>

> >>>> +

> >>>> +/**

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

> >>>> +  (standalone 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

> >>>> +  )

> >>>> +{

> >>>> +  return CalculateCrc32 (Buffer, Length);

> >>>> +}

> >>>

> >>> Please add function comment header for it, otherwise some coding style

> >>> tool may report error.

> >>>

> >>>> +

> >>>> +VOID

> >>>> +FtwNotifySmmReady (

> >>>> +  VOID

> >>>> +  )

> >>>> +{

> >>>> +}

> >>>

> >>> Please add function comment header for it, otherwise some coding style

> >>> tool may report error.

> >>>

> >>> Thanks,

> >>> Star

> >>>

> >>>> +

> >>>> +EFI_STATUS

> >>>> +EFIAPI

> >>>> +StandaloneMmFaultTolerantWriteInitialize (

> >>>> +  IN EFI_HANDLE            ImageHandle,

> >>>> +  IN EFI_MM_SYSTEM_TABLE   *MmSystemTable

> >>>> +  )

> >>>> +{

> >>>> +  return MmFaultTolerantWriteInitialize ();

> >>>> +}

> >>>> diff --git

> >>>>

> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

> oneMm.inf

> >>>>

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

> oneMm.inf

> >>>>

> >>>> new file mode 100644

> >>>> index 000000000000..99bd62ad5ceb

> >>>> --- /dev/null

> >>>> +++

> >>>>

> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal

> oneMm.inf

> >>>>

> >>>> @@ -0,0 +1,90 @@

> >>>> + ## @file

> >>>> +#   Fault Tolerant Write Smm Driver.

> >>>> +#

> >>>> +#   This driver installs SMM Fault Tolerant Write (FTW) protocol,

> >>>> which provides fault

> >>>> +#   tolerant write capability in SMM environment for block devices.

> >>>> Its implementation

> >>>> +#   depends on the full functionality SMM FVB protocol that support

> >>>> read, write/erase

> >>>> +#   flash access.

> >>>> +#

> >>>> +# Copyright (c) 2010 - 2018, Intel Corporation. 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.

> >>>> +#

> >>>> +##

> >>>> +

> >>>> +[Defines]

> >>>> +  INF_VERSION                    = 0x0001001A

> >>>> +  BASE_NAME                      = FaultTolerantWriteStandaloneMm

> >>>> +  FILE_GUID                      = 3aade4ec-63cc-4a48-a928-5a374dd463eb

> >>>> +  MODULE_TYPE                    = MM_STANDALONE

> >>>> +  VERSION_STRING                 = 1.0

> >>>> +  PI_SPECIFICATION_VERSION       = 0x00010032

> >>>> +  ENTRY_POINT                    =

> >>>> StandaloneMmFaultTolerantWriteInitialize

> >>>> +

> >>>> +#

> >>>> +# The following information is for reference only and not required by

> >>>> the build tools.

> >>>> +#

> >>>> +#  VALID_ARCHITECTURES           = AARCH64

> >>>> +#

> >>>> +

> >>>> +[Sources]

> >>>> +  FtwMisc.c

> >>>> +  UpdateWorkingBlock.c

> >>>> +  FaultTolerantWrite.c

> >>>> +  FaultTolerantWriteStandaloneMm.c

> >>>> +  FaultTolerantWriteSmm.c

> >>>> +  FaultTolerantWrite.h

> >>>> +  FaultTolerantWriteSmmCommon.h

> >>>> +

> >>>> +[Packages]

> >>>> +  MdePkg/MdePkg.dec

> >>>> +  MdeModulePkg/MdeModulePkg.dec

> >>>> +  StandaloneMmPkg/StandaloneMmPkg.dec

> >>>> +

> >>>> +[LibraryClasses]

> >>>> +  BaseLib

> >>>> +  BaseMemoryLib

> >>>> +  DebugLib

> >>>> +  MemoryAllocationLib

> >>>> +  MmServicesTableLib

> >>>> +  PcdLib

> >>>> +  ReportStatusCodeLib

> >>>> +  StandaloneMmDriverEntryPoint

> >>>> +

> >>>> +[Guids]

> >>>> +  #

> >>>> +  # Signature in EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER

> >>>> +  #

> >>>> +  ## CONSUMES           ## GUID

> >>>> +  ## PRODUCES           ## GUID

> >>>> +  gEdkiiWorkingBlockSignatureGuid

> >>>> +

> >>>> +[Protocols]

> >>>> +  gEfiSmmSwapAddressRangeProtocolGuid |

> >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable  ##

> >>>> SOMETIMES_CONSUMES

> >>>> +  ## NOTIFY

> >>>> +  ## CONSUMES

> >>>> +  gEfiSmmFirmwareVolumeBlockProtocolGuid

> >>>> +  ## PRODUCES

> >>>> +  ## UNDEFINED # SmiHandlerRegister

> >>>> +  gEfiSmmFaultTolerantWriteProtocolGuid

> >>>> +  gEfiMmEndOfDxeProtocolGuid                       ## CONSUMES

> >>>> +

> >>>> +[FeaturePcd]

> >>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable    ##

> CONSUMES

> >>>> +

> >>>> +[Pcd]

> >>>> +

> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase

> >>>> ## SOMETIMES_CONSUMES

> >>>> +

> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64

> >>>> ## CONSUMES

> >>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize

> >>>> ## CONSUMES

> >>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase

> >>>> ## SOMETIMES_CONSUMES

> >>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64

> >>>> ## CONSUMES

> >>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize

> >>>> ## CONSUMES

> >>>> +

> >>>> +[Depex]

> >>>> +  TRUE

> >>>> +

> >>>>

> >>

> 

> _______________________________________________

> edk2-devel mailing list

> edk2-devel@lists.01.org

> https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Jan. 10, 2019, 1:03 p.m. UTC | #9
On 01/10/19 08:59, Zeng, Star wrote:
> On 2019/1/10 15:33, Ard Biesheuvel wrote:
>> On Thu, 10 Jan 2019 at 08:30, Zeng, Star <star.zeng@intel.com> wrote:
>>>
>>> Hi Ard,
>>>
>>> Another minor feedback.
>>>
>>> On 2019/1/10 14:47, Zeng, Star wrote:
>>>> Hi Ard,
>>>>
>>>> Some minor feedback added inline.
>>>>
>>>> On 2019/1/4 2:28, Ard Biesheuvel wrote:
>>>>> Implement a new version of the fault tolerant write driver that can
>>>>> be used in the context of a standalone MM implementation.
>>>>>
>>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>> ---
>>>>>
>>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c
>>>>>
>>>>> | 70 +++++++++++++++
>>>>>
>>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
>>>>>
>>>>> | 90 ++++++++++++++++++++
>>>>>    2 files changed, 160 insertions(+)
>>>
>>> Please add it into MdeModulePkg.dsc for package build verification.
>>>
>>
>> Hello Star,
>>
>> Thanks for all the feedback. I will respond in more detail later.
>>
>> However, to the point raised here: it is not possible to add these
>> drivers to MdeModulePkg.dsc unless we add a dummy implementation of
>> StandaloneMmDriverEntryPoint to MdeModulePkg. Do you think we should
>> do that?
> 
> Oh, good information.
> To have full code building coverage for the package, personally I think
> we can move StandaloneMmDriverEntryPoint library class and instance into
> MdePkg, and even the MmServicesTableLib for MM_STANDALONE, they should
> be generic enough.
> 
> I do not want to block this patch set because of this. So let's discuss
> this in parallel as separated topic.
> 
> Mike, Liming, Laszlo, Jian and Hao,\
> What's your opinion?

It should be possible to build all library instances in a central
Package (well, all Packages really), using the Package's DSC file. To my
understanding, libraries built like this are not expected to be used in
actual (shipped) drivers / applications, nor is their indiscriminate
distribution (as LIBs) expected. For example, shipping a BaseXxxLibNull
library instance in binary form seems quite useless.

With that in mind, I think a Null instance for the entry point in
question makes sense, under MdeModulePkg.

Thanks
Laszlo


> 
> 
> Thanks,
> Star
> 
>>
>>
>>>>>
>>>>> diff --git
>>>>> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c
>>>>>
>>>>> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c
>>>>>
>>>>>
>>>>> new file mode 100644
>>>>> index 000000000000..b6fbf6c64f8a
>>>>> --- /dev/null
>>>>> +++
>>>>> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c
>>>>>
>>>>>
>>>>> @@ -0,0 +1,70 @@
>>>>> +/** @file
>>>>> +
>>>>> +  Parts of the SMM/MM implementation that are specific to
>>>>> standalone MM
>>>>> +
>>>>> +Copyright (c) 2010 - 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
>>>>> +  )
>>>>> +{
>>>>> +  return TRUE;
>>>>> +}
>>>>
>>>> Please add function comment header for it, otherwise some coding style
>>>> tool may report error.
>>>>
>>>>> +
>>>>> +/**
>>>>> +  Internal implementation of CRC32. Depending on the execution
>>>>> context
>>>>> +  (standalone 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
>>>>> +  )
>>>>> +{
>>>>> +  return CalculateCrc32 (Buffer, Length);
>>>>> +}
>>>>
>>>> Please add function comment header for it, otherwise some coding style
>>>> tool may report error.
>>>>
>>>>> +
>>>>> +VOID
>>>>> +FtwNotifySmmReady (
>>>>> +  VOID
>>>>> +  )
>>>>> +{
>>>>> +}
>>>>
>>>> Please add function comment header for it, otherwise some coding style
>>>> tool may report error.
>>>>
>>>> Thanks,
>>>> Star
>>>>
>>>>> +
>>>>> +EFI_STATUS
>>>>> +EFIAPI
>>>>> +StandaloneMmFaultTolerantWriteInitialize (
>>>>> +  IN EFI_HANDLE            ImageHandle,
>>>>> +  IN EFI_MM_SYSTEM_TABLE   *MmSystemTable
>>>>> +  )
>>>>> +{
>>>>> +  return MmFaultTolerantWriteInitialize ();
>>>>> +}
>>>>> diff --git
>>>>> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
>>>>>
>>>>> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
>>>>>
>>>>>
>>>>> new file mode 100644
>>>>> index 000000000000..99bd62ad5ceb
>>>>> --- /dev/null
>>>>> +++
>>>>> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
>>>>>
>>>>>
>>>>> @@ -0,0 +1,90 @@
>>>>> + ## @file
>>>>> +#   Fault Tolerant Write Smm Driver.
>>>>> +#
>>>>> +#   This driver installs SMM Fault Tolerant Write (FTW) protocol,
>>>>> which provides fault
>>>>> +#   tolerant write capability in SMM environment for block devices.
>>>>> Its implementation
>>>>> +#   depends on the full functionality SMM FVB protocol that support
>>>>> read, write/erase
>>>>> +#   flash access.
>>>>> +#
>>>>> +# Copyright (c) 2010 - 2018, Intel Corporation. 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.
>>>>> +#
>>>>> +##
>>>>> +
>>>>> +[Defines]
>>>>> +  INF_VERSION                    = 0x0001001A
>>>>> +  BASE_NAME                      = FaultTolerantWriteStandaloneMm
>>>>> +  FILE_GUID                      =
>>>>> 3aade4ec-63cc-4a48-a928-5a374dd463eb
>>>>> +  MODULE_TYPE                    = MM_STANDALONE
>>>>> +  VERSION_STRING                 = 1.0
>>>>> +  PI_SPECIFICATION_VERSION       = 0x00010032
>>>>> +  ENTRY_POINT                    =
>>>>> StandaloneMmFaultTolerantWriteInitialize
>>>>> +
>>>>> +#
>>>>> +# The following information is for reference only and not required by
>>>>> the build tools.
>>>>> +#
>>>>> +#  VALID_ARCHITECTURES           = AARCH64
>>>>> +#
>>>>> +
>>>>> +[Sources]
>>>>> +  FtwMisc.c
>>>>> +  UpdateWorkingBlock.c
>>>>> +  FaultTolerantWrite.c
>>>>> +  FaultTolerantWriteStandaloneMm.c
>>>>> +  FaultTolerantWriteSmm.c
>>>>> +  FaultTolerantWrite.h
>>>>> +  FaultTolerantWriteSmmCommon.h
>>>>> +
>>>>> +[Packages]
>>>>> +  MdePkg/MdePkg.dec
>>>>> +  MdeModulePkg/MdeModulePkg.dec
>>>>> +  StandaloneMmPkg/StandaloneMmPkg.dec
>>>>> +
>>>>> +[LibraryClasses]
>>>>> +  BaseLib
>>>>> +  BaseMemoryLib
>>>>> +  DebugLib
>>>>> +  MemoryAllocationLib
>>>>> +  MmServicesTableLib
>>>>> +  PcdLib
>>>>> +  ReportStatusCodeLib
>>>>> +  StandaloneMmDriverEntryPoint
>>>>> +
>>>>> +[Guids]
>>>>> +  #
>>>>> +  # Signature in EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER
>>>>> +  #
>>>>> +  ## CONSUMES           ## GUID
>>>>> +  ## PRODUCES           ## GUID
>>>>> +  gEdkiiWorkingBlockSignatureGuid
>>>>> +
>>>>> +[Protocols]
>>>>> +  gEfiSmmSwapAddressRangeProtocolGuid |
>>>>> gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable  ##
>>>>> SOMETIMES_CONSUMES
>>>>> +  ## NOTIFY
>>>>> +  ## CONSUMES
>>>>> +  gEfiSmmFirmwareVolumeBlockProtocolGuid
>>>>> +  ## PRODUCES
>>>>> +  ## UNDEFINED # SmiHandlerRegister
>>>>> +  gEfiSmmFaultTolerantWriteProtocolGuid
>>>>> +  gEfiMmEndOfDxeProtocolGuid                       ## CONSUMES
>>>>> +
>>>>> +[FeaturePcd]
>>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable    ##
>>>>> CONSUMES
>>>>> +
>>>>> +[Pcd]
>>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase
>>>>> ## SOMETIMES_CONSUMES
>>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64
>>>>> ## CONSUMES
>>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
>>>>> ## CONSUMES
>>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
>>>>> ## SOMETIMES_CONSUMES
>>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64
>>>>> ## CONSUMES
>>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
>>>>> ## CONSUMES
>>>>> +
>>>>> +[Depex]
>>>>> +  TRUE
>>>>> +
>>>>>
>>>
>
Ard Biesheuvel Jan. 10, 2019, 4:23 p.m. UTC | #10
On Thu, 10 Jan 2019 at 14:03, Laszlo Ersek <lersek@redhat.com> wrote:
>

> On 01/10/19 08:59, Zeng, Star wrote:

> > On 2019/1/10 15:33, Ard Biesheuvel wrote:

> >> On Thu, 10 Jan 2019 at 08:30, Zeng, Star <star.zeng@intel.com> wrote:

> >>>

> >>> Hi Ard,

> >>>

> >>> Another minor feedback.

> >>>

> >>> On 2019/1/10 14:47, Zeng, Star wrote:

> >>>> Hi Ard,

> >>>>

> >>>> Some minor feedback added inline.

> >>>>

> >>>> On 2019/1/4 2:28, Ard Biesheuvel wrote:

> >>>>> Implement a new version of the fault tolerant write driver that can

> >>>>> be used in the context of a standalone MM implementation.

> >>>>>

> >>>>> Contributed-under: TianoCore Contribution Agreement 1.1

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

> >>>>> ---

> >>>>>

> >>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c

> >>>>>

> >>>>> | 70 +++++++++++++++

> >>>>>

> >>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf

> >>>>>

> >>>>> | 90 ++++++++++++++++++++

> >>>>>    2 files changed, 160 insertions(+)

> >>>

> >>> Please add it into MdeModulePkg.dsc for package build verification.

> >>>

> >>

> >> Hello Star,

> >>

> >> Thanks for all the feedback. I will respond in more detail later.

> >>

> >> However, to the point raised here: it is not possible to add these

> >> drivers to MdeModulePkg.dsc unless we add a dummy implementation of

> >> StandaloneMmDriverEntryPoint to MdeModulePkg. Do you think we should

> >> do that?

> >

> > Oh, good information.

> > To have full code building coverage for the package, personally I think

> > we can move StandaloneMmDriverEntryPoint library class and instance into

> > MdePkg, and even the MmServicesTableLib for MM_STANDALONE, they should

> > be generic enough.

> >

> > I do not want to block this patch set because of this. So let's discuss

> > this in parallel as separated topic.

> >

> > Mike, Liming, Laszlo, Jian and Hao,\

> > What's your opinion?

>

> It should be possible to build all library instances in a central

> Package (well, all Packages really), using the Package's DSC file. To my

> understanding, libraries built like this are not expected to be used in

> actual (shipped) drivers / applications, nor is their indiscriminate

> distribution (as LIBs) expected. For example, shipping a BaseXxxLibNull

> library instance in binary form seems quite useless.

>

> With that in mind, I think a Null instance for the entry point in

> question makes sense, under MdeModulePkg.

>


I will look into this a bit deeper next week. I think it makes sense
for the core PI architected pieces to all live in MdePkg rather than
StandaloneMmPkg. For instance, MmServicesTableLib for standalone MM
should live there, MmEntryPoint should live there (and have
traditional and standalone MM implementation) and perhaps some other
core pieces as well.

This may be a slippery slope, so I will dedicate some time to look
into this carefully, at least with the goal to make the
FaultTolerantWrite and Variable driver buildable from within
MdeModulePkg.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Zeng, Star Jan. 11, 2019, 2:18 a.m. UTC | #11
On 2019/1/11 0:23, Ard Biesheuvel wrote:
> On Thu, 10 Jan 2019 at 14:03, Laszlo Ersek <lersek@redhat.com> wrote:

>>

>> On 01/10/19 08:59, Zeng, Star wrote:

>>> On 2019/1/10 15:33, Ard Biesheuvel wrote:

>>>> On Thu, 10 Jan 2019 at 08:30, Zeng, Star <star.zeng@intel.com> wrote:

>>>>>

>>>>> Hi Ard,

>>>>>

>>>>> Another minor feedback.

>>>>>

>>>>> On 2019/1/10 14:47, Zeng, Star wrote:

>>>>>> Hi Ard,

>>>>>>

>>>>>> Some minor feedback added inline.

>>>>>>

>>>>>> On 2019/1/4 2:28, Ard Biesheuvel wrote:

>>>>>>> Implement a new version of the fault tolerant write driver that can

>>>>>>> be used in the context of a standalone MM implementation.

>>>>>>>

>>>>>>> Contributed-under: TianoCore Contribution Agreement 1.1

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

>>>>>>> ---

>>>>>>>

>>>>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c

>>>>>>>

>>>>>>> | 70 +++++++++++++++

>>>>>>>

>>>>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf

>>>>>>>

>>>>>>> | 90 ++++++++++++++++++++

>>>>>>>     2 files changed, 160 insertions(+)

>>>>>

>>>>> Please add it into MdeModulePkg.dsc for package build verification.

>>>>>

>>>>

>>>> Hello Star,

>>>>

>>>> Thanks for all the feedback. I will respond in more detail later.

>>>>

>>>> However, to the point raised here: it is not possible to add these

>>>> drivers to MdeModulePkg.dsc unless we add a dummy implementation of

>>>> StandaloneMmDriverEntryPoint to MdeModulePkg. Do you think we should

>>>> do that?

>>>

>>> Oh, good information.

>>> To have full code building coverage for the package, personally I think

>>> we can move StandaloneMmDriverEntryPoint library class and instance into

>>> MdePkg, and even the MmServicesTableLib for MM_STANDALONE, they should

>>> be generic enough.

>>>

>>> I do not want to block this patch set because of this. So let's discuss

>>> this in parallel as separated topic.

>>>

>>> Mike, Liming, Laszlo, Jian and Hao,\

>>> What's your opinion?

>>

>> It should be possible to build all library instances in a central

>> Package (well, all Packages really), using the Package's DSC file. To my

>> understanding, libraries built like this are not expected to be used in

>> actual (shipped) drivers / applications, nor is their indiscriminate

>> distribution (as LIBs) expected. For example, shipping a BaseXxxLibNull

>> library instance in binary form seems quite useless.

>>

>> With that in mind, I think a Null instance for the entry point in

>> question makes sense, under MdeModulePkg.

>>

> 

> I will look into this a bit deeper next week. I think it makes sense

> for the core PI architected pieces to all live in MdePkg rather than

> StandaloneMmPkg. For instance, MmServicesTableLib for standalone MM

> should live there, MmEntryPoint should live there (and have

> traditional and standalone MM implementation) and perhaps some other

> core pieces as well.

> 

> This may be a slippery slope, so I will dedicate some time to look

> into this carefully, at least with the goal to make the

> FaultTolerantWrite and Variable driver buildable from within

> MdeModulePkg.


Make sense to me. You'd better to submit a bugzilla to track this after 
this patchset is pushed.

Thanks,
Star


> 


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

Patch

diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c
new file mode 100644
index 000000000000..b6fbf6c64f8a
--- /dev/null
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c
@@ -0,0 +1,70 @@ 
+/** @file
+
+  Parts of the SMM/MM implementation that are specific to standalone MM
+
+Copyright (c) 2010 - 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
+  )
+{
+  return TRUE;
+}
+
+/**
+  Internal implementation of CRC32. Depending on the execution context
+  (standalone 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
+  )
+{
+  return CalculateCrc32 (Buffer, Length);
+}
+
+VOID
+FtwNotifySmmReady (
+  VOID
+  )
+{
+}
+
+EFI_STATUS
+EFIAPI
+StandaloneMmFaultTolerantWriteInitialize (
+  IN EFI_HANDLE            ImageHandle,
+  IN EFI_MM_SYSTEM_TABLE   *MmSystemTable
+  )
+{
+  return MmFaultTolerantWriteInitialize ();
+}
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
new file mode 100644
index 000000000000..99bd62ad5ceb
--- /dev/null
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
@@ -0,0 +1,90 @@ 
+ ## @file
+#   Fault Tolerant Write Smm Driver.
+#
+#   This driver installs SMM Fault Tolerant Write (FTW) protocol, which provides fault
+#   tolerant write capability in SMM environment for block devices. Its implementation
+#   depends on the full functionality SMM FVB protocol that support read, write/erase
+#   flash access.
+#
+# Copyright (c) 2010 - 2018, Intel Corporation. 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.
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x0001001A
+  BASE_NAME                      = FaultTolerantWriteStandaloneMm
+  FILE_GUID                      = 3aade4ec-63cc-4a48-a928-5a374dd463eb
+  MODULE_TYPE                    = MM_STANDALONE
+  VERSION_STRING                 = 1.0
+  PI_SPECIFICATION_VERSION       = 0x00010032
+  ENTRY_POINT                    = StandaloneMmFaultTolerantWriteInitialize
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = AARCH64
+#
+
+[Sources]
+  FtwMisc.c
+  UpdateWorkingBlock.c
+  FaultTolerantWrite.c
+  FaultTolerantWriteStandaloneMm.c
+  FaultTolerantWriteSmm.c
+  FaultTolerantWrite.h
+  FaultTolerantWriteSmmCommon.h
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  StandaloneMmPkg/StandaloneMmPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  MemoryAllocationLib
+  MmServicesTableLib
+  PcdLib
+  ReportStatusCodeLib
+  StandaloneMmDriverEntryPoint
+
+[Guids]
+  #
+  # Signature in EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER
+  #
+  ## CONSUMES           ## GUID
+  ## PRODUCES           ## GUID
+  gEdkiiWorkingBlockSignatureGuid
+
+[Protocols]
+  gEfiSmmSwapAddressRangeProtocolGuid | gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable  ## SOMETIMES_CONSUMES
+  ## NOTIFY
+  ## CONSUMES
+  gEfiSmmFirmwareVolumeBlockProtocolGuid
+  ## PRODUCES
+  ## UNDEFINED # SmiHandlerRegister
+  gEfiSmmFaultTolerantWriteProtocolGuid
+  gEfiMmEndOfDxeProtocolGuid                       ## CONSUMES
+
+[FeaturePcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFullFtwServiceEnable    ## CONSUMES
+
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase    ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64  ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase      ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64    ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ## CONSUMES
+
+[Depex]
+  TRUE
+