From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::141; helo=mail-it1-x141.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x141.google.com (mail-it1-x141.google.com [IPv6:2607:f8b0:4864:20::141]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 26B42211B63E9 for ; Wed, 9 Jan 2019 23:33:29 -0800 (PST) Received: by mail-it1-x141.google.com with SMTP id g76so15896250itg.2 for ; Wed, 09 Jan 2019 23:33:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Fdoj4KjddQqBwlO3mTkNkhEEp7JhdehSOrgTGxOOrO4=; b=gy0WcWQ39N3jvmRKg3wYpoczzIijOgFJFWT1GTsA/voift+9PR1QHkOodoRoT8ae68 0eH8RvWaE0MykYn1fRpdythsAZKAbjlM/pbLHgz76UgSgNdxHApElNoXKwTyykBbuDo4 O6p5oWGI6MtrGj0BSZf+U7AgUdC+SvxF27kto= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Fdoj4KjddQqBwlO3mTkNkhEEp7JhdehSOrgTGxOOrO4=; b=VEWSquQjUfjOW/b1SKfGgYH/RHPemRfLIU1aC0gddkPagkfn+3ahCodgcWfW2sXSij 3L+x5bXOWAix9mX6RHmim4Ta3h0fksz1BOP55qIafie7ctl24paKuxN2KIBtYgh51LFF GlhzUohW8JbNHTpG1egwCFPmW0tZvnWJtQo858Zvp+LO3CFR42EdHLW5B2jIzQQqegut YbroDmpyk75yqo+Yb0p+W00bgAnBa9Qrvu7B3kNlhYroGzerPkimOqmbdJRe4qqF9cHu Hl65N563osxgxWF09iA0ScrPIpTscQAz1VH8rfE0+cTe24TZB/uGCLdw93a1DgF2YxbQ dV6w== X-Gm-Message-State: AJcUukcLJXmnLVZSh1yZc0l3XdG+l5aA7jUfOFi8m0x1xvAfqD9we5EE 4NJw+O6kQ4SRXf8MrIuuGx5UbmPJftOvz305HCcRHQ== X-Google-Smtp-Source: ALg8bN6QyaWPNvf2xYZp6JvxFROJu22dgDYFXdAMZdbWj5wZwc+3/WEXG/K/R6oyWNKWG0zgUR/2/odjLtaoNqO/qL4= X-Received: by 2002:a24:710:: with SMTP id f16mr5586259itf.121.1547105608872; Wed, 09 Jan 2019 23:33:28 -0800 (PST) MIME-Version: 1.0 References: <20190103182825.32231-1-ard.biesheuvel@linaro.org> <20190103182825.32231-6-ard.biesheuvel@linaro.org> <4a353237-1191-a831-1b6e-e981b7c59a7d@intel.com> <539a4ec0-eb09-588b-2a24-4b1f450f587b@intel.com> In-Reply-To: <539a4ec0-eb09-588b-2a24-4b1f450f587b@intel.com> From: Ard Biesheuvel Date: Thu, 10 Jan 2019 08:33:17 +0100 Message-ID: To: "Zeng, Star" Cc: "edk2-devel@lists.01.org" , Hao Wu , Michael D Kinney , Laszlo Ersek , Liming Gao Subject: Re: [PATCH 4/6] MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM version X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Jan 2019 07:33:30 -0000 Content-Type: text/plain; charset="UTF-8" On Thu, 10 Jan 2019 at 08:30, Zeng, Star 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 > >> --- > >> > >> 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.
> >> +Copyright (c) 2018, Linaro, Ltd. All rights reserved.
> >> +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 > >> +#include > >> +#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.
> >> +# > >> +# 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 > >> + > >> >