[edk2-devel] [PATCH v1 0/2] BaseTools: Make undefined VFR macro an error

2021-03-02 Thread Daniel Schaefer
See the individual commit descriptions.
I split it up into GCC/CLANG and MSVC commits but feel free to squash
them if you think they belong together.

We found a few bugs and lots of dead code with this in our internal
code-base.
I only tested GCC5, CLANBPDB and VS2015 toolchains. Not 100% sure if
this warnings exists in the old MSVC toolchains.

Cc: Bob Feng 
Cc: Liming Gao 
Cc: Yuwei Chen 
Cc: Derek Lin 

Daniel Schaefer (2):
  BaseTools: Make undefined VFR macro an error (GCC)
  BaseTools: Make undefined VFR macro an error (MSVC)

 BaseTools/Conf/tools_def.template | 50 ++--
 1 file changed, 25 insertions(+), 25 deletions(-)

-- 
2.30.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72325): https://edk2.groups.io/g/devel/message/72325
Mute This Topic: https://groups.io/mt/81020989/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v1 1/2] BaseTools: Make undefined VFR macro an error (GCC)

2021-03-02 Thread Daniel Schaefer
VFR successfully compiles if we forget to include a header that defines
a macro. In that case the HII option was hidden when it shouldn't be
just because the macro was used but not defined.

The behaviour is totally intended by the C/PP standard. When a macro is
undefined it evaluates to 0.
GCC, MSVC and Clang have warnings to catch this type of mistake. With
this commit we enable this warning and make it a compiler error.

Cc: Bob Feng 
Cc: Liming Gao 
Cc: Yuwei Chen 
Cc: Derek Lin 
---
 BaseTools/Conf/tools_def.template | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 933b3160fd2b..728c1d3119e4 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -3,7 +3,7 @@
 #  Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.

 #  Portions copyright (c) 2011 - 2019, ARM Ltd. All rights reserved.

 #  Copyright (c) 2015, Hewlett-Packard Development Company, L.P.

-#  (C) Copyright 2020, Hewlett Packard Enterprise Development LP

+#  (C) Copyright 2020-2021, Hewlett Packard Enterprise Development LP

 #  Copyright (c) Microsoft Corporation

 #

 #  SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -1938,7 +1938,7 @@ DEFINE GCC_AARCH64_ASLDLINK_FLAGS  = 
DEF(GCC_AARCH64_DLINK_FLAGS) -Wl,--entry,Re
 DEFINE GCC_IA32_X64_DLINK_FLAGS= DEF(GCC_IA32_X64_DLINK_COMMON) --entry 
_$(IMAGE_ENTRY_POINT) --file-alignment 0x20 --section-alignment 0x20 -Map 
$(DEST_DIR_DEBUG)/$(BASE_NAME).map

 DEFINE GCC_ASM_FLAGS   = -c -x assembler -imacros AutoGen.h

 DEFINE GCC_PP_FLAGS= -E -x assembler-with-cpp -include 
AutoGen.h

-DEFINE GCC_VFRPP_FLAGS = -x c -E -P -DVFRCOMPILE --include 
$(MODULE_NAME)StrDefs.h

+DEFINE GCC_VFRPP_FLAGS = -x c -E -P -DVFRCOMPILE --include 
$(MODULE_NAME)StrDefs.h -Wundef -Werror

 DEFINE GCC_ASLPP_FLAGS = -x c -E -include AutoGen.h

 DEFINE GCC_ASLCC_FLAGS = -x c

 DEFINE GCC_WINDRES_FLAGS   = -J rc -O coff

-- 
2.30.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72326): https://edk2.groups.io/g/devel/message/72326
Mute This Topic: https://groups.io/mt/81020991/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v1 2/2] BaseTools: Make undefined VFR macro an error (MSVC)

2021-03-02 Thread Daniel Schaefer
VFR successfully compiles if we forget to include a header that defines
a macro. In that case the HII option was hidden when it shouldn't be
just because the macro was used but not defined.

The behaviour is totally intended by the C/PP standard. When a macro is
undefined it evaluates to 0.
GCC, MSVC and Clang have warnings to catch this type of mistake. With
this commit we enable this warning and make it a compiler error.

Cc: Bob Feng 
Cc: Liming Gao 
Cc: Yuwei Chen 
Cc: Derek Lin 
---
 BaseTools/Conf/tools_def.template | 46 ++--
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 728c1d3119e4..56c7bd13f157 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -422,7 +422,7 @@ DEFINE DTC_BIN = ENV(DTC_PREFIX)dtc
 *_VS2008_*_SLINK_FLAGS= /NOLOGO /LTCG

 *_VS2008_*_APP_FLAGS  = /nologo /E /TC

 *_VS2008_*_PP_FLAGS   = /nologo /E /TC /FIAutoGen.h

-*_VS2008_*_VFRPP_FLAGS= /nologo /E /TC /DVFRCOMPILE 
/FI$(MODULE_NAME)StrDefs.h

+*_VS2008_*_VFRPP_FLAGS= /nologo /E /TC /DVFRCOMPILE 
/FI$(MODULE_NAME)StrDefs.h /we4668

 *_VS2008_*_DEPS_FLAGS= DEF(MSFT_DEPS_FLAGS)

 *_VS2008_*_ASM16_PATH = DEF(VS2008_BIN)\ml.exe

 

@@ -518,7 +518,7 @@ NOOPT_VS2008_X64_DLINK_FLAGS  = /NOLOGO /NODEFAULTLIB 
/IGNORE:4001 /OPT:REF /OPT
 *_VS2008_EBC_MAKE_FLAGS  = /nologo

 *_VS2008_EBC_PP_FLAGS= /nologo /E /TC /FIAutoGen.h

 *_VS2008_EBC_CC_FLAGS= /nologo /c /WX /W3 /FIAutoGen.h 
/D$(MODULE_ENTRY_POINT)=$(ARCH_ENTRY_POINT)

-*_VS2008_EBC_VFRPP_FLAGS = /nologo /E /TC /DVFRCOMPILE 
/FI$(MODULE_NAME)StrDefs.h

+*_VS2008_EBC_VFRPP_FLAGS = /nologo /E /TC /DVFRCOMPILE 
/FI$(MODULE_NAME)StrDefs.h /we4668

 *_VS2008_EBC_SLINK_FLAGS = /lib /NOLOGO /MACHINE:EBC

 *_VS2008_EBC_DLINK_FLAGS = "C:\Program Files\Intel\EBC\Lib\EbcLib.lib" 
/NOLOGO /NODEFAULTLIB /MACHINE:EBC /OPT:REF /ENTRY:$(IMAGE_ENTRY_POINT) 
/SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /MAP /ALIGN:32 /DRIVER

 

@@ -538,7 +538,7 @@ NOOPT_VS2008_X64_DLINK_FLAGS  = /NOLOGO /NODEFAULTLIB 
/IGNORE:4001 /OPT:REF /OPT
 *_VS2008x86_*_SLINK_FLAGS = /NOLOGO /LTCG

 *_VS2008x86_*_APP_FLAGS   = /nologo /E /TC

 *_VS2008x86_*_PP_FLAGS= /nologo /E /TC /FIAutoGen.h

-*_VS2008x86_*_VFRPP_FLAGS = /nologo /E /TC /DVFRCOMPILE 
/FI$(MODULE_NAME)StrDefs.h

+*_VS2008x86_*_VFRPP_FLAGS = /nologo /E /TC /DVFRCOMPILE 
/FI$(MODULE_NAME)StrDefs.h /we4668

 *_VS2008x86_*_DEPS_FLAGS  = DEF(MSFT_DEPS_FLAGS)

 *_VS2008x86_*_ASM16_PATH  = DEF(VS2008x86_BIN)\ml.exe

 

@@ -633,7 +633,7 @@ NOOPT_VS2008x86_X64_DLINK_FLAGS= /NOLOGO /NODEFAULTLIB 
/IGNORE:4001 /OPT:REF
 *_VS2008x86_EBC_MAKE_FLAGS  = /nologo

 *_VS2008x86_EBC_PP_FLAGS= /nologo /E /TC /FIAutoGen.h

 *_VS2008x86_EBC_CC_FLAGS= /nologo /c /WX /W3 /FIAutoGen.h 
/D$(MODULE_ENTRY_POINT)=$(ARCH_ENTRY_POINT)

-*_VS2008x86_EBC_VFRPP_FLAGS = /nologo /E /TC /DVFRCOMPILE 
/FI$(MODULE_NAME)StrDefs.h

+*_VS2008x86_EBC_VFRPP_FLAGS = /nologo /E /TC /DVFRCOMPILE 
/FI$(MODULE_NAME)StrDefs.h /we4668

 *_VS2008x86_EBC_SLINK_FLAGS = /lib /NOLOGO /MACHINE:EBC

 *_VS2008x86_EBC_DLINK_FLAGS = "C:\Program Files 
(x86)\Intel\EBC\Lib\EbcLib.lib" /NOLOGO /NODEFAULTLIB /MACHINE:EBC /OPT:REF 
/ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /MAP /ALIGN:32 
/DRIVER

 

@@ -656,7 +656,7 @@ NOOPT_VS2008x86_X64_DLINK_FLAGS= /NOLOGO /NODEFAULTLIB 
/IGNORE:4001 /OPT:REF
 *_VS2010_*_SLINK_FLAGS= /NOLOGO /LTCG

 *_VS2010_*_APP_FLAGS  = /nologo /E /TC

 *_VS2010_*_PP_FLAGS   = /nologo /E /TC /FIAutoGen.h

-*_VS2010_*_VFRPP_FLAGS= /nologo /E /TC /DVFRCOMPILE 
/FI$(MODULE_NAME)StrDefs.h

+*_VS2010_*_VFRPP_FLAGS= /nologo /E /TC /DVFRCOMPILE 
/FI$(MODULE_NAME)StrDefs.h /we4668

 *_VS2010_*_DEPS_FLAGS  = DEF(MSFT_DEPS_FLAGS)

 *_VS2010_*_ASM16_PATH = DEF(VS2010_BIN)\ml.exe

 

@@ -752,7 +752,7 @@ NOOPT_VS2010_X64_DLINK_FLAGS  = /NOLOGO /NODEFAULTLIB 
/IGNORE:4001 /OPT:REF /OPT
 *_VS2010_EBC_MAKE_FLAGS  = /nologo

 *_VS2010_EBC_PP_FLAGS= /nologo /E /TC /FIAutoGen.h

 *_VS2010_EBC_CC_FLAGS= /nologo /c /WX /W3 /FIAutoGen.h 
/D$(MODULE_ENTRY_POINT)=$(ARCH_ENTRY_POINT)

-*_VS2010_EBC_VFRPP_FLAGS = /nologo /E /TC /DVFRCOMPILE 
/FI$(MODULE_NAME)StrDefs.h

+*_VS2010_EBC_VFRPP_FLAGS = /nologo /E /TC /DVFRCOMPILE 
/FI$(MODULE_NAME)StrDefs.h /we4668

 *_VS2010_EBC_SLINK_FLAGS = /lib /NOLOGO /MACHINE:EBC

 *_VS2010_EBC_DLINK_FLAGS = "C:\Program Files\Intel\EBC\Lib\EbcLib.lib" 
/NOLOGO /NODEFAULTLIB /MACHINE:EBC /OPT:REF /ENTRY:$(IMAGE_ENTRY_POINT) 
/SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /MAP /ALIGN:32 /DRIVER

 

@@ -772,7 +772,7 @@ NOOPT_VS2010_X64_DLINK_FLAGS  =

Re: [edk2-devel] [PATCH v3 1/7] MdePkg: MmUnblockMemoryLib: Added definition and null instance

2021-03-02 Thread Laszlo Ersek
On 03/01/21 20:03, Kun Qin wrote:
> Hi Laszlo,
> 
> The library is intended to allow MM modules to access requested areas that 
> are outside of MMRAM. The idea behind this library is to create an MM model 
> that will block access to all non-MMRAM regions except the requested areas 
> for isolation/protection purpose. To resolve your confusion, the agents that 
> are responsible for unblocking memory are the ones that sets up these regions 
> that need to be accessed by MM modules.
> 
> For example:
> 
>   1.  To enable runtime cache feature for variable service, Variable MM 
> module needs to access the allocated runtime buffer, which is set up in 
> VariableSmmRuntimeDxe;
>   2.  For TPM ACPI table to communicate to physical presence handler, the 
> corresponding NVS regions has to be accessible from inside MM, which is set 
> up when assigning NVS region in DXE environment;
> 
> So the library is not necessarily restricted to DXE_RUNTIME drivers, but 
> could be applicable to DXE drivers and even PEI modules as well.

Very nice, so please include this in the commit message.

> 
> Thanks for the suggestion, I will replace the “EFI_” error code with 
> “RETURN_” error codes and add more statements in commit message to make the 
> purpose of this library more explanatory.

Thanks!
Laszlo

> 
> Regards,
> Kun
> 
> From: Laszlo Ersek
> Sent: Monday, March 1, 2021 08:40
> To: devel@edk2.groups.io; 
> ku...@outlook.com
> Cc: Michael D Kinney; Liming 
> Gao; Zhiguang 
> Liu; Hao A Wu; 
> Jiewen Yao
> Subject: Re: [edk2-devel] [PATCH v3 1/7] MdePkg: MmUnblockMemoryLib: Added 
> definition and null instance
> 
> On 02/26/21 23:51, Kun Qin wrote:
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3168
>>
>> This interface definition provides an abstraction layer for applicable
>> drivers to request certain memory blocks to be mapped/unblocked for
>> accessibility inside MM environment.
>>
>> Cc: Michael D Kinney 
>> Cc: Liming Gao 
>> Cc: Zhiguang Liu 
>> Cc: Hao A Wu 
>> Cc: Jiewen Yao 
>>
>> Signed-off-by: Kun Qin 
>> ---
>>
>> Notes:
>> v3:
>> - Move interface to MdePkg [Hao, Liming, Jiewen]
>> - Remove Dxe prefix [Jiewen]
>>
>> v2:
>> - Resend with practical usage. No change [Hao]
>>
>>  MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c   | 40 
>> 
>>  MdePkg/Include/Library/MmUnblockMemoryLib.h  | 40 
>> 
>>  MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf | 29 
>> ++
>>  MdePkg/MdePkg.dec|  5 +++
>>  MdePkg/MdePkg.dsc|  1 +
>>  5 files changed, 115 insertions(+)
>>
>> diff --git a/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c 
>> b/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c
>> new file mode 100644
>> index ..ed9a45587b64
>> --- /dev/null
>> +++ b/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c
>> @@ -0,0 +1,40 @@
>> +/** @file
>> +  Null instance of MM Unblock Page Library.
>> +
>> +  This library provides an abstraction layer of requesting certain page 
>> access to be unblocked
>> +  by MM environment if applicable.
>> +
>> +  Copyright (c) Microsoft Corporation.
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +#include 
>> +
>> +/**
>> +  This API provides a way to unblock certain data pages to be accessible 
>> inside MM environment.
>> +
>> +  @param  UnblockAddress  The address of buffer caller requests to 
>> unblock, the address
>> +  has to be page aligned.
>> +  @param  NumberOfPages   The number of pages requested to be 
>> unblocked from MM
>> +  environment.
>> +
>> +  @return EFI_SUCCESS The request goes through successfully.
>> +  @return EFI_NOT_AVAILABLE_YET   The requested functionality is not 
>> produced yet.
>> +  @return EFI_UNSUPPORTED The requested functionality is not 
>> supported on current platform.
>> +  @return EFI_SECURITY_VIOLATION  The requested address failed to pass 
>> security check for
>> +  unblocking.
>> +  @return EFI_INVALID_PARAMETER   Input address either NULL pointer or not 
>> page aligned.
>> +  @return EFI_ACCESS_DENIED   The request is rejected due to system has 
>> passed certain boot
>> +  phase.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +MmUnblockMemoryRequest (
>> +  IN EFI_PHYSICAL_ADDRESS   UnblockAddress,
>> +  IN UINT64 NumberOfPages
>> +  )
>> +{
>> +  return EFI_UNSUPPORTED;
>> +}
>> diff --git a/MdePkg/Include/Library/MmUnblockMemoryLib.h 
>> b/MdePkg/Include/Libr

[edk2-devel] [PATCH] [edk2-platforms]Intel/BoardModulePkg: Always sort load option

2021-03-02 Thread Zhiguang Liu
Currently, load option is only sorted when setup is the first priority in boot 
option.
This condition is not needed because the below reasons:
1. Setup option may have different string name depending on platform side.
   It shouldn't be hardcoded here.
2. Always sorting meets the needs that setup should not be the first priority

Cc: Eric Dong 
Cc: Liming Gao 
Cc: Nate DeSimone 
Cc: Prince Agyeman 

Signed-off-by: Zhiguang Liu 
---
 Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c | 35 
+--
 1 file changed, 1 insertion(+), 34 deletions(-)

diff --git 
a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c 
b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
index d7612fb80a..60acf48dd6 100644
--- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
+++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
@@ -992,37 +992,6 @@ ConnectSequence (
   EfiBootManagerConnectAll ();
 }
 
-
-/**
-  The function is to consider the boot order which is not in our expectation.
-  In the case that we need to re-sort the boot option.
-
-  @retval  TRUE Need to sort Boot Option.
-  @retval  FALSEDon't need to sort Boot Option.
-**/
-BOOLEAN
-IsNeedSortBootOption (
-  VOID
-  )
-{
-  EFI_BOOT_MANAGER_LOAD_OPTION  *BootOptions;
-  UINTN BootOptionCount;
-
-  BootOptions = EfiBootManagerGetLoadOptions (&BootOptionCount, 
LoadOptionTypeBoot);
-
-  //
-  // If setup is the first priority in boot option, we need to sort boot 
option.
-  //
-  if ((BootOptionCount > 1) &&
-(((StrnCmp (BootOptions->Description, L"Enter Setup", StrLen (L"Enter 
Setup"))) == 0) ||
-((StrnCmp (BootOptions->Description, L"BootManagerMenuApp", StrLen 
(L"BootManagerMenuApp"))) == 0))) {
-return TRUE;
-  }
-
-  return FALSE;
-}
-
-
 /**
   Connects Root Bridge
  **/
@@ -1383,7 +1352,5 @@ BdsAfterConsoleReadyBeforeBootOptionCallback (
 
   EfiBootManagerRefreshAllBootOption ();
 
-  if (IsNeedSortBootOption()) {
-EfiBootManagerSortLoadOptionVariable (LoadOptionTypeBoot, 
CompareBootOption);
-  }
+  EfiBootManagerSortLoadOptionVariable (LoadOptionTypeBoot, CompareBootOption);
 }
-- 
2.30.0.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72329): https://edk2.groups.io/g/devel/message/72329
Mute This Topic: https://groups.io/mt/81021303/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH] [edk2-platforms]Intel/BoardModulePkg: Always sort load option

2021-03-02 Thread Ni, Ray
Zhiguang,

Reviewed-by: Ray Ni 

I think you can add a third reason in commit message:

3. Below change in UefiBootManagerLib puts setup in the end
  MdeModulePkg/UefiBootManagerLib: Put BootMenu at the end of BootOrder
  SHA-1: 7f34681c488aee2563eaa2afcc6a2c8aa7c5b912

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Zhiguang Liu
> Sent: Tuesday, March 2, 2021 5:04 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Liming Gao ; 
> Desimone, Nathaniel L
> ; Agyeman, Prince 
> Subject: [edk2-devel] [PATCH] [edk2-platforms]Intel/BoardModulePkg: Always 
> sort load option
> 
> Currently, load option is only sorted when setup is the first priority in 
> boot option.
> This condition is not needed because the below reasons:
> 1. Setup option may have different string name depending on platform side.
>It shouldn't be hardcoded here.
> 2. Always sorting meets the needs that setup should not be the first priority
> 
> Cc: Eric Dong 
> Cc: Liming Gao 
> Cc: Nate DeSimone 
> Cc: Prince Agyeman 
> 
> Signed-off-by: Zhiguang Liu 
> ---
>  Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c | 35 
> +--
>  1 file changed, 1 insertion(+), 34 deletions(-)
> 
> diff --git 
> a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
> b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
> index d7612fb80a..60acf48dd6 100644
> --- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
> +++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
> @@ -992,37 +992,6 @@ ConnectSequence (
>EfiBootManagerConnectAll ();
> 
>  }
> 
> 
> 
> -
> 
> -/**
> 
> -  The function is to consider the boot order which is not in our expectation.
> 
> -  In the case that we need to re-sort the boot option.
> 
> -
> 
> -  @retval  TRUE Need to sort Boot Option.
> 
> -  @retval  FALSEDon't need to sort Boot Option.
> 
> -**/
> 
> -BOOLEAN
> 
> -IsNeedSortBootOption (
> 
> -  VOID
> 
> -  )
> 
> -{
> 
> -  EFI_BOOT_MANAGER_LOAD_OPTION  *BootOptions;
> 
> -  UINTN BootOptionCount;
> 
> -
> 
> -  BootOptions = EfiBootManagerGetLoadOptions (&BootOptionCount, 
> LoadOptionTypeBoot);
> 
> -
> 
> -  //
> 
> -  // If setup is the first priority in boot option, we need to sort boot 
> option.
> 
> -  //
> 
> -  if ((BootOptionCount > 1) &&
> 
> -(((StrnCmp (BootOptions->Description, L"Enter Setup", StrLen (L"Enter 
> Setup"))) == 0) ||
> 
> -((StrnCmp (BootOptions->Description, L"BootManagerMenuApp", StrLen 
> (L"BootManagerMenuApp"))) == 0))) {
> 
> -return TRUE;
> 
> -  }
> 
> -
> 
> -  return FALSE;
> 
> -}
> 
> -
> 
> -
> 
>  /**
> 
>Connects Root Bridge
> 
>   **/
> 
> @@ -1383,7 +1352,5 @@ BdsAfterConsoleReadyBeforeBootOptionCallback (
> 
> 
>EfiBootManagerRefreshAllBootOption ();
> 
> 
> 
> -  if (IsNeedSortBootOption()) {
> 
> -EfiBootManagerSortLoadOptionVariable (LoadOptionTypeBoot, 
> CompareBootOption);
> 
> -  }
> 
> +  EfiBootManagerSortLoadOptionVariable (LoadOptionTypeBoot, 
> CompareBootOption);
> 
>  }
> 
> --
> 2.30.0.windows.2
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#72329): https://edk2.groups.io/g/devel/message/72329
> Mute This Topic: https://groups.io/mt/81021303/1712937
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray...@intel.com]
> -=-=-=-=-=-=
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72330): https://edk2.groups.io/g/devel/message/72330
Mute This Topic: https://groups.io/mt/81021303/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH] [edk2-platforms]Intel/BoardModulePkg: Always sort load option

2021-03-02 Thread Gao, Zhichao
Hi Ray,

I just think of that if we always do the sort, it may cause the changed boot 
order (by the user of the platform) resort again. That's unexpected.

Thanks,
Zhichao

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Ni, Ray
> Sent: Tuesday, March 2, 2021 5:12 PM
> To: devel@edk2.groups.io; Liu, Zhiguang 
> Cc: Dong, Eric ; Liming Gao
> ; Desimone, Nathaniel L
> ; Agyeman, Prince
> 
> Subject: Re: [edk2-devel] [PATCH] [edk2-platforms]Intel/BoardModulePkg:
> Always sort load option
> 
> Zhiguang,
> 
> Reviewed-by: Ray Ni 
> 
> I think you can add a third reason in commit message:
> 
> 3. Below change in UefiBootManagerLib puts setup in the end
>   MdeModulePkg/UefiBootManagerLib: Put BootMenu at the end of BootOrder
>   SHA-1: 7f34681c488aee2563eaa2afcc6a2c8aa7c5b912
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of
> > Zhiguang Liu
> > Sent: Tuesday, March 2, 2021 5:04 PM
> > To: devel@edk2.groups.io
> > Cc: Dong, Eric ; Liming Gao
> > ; Desimone, Nathaniel L
> > ; Agyeman, Prince
> > 
> > Subject: [edk2-devel] [PATCH] [edk2-platforms]Intel/BoardModulePkg:
> > Always sort load option
> >
> > Currently, load option is only sorted when setup is the first priority in 
> > boot
> option.
> > This condition is not needed because the below reasons:
> > 1. Setup option may have different string name depending on platform side.
> >It shouldn't be hardcoded here.
> > 2. Always sorting meets the needs that setup should not be the first
> > priority
> >
> > Cc: Eric Dong 
> > Cc: Liming Gao 
> > Cc: Nate DeSimone 
> > Cc: Prince Agyeman 
> >
> > Signed-off-by: Zhiguang Liu 
> > ---
> >
> > Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.
> > c | 35 +--
> >  1 file changed, 1 insertion(+), 34 deletions(-)
> >
> > diff --git
> > a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLi
> > b.c
> > b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLi
> > b.c
> > index d7612fb80a..60acf48dd6 100644
> > ---
> > a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLi
> > b.c
> > +++
> b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHo
> > +++ okLib.c
> > @@ -992,37 +992,6 @@ ConnectSequence (
> >EfiBootManagerConnectAll ();
> >
> >  }
> >
> >
> >
> > -
> >
> > -/**
> >
> > -  The function is to consider the boot order which is not in our 
> > expectation.
> >
> > -  In the case that we need to re-sort the boot option.
> >
> > -
> >
> > -  @retval  TRUE Need to sort Boot Option.
> >
> > -  @retval  FALSEDon't need to sort Boot Option.
> >
> > -**/
> >
> > -BOOLEAN
> >
> > -IsNeedSortBootOption (
> >
> > -  VOID
> >
> > -  )
> >
> > -{
> >
> > -  EFI_BOOT_MANAGER_LOAD_OPTION  *BootOptions;
> >
> > -  UINTN BootOptionCount;
> >
> > -
> >
> > -  BootOptions = EfiBootManagerGetLoadOptions (&BootOptionCount,
> > LoadOptionTypeBoot);
> >
> > -
> >
> > -  //
> >
> > -  // If setup is the first priority in boot option, we need to sort boot 
> > option.
> >
> > -  //
> >
> > -  if ((BootOptionCount > 1) &&
> >
> > -(((StrnCmp (BootOptions->Description, L"Enter Setup", StrLen (L"Enter
> Setup"))) == 0) ||
> >
> > -((StrnCmp (BootOptions->Description, L"BootManagerMenuApp", StrLen
> (L"BootManagerMenuApp"))) == 0))) {
> >
> > -return TRUE;
> >
> > -  }
> >
> > -
> >
> > -  return FALSE;
> >
> > -}
> >
> > -
> >
> > -
> >
> >  /**
> >
> >Connects Root Bridge
> >
> >   **/
> >
> > @@ -1383,7 +1352,5 @@ BdsAfterConsoleReadyBeforeBootOptionCallback (
> >
> >
> >EfiBootManagerRefreshAllBootOption ();
> >
> >
> >
> > -  if (IsNeedSortBootOption()) {
> >
> > -EfiBootManagerSortLoadOptionVariable (LoadOptionTypeBoot,
> CompareBootOption);
> >
> > -  }
> >
> > +  EfiBootManagerSortLoadOptionVariable (LoadOptionTypeBoot,
> > + CompareBootOption);
> >
> >  }
> >
> > --
> > 2.30.0.windows.2
> >
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#72329):
> > https://edk2.groups.io/g/devel/message/72329
> > Mute This Topic: https://groups.io/mt/81021303/1712937
> > Group Owner: devel+ow...@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray...@intel.com]
> > -=-=-=-=-=-=
> >
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72331): https://edk2.groups.io/g/devel/message/72331
Mute This Topic: https://groups.io/mt/81021303/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH edk2-platforms v2 1/4] SbsaQemu: Build infrastructure for StandaloneMm image

2021-03-02 Thread Masahisa Kojima
Hi Leif,

Thank you for you comments.

On Tue, 2 Mar 2021 at 02:05, Leif Lindholm  wrote:
>
> On Mon, Mar 01, 2021 at 14:19:49 +0900, Masahisa Kojima wrote:
> > Add the build infrastructure for compilation of StandaloneMm image.
> >
> > Signed-off-by: Masahisa Kojima 
> > ---
> >  .../Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc| 132 ++
>
> Please use --stat=1000 --stat-graph-width=20 when generating patches.

Sorry I forgot to add these options, will be included in the next version.

>
> >  Platform/Qemu/SbsaQemu/SbsaQemu.fdf   |   6 +-
>
> It is not immediately obvious to me why the pre-existing
> SbsaQemuStandaloneMm.dsc needs to change. Is this something that can
> be clarified in commit message?

I probably does not understand your comment correctly, but
SbsaQemuStandaloneMm.dsc
is newly created file with this commit.

Thanks,
Masahisa

>
> Best Regards,
>
> Leif
>
> >  .../Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf|  93 
> >  3 files changed, 228 insertions(+), 3 deletions(-)
> >  create mode 100644 Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc
> >  create mode 100644 Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf
> >
> > diff --git a/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc 
> > b/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc
> > new file mode 100644
> > index ..87f5ee351eaa
> > --- /dev/null
> > +++ b/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc
> > @@ -0,0 +1,132 @@
> > +#
> > +#  Copyright (c) 2020, Linaro Limited. All rights reserved.
> > +#
> > +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> > +#
> > +
> > +
> > +#
> > +# Defines Section - statements that will be processed to create a Makefile.
> > +#
> > +
> > +[Defines]
> > +  PLATFORM_NAME  = SbsaQemuStandaloneMm
> > +  PLATFORM_GUID  = A64CC0F5-7ACD-4975-BBE7-7EF6739C8668
> > +  PLATFORM_VERSION   = 1.0
> > +  DSC_SPECIFICATION  = 0x00010011
> > +  OUTPUT_DIRECTORY   = Build/$(PLATFORM_NAME)
> > +  SUPPORTED_ARCHITECTURES= AARCH64
> > +  BUILD_TARGETS  = DEBUG|RELEASE|NOOPT
> > +  SKUID_IDENTIFIER   = DEFAULT
> > +  FLASH_DEFINITION   = 
> > Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf
> > +  DEFINE DEBUG_MESSAGE   = TRUE
> > +
> > +  # LzmaF86
> > +  DEFINE COMPRESSION_TOOL_GUID   = D42AE6BD-1352-4bfb-909A-CA72A6EAE889
> > +
> > +
> > +#
> > +# Library Class section - list of all Library Classes needed by this 
> > Platform.
> > +#
> > +
> > +[LibraryClasses]
> > +  #
> > +  # Basic
> > +  #
> > +  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> > +  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> > +  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> > +  
> > DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
> > +  
> > ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf
> > +  FvLib|StandaloneMmPkg/Library/FvLib/FvLib.inf
> > +  
> > HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf
> > +  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> > +  MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf
> > +  
> > MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmCoreMemoryAllocationLib/StandaloneMmCoreMemoryAllocationLib.inf
> > +  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> > +  PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
> > +  PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
> > +  
> > ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
> > +
> > +  #
> > +  # Entry point
> > +  #
> > +  
> > StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf
> > +
> > +  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> > +  
> > StandaloneMmMmuLib|ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
> > +  ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
> > +  
> > CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
> > +  
> > PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
> > +
> > +  # ARM PL011 UART Driver
> > +  
> > PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
> > +  PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
> > +  
> > SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> > +
> > +  
> > StandaloneMmCoreEntryPoint|Standa

[edk2-devel] [PATCH edk2-platforms 1/1] Silicon/Qemu: Move SbsaQemu MPIDR-retrieval function to FdtHelperLib

2021-03-02 Thread Leif Lindholm
Commit 822634fc1bf1 ("SbsaQemu: Update SbsaQemuAcpiDxe to use FdtHelperLib")
replaced the CountCpusFromFdt() function in SbsaQemuAcpiDxe with a call to
FdtHelperCountCpus() in FdtHelperLib. This ended up leaving static variables
FdtFirstCpuOffset and FdtCpuNodeSize uninitialised, such that the GetMpidr()
function kept returning the value for cpu 0.

Resolve this by moving the GetMpidr() function over to FdtHelperLib, where
it can again share these variables with FdtHelperCountCpus().

Fix up coding style issues as part of copy:
- Add m prefix to module-global variables.
- Add doxygen function comment header.

Cc: Ard Biesheuvel 
Cc: Graeme Gregory 
Cc: Radoslaw Biernacki 
Cc: Tanmay Jagdale 
Cc: Rebecca Cran 
Reported-by: Marcin Juszkiewicz 
Signed-off-by: Leif Lindholm 
---
 Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf |  2 --
 Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf   |  5 +++
 Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h  | 12 +++
 Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c   | 35 
+--
 Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c | 36 

 5 files changed, 54 insertions(+), 36 deletions(-)

diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf 
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
index a58ebfaf76d5..c6de685bd2c4 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
@@ -35,7 +35,6 @@ [LibraryClasses]
   DebugLib
   DxeServicesLib
   FdtHelperLib
-  FdtLib
   PcdLib
   PrintLib
   UefiDriverEntryPoint
@@ -46,7 +45,6 @@ [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount
-  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
 
 [Depex]
   gEfiAcpiTableProtocolGuid   ## CONSUMES
diff --git a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf 
b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
index d84c16f888d1..9c059f3e5851 100644
--- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
+++ b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
@@ -24,5 +24,10 @@ [Packages]
   MdePkg/MdePkg.dec
   Silicon/Qemu/SbsaQemu/SbsaQemu.dec
 
+[LibraryClasses]
+  DebugLib
+  FdtLib
+  PcdLib
+
 [FixedPcd]
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h 
b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
index f9045fd5df45..ea9159857215 100644
--- a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
+++ b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
@@ -10,6 +10,18 @@
 #ifndef FDT_HELPER_LIB_
 #define FDT_HELPER_LIB_
 
+/**
+  Get MPIDR for a given cpu from device tree passed by Qemu.
+
+  @param [in]   CpuIdIndex of cpu to retrieve MPIDR value for.
+
+  @retvalMPIDR value of CPU at index 
+**/
+UINT64
+FdtHelperGetMpidr (
+  IN UINTN   CpuId
+  );
+
 /** Walks through the Device Tree created by Qemu and counts the number
 of CPUs present in it.
 
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c 
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
index 84120f1c1b51..b8901030ecd0 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -20,39 +20,6 @@
 #include 
 #include 
 #include 
-#include 
-#include 
-
-STATIC INT32 FdtFirstCpuOffset;
-STATIC INT32 FdtCpuNodeSize;
-
-/*
- * Get MPIDR from device tree passed by Qemu
- */
-STATIC
-UINT64
-GetMpidr (
-  IN UINTN   CpuId
-  )
-{
-  VOID   *DeviceTreeBase;
-  CONST UINT64   *RegVal;
-  INT32  Len;
-
-  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
-  ASSERT (DeviceTreeBase != NULL);
-
-  RegVal = fdt_getprop (DeviceTreeBase,
- FdtFirstCpuOffset + (CpuId * FdtCpuNodeSize),
- "reg",
- &Len);
-  if (!RegVal) {
-DEBUG ((DEBUG_ERROR, "Couldn't find reg property for CPU:%d\n", CpuId));
-return 0;
-  }
-
-  return (fdt64_to_cpu (ReadUnaligned64 (RegVal)));
-}
 
 /*
  * A Function to Compute the ACPI Table Checksum
@@ -159,7 +126,7 @@ AddMadtTable (
 CopyMem (New, &Gicc, sizeof (EFI_ACPI_6_0_GIC_STRUCTURE));
 GiccPtr = (EFI_ACPI_6_0_GIC_STRUCTURE *) New;
 GiccPtr->AcpiProcessorUid = CoreIndex;
-GiccPtr->MPIDR = GetMpidr (CoreIndex);
+GiccPtr->MPIDR = FdtHelperGetMpidr (CoreIndex);
 New += sizeof (EFI_ACPI_6_0_GIC_STRUCTURE);
   }
 
diff --git a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c 
b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c
index bbd756f01a21..7fdfb055db76 100644
--- a/Silicon/Qemu/SbsaQemu/Libr

Re: [edk2-devel] [PATCH] [edk2-platforms]Intel/BoardModulePkg: Always sort load option

2021-03-02 Thread Ni, Ray
Good catch.
BdsAfterConsoleReadyBeforeBootOptionCallback() in BoardModulePkg is not 
implemented properly.
It should only do the boot option sort either:
1. in the first boot after flashing the firmware, or
2. in BOOT_WITH_FULL_CONFIGURATION boot path if the platform PEI can correctly 
changes the boot mode to other boot modes when no configuration changes.

Thanks,
Ray

> -Original Message-
> From: Gao, Zhichao 
> Sent: Tuesday, March 2, 2021 6:46 PM
> To: devel@edk2.groups.io; Ni, Ray ; Liu, Zhiguang 
> 
> Cc: Dong, Eric ; Liming Gao ; 
> Desimone, Nathaniel L
> ; Agyeman, Prince 
> Subject: RE: [edk2-devel] [PATCH] [edk2-platforms]Intel/BoardModulePkg: 
> Always sort load option
> 
> Hi Ray,
> 
> I just think of that if we always do the sort, it may cause the changed boot 
> order (by the user of the platform) resort again.
> That's unexpected.
> 
> Thanks,
> Zhichao
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Ni, Ray
> > Sent: Tuesday, March 2, 2021 5:12 PM
> > To: devel@edk2.groups.io; Liu, Zhiguang 
> > Cc: Dong, Eric ; Liming Gao
> > ; Desimone, Nathaniel L
> > ; Agyeman, Prince
> > 
> > Subject: Re: [edk2-devel] [PATCH] [edk2-platforms]Intel/BoardModulePkg:
> > Always sort load option
> >
> > Zhiguang,
> >
> > Reviewed-by: Ray Ni 
> >
> > I think you can add a third reason in commit message:
> >
> > 3. Below change in UefiBootManagerLib puts setup in the end
> >   MdeModulePkg/UefiBootManagerLib: Put BootMenu at the end of BootOrder
> >   SHA-1: 7f34681c488aee2563eaa2afcc6a2c8aa7c5b912
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of
> > > Zhiguang Liu
> > > Sent: Tuesday, March 2, 2021 5:04 PM
> > > To: devel@edk2.groups.io
> > > Cc: Dong, Eric ; Liming Gao
> > > ; Desimone, Nathaniel L
> > > ; Agyeman, Prince
> > > 
> > > Subject: [edk2-devel] [PATCH] [edk2-platforms]Intel/BoardModulePkg:
> > > Always sort load option
> > >
> > > Currently, load option is only sorted when setup is the first priority in 
> > > boot
> > option.
> > > This condition is not needed because the below reasons:
> > > 1. Setup option may have different string name depending on platform side.
> > >It shouldn't be hardcoded here.
> > > 2. Always sorting meets the needs that setup should not be the first
> > > priority
> > >
> > > Cc: Eric Dong 
> > > Cc: Liming Gao 
> > > Cc: Nate DeSimone 
> > > Cc: Prince Agyeman 
> > >
> > > Signed-off-by: Zhiguang Liu 
> > > ---
> > >
> > > Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.
> > > c | 35 +--
> > >  1 file changed, 1 insertion(+), 34 deletions(-)
> > >
> > > diff --git
> > > a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLi
> > > b.c
> > > b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLi
> > > b.c
> > > index d7612fb80a..60acf48dd6 100644
> > > ---
> > > a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLi
> > > b.c
> > > +++
> > b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHo
> > > +++ okLib.c
> > > @@ -992,37 +992,6 @@ ConnectSequence (
> > >EfiBootManagerConnectAll ();
> > >
> > >  }
> > >
> > >
> > >
> > > -
> > >
> > > -/**
> > >
> > > -  The function is to consider the boot order which is not in our 
> > > expectation.
> > >
> > > -  In the case that we need to re-sort the boot option.
> > >
> > > -
> > >
> > > -  @retval  TRUE Need to sort Boot Option.
> > >
> > > -  @retval  FALSEDon't need to sort Boot Option.
> > >
> > > -**/
> > >
> > > -BOOLEAN
> > >
> > > -IsNeedSortBootOption (
> > >
> > > -  VOID
> > >
> > > -  )
> > >
> > > -{
> > >
> > > -  EFI_BOOT_MANAGER_LOAD_OPTION  *BootOptions;
> > >
> > > -  UINTN BootOptionCount;
> > >
> > > -
> > >
> > > -  BootOptions = EfiBootManagerGetLoadOptions (&BootOptionCount,
> > > LoadOptionTypeBoot);
> > >
> > > -
> > >
> > > -  //
> > >
> > > -  // If setup is the first priority in boot option, we need to sort boot 
> > > option.
> > >
> > > -  //
> > >
> > > -  if ((BootOptionCount > 1) &&
> > >
> > > -(((StrnCmp (BootOptions->Description, L"Enter Setup", StrLen (L"Enter
> > Setup"))) == 0) ||
> > >
> > > -((StrnCmp (BootOptions->Description, L"BootManagerMenuApp", StrLen
> > (L"BootManagerMenuApp"))) == 0))) {
> > >
> > > -return TRUE;
> > >
> > > -  }
> > >
> > > -
> > >
> > > -  return FALSE;
> > >
> > > -}
> > >
> > > -
> > >
> > > -
> > >
> > >  /**
> > >
> > >Connects Root Bridge
> > >
> > >   **/
> > >
> > > @@ -1383,7 +1352,5 @@ BdsAfterConsoleReadyBeforeBootOptionCallback (
> > >
> > >
> > >EfiBootManagerRefreshAllBootOption ();
> > >
> > >
> > >
> > > -  if (IsNeedSortBootOption()) {
> > >
> > > -EfiBootManagerSortLoadOptionVariable (LoadOptionTypeBoot,
> > CompareBootOption);
> > >
> > > -  }
> > >
> > > +  EfiBootManagerSortLoadOptionVariable (LoadOptionTypeBoot,
> > > + CompareBootOption);
> > >
> > >  }
> > >
> > > --
> > > 2.

Re: [edk2-devel] [PATCH edk2-platforms 1/1] Silicon/Qemu: Move SbsaQemu MPIDR-retrieval function to FdtHelperLib

2021-03-02 Thread Ard Biesheuvel
On Tue, 2 Mar 2021 at 14:38, Leif Lindholm  wrote:
>
> Commit 822634fc1bf1 ("SbsaQemu: Update SbsaQemuAcpiDxe to use FdtHelperLib")
> replaced the CountCpusFromFdt() function in SbsaQemuAcpiDxe with a call to
> FdtHelperCountCpus() in FdtHelperLib. This ended up leaving static variables
> FdtFirstCpuOffset and FdtCpuNodeSize uninitialised, such that the GetMpidr()
> function kept returning the value for cpu 0.
>
> Resolve this by moving the GetMpidr() function over to FdtHelperLib, where
> it can again share these variables with FdtHelperCountCpus().
>
> Fix up coding style issues as part of copy:
> - Add m prefix to module-global variables.
> - Add doxygen function comment header.
>
> Cc: Ard Biesheuvel 
> Cc: Graeme Gregory 
> Cc: Radoslaw Biernacki 
> Cc: Tanmay Jagdale 
> Cc: Rebecca Cran 
> Reported-by: Marcin Juszkiewicz 
> Signed-off-by: Leif Lindholm 

Acked-by: Ard Biesheuvel 

> ---
>  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf |  2 --
>  Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf   |  5 +++
>  Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h  | 12 
> +++
>  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c   | 35 
> +--
>  Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c | 36 
> 
>  5 files changed, 54 insertions(+), 36 deletions(-)
>
> diff --git 
> a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf 
> b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> index a58ebfaf76d5..c6de685bd2c4 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> @@ -35,7 +35,6 @@ [LibraryClasses]
>DebugLib
>DxeServicesLib
>FdtHelperLib
> -  FdtLib
>PcdLib
>PrintLib
>UefiDriverEntryPoint
> @@ -46,7 +45,6 @@ [Pcd]
>gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile
>gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
>gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount
> -  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
>
>  [Depex]
>gEfiAcpiTableProtocolGuid   ## CONSUMES
> diff --git a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf 
> b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> index d84c16f888d1..9c059f3e5851 100644
> --- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> +++ b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> @@ -24,5 +24,10 @@ [Packages]
>MdePkg/MdePkg.dec
>Silicon/Qemu/SbsaQemu/SbsaQemu.dec
>
> +[LibraryClasses]
> +  DebugLib
> +  FdtLib
> +  PcdLib
> +
>  [FixedPcd]
>gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
> diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h 
> b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> index f9045fd5df45..ea9159857215 100644
> --- a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> +++ b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> @@ -10,6 +10,18 @@
>  #ifndef FDT_HELPER_LIB_
>  #define FDT_HELPER_LIB_
>
> +/**
> +  Get MPIDR for a given cpu from device tree passed by Qemu.
> +
> +  @param [in]   CpuIdIndex of cpu to retrieve MPIDR value for.
> +
> +  @retvalMPIDR value of CPU at index 
> +**/
> +UINT64
> +FdtHelperGetMpidr (
> +  IN UINTN   CpuId
> +  );
> +
>  /** Walks through the Device Tree created by Qemu and counts the number
>  of CPUs present in it.
>
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c 
> b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> index 84120f1c1b51..b8901030ecd0 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> @@ -20,39 +20,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
> -#include 
> -
> -STATIC INT32 FdtFirstCpuOffset;
> -STATIC INT32 FdtCpuNodeSize;
> -
> -/*
> - * Get MPIDR from device tree passed by Qemu
> - */
> -STATIC
> -UINT64
> -GetMpidr (
> -  IN UINTN   CpuId
> -  )
> -{
> -  VOID   *DeviceTreeBase;
> -  CONST UINT64   *RegVal;
> -  INT32  Len;
> -
> -  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
> -  ASSERT (DeviceTreeBase != NULL);
> -
> -  RegVal = fdt_getprop (DeviceTreeBase,
> - FdtFirstCpuOffset + (CpuId * FdtCpuNodeSize),
> - "reg",
> - &Len);
> -  if (!RegVal) {
> -DEBUG ((DEBUG_ERROR, "Couldn't find reg property for CPU:%d\n", CpuId));
> -return 0;
> -  }
> -
> -  return (fdt64_to_cpu (ReadUnaligned64 (RegVal)));
> -}
>
>  /*
>   * A Function to Compute the ACPI Table Checksum
> @@ -159,7 +126,7 @@ AddMadtTable (
>  CopyMem (New, &Gicc, sizeof (EFI_ACPI_6_0_GIC_STRUCTURE));
>  GiccPtr = (EFI_ACPI_6_0_GIC_STRUCTURE *) New;
>  GiccPtr->AcpiProcessorUid = CoreIndex;
> - 

Re: [edk2-devel] [PATCH edk2-platforms v2 1/4] SbsaQemu: Build infrastructure for StandaloneMm image

2021-03-02 Thread Leif Lindholm
On Tue, Mar 02, 2021 at 21:45:26 +0900, Masahisa Kojima wrote:
> Hi Leif,
> 
> Thank you for you comments.
> 
> On Tue, 2 Mar 2021 at 02:05, Leif Lindholm  wrote:
> >
> > On Mon, Mar 01, 2021 at 14:19:49 +0900, Masahisa Kojima wrote:
> > > Add the build infrastructure for compilation of StandaloneMm image.
> > >
> > > Signed-off-by: Masahisa Kojima 
> > > ---
> > >  .../Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc| 132 ++
> >
> > Please use --stat=1000 --stat-graph-width=20 when generating patches.
> 
> Sorry I forgot to add these options, will be included in the next version.
> 
> >
> > >  Platform/Qemu/SbsaQemu/SbsaQemu.fdf   |   6 +-
> >
> > It is not immediately obvious to me why the pre-existing
> > SbsaQemuStandaloneMm.dsc needs to change. Is this something that can
> > be clarified in commit message?
> 
> I probably does not understand your comment correctly, but
> SbsaQemuStandaloneMm.dsc
> is newly created file with this commit.

Sorry, that's just my brain failure: I meant to say SbsaQemu.fdf.

Regards,

Leif

> Thanks,
> Masahisa
> 
> >
> > Best Regards,
> >
> > Leif
> >
> > >  .../Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf|  93 
> > >  3 files changed, 228 insertions(+), 3 deletions(-)
> > >  create mode 100644 Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc
> > >  create mode 100644 Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf
> > >
> > > diff --git a/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc 
> > > b/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc
> > > new file mode 100644
> > > index ..87f5ee351eaa
> > > --- /dev/null
> > > +++ b/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc
> > > @@ -0,0 +1,132 @@
> > > +#
> > > +#  Copyright (c) 2020, Linaro Limited. All rights reserved.
> > > +#
> > > +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > +#
> > > +
> > > +
> > > +#
> > > +# Defines Section - statements that will be processed to create a 
> > > Makefile.
> > > +#
> > > +
> > > +[Defines]
> > > +  PLATFORM_NAME  = SbsaQemuStandaloneMm
> > > +  PLATFORM_GUID  = A64CC0F5-7ACD-4975-BBE7-7EF6739C8668
> > > +  PLATFORM_VERSION   = 1.0
> > > +  DSC_SPECIFICATION  = 0x00010011
> > > +  OUTPUT_DIRECTORY   = Build/$(PLATFORM_NAME)
> > > +  SUPPORTED_ARCHITECTURES= AARCH64
> > > +  BUILD_TARGETS  = DEBUG|RELEASE|NOOPT
> > > +  SKUID_IDENTIFIER   = DEFAULT
> > > +  FLASH_DEFINITION   = 
> > > Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf
> > > +  DEFINE DEBUG_MESSAGE   = TRUE
> > > +
> > > +  # LzmaF86
> > > +  DEFINE COMPRESSION_TOOL_GUID   = D42AE6BD-1352-4bfb-909A-CA72A6EAE889
> > > +
> > > +
> > > +#
> > > +# Library Class section - list of all Library Classes needed by this 
> > > Platform.
> > > +#
> > > +
> > > +[LibraryClasses]
> > > +  #
> > > +  # Basic
> > > +  #
> > > +  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> > > +  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> > > +  
> > > DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> > > +  
> > > DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
> > > +  
> > > ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf
> > > +  FvLib|StandaloneMmPkg/Library/FvLib/FvLib.inf
> > > +  
> > > HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf
> > > +  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> > > +  
> > > MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf
> > > +  
> > > MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmCoreMemoryAllocationLib/StandaloneMmCoreMemoryAllocationLib.inf
> > > +  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> > > +  PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
> > > +  PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
> > > +  
> > > ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
> > > +
> > > +  #
> > > +  # Entry point
> > > +  #
> > > +  
> > > StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf
> > > +
> > > +  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> > > +  
> > > StandaloneMmMmuLib|ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
> > > +  ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
> > > +  
> > > CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
> > > +  
> > > PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMm

Re: [edk2-devel] [PATCH edk2-platforms 1/1] Silicon/Qemu: Move SbsaQemu MPIDR-retrieval function to FdtHelperLib

2021-03-02 Thread Graeme Gregory

On 02/03/2021 13:38, Leif Lindholm wrote:

Commit 822634fc1bf1 ("SbsaQemu: Update SbsaQemuAcpiDxe to use FdtHelperLib")
replaced the CountCpusFromFdt() function in SbsaQemuAcpiDxe with a call to
FdtHelperCountCpus() in FdtHelperLib. This ended up leaving static variables
FdtFirstCpuOffset and FdtCpuNodeSize uninitialised, such that the GetMpidr()
function kept returning the value for cpu 0.

Resolve this by moving the GetMpidr() function over to FdtHelperLib, where
it can again share these variables with FdtHelperCountCpus().

Fix up coding style issues as part of copy:
- Add m prefix to module-global variables.
- Add doxygen function comment header.

Cc: Ard Biesheuvel 
Cc: Graeme Gregory 
Cc: Radoslaw Biernacki 
Cc: Tanmay Jagdale 
Cc: Rebecca Cran 
Reported-by: Marcin Juszkiewicz 
Signed-off-by: Leif Lindholm 


Tested-By: Graeme Gregory 

This fixes the issue from inspection of APIC table with acpiview!


---
  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf |  2 --
  Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf   |  5 +++
  Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h  | 12 +++
  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c   | 35 
+--
  Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c | 36 

  5 files changed, 54 insertions(+), 36 deletions(-)

diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf 
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
index a58ebfaf76d5..c6de685bd2c4 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
@@ -35,7 +35,6 @@ [LibraryClasses]
DebugLib
DxeServicesLib
FdtHelperLib
-  FdtLib
PcdLib
PrintLib
UefiDriverEntryPoint
@@ -46,7 +45,6 @@ [Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount
-  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
  
  [Depex]

gEfiAcpiTableProtocolGuid   ## CONSUMES
diff --git a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf 
b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
index d84c16f888d1..9c059f3e5851 100644
--- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
+++ b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
@@ -24,5 +24,10 @@ [Packages]
MdePkg/MdePkg.dec
Silicon/Qemu/SbsaQemu/SbsaQemu.dec
  
+[LibraryClasses]

+  DebugLib
+  FdtLib
+  PcdLib
+
  [FixedPcd]
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h 
b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
index f9045fd5df45..ea9159857215 100644
--- a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
+++ b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
@@ -10,6 +10,18 @@
  #ifndef FDT_HELPER_LIB_
  #define FDT_HELPER_LIB_
  
+/**

+  Get MPIDR for a given cpu from device tree passed by Qemu.
+
+  @param [in]   CpuIdIndex of cpu to retrieve MPIDR value for.
+
+  @retvalMPIDR value of CPU at index 
+**/
+UINT64
+FdtHelperGetMpidr (
+  IN UINTN   CpuId
+  );
+
  /** Walks through the Device Tree created by Qemu and counts the number
  of CPUs present in it.
  
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c

index 84120f1c1b51..b8901030ecd0 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -20,39 +20,6 @@
  #include 
  #include 
  #include 
-#include 
-#include 
-
-STATIC INT32 FdtFirstCpuOffset;
-STATIC INT32 FdtCpuNodeSize;
-
-/*
- * Get MPIDR from device tree passed by Qemu
- */
-STATIC
-UINT64
-GetMpidr (
-  IN UINTN   CpuId
-  )
-{
-  VOID   *DeviceTreeBase;
-  CONST UINT64   *RegVal;
-  INT32  Len;
-
-  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
-  ASSERT (DeviceTreeBase != NULL);
-
-  RegVal = fdt_getprop (DeviceTreeBase,
- FdtFirstCpuOffset + (CpuId * FdtCpuNodeSize),
- "reg",
- &Len);
-  if (!RegVal) {
-DEBUG ((DEBUG_ERROR, "Couldn't find reg property for CPU:%d\n", CpuId));
-return 0;
-  }
-
-  return (fdt64_to_cpu (ReadUnaligned64 (RegVal)));
-}
  
  /*

   * A Function to Compute the ACPI Table Checksum
@@ -159,7 +126,7 @@ AddMadtTable (
  CopyMem (New, &Gicc, sizeof (EFI_ACPI_6_0_GIC_STRUCTURE));
  GiccPtr = (EFI_ACPI_6_0_GIC_STRUCTURE *) New;
  GiccPtr->AcpiProcessorUid = CoreIndex;
-GiccPtr->MPIDR = GetMpidr (CoreIndex);
+GiccPtr->MPIDR = FdtHelperGetMpidr (CoreIndex);
  New += sizeof (EFI_ACPI_6_0_GIC_STRUCTURE);
}
  
diff --git a/Sili

[edk2-devel] [PATCH] drop Tanmay Jagdale from sbsa-ref maintainers

2021-03-02 Thread Marcin Juszkiewicz
Tanmay is no longer at Linaro

Signed-off-by: Marcin Juszkiewicz 
---
 Maintainers.txt | 1 -
 1 file changed, 1 deletion(-)

diff --git Maintainers.txt Maintainers.txt
index 2e6e87bb6d..afbd2cff0e 100644
--- Maintainers.txt
+++ Maintainers.txt
@@ -295,7 +295,6 @@ M: Ard Biesheuvel 
 M: Leif Lindholm 
 R: Graeme Gregory 
 R: Radoslaw Biernacki 
-R: Tanmay Jagdale 
 
 Raspberry Pi platforms and silicon
 F: Platform/RaspberryPi/
-- 
2.29.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72338): https://edk2.groups.io/g/devel/message/72338
Mute This Topic: https://groups.io/mt/81026926/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH edk2-platforms 1/1] Silicon/Qemu: Move SbsaQemu MPIDR-retrieval function to FdtHelperLib

2021-03-02 Thread Leif Lindholm
On Tue, Mar 02, 2021 at 16:01:56 +0100, Marcin Juszkiewicz wrote:
> W dniu 02.03.2021 o 15:14, Graeme Gregory pisze:
> > On 02/03/2021 13:38, Leif Lindholm wrote:
> > > Commit 822634fc1bf1 ("SbsaQemu: Update SbsaQemuAcpiDxe to use
> > > FdtHelperLib")
> > > replaced the CountCpusFromFdt() function in SbsaQemuAcpiDxe with a
> > > call to
> > > FdtHelperCountCpus() in FdtHelperLib. This ended up leaving static
> > > variables
> > > FdtFirstCpuOffset and FdtCpuNodeSize uninitialised, such that the
> > > GetMpidr()
> > > function kept returning the value for cpu 0.
> > > 
> > > Resolve this by moving the GetMpidr() function over to FdtHelperLib,
> > > where
> > > it can again share these variables with FdtHelperCountCpus().
> > > 
> > > Fix up coding style issues as part of copy:
> > > - Add m prefix to module-global variables.
> > > - Add doxygen function comment header.
> > > 
> > > Cc: Ard Biesheuvel 
> > > Cc: Graeme Gregory 
> > > Cc: Radoslaw Biernacki 
> > > Cc: Tanmay Jagdale 
> > > Cc: Rebecca Cran 
> > > Reported-by: Marcin Juszkiewicz 
> > > Signed-off-by: Leif Lindholm 
> > 
> > Tested-By: Graeme Gregory 
> 
> Tested-by: Marcin Juszkiewicz 
> 
> sbsa-acs now finish in seconds like before.

Thanks all.
Pushed as a3ce6f8df2b6.

> > 
> > > ---
> > >   Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> > > |  2 --
> > >   Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf  
> > > |  5 +++
> > >   Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h 
> > > | 12 +++
> > >   Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c  
> > > | 35 +--
> > >   Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c
> > > | 36 
> > >   5 files changed, 54 insertions(+), 36 deletions(-)
> > > 
> > > diff --git
> > > a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> > > b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> > > index a58ebfaf76d5..c6de685bd2c4 100644
> > > --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> > > +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> > > @@ -35,7 +35,6 @@ [LibraryClasses]
> > >     DebugLib
> > >     DxeServicesLib
> > >     FdtHelperLib
> > > -  FdtLib
> > >     PcdLib
> > >     PrintLib
> > >     UefiDriverEntryPoint
> > > @@ -46,7 +45,6 @@ [Pcd]
> > >     gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile
> > >     gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
> > >     gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount
> > > -  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
> > >   [Depex]
> > >     gEfiAcpiTableProtocolGuid   ## CONSUMES
> > > diff --git
> > > a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> > > b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> > > index d84c16f888d1..9c059f3e5851 100644
> > > --- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> > > +++ b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
> > > @@ -24,5 +24,10 @@ [Packages]
> > >     MdePkg/MdePkg.dec
> > >     Silicon/Qemu/SbsaQemu/SbsaQemu.dec
> > > +[LibraryClasses]
> > > +  DebugLib
> > > +  FdtLib
> > > +  PcdLib
> > > +
> > >   [FixedPcd]
> > >     gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
> > > diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> > > b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> > > index f9045fd5df45..ea9159857215 100644
> > > --- a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> > > +++ b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
> > > @@ -10,6 +10,18 @@
> > >   #ifndef FDT_HELPER_LIB_
> > >   #define FDT_HELPER_LIB_
> > > +/**
> > > +  Get MPIDR for a given cpu from device tree passed by Qemu.
> > > +
> > > +  @param [in]   CpuId    Index of cpu to retrieve MPIDR value for.
> > > +
> > > +  @retval    MPIDR value of CPU at index 
> > > +**/
> > > +UINT64
> > > +FdtHelperGetMpidr (
> > > +  IN UINTN   CpuId
> > > +  );
> > > +
> > >   /** Walks through the Device Tree created by Qemu and counts the number
> > >   of CPUs present in it.
> > > diff --git
> > > a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> > > b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> > > index 84120f1c1b51..b8901030ecd0 100644
> > > --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> > > +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> > > @@ -20,39 +20,6 @@
> > >   #include 
> > >   #include 
> > >   #include 
> > > -#include 
> > > -#include 
> > > -
> > > -STATIC INT32 FdtFirstCpuOffset;
> > > -STATIC INT32 FdtCpuNodeSize;
> > > -
> > > -/*
> > > - * Get MPIDR from device tree passed by Qemu
> > > - */
> > > -STATIC
> > > -UINT64
> > > -GetMpidr (
> > > -  IN UINTN   CpuId
> > > -  )
> > > -{
> > > -  VOID   *DeviceTreeBase;
> > > -

Re: [edk2-devel] [PATCH] drop Tanmay Jagdale from sbsa-ref maintainers

2021-03-02 Thread Leif Lindholm
On Tue, Mar 02, 2021 at 15:10:16 +0100, Marcin Juszkiewicz wrote:
> Tanmay is no longer at Linaro
> 
> Signed-off-by: Marcin Juszkiewicz 

Reviewed-by: Leif Lindholm 
Thanks!

Pushed as db922e1253cb.

> ---
>  Maintainers.txt | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git Maintainers.txt Maintainers.txt
> index 2e6e87bb6d..afbd2cff0e 100644
> --- Maintainers.txt
> +++ Maintainers.txt
> @@ -295,7 +295,6 @@ M: Ard Biesheuvel 
>  M: Leif Lindholm 
>  R: Graeme Gregory 
>  R: Radoslaw Biernacki 
> -R: Tanmay Jagdale 
>  
>  Raspberry Pi platforms and silicon
>  F: Platform/RaspberryPi/
> -- 
> 2.29.2
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72340): https://edk2.groups.io/g/devel/message/72340
Mute This Topic: https://groups.io/mt/81026926/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH edk2-test 1/1] SctPkg: remove CR in uefi-sct/SctPkg/build.sh

2021-03-02 Thread G Edhaya Chandran
Reviewed-by: G Edhaya Chandran

> -Original Message-
> From: Heinrich Schuchardt 
> Sent: 26 February 2021 18:10
> To: EDK II Development 
> Cc: Eric Jin ; G Edhaya Chandran
> ; Barton Gao ; Arvin
> Chen ; Samer El-Haj-Mahmoud  mahm...@arm.com>; Heinrich Schuchardt 
> Subject: [PATCH edk2-test 1/1] SctPkg: remove CR in uefi-sct/SctPkg/build.sh
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3239
>
> A superfluous carriage return leads to an error:
>
> uefi-sct/SctPkg/build.sh: line 61:
> syntax error near unexpected token `fi'
> uefi-sct/SctPkg/build.sh: line 61:
> `fi'
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  uefi-sct/SctPkg/build.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/uefi-sct/SctPkg/build.sh b/uefi-sct/SctPkg/build.sh index
> 2efc5535b8fc..de7a10034e3d 100755
> --- a/uefi-sct/SctPkg/build.sh
> +++ b/uefi-sct/SctPkg/build.sh
> @@ -56,7 +56,7 @@ function get_gcc_version  {
>  gcc_version=$($1 -dumpversion)
>
> -if [ ${gcc_version%%.*} -ge 5 ]; then
> +if [ ${gcc_version%%.*} -ge 5 ]; then
>  gcc_version=5
>  fi
>
> --
> 2.30.0

IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72341): https://edk2.groups.io/g/devel/message/72341
Mute This Topic: https://groups.io/mt/80925661/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH edk2-platforms 1/1] Silicon/Qemu: Move SbsaQemu MPIDR-retrieval function to FdtHelperLib

2021-03-02 Thread Marcin Juszkiewicz

W dniu 02.03.2021 o 15:14, Graeme Gregory pisze:

On 02/03/2021 13:38, Leif Lindholm wrote:
Commit 822634fc1bf1 ("SbsaQemu: Update SbsaQemuAcpiDxe to use 
FdtHelperLib")
replaced the CountCpusFromFdt() function in SbsaQemuAcpiDxe with a 
call to
FdtHelperCountCpus() in FdtHelperLib. This ended up leaving static 
variables
FdtFirstCpuOffset and FdtCpuNodeSize uninitialised, such that the 
GetMpidr()

function kept returning the value for cpu 0.

Resolve this by moving the GetMpidr() function over to FdtHelperLib, 
where

it can again share these variables with FdtHelperCountCpus().

Fix up coding style issues as part of copy:
- Add m prefix to module-global variables.
- Add doxygen function comment header.

Cc: Ard Biesheuvel 
Cc: Graeme Gregory 
Cc: Radoslaw Biernacki 
Cc: Tanmay Jagdale 
Cc: Rebecca Cran 
Reported-by: Marcin Juszkiewicz 
Signed-off-by: Leif Lindholm 


Tested-By: Graeme Gregory 


Tested-by: Marcin Juszkiewicz 

sbsa-acs now finish in seconds like before.




---
  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf |  
2 --
  Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf   |  
5 +++
  Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h  | 
12 +++
  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c   | 
35 +--
  Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c | 
36 

  5 files changed, 54 insertions(+), 36 deletions(-)

diff --git 
a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf 
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf

index a58ebfaf76d5..c6de685bd2c4 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
@@ -35,7 +35,6 @@ [LibraryClasses]
    DebugLib
    DxeServicesLib
    FdtHelperLib
-  FdtLib
    PcdLib
    PrintLib
    UefiDriverEntryPoint
@@ -46,7 +45,6 @@ [Pcd]
    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile
    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount
    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdClusterCount
-  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
  [Depex]
    gEfiAcpiTableProtocolGuid   ## CONSUMES
diff --git 
a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf 
b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf

index d84c16f888d1..9c059f3e5851 100644
--- a/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
+++ b/Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.inf
@@ -24,5 +24,10 @@ [Packages]
    MdePkg/MdePkg.dec
    Silicon/Qemu/SbsaQemu/SbsaQemu.dec
+[LibraryClasses]
+  DebugLib
+  FdtLib
+  PcdLib
+
  [FixedPcd]
    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress
diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h 
b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h

index f9045fd5df45..ea9159857215 100644
--- a/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
+++ b/Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h
@@ -10,6 +10,18 @@
  #ifndef FDT_HELPER_LIB_
  #define FDT_HELPER_LIB_
+/**
+  Get MPIDR for a given cpu from device tree passed by Qemu.
+
+  @param [in]   CpuId    Index of cpu to retrieve MPIDR value for.
+
+  @retval    MPIDR value of CPU at index 
+**/
+UINT64
+FdtHelperGetMpidr (
+  IN UINTN   CpuId
+  );
+
  /** Walks through the Device Tree created by Qemu and counts the number
  of CPUs present in it.
diff --git 
a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c 
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c

index 84120f1c1b51..b8901030ecd0 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -20,39 +20,6 @@
  #include 
  #include 
  #include 
-#include 
-#include 
-
-STATIC INT32 FdtFirstCpuOffset;
-STATIC INT32 FdtCpuNodeSize;
-
-/*
- * Get MPIDR from device tree passed by Qemu
- */
-STATIC
-UINT64
-GetMpidr (
-  IN UINTN   CpuId
-  )
-{
-  VOID   *DeviceTreeBase;
-  CONST UINT64   *RegVal;
-  INT32  Len;
-
-  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
-  ASSERT (DeviceTreeBase != NULL);
-
-  RegVal = fdt_getprop (DeviceTreeBase,
- FdtFirstCpuOffset + (CpuId * FdtCpuNodeSize),
- "reg",
- &Len);
-  if (!RegVal) {
-    DEBUG ((DEBUG_ERROR, "Couldn't find reg property for CPU:%d\n", 
CpuId));

-    return 0;
-  }
-
-  return (fdt64_to_cpu (ReadUnaligned64 (RegVal)));
-}
  /*
   * A Function to Compute the ACPI Table Checksum
@@ -159,7 +126,7 @@ AddMadtTable (
  CopyMem (New, &Gicc, sizeof (EFI_ACPI_6_0_GIC_STRUCTURE));
  GiccPtr = (EFI_ACPI_6_0_GIC_STRUCTURE *) New;
  GiccPtr->AcpiProcessorUid = CoreIndex;
-    GiccPtr->MPIDR = GetMpidr (CoreIndex);
+    GiccPtr->MPIDR = FdtHelperGetMpidr (CoreIndex);
  New += 

[edk2-devel] [PATCH v4 1/7] MdePkg: MmUnblockMemoryLib: Added definition and null instance

2021-03-02 Thread Kun Qin
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3168

This interface provides an abstration layer to allow MM modules to access
requested areas that are outside of MMRAM. On MM model that blocks all
non-MMRAM accesses, areas requested through this API will be mapped or
unblocked for accessibility inside MM environment.

For MM modules that need to access regions outside of MMRAMs, the agents
that set up these regions are responsible for invoking this API in order
for these memory areas to be accessible from inside MM.

Example usages:
1. To enable runtime cache feature for variable service, Variable MM
module will need to access the allocated runtime buffer. Thus the agent
sets up these buffers, VariableSmmRuntimeDxe, will need to invoke this
API to make these regions accessible by Variable MM.
2. For TPM ACPI table to communicate to physical presence handler, the
corresponding NVS region has to be accessible from inside MM. Once the
NVS region are assigned, it needs to be unblocked thourgh this API.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Hao A Wu 
Cc: Jiewen Yao 
Cc: Laszlo Ersek 

Signed-off-by: Kun Qin 
---

Notes:
v4:
- Added more commit message [Laszlo]
- Added UNI file [Hao]

v3:
- Move interface to MdePkg [Hao, Liming, Jiewen]
- Remove Dxe prefix [Jiewen]

v2:
- Resend with practical usage. No change [Hao]

 MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c   | 44 

 MdePkg/Include/Library/MmUnblockMemoryLib.h  | 44 

 MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf | 34 
+++
 MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.uni | 21 ++
 MdePkg/MdePkg.dec|  5 +++
 MdePkg/MdePkg.dsc|  1 +
 6 files changed, 149 insertions(+)

diff --git a/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c 
b/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c
new file mode 100644
index ..abdce41f10d1
--- /dev/null
+++ b/MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c
@@ -0,0 +1,44 @@
+/** @file
+  Null instance of MM Unblock Page Library.
+
+  This library provides an interface to request non-MMRAM pages to be 
mapped/unblocked
+  from inside MM environment.
+
+  For MM modules that need to access regions outside of MMRAMs, the agents 
that set up
+  these regions are responsible for invoking this API in order for these 
memory areas
+  to be accessed from inside MM.
+
+  Copyright (c) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+
+/**
+  This API provides a way to unblock certain data pages to be accessible 
inside MM environment.
+
+  @param  UnblockAddress  The address of buffer caller requests to 
unblock, the address
+  has to be page aligned.
+  @param  NumberOfPages   The number of pages requested to be 
unblocked from MM
+  environment.
+
+  @return EFI_SUCCESS The request goes through successfully.
+  @return EFI_NOT_AVAILABLE_YET   The requested functionality is not produced 
yet.
+  @return EFI_UNSUPPORTED The requested functionality is not supported 
on current platform.
+  @return EFI_SECURITY_VIOLATION  The requested address failed to pass 
security check for
+  unblocking.
+  @return EFI_INVALID_PARAMETER   Input address either NULL pointer or not 
page aligned.
+  @return EFI_ACCESS_DENIED   The request is rejected due to system has 
passed certain boot
+  phase.
+
+**/
+EFI_STATUS
+EFIAPI
+MmUnblockMemoryRequest (
+  IN EFI_PHYSICAL_ADDRESS   UnblockAddress,
+  IN UINT64 NumberOfPages
+  )
+{
+  return RETURN_UNSUPPORTED;
+}
diff --git a/MdePkg/Include/Library/MmUnblockMemoryLib.h 
b/MdePkg/Include/Library/MmUnblockMemoryLib.h
new file mode 100644
index ..980afe9a5fd3
--- /dev/null
+++ b/MdePkg/Include/Library/MmUnblockMemoryLib.h
@@ -0,0 +1,44 @@
+/** @file
+  MM Unblock Memory Library Interface.
+
+  This library provides an interface to request non-MMRAM pages to be 
mapped/unblocked
+  from inside MM environment.
+
+  For MM modules that need to access regions outside of MMRAMs, the agents 
that set up
+  these regions are responsible for invoking this API in order for these 
memory areas
+  to be accessed from inside MM.
+
+  Copyright (c) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef MM_UNBLOCK_MEMORY_LIB_H_
+#define MM_UNBLOCK_MEMORY_LIB_H_
+
+/**
+  This API provides a way to unblock certain data pages to be accessible 
inside MM environment.
+
+  @param  UnblockAddress  The address of buffer caller requests to 
unblock, the address
+  has to be page aligned.
+  @param  NumberOfPages  

[edk2-devel] [PATCH v4 0/7] Add MmUnblockMemoryLib Interface and Usages

2021-03-02 Thread Kun Qin
This patch series is a follow up of previous submission:
https://edk2.groups.io/g/devel/message/72239

The module changes are validated on two different physical platforms and
QEMU based Q35 plastform. Standalone and traditional MM are both tested
to be functional on these systems.

v4 patches mainly focus on feedback for reviewed commits in v3 patches,
including:
a. Adding "Reviewed-by" tags for applicable patches;
b. Added UNI file for MmUnblockMemoryLib;
c. Added description in commit message for the patch that adds interface;
d. Changed dependency module from library to DXE driver;

Patch v4 branch: https://github.com/kuqin12/edk2/tree/unblock_mem_v4

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Hao A Wu 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Jordan Justen 
Cc: Qi Zhang 
Cc: Rahul Kumar 

Kun Qin (7):
  MdePkg: MmUnblockMemoryLib: Added definition and null instance
  OvmfPkg: resolve MmUnblockMemoryLib (mainly for VariableSmmRuntimeDxe)
  MdeModulePkg: VariableSmmRuntimeDxe: Added request unblock memory
interface
  SecurityPkg: Tcg2Smm: Switching from gSmst to gMmst
  SecurityPkg: Tcg2Smm: Separate Tcg2Smm into 2 modules
  SecurityPkg: Tcg2Smm: Added support for Standalone Mm
  SecurityPkg: Tcg2Acpi: Added unblock memory interface for NVS region

 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c   |  42 +
 MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c   |  44 +
 SecurityPkg/Tcg/{Tcg2Smm/Tcg2Smm.c => Tcg2Acpi/Tcg2Acpi.c}   | 358 

 SecurityPkg/Tcg/Tcg2Smm/Tcg2MmDependencyDxe.c|  48 ++
 SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c| 857 

 SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.c   |  71 ++
 SecurityPkg/Tcg/Tcg2Smm/Tcg2TraditionalMm.c  |  82 ++
 MdeModulePkg/MdeModulePkg.dsc|   1 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf |   1 +
 MdePkg/Include/Library/MmUnblockMemoryLib.h  |  44 +
 MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf |  34 +
 MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.uni |  21 +
 MdePkg/MdePkg.dec|   5 +
 MdePkg/MdePkg.dsc|   1 +
 OvmfPkg/OvmfPkgIa32.dsc  |   3 +
 OvmfPkg/OvmfPkgIa32X64.dsc   |   3 +
 OvmfPkg/OvmfPkgX64.dsc   |   3 +
 SecurityPkg/Include/Guid/TpmNvsMm.h  |  68 ++
 SecurityPkg/SecurityPkg.ci.yaml  |   1 +
 SecurityPkg/SecurityPkg.dec  |   8 +
 SecurityPkg/SecurityPkg.dsc  |  12 +
 SecurityPkg/Tcg/{Tcg2Smm/Tcg2Smm.inf => Tcg2Acpi/Tcg2Acpi.inf}   |  35 +-
 SecurityPkg/Tcg/{Tcg2Smm => Tcg2Acpi}/Tpm.asl|   0
 SecurityPkg/Tcg/Tcg2Smm/Tcg2MmDependencyDxe.inf  |  43 +
 SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h| 121 +--
 SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf  |  27 +-
 SecurityPkg/Tcg/Tcg2Smm/{Tcg2Smm.inf => Tcg2StandaloneMm.inf}|  50 +-
 27 files changed, 950 insertions(+), 1033 deletions(-)
 create mode 100644 MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.c
 copy SecurityPkg/Tcg/{Tcg2Smm/Tcg2Smm.c => Tcg2Acpi/Tcg2Acpi.c} (72%)
 create mode 100644 SecurityPkg/Tcg/Tcg2Smm/Tcg2MmDependencyDxe.c
 create mode 100644 SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.c
 create mode 100644 SecurityPkg/Tcg/Tcg2Smm/Tcg2TraditionalMm.c
 create mode 100644 MdePkg/Include/Library/MmUnblockMemoryLib.h
 create mode 100644 MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
 create mode 100644 MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.uni
 create mode 100644 SecurityPkg/Include/Guid/TpmNvsMm.h
 copy SecurityPkg/Tcg/{Tcg2Smm/Tcg2Smm.inf => Tcg2Acpi/Tcg2Acpi.inf} (76%)
 rename SecurityPkg/Tcg/{Tcg2Smm => Tcg2Acpi}/Tpm.asl (100%)
 create mode 100644 SecurityPkg/Tcg/Tcg2Smm/Tcg2MmDependencyDxe.inf
 copy SecurityPkg/Tcg/Tcg2Smm/{Tcg2Smm.inf => Tcg2StandaloneMm.inf} (52%)

-- 
2.30.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72343): https://edk2.groups.io/g/devel/message/72343
Mute This Topic: https://groups.io/mt/81035307/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v4 2/7] OvmfPkg: resolve MmUnblockMemoryLib (mainly for VariableSmmRuntimeDxe)

2021-03-02 Thread Kun Qin
This change added NULL MmUnblockMemoryLib instance in dsc files of
OvmfPkg to pass CI build. When SMM_REQUIRE flag is set, the library
interface is consumed by VariableSmmRuntimeDxe to better support variable
runtime cache feature.

Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Jordan Justen 

Signed-off-by: Kun Qin 
Reviewed-by: Laszlo Ersek 
---

Notes:
v4:
- Updated patch title. [Laszlo]
- Moved this patch before the variable driver change. [Laszlo]
- Added reviewed-by tag. [Laszlo]

v3:
- Newly added in v3. [Hao]

 OvmfPkg/OvmfPkgIa32.dsc| 3 +++
 OvmfPkg/OvmfPkgIa32X64.dsc | 3 +++
 OvmfPkg/OvmfPkgX64.dsc | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 1b8d34052b01..1eaf3e99c6c5 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -347,6 +347,9 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
   
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf
+!if $(SMM_REQUIRE) == TRUE
+  
MmUnblockMemoryLib|MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
+!endif
 
 [LibraryClasses.common.UEFI_DRIVER]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 9c1aee87e783..4a5a43014725 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -351,6 +351,9 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
   
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf
+!if $(SMM_REQUIRE) == TRUE
+  
MmUnblockMemoryLib|MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
+!endif
 
 [LibraryClasses.common.UEFI_DRIVER]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index fabb8b2f29e4..d4d601b44476 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -353,6 +353,9 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
   
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf
+!if $(SMM_REQUIRE) == TRUE
+  
MmUnblockMemoryLib|MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
+!endif
 
 [LibraryClasses.common.UEFI_DRIVER]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
-- 
2.30.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72345): https://edk2.groups.io/g/devel/message/72345
Mute This Topic: https://groups.io/mt/81035311/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v4 4/7] SecurityPkg: Tcg2Smm: Switching from gSmst to gMmst

2021-03-02 Thread Kun Qin
This change replaced gSmst with gMmst to support broader compatibility
under MM environment for Tcg2Smm driver.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Qi Zhang 
Cc: Rahul Kumar 

Signed-off-by: Kun Qin 
Reviewed-by: Jiewen Yao 
---

Notes:
v4:
- Previously reviewed, no change.

v3:
- Added reviewed-by tag. [Jiewen]

v2:
- Newly added in v2.

 SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c   | 4 ++--
 SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h   | 2 +-
 SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c 
b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
index 91aebb62b8bf..08105c3692ba 100644
--- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
+++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
@@ -870,7 +870,7 @@ InitializeTcgSmm (
   //
   // Get the Sw dispatch protocol and register SMI callback functions.
   //
-  Status = gSmst->SmmLocateProtocol (&gEfiSmmSwDispatch2ProtocolGuid, NULL, 
(VOID**)&SwDispatch);
+  Status = gMmst->MmLocateProtocol (&gEfiSmmSwDispatch2ProtocolGuid, NULL, 
(VOID**)&SwDispatch);
   ASSERT_EFI_ERROR (Status);
   SwContext.SwSmiInputValue = (UINTN) -1;
   Status = SwDispatch->Register (SwDispatch, PhysicalPresenceCallback, 
&SwContext, &SwHandle);
@@ -891,7 +891,7 @@ InitializeTcgSmm (
   //
   // Locate SmmVariableProtocol.
   //
-  Status = gSmst->SmmLocateProtocol (&gEfiSmmVariableProtocolGuid, NULL, 
(VOID**)&mSmmVariable);
+  Status = gMmst->MmLocateProtocol (&gEfiSmmVariableProtocolGuid, NULL, 
(VOID**)&mSmmVariable);
   ASSERT_EFI_ERROR (Status);
 
   //
diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h 
b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
index fd19e7dc0553..d7328c8f2ac9 100644
--- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
+++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
@@ -24,7 +24,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf 
b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
index 2ebf2e05f2ea..872ed27cbe71 100644
--- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
+++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
@@ -50,7 +50,7 @@ [LibraryClasses]
   BaseLib
   BaseMemoryLib
   UefiDriverEntryPoint
-  SmmServicesTableLib
+  MmServicesTableLib
   UefiBootServicesTableLib
   DebugLib
   DxeServicesLib
-- 
2.30.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72347): https://edk2.groups.io/g/devel/message/72347
Mute This Topic: https://groups.io/mt/81035313/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v4 3/7] MdeModulePkg: VariableSmmRuntimeDxe: Added request unblock memory interface

2021-03-02 Thread Kun Qin
This changes added usage of MmUnblockMemoryLib to explicitly request
runtime cache regions(and its indicators) to be accessible from MM
environment when PcdEnableVariableRuntimeCache is enabled. It will bring
in compatibility with architectures that supports full memory blockage
inside MM.

Cc: Jian J Wang 
Cc: Hao A Wu 
Cc: Liming Gao 

Signed-off-by: Kun Qin 
Reviewed-by: Hao A Wu 
---

Notes:
v4:
- Added reviewed-by tag. [Hao]

v3:
- Removed Dxe prefix to match interface change. [Jiewen]

v2:
- Newly added in v2.

 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c   | 42 

 MdeModulePkg/MdeModulePkg.dsc|  1 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf |  1 +
 3 files changed, 44 insertions(+)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index c47e614d81f4..a166d284dfe4 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -35,6 +35,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -165,6 +166,7 @@ InitVariableCache (
   )
 {
   VARIABLE_STORE_HEADER   *VariableCacheStorePtr;
+  EFI_STATUS  Status;
 
   if (TotalVariableCacheSize == NULL) {
 return EFI_INVALID_PARAMETER;
@@ -186,6 +188,18 @@ InitVariableCache (
   if (*VariableCacheBuffer == NULL) {
 return EFI_OUT_OF_RESOURCES;
   }
+
+  //
+  // Request to unblock the newly allocated cache region to be accessible from 
inside MM
+  //
+  Status = MmUnblockMemoryRequest (
+(EFI_PHYSICAL_ADDRESS) (UINTN) *VariableCacheBuffer,
+EFI_SIZE_TO_PAGES (*TotalVariableCacheSize)
+);
+  if (Status != EFI_UNSUPPORTED && EFI_ERROR (Status)) {
+return Status;
+  }
+
   VariableCacheStorePtr = *VariableCacheBuffer;
   SetMem32 ((VOID *) VariableCacheStorePtr, *TotalVariableCacheSize, (UINT32) 
0x);
 
@@ -1536,6 +1550,34 @@ SendRuntimeVariableCacheContextToSmm (
   SmmRuntimeVarCacheContext->ReadLock = &mVariableRuntimeCacheReadLock;
   SmmRuntimeVarCacheContext->HobFlushComplete = &mHobFlushComplete;
 
+  //
+  // Request to unblock this region to be accessible from inside MM environment
+  // These fields "should" be all on the same page, but just to be on the safe 
side...
+  //
+  Status = MmUnblockMemoryRequest (
+(EFI_PHYSICAL_ADDRESS) ALIGN_VALUE ((UINTN) 
SmmRuntimeVarCacheContext->PendingUpdate - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE),
+EFI_SIZE_TO_PAGES (sizeof(mVariableRuntimeCachePendingUpdate))
+);
+  if (Status != EFI_UNSUPPORTED && EFI_ERROR (Status)) {
+goto Done;
+  }
+
+  Status = MmUnblockMemoryRequest (
+(EFI_PHYSICAL_ADDRESS) ALIGN_VALUE ((UINTN) 
SmmRuntimeVarCacheContext->ReadLock - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE),
+EFI_SIZE_TO_PAGES (sizeof(mVariableRuntimeCacheReadLock))
+);
+  if (Status != EFI_UNSUPPORTED && EFI_ERROR (Status)) {
+goto Done;
+  }
+
+  Status = MmUnblockMemoryRequest (
+(EFI_PHYSICAL_ADDRESS) ALIGN_VALUE ((UINTN) 
SmmRuntimeVarCacheContext->HobFlushComplete - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE),
+EFI_SIZE_TO_PAGES (sizeof(mHobFlushComplete))
+);
+  if (Status != EFI_UNSUPPORTED && EFI_ERROR (Status)) {
+goto Done;
+  }
+
   //
   // Send data to SMM.
   //
diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index 7ca4a1bb3080..9272da89a998 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -100,6 +100,7 @@ [LibraryClasses]
   SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
   
DisplayUpdateProgressLib|MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.inf
   
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
+  
MmUnblockMemoryLib|MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
 
 [LibraryClasses.EBC.PEIM]
   IoLib|MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf
diff --git 
a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
index b6dbc839e023..a0d8b2267e92 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
@@ -60,6 +60,7 @@ [LibraryClasses]
   TpmMeasurementLib
   SafeIntLib
   PcdLib
+  MmUnblockMemoryLib
 
 [Protocols]
   gEfiVariableWriteArchProtocolGuid ## PRODUCES
-- 
2.30.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72346): https://edk2.groups.io/g/devel/message/72346
Mute This Topic: https://groups.io/m

[edk2-devel] [PATCH v4 5/7] SecurityPkg: Tcg2Smm: Separate Tcg2Smm into 2 modules

2021-03-02 Thread Kun Qin
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3169

This change separated the original Tcg2Smm module into 2 drivers: the
SMM driver that registers callback for physical presence and memory
clear; the Tcg2Acpi driver that patches and publishes ACPI table for
runtime use.

Tcg2Smm introduced an SMI root handler to allow Tcg2Acpi to communicate
the NVS region used by Tpm.asl and exchange the registered SwSmiValue.

Lastly, Tcg2Smm driver will publish gTcg2MmSwSmiRegisteredGuid at the end
of entrypoint to ensure Tcg2Acpi to load after Tcg2Smm is ready to
communicate.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Qi Zhang 
Cc: Rahul Kumar 

Signed-off-by: Kun Qin 
Reviewed-by: Jiewen Yao 
---

Notes:
v4:
- Previously reviewed, no change.

v3:
- Added review-by tag. [Jiewen]
- Added expected usage in driver header.
- Initialized pointer variables to null at entrypoint.

v2:
- Newly added.

 SecurityPkg/Tcg/{Tcg2Smm/Tcg2Smm.c => Tcg2Acpi/Tcg2Acpi.c} | 352 
 SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c  | 853 

 SecurityPkg/Tcg/Tcg2Smm/Tcg2TraditionalMm.c|  82 ++
 SecurityPkg/Include/Guid/TpmNvsMm.h|  68 ++
 SecurityPkg/SecurityPkg.dec|   7 +
 SecurityPkg/SecurityPkg.dsc|   1 +
 SecurityPkg/Tcg/{Tcg2Smm/Tcg2Smm.inf => Tcg2Acpi/Tcg2Acpi.inf} |  34 +-
 SecurityPkg/Tcg/{Tcg2Smm => Tcg2Acpi}/Tpm.asl  |   0
 SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h  | 119 +--
 SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf|  25 +-
 10 files changed, 549 insertions(+), 992 deletions(-)

diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c 
b/SecurityPkg/Tcg/Tcg2Acpi/Tcg2Acpi.c
similarity index 72%
copy from SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
copy to SecurityPkg/Tcg/Tcg2Acpi/Tcg2Acpi.c
index 08105c3692ba..9d6bc09bdc0d 100644
--- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
+++ b/SecurityPkg/Tcg/Tcg2Acpi/Tcg2Acpi.c
@@ -1,20 +1,76 @@
 /** @file
-  It updates TPM2 items in ACPI table and registers SMI2 callback
-  functions for Tcg2 physical presence, ClearMemory, and sample
-  for dTPM StartMethod.
+  This driver implements TPM 2.0 definition block in ACPI table and
+  populates registered SMI callback functions for Tcg2 physical presence
+  and MemoryClear to handle the requests for ACPI method. It needs to be
+  used together with Tcg2 MM drivers to exchange information on registered
+  SwSmiValue and allocated NVS region address.
 
   Caution: This module requires additional review when modified.
   This driver will have external input - variable and ACPINvs data in SMM mode.
   This external input must be validated carefully to avoid security issue.
 
-  PhysicalPresenceCallback() and MemoryClearCallback() will receive untrusted 
input and do some check.
-
 Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) Microsoft Corporation.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
-#include "Tcg2Smm.h"
+#include 
+
+#include 
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+//
+// Physical Presence Interface Version supported by Platform
+//
+#define PHYSICAL_PRESENCE_VERSION_TAG  "$PV"
+#define PHYSICAL_PRESENCE_VERSION_SIZE 4
+
+//
+// PNP _HID for TPM2 device
+//
+#define TPM_HID_TAG""
+#define TPM_HID_PNP_SIZE   8
+#define TPM_HID_ACPI_SIZE  9
+
+#define TPM_PRS_RESL   "RESL"
+#define TPM_PRS_RESS   "RESS"
+#define TPM_PRS_RES_NAME_SIZE  4
+//
+// Minimum PRS resource template size
+//  1 bytefor  BufferOp
+//  1 bytefor  PkgLength
+//  2 bytes   for  BufferSize
+//  12 bytes  for  Memory32Fixed descriptor
+//  5 bytes   for  Interrupt descriptor
+//  2 bytes   for  END Tag
+//
+#define TPM_POS_RES_TEMPLATE_MIN_SIZE  (1 + 1 + 2 
+ 12 + 5 + 2)
+
+//
+// Max Interrupt buffer size for PRS interrupt resource
+// Now support 15 interrupts in maxmum
+//
+#define MAX_PRS_INT_BUF_SIZE   (15*4)
 
 #pragma pack(1)
 
@@ -49,142 +105,8 @@ EFI_TPM2_ACPI_TABLE_V4  mTpm2AcpiTemplate = {
   EFI_TPM2_ACPI_TABLE_START_METHOD_TIS, // StartMethod
 };
 
-EFI_SMM_VARIABLE_PROTOCOL  *mSmmVariable;
 TCG_NVS*mTcgNvs;
 
-/**
-  Software SMI callback for TPM physical presence which is called from ACPI 
method.
-
-  Caution: This function may receive untrusted input.
-  Variable and ACPINvs are external input, so this function will validate
-  its 

[edk2-devel] [PATCH v4 6/7] SecurityPkg: Tcg2Smm: Added support for Standalone Mm

2021-03-02 Thread Kun Qin
https://bugzilla.tianocore.org/show_bug.cgi?id=3169

This change added Standalone MM instance of Tcg2. The notify function for
Standalone MM instance is left empty.

A dependency DXE driver with a Depex of gEfiMmCommunication2ProtocolGuid
was created to indicate the readiness of Standalone MM Tcg2 driver.

Lastly, the support of CI build for Tcg2 Standalone MM module is added.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Qi Zhang 
Cc: Rahul Kumar 

Signed-off-by: Kun Qin 
---

Notes:
v4:
- Changed dependency module from anonymous lib to Dxe driver. [Jiewen]

v3:
- No change.

v2:
- Newly added.

 SecurityPkg/Tcg/Tcg2Smm/Tcg2MmDependencyDxe.c   | 48 
 SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.c  | 71 ++
 SecurityPkg/SecurityPkg.ci.yaml |  1 +
 SecurityPkg/SecurityPkg.dec |  1 +
 SecurityPkg/SecurityPkg.dsc | 10 +++
 SecurityPkg/Tcg/Tcg2Smm/Tcg2MmDependencyDxe.inf | 43 +++
 SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.inf| 77 
 7 files changed, 251 insertions(+)

diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2MmDependencyDxe.c 
b/SecurityPkg/Tcg/Tcg2Smm/Tcg2MmDependencyDxe.c
new file mode 100644
index ..4f2d7c58ed86
--- /dev/null
+++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2MmDependencyDxe.c
@@ -0,0 +1,48 @@
+/** @file
+  Runtime DXE part corresponding to StandaloneMM Tcg2 module.
+
+This module installs gTcg2MmSwSmiRegisteredGuid to notify readiness of
+StandaloneMM Tcg2 module.
+
+Copyright (c) 2019 - 2021, Arm Ltd. All rights reserved.
+Copyright (c) Microsoft Corporation.
+
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+
+#include 
+#include 
+
+/**
+  The constructor function installs gTcg2MmSwSmiRegisteredGuid to notify
+  readiness of StandaloneMM Tcg2 module.
+
+  @param  ImageHandle   The firmware allocated handle for the EFI image.
+  @param  SystemTable   A pointer to the Management mode System Table.
+
+  @retval EFI_SUCCESS   The constructor always returns EFI_SUCCESS.
+
+**/
+EFI_STATUS
+EFIAPI
+Tcg2MmDependencyDxeEntryPoint (
+  IN EFI_HANDLE   ImageHandle,
+  IN EFI_SYSTEM_TABLE *SystemTable
+  )
+{
+  EFI_STATUSStatus;
+  EFI_HANDLEHandle;
+
+  Handle = NULL;
+  Status = gBS->InstallProtocolInterface (
+  &Handle,
+  &gTcg2MmSwSmiRegisteredGuid,
+  EFI_NATIVE_INTERFACE,
+  NULL
+  );
+  ASSERT_EFI_ERROR (Status);
+  return EFI_SUCCESS;
+}
diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.c 
b/SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.c
new file mode 100644
index ..9e0095efbc5e
--- /dev/null
+++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.c
@@ -0,0 +1,71 @@
+/** @file
+  TCG2 Standalone MM driver that updates TPM2 items in ACPI table and registers
+  SMI2 callback functions for Tcg2 physical presence, ClearMemory, and
+  sample for dTPM StartMethod.
+
+  Caution: This module requires additional review when modified.
+  This driver will have external input - variable and ACPINvs data in SMM mode.
+  This external input must be validated carefully to avoid security issue.
+
+  PhysicalPresenceCallback() and MemoryClearCallback() will receive untrusted 
input and do some check.
+
+Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) Microsoft Corporation.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "Tcg2Smm.h"
+#include 
+
+/**
+  Notify the system that the SMM variable driver is ready.
+**/
+VOID
+Tcg2NotifyMmReady (
+  VOID
+  )
+{
+  // Do nothing
+}
+
+/**
+  This function is an abstraction layer for implementation specific Mm buffer 
validation routine.
+
+  @param Buffer  The buffer start address to be checked.
+  @param Length  The buffer length to be checked.
+
+  @retval TRUE  This buffer is valid per processor architecture and not 
overlap with SMRAM.
+  @retval FALSE This buffer is not valid per processor architecture or overlap 
with SMRAM.
+**/
+BOOLEAN
+IsBufferOutsideMmValid (
+  IN EFI_PHYSICAL_ADDRESS  Buffer,
+  IN UINT64Length
+  )
+{
+  return MmIsBufferOutsideMmValid (Buffer, Length);
+}
+
+/**
+  The driver's entry point.
+
+  It install callbacks for TPM physical presence and MemoryClear, and locate
+  SMM variable to be used in the callback function.
+
+  @param[in] ImageHandle  The firmware allocated handle for the EFI image.
+  @param[in] SystemTable  A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS The entry point is executed successfully.
+  @retval Others  Some error occurs when executing this entry point.
+
+**/
+EFI_STATUS
+EFIAPI
+InitializeTcgStandaloneMm (
+  IN EFI_HANDLE  ImageHandle,
+  IN EFI_MM_SYSTEM_TABLE *SystemTable
+  )
+{
+  return InitializeTcgCommon ();
+}
diff --git a/SecurityPkg/SecurityPkg.ci.yaml b/SecurityPkg/SecurityPkg.ci.yaml
i

[edk2-devel] [PATCH v4 7/7] SecurityPkg: Tcg2Acpi: Added unblock memory interface for NVS region

2021-03-02 Thread Kun Qin
This changes added usage of MmUnblockMemoryLib to explicitly request
allocated NVS region to be accessible from MM environment. It will bring
in compatibility with architectures that supports full memory blockage
inside MM.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Qi Zhang 
Cc: Rahul Kumar 

Signed-off-by: Kun Qin 
Reviewed-by: Jiewen Yao 
---

Notes:
v4:
- Previously reviewed, no change.

v3:
- Added review-by tag. [Jiewen]
- Remove Dxe prefix to match interface update. [Jiewen]

v2:
- Newly added in v2.

 SecurityPkg/Tcg/Tcg2Acpi/Tcg2Acpi.c   | 6 ++
 SecurityPkg/SecurityPkg.dsc   | 1 +
 SecurityPkg/Tcg/Tcg2Acpi/Tcg2Acpi.inf | 1 +
 3 files changed, 8 insertions(+)

diff --git a/SecurityPkg/Tcg/Tcg2Acpi/Tcg2Acpi.c 
b/SecurityPkg/Tcg/Tcg2Acpi/Tcg2Acpi.c
index 9d6bc09bdc0d..db2e56b6122c 100644
--- a/SecurityPkg/Tcg/Tcg2Acpi/Tcg2Acpi.c
+++ b/SecurityPkg/Tcg/Tcg2Acpi/Tcg2Acpi.c
@@ -38,6 +38,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 #include 
+#include 
 
 //
 // Physical Presence Interface Version supported by Platform
@@ -147,6 +148,11 @@ AssignOpRegion (
   ZeroMem ((VOID *)(UINTN)MemoryAddress, Size);
   OpRegion->RegionOffset = (UINT32) (UINTN) MemoryAddress;
   OpRegion->RegionLen= (UINT8) Size;
+  // Request to unblock this region from MM core
+  Status = MmUnblockMemoryRequest (MemoryAddress, EFI_SIZE_TO_PAGES 
(Size));
+  if (Status != EFI_UNSUPPORTED && EFI_ERROR (Status)) {
+ASSERT_EFI_ERROR (Status);
+  }
   break;
 }
   }
diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc
index 74ec42966273..a77665518bdd 100644
--- a/SecurityPkg/SecurityPkg.dsc
+++ b/SecurityPkg/SecurityPkg.dsc
@@ -67,6 +67,7 @@ [LibraryClasses]
   VariableKeyLib|SecurityPkg/Library/VariableKeyLibNull/VariableKeyLibNull.inf
   RpmcLib|SecurityPkg/Library/RpmcLibNull/RpmcLibNull.inf
   
TcgEventLogRecordLib|SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLogRecordLib.inf
+  
MmUnblockMemoryLib|MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
 
 [LibraryClasses.ARM]
   #
diff --git a/SecurityPkg/Tcg/Tcg2Acpi/Tcg2Acpi.inf 
b/SecurityPkg/Tcg/Tcg2Acpi/Tcg2Acpi.inf
index 42ddb4bd1f39..f1c6ae5b1cb4 100644
--- a/SecurityPkg/Tcg/Tcg2Acpi/Tcg2Acpi.inf
+++ b/SecurityPkg/Tcg/Tcg2Acpi/Tcg2Acpi.inf
@@ -57,6 +57,7 @@ [LibraryClasses]
   Tpm2CommandLib
   Tcg2PhysicalPresenceLib
   PcdLib
+  MmUnblockMemoryLib
 
 [Guids]
   gEfiTpmDeviceInstanceTpm20DtpmGuid## PRODUCES
   ## GUID   # TPM device identifier
-- 
2.30.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72350): https://edk2.groups.io/g/devel/message/72350
Mute This Topic: https://groups.io/mt/81035317/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [RFC PATCH 06/14] OvmfPkg/AmdSev: Setup Migration Handler Mailbox

2021-03-02 Thread Tobin Feldman-Fitzthum
The migration handler communicates with the hypervisor using a
special mailbox, a page of shared memory where pending commands
can be written. Another shared page is used to pass the incoming
or outgoing guest memory pages. These pages are set aside in MEMFD,
which this patch expands, and reserved as runtime memory in
ConfidentialMigrationPei, which this patch introduces.

Signed-off-by: Tobin Feldman-Fitzthum 
---
 OvmfPkg/OvmfPkg.dec   |  5 +++
 OvmfPkg/AmdSev/AmdSevX64.dsc  |  1 +
 OvmfPkg/AmdSev/AmdSevX64.fdf  | 12 ---
 .../ConfidentialMigrationPei.inf  | 35 +++
 .../ConfidentialMigrationPei.c| 25 +
 5 files changed, 74 insertions(+), 4 deletions(-)
 create mode 100644 
OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
 create mode 100644 
OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.c

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 402c3b61fa..5c55e3c7c9 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -318,6 +318,11 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase|0x0|UINT32|0x42
   gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize|0x0|UINT32|0x43
 
+  ## Area used by the confidential migration handler to communicate with
+  # the hypervisor.
+  
gUefiOvmfPkgTokenSpaceGuid.PcdConfidentialMigrationMailboxBase|0x0|UINT32|0x48
+  
gUefiOvmfPkgTokenSpaceGuid.PcdConfidentialMigrationMailboxSize|0x0|UINT32|0x49
+
 [PcdsDynamic, PcdsDynamicEx]
   gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10
diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index fa68143663..4f748a0015 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -620,6 +620,7 @@
   UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
   UefiCpuPkg/CpuMpPei/CpuMpPei.inf
   OvmfPkg/AmdSev/SecretPei/SecretPei.inf
+  OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
 
 !if $(TPM_ENABLE) == TRUE
   OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
diff --git a/OvmfPkg/AmdSev/AmdSevX64.fdf b/OvmfPkg/AmdSev/AmdSevX64.fdf
index 6ef6dc89f2..94468f2ca0 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.fdf
+++ b/OvmfPkg/AmdSev/AmdSevX64.fdf
@@ -36,10 +36,10 @@ FV = SECFV
 
 [FD.MEMFD]
 BaseAddress   = $(MEMFD_BASE_ADDRESS)
-Size  = 0xD0
+Size  = 0xE0
 ErasePolarity = 1
 BlockSize = 0x1
-NumBlocks = 0xD0
+NumBlocks = 0xE0
 
 0x00|0x006000
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
@@ -68,11 +68,14 @@ 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTokenSpaceGuid.P
 0x01|0x01
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
 
-0x02|0x0E
+0x02|0x003000
+gUefiOvmfPkgTokenSpaceGuid.PcdConfidentialMigrationMailboxBase|gUefiOvmfPkgTokenSpaceGuid.PcdConfidentialMigrationMailboxSize
+
+0x12|0x0E
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
 FV = PEIFV
 
-0x10|0xC0
+0x20|0xC0
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
 FV = DXEFV
 
@@ -145,6 +148,7 @@ INF  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
 INF  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
 INF  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
 INF  OvmfPkg/AmdSev/SecretPei/SecretPei.inf
+INF  OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
 
 !if $(TPM_ENABLE) == TRUE
 INF  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf 
b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
new file mode 100644
index 00..918cf22abd
--- /dev/null
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
@@ -0,0 +1,35 @@
+## @file
+#  PEI support for confidential migration.
+#
+#  Copyright (C) 2021 IBM Corporation.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = ConfidentialMigration
+  FILE_GUID  = a747792e-71a1-4c24-84a9-a76a0a279878
+  MODULE_TYPE= PEIM
+  VERSION_STRING = 1.0
+  ENTRY_POINT= InitializeConfidentialMigrationPei
+
+[Sources]
+  ConfidentialMigrationPei.c
+
+[Packages]
+  OvmfPkg/OvmfPkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  HobLib
+  PeimEntryPoint
+  PcdLib
+
+[FixedPcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdConfidentialMigrationMailboxBase
+  gUefiOvmfPkgTokenSpaceGuid.PcdConfidentialMigrationMailboxSize
+
+[Depex]
+  TRUE
diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.c 
b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.c
new f

[edk2-devel] [RFC PATCH 04/14] OvmfPkg/AmdSev: Base for Confidential Migration Handler

2021-03-02 Thread Tobin Feldman-Fitzthum
Base enablement of DXE driver that supports confidential migration.

Signed-off-by: Tobin Feldman-Fitzthum 
---
 OvmfPkg/OvmfPkg.dec   |  5 ++
 OvmfPkg/AmdSev/AmdSevX64.dsc  |  1 +
 OvmfPkg/AmdSev/AmdSevX64.fdf  |  1 +
 .../ConfidentialMigrationDxe.inf  | 39 +
 .../ConfidentialMigrationDxe.c| 83 +++
 5 files changed, 129 insertions(+)
 create mode 100644 
OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
 create mode 100644 
OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 4450d78b91..402c3b61fa 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -324,6 +324,11 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId|0|UINT16|0x1b
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE|BOOLEAN|0x21
 
+  ## Set via FW_CFG to enable confidentialmigration as source or target.
+  #
+  
gUefiOvmfPkgTokenSpaceGuid.PcdIsConfidentialMigrationTarget|FALSE|BOOLEAN|0x46
+  
gUefiOvmfPkgTokenSpaceGuid.PcdStartConfidentialMigrationHandler|FALSE|BOOLEAN|0x47
+
   ## The IO port aperture shared by all PCI root bridges.
   #
   gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase|0x0|UINT64|0x22
diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index ca21fd6e5f..fa68143663 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -787,6 +787,7 @@
 !endif
   OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
   OvmfPkg/AmdSev/Grub/Grub.inf
+  OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
 !if $(BUILD_SHELL) == TRUE
   ShellPkg/Application/Shell/Shell.inf {
 
diff --git a/OvmfPkg/AmdSev/AmdSevX64.fdf b/OvmfPkg/AmdSev/AmdSevX64.fdf
index c0098502aa..6ef6dc89f2 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.fdf
+++ b/OvmfPkg/AmdSev/AmdSevX64.fdf
@@ -273,6 +273,7 @@ INF  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
 INF  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
 !endif
 INF OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
+INF OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
 INF  OvmfPkg/AmdSev/Grub/Grub.inf
 !if $(BUILD_SHELL) == TRUE
 INF  ShellPkg/Application/Shell/Shell.inf
diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf 
b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
new file mode 100644
index 00..a4906a2451
--- /dev/null
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
@@ -0,0 +1,39 @@
+## @file
+#
+#  Copyright (C) 2021 IBM Corporation.
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = ConfidentialMigration
+  FILE_GUID  = 5c2978f4-f175-434b-9e6c-9b03bd7e346f
+  MODULE_TYPE= DXE_DRIVER
+  VERSION_STRING = 1.0
+  ENTRY_POINT= LaunchMigrationHandler
+
+[Sources]
+  ConfidentialMigrationDxe.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+  UefiCpuPkg/UefiCpuPkg.dec
+
+[LibraryClasses]
+  MemoryAllocationLib
+  DebugLib
+  UefiBootServicesTableLib
+  MpInitLib
+  UefiDriverEntryPoint
+
+[Protocols]
+  gEfiMpServiceProtocolGuid
+
+[Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdIsConfidentialMigrationTarget
+  gUefiOvmfPkgTokenSpaceGuid.PcdStartConfidentialMigrationHandler
+
+[Depex]
+  gEfiMpServiceProtocolGuid
diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c 
b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
new file mode 100644
index 00..6d9fe7043b
--- /dev/null
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
@@ -0,0 +1,83 @@
+/** @file
+  In-guest support for confidential migration
+
+  Copyright (C) 2021 IBM Coporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+#include 
+#include 
+#include 
+#include 
+#include 
+
+UINTN MigrationHandlerCpuIndex;
+
+VOID
+EFIAPI
+MigrationHandlerMain (
+  IN OUT VOID *Buffer
+  )
+{
+  DebugPrint (DEBUG_INFO,"MIGRATION Handler Started\n");
+}
+
+EFI_STATUS
+EFIAPI
+LaunchMigrationHandler (
+  IN EFI_HANDLE   ImageHandle,
+  IN EFI_SYSTEM_TABLE *SystemTable
+  )
+{
+  EFI_MP_SERVICES_PROTOCOL  *MpProto;
+  EFI_PROCESSOR_INFORMATION Tcb;
+  EFI_STATUSStatus;
+  UINTN NumProc;
+  UINTN NumEnabled;
+
+  gST = SystemTable;
+  gBS = gST->BootServices;
+  gRT = gST->RuntimeServices;
+
+  Status = EFI_NOT_STARTED;
+
+  if (!PcdGetBool(PcdStartConfidentialMigrationHandler)) {
+return 0;
+  }
+
+  //
+  // Use the MP Service protocol to start Migration Handler on AP
+  //
+  gBS->LocateProtocol (&gEfiMpServiceProtocolGuid, NULL, (void**)&MpProto);
+  MpProto->GetNumberOfProcessors (MpProto, &NumProc, &NumEnabled);
+  if (NumProc < 2) {
+DebugPrint (DEBUG_ERROR,"Only on

[edk2-devel] [RFC PATCH 09/14] UefiCpuPkg/MpInitLib: Allocate MP buffer as runtime memory

2021-03-02 Thread Tobin Feldman-Fitzthum
Another temporary change to support the persistence of the MH.
The Mp buffer needs to be allocated as runtime memory or it
may be overwritten by the OS.

Signed-off-by: Tobin Feldman-Fitzthum 
---
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 2 ++
 UefiCpuPkg/Library/MpInitLib/MpLib.c  | 7 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf 
b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index 34abf25d43..0b26cf6aaf 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -39,6 +39,7 @@
   MdePkg/MdePkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
   MdeModulePkg/MdeModulePkg.dec
+  OvmfPkg/OvmfPkg.dec
 
 [LibraryClasses]
   BaseLib
@@ -65,6 +66,7 @@
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled  ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase   ## 
SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase   ## CONSUMES
+  gUefiOvmfPkgTokenSpaceGuid.PcdStartConfidentialMigrationHandler
 
 [Ppis]
   gEdkiiPeiShadowMicrocodePpiGuid## SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 2568986d8c..0ca2858ca3 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1974,7 +1974,12 @@ MpInitLibInitialize (
   BufferSize += VolatileRegisters.Idtr.Limit + 1;
   BufferSize += sizeof (CPU_MP_DATA);
   BufferSize += (sizeof (CPU_AP_DATA) + sizeof (CPU_INFO_IN_HOB))* 
MaxLogicalProcessorNumber;
-  MpBuffer= AllocatePages (EFI_SIZE_TO_PAGES (BufferSize));
+  if (PcdGetBool (PcdStartConfidentialMigrationHandler)) {
+MpBuffer= AllocateRuntimePages (EFI_SIZE_TO_PAGES (BufferSize));
+  }
+  else {
+MpBuffer= AllocatePages (EFI_SIZE_TO_PAGES (BufferSize));
+  }
   ASSERT (MpBuffer != NULL);
   ZeroMem (MpBuffer, BufferSize);
   Buffer = (UINTN) MpBuffer;
-- 
2.20.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72359): https://edk2.groups.io/g/devel/message/72359
Mute This Topic: https://groups.io/mt/81036374/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [RFC PATCH 05/14] OvmfPkg/PlatfomPei: Set Confidential Migration PCD

2021-03-02 Thread Tobin Feldman-Fitzthum
Confidential Migration relies on two boolean PCDs set from FW_CFG

Signed-off-by: Tobin Feldman-Fitzthum 
---
 OvmfPkg/PlatformPei/PlatformPei.inf |  2 ++
 OvmfPkg/PlatformPei/Platform.c  | 10 ++
 2 files changed, 12 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf 
b/OvmfPkg/PlatformPei/PlatformPei.inf
index 6ef77ba7bb..66e6fcfa4f 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -92,6 +92,8 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase
+  gUefiOvmfPkgTokenSpaceGuid.PcdStartConfidentialMigrationHandler
+  gUefiOvmfPkgTokenSpaceGuid.PcdIsConfidentialMigrationTarget
   gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 96468701e3..5926c8d414 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -275,6 +275,15 @@ NoexecDxeInitialization (
   UPDATE_BOOLEAN_PCD_FROM_FW_CFG (PcdSetNxForStack);
 }
 
+VOID
+ConfidentialMigrationInitialization (
+  VOID
+  )
+{
+  UPDATE_BOOLEAN_PCD_FROM_FW_CFG (PcdStartConfidentialMigrationHandler);
+  UPDATE_BOOLEAN_PCD_FROM_FW_CFG (PcdIsConfidentialMigrationTarget);
+}
+
 VOID
 PciExBarInitialization (
   VOID
@@ -752,6 +761,7 @@ InitializePlatform (
 
   InstallClearCacheCallback ();
   AmdSevInitialize ();
+  ConfidentialMigrationInitialization ();
   MiscInitialization ();
   InstallFeatureControlCallback ();
 
-- 
2.20.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72357): https://edk2.groups.io/g/devel/message/72357
Mute This Topic: https://groups.io/mt/81036371/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [RFC PATCH 12/14] OvmfPkg/AmdSev: Don't overwrite mailbox or pagetables

2021-03-02 Thread Tobin Feldman-Fitzthum
While restoring pages, the MH should avoid overwriting its
pagetables or the mailbox it uses to communicate with the HV.

Signed-off-by: Tobin Feldman-Fitzthum 
---
 .../ConfidentialMigrationDxe.c| 22 +--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c 
b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
index 3df3b09732..f609e16f8d 100644
--- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
@@ -128,6 +128,10 @@ MigrationHandlerMain (
   IN OUT VOID *Buffer
   )
 {
+  UINT64   mailbox_start;
+  UINT64   mailbox_end;
+  UINT64   pagetable_start;
+  UINT64   pagetable_end;
   UINT64   params_base;
   MH_COMMAND_PARAMETERS*params;
   VOID *page_va;
@@ -139,10 +143,16 @@ MigrationHandlerMain (
   //
   // Shared pages must be offset by UNENC_VIRT_ADDR_BASE.
   //
-  params_base = PcdGet32 (PcdConfidentialMigrationMailboxBase) + 
UNENC_VIRT_ADDR_BASE;
+  mailbox_start = PcdGet32 (PcdConfidentialMigrationMailboxBase);
+  params_base = mailbox_start + UNENC_VIRT_ADDR_BASE;
   params = (VOID *)params_base;
   page_va = (VOID *)params_base + 0x1000;
 
+  mailbox_end = mailbox_start + 2 * EFI_PAGE_SIZE;
+
+  pagetable_start = mMigrationHelperPageTables;
+  pagetable_end = pagetable_start + 11 * EFI_PAGE_SIZE;
+
   DisableInterrupts();
   params->go = 0;
 
@@ -163,7 +173,15 @@ MigrationHandlerMain (
   break;
 
 case MH_FUNC_RESTORE_PAGE:
-  CopyMem((VOID *)params->gpa, page_va, 4096);
+  //
+  // Don't import a page that covers the mailbox or pagetables.
+  //
+  if ((params->gpa >= mailbox_start && params->gpa < mailbox_end) ||
+  (params->gpa >= pagetable_start && params->gpa < pagetable_end)) {
+  }
+  else {
+CopyMem((VOID *)params->gpa, page_va, 4096);
+  }
   params->ret = MH_SUCCESS;
   break;
 
-- 
2.20.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72361): https://edk2.groups.io/g/devel/message/72361
Mute This Topic: https://groups.io/mt/81036378/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [RFC PATCH 00/14] Firmware Support for Fast Live Migration for AMD SEV

2021-03-02 Thread Tobin Feldman-Fitzthum
This is a demonstration of fast migration for encrypted virtual machines
using a Migration Handler that lives in OVMF. This demo uses AMD SEV,
but the ideas may generalize to other confidential computing platforms.
With AMD SEV, guest memory is encrypted and the hypervisor cannot access
or move it. This makes migration tricky. In this demo, we show how the
HV can ask a Migration Handler (MH) in the firmware for an encrypted
page. The MH encrypts the page with a transport key prior to releasing
it to the HV. The target machine also runs an MH that decrypts the page
once it is passed in by the target HV. These patches are not ready for
production, but the are a full end-to-end solution that facilitates a
fast live migration between two SEV VMs.

Corresponding patches for QEMU have been posted my colleague Dov Murik
on qemu-devel. Our approach needs little kernel support, requiring only
one hypercall that the guest can use to mark a page as encrypted or
shared. This series includes updated patches from Ashish Kalra and
Brijesh Singh that allow OVMF to use this hypercall. 

The MH runs continuously in the guest, waiting for communication from
the HV. The HV starts an additional vCPU for the MH but does not expose
it to the guest OS via ACPI. We use the MpService to start the MH. The
MpService is only available at runtime and processes that are started by
it are usually cleaned up on ExitBootServices. Since we need the MH to
run continuously, we had to make some modifications. Ideally a feature
could be added to the MpService to allow for the starting of
long-running processes. Besides migration, this could support other
background processes that need to operate within the encryption
boundary. For now, we have included a handful of patches that modify the
MpService to allow the MH to keep running after ExitBootServices. These
are temporary. 

Ashish Kalra (2):
  OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.
  OvmfPkg/PlatformDxe: Add support for SEV live migration.

Brijesh Singh (1):
  OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall

Dov Murik (1):
  OvmfPkg/AmdSev: Build page table for migration handler

Tobin Feldman-Fitzthum (10):
  OvmfPkg/AmdSev: Base for Confidential Migration Handler
  OvmfPkg/PlatfomPei: Set Confidential Migration PCD
  OvmfPkg/AmdSev: Setup Migration Handler Mailbox
  OvmfPkg/AmdSev: MH support for mailbox protocol
  UefiCpuPkg/MpInitLib: temp removal of MpLib cleanup
  UefiCpuPkg/MpInitLib: Allocate MP buffer as runtime memory
  UefiCpuPkg/CpuExceptionHandlerLib: Exception handling as runtime
memory
  OvmfPkg/AmdSev: Don't overwrite mailbox or pagetables
  OvmfPkg/AmdSev: Don't overwrite MH stack
  OvmfPkg/AmdSev: MH page encryption POC

 OvmfPkg/OvmfPkg.dec   |  11 +
 OvmfPkg/AmdSev/AmdSevX64.dsc  |   2 +
 OvmfPkg/AmdSev/AmdSevX64.fdf  |  13 +-
 .../ConfidentialMigrationDxe.inf  |  45 +++
 .../ConfidentialMigrationPei.inf  |  35 ++
 .../DxeMemEncryptSevLib.inf   |   1 +
 .../PeiMemEncryptSevLib.inf   |   1 +
 OvmfPkg/PlatformDxe/Platform.inf  |   2 +
 OvmfPkg/PlatformPei/PlatformPei.inf   |   2 +
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   2 +
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   2 +
 OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h  | 235 +
 .../ConfidentialMigration/VirtualMemory.h | 177 ++
 OvmfPkg/Include/Guid/MemEncryptLib.h  |  16 +
 OvmfPkg/PlatformDxe/PlatformConfig.h  |   5 +
 .../ConfidentialMigrationDxe.c| 325 ++
 .../ConfidentialMigrationPei.c|  25 ++
 .../X64/PeiDxeVirtualMemory.c |  18 +
 OvmfPkg/PlatformDxe/AmdSev.c  |  99 ++
 OvmfPkg/PlatformDxe/Platform.c|   6 +
 OvmfPkg/PlatformPei/AmdSev.c  |  10 +
 OvmfPkg/PlatformPei/Platform.c|  10 +
 .../CpuExceptionHandlerLib/DxeException.c |   8 +-
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c   |  21 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.c  |   7 +-
 25 files changed, 1061 insertions(+), 17 deletions(-)
 create mode 100644 
OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
 create mode 100644 
OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.inf
 create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h
 create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
 create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
 create mode 100644 
OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
 create mode 100644 
OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationPei.c
 create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c

-- 
2.20.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72353): https://edk2.groups.io/g/devel/message/72353

[edk2-devel] [RFC PATCH 02/14] OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.

2021-03-02 Thread Tobin Feldman-Fitzthum
From: Ashish Kalra 

Mark the SEC GHCB page that is mapped as unencrypted in
ResetVector code in the hypervisor page encryption bitmap.

Cc: Jordan Justen 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 

Signed-off-by: Ashish Kalra 
---
 OvmfPkg/PlatformPei/AmdSev.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index dddffdebda..c72eeb37c5 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -52,6 +53,15 @@ AmdSevEsInitialize (
   PcdStatus = PcdSetBoolS (PcdSevEsIsEnabled, TRUE);
   ASSERT_RETURN_ERROR (PcdStatus);
 
+  //
+  // GHCB_BASE setup during reset-vector needs to be marked as
+  // decrypted in the hypervisor page encryption bitmap.
+  //
+  SetMemoryEncDecHypercall3 (FixedPcdGet32 (PcdOvmfSecGhcbBase),
+EFI_SIZE_TO_PAGES(FixedPcdGet32 (PcdOvmfSecGhcbSize)),
+FALSE
+);
+
   //
   // Allocate GHCB and per-CPU variable pages.
   //   Since the pages must survive across the UEFI to OS transition
-- 
2.20.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72352): https://edk2.groups.io/g/devel/message/72352
Mute This Topic: https://groups.io/mt/81036364/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [RFC PATCH 03/14] OvmfPkg/PlatformDxe: Add support for SEV live migration.

2021-03-02 Thread Tobin Feldman-Fitzthum
From: Ashish Kalra 

Detect for KVM hypervisor and check for SEV live migration
feature support via KVM_FEATURE_CPUID, if detected setup a new
UEFI enviroment variable to indicate OVMF support for SEV
live migration.

Signed-off-by: Ashish Kalra 
---
 OvmfPkg/OvmfPkg.dec  |  1 +
 OvmfPkg/PlatformDxe/Platform.inf |  2 +
 OvmfPkg/Include/Guid/MemEncryptLib.h | 16 +
 OvmfPkg/PlatformDxe/PlatformConfig.h |  5 ++
 OvmfPkg/PlatformDxe/AmdSev.c | 99 
 OvmfPkg/PlatformDxe/Platform.c   |  6 ++
 6 files changed, 129 insertions(+)
 create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
 create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 4348bb45c6..4450d78b91 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -122,6 +122,7 @@
   gQemuKernelLoaderFsMediaGuid  = {0x1428f772, 0xb64a, 0x441e, {0xb8, 
0xc3, 0x9e, 0xbd, 0xd7, 0xf8, 0x93, 0xc7}}
   gGrubFileGuid = {0xb5ae312c, 0xbc8a, 0x43b1, {0x9c, 
0x62, 0xeb, 0xb8, 0x26, 0xdd, 0x5d, 0x07}}
   gConfidentialComputingSecretGuid  = {0xadf956ad, 0xe98c, 0x484c, {0xae, 
0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47}}
+  gMemEncryptGuid   = {0x0cf29b71, 0x9e51, 0x433a, {0xa3, 
0xb7, 0x81, 0xf3, 0xab, 0x16, 0xb8, 0x75}}
 
 [Ppis]
   # PPI whose presence in the PPI database signals that the TPM base address
diff --git a/OvmfPkg/PlatformDxe/Platform.inf b/OvmfPkg/PlatformDxe/Platform.inf
index 14727c1220..2896f0a1d1 100644
--- a/OvmfPkg/PlatformDxe/Platform.inf
+++ b/OvmfPkg/PlatformDxe/Platform.inf
@@ -24,6 +24,7 @@
   PlatformConfig.c
   PlatformConfig.h
   PlatformForms.vfr
+  AmdSev.c
 
 [Packages]
   MdePkg/MdePkg.dec
@@ -56,6 +57,7 @@
 [Guids]
   gEfiIfrTianoGuid
   gOvmfPlatformConfigGuid
+  gMemEncryptGuid
 
 [Depex]
   gEfiHiiConfigRoutingProtocolGuid  AND
diff --git a/OvmfPkg/Include/Guid/MemEncryptLib.h 
b/OvmfPkg/Include/Guid/MemEncryptLib.h
new file mode 100644
index 00..8264a647af
--- /dev/null
+++ b/OvmfPkg/Include/Guid/MemEncryptLib.h
@@ -0,0 +1,16 @@
+/** @file
+  AMD Memory Encryption GUID, define a new GUID for defining
+  new UEFI enviroment variables assocaiated with SEV Memory Encryption.
+  Copyright (c) 2020, AMD Inc. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef __MEMENCRYPT_LIB_H__
+#define __MEMENCRYPT_LIB_H__
+
+#define MEMENCRYPT_GUID \
+{0x0cf29b71, 0x9e51, 0x433a, {0xa3, 0xb7, 0x81, 0xf3, 0xab, 0x16, 0xb8, 0x75}}
+
+extern EFI_GUID gMemEncryptGuid;
+
+#endif
diff --git a/OvmfPkg/PlatformDxe/PlatformConfig.h 
b/OvmfPkg/PlatformDxe/PlatformConfig.h
index 716514da21..4f662aafa4 100644
--- a/OvmfPkg/PlatformDxe/PlatformConfig.h
+++ b/OvmfPkg/PlatformDxe/PlatformConfig.h
@@ -44,6 +44,11 @@ PlatformConfigLoad (
   OUT UINT64  *OptionalElements
   );
 
+VOID
+AmdSevSetConfig(
+  VOID
+  );
+
 //
 // Feature flags for OptionalElements.
 //
diff --git a/OvmfPkg/PlatformDxe/AmdSev.c b/OvmfPkg/PlatformDxe/AmdSev.c
new file mode 100644
index 00..1f804984b7
--- /dev/null
+++ b/OvmfPkg/PlatformDxe/AmdSev.c
@@ -0,0 +1,99 @@
+/**@file
+  Detect KVM hypervisor support for SEV live migration and if
+  detected, setup a new UEFI enviroment variable indicating
+  OVMF support for SEV live migration.
+  Copyright (c) 2020, Advanced Micro Devices. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+//
+// The package level header files this module uses
+//
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+  Figures out if we are running inside KVM HVM and
+  KVM HVM supports SEV Live Migration feature.
+  @retval TRUE   KVM was detected and Live Migration supported
+  @retval FALSE  KVM was not detected or Live Migration not supported
+**/
+BOOLEAN
+KvmDetectSevLiveMigrationFeature(
+  VOID
+  )
+{
+  UINT8 Signature[13];
+  UINT32 mKvmLeaf = 0;
+  UINT32 RegEax, RegEbx, RegEcx, RegEdx;
+
+  Signature[12] = '\0';
+  for (mKvmLeaf = 0x4000; mKvmLeaf < 0x4001; mKvmLeaf += 0x100) {
+AsmCpuid (mKvmLeaf,
+  NULL,
+  (UINT32 *) &Signature[0],
+  (UINT32 *) &Signature[4],
+  (UINT32 *) &Signature[8]);
+
+if (!AsciiStrCmp ((CHAR8 *) Signature, "KVMKVMKVM\0\0\0")) {
+   DEBUG ((
+DEBUG_ERROR,
+"%a: KVM Detected, signature = %s\n",
+__FUNCTION__,
+Signature
+));
+
+RegEax = 0x4001;
+RegEcx = 0;
+  AsmCpuid (0x4001, &RegEax, &RegEbx, &RegEcx, &RegEdx);
+  if (RegEax & (1 << 14)) {
+ DEBUG ((
+DEBUG_ERROR,
+"%a: Live Migration feature supported\n",
+__FUNCTION__
+));
+return TRUE;
+ }
+}
+  }
+
+  return FALSE;
+}
+
+/**
+  Function checks if SEV Live Migration support is available, if present then 
it sets
+  a UEFI enviroment variable to be queried later using Runtime services.
+  **/
+VOID
+AmdSevSetConfig(
+  VOID
+  )
+{
+  EFI_STATUS Status;
+  BOO

[edk2-devel] [RFC PATCH 01/14] OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall

2021-03-02 Thread Tobin Feldman-Fitzthum
From: Brijesh Singh 

By default all the SEV guest memory regions are considered encrypted,
if a guest changes the encryption attribute of the page (e.g mark a
page as decrypted) then notify hypervisor. Hypervisor will need to
track the unencrypted pages. The information will be used during
guest live migration, guest page migration and guest debugging.

Invoke hypercall via the new hypercall library.

This hypercall is used to notify hypervisor when a page is marked as
'decrypted' (i.e C-bit removed).

Cc: Jordan Justen 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 

Signed-off-by: Brijesh Singh 
Signed-off-by: Ashish Kalra 
---
 .../DxeMemEncryptSevLib.inf|  1 +
 .../PeiMemEncryptSevLib.inf|  1 +
 .../X64/PeiDxeVirtualMemory.c  | 18 ++
 3 files changed, 20 insertions(+)

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf 
b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
index f2e162d680..aefcd7c0f7 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
@@ -49,6 +49,7 @@
   DebugLib
   MemoryAllocationLib
   PcdLib
+  MemEncryptHypercallLib
 
 [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf 
b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
index 03a78c32df..7503f56a0b 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
@@ -49,6 +49,7 @@
   DebugLib
   MemoryAllocationLib
   PcdLib
+  MemEncryptHypercallLib
 
 [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c 
b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
index d3455e812b..98a1d2e3a8 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "VirtualMemory.h"
 
@@ -585,6 +586,9 @@ SetMemoryEncDec (
   UINT64 AddressEncMask;
   BOOLEANIsWpEnabled;
   RETURN_STATUS  Status;
+  UINTN  Size;
+  BOOLEANCBitChanged;
+  PHYSICAL_ADDRESS   OrigPhysicalAddress;
 
   //
   // Set PageMapLevel4Entry to suppress incorrect compiler/analyzer warnings.
@@ -636,6 +640,10 @@ SetMemoryEncDec (
 
   Status = EFI_SUCCESS;
 
+  Size = Length;
+  CBitChanged = FALSE;
+  OrigPhysicalAddress = PhysicalAddress;
+
   while (Length != 0)
   {
 //
@@ -695,6 +703,7 @@ SetMemoryEncDec (
   ));
 PhysicalAddress += BIT30;
 Length -= BIT30;
+CBitChanged = TRUE;
   } else {
 //
 // We must split the page
@@ -749,6 +758,7 @@ SetMemoryEncDec (
   SetOrClearCBit (&PageDirectory2MEntry->Uint64, Mode);
   PhysicalAddress += BIT21;
   Length -= BIT21;
+  CBitChanged = TRUE;
 } else {
   //
   // We must split up this page into 4K pages
@@ -791,6 +801,7 @@ SetMemoryEncDec (
 SetOrClearCBit (&PageTableEntry->Uint64, Mode);
 PhysicalAddress += EFI_PAGE_SIZE;
 Length -= EFI_PAGE_SIZE;
+CBitChanged = TRUE;
   }
 }
   }
@@ -808,6 +819,13 @@ SetMemoryEncDec (
   //
   CpuFlushTlb();
 
+  //
+  // Notify Hypervisor on C-bit status
+  //
+  if (CBitChanged) {
+SetMemoryEncDecHypercall3 (OrigPhysicalAddress, EFI_SIZE_TO_PAGES(Size), 
!Mode);
+  }
+
 Done:
   //
   // Restore page table write protection, if any.
-- 
2.20.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72351): https://edk2.groups.io/g/devel/message/72351
Mute This Topic: https://groups.io/mt/81036363/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [RFC PATCH 11/14] OvmfPkg/AmdSev: Build page table for migration handler

2021-03-02 Thread Tobin Feldman-Fitzthum
From: Dov Murik 

The migration handler builds its own page tables and switches
to them. The MH pagetables are reserved as runtime memory.

When the hypervisor asks the MH to import/export a page, the HV
writes the guest physical address of the page in question to the
mailbox. The MH uses an identity mapping so that it can read/write
whatever GPA is requested by the HV. The hypervisor only asks the
MH to import/export encrypted pages. Thus, the C-Bit can be set
for every page in the identity map.

The MH also needs to read shared pages, such as the mailbox.
These are mapped at an offset. The offset must be added to
the physical address before it can be resolved.

Signed-off-by: Tobin Feldman-Fitzthum 
Signed-off-by: Dov Murik 
---
 .../ConfidentialMigrationDxe.inf  |   1 +
 .../ConfidentialMigration/VirtualMemory.h | 177 ++
 .../ConfidentialMigrationDxe.c|  88 -
 3 files changed, 265 insertions(+), 1 deletion(-)
 create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h

diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf 
b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
index 49457d5d17..8dadfd1d13 100644
--- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
@@ -15,6 +15,7 @@
 
 [Sources]
   ConfidentialMigrationDxe.c
+  VirtualMemory.h
 
 [Packages]
   MdePkg/MdePkg.dec
diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h 
b/OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
new file mode 100644
index 00..c50cb64c63
--- /dev/null
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/VirtualMemory.h
@@ -0,0 +1,177 @@
+/** @file
+  Virtual Memory Management Services to set or clear the memory encryption bit
+  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2017, AMD Incorporated. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+  Code is derived from OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
+
+**/
+
+#ifndef __VIRTUAL_MEMORY__
+#define __VIRTUAL_MEMORY__
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define SYS_CODE64_SEL 0x38
+
+#pragma pack(1)
+
+//
+// Page-Map Level-4 Offset (PML4) and
+// Page-Directory-Pointer Offset (PDPE) entries 4K & 2MB
+//
+
+typedef union {
+  struct {
+UINT64  Present:1;// 0 = Not present in memory,
+  //   1 = Present in memory
+UINT64  ReadWrite:1;  // 0 = Read-Only, 1= Read/Write
+UINT64  UserSupervisor:1; // 0 = Supervisor, 1=User
+UINT64  WriteThrough:1;   // 0 = Write-Back caching,
+  //   1 = Write-Through caching
+UINT64  CacheDisabled:1;  // 0 = Cached, 1=Non-Cached
+UINT64  Accessed:1;   // 0 = Not accessed,
+  //   1 = Accessed (set by CPU)
+UINT64  Reserved:1;   // Reserved
+UINT64  MustBeZero:2; // Must Be Zero
+UINT64  Available:3;  // Available for use by system software
+UINT64  PageTableBaseAddress:40;  // Page Table Base Address
+UINT64  AvabilableHigh:11;// Available for use by system software
+UINT64  Nx:1; // No Execute bit
+  } Bits;
+  UINT64Uint64;
+} PAGE_MAP_AND_DIRECTORY_POINTER;
+
+//
+// Page Table Entry 4KB
+//
+typedef union {
+  struct {
+UINT64  Present:1;// 0 = Not present in memory,
+  //   1 = Present in memory
+UINT64  ReadWrite:1;  // 0 = Read-Only, 1= Read/Write
+UINT64  UserSupervisor:1; // 0 = Supervisor, 1=User
+UINT64  WriteThrough:1;   // 0 = Write-Back caching,
+  //   1 = Write-Through caching
+UINT64  CacheDisabled:1;  // 0 = Cached, 1=Non-Cached
+UINT64  Accessed:1;   // 0 = Not accessed,
+  //   1 = Accessed (set by CPU)
+UINT64  Dirty:1;  // 0 = Not Dirty, 1 = written by
+  //   processor on access to page
+UINT64  PAT:1;//
+UINT64  Global:1; // 0 = Not global page, 1 = global page
+  //   TLB not cleared on CR3 write
+UINT64  Available:3;  // Available for use by system software
+UINT64  PageTableBaseAddress:40;  // Page Table Base Address
+UINT64  AvabilableHigh:11;// Available for use by system software
+UINT64  Nx:1; // 0 = Execute Code,
+  //   1 = No Code Execution
+  } Bits;
+  UINT64Uint64;
+} PAGE_TABLE_4K_ENTRY;
+
+//
+// Page Table Entry 2MB
+//
+typedef union {
+  struct {
+UINT64  Present:1;   

[edk2-devel] [RFC PATCH 10/14] UefiCpuPkg/CpuExceptionHandlerLib: Exception handling as runtime memory

2021-03-02 Thread Tobin Feldman-Fitzthum
Reserve IDT and other exception-related memory as runtime so
it won't be overwritten by the OS while the MH is running.

Signed-off-by: Tobin Feldman-Fitzthum 
---
 UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c 
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
index fd59f09ecd..35610f8cf5 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
@@ -102,7 +102,7 @@ InitializeCpuInterruptHandlers (
   EFI_CPU_INTERRUPT_HANDLER  *ExternalInterruptHandler;
 
   Status = gBS->AllocatePool (
-  EfiBootServicesCode,
+  EfiRuntimeServicesCode,
   sizeof (RESERVED_VECTORS_DATA) * CPU_INTERRUPT_NUM,
   (VOID **)&ReservedVectors
   );
@@ -116,7 +116,7 @@ InitializeCpuInterruptHandlers (
 }
   }
 
-  ExternalInterruptHandler = AllocateZeroPool (sizeof 
(EFI_CPU_INTERRUPT_HANDLER) * CPU_INTERRUPT_NUM);
+  ExternalInterruptHandler = AllocateRuntimeZeroPool (sizeof 
(EFI_CPU_INTERRUPT_HANDLER) * CPU_INTERRUPT_NUM);
   ASSERT (ExternalInterruptHandler != NULL);
 
   //
@@ -130,7 +130,7 @@ InitializeCpuInterruptHandlers (
   //
   // Create Interrupt Descriptor Table and Copy the old IDT table in
   //
-  IdtTable = AllocateZeroPool (sizeof (IA32_IDT_GATE_DESCRIPTOR) * 
CPU_INTERRUPT_NUM);
+  IdtTable = AllocateRuntimeZeroPool (sizeof (IA32_IDT_GATE_DESCRIPTOR) * 
CPU_INTERRUPT_NUM);
   ASSERT (IdtTable != NULL);
   CopyMem (IdtTable, (VOID *)IdtDescriptor.Base, sizeof 
(IA32_IDT_GATE_DESCRIPTOR) * IdtEntryCount);
 
@@ -138,7 +138,7 @@ InitializeCpuInterruptHandlers (
   ASSERT (TemplateMap.ExceptionStubHeaderSize <= HOOKAFTER_STUB_SIZE);
 
   Status = gBS->AllocatePool (
-  EfiBootServicesCode,
+  EfiRuntimeServicesCode,
   TemplateMap.ExceptionStubHeaderSize * CPU_INTERRUPT_NUM,
   (VOID **)&InterruptEntryCode
   );
-- 
2.20.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72360): https://edk2.groups.io/g/devel/message/72360
Mute This Topic: https://groups.io/mt/81036375/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [RFC PATCH 08/14] UefiCpuPkg/MpInitLib: temp removal of MpLib cleanup

2021-03-02 Thread Tobin Feldman-Fitzthum
The Migration Handdler is started using the Mp Service, which
is only designed to function during boot time. The MH needs
to run continuously. In the abscence of a generalized persitent
Mp Service, temporary alterations were made to keep the MH running.

Here, we skip registering the ExitBootServices callback that
would normally clean up the APs. Obviously this is not suitable
for production, as it does not generalize for multiple APs
(it leaves all APs untouched rather than just the MH) and it
introduces a weird dependency where the MpLib needs an
OVMF PCD.

Signed-off-by: Tobin Feldman-Fitzthum 
---
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  2 ++
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c   | 21 ---
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf 
b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index 1771575c69..71cc968de8 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -39,6 +39,7 @@
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
+  OvmfPkg/OvmfPkg.dec
 
 [LibraryClasses]
   BaseLib
@@ -76,3 +77,4 @@
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase   ## 
SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard  ## 
CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase   ## 
CONSUMES
+  gUefiOvmfPkgTokenSpaceGuid.PcdStartConfidentialMigrationHandler
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c 
b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 7839c24976..7d59ec4a92 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -569,14 +569,19 @@ InitMpGlobalData (
   );
   ASSERT_EFI_ERROR (Status);
 
-  Status = gBS->CreateEvent (
-  EVT_SIGNAL_EXIT_BOOT_SERVICES,
-  TPL_CALLBACK,
-  MpInitChangeApLoopCallback,
-  NULL,
-  &mMpInitExitBootServicesEvent
-  );
-  ASSERT_EFI_ERROR (Status);
+  //
+  // Workaround for persistent processes .
+  //
+  if (!PcdGetBool (PcdStartConfidentialMigrationHandler)) {
+Status = gBS->CreateEvent (
+EVT_SIGNAL_EXIT_BOOT_SERVICES,
+TPL_CALLBACK,
+MpInitChangeApLoopCallback,
+NULL,
+&mMpInitExitBootServicesEvent
+);
+ASSERT_EFI_ERROR (Status);
+  }
 
   Status = gBS->CreateEventEx (
   EVT_NOTIFY_SIGNAL,
-- 
2.20.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72358): https://edk2.groups.io/g/devel/message/72358
Mute This Topic: https://groups.io/mt/81036373/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [RFC PATCH 13/14] OvmfPkg/AmdSev: Don't overwrite MH stack

2021-03-02 Thread Tobin Feldman-Fitzthum
When restoring pages, the Migration Handler shoudl avoid overwriting
its own stack.

Signed-off-by: Tobin Feldman-Fitzthum 
---
 .../ConfidentialMigrationDxe.inf  |   2 +
 OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h  | 235 ++
 .../ConfidentialMigrationDxe.c|  30 ++-
 3 files changed, 266 insertions(+), 1 deletion(-)
 create mode 100644 OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h

diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf 
b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
index 8dadfd1d13..2816952863 100644
--- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
@@ -16,6 +16,7 @@
 [Sources]
   ConfidentialMigrationDxe.c
   VirtualMemory.h
+  MpLib.h
 
 [Packages]
   MdePkg/MdePkg.dec
@@ -36,6 +37,7 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdIsConfidentialMigrationTarget
   gUefiOvmfPkgTokenSpaceGuid.PcdStartConfidentialMigrationHandler
   gUefiOvmfPkgTokenSpaceGuid.PcdConfidentialMigrationMailboxBase
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize
 
 [Depex]
   gEfiMpServiceProtocolGuid
diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h 
b/OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h
new file mode 100644
index 00..5007e25243
--- /dev/null
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/MpLib.h
@@ -0,0 +1,235 @@
+/** @file
+  Common header file for MP Initialize Library.
+  -- adapted from UefiCpuPkg/Library/MpInitLib/MpLib.h
+  Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.
+  Copyright (c) 2020, AMD Inc. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef _MP_LIB_H_
+#define _MP_LIB_H_
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define CPU_INIT_MP_LIB_HOB_GUID \
+  { \
+0x58eb6a19, 0x3699, 0x4c68, { 0xa8, 0x36, 0xda, 0xcd, 0x8e, 0xdc, 0xad, 
0x4a } \
+  }
+
+
+//
+// CPU exchange information for switch BSP
+//
+typedef struct {
+  UINT8 State;// offset 0
+  UINTN StackPointer; // offset 4 / 8
+  IA32_DESCRIPTOR   Gdtr; // offset 8 / 16
+  IA32_DESCRIPTOR   Idtr; // offset 14 / 26
+} CPU_EXCHANGE_ROLE_INFO;
+
+//
+// AP initialization state during APs wakeup
+//
+typedef enum {
+  ApInitConfig   = 1,
+  ApInitReconfig = 2,
+  ApInitDone = 3
+} AP_INIT_STATE;
+
+//
+// AP state
+//
+// The state transitions for an AP when it process a procedure are:
+//  Idle > Ready > Busy > Idle
+//   [BSP]   [AP]   [AP]
+//
+typedef enum {
+  CpuStateIdle,
+  CpuStateReady,
+  CpuStateBusy,
+  CpuStateFinished,
+  CpuStateDisabled
+} CPU_STATE;
+
+//
+// CPU volatile registers around INIT-SIPI-SIPI
+//
+typedef struct {
+  UINTN  Cr0;
+  UINTN  Cr3;
+  UINTN  Cr4;
+  UINTN  Dr0;
+  UINTN  Dr1;
+  UINTN  Dr2;
+  UINTN  Dr3;
+  UINTN  Dr6;
+  UINTN  Dr7;
+  IA32_DESCRIPTORGdtr;
+  IA32_DESCRIPTORIdtr;
+  UINT16 Tr;
+} CPU_VOLATILE_REGISTERS;
+
+//
+// AP related data
+//
+typedef struct {
+  SPIN_LOCK  ApLock;
+  volatile UINT32*StartupApSignal;
+  volatile UINTN ApFunction;
+  volatile UINTN ApFunctionArgument;
+  BOOLEANCpuHealthy;
+  volatile CPU_STATE State;
+  CPU_VOLATILE_REGISTERS VolatileRegisters;
+  BOOLEANWaiting;
+  BOOLEAN*Finished;
+  UINT64 ExpectedTime;
+  UINT64 CurrentTime;
+  UINT64 TotalTime;
+  EFI_EVENT  WaitEvent;
+  UINT32 ProcessorSignature;
+  UINT8  PlatformId;
+  UINT64 MicrocodeEntryAddr;
+} CPU_AP_DATA;
+
+//
+// Basic CPU information saved in Guided HOB.
+// Because the contents will be shard between PEI and DXE,
+// we need to make sure the each fields offset same in different
+// architecture.
+//
+#pragma pack (1)
+typedef struct {
+  UINT32 InitialApicId;
+  UINT32 ApicId;
+  UINT32 Health;
+  UINT64 ApTopOfStack;
+} CPU_INFO_IN_HOB;
+#pragma pack ()
+
+//
+// AP reset code information including code address and size,
+// this structure will be shared be C code and assembly code.
+// It is natural aligned by design.
+//
+typedef struct {
+  UINT8 *RendezvousFunnelAddress;
+  UINTN ModeEntryOffset;
+  UINTN RendezvousFunnelSize;
+  UINT8 *RelocateApLoopFuncAddress;
+  UINTN RelocateApLoopFuncSiz

[edk2-devel] [RFC PATCH 14/14] OvmfPkg/AmdSev: MH page encryption POC

2021-03-02 Thread Tobin Feldman-Fitzthum
This code is for demonstration purposes only. It is not secure or
robust. The purpose is to show where encryption will be incorporated
and to get a sense of the performance impact of adding encryption.

We plan to use AES-GCM to encrypt the pages as a stream. This will
also allow us to verify the GPA as part of the AAD, ensuring that
a malicious hypervisor hasn't exchanged pages in-flight.

Currently the CryptoPkg and the BaseCryptLib do not expose AES-GCM,
despite it being included in OpensslLib. Thus, we use CBC here.

Key sharing is out of scope for this part of the RFC. We assume that
the source and destination MH share a key. This will probably be
implemented via inject-launch-secret in the future. For now, we
hardcode a key, but this is strictly temporary.

We have had trouble using RandomBytes() in the MH when SEV is
enabled. Thus the IV is hardcoded. Again, this is temporary.

The HV and the MH will exchange information pertaining to encryption,
like the IV, via an additional header on the shared mailbox page.

This patch does not do any safety checks or handle encrypt/decrypt
failures. Again, this is only here to show where encryption will
go and generally how the MH on the source and target can share
pages without exposing guest memory to the HV.

Signed-off-by: Tobin Feldman-Fitzthum 
---
 .../ConfidentialMigrationDxe.inf  |  2 ++
 .../ConfidentialMigrationDxe.c| 36 +--
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf 
b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
index 2816952863..ae074a8b07 100644
--- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
@@ -22,6 +22,7 @@
   MdePkg/MdePkg.dec
   OvmfPkg/OvmfPkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
+  CryptoPkg/CryptoPkg.dec
 
 [LibraryClasses]
   MemoryAllocationLib
@@ -29,6 +30,7 @@
   UefiBootServicesTableLib
   MpInitLib
   UefiDriverEntryPoint
+  BaseCryptLib
 
 [Protocols]
   gEfiMpServiceProtocolGuid
diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c 
b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
index 42b99be552..a9cb490561 100644
--- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "VirtualMemory.h"
 #include "MpLib.h"
@@ -46,6 +47,14 @@ typedef volatile struct {
   UINT32   done;
 } MH_COMMAND_PARAMETERS;
 
+//
+// Additional header for encryption support.
+//
+struct page_hdr {
+  UINT8IV[16];
+  UINT8tag[16];
+};
+
 //
 // Addresses for MH page table.
 //
@@ -57,6 +66,20 @@ STATIC PHYSICAL_ADDRESS  mMigrationHelperPageTables = 0;
 //
 #define UNENC_VIRT_ADDR_BASE0xff80ULL
 
+//
+// Key shared between source and target MH (temporary)
+//
+GLOBAL_REMOVE_IF_UNREFERENCED CONST UINT8 cipher_key[] = {
+  0xc2, 0x86, 0x69, 0x6d, 0x88, 0x7c, 0x9a, 0xa0, 0x61, 0x1b, 0xbb, 0x3e, 
0x20, 0x25, 0xa4, 0x5a
+ };
+
+//
+// IV for CBC cipher (temporary). We are having trouble with
+// calling RandomBytes from inside the Migration Handler
+//
+GLOBAL_REMOVE_IF_UNREFERENCED UINT8 Ivec[] = {
+  0xfe, 0xdc, 0xba, 0x98, 0x76, 0x54, 0x32, 0x10
+};
 
 /**
   Allocates and fills in custom page tables for Migration Handler.
@@ -159,6 +182,8 @@ MigrationHandlerMain (
   UINT64   params_base;
   MH_COMMAND_PARAMETERS*params;
   VOID *page_va;
+  VOID *cipher_ctx;
+  INTN ctx_size;
 
   DebugPrint (DEBUG_INFO,"MIGRATION Handler Started\n");
 
@@ -171,6 +196,7 @@ MigrationHandlerMain (
   params_base = mailbox_start + UNENC_VIRT_ADDR_BASE;
   params = (VOID *)params_base;
   page_va = (VOID *)params_base + 0x1000;
+  //struct page_hdr *hdr_va = (void *) params_base + 0x800;
 
   mailbox_end = mailbox_start + 2 * EFI_PAGE_SIZE;
 
@@ -180,6 +206,9 @@ MigrationHandlerMain (
   stack_end = GetMHTopOfStack();
   stack_start = stack_end - PcdGet32(PcdCpuApStackSize);
 
+  ctx_size = AesGetContextSize ();
+  cipher_ctx = AllocateRuntimePool (ctx_size);
+
   DisableInterrupts();
   params->go = 0;
 
@@ -195,7 +224,9 @@ MigrationHandlerMain (
   break;
 
 case MH_FUNC_SAVE_PAGE:
-  CopyMem(page_va, (VOID *)params->gpa, 4096);
+  AesInit (cipher_ctx, cipher_key, 128);
+  AesCbcEncrypt(cipher_ctx, (VOID *)params->gpa, 4096, Ivec, page_va);
+
   params->ret = MH_SUCCESS;
   break;
 
@@ -208,7 +239,8 @@ MigrationHandlerMain (
   (params->gpa >= stack_start && params->gpa < stack_end)) {
   }
   else {
-CopyMem((VOID *)params->gpa, page_va, 4096);
+AesInit (cipher_ctx, cipher_key, 128);
+AesCbcDecrypt (cipher_ctx, page_va, 4096, Ivec, (VOID *)para

[edk2-devel] [RFC PATCH 07/14] OvmfPkg/AmdSev: MH support for mailbox protocol

2021-03-02 Thread Tobin Feldman-Fitzthum
The migration handler communicates with the hypervisor
via a shared mailbox page. The MH can perform four functions
at the behest of the HV: init, save page, restore page, and
reset.

Signed-off-by: Tobin Feldman-Fitzthum 
---
 .../ConfidentialMigrationDxe.inf  |  1 +
 .../ConfidentialMigrationDxe.c| 78 +++
 2 files changed, 79 insertions(+)

diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf 
b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
index a4906a2451..49457d5d17 100644
--- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.inf
@@ -34,6 +34,7 @@
 [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdIsConfidentialMigrationTarget
   gUefiOvmfPkgTokenSpaceGuid.PcdStartConfidentialMigrationHandler
+  gUefiOvmfPkgTokenSpaceGuid.PcdConfidentialMigrationMailboxBase
 
 [Depex]
   gEfiMpServiceProtocolGuid
diff --git a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c 
b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
index 6d9fe7043b..8402fcc4fa 100644
--- a/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
+++ b/OvmfPkg/AmdSev/ConfidentialMigration/ConfidentialMigrationDxe.c
@@ -9,16 +9,94 @@
 #include 
 #include 
 #include 
+#include 
 
+//
+// Functions implemented by the migration handler
+//
+#define MH_FUNC_INIT  0
+#define MH_FUNC_SAVE_PAGE 1
+#define MH_FUNC_RESTORE_PAGE  2
+#define MH_FUNC_RESET 3
+
+//
+// Return codes for MH functions
+//
+#define MH_SUCCESS0
+#define MH_INVALID_FUNC (-1)
+#define MH_AUTH_ERR (-2)
+
+//
+// Index of CPU that MH runs on.
+//
 UINTN MigrationHandlerCpuIndex;
 
+//
+// Mailbox for communication with hypervisor
+//
+typedef volatile struct {
+  UINT64   nr;
+  UINT64   gpa;
+  UINT32   do_prefetch;
+  UINT32   ret;
+  UINT32   go;
+  UINT32   done;
+} MH_COMMAND_PARAMETERS;
+
+
 VOID
 EFIAPI
 MigrationHandlerMain (
   IN OUT VOID *Buffer
   )
 {
+  UINT64   params_base;
+  MH_COMMAND_PARAMETERS*params;
+  VOID *page_va;
+
   DebugPrint (DEBUG_INFO,"MIGRATION Handler Started\n");
+
+  params_base = PcdGet32 (PcdConfidentialMigrationMailboxBase);
+  params = (VOID *)params_base;
+  page_va = (VOID *)params_base + 0x1000;
+
+  DisableInterrupts();
+  params->go = 0;
+
+  while (1) {
+while (!params->go) {
+  CpuPause();
+}
+params->done = 0;
+
+switch (params->nr) {
+case MH_FUNC_INIT:
+  params->ret = MH_SUCCESS;
+  break;
+
+case MH_FUNC_SAVE_PAGE:
+  CopyMem(page_va, (VOID *)params->gpa, 4096);
+  params->ret = MH_SUCCESS;
+  break;
+
+case MH_FUNC_RESTORE_PAGE:
+  CopyMem((VOID *)params->gpa, page_va, 4096);
+  params->ret = MH_SUCCESS;
+  break;
+
+case MH_FUNC_RESET:
+  params->ret = MH_SUCCESS;
+  break;
+
+default:
+  params->ret = MH_INVALID_FUNC;
+  break;
+}
+
+params->go = 0;
+params->done = 1;
+
+  }
 }
 
 EFI_STATUS
-- 
2.20.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72365): https://edk2.groups.io/g/devel/message/72365
Mute This Topic: https://groups.io/mt/81036401/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [RFC PATCH 02/14] OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.

2021-03-02 Thread Ashish Kalra
Hello Tobin,

Just a high level question, why is this patch included in this
patch series, i don't think you are supporting SEV-ES platform 
migration in this patch-set ?

Thanks,
Ashish

On Tue, Mar 02, 2021 at 03:48:27PM -0500, Tobin Feldman-Fitzthum wrote:
> From: Ashish Kalra 
> 
> Mark the SEC GHCB page that is mapped as unencrypted in
> ResetVector code in the hypervisor page encryption bitmap.
> 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> 
> Signed-off-by: Ashish Kalra 
> ---
>  OvmfPkg/PlatformPei/AmdSev.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
> index dddffdebda..c72eeb37c5 100644
> --- a/OvmfPkg/PlatformPei/AmdSev.c
> +++ b/OvmfPkg/PlatformPei/AmdSev.c
> @@ -15,6 +15,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -52,6 +53,15 @@ AmdSevEsInitialize (
>PcdStatus = PcdSetBoolS (PcdSevEsIsEnabled, TRUE);
>ASSERT_RETURN_ERROR (PcdStatus);
>  
> +  //
> +  // GHCB_BASE setup during reset-vector needs to be marked as
> +  // decrypted in the hypervisor page encryption bitmap.
> +  //
> +  SetMemoryEncDecHypercall3 (FixedPcdGet32 (PcdOvmfSecGhcbBase),
> +EFI_SIZE_TO_PAGES(FixedPcdGet32 (PcdOvmfSecGhcbSize)),
> +FALSE
> +);
> +
>//
>// Allocate GHCB and per-CPU variable pages.
>//   Since the pages must survive across the UEFI to OS transition
> -- 
> 2.20.1
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72366): https://edk2.groups.io/g/devel/message/72366
Mute This Topic: https://groups.io/mt/81036364/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH edk2-platforms v2 1/4] SbsaQemu: Build infrastructure for StandaloneMm image

2021-03-02 Thread Masahisa Kojima
On Tue, 2 Mar 2021 at 23:13, Leif Lindholm  wrote:
>
> On Tue, Mar 02, 2021 at 21:45:26 +0900, Masahisa Kojima wrote:
> > Hi Leif,
> >
> > Thank you for you comments.
> >
> > On Tue, 2 Mar 2021 at 02:05, Leif Lindholm  wrote:
> > >
> > > On Mon, Mar 01, 2021 at 14:19:49 +0900, Masahisa Kojima wrote:
> > > > Add the build infrastructure for compilation of StandaloneMm image.
> > > >
> > > > Signed-off-by: Masahisa Kojima 
> > > > ---
> > > >  .../Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc| 132 ++
> > >
> > > Please use --stat=1000 --stat-graph-width=20 when generating patches.
> >
> > Sorry I forgot to add these options, will be included in the next version.
> >
> > >
> > > >  Platform/Qemu/SbsaQemu/SbsaQemu.fdf   |   6 +-
> > >
> > > It is not immediately obvious to me why the pre-existing
> > > SbsaQemuStandaloneMm.dsc needs to change. Is this something that can
> > > be clarified in commit message?
> >
> > I probably does not understand your comment correctly, but
> > SbsaQemuStandaloneMm.dsc
> > is newly created file with this commit.
>
> Sorry, that's just my brain failure: I meant to say SbsaQemu.fdf.

SbsaQemu.fdf is modified to extend the FLASH0 region enough big to
contain StandaloneMM image(BL32).
I will note in the commit message in the next version.

Thanks,
Masahisa


>
> Regards,
>
> Leif
>
> > Thanks,
> > Masahisa
> >
> > >
> > > Best Regards,
> > >
> > > Leif
> > >
> > > >  .../Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf|  93 
> > > >  3 files changed, 228 insertions(+), 3 deletions(-)
> > > >  create mode 100644 Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc
> > > >  create mode 100644 Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf
> > > >
> > > > diff --git a/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc 
> > > > b/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc
> > > > new file mode 100644
> > > > index ..87f5ee351eaa
> > > > --- /dev/null
> > > > +++ b/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc
> > > > @@ -0,0 +1,132 @@
> > > > +#
> > > > +#  Copyright (c) 2020, Linaro Limited. All rights reserved.
> > > > +#
> > > > +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > +#
> > > > +
> > > > +
> > > > +#
> > > > +# Defines Section - statements that will be processed to create a 
> > > > Makefile.
> > > > +#
> > > > +
> > > > +[Defines]
> > > > +  PLATFORM_NAME  = SbsaQemuStandaloneMm
> > > > +  PLATFORM_GUID  = A64CC0F5-7ACD-4975-BBE7-7EF6739C8668
> > > > +  PLATFORM_VERSION   = 1.0
> > > > +  DSC_SPECIFICATION  = 0x00010011
> > > > +  OUTPUT_DIRECTORY   = Build/$(PLATFORM_NAME)
> > > > +  SUPPORTED_ARCHITECTURES= AARCH64
> > > > +  BUILD_TARGETS  = DEBUG|RELEASE|NOOPT
> > > > +  SKUID_IDENTIFIER   = DEFAULT
> > > > +  FLASH_DEFINITION   = 
> > > > Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf
> > > > +  DEFINE DEBUG_MESSAGE   = TRUE
> > > > +
> > > > +  # LzmaF86
> > > > +  DEFINE COMPRESSION_TOOL_GUID   = D42AE6BD-1352-4bfb-909A-CA72A6EAE889
> > > > +
> > > > +
> > > > +#
> > > > +# Library Class section - list of all Library Classes needed by this 
> > > > Platform.
> > > > +#
> > > > +
> > > > +[LibraryClasses]
> > > > +  #
> > > > +  # Basic
> > > > +  #
> > > > +  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> > > > +  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> > > > +  
> > > > DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> > > > +  
> > > > DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
> > > > +  
> > > > ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf
> > > > +  FvLib|StandaloneMmPkg/Library/FvLib/FvLib.inf
> > > > +  
> > > > HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf
> > > > +  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> > > > +  
> > > > MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf
> > > > +  
> > > > MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmCoreMemoryAllocationLib/StandaloneMmCoreMemoryAllocationLib.inf
> > > > +  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> > > > +  PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
> > > > +  PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
> > > > +  
> > > > ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
> > > > +
> > > > +  #
> > > > +  # Entry point
> > > > +  #
> > > > +  
> > > > StandaloneMmDriverEntryPoint|MdePk

Re: [edk2-devel] [PATCH v4 6/7] SecurityPkg: Tcg2Smm: Added support for Standalone Mm

2021-03-02 Thread Yao, Jiewen
Reviewed-by: Jiewen Yao 

> -Original Message-
> From: Kun Qin 
> Sent: Wednesday, March 3, 2021 4:05 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen ; Wang, Jian J ;
> Zhang, Qi1 ; Kumar, Rahul1 
> Subject: [PATCH v4 6/7] SecurityPkg: Tcg2Smm: Added support for Standalone
> Mm
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=3169
> 
> This change added Standalone MM instance of Tcg2. The notify function for
> Standalone MM instance is left empty.
> 
> A dependency DXE driver with a Depex of gEfiMmCommunication2ProtocolGuid
> was created to indicate the readiness of Standalone MM Tcg2 driver.
> 
> Lastly, the support of CI build for Tcg2 Standalone MM module is added.
> 
> Cc: Jiewen Yao 
> Cc: Jian J Wang 
> Cc: Qi Zhang 
> Cc: Rahul Kumar 
> 
> Signed-off-by: Kun Qin 
> ---
> 
> Notes:
> v4:
> - Changed dependency module from anonymous lib to Dxe driver. [Jiewen]
> 
> v3:
> - No change.
> 
> v2:
> - Newly added.
> 
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2MmDependencyDxe.c   | 48 
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.c  | 71 ++
>  SecurityPkg/SecurityPkg.ci.yaml |  1 +
>  SecurityPkg/SecurityPkg.dec |  1 +
>  SecurityPkg/SecurityPkg.dsc | 10 +++
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2MmDependencyDxe.inf | 43 +++
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.inf| 77
> 
>  7 files changed, 251 insertions(+)
> 
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2MmDependencyDxe.c
> b/SecurityPkg/Tcg/Tcg2Smm/Tcg2MmDependencyDxe.c
> new file mode 100644
> index ..4f2d7c58ed86
> --- /dev/null
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2MmDependencyDxe.c
> @@ -0,0 +1,48 @@
> +/** @file
> +  Runtime DXE part corresponding to StandaloneMM Tcg2 module.
> +
> +This module installs gTcg2MmSwSmiRegisteredGuid to notify readiness of
> +StandaloneMM Tcg2 module.
> +
> +Copyright (c) 2019 - 2021, Arm Ltd. All rights reserved.
> +Copyright (c) Microsoft Corporation.
> +
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include 
> +
> +#include 
> +#include 
> +
> +/**
> +  The constructor function installs gTcg2MmSwSmiRegisteredGuid to notify
> +  readiness of StandaloneMM Tcg2 module.
> +
> +  @param  ImageHandle   The firmware allocated handle for the EFI image.
> +  @param  SystemTable   A pointer to the Management mode System Table.
> +
> +  @retval EFI_SUCCESS   The constructor always returns EFI_SUCCESS.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +Tcg2MmDependencyDxeEntryPoint (
> +  IN EFI_HANDLE   ImageHandle,
> +  IN EFI_SYSTEM_TABLE *SystemTable
> +  )
> +{
> +  EFI_STATUSStatus;
> +  EFI_HANDLEHandle;
> +
> +  Handle = NULL;
> +  Status = gBS->InstallProtocolInterface (
> +  &Handle,
> +  &gTcg2MmSwSmiRegisteredGuid,
> +  EFI_NATIVE_INTERFACE,
> +  NULL
> +  );
> +  ASSERT_EFI_ERROR (Status);
> +  return EFI_SUCCESS;
> +}
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.c
> b/SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.c
> new file mode 100644
> index ..9e0095efbc5e
> --- /dev/null
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2StandaloneMm.c
> @@ -0,0 +1,71 @@
> +/** @file
> +  TCG2 Standalone MM driver that updates TPM2 items in ACPI table and
> registers
> +  SMI2 callback functions for Tcg2 physical presence, ClearMemory, and
> +  sample for dTPM StartMethod.
> +
> +  Caution: This module requires additional review when modified.
> +  This driver will have external input - variable and ACPINvs data in SMM 
> mode.
> +  This external input must be validated carefully to avoid security issue.
> +
> +  PhysicalPresenceCallback() and MemoryClearCallback() will receive untrusted
> input and do some check.
> +
> +Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) Microsoft Corporation.
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include "Tcg2Smm.h"
> +#include 
> +
> +/**
> +  Notify the system that the SMM variable driver is ready.
> +**/
> +VOID
> +Tcg2NotifyMmReady (
> +  VOID
> +  )
> +{
> +  // Do nothing
> +}
> +
> +/**
> +  This function is an abstraction layer for implementation specific Mm buffer
> validation routine.
> +
> +  @param Buffer  The buffer start address to be checked.
> +  @param Length  The buffer length to be checked.
> +
> +  @retval TRUE  This buffer is valid per processor architecture and not 
> overlap
> with SMRAM.
> +  @retval FALSE This buffer is not valid per processor architecture or 
> overlap
> with SMRAM.
> +**/
> +BOOLEAN
> +IsBufferOutsideMmValid (
> +  IN EFI_PHYSICAL_ADDRESS  Buffer,
> +  IN UINT64Length
> +  )
> +{
> +  return MmIsBufferOutsideMmValid (Buffer, Length);
> +}
> +
> +/**
> +  The driver's entry point.
> +
> +  It install callbacks for TPM physical presence and MemoryClear, and locate
> +  SMM

[edk2-devel] [PATCH 1/1] BaseTools: Modify struct parser for StructPcd

2021-03-02 Thread Yuwei Chen
Currently the struct parser for StructPcd Generation does not
fliter the types such as UINT8 which should be ignored successfully.
This patch modifies this issue.

Cc: Bob Feng 
Cc: Liming Gao 
Signed-off-by: Yuwei Chen 
---
 BaseTools/Scripts/ConvertFceToStructurePcd.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/BaseTools/Scripts/ConvertFceToStructurePcd.py 
b/BaseTools/Scripts/ConvertFceToStructurePcd.py
index 867660fba9cf..d72a20d62a5d 100644
--- a/BaseTools/Scripts/ConvertFceToStructurePcd.py
+++ b/BaseTools/Scripts/ConvertFceToStructurePcd.py
@@ -197,6 +197,8 @@ class parser_lst(object):
 efitxt = efivarstore_format.findall(self.text)
 for i in efitxt:
   struct = struct_re.findall(i.replace(' ',''))
+  if struct[0] in self._ignore:
+  continue
   name = name_re.findall(i.replace(' ',''))
   if struct and name:
 efivarstore_dict[name[0]]=struct[0]
-- 
2.27.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72369): https://edk2.groups.io/g/devel/message/72369
Mute This Topic: https://groups.io/mt/81044879/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v3] ShellPkg: add more items for smbiosview -t 3 .

2021-03-02 Thread Gao, Zhichao
Hi Mars,

I still cannot extract your patch thru this email. But I got it from the BZ 
link. There still some problems I missed last time.
You should add a length check before showing the contained elements.

Thanks,
Zhichao

From: Mars CC Lin 
Sent: Tuesday, March 2, 2021 11:14 AM
To: devel@edk2.groups.io
Cc: mars_cc_...@phoenix.com; Gao, Zhichao ; Philippe 
Mathieu-Daude ; Liming Gao 
Subject: [PATCH v3] ShellPkg: add more items for smbiosview -t 3 .

https://bugzilla.tianocore.org/show_bug.cgi?id=3177
Add ContainedElementCount, ContainedElementRecordLength and
ContainedElements for smbiosview type 3.

Signed-off-by: Mars CC Lin 
mailto:mars_cc_...@phoenix.com>>
Cc: Zhichao Gao mailto:zhichao@intel.com>>
Cc: Philippe Mathieu-Daude mailto:phi...@redhat.com>>
Cc: Liming Gao mailto:gaolim...@byosoft.com.cn>>
---
.../SmbiosView/PrintInfo.c | 14 ++
.../SmbiosView/SmbiosViewStrings.uni | 1 +
2 files changed, 15 insertions(+)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
index a3dc7b68c4..4be6cf2df4 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
@@ -280,6 +280,7 @@ SmbiosPrintStructure (
)
{
UINT8 Index;
+ UINT8 Index2 = 0;
UINT8 *Buffer;

if (Struct == NULL) {
@@ -404,6 +405,19 @@ SmbiosPrintStructure (
if (Struct->Hdr->Length > 0x12) {
PRINT_STRUCT_VALUE (Struct, Type3, NumberofPowerCords);
}
+ if (Struct->Hdr->Length > 0x13) {
+ PRINT_STRUCT_VALUE (Struct, Type3, ContainedElementCount);
+ }
+ if (Struct->Hdr->Length > 0x14) {
+ PRINT_STRUCT_VALUE (Struct, Type3, ContainedElementRecordLength);
+ }
+ for (Index = 0; Index < Struct->Type3->ContainedElementCount; Index++) {
+ ShellPrintHiiEx(-1,-1,NULL,STRING_TOKEN 
(STR_SMBIOSVIEW_PRINTINFO_CONTAINED_ELEMENT), gShellDebug1HiiHandle, Index+1);
+ for (Index2 = 0; Index2< Struct->Type3->ContainedElementRecordLength; 
Index2++) {
+ Print (L"%02X ", Buffer[0x15 + (Index * 
Struct->Type3->ContainedElementRecordLength) + Index2]);
+ }
+ Print (L"\n");
+ }
}
if (AE_SMBIOS_VERSION (0x2, 0x7) && (Struct->Hdr->Length > 0x13)) {
if (Struct->Hdr->Length > (0x15 + (Struct->Type3->ContainedElementCount * 
Struct->Type3->ContainedElementRecordLength))) {
diff --git 
a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStrings.uni 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStrings.uni
index 8bcba7ccf7..9433e8a25f 100644
--- 
a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStrings.uni
+++ 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStrings.uni
@@ -93,6 +93,7 @@
#string STR_SMBIOSVIEW_PRINTINFO_POWER_SUPPLY_STATE #language en-US "Power 
Supply State "
#string STR_SMBIOSVIEW_PRINTINFO_THERMAL_STATE #language en-US "Thermal state "
#string STR_SMBIOSVIEW_PRINTINFO_SECURITY_STATUS #language en-US "Security 
Status "
+#string STR_SMBIOSVIEW_PRINTINFO_CONTAINED_ELEMENT #language en-US "Contained 
Element %d: "
#string STR_SMBIOSVIEW_PRINTINFO_SUPOPRT #language en-US "Support "
#string STR_SMBIOSVIEW_PRINTINFO_CURRENT #language en-US "Current "
#string STR_SMBIOSVIEW_PRINTINFO_INSTALLED #language en-US "Installed "
--
2.29.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72370): https://edk2.groups.io/g/devel/message/72370
Mute This Topic: https://groups.io/mt/81017478/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH edk2-platforms v2 2/4] SbsaQemu: add MM based UEFI secure boot support

2021-03-02 Thread Masahisa Kojima
On Tue, 2 Mar 2021 at 02:22, Leif Lindholm  wrote:
>
> On Mon, Mar 01, 2021 at 14:19:50 +0900, Masahisa Kojima wrote:
> > This implements support for UEFI secure boot on SbsaQemu using
> > the standalone MM framework. This moves all of the software handling
> > of the UEFI authenticated variable store into the standalone MM
> > context residing in a secure partition.
> >
> > Secure variable storage is located at 0x0100 in secure NOR Flash.
> >
> > Non-secure shared memory between UEFI and standalone MM
> > is allocated at the top of DRAM.
> > DRAM size of SbsaQemu varies depends on the QEMU parameter,
> > the non-secure shared memory base address is passed from
> > trusted-firmware through the device tree "/reserved-memory" node.
> >
> > Signed-off-by: Masahisa Kojima 
> > ---
> >  Platform/Qemu/SbsaQemu/SbsaQemu.dsc   | 43 +++---
> >  .../Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc| 39 +
> >  Platform/Qemu/SbsaQemu/SbsaQemu.fdf   | 82 +--
> >  .../Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf|  7 +-
> >  .../Library/SbsaQemuLib/SbsaQemuLib.inf   |  2 +
> >  .../Library/SbsaQemuLib/SbsaQemuMem.c | 37 -
> >  6 files changed, 190 insertions(+), 20 deletions(-)
> >
> > diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc 
> > b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> > index c1f8a4696560..a75116ee70fc 100644
> > --- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> > +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> > @@ -28,6 +28,8 @@ [Defines]
> >
> >DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x804F
> >
> > +  DEFINE SECURE_BOOT_ENABLE  = FALSE
> > +
> >  #
> >  # Network definition
> >  #
> > @@ -152,12 +154,10 @@ [LibraryClasses.common]
> ># Secure Boot dependencies
> >#
> >
> > TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> > -  AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> >
> ># re-use the UserPhysicalPresent() dummy implementation from the ovmf 
> > tree
> >PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> >
> > -  VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> >
> > VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
> >
> > VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
> >
> > @@ -171,6 +171,7 @@ [LibraryClasses.common]
> >
> > ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
> >
> >TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
> > +
>
> This blank line is added for no apparent reason.
>
> >
> > NorFlashPlatformLib|Silicon/Qemu/SbsaQemu/Library/SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.inf
> >
> >CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
> > @@ -300,6 +301,8 @@ [PcdsFeatureFlag.common]
> >gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
> >gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
> >
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE
> > +
> >  [PcdsFixedAtBuild.common]
> >gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength|100
> >gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength|100
> > @@ -551,6 +554,9 @@ [PcdsDynamicDefault.common]
> >gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisAssetTag|L"AT"
> >gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisSKU|L"SK"
> >
> > +  gArmTokenSpaceGuid.PcdMmBufferBase|0x100
> > +  gArmTokenSpaceGuid.PcdMmBufferSize|0x0020
> > +
> >  
> > 
> >  #
> >  # Components Section - list of all EDK II Modules needed by this Platform
> > @@ -604,7 +610,6 @@ [Components.common]
> >ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
> >ArmPkg/Drivers/CpuPei/CpuPei.inf
> >
> > -
> >MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
> >  
> >
> > NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
> > @@ -628,24 +633,40 @@ [Components.common]
> >#
> >ArmPkg/Drivers/CpuDxe/CpuDxe.inf
> >MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
> > -  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
> > -
> > -  NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
> > -  # don't use unaligned CopyMem () on the UEFI varstore NOR flash 
> > region
> > -  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> > -  }
> >MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
> >  
> > +!if $(SECURE_BOOT_ENABLE) == TRUE
> >
> > NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> > +!endif
> >}
> > -  
> > SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
> >MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> > -  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.i

[edk2-devel] [PATCH edk2-platforms v3 0/4] add MM based UEFI secure boot on SbsaQemu

2021-03-02 Thread Masahisa Kojima
This patch series implment the UEFI secure boot on SbsaQemu.

Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Cc: Graeme Gregory 
Cc: Radoslaw Biernacki 
Cc: Shashi Mallela 

v3:
 - create device-tree parsing helper functions
 - update the .dsc file layout to minimize the modification
 - remove unnesessary blank line updates

v2:
 - get aligned to the tf-a update, it supports 512 cores
   and memory map is updated.

Masahisa Kojima (4):
  SbsaQemu: Build infrastructure for StandaloneMm image
  SbsaQemu: add MM based UEFI secure boot support
  SbsaQemu: add standalone MM build instruction
  SbsaQemu: fix typo

 Platform/Qemu/SbsaQemu/SbsaQemu.dsc   |  25 ++-
 .../Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc| 171 ++
 Platform/Qemu/SbsaQemu/SbsaQemu.fdf   |  83 -
 .../Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf|  96 ++
 .../Library/SbsaQemuLib/SbsaQemuLib.inf   |   3 +
 .../SbsaQemu/Include/Library/FdtHelperLib.h   |  27 +++
 .../Library/FdtHelperLib/FdtHelperLib.c   | 111 
 .../Library/SbsaQemuLib/SbsaQemuMem.c |  55 ++
 Platform/Qemu/SbsaQemu/Readme.md  |  37 +++-
 9 files changed, 554 insertions(+), 54 deletions(-)
 create mode 100644 Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc
 create mode 100644 Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf

-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72372): https://edk2.groups.io/g/devel/message/72372
Mute This Topic: https://groups.io/mt/81046682/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH edk2-platforms v3 1/4] SbsaQemu: Build infrastructure for StandaloneMm image

2021-03-02 Thread Masahisa Kojima
Add the build infrastructure for compilation of StandaloneMm image.
SbsaQemu.fdf is modified to extend the FLASH0 region enough big to
contain StandaloneMM image(BL32).

Signed-off-by: Masahisa Kojima 
---
 Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc | 132 
 Platform/Qemu/SbsaQemu/SbsaQemu.fdf |   6 +-
 Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf |  93 ++
 3 files changed, 228 insertions(+), 3 deletions(-)

diff --git a/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc 
b/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc
new file mode 100644
index ..87f5ee351eaa
--- /dev/null
+++ b/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc
@@ -0,0 +1,132 @@
+#
+#  Copyright (c) 2020, Linaro Limited. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+
+#
+# Defines Section - statements that will be processed to create a Makefile.
+#
+
+[Defines]
+  PLATFORM_NAME  = SbsaQemuStandaloneMm
+  PLATFORM_GUID  = A64CC0F5-7ACD-4975-BBE7-7EF6739C8668
+  PLATFORM_VERSION   = 1.0
+  DSC_SPECIFICATION  = 0x00010011
+  OUTPUT_DIRECTORY   = Build/$(PLATFORM_NAME)
+  SUPPORTED_ARCHITECTURES= AARCH64
+  BUILD_TARGETS  = DEBUG|RELEASE|NOOPT
+  SKUID_IDENTIFIER   = DEFAULT
+  FLASH_DEFINITION   = 
Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf
+  DEFINE DEBUG_MESSAGE   = TRUE
+
+  # LzmaF86
+  DEFINE COMPRESSION_TOOL_GUID   = D42AE6BD-1352-4bfb-909A-CA72A6EAE889
+
+
+#
+# Library Class section - list of all Library Classes needed by this Platform.
+#
+
+[LibraryClasses]
+  #
+  # Basic
+  #
+  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
+  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
+  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
+  
DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
+  
ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf
+  FvLib|StandaloneMmPkg/Library/FvLib/FvLib.inf
+  
HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf
+  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+  MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf
+  
MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmCoreMemoryAllocationLib/StandaloneMmCoreMemoryAllocationLib.inf
+  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
+  PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
+  
ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
+
+  #
+  # Entry point
+  #
+  
StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf
+
+  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
+  
StandaloneMmMmuLib|ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
+  ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
+  
CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
+  
PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
+
+  # ARM PL011 UART Driver
+  
PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
+  PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
+  
SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
+
+  
StandaloneMmCoreEntryPoint|StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
+
+  #
+  # It is not possible to prevent the ARM compiler for generic intrinsic 
functions.
+  # This library provides the instrinsic functions generate by a given 
compiler.
+  # And NULL mean link this library into all ARM images.
+  #
+  NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
+
+[LibraryClasses.common.MM_STANDALONE]
+  HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
+  
MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
+  
MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
+
+
+#
+# Pcd Section - list of all EDK II PCD Entries defined by this Platform
+#
+
+[PcdsFixedAtBuild]
+  gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x80CF
+  gEfiMdePkgTokenSpac

[edk2-devel] [PATCH edk2-platforms v3 2/4] SbsaQemu: add MM based UEFI secure boot support

2021-03-02 Thread Masahisa Kojima
This implements support for UEFI secure boot on SbsaQemu using
the standalone MM framework. This moves all of the software handling
of the UEFI authenticated variable store into the standalone MM
context residing in a secure partition.

Secure variable storage is located at 0x0100 in secure NOR Flash.

Non-secure shared memory between UEFI and standalone MM
is allocated at the top of DRAM.
DRAM size of SbsaQemu varies depends on the QEMU parameter,
the non-secure shared memory base address is passed from
trusted-firmware through the device tree "/reserved-memory" node.

Together with "/reserved-memory" parsing implementation newly added
in this commit, pre-existing "/memory" node parsing is moved to
a helper function in FdtHelperLib.

Signed-off-by: Masahisa Kojima 
---
 Platform/Qemu/SbsaQemu/SbsaQemu.dsc   |  25 -
 Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.dsc   |  39 +++
 Platform/Qemu/SbsaQemu/SbsaQemu.fdf   |  81 --
 Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMm.fdf   |   7 +-
 Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuLib.inf |   3 +
 Silicon/Qemu/SbsaQemu/Include/Library/FdtHelperLib.h  |  27 +
 Silicon/Qemu/SbsaQemu/Library/FdtHelperLib/FdtHelperLib.c | 111 

 Silicon/Qemu/SbsaQemu/Library/SbsaQemuLib/SbsaQemuMem.c   |  55 +++---
 8 files changed, 294 insertions(+), 54 deletions(-)

diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc 
b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
index c1f8a4696560..8a239bd17138 100644
--- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
+++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
@@ -28,6 +28,8 @@ [Defines]
 
   DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x804F
 
+  DEFINE SECURE_BOOT_ENABLE  = FALSE
+
 #
 # Network definition
 #
@@ -152,12 +154,10 @@ [LibraryClasses.common]
   # Secure Boot dependencies
   #
   
TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
-  AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
 
   # re-use the UserPhysicalPresent() dummy implementation from the ovmf tree
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
 
-  VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
   
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
   
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
 
@@ -300,6 +300,8 @@ [PcdsFeatureFlag.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
 
+  gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALSE
+
 [PcdsFixedAtBuild.common]
   gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength|100
   gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength|100
@@ -551,6 +553,9 @@ [PcdsDynamicDefault.common]
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisAssetTag|L"AT"
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisSKU|L"SK"
 
+  gArmTokenSpaceGuid.PcdMmBufferBase|0x100
+  gArmTokenSpaceGuid.PcdMmBufferSize|0x0020
+
 

 #
 # Components Section - list of all EDK II Modules needed by this Platform
@@ -628,19 +633,31 @@ [Components.common]
   #
   ArmPkg/Drivers/CpuDxe/CpuDxe.inf
   MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
+!if $(SECURE_BOOT_ENABLE) == FALSE
+  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
 
   NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
+  
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
+  VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
   # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
   BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
   }
+!else
+  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf {
+
+  
NULL|StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf
+  }
+  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
+  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
+!endif
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
 
+!if $(SECURE_BOOT_ENABLE) == TRUE
   
NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
+!endif
   }
-  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
   MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
-  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
   
MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
   MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
   EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
diff --git a/Platform/Qemu/SbsaQemu/Sbsa

[edk2-devel] [PATCH edk2-platforms v3 4/4] SbsaQemu: fix typo

2021-03-02 Thread Masahisa Kojima
Fix typo in Readme.md

Signed-off-by: Masahisa Kojima 
Reviewed-by: Leif Lindholm 
---
 Platform/Qemu/SbsaQemu/Readme.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/Qemu/SbsaQemu/Readme.md b/Platform/Qemu/SbsaQemu/Readme.md
index 50f61b6e3bf4..cef98383884a 100644
--- a/Platform/Qemu/SbsaQemu/Readme.md
+++ b/Platform/Qemu/SbsaQemu/Readme.md
@@ -97,7 +97,7 @@ Create a directory $WORKSPACE that would hold source code of 
the components.
   cd $WORKSPACE
   build -b RELEASE -a AARCH64 -t GCC5 -p 
edk2-platforms/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
   ```
-  Copy SBSA_FLASH0.fd and SBSA_FLASH0.fd to top $WORKSPACE directory.
+  Copy SBSA_FLASH0.fd and SBSA_FLASH1.fd to top $WORKSPACE directory.
   Then extend the file size to match the machine flash size.
   ```
   cp Build/SbsaQemu/RELEASE_GCC5/FV/SBSA_FLASH[01].fd .
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72376): https://edk2.groups.io/g/devel/message/72376
Mute This Topic: https://groups.io/mt/81046687/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH edk2-platforms v3 3/4] SbsaQemu: add standalone MM build instruction

2021-03-02 Thread Masahisa Kojima
This commit adds the standalone MM build instruction
to enable UEFI secure boot.

Signed-off-by: Masahisa Kojima 
---
 Platform/Qemu/SbsaQemu/Readme.md | 35 
 1 file changed, 35 insertions(+)

diff --git a/Platform/Qemu/SbsaQemu/Readme.md b/Platform/Qemu/SbsaQemu/Readme.md
index 63786d9d0fd3..50f61b6e3bf4 100644
--- a/Platform/Qemu/SbsaQemu/Readme.md
+++ b/Platform/Qemu/SbsaQemu/Readme.md
@@ -104,6 +104,41 @@ Create a directory $WORKSPACE that would hold source code 
of the components.
   truncate -s 256M SBSA_FLASH[01].fd
   ```
 
+## Build UEFI with standalone MM based UEFI secure boot
+
+1. Compile standalone MM image
+
+  ```
+  cd $WORKSPACE
+  build -b RELEASE -a AARCH64 -t GCC5 -p 
edk2-platforms/Platform/Qemu/SbsaQemu/SbsaQemuStandaloneMM.dsc
+  ```
+
+2. Compile TF-A with BL32(Secure Payload)
+
+  Detailed build instructions can be found on the following link:
+  
https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/tree/docs/plat/qemu-sbsa.rst
+
+  Then copy `bl1.bin` and `fip.bin` to the the edk2-non-osi directory:
+
+3. Compile EDK2 with UEFI secure boot enabled
+
+  ```
+  cd $WORKSPACE
+  build -b RELEASE -a AARCH64 -t GCC5 -p 
edk2-platforms/Platform/Qemu/SbsaQemu/SbsaQemu.dsc -DSECURE_BOOT_ENABLE=TRUE
+  ```
+
+  Copy SBSA_FLASH0.fd and SBSA_FLASH1.fd to top $WORKSPACE directory.
+  Then extend the file size to match the machine flash size.
+  ```
+  cp Build/SbsaQemu/RELEASE_GCC5/FV/SBSA_FLASH[01].fd .
+  truncate -s 256M SBSA_FLASH[01].fd
+  ```
+
+  To keep the UEFI variable storage after the succeeding build, use `dd` 
instead of `cp`.
+  ```
+  dd if=./Build/SbsaQemu/RELEASE_GCC5/FV/SBSA_FLASH0.fd of=./SBSA_FLASH0.fd 
conv=notrunc bs=2M count=8
+  ```
+
 # Running
 
   The resulting SBSA_FLASH0.fd file will contain Secure flash0 image (TF-A 
code).
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72375): https://edk2.groups.io/g/devel/message/72375
Mute This Topic: https://groups.io/mt/81046686/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH V5] ShellPkg: add more items for smbiosview -t 3 .

2021-03-02 Thread Mars CC Lin
https://bugzilla.tianocore.org/show_bug.cgi?id=3177
Add ContainedElementCount, ContainedElementRecordLength and
ContainedElements for smbiosview type 3.

Signed-off-by: Mars CC Lin 
Cc: Zhichao Gao 
Cc: Philippe Mathieu-Daude 
Cc: Liming Gao 
---
 .../SmbiosView/PrintInfo.c| 15 +++
 .../SmbiosView/SmbiosViewStrings.uni  |  1 +
 2 files changed, 16 insertions(+)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
index 478f63078a..04e4882272 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
@@ -404,6 +404,21 @@ SmbiosPrintStructure (
   if (Struct->Hdr->Length > 0x12) {
 PRINT_STRUCT_VALUE (Struct, Type3, NumberofPowerCords);
   }
+  if (Struct->Hdr->Length > 0x13) {
+PRINT_STRUCT_VALUE (Struct, Type3, ContainedElementCount);
+  }
+  if (Struct->Hdr->Length > 0x14) {
+PRINT_STRUCT_VALUE (Struct, Type3, ContainedElementRecordLength);
+  }
+  if (Struct->Hdr->Length > 0x15) {
+for (Index = 0; Index < Struct->Type3->ContainedElementCount; Index++) 
{
+  ShellPrintHiiEx(-1,-1,NULL,STRING_TOKEN 
(STR_SMBIOSVIEW_PRINTINFO_CONTAINED_ELEMENT), gShellDebug1HiiHandle, Index+1);
+  for (Index2 = 0; Index2< 
Struct->Type3->ContainedElementRecordLength; Index2++) {
+Print (L"%02X ", Buffer[0x15 + (Index * 
Struct->Type3->ContainedElementRecordLength) + Index2]);
+  }
+  Print (L"\n");
+}
+  }
 }
 if (AE_SMBIOS_VERSION (0x2, 0x7) && (Struct->Hdr->Length > 0x13)) {
   if (Struct->Hdr->Length > (0x15 + (Struct->Type3->ContainedElementCount 
* Struct->Type3->ContainedElementRecordLength))) {
diff --git 
a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStrings.uni 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStrings.uni
index 97e1d54fcf..20a556a175 100644
--- 
a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStrings.uni
+++ 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStrings.uni
@@ -93,6 +93,7 @@
 #string STR_SMBIOSVIEW_PRINTINFO_POWER_SUPPLY_STATE #language 
en-US "Power Supply State "
 #string STR_SMBIOSVIEW_PRINTINFO_THERMAL_STATE  #language 
en-US "Thermal state "
 #string STR_SMBIOSVIEW_PRINTINFO_SECURITY_STATUS#language 
en-US "Security Status "
+#string STR_SMBIOSVIEW_PRINTINFO_CONTAINED_ELEMENT  #language 
en-US "Contained Element %d: "
 #string STR_SMBIOSVIEW_PRINTINFO_SUPOPRT#language 
en-US "Support "
 #string STR_SMBIOSVIEW_PRINTINFO_CURRENT#language 
en-US "Current "
 #string STR_SMBIOSVIEW_PRINTINFO_INSTALLED  #language 
en-US "Installed "
-- 
2.29.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72377): https://edk2.groups.io/g/devel/message/72377
Mute This Topic: https://groups.io/mt/81047033/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-