On Mon, 7 Jan 2019 at 14:09, Jagadeesh Ujja <jagadeesh.u...@arm.com> wrote:
>
> In-Reply-To:
>
> Changes since v2:
> -Addressed the comments from Jian Wang
>  - CommonMmServicesLib library implemented in MdePkg.
>  - Picked the Reviewed-by tags from Ard Biesheuvel.
>

Jagadeesh,

I have been very clear about how the PCD approach is not acceptable.
Also, as I have attempted to explain, gMmst and gSmst are not
fundamentally, different, so having a library that encapsulates
choosing between the two for each MMST service is wrong as well.

I am happy to discuss this further, and to explain in more detail if necessary.

However, *please* stop sending new revisions of your series until we
reached an agreement between us and with the MdePkg maintainers. This
way, you are just wasting reviewer bandwidth.

-- 
Ard.


> Changes since v1:
> -Addressed the comments from Liming Gao
>  - StandaloneMmServicesTableLib library implemented in MdePkg.
> - Addressed all the comments from Ard Biesheuvel.
> - For comment from Jian Wang about avoiding if..else, this
>   requires a bit more clarity and so this comment has not
>   been addressed.
> - All the patches in this series can be pulled from
>   https://github.com/jagadeeshujja/edk2.git branch: topics/aarch64_secure_vars
>
> Changes since RFC v4:
> - Addressed all the comments from Liming Gao
>   - Added an additional PCD 'PcdStandaloneMmCodeEnabled' to indicate
>     presence of StandaloneMM support.
>   - MdePkg.dec file updated to include StandaloneMmServiceTableLib and
>     StandaloneMmRuntimeDxe library.
>   - Platform specific changes will be posted in a seperate patchset.
>   - AsmLfence wrapper function is supported for AArch64 platforms.
>   - All the patches in this series can be pulled from
>     https://github.com/jagadeeshujja/edk2 (branch: topics/aarch64_secure_vars)
>
> Changes since RFC v3:
> - Addressed all the comments from Liming Gao
>   - Added a AArch64 implementation of AsmLfence which is a wrapper for
>     MemoryFence. The changes in variable service driver in v3 of this
>     patchset that used MemoryFence instead of AsmLfence have been removed.
>   - Added StandaloneMmServicesTableLib.h and StandaloneMmRuntimeDxe
>     library into MdePkg.
>   - Renamed PcdStandaloneMmEnable as PcdStandaloneMmVariableEnabled and
>     added to in to MdePkg.
>   - Now with above changes, edk2 packages don't need to depend on
>     StandaloneMmPkg/StandaloneMmPkg.dec
> - Addressed comments from Ting Ye
>   - Removed the hacks in the v3 version.
>   - Will relook into the “TimerWrapp.c” file and add a appropriate
>     implementation of this for MM Standalone mode code.
>
> Changes since RFC v2:
> - Added 'Contributed-under' tag, removed Change-ID tag and
>   maintained a single signed-off-by for the all the patches.
>
> Changes since RFC v1:
> - Addressed all the comments from Liming Gao
>   - Removed the use of #ifdef/#else/#endif and used a Pcd instead to
>     select between MM and non-MM paths.
>   - Removed all dependencies on edk2-platforms.
>   - Dropped the use of mMmst and used gSmst instead.
>   - Added a dummy implementation UefiRuntimeServiceTableLib for
>     MM_STANDALONE usage
> - Replaced all uses of AsmLfence with MemoryFence from variable
>   service code.
> - Add a new StandaloneMmRuntimeDxe library to for use by non-MM code.
>
> This patch series extends the existing secure variable service support for
> use with Standalone MM. This is applicable to paltforms that use Standalone
> Management Mode to protect access to non-volatile memory (NOR flash in case
> of these patches) used to store the secure EFI variables.
>
> The first patch pulls in additional libraries from the staging branch of
> StandaloneMmPkg into the edk2's StandaloneMmPkg. The existing secure variable
> service implementation supports only the traditional MM mode and so the rest
> of the patches extends the existing secure variable service support to be
> useable with Standalone MM mode as well.
>
> Jagadeesh Ujja (11):
>   StandaloneMmPkg: Remove MM_STANDALONE LIBRARY_CLASS from
>     StandaloneMmCoreHobLib
>   StandaloneMmPkg: Adding the library packages used by MM_STANDALONE
>     drivers
>   MdeModulePkg: Add a PCD to indicate Standalone MM supports secure
>     variable
>   MdePkg/Include: Add StandaloneMmServicesTableLib library
>   MdePkg/Library: Add CommonMmServicesLib library
>   MdeModulePkg/FaultTolerantWriteDxe: allow reusability as a MM driver
>   MdeModulePkg/Variable/RuntimeDxe: adapt for usability with MM
>     Standalone
>   MdeModulePkg/Variable/RuntimeDxe: adapt as a MM Standalone driver
>   MdeModulePkg/VarCheckLib: allow MM_STANDALONE drivers to use this
>     library
>   ArmPlatformPkg/NorFlashDxe: allow reusability as a MM driver
>   CryptoPkg/BaseCryptLib: allow MM_STANDALONE drivers to use SmmCryptLib
>
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h                             
>                |    4 +-
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf                           
>                |    2 +
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c                          
>                |   96 +-
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c                    
>                | 1339 ++++++++++++++++++++
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf                  
>                |   76 ++
>  CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf                               
>                |    2 +-
>  MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf                             
>                |    5 +-
>  MdeModulePkg/MdeModulePkg.dec                                                
>                |    5 +
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf       
>                |    1 +
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c         
>                |  149 ++-
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf       
>                |    4 +-
>  
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
>              |  102 ++
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c            
>                |   27 +-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c                        
>                |   37 +-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf            
>                |    1 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c                     
>                |  165 ++-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf                   
>                |    2 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c           
>                |   31 +-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf         
>                |    3 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf          
>                |  133 ++
>  MdePkg/Include/Library/CommonMmServicesLibrary.h                             
>                |  131 ++
>  MdePkg/Include/Library/StandaloneMmServicesTableLib.h                        
>                |   43 +
>  MdePkg/Library/CommonMmServicesLibrary/CommonMmServicesLibrary.c             
>                |  224 ++++
>  MdePkg/Library/CommonMmServicesLibrary/CommonMmServicesLibrary.inf           
>                |   42 +
>  MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.c   
>                |   39 +
>  MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf 
>                |   36 +
>  MdePkg/MdePkg.dec                                                            
>                |    4 +
>  StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf    
>                |    2 +-
>  
> StandaloneMmPkg/Library/StandaloneMmHobLib/AArch64/StandaloneMmCoreHobLibInternal.c
>          |   64 +
>  StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c              
>                |  651 ++++++++++
>  StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf            
>                |   48 +
>  
> StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.c
>    |  823 ++++++++++++
>  
> StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
>  |   45 +
>  
> StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.c
>          |   64 +
>  
> StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
>        |   36 +
>  35 files changed, 4257 insertions(+), 179 deletions(-)
>  create mode 100644 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
>  create mode 100644 
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
>  create mode 100644 
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
>  create mode 100644 
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
>  create mode 100644 MdePkg/Include/Library/CommonMmServicesLibrary.h
>  create mode 100644 MdePkg/Include/Library/StandaloneMmServicesTableLib.h
>  create mode 100644 
> MdePkg/Library/CommonMmServicesLibrary/CommonMmServicesLibrary.c
>  create mode 100644 
> MdePkg/Library/CommonMmServicesLibrary/CommonMmServicesLibrary.inf
>  create mode 100644 
> MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.c
>  create mode 100644 
> MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmHobLib/AArch64/StandaloneMmCoreHobLibInternal.c
>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.c
>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.c
>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
>
> --
> 2.7.4
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to