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 <[email protected]> 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 <[email protected]> >>>>> --- >>>>> >>>>> 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 >>>>> + >>>>> >>> > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
