[edk2-devel] Google Summer of Code (GSoC) 2021!

2021-03-09 Thread Nate DeSimone
Hi Everyone,

I am pleased to announce that TianoCore has been accepted to participate in 
GSoC 2021! Student applications will start rolling in on March 29th, so now we 
need some volunteers to be mentors. Thank you to all who have volunteered! 
Mentors will need to be able to commit to the following:

* Generally, we prefer mentors who have been long term contributors to the 
TianoCore project
* During the student application period (March 29 - April 13), mentors need to 
be available to help review student applications.
* On average, we expect each student will need roughly 5 hours of help each 
week during the 10 week development period (June 7 - August 16). I know that 
most people here have day jobs (including myself) so I'd like to assign 2 
mentors per student whenever possible, this will reduce the time investment to 
~2.5 hours per week.
* The student and their mentors should meet for 30min at absolute minimum on a 
weekly basis. The mentors should be able to make themselves available for more 
time if the student requires additional mentoring. In general, mentors should 
help their students succeed.
* Help the students get involved the rest of the TianoCore community. A lot of 
times, students have no prior open source experience and can at times be shy. 
One of the goals for GSoC is to give students new experiences and exposure to 
the open source community. Mentors should help students present their project 
at one of the TianoCore Design Meetings and help them solicit feedback from 
other community members on the mailing list.
* Mentors will need to write up 2 evaluations (July and August) for the 
students they are mentoring.

If you are a student and are interested in participating this year, feel free 
to introduce yourself on the mailing list and discuss potential project 
proposals.

If you are interested, please send me an off-list/private email.

Thanks!
Nate


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




Re: [edk2-devel] [PATCH 0/2] Update Maintainers.txt for TDX and Confidential Computing

2021-03-09 Thread Yao, Jiewen
Both 1 and 2 - Reviewed-by: Jiewen Yao 

> -Original Message-
> From: Xu, Min M 
> Sent: Wednesday, March 10, 2021 10:56 AM
> To: devel@edk2.groups.io
> Cc: Xu, Min M ; Andrew Fish ; Laszlo
> Ersek ; Leif Lindholm ; Kinney,
> Michael D ; Yao, Jiewen 
> Subject: [PATCH 0/2] Update Maintainers.txt for TDX and Confidential
> Computing
> 
> Register reviewers for the TDX-related and Confidential Computing related
> files in OvmfPkg.
> 
> Cc: Andrew Fish 
> Cc: Laszlo Ersek 
> Cc: Leif Lindholm 
> Cc: Michael D Kinney 
> Cc: Jiewen Yao 
> Signed-off-by: Min Xu 
> 
> Min Xu (2):
>   Maintainers.txt: Add reviewers for the OvmfPkg TDX-related files
>   Maintainers.txt: Add reviewers for Confidential Computing related
> modules
> 
>  Maintainers.txt | 7 +++
>  1 file changed, 7 insertions(+)
> 
> --
> 2.29.2.windows.2



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




[edk2-devel] [PATCH 1/2] Maintainers.txt: Add reviewers for the OvmfPkg TDX-related files

2021-03-09 Thread Min Xu
Register reviewers for the TDX-related files in OvmfPkg.

Cc: Andrew Fish 
Cc: Laszlo Ersek 
Cc: Leif Lindholm 
Cc: Michael D Kinney 
Cc: Jiewen Yao 
Signed-off-by: Min Xu 
---
 Maintainers.txt | 4 
 1 file changed, 4 insertions(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index e38497123857..7d9fe89d6d28 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -465,6 +465,10 @@ F: OvmfPkg/PlatformPei/AmdSev.c
 R: Tom Lendacky 
 R: Brijesh Singh 
 
+OvmfPkg: TDX-related modules
+R: Jiewen Yao 
+R: Min Xu 
+
 OvmfPkg: TCG- and TPM2-related modules
 F: OvmfPkg/Include/IndustryStandard/QemuTpm.h
 F: OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
-- 
2.29.2.windows.2



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




[edk2-devel] [PATCH 0/2] Update Maintainers.txt for TDX and Confidential Computing

2021-03-09 Thread Min Xu
Register reviewers for the TDX-related and Confidential Computing related
files in OvmfPkg.

Cc: Andrew Fish 
Cc: Laszlo Ersek 
Cc: Leif Lindholm 
Cc: Michael D Kinney 
Cc: Jiewen Yao 
Signed-off-by: Min Xu 

Min Xu (2):
  Maintainers.txt: Add reviewers for the OvmfPkg TDX-related files
  Maintainers.txt: Add reviewers for Confidential Computing related
modules

 Maintainers.txt | 7 +++
 1 file changed, 7 insertions(+)

-- 
2.29.2.windows.2



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




[edk2-devel] [PATCH 2/2] Maintainers.txt: Add reviewers for Confidential Computing related modules

2021-03-09 Thread Min Xu
Register reviewers for the Confidential Computing related modules in
OvmfPkg.

Cc: Andrew Fish 
Cc: Laszlo Ersek 
Cc: Leif Lindholm 
Cc: Michael D Kinney 
Cc: Jiewen Yao 
Signed-off-by: Min Xu 
---
 Maintainers.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index 7d9fe89d6d28..220af0ee9b80 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -469,6 +469,9 @@ OvmfPkg: TDX-related modules
 R: Jiewen Yao 
 R: Min Xu 
 
+OvmfPkg: Confidential Computing related modules
+R: Jiewen Yao 
+
 OvmfPkg: TCG- and TPM2-related modules
 F: OvmfPkg/Include/IndustryStandard/QemuTpm.h
 F: OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
-- 
2.29.2.windows.2



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




[edk2-devel] [PATCH] [edk2-staging] BaseTools/FMMT: Replace file failure when FV level over 2

2021-03-09 Thread GregX Yeh
Fixed replace file failure when FFS in multiple level FV and FV level over 2

Signed-off-by: GregX Yeh 
Cc: Bob Feng 
Cc: Liming Gao 
---
 BaseTools/Source/C/FMMT/FmmtLib.c | 56 ++-
 1 file changed, 37 insertions(+), 19 deletions(-)

diff --git a/BaseTools/Source/C/FMMT/FmmtLib.c 
b/BaseTools/Source/C/FMMT/FmmtLib.c
index b945e9b63d..26df0181c7 100644
--- a/BaseTools/Source/C/FMMT/FmmtLib.c
+++ b/BaseTools/Source/C/FMMT/FmmtLib.c
@@ -494,7 +494,7 @@ LibReadFvHeader (
 if ((FvLevel -1) == 0) {
   printf ("\n%s :\n", FvName);
 } else {
-  printf ("%sChild FV named FV%d of %s\n", BlankSpace, FvCount, FvName);
+  printf ("\n%sChild FV named FV%d of %s\n", BlankSpace, FvCount, FvName);
 }
   }
 
@@ -502,7 +502,7 @@ LibReadFvHeader (
   // Print FV header information
   //
   if (ViewFlag) {
-printf ("\n%sAttributes:%X\n", BlankSpace, (unsigned) 
VolumeHeader->Attributes);
+printf ("%sAttributes:%X\n", BlankSpace, (unsigned) 
VolumeHeader->Attributes);
 printf ("%sTotal Volume Size: 0x%08X\n", BlankSpace, (unsigned) 
VolumeHeader->FvLength);
 printf ("%sFree Volume Size:  0x%08X\n", BlankSpace, (unsigned) 
(VolumeHeader->FvLength - GetFreeOffset(InputFv)));
   }
@@ -789,7 +789,8 @@ LibParseSection (
   UINT8  *FvCount,
   BOOLEANViewFlag,
   BOOLEANErasePolarity,
-  BOOLEAN*IsFfsGenerated
+  BOOLEAN*IsFfsGenerated,
+  BOOLEANIsFfs
   )
 {
   UINT32  ParsedLength;
@@ -997,8 +998,12 @@ LibParseSection (
   break;
 
 case EFI_SECTION_COMPRESSION:
-  if (FirstInFlag) {
-Level ++;
+   if (IsFfs){
+ Level ++;
+   } else {
+if (FirstInFlag) {
+  Level ++;
+}
   }
   NumberOfSections ++;
 
@@ -1159,7 +1164,9 @@ LibParseSection (
   FvCount,
   ViewFlag,
   ErasePolarity,
-  IsFfsGenerated);
+  IsFfsGenerated,
+  FALSE
+  );
 
   if (CompressionType == EFI_STANDARD_COMPRESSION) {
 //
@@ -1181,8 +1188,12 @@ LibParseSection (
   // looks up the appropriate tool to use for extracting
   // a GUID defined FV section.
   //
-  if (FirstInFlag) {
+  if (IsFfs) {
 Level ++;
+  } else {
+if (FirstInFlag) {
+  Level ++;
+}
   }
   NumberOfSections++;
   EncapDataNeedUpdata = TRUE;
@@ -1216,7 +1227,8 @@ LibParseSection (
 FvCount,
 ViewFlag,
 ErasePolarity,
-IsFfsGenerated
+IsFfsGenerated,
+FALSE
 );
 if (EFI_ERROR(Status)) {
   Error(NULL, 0, 0003, "parse of decoded GUIDED section failed", NULL);
@@ -1471,7 +1483,8 @@ LibParseSection (
   FvCount,
   ViewFlag,
   ErasePolarity,
-  IsFfsGenerated
+  IsFfsGenerated,
+  FALSE
   );
 if (EFI_ERROR (Status)) {
   Error (NULL, 0, 0003, "parse of decoded GUIDED section failed", 
NULL);
@@ -1491,7 +1504,8 @@ LibParseSection (
 FvCount,
 ViewFlag,
 ErasePolarity,
-IsFfsGenerated
+IsFfsGenerated,
+FALSE
 );
   if (ExtractionTool != NULL) {
 free (ExtractionTool);
@@ -2016,7 +2030,7 @@ LibGetFileInfo (
 
   LocalEncapData->Level = Level;
   LocalEncapData->Type  = FMMT_ENCAP_TREE_FFS;
-LocalEncapData->FvExtHeader = NULL;
+  LocalEncapData->FvExtHeader = NULL;
   LocalEncapData->Depex = NULL;
   LocalEncapData->DepexLen = 0;
   LocalEncapData->UiNameSize = 0;
@@ -2099,7 +2113,8 @@ LibGetFileInfo (
   FvCount,
   ViewFlag,
   ErasePolarity,
-  
+  ,
+  TRUE
   );
 }
 if (EFI_ERROR (Status)) {
@@ -4198,10 +4213,13 @@ LibEncapNewFvFile(
   UINT32  header;
   UINT8   AlignN;
   UINT8   AlignV[1] = {0xFF};
+  UINT32  EntryFvId;
+
   AlignN  = 0;
   Id  = 0;
   InputFileSize   = 0;
   TmpFileSize = 0;
+  AlignmentFileSize   = 0;
   EncapFvIndex= 0;
   Index   = 0;
   OuterIndex  = 0;
@@ -4224,7 +4242,7 @@ LibEncapNewFvFile(
   IsLargeFile = FALSE;
   OutputFileSize  = 0;
   LargeFileSize   = 0x100;
-
+  EntryFvId   = 0;
 
   OutputFileNameList = (FFS_INFORMATION *)malloc(sizeof(FFS_INFORMATION));
   if (OutputFileNameList == NULL) {
@@ 

Re: [edk2-devel] [PATCH v2 1/1] ShellPkg: Validate that the Boot CPU is present in MADT

2021-03-09 Thread Ni, Ray
Is this for ARM only?

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Joey
> Gouly
> Sent: Friday, January 15, 2021 10:44 PM
> To: devel@edk2.groups.io
> Cc: joey.go...@arm.com; ardb+tianoc...@kernel.org; l...@nuviainc.com;
> sami.muja...@arm.com; n...@arm.com
> Subject: [edk2-devel] [PATCH v2 1/1] ShellPkg: Validate that the Boot CPU is
> present in MADT
> 
> The ACPI 6.3 Specification, January 2019, section 5.2.12.14 states that
> the firmware must convey each processor’s GIC information to the OS using
> the GICC structure.
> 
> If a GICC structure for the boot CPU is missing some standards-based
> operating system may crash with an error code. It may be difficult to
> diagnose the reason for the crash as the error code may be too generic
> and mean firmware error.
> 
> Therefore add validation to the MADT table parser to check that a GICC is
> present for the boot CPU.
> 
> Signed-off-by: Joey Gouly 
> ---
> 
> Changes since v1:
>   - Added 'm' prefix for global variables
>   - Reordered ci yaml file
> 
> The changes can be seen at
> https://github.com/jgouly/edk2/tree/1474_validate_boot_cpu_mpidr_v2
> 
>  ShellPkg/ShellPkg.dsc
> |  3 ++
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman
> dLib.inf |  5 ++
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.
> c   | 54 +++-
>  ShellPkg/ShellPkg.ci.yaml
> |  2 +
>  4 files changed, 62 insertions(+), 2 deletions(-)
> 
> diff --git a/ShellPkg/ShellPkg.dsc b/ShellPkg/ShellPkg.dsc
> index
> c42bc9464a0f7be111ee3086a664506c8288928c..661cc8b02b0971280c3649e0f2
> 9e109305bcc776 100644
> --- a/ShellPkg/ShellPkg.dsc
> +++ b/ShellPkg/ShellPkg.dsc
> @@ -71,6 +71,9 @@ [LibraryClasses.ARM,LibraryClasses.AARCH64]
># Add support for GCC stack protector
>NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> 
> +  # Add support for reading MPIDR
> +  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> +
>  [PcdsFixedAtBuild]
>gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
>gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|16000
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.inf
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.inf
> index
> 947fb1f375667e12f8603e4264fef5e69cb98919..00e770d677ec1f2a23c1650fe2f
> 4a94f2f86649f 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.inf
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> andLib.inf
> @@ -60,6 +60,9 @@ [Packages]
>MdePkg/MdePkg.dec
>ShellPkg/ShellPkg.dec
> 
> +[Packages.ARM, Packages.AARCH64]
> +  ArmPkg/ArmPkg.dec
> +
>  [LibraryClasses]
>BaseLib
>BaseMemoryLib
> @@ -75,6 +78,8 @@ [LibraryClasses]
>UefiLib
>UefiRuntimeServicesTableLib
> 
> +[LibraryClasses.ARM, LibraryClasses.AARCH64]
> +  ArmLib
> 
>  [FixedPcd]
>gEfiShellPkgTokenSpaceGuid.PcdShellProfileMask ## CONSUMES
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> index
> 15aa2392b60cee9e3843c7c560b0ab84e0be4174..9935537aaee28381fecec08d0
> 057db83aaca1e1d 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> @@ -1,7 +1,7 @@
>  /** @file
>MADT table parser
> 
> -  Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.
> +  Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.
>SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>@par Reference(s):
> @@ -13,6 +13,9 @@
> 
>  #include 
>  #include 
> +#if defined(MDE_CPU_ARM) || defined(MDE_CPU_AARCH64)
> +#include 
> +#endif
>  #include "AcpiParser.h"
>  #include "AcpiTableParser.h"
>  #include "AcpiViewConfig.h"
> @@ -23,6 +26,11 @@ STATIC CONST UINT8* MadtInterruptControllerType;
>  STATIC CONST UINT8* MadtInterruptControllerLength;
>  STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
> 
> +#if defined(MDE_CPU_ARM) || defined(MDE_CPU_AARCH64)
> +STATIC UINT64 mBootMpidr;
> +STATIC BOOLEAN mHasBootMpidrGicc = FALSE;
> +#endif
> +
>  /**
>This function validates the System Vector Base in the GICD.
> 
> @@ -95,6 +103,33 @@ ValidateSpeOverflowInterrupt (
>}
>  }
> 
> +/**
> +  This function validates that the GICC structure contains an entry for
> +  the Boot CPU.
> +
> +  @param [in] Ptr Pointer to the start of the field data.
> +  @param [in] Context Pointer to context specific information e.g. this
> +  could be a pointer to the ACPI table header.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ValidateBootMpidr (
> +  IN UINT8* Ptr,
> +  IN VOID*  Context
> +  )
> +{
> +#if defined(MDE_CPU_ARM) || defined(MDE_CPU_AARCH64)
> +  UINT64 

[edk2-devel] [PATCH 1/1] BaseTools/Ecc: Update structpcd parsing method.

2021-03-09 Thread Yuwei Chen
From: mliang2x 

Update the pcdparser method in Dec and DSC files.

Signed-off-by: Mingyue Liang 
Cc: Bob Feng 
Cc: Liming Gao 
Cc: Yuwei Chen 
---
 .../Ecc/MetaFileWorkspace/MetaFileParser.py   | 464 ++
 1 file changed, 265 insertions(+), 199 deletions(-)

diff --git a/BaseTools/Source/Python/Ecc/MetaFileWorkspace/MetaFileParser.py 
b/BaseTools/Source/Python/Ecc/MetaFileWorkspace/MetaFileParser.py
index 9c27c8e16a05..588d3dbe6ed5 100644
--- a/BaseTools/Source/Python/Ecc/MetaFileWorkspace/MetaFileParser.py
+++ b/BaseTools/Source/Python/Ecc/MetaFileWorkspace/MetaFileParser.py
@@ -22,7 +22,7 @@ import Ecc.EccToolError as EccToolError
 from CommonDataClass.DataClass import *
 from Common.DataType import *
 from Common.StringUtils import *
-from Common.Misc import GuidStructureStringToGuidString, CheckPcdDatum, 
PathClass, AnalyzePcdData
+from Common.Misc import GuidStructureStringToGuidString, CheckPcdDatum, 
PathClass, AnalyzePcdData, AnalyzeDscPcd, AnalyzePcdExpression, 
ParseFieldValue, StructPattern
 from Common.Expression import *
 from CommonDataClass.Exceptions import *
 
@@ -31,6 +31,8 @@ from GenFds.FdfParser import FdfParser
 from Common.LongFilePathSupport import OpenLongFilePath as open
 from Common.LongFilePathSupport import CodecOpenLongFilePath
 
+CODEPattern = re.compile(r"{CODE\([a-fA-F0-9Xx\{\},\s]*\)}")
+
 ## A decorator used to parse macro definition
 def ParseMacro(Parser):
 def MacroParser(self):
@@ -174,6 +176,11 @@ class MetaFileParser(object):
 # UNI object and extra UNI object
 self._UniObj = None
 self._UniExtraObj = None
+# StructPcd var
+self._PcdCodeValue = ""
+self._PcdDataTypeCODE = False
+self._CurrentPcdName = ""
+self._GuidDict = {}  # for Parser PCD value {GUID(gTokeSpaceGuidName)}
 
 ## Store the parsed data in table
 def _Store(self, *Args):
@@ -395,6 +402,40 @@ class MetaFileParser(object):
 Macros.update(self._SectionsMacroDict[(self._SectionType, 
Scope1, Scope2)])
 return Macros
 
+def ProcessMultipleLineCODEValue(self, Content):
+CODEBegin = False
+CODELine = ""
+continuelinecount = 0
+newContent = []
+for Index in range(0, len(Content)):
+Line = Content[Index]
+if CODEBegin:
+CODELine = CODELine + Line
+continuelinecount +=1
+if ")}" in Line:
+newContent.append(CODELine)
+for _ in range(continuelinecount):
+newContent.append("")
+CODEBegin = False
+CODELine = ""
+continuelinecount = 0
+else:
+if not Line:
+newContent.append(Line)
+continue
+if "{CODE(" not in Line:
+newContent.append(Line)
+continue
+elif CODEPattern.findall(Line):
+newContent.append(Line)
+continue
+else:
+CODEBegin = True
+CODELine = Line
+
+return newContent
+
+
 _SectionParser  = {}
 Finished= property(_GetFinished, _SetFinished)
 _Macros = property(_GetMacros)
@@ -812,6 +853,8 @@ class DscParser(MetaFileParser):
 Content = open(str(self.MetaFile.Path), 'r').readlines()
 except:
 EdkLogger.error("Parser", FILE_READ_FAILURE, 
ExtraData=self.MetaFile)
+
+Content = self.ProcessMultipleLineCODEValue(Content)
 #
 # Insert a record for file
 #
@@ -1018,24 +1061,71 @@ class DscParser(MetaFileParser):
 #
 @ParseMacro
 def _PcdParser(self):
+if self._PcdDataTypeCODE:
+self._PcdCodeValue = self._PcdCodeValue + "\n " + self._CurrentLine
+if self._CurrentLine.endswith(")}"):
+self._CurrentLine = "|".join((self._CurrentPcdName, 
self._PcdCodeValue))
+self._PcdDataTypeCODE = False
+self._PcdCodeValue = ""
+else:
+self._ValueList = None
+return
 TokenList = GetSplitValueList(self._CurrentLine, TAB_VALUE_SPLIT, 1)
+self._CurrentPcdName = TokenList[0]
+if len(TokenList) == 2 and TokenList[1].strip().startswith("{CODE"):
+self._PcdDataTypeCODE = True
+self._PcdCodeValue = TokenList[1].strip()
+
+if self._PcdDataTypeCODE:
+if self._CurrentLine.endswith(")}"):
+self._PcdDataTypeCODE = False
+self._PcdCodeValue = ""
+else:
+self._ValueList = None
+return
 self._ValueList[0:1] = GetSplitValueList(TokenList[0], TAB_SPLIT)
+PcdNameTockens = GetSplitValueList(TokenList[0], TAB_SPLIT)
+if len(PcdNameTockens) == 2:
+

Re: [edk2-devel] [edk2-announce] TianoCore Community Meeting Minutes - March

2021-03-09 Thread Rebecca Cran

On 3/9/21 5:29 PM, Soumya Guptha wrote:


STABLE TAG:
edk2-stable202102 is in planning stage. Please send your feature requests soon.
you can visit here for more information - 
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning


edk2-stable202102 has been released: edk2-stable202105 is in the 
planning stage.


--
Rebecca Cran



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




[edk2-devel] TianoCore Community Meeting Minutes - March

2021-03-09 Thread Soumya Guptha
TianoCore Community Meeting Minutes
March 4, 2021

EVENTS:
UEFI Plugfest:
Currently virtual webinars are held during 2021 and this will continue to the 
remainder of 2021. Webinars are listed here (https://uefi.org/events/upcoming). 
If you have topics to propose, please visit uefi.org/events and submit your 
abstracts.
ICWG team is planning to host a face-to-face UEFI plugfest in Oregon during 
spring 2022. Virtual webinars during 2021 are listed here 
(https://uefi.org/events/upcoming)

STABLE TAG:
edk2-stable202102 is in planning stage. Please send your feature requests soon.
you can visit here for more information - 
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning

STEWARDS DOWNLOAD: (Mike Kinney)
No new topics this month.

Please get new RFCs, tech proposals etc.. and bring those in Design Review 
Meetings run by Ray Ni.

Opens:
1) Community meetings will be moved from Webex to Microsoft Teams starting late 
this month. No objections from the attendees.
2) Status on - Line ending conversations - prioritized after Git hub pull 
request. Targeting during 2H' 2021. We need to do some trial repo before we 
push it. Some files in edk2 repo, we may not be able to convert, need time to 
experiment.
3) update from Nate Desimone - Tigerlake Min platform released last month on 
EDK2. Community has been filing bugs. Whitley and Cedar Island Min platform is 
work in progress.
4) Google summer code (sponsored internship program run by Google) - Nate 
Desimone sent the application for TianoCore open source to Google. Nate singed 
up to be the program coordinator. Lot of interest from the community to be a 
mentor. We will hear by Mar 9th, if we get accepted, we need to get the mentors 
together and work on processing student applications.


Regards,
Soumya


Soumya Guptha
Firmware Ecosystem Enabling Manager, Intel






-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72599): https://edk2.groups.io/g/devel/message/72599
Mute This Topic: https://groups.io/mt/81216358/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 0/3] Add TdxLib support for Intel TDX

2021-03-09 Thread Yao, Jiewen
Very good suggestion. Thanks Laszlo.

For 3), Min Xu and I will be the reviewer for Intel TDX change for OVMF.

For 6), agree. Although there is some architecture difference, e.g, AMD using 
PSP - a co-processor while Intel using TDX module - a new CPU execution mode, 
we should align as much as possible between Intel TDX and AMD SEV, especially 
for pure software architecture.
I will be the Intel reviewer for confidential computing topic.
Welcome AMD/IBM/... having a representative too.

Min and I will sync and submit the patch for maintainer.txt


Thank you
Yao Jiewen 


> -Original Message-
> From: Laszlo Ersek 
> Sent: Tuesday, March 9, 2021 9:06 PM
> To: Xu, Min M ; devel@edk2.groups.io
> Cc: Liming Gao ; Liu, Zhiguang
> ; Justen, Jordan L ; Yao,
> Jiewen ; Tom Lendacky ;
> Brijesh Singh ; James Bottomley
> ; Tobin Feldman-Fitzthum ; Dov Murik
> ; Dr. David Alan Gilbert 
> Subject: Re: [PATCH V3 0/3] Add TdxLib support for Intel TDX
> 
> On 03/09/21 13:57, Laszlo Ersek wrote:
> > On 03/09/21 07:12, Min Xu wrote:
> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3249
> >>
> >> The patch series provides lib support for Intel Trust Domain Extensions
> >> (Intel TDX).
> >>
> >> Intel's Trust Domain Extensions (Intel TDX) refers to an Intel technology
> >> that extends Virtual Machines Extensions (VMX) and Multi-Key Total Memory
> >> Encryption (MKTME) with a new kind of virutal machines guest called a
> >> Trust Domain (TD). A TD is desinged to run in a CPU mode that protects the
> >> confidentiality of TD memory contents and the TD's CPU state from other
> >> software, including the hosting Virtual-Machine Monitor (VMM), unless
> >> explicitly shared by the TD itself.
> >>
> >> The Intel TDX module uses the instruction-set architecture for Intel TDX
> >> and the MKTME engine in the SOC to help serve as an intermediary between
> >> the host VMM and the guest TD. TDCALL is the instruction which allows TD
> >> guest privileged software to make a call for service into an underlying
> >> TDX-module.
> >>
> >> TdxLib is created with functions to perform the related Tdx operation.
> >> This includes functions for:
> >>   - TdCall : to cause a VM exit to the Intel TDX module
> >>   - TdVmCall   : it is a leaf function 0 for TDCALL
> >>   - TdVmCallCpuid  : enable the TD guest to request VMM to emulate CPUID
> >>   - TdReport   : to retrieve TDREPORT_STRUCT
> >>   - TdAcceptPages  : to accept pending private pages
> >>   - TdExtendRtmr   : to extend one of the RTMR registers
> >>
> >> The base function in MdePkg will not do anything and will return an error
> >> if a return value is required. It is expected that other packages
> >> (like OvmfPkg) will create a version of the library to fully support a TD
> >> guest.
> >>
> >> We create an OVMF version of this library to begin the process of providing
> >> full support of TDX in OVMF.
> >>
> >> To support the emulation and test purpose, 2 PCDs are added in OvmfPkg.dec
> >>   - PcdUseTdxAcceptPage
> >> Indicate whether TdCall(AcceptPage) is used.
> >>   - PcdUseTdxEmulation
> >> Indicate whether TdxEmulation is used.
> >
> > (1) per Jiewen's feedback, please drop these PCDs -- importantly, please
> > drop DB-encoded instructions in assembly source code
> >
> > (2) It's not really helpful to post three versions of a patch set over
> > the course of a few hours. I don't suggest posting more frequently than
> > once per day, unless agreed otherwise.
> >
> > (3) Please add a new section to Maintainers.txt for TDX content in
> > OvmfPkg. At least two Intel developers should be listed there as
> > Reviewers. I'd like to permanently delegate TDX reviews to Intel
> > contributors.
> >
> > See also the "OvmfPkg: SEV-related modules" section in "Maintainers.txt".
> >
> > (4) The patches contain numerous style issues:
> >
> > - overlong lines,
> >
> > - incomplete "@retval" comments,
> >
> > - Library #include directives mixed with non-library #include directives,
> >
> > - variables that should be STATIC but are not declared like that,
> >
> > - whitespace errors: missing space character between function designator
> > (or macro name) and opening paren
> >
> > - more whitespace errors: missing space characters around "if" and
> > "else" keywords
> >
> > (5) Some of the source files have outdated license blocks (e.g.,
> > open-coding the 2-clause BSDL and stating a copyright year of 2020,
> > rather than stating 2021 and using "SPDX-License-Identifier:
> > BSD-2-Clause-Patent")
> >
> > Please go over the patches with a fine-toothed comb and refresh them.
> >
> > (6) It would be nice if SEV-related patch sets and TDX-related patch
> > sets were cross-CC'd between AMD and Intel contributors. (With the
> > intent being code reuse, and perhaps "design reuse".)
> >
> > Maybe we should have an additional "confidential computing" reviewers
> > section in "Maintainers.txt", covering both SEV and TDX modules. This
> > would allow for a wider set 

[edk2-devel] [PATCH edk2-test 1/1] uefi-sct/SctPkg: type mismatch in SimpleTextOut test

2021-03-09 Thread Heinrich Schuchardt
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3253

SctPrint() requires that %d refers to an UINTN parameter.

SimpleTextOutBBTestFunction_uefi.c has a lot of
StandardLib->RecordAssertion() calls where an INT32 is passed
as argument for a '%d' print code.

This leads to incorrect output like:

MaxMode=-549755813885,

-549755813885 is 0x0xFF83. So MaxMode actually
is an INT32 with value 3 in this example.

Convert the parameters to UINTN.

Signed-off-by: Heinrich Schuchardt 
---
 .../SimpleTextOutBBTestFunction_uefi.c| 624 +-
 1 file changed, 312 insertions(+), 312 deletions(-)

diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextOut/BlackBoxTest/SimpleTextOutBBTestFunction_uefi.c
 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextOut/BlackBoxTest/SimpleTextOutBBTestFunction_uefi.c
index 9b0ae233ce5f..bbe3f4e27077 100644
--- 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextOut/BlackBoxTest/SimpleTextOutBBTestFunction_uefi.c
+++ 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleTextOut/BlackBoxTest/SimpleTextOutBBTestFunction_uefi.c
@@ -176,12 +176,12 @@ BBTestResetFunctionManualTest (
  L" Expected:Cursor Position(%d x %d), MaxMode=%d.",
  __FILE__,
  (UINTN)__LINE__,
- SimpleOut->Mode->CursorColumn,
- SimpleOut->Mode->CursorRow,
- SimpleOut->Mode->MaxMode,
- ModeExpected.CursorColumn,
- ModeExpected.CursorRow,
- ModeExpected.MaxMode
+ (UINTN)SimpleOut->Mode->CursorColumn,
+ (UINTN)SimpleOut->Mode->CursorRow,
+ (UINTN)SimpleOut->Mode->MaxMode,
+ (UINTN)ModeExpected.CursorColumn,
+ (UINTN)ModeExpected.CursorRow,
+ (UINTN)ModeExpected.MaxMode
  );

   //
@@ -272,12 +272,12 @@ BBTestResetFunctionManualTest (
  L" Expected:Cursor Position(%d x %d), MaxMode=%d.",
  __FILE__,
  (UINTN)__LINE__,
- SimpleOut->Mode->CursorColumn,
- SimpleOut->Mode->CursorRow,
- SimpleOut->Mode->MaxMode,
- ModeExpected.CursorColumn,
- ModeExpected.CursorRow,
- ModeExpected.MaxMode
+ (UINTN)SimpleOut->Mode->CursorColumn,
+ (UINTN)SimpleOut->Mode->CursorRow,
+ (UINTN)SimpleOut->Mode->MaxMode,
+ (UINTN)ModeExpected.CursorColumn,
+ (UINTN)ModeExpected.CursorRow,
+ (UINTN)ModeExpected.MaxMode
  );

   //
@@ -505,12 +505,12 @@ BBTestResetFunctionAutoTest (
L"Expected:Cursor Position(%d x %d), MaxMode=%d.",
__FILE__,
(UINTN)__LINE__,
-   SimpleOut->Mode->CursorColumn,
-   SimpleOut->Mode->CursorRow,
-   SimpleOut->Mode->MaxMode,
-   ModeExpected.CursorColumn,
-   ModeExpected.CursorRow,
-   ModeExpected.MaxMode
+   (UINTN)SimpleOut->Mode->CursorColumn,
+   (UINTN)SimpleOut->Mode->CursorRow,
+   (UINTN)SimpleOut->Mode->MaxMode,
+   (UINTN)ModeExpected.CursorColumn,
+   (UINTN)ModeExpected.CursorRow,
+   (UINTN)ModeExpected.MaxMode
);

 //
@@ -582,12 +582,12 @@ BBTestResetFunctionAutoTest (
L" Expected:Cursor Position(%d x %d), MaxMode=%d.",
__FILE__,
(UINTN)__LINE__,
-   SimpleOut->Mode->CursorColumn,
-   SimpleOut->Mode->CursorRow,
-   SimpleOut->Mode->MaxMode,
-   ModeExpected.CursorColumn,
-   ModeExpected.CursorRow,
-   ModeExpected.MaxMode
+   (UINTN)SimpleOut->Mode->CursorColumn,
+   (UINTN)SimpleOut->Mode->CursorRow,
+   (UINTN)SimpleOut->Mode->MaxMode,
+   (UINTN)ModeExpected.CursorColumn,
+   (UINTN)ModeExpected.CursorRow,
+   (UINTN)ModeExpected.MaxMode
);

 //
@@ -850,18 +850,18 @@ BBTestOutputStringFunctionAutoTest (
  L" Expected:Cursor Position(%d x %d), Mode=%d, MaxMode=%d, 
Attribute=%d, CursorVisible=%d.",
  __FILE__,
  (UINTN)__LINE__,
- SimpleOut->Mode->CursorColumn,
- SimpleOut->Mode->CursorRow,
- SimpleOut->Mode->Mode,
- SimpleOut->Mode->MaxMode,
- SimpleOut->Mode->Attribute,
- SimpleOut->Mode->CursorVisible,
- ModeExpected.CursorColumn,
- ModeExpected.CursorRow,
- ModeExpected.Mode,
- 

[edk2-devel] [PATCH] ArmPkg/ArmGic: Fix maximum number of interrupts in GICv3

2021-03-09 Thread Joey Gouly
From: Andreas Sandberg 

The GICv3 architecture supports up to 1020 ordinary interrupt
lines. The actual number of interrupts supported is described by the
ITLinesNumber field in the GICD_TYPER register. The total number of
implemented registers is normally calculated as
32*(ITLinesNumber+1). However, maximum value (0x1f) is a special case
since that would indicate that 1024 interrupts are implemented.

Add handling for this special case in ArmGicGetMaxNumInterrupts.

Signed-off-by: Andreas Sandberg 
Signed-off-by: Joey Gouly 
---

The changes can be seen at 
https://github.com/jgouly/edk2/tree/1396_gic_max_num_intr_v1

 ArmPkg/Drivers/ArmGic/ArmGicLib.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c 
b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
index 
6b01c88206ad8adef3100dd44c0d57660db77783..c4996970eb299776552fe7b54f34fd5a68afff29
 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
+++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
@@ -1,6 +1,6 @@
 /** @file
 *
-*  Copyright (c) 2011-2018, ARM Limited. All rights reserved.
+*  Copyright (c) 2011-2021, ARM Limited. All rights reserved.
 *
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
 *
@@ -120,7 +120,12 @@ ArmGicGetMaxNumInterrupts (
   IN  INTN  GicDistributorBase
   )
 {
-  return 32 * ((MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) & 0x1F) + 1);
+  UINTN ItLines;
+
+  ItLines = MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) & 0x1F;
+
+  // Interrupt ID 1020-1023 are reserved;
+  return (ItLines == 0x1f) ? 1020 : 32 * (ItLines + 1);
 }
 
 VOID
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72596): https://edk2.groups.io/g/devel/message/72596
Mute This Topic: https://groups.io/mt/81205553/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/1] BaseTools/Ecc: Make Ecc only check first include guard

2021-03-09 Thread PierreGondois
From: Pierre Gondois 

The Ecc tool checks the format of the include guard. This check is
currently done on all the names following the '#ifndef' statement.
It should only be done on the first include guard.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3252
Signed-off-by: Pierre Gondois 
---
The changes can be seen at: 
https://github.com/PierreARM/edk2/tree/1640_Ecc_tool_corrections

 BaseTools/Source/Python/Ecc/Check.py | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/Python/Ecc/Check.py 
b/BaseTools/Source/Python/Ecc/Check.py
index 7a012617fd35..d82b42de0119 100644
--- a/BaseTools/Source/Python/Ecc/Check.py
+++ b/BaseTools/Source/Python/Ecc/Check.py
@@ -3,6 +3,7 @@
 #
 # Copyright (c) 2021, Arm Limited. All rights reserved.
 # Copyright (c) 2008 - 2020, Intel Corporation. All rights reserved.
+# Copyright (c) 2021, Arm Limited. All rights reserved.
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 from __future__ import absolute_import
@@ -1437,11 +1438,13 @@ class Check(object):

 SqlCommand = """select ID, Value from %s where Model = %s""" % 
(FileTable, MODEL_IDENTIFIER_MACRO_IFNDEF)
 RecordSet = EccGlobalData.gDb.TblFile.Exec(SqlCommand)
-for Record in RecordSet:
-Name = Record[1].replace('#ifndef', '').strip()
+if RecordSet:
+# Only check the first ifndef statement of the file
+FirstDefine = sorted(RecordSet, key=lambda Record: 
Record[0])[0]
+Name = FirstDefine[1].replace('#ifndef', '').strip()
 if Name[0] == '_' or Name[-1] != '_' or Name[-2] == '_':
 if not 
EccGlobalData.gException.IsException(ERROR_NAMING_CONVENTION_CHECK_IFNDEF_STATEMENT,
 Name):
-
EccGlobalData.gDb.TblReport.Insert(ERROR_NAMING_CONVENTION_CHECK_IFNDEF_STATEMENT,
 OtherMsg="The #ifndef name [%s] does not follow the rules" % (Name), 
BelongsToTable=FileTable, BelongsToItem=Record[0])
+
EccGlobalData.gDb.TblReport.Insert(ERROR_NAMING_CONVENTION_CHECK_IFNDEF_STATEMENT,
 OtherMsg="The #ifndef name [%s] does not follow the rules" % (Name), 
BelongsToTable=FileTable, BelongsToItem=FirstDefine[0])

 # Rule for path name, variable name and function name
 # 1. First character should be upper case
--
2.17.1



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




Re: [edk2-devel] [PATCH v2 1/1] ShellPkg: Validate that the Boot CPU is present in MADT

2021-03-09 Thread Joey Gouly
> From: Joey Gouly 
> Subject: [PATCH v2 1/1] ShellPkg: Validate that the Boot CPU is present in 
> MADT
>
> The ACPI 6.3 Specification, January 2019, section 5.2.12.14 states that
> the firmware must convey each processor’s GIC information to the OS using
> the GICC structure.
>
> If a GICC structure for the boot CPU is missing some standards-based
> operating system may crash with an error code. It may be difficult to
> diagnose the reason for the crash as the error code may be too generic
> and mean firmware error.
>
> Therefore add validation to the MADT table parser to check that a GICC is
> present for the boot CPU.

Ping.

Is anyone able to review this patch adding some more validation checks to 
Acpiview / MADT?

Thanks,
Joey

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




Re: [edk2-devel] [PATCH 3/3 v6] Maintainers: Add maintainers for StandAloneMM and RPMD driver

2021-03-09 Thread Ilias Apalodimas
I just noticed I somehow managed to remove OptionRomPkg from the list.

I'll wait in case anyone has more remarks on the v6 and I'll send a v7
fixing this.
Sorry for the noise.

On Tue, 9 Mar 2021 at 16:01, Ilias Apalodimas via groups.io
 wrote:
>
> Add Sami and myself as maintainers for the new StandAlonemmPkg
> and the relevant RPMB driver that can be used in OP-TEE
>
> Signed-off-by: Ilias Apalodimas 
> ---
>  Maintainers.txt | 15 ++-
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/Maintainers.txt b/Maintainers.txt
> index 56e16fc48cb4..1aa4234f52ed 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -118,11 +118,6 @@ F: Platform/Comcast/
>  M: Ard Biesheuvel 
>  M: Leif Lindholm 
>
> -OptionRomPkg
> -F: Drivers/OptionRomPkg/
> -W: https://github.com/tianocore/tianocore.github.io/wiki/OptionRomPkg
> -M: Ray Ni 
> -
>  DisplayLink
>  F: Drivers/DisplayLink/
>  M: Leif Lindholm 
> @@ -256,6 +251,11 @@ M: Bob Feng 
>  M: Liming Gao 
>  R: Yuwei Chen 
>
> +StandAloneMMPkg for OP-TEE
> +F: Platform/StandaloneMm/PlatformStandaloneMmPkg/
> +M: Sami Mujawar 
> +M: Ilias Apalodimas 
> +
>  Marvell platforms and silicon
>  F: Platform/Marvell/
>  F: Platform/SolidRun/Armada80x0McBin/
> @@ -292,6 +292,11 @@ M: Ard Biesheuvel 
>  M: Leif Lindholm 
>  R: Pete Batard 
>
> +RPMB driver for OP-TEE
> +F: Drivers/OpTee/OpteeRpmbPkg/
> +M: Sami Mujawar 
> +M: Ilias Apalodimas 
> +
>  Socionext platforms and silicon
>  F: Platform/Socionext/
>  F: Silicon/NXP/Library/Pcf8563RealTimeClockLib/
> --
> 2.30.1
>
>
>
> 
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#72590): https://edk2.groups.io/g/devel/message/72590
> Mute This Topic: https://groups.io/mt/81201376/4721553
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub 
> [ilias.apalodi...@linaro.org]
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72593): https://edk2.groups.io/g/devel/message/72593
Mute This Topic: https://groups.io/mt/81203420/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 0/3] Add TdxLib support for Intel TDX

2021-03-09 Thread Laszlo Ersek
On 03/09/21 14:06, Laszlo Ersek wrote:
> On 03/09/21 13:57, Laszlo Ersek wrote:
>> On 03/09/21 07:12, Min Xu wrote:
>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3249
>>>
>>> The patch series provides lib support for Intel Trust Domain Extensions
>>> (Intel TDX).
>>>
>>> Intel's Trust Domain Extensions (Intel TDX) refers to an Intel technology
>>> that extends Virtual Machines Extensions (VMX) and Multi-Key Total Memory
>>> Encryption (MKTME) with a new kind of virutal machines guest called a 
>>> Trust Domain (TD). A TD is desinged to run in a CPU mode that protects the
>>> confidentiality of TD memory contents and the TD's CPU state from other
>>> software, including the hosting Virtual-Machine Monitor (VMM), unless
>>> explicitly shared by the TD itself.
>>>
>>> The Intel TDX module uses the instruction-set architecture for Intel TDX
>>> and the MKTME engine in the SOC to help serve as an intermediary between
>>> the host VMM and the guest TD. TDCALL is the instruction which allows TD
>>> guest privileged software to make a call for service into an underlying
>>> TDX-module.
>>>
>>> TdxLib is created with functions to perform the related Tdx operation.
>>> This includes functions for:
>>>   - TdCall : to cause a VM exit to the Intel TDX module
>>>   - TdVmCall   : it is a leaf function 0 for TDCALL
>>>   - TdVmCallCpuid  : enable the TD guest to request VMM to emulate CPUID
>>>   - TdReport   : to retrieve TDREPORT_STRUCT
>>>   - TdAcceptPages  : to accept pending private pages
>>>   - TdExtendRtmr   : to extend one of the RTMR registers
>>>
>>> The base function in MdePkg will not do anything and will return an error
>>> if a return value is required. It is expected that other packages
>>> (like OvmfPkg) will create a version of the library to fully support a TD
>>> guest.
>>>
>>> We create an OVMF version of this library to begin the process of providing
>>> full support of TDX in OVMF.
>>>
>>> To support the emulation and test purpose, 2 PCDs are added in OvmfPkg.dec
>>>   - PcdUseTdxAcceptPage
>>> Indicate whether TdCall(AcceptPage) is used.
>>>   - PcdUseTdxEmulation
>>> Indicate whether TdxEmulation is used.
>>
>> (1) per Jiewen's feedback, please drop these PCDs -- importantly, please
>> drop DB-encoded instructions in assembly source code
>>
>> (2) It's not really helpful to post three versions of a patch set over
>> the course of a few hours. I don't suggest posting more frequently than
>> once per day, unless agreed otherwise.
>>
>> (3) Please add a new section to Maintainers.txt for TDX content in
>> OvmfPkg. At least two Intel developers should be listed there as
>> Reviewers. I'd like to permanently delegate TDX reviews to Intel
>> contributors.
>>
>> See also the "OvmfPkg: SEV-related modules" section in "Maintainers.txt".
>>
>> (4) The patches contain numerous style issues:
>>
>> - overlong lines,
>>
>> - incomplete "@retval" comments,
>>
>> - Library #include directives mixed with non-library #include directives,
>>
>> - variables that should be STATIC but are not declared like that,
>>
>> - whitespace errors: missing space character between function designator
>> (or macro name) and opening paren
>>
>> - more whitespace errors: missing space characters around "if" and
>> "else" keywords
>>
>> (5) Some of the source files have outdated license blocks (e.g.,
>> open-coding the 2-clause BSDL and stating a copyright year of 2020,
>> rather than stating 2021 and using "SPDX-License-Identifier:
>> BSD-2-Clause-Patent")
>>
>> Please go over the patches with a fine-toothed comb and refresh them.
>>
>> (6) It would be nice if SEV-related patch sets and TDX-related patch
>> sets were cross-CC'd between AMD and Intel contributors. (With the
>> intent being code reuse, and perhaps "design reuse".)
>>
>> Maybe we should have an additional "confidential computing" reviewers
>> section in "Maintainers.txt", covering both SEV and TDX modules. This
>> would allow for a wider set of CC's, without obscuring who should review
>> TDX vs. who should review SEV. I think this unified section should list
>> a number of IBM developers too.
> 
> (7) Some more admin stuff:
> 
> (7a) every patch in this series should carry the following line in the
> commit message:
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3249
> 
> (7b) whenever you post a new version of the patch set, please add a new
> comment to ,
> linking the just-posted version (the cover letter email) from the
> mailing list archive.
> 
> This is important in case we want to review the evolution of the patch
> series later. It's more difficult to find relevant email threads later
> than to link each posting immediately in the bugzilla ticket.

(8) As-is, the patch set does not enable the new library instance under
OvmfPkg to be built, at all. That's wrong; we shouldn't add a new lib
instance that can't even be build-tested -- the CI on github.com won't

Re: [edk2-devel] [PATCH v3] UefiCpuPkg/PiSmmCpu: Don't allocate Token for SmmStartupThisAp

2021-03-09 Thread Dong, Eric
Reviewed-by: Eric Dong 

-Original Message-
From: Ni, Ray  
Sent: Tuesday, March 9, 2021 5:09 PM
To: devel@edk2.groups.io
Cc: Dong, Eric ; Laszlo Ersek ; Kumar, 
Rahul1 
Subject: [PATCH v3] UefiCpuPkg/PiSmmCpu: Don't allocate Token for 
SmmStartupThisAp

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3199

When Token points to mSmmStartupThisApToken, this routine is called
from SmmStartupThisAp() in non-blocking mode due to
PcdCpuSmmBlockStartupThisAp == FALSE.

In this case, caller wants to startup AP procedure in non-blocking
mode and cannot get the completion status from the Token because there
is no way to return the Token to caller from SmmStartupThisAp().
Caller needs to use its specific way to query the completion status.

There is no need to allocate a token for such case so the 3 overheads
can be avoided:
1. Call AllocateTokenBuffer() when there is no free token.
2. Get a free token from the token buffer.
3. Call ReleaseToken() in APHandler().

Signed-off-by: Ray Ni 
Cc: Eric Dong 
Reviewed-by: Laszlo Ersek 
Cc: Rahul Kumar 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 30 ---
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 6227b2428a..4f3c5f60a1 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -1,7 +1,7 @@
 /** @file

 SMM MP service implementation

 

-Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.

+Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.

 Copyright (c) 2017, AMD Incorporated. All rights reserved.

 

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

@@ -22,6 +22,7 @@ UINTN   mSemaphoreSize;
 SPIN_LOCK   *mPFLock = NULL;

 SMM_CPU_SYNC_MODE   mCpuSmmSyncMode;

 BOOLEAN mMachineCheckSupported = FALSE;

+MM_COMPLETION   mSmmStartupThisApToken;

 

 extern UINTN mSmmShadowStackSize;

 

@@ -1240,9 +1241,26 @@ InternalSmmStartupThisAp (
   mSmmMpSyncData->CpuData[CpuIndex].Procedure = Procedure;

   mSmmMpSyncData->CpuData[CpuIndex].Parameter = ProcArguments;

   if (Token != NULL) {

-ProcToken= GetFreeToken (1);

-mSmmMpSyncData->CpuData[CpuIndex].Token = ProcToken;

-*Token = (MM_COMPLETION)ProcToken->SpinLock;

+if (Token != ) {

+  //

+  // When Token points to mSmmStartupThisApToken, this routine is called

+  // from SmmStartupThisAp() in non-blocking mode 
(PcdCpuSmmBlockStartupThisAp == FALSE).

+  //

+  // In this case, caller wants to startup AP procedure in non-blocking

+  // mode and cannot get the completion status from the Token because there

+  // is no way to return the Token to caller from SmmStartupThisAp().

+  // Caller needs to use its implementation specific way to query the 
completion status.

+  //

+  // There is no need to allocate a token for such case so the 3 overheads

+  // can be avoided:

+  // 1. Call AllocateTokenBuffer() when there is no free token.

+  // 2. Get a free token from the token buffer.

+  // 3. Call ReleaseToken() in APHandler().

+  //

+  ProcToken= GetFreeToken (1);

+  mSmmMpSyncData->CpuData[CpuIndex].Token = ProcToken;

+  *Token = (MM_COMPLETION)ProcToken->SpinLock;

+}

   }

   mSmmMpSyncData->CpuData[CpuIndex].Status= CpuStatus;

   if (mSmmMpSyncData->CpuData[CpuIndex].Status != NULL) {

@@ -1474,8 +1492,6 @@ SmmStartupThisAp (
   IN OUT  VOID  *ProcArguments OPTIONAL

   )

 {

-  MM_COMPLETION   Token;

-

   gSmmCpuPrivate->ApWrapperFunc[CpuIndex].Procedure = Procedure;

   gSmmCpuPrivate->ApWrapperFunc[CpuIndex].ProcedureArgument = ProcArguments;

 

@@ -1486,7 +1502,7 @@ SmmStartupThisAp (
 ProcedureWrapper,

 CpuIndex,

 >ApWrapperFunc[CpuIndex],

-FeaturePcdGet (PcdCpuSmmBlockStartupThisAp) ? NULL : ,

+FeaturePcdGet (PcdCpuSmmBlockStartupThisAp) ? NULL : 
,

 0,

 NULL

 );

-- 
2.27.0.windows.1



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




[edk2-devel] [PATCH 3/3 v6] Maintainers: Add maintainers for StandAloneMM and RPMD driver

2021-03-09 Thread Ilias Apalodimas
Add Sami and myself as maintainers for the new StandAlonemmPkg
and the relevant RPMB driver that can be used in OP-TEE

Signed-off-by: Ilias Apalodimas 
---
 Maintainers.txt | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/Maintainers.txt b/Maintainers.txt
index 56e16fc48cb4..1aa4234f52ed 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -118,11 +118,6 @@ F: Platform/Comcast/
 M: Ard Biesheuvel 
 M: Leif Lindholm 
 
-OptionRomPkg
-F: Drivers/OptionRomPkg/
-W: https://github.com/tianocore/tianocore.github.io/wiki/OptionRomPkg
-M: Ray Ni 
-
 DisplayLink
 F: Drivers/DisplayLink/
 M: Leif Lindholm 
@@ -256,6 +251,11 @@ M: Bob Feng 
 M: Liming Gao 
 R: Yuwei Chen 
 
+StandAloneMMPkg for OP-TEE
+F: Platform/StandaloneMm/PlatformStandaloneMmPkg/
+M: Sami Mujawar 
+M: Ilias Apalodimas 
+
 Marvell platforms and silicon
 F: Platform/Marvell/
 F: Platform/SolidRun/Armada80x0McBin/
@@ -292,6 +292,11 @@ M: Ard Biesheuvel 
 M: Leif Lindholm 
 R: Pete Batard 
 
+RPMB driver for OP-TEE
+F: Drivers/OpTee/OpteeRpmbPkg/
+M: Sami Mujawar 
+M: Ilias Apalodimas 
+
 Socionext platforms and silicon
 F: Platform/Socionext/
 F: Silicon/NXP/Library/Pcf8563RealTimeClockLib/
-- 
2.30.1



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




[edk2-devel] [PATCH 2/3 v6] StMMRpmb: Add support for building StandaloneMm image for OP-TEE

2021-03-09 Thread Ilias Apalodimas
With some recent changes in OP-TEE [1] and U-Boot [2] we can compile StMM
and launch it from an OP-TEE secure partition which is mimicking SPM.

There's a number of advantages in this approach. In Arm world SPM,
currently used for dispatching StMM, and SPD used for OP-TEE, are
mutually exclusive. Since there's no application in OP-TEE for managing
EFI variables, this means that one can have a secure OS or secure
variable storage.

By re-using StMM we have EDK2s approved application controlling
variable storage and the ability to run a secure world OS. This also
allows various firmware implementations to adopt EDK2 way of storing
variables (including the FTW implementation), as long as OP-TEE is
available on that given platform (or any other secure OS that can launch
StMM and has a supplicant for handling the RPMB partition).
Another advantage is that OP-TEE has the ability to access an eMMC RPMB
partition to store those variables. This requires a normal world
supplicant, which is implemented in U-Boot currently. The supplicant
picks up the encrypted buffer from OP-TEE and wires it to the eMMC
driver(s). Similar functionality can be added in EDK2 by porting the
supplicant and adapt it to using the native eMMC drivers.

There's is one drawback in using OP-TEE. The current SPM calls need to run
to completion. This contradicts the current OP-TEE RPC call requirements,
used to access the RPMB storage. Thats leads to two different SMC calls for
entering secure world to access StMM.

So let's add support for a platform that compiles StMM and an RPMB
driver that communicates with OP-TEE to read/write the variables.
For anyone interested in testing this there's repo that builds all the
sources and works on QEMU [3].

[1] https://github.com/OP-TEE/optee_os/pull/3973
[2] 
http://u-boot.10912.n7.nabble.com/PATCH-0-7-v4-EFI-variable-support-via-OP-TEE-td412499.html
[3] https://git.linaro.org/people/ilias.apalodimas/efi_optee_variables.git/

Reviewed-by: Sami Mujawar 
Signed-off-by: Ilias Apalodimas 
---
 .../PlatformStandaloneMmRpmb.dsc  | 162 ++
 .../PlatformStandaloneMmRpmb.fdf  | 111 
 2 files changed, 273 insertions(+)
 create mode 100644 
Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc
 create mode 100644 
Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.fdf

diff --git 
a/Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc 
b/Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc
new file mode 100644
index ..f99a47ebf605
--- /dev/null
+++ b/Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc
@@ -0,0 +1,162 @@
+#
+#  Copyright (c) 2018, ARM Limited. All rights reserved.
+#  Copyright (c) 2020, Linaro Ltd. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+
+#
+# Defines Section - statements that will be processed to create a Makefile.
+#
+
+[Defines]
+  PLATFORM_NAME  = MmStandaloneRpmb
+  PLATFORM_GUID  = A27A486E-D7B9-4D70-9F37-FED9ABE041A2
+  PLATFORM_VERSION   = 1.0
+  DSC_SPECIFICATION  = 0x0001001C
+  OUTPUT_DIRECTORY   = Build/$(PLATFORM_NAME)
+  SUPPORTED_ARCHITECTURES= AARCH64
+  BUILD_TARGETS  = DEBUG|RELEASE|NOOPT
+  SKUID_IDENTIFIER   = DEFAULT
+  FLASH_DEFINITION   = 
Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.fdf
+  DEFINE DEBUG_MESSAGE   = TRUE
+
+
+#
+# Library Class section - list of all Library Classes needed by this Platform.
+#
+
+[LibraryClasses]
+  ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
+  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
+  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
+  SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
+  
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
+  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.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
+  

[edk2-devel] [PATCH 1/3 v6] Drivers/OpTeeRpmb: Add an OP-TEE backed RPMB driver

2021-03-09 Thread Ilias Apalodimas
A following patch is adding support for building StMM in order to run it
from OP-TEE.
OP-TEE in combination with a NS-world supplicant can use the RPMB
partition of an eMMC to store EFI variables. The supplicant
functionality is currently available in U-Boot only but can be ported
into EDK2. Assuming similar functionality is added in EDK2, this will
allow any hardware with an RPMB partition to store EFI variables
securely.

So let's add a driver that enables access of the RPMB partition through
OP-TEE. Since the upper layers expect a byte addressable interface,
the driver allocates memory and patches the PCDs, while syncing the
memory/hardware on read/write callbacks.

Signed-off-by: Ilias Apalodimas 
---
 Drivers/OpTee/OpteeRpmbPkg/FixupPcd.c  |  80 ++
 Drivers/OpTee/OpteeRpmbPkg/FixupPcd.inf|  43 +
 Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFv.inf |  58 ++
 Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.c  | 893 +
 Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.h  |  51 ++
 5 files changed, 1125 insertions(+)
 create mode 100644 Drivers/OpTee/OpteeRpmbPkg/FixupPcd.c
 create mode 100644 Drivers/OpTee/OpteeRpmbPkg/FixupPcd.inf
 create mode 100644 Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFv.inf
 create mode 100644 Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.c
 create mode 100644 Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.h

diff --git a/Drivers/OpTee/OpteeRpmbPkg/FixupPcd.c 
b/Drivers/OpTee/OpteeRpmbPkg/FixupPcd.c
new file mode 100644
index ..7e0655f0bfd9
--- /dev/null
+++ b/Drivers/OpTee/OpteeRpmbPkg/FixupPcd.c
@@ -0,0 +1,80 @@
+/** @file
+
+  Update the patched PCDs to their correct value
+
+  Copyright (c) 2020, Linaro Ltd. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+/**
+ * Patch the relevant PCDs of the RPMB driver with the correct address of the
+ * allocated memory
+ *
+**/
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include "OpTeeRpmbFvb.h"
+
+/**
+  Fixup the Pcd values for variable storage
+
+  Since the upper layers of EDK2 expect a memory mapped interface and we can't
+  offer that from an RPMB, the driver allocates memory on init and passes that
+  on the upper layers. Since the memory is dynamically allocated and we can't 
set the
+  PCD in StMM context, we need to patch it correctly on each access
+
+  @retval EFI_SUCCESS Protocol was found and PCDs patched up
+
+**/
+EFI_STATUS
+EFIAPI
+FixPcdMemory (
+  VOID
+  )
+{
+  EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL  *FvbProtocol;
+  MEM_INSTANCE*Instance;
+  EFI_STATUS  Status;
+
+  //
+  // Locate SmmFirmwareVolumeBlockProtocol
+  //
+  Status = gMmst->MmLocateProtocol (
+,
+NULL,
+(VOID **) 
+);
+  ASSERT_EFI_ERROR (Status);
+
+  Instance = INSTANCE_FROM_FVB_THIS (FvbProtocol);
+
+  // Patch PCDs with the correct values
+  PatchPcdSet64 (PcdFlashNvStorageVariableBase64, Instance->MemBaseAddress);
+  PatchPcdSet64 (
+PcdFlashNvStorageFtwWorkingBase64,
+Instance->MemBaseAddress + PcdGet32 (PcdFlashNvStorageVariableSize)
+);
+  PatchPcdSet64 (
+PcdFlashNvStorageFtwSpareBase64,
+Instance->MemBaseAddress +
+PcdGet32 (PcdFlashNvStorageVariableSize) +
+PcdGet32 (PcdFlashNvStorageFtwWorkingSize)
+);
+
+  DEBUG ((DEBUG_INFO, "%a: Fixup PcdFlashNvStorageVariableBase64: 0x%lx\n",
+__FUNCTION__, PcdGet64 (PcdFlashNvStorageVariableBase64)));
+  DEBUG ((DEBUG_INFO, "%a: Fixup PcdFlashNvStorageFtwWorkingBase64: 0x%lx\n",
+__FUNCTION__, PcdGet64 (PcdFlashNvStorageFtwWorkingBase64)));
+  DEBUG ((DEBUG_INFO, "%a: Fixup PcdFlashNvStorageFtwSpareBase64: 0x%lx\n",
+__FUNCTION__, PcdGet64 (PcdFlashNvStorageFtwSpareBase64)));
+
+  return Status;
+}
diff --git a/Drivers/OpTee/OpteeRpmbPkg/FixupPcd.inf 
b/Drivers/OpTee/OpteeRpmbPkg/FixupPcd.inf
new file mode 100644
index ..5146257993ef
--- /dev/null
+++ b/Drivers/OpTee/OpteeRpmbPkg/FixupPcd.inf
@@ -0,0 +1,43 @@
+## @file
+#  Instance of Base Memory Library without assembly.
+#
+#  Copyright (c) 2020, Linaro Ltd. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+##
+
+[Defines]
+  INF_VERSION= 0x0001001A
+  BASE_NAME  = FixupPcd
+  FILE_GUID  = a827c337-a9c6-301b-aeb7-acbc95d8da22
+  MODULE_TYPE= BASE
+  VERSION_STRING = 0.1
+  LIBRARY_CLASS  = RpmbPcdFixup|MM_STANDALONE
+  CONSTRUCTOR= FixPcdMemory
+
+[Sources]
+  FixupPcd.c
+  OpTeeRpmbFvb.h
+
+[Packages]
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  MmServicesTableLib
+  PcdLib
+
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64
+  

[edk2-devel] [PATCH 0/3 v6] Add support for running StandaloneMm as OP-TEE TA

2021-03-09 Thread Ilias Apalodimas
Hi, 
This is v6 of [1] 

Changes since V5:
 - Addressed coding style fixes from Pierre
 - Removed redundant checks in memory allocation and block usage
 - Removed unused COMPRESSION_TOOL_GUID
 - Renamed the files and directories following Leif's sugestion
 - Added Sami and myself as maintainers

Changes since V4:
 - More coding stule fixes proposed by Sami, which Ecc or Patchcheck didn't
   report.
 - Adding missing error handling in InitializeFvAndVariableStoreHeaders().
   An allocation wasn't properly checked for success

Changes since V3:
 - Coding style fixes proposed by Sami
 - Fixed all reported PatchCheck errors
 - Added overflow checks on the base aaddress allocated for EFI variables.
   The size of the partition is user defined (via Pcd's) and the memory layout
   and allocation address depends on OP-TEE. So let's make sure we won't 
overflow
   when calculating the 3 partitions needed for FTW
 - Switched some PcdGet/Set32 to 64 to accomodate 64-bit addressing
 - Removed some duplicate entries in Platform/StMMRpmb/PlatformStandaloneMm.dsc
 - Added reviewed-by tags on patch 2/2

Changes since V2:
 - Allocate a dynamic number of pages based on the Pcd values instead
   of a static number
 - Clean up unused structs in header file
 - Added checks in OpTeeRpmbFvbGetBlockSize and handle NumLba=0

Changes since V1:
Some enhancements made by Ilias to the Optee Rpmb driver


[1] 
https://edk2.groups.io/g/devel/message/66483?p=,,,20,0,0,0::Created,,ilias+apalodimas,20,2,0,77703661

Ilias Apalodimas (3):
  Drivers/OpTeeRpmb: Add an OP-TEE backed RPMB driver
  StMMRpmb: Add support for building StandaloneMm image for OP-TEE
  Maintainers: Add maintainers for StandAloneMM and RPMD driver

 Drivers/OpTee/OpteeRpmbPkg/FixupPcd.c |  80 ++
 Drivers/OpTee/OpteeRpmbPkg/FixupPcd.inf   |  43 +
 Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFv.inf|  58 ++
 Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.c | 893 ++
 Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.h |  51 +
 Maintainers.txt   |  15 +-
 .../PlatformStandaloneMmRpmb.dsc  | 162 
 .../PlatformStandaloneMmRpmb.fdf  | 111 +++
 8 files changed, 1408 insertions(+), 5 deletions(-)
 create mode 100644 Drivers/OpTee/OpteeRpmbPkg/FixupPcd.c
 create mode 100644 Drivers/OpTee/OpteeRpmbPkg/FixupPcd.inf
 create mode 100644 Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFv.inf
 create mode 100644 Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.c
 create mode 100644 Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.h
 create mode 100644 
Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc
 create mode 100644 
Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.fdf

-- 
2.30.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72587): https://edk2.groups.io/g/devel/message/72587
Mute This Topic: https://groups.io/mt/81201371/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 0/3] Add TdxLib support for Intel TDX

2021-03-09 Thread Laszlo Ersek
On 03/09/21 13:57, Laszlo Ersek wrote:
> On 03/09/21 07:12, Min Xu wrote:
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3249
>>
>> The patch series provides lib support for Intel Trust Domain Extensions
>> (Intel TDX).
>>
>> Intel's Trust Domain Extensions (Intel TDX) refers to an Intel technology
>> that extends Virtual Machines Extensions (VMX) and Multi-Key Total Memory
>> Encryption (MKTME) with a new kind of virutal machines guest called a 
>> Trust Domain (TD). A TD is desinged to run in a CPU mode that protects the
>> confidentiality of TD memory contents and the TD's CPU state from other
>> software, including the hosting Virtual-Machine Monitor (VMM), unless
>> explicitly shared by the TD itself.
>>
>> The Intel TDX module uses the instruction-set architecture for Intel TDX
>> and the MKTME engine in the SOC to help serve as an intermediary between
>> the host VMM and the guest TD. TDCALL is the instruction which allows TD
>> guest privileged software to make a call for service into an underlying
>> TDX-module.
>>
>> TdxLib is created with functions to perform the related Tdx operation.
>> This includes functions for:
>>   - TdCall : to cause a VM exit to the Intel TDX module
>>   - TdVmCall   : it is a leaf function 0 for TDCALL
>>   - TdVmCallCpuid  : enable the TD guest to request VMM to emulate CPUID
>>   - TdReport   : to retrieve TDREPORT_STRUCT
>>   - TdAcceptPages  : to accept pending private pages
>>   - TdExtendRtmr   : to extend one of the RTMR registers
>>
>> The base function in MdePkg will not do anything and will return an error
>> if a return value is required. It is expected that other packages
>> (like OvmfPkg) will create a version of the library to fully support a TD
>> guest.
>>
>> We create an OVMF version of this library to begin the process of providing
>> full support of TDX in OVMF.
>>
>> To support the emulation and test purpose, 2 PCDs are added in OvmfPkg.dec
>>   - PcdUseTdxAcceptPage
>> Indicate whether TdCall(AcceptPage) is used.
>>   - PcdUseTdxEmulation
>> Indicate whether TdxEmulation is used.
> 
> (1) per Jiewen's feedback, please drop these PCDs -- importantly, please
> drop DB-encoded instructions in assembly source code
> 
> (2) It's not really helpful to post three versions of a patch set over
> the course of a few hours. I don't suggest posting more frequently than
> once per day, unless agreed otherwise.
> 
> (3) Please add a new section to Maintainers.txt for TDX content in
> OvmfPkg. At least two Intel developers should be listed there as
> Reviewers. I'd like to permanently delegate TDX reviews to Intel
> contributors.
> 
> See also the "OvmfPkg: SEV-related modules" section in "Maintainers.txt".
> 
> (4) The patches contain numerous style issues:
> 
> - overlong lines,
> 
> - incomplete "@retval" comments,
> 
> - Library #include directives mixed with non-library #include directives,
> 
> - variables that should be STATIC but are not declared like that,
> 
> - whitespace errors: missing space character between function designator
> (or macro name) and opening paren
> 
> - more whitespace errors: missing space characters around "if" and
> "else" keywords
> 
> (5) Some of the source files have outdated license blocks (e.g.,
> open-coding the 2-clause BSDL and stating a copyright year of 2020,
> rather than stating 2021 and using "SPDX-License-Identifier:
> BSD-2-Clause-Patent")
> 
> Please go over the patches with a fine-toothed comb and refresh them.
> 
> (6) It would be nice if SEV-related patch sets and TDX-related patch
> sets were cross-CC'd between AMD and Intel contributors. (With the
> intent being code reuse, and perhaps "design reuse".)
> 
> Maybe we should have an additional "confidential computing" reviewers
> section in "Maintainers.txt", covering both SEV and TDX modules. This
> would allow for a wider set of CC's, without obscuring who should review
> TDX vs. who should review SEV. I think this unified section should list
> a number of IBM developers too.

(7) Some more admin stuff:

(7a) every patch in this series should carry the following line in the
commit message:

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3249

(7b) whenever you post a new version of the patch set, please add a new
comment to ,
linking the just-posted version (the cover letter email) from the
mailing list archive.

This is important in case we want to review the evolution of the patch
series later. It's more difficult to find relevant email threads later
than to link each posting immediately in the bugzilla ticket.

Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72586): https://edk2.groups.io/g/devel/message/72586
Mute This Topic: https://groups.io/mt/81195550/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 0/3] Add TdxLib support for Intel TDX

2021-03-09 Thread Laszlo Ersek
On 03/09/21 07:12, Min Xu wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3249
> 
> The patch series provides lib support for Intel Trust Domain Extensions
> (Intel TDX).
> 
> Intel's Trust Domain Extensions (Intel TDX) refers to an Intel technology
> that extends Virtual Machines Extensions (VMX) and Multi-Key Total Memory
> Encryption (MKTME) with a new kind of virutal machines guest called a 
> Trust Domain (TD). A TD is desinged to run in a CPU mode that protects the
> confidentiality of TD memory contents and the TD's CPU state from other
> software, including the hosting Virtual-Machine Monitor (VMM), unless
> explicitly shared by the TD itself.
> 
> The Intel TDX module uses the instruction-set architecture for Intel TDX
> and the MKTME engine in the SOC to help serve as an intermediary between
> the host VMM and the guest TD. TDCALL is the instruction which allows TD
> guest privileged software to make a call for service into an underlying
> TDX-module.
> 
> TdxLib is created with functions to perform the related Tdx operation.
> This includes functions for:
>   - TdCall : to cause a VM exit to the Intel TDX module
>   - TdVmCall   : it is a leaf function 0 for TDCALL
>   - TdVmCallCpuid  : enable the TD guest to request VMM to emulate CPUID
>   - TdReport   : to retrieve TDREPORT_STRUCT
>   - TdAcceptPages  : to accept pending private pages
>   - TdExtendRtmr   : to extend one of the RTMR registers
> 
> The base function in MdePkg will not do anything and will return an error
> if a return value is required. It is expected that other packages
> (like OvmfPkg) will create a version of the library to fully support a TD
> guest.
> 
> We create an OVMF version of this library to begin the process of providing
> full support of TDX in OVMF.
> 
> To support the emulation and test purpose, 2 PCDs are added in OvmfPkg.dec
>   - PcdUseTdxAcceptPage
> Indicate whether TdCall(AcceptPage) is used.
>   - PcdUseTdxEmulation
> Indicate whether TdxEmulation is used.

(1) per Jiewen's feedback, please drop these PCDs -- importantly, please
drop DB-encoded instructions in assembly source code

(2) It's not really helpful to post three versions of a patch set over
the course of a few hours. I don't suggest posting more frequently than
once per day, unless agreed otherwise.

(3) Please add a new section to Maintainers.txt for TDX content in
OvmfPkg. At least two Intel developers should be listed there as
Reviewers. I'd like to permanently delegate TDX reviews to Intel
contributors.

See also the "OvmfPkg: SEV-related modules" section in "Maintainers.txt".

(4) The patches contain numerous style issues:

- overlong lines,

- incomplete "@retval" comments,

- Library #include directives mixed with non-library #include directives,

- variables that should be STATIC but are not declared like that,

- whitespace errors: missing space character between function designator
(or macro name) and opening paren

- more whitespace errors: missing space characters around "if" and
"else" keywords

(5) Some of the source files have outdated license blocks (e.g.,
open-coding the 2-clause BSDL and stating a copyright year of 2020,
rather than stating 2021 and using "SPDX-License-Identifier:
BSD-2-Clause-Patent")

Please go over the patches with a fine-toothed comb and refresh them.

(6) It would be nice if SEV-related patch sets and TDX-related patch
sets were cross-CC'd between AMD and Intel contributors. (With the
intent being code reuse, and perhaps "design reuse".)

Maybe we should have an additional "confidential computing" reviewers
section in "Maintainers.txt", covering both SEV and TDX modules. This
would allow for a wider set of CC's, without obscuring who should review
TDX vs. who should review SEV. I think this unified section should list
a number of IBM developers too.

Thanks,
Laszlo

> 
>  intel-trust-domain-extensions.html>, defitions in TdxLib comes from:
>   [1] Intel TDX(R) Module 1.0 EAS
>   [2] Intel(R) TDX Guest-Hypervisor Communication Interface
> 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Cc: Jiewen Yao 
> 
> Signed-off-by: Min Xu 
> 
> Min Xu (3):
>   MdePkg: Add Tdx support lib
>   OvmfPkg: Add PCDs for TdxLib
>   OvmfPkg: Implement library support for TdxLib SEC and DXE on OVMF
> 
>  MdePkg/Include/IndustryStandard/Tdx.h| 201 +
>  MdePkg/Include/Library/TdxLib.h  | 165 ++
>  MdePkg/Include/Protocol/Tdx.h|  29 
>  MdePkg/Library/TdxLib/TdxLibNull.c   | 155 +
>  MdePkg/Library/TdxLib/TdxLibNull.inf |  33 
>  OvmfPkg/Library/TdxLib/AcceptPages.c |  68 
>  OvmfPkg/Library/TdxLib/Rtmr.c|  80 +
>  OvmfPkg/Library/TdxLib/TdReport.c| 102 +++
>  OvmfPkg/Library/TdxLib/TdxLib.inf|  48 ++
>  OvmfPkg/Library/TdxLib/TdxLibSec.inf 

Re: [edk2-devel] [RFC][patch] Add a new library class RegisterFilterLib in edk2 to filter/trace port IO/MMIO/MSR access

2021-03-09 Thread Laszlo Ersek
On 03/08/21 22:55, Kinney, Michael D wrote:
> 
> 
>> -Original Message-
>> From: Laszlo Ersek 
>> Sent: Monday, March 8, 2021 7:38 AM
>> To: devel@edk2.groups.io; Bi, Dandan 
>> Cc: Kinney, Michael D ; Liming Gao 
>> ; Liu, Zhiguang
>> 
>> Subject: Re: [edk2-devel] [RFC][patch] Add a new library class 
>> RegisterFilterLib in edk2 to filter/trace port IO/MMIO/MSR
>> access
>>
>> On 03/08/21 06:15, Dandan Bi wrote:
>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3246
>>>
>>> 1.Purpose:
>>>   Skip port IO/MMIO/MSR access in some emulatoion env.
>>>   Trace port IO/MMIO/MSR access.
>>>
>>> 2.Plan to do in Edk2:
>>>   Filter and trace in low level APIs in BaseIoLibIntrinsic and BaseLib.
>>>   Add a new library class (RegisterFilterLib) for the filter and trace 
>>> functionality.
>>>
>>> 3.Plan to filter and trace scope in Edk2 :
>>>   a. Port IO R/W: IA32 X64 (Only filter/trace for IA32 X64)
>>>   b. MMIO R/W: IA32 X64 EBC ARM AARCH64 RISCV64 (Filter/trace for the 
>>> Arches supported in BaseIoLibIntrinsic.inf)
>>>   c. MSR R/W: IA32 X64 (Only filter/trace for IA32 X64, if other ARCH has 
>>> similar use case can add new APIs per needs)
>>>
>>> 4.RegisterFilterLib Library Class:
>>>   a. Add RegisterFilterLib library class for the filter and trace operation.
>>>   b. Add RegisterFilterLib.h in MdePkg/Include/Library.
>>>   c. 12 APIs will be added to filter and trace port IO, MMIO and MSR access.
>>>   d. Add a NULL instance RegisterFilterLibNull in MdePkg/Library.(Verified 
>>> that null instance will not impact binary
>> size.)
>>>   e. Platform can implement its own RegisterFilterLib instance.
>>>
>>>   12 APIs can be divided into 2 categories:
>>>   6 [Before] APIs use to check whether need to execute port IO/MMIO/MSR 
>>> access or do some tracing before access.
>>>   6 [After] APIs use to trace after port IO/MMIO/MSR access.
>>>   The detailed API definitions are included in this patch.
>>>
>>>   For port IO access:
>>>   FilterBeforeIoRead
>>>   FilterAfterIoRead
>>>   FilterBeforeIoWrite
>>>   FilterAfterIoWrite
>>>
>>>   For MMIO access:
>>>   FilterBeforeMmIoRead
>>>   FilterAfterMmIoRead
>>>   FilterBeforeMmIoWrite
>>>   FilterAfterMmIoWrite
>>>
>>>   For MSR access:
>>>   FilterBeforeMsrRead
>>>   FilterAfterMsrRead
>>>   FilterBeforeMsrWrite
>>>   FilterAfterMsrWrite
>>>
>>> 5.Change and Impact
>>>   a. Add the RegisterFilterLib libary class and RegisterFilterLibNull 
>>> instance firstly.
>>>   b. Update the dsc in edk2 and edk2-platform repo to consume the 
>>> RegisterFilterLibNull instance.
>>>   c. Update the BaseLib and IoLib to consume RegisterFilterLib.
>>>
>>>   This is an incompatible change.
>>>   No code change in BaseLib and IoLib consumers, only need to change dsc to 
>>> consume new FilterLib instance.
>>>   Update BaseIoLibIntrinsic.inf and BaseIoLibIntrinsicSev.inf to consume 
>>> RegisterFilterLib for all supported Arch
>>>   Update BaseLib.inf to consume RegisterFilterLib only for IA32 and X64
>>
>> Seems like a sound plan, but there are more IoLib instances than the above:
>>
>> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf
>> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> 
> I agree that all 3 of these need to be included in the plan.
> 
>> MdePkg/Library/DxeIoLibCpuIo2/DxeIoLibCpuIo2.inf
>> MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf
>> MdePkg/Library/SmmIoLibSmmCpuIo2/SmmIoLibSmmCpuIo2.inf
> 
> The IoLib instances all perform their I/O operation by calling
> a dynamic PPI/Protocol services.  I would recommend that we do not update
> these instances, and instead only apply the RegisterFilterLib to 
> IoLib instances that perform he direct access to the hardware.
> Any IoLib instances that access the hardware through a PPI/Protocol
> should not be updated.
> 
> We have a few implementations of the CPI I/O PPI/Protocol that
> use the BaseIoLibIntrinsics, so those would actually be covered
> by the first set of lib instances.  If a platform decides to 
> implement a new version of the CPU I/O PPI/Protocol that does not
> use one of the BaseIoLibInstrinsic instances, then they would
> have the option of using the RegisterFilterLib in that new
> implementation of the CPI I/O PPI/Protocol.

Thanks for the explanation; I agree.

Laszlo



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




Re: [edk2-devel] [PATCH v2] UefiCpuPkg/PiSmmCpu: Don't allocate Token for SmmStartupThisAp

2021-03-09 Thread Ni, Ray
I don't want to break the community rule but somehow "git send-email" cannot 
send out the V3 patch out.
To avoid reviewers spending time reviewing v2 patch, I have to send out the v3 
as attachment ASAP.
Meanwhile, I will investigate what happened to my system and avoid sending 
attachments in future.

Thanks,
Ray
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Ni, Ray
> Sent: Tuesday, March 9, 2021 4:58 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Laszlo Ersek ;
> Kumar, Rahul1 
> Subject: [edk2-devel] [PATCH v2] UefiCpuPkg/PiSmmCpu: Don't allocate
> Token for SmmStartupThisAp
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3199
> 
> When Token points to mSmmStartupThisApToken, this routine is called
> from SmmStartupThisAp() in non-blocking mode due to
> PcdCpuSmmBlockStartupThisAp == FALSE.
> 
> In this case, caller wants to startup AP procedure in non-blocking
> mode and cannot get the completion status from the Token because there
> is no way to return the Token to caller from SmmStartupThisAp().
> Caller needs to use its specific way to query the completion status.
> 
> There is no need to allocate a token for such case so the 3 overheads
> can be avoided:
> 1. Call AllocateTokenBuffer() when there is no free token.
> 2. Get a free token from the token buffer.
> 3. Call ReleaseToken() in APHandler().
> 
> Signed-off-by: Ray Ni 
> Cc: Eric Dong 
> Reviewed-by: Laszlo Ersek 
> Cc: Rahul Kumar 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 27
> ---
>  1 file changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index 6227b2428a..91452ec398 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -1,7 +1,7 @@
>  /** @file
> 
>  SMM MP service implementation
> 
> 
> 
> -Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.
> 
> +Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.
> 
>  Copyright (c) 2017, AMD Incorporated. All rights reserved.
> 
> 
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -22,6 +22,7 @@ UINTN   mSemaphoreSize;
>  SPIN_LOCK   *mPFLock = NULL;
> 
>  SMM_CPU_SYNC_MODE   mCpuSmmSyncMode;
> 
>  BOOLEAN mMachineCheckSupported = FALSE;
> 
> +MM_COMPLETION   mSmmStartupThisApToken;
> 
> 
> 
>  extern UINTN mSmmShadowStackSize;
> 
> 
> 
> @@ -1240,9 +1241,23 @@ InternalSmmStartupThisAp (
>mSmmMpSyncData->CpuData[CpuIndex].Procedure = Procedure;
> 
>mSmmMpSyncData->CpuData[CpuIndex].Parameter = ProcArguments;
> 
>if (Token != NULL) {
> 
> -ProcToken= GetFreeToken (1);
> 
> -mSmmMpSyncData->CpuData[CpuIndex].Token = ProcToken;
> 
> -*Token = (MM_COMPLETION)ProcToken->SpinLock;
> 
> +if (Token != ) {
> 
> +  //
> 
> +  // When Token points to mSmmStartupThisApToken, this routine is
> called
> 
> +  // from SmmStartupThisAp() in non-blocking mode
> (PcdCpuSmmBlockStartupThisAp == FALSE).
> 
> +  //
> 
> +  // In this case, caller wants to startup AP procedure in non-blocking
> 
> +  // mode and cannot get the completion status from the Token because
> there
> 
> +  // is no way to return the Token to caller from SmmStartupThisAp().
> 
> +  // Caller needs to use its implementation specific way to query the
> completion status.
> 
> +  //
> 
> +  // There is no need to allocate a token for such case so the overhead 
> of
> SMRAM and
> 
> +  // the allocation operation can be avoided.
> 
> +  //
> 
> +  ProcToken= GetFreeToken (1);
> 
> +  mSmmMpSyncData->CpuData[CpuIndex].Token = ProcToken;
> 
> +  *Token = (MM_COMPLETION)ProcToken->SpinLock;
> 
> +}
> 
>}
> 
>mSmmMpSyncData->CpuData[CpuIndex].Status= CpuStatus;
> 
>if (mSmmMpSyncData->CpuData[CpuIndex].Status != NULL) {
> 
> @@ -1474,8 +1489,6 @@ SmmStartupThisAp (
>IN OUT  VOID  *ProcArguments OPTIONAL
> 
>)
> 
>  {
> 
> -  MM_COMPLETION   Token;
> 
> -
> 
>gSmmCpuPrivate->ApWrapperFunc[CpuIndex].Procedure = Procedure;
> 
>gSmmCpuPrivate->ApWrapperFunc[CpuIndex].ProcedureArgument =
> ProcArguments;
> 
> 
> 
> @@ -1486,7 +1499,7 @@ SmmStartupThisAp (
>  ProcedureWrapper,
> 
>  CpuIndex,
> 
>  >ApWrapperFunc[CpuIndex],
> 
> -FeaturePcdGet (PcdCpuSmmBlockStartupThisAp) ? NULL : ,
> 
> +FeaturePcdGet (PcdCpuSmmBlockStartupThisAp) ? NULL :
> ,
> 
>  0,
> 
>  NULL
> 
>  );
> 
> --
> 2.27.0.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#72579): https://edk2.groups.io/g/devel/message/72579
> Mute This Topic: https://groups.io/mt/81197093/1712937
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: 

[edk2-devel] [PATCH v3] UefiCpuPkg/PiSmmCpu: Don't allocate Token for SmmStartupThisAp

2021-03-09 Thread Ni, Ray
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3199

When Token points to mSmmStartupThisApToken, this routine is called
from SmmStartupThisAp() in non-blocking mode due to
PcdCpuSmmBlockStartupThisAp == FALSE.

In this case, caller wants to startup AP procedure in non-blocking
mode and cannot get the completion status from the Token because there
is no way to return the Token to caller from SmmStartupThisAp().
Caller needs to use its specific way to query the completion status.

There is no need to allocate a token for such case so the 3 overheads
can be avoided:
1. Call AllocateTokenBuffer() when there is no free token.
2. Get a free token from the token buffer.
3. Call ReleaseToken() in APHandler().

Signed-off-by: Ray Ni 
Cc: Eric Dong 
Reviewed-by: Laszlo Ersek 
Cc: Rahul Kumar 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 30 ---
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 6227b2428a..4f3c5f60a1 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -1,7 +1,7 @@
 /** @file
 SMM MP service implementation
 
-Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.
 Copyright (c) 2017, AMD Incorporated. All rights reserved.
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -22,6 +22,7 @@ UINTN   mSemaphoreSize;
 SPIN_LOCK   *mPFLock = NULL;
 SMM_CPU_SYNC_MODE   mCpuSmmSyncMode;
 BOOLEAN mMachineCheckSupported = FALSE;
+MM_COMPLETION   mSmmStartupThisApToken;
 
 extern UINTN mSmmShadowStackSize;
 
@@ -1240,9 +1241,26 @@ InternalSmmStartupThisAp (
   mSmmMpSyncData->CpuData[CpuIndex].Procedure = Procedure;
   mSmmMpSyncData->CpuData[CpuIndex].Parameter = ProcArguments;
   if (Token != NULL) {
-ProcToken= GetFreeToken (1);
-mSmmMpSyncData->CpuData[CpuIndex].Token = ProcToken;
-*Token = (MM_COMPLETION)ProcToken->SpinLock;
+if (Token != ) {
+  //
+  // When Token points to mSmmStartupThisApToken, this routine is called
+  // from SmmStartupThisAp() in non-blocking mode 
(PcdCpuSmmBlockStartupThisAp == FALSE).
+  //
+  // In this case, caller wants to startup AP procedure in non-blocking
+  // mode and cannot get the completion status from the Token because there
+  // is no way to return the Token to caller from SmmStartupThisAp().
+  // Caller needs to use its implementation specific way to query the 
completion status.
+  //
+  // There is no need to allocate a token for such case so the 3 overheads
+  // can be avoided:
+  // 1. Call AllocateTokenBuffer() when there is no free token.
+  // 2. Get a free token from the token buffer.
+  // 3. Call ReleaseToken() in APHandler().
+  //
+  ProcToken= GetFreeToken (1);
+  mSmmMpSyncData->CpuData[CpuIndex].Token = ProcToken;
+  *Token = (MM_COMPLETION)ProcToken->SpinLock;
+}
   }
   mSmmMpSyncData->CpuData[CpuIndex].Status= CpuStatus;
   if (mSmmMpSyncData->CpuData[CpuIndex].Status != NULL) {
@@ -1474,8 +1492,6 @@ SmmStartupThisAp (
   IN OUT  VOID  *ProcArguments OPTIONAL
   )
 {
-  MM_COMPLETION   Token;
-
   gSmmCpuPrivate->ApWrapperFunc[CpuIndex].Procedure = Procedure;
   gSmmCpuPrivate->ApWrapperFunc[CpuIndex].ProcedureArgument = ProcArguments;
 
@@ -1486,7 +1502,7 @@ SmmStartupThisAp (
 ProcedureWrapper,
 CpuIndex,
 >ApWrapperFunc[CpuIndex],
-FeaturePcdGet (PcdCpuSmmBlockStartupThisAp) ? NULL : ,
+FeaturePcdGet (PcdCpuSmmBlockStartupThisAp) ? NULL : 
,
 0,
 NULL
 );
-- 
2.27.0.windows.1



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




Re: [edk2-devel] [PATCH - resend] MdeModulePkg/Universal/SmbiosDxe: Scan for existing tables

2021-03-09 Thread Ni, Ray
Patrick,
Can you please send out a new patch which modifies SmbiosDxe to consume ...?
1. A single gEfiSmbios3TableGuid HOB which contains the whole SMBIOS table 
(starting with SMBIOS_TABLE_3_0_ENTRY_POINT), or
2. A single gEfiSmbiosTableGuid HOB which contains the whole SMBIOS table 
(starting with SMBIOS_TABLE_ENTRY_POINT).

The code change that consumes multiple gEdkiiSmbiosStructureGuid HOBs which 
contains an individual SMBIOS structure (starting with SMBIOS_STRUCTURE) can be 
done later.

Thanks,
Ray

> -Original Message-
> From: Ni, Ray
> Sent: Thursday, March 4, 2021 9:03 AM
> To: Dong, Guo ; Patrick Rudolph 
> 
> Cc: devel@edk2.groups.io; Chaganty, Rangasai V 
> ; Bi, Dandan ; Zeng,
> Star ; Gao, Zhichao ; You, 
> Benjamin ;
> philipp.deppenwi...@9elements.com; Ma, Maurice 
> Subject: RE: [edk2-devel] [PATCH - resend] MdeModulePkg/Universal/SmbiosDxe: 
> Scan for existing tables
> 
> To be specific, the reasons I like to use multiple HOBs each containing a 
> SMBIOS structure are:
> 1. A well modularized bootloader may have one module producing type 4 
> structure, another module producing type 19 structure.
> 2. Try to think about what the optimal design could be regarding the 
> universal payload spec
> (https://universalpayload.github.io/documentation/spec/spec.html). (The spec 
> is not widely accepted and just an RFC.)
> 
> There are two style of consumer code:
> A. SmbiosDxe consumes a Guid HOB which contains a full SMBIOS table.
> B. SmbiosDxe consumes multiple Guid HOBs each contains a SMBIOS structure.
> 
> There are two options of implementations:
> 1. Support style A for coreboot and extend to style B for more bootloaders.
> 2. Support style B only. PayloadEntry breaks the coreboot SMBIOS table to 
> multiple Guid HOBs each contains a SMBIOS structure.
> 
> Either option works for me though I will be more comfortable if choosing 2. 
> 
> Thanks,
> Ray
> 
> > -Original Message-
> > From: Dong, Guo 
> > Sent: Thursday, March 4, 2021 1:54 AM
> > To: Ni, Ray ; Patrick Rudolph 
> > 
> > Cc: devel@edk2.groups.io; Chaganty, Rangasai V 
> > ; Bi, Dandan ; Zeng,
> > Star ; Gao, Zhichao ; You, 
> > Benjamin ;
> > philipp.deppenwi...@9elements.com; Ma, Maurice 
> > Subject: RE: [edk2-devel] [PATCH - resend] 
> > MdeModulePkg/Universal/SmbiosDxe: Scan for existing tables
> >
> >
> > hi Ray,
> >
> > Just saw the discussion on this patch.
> > Both coreboot and SBL would build the whole SMBIOS table and report it to 
> > payloads.
> >
> > For UEFI payload, I think it is not necessary to let other driver 
> > (BlSupportDxe) to split the whole SMBIOS table into records.
> > I would prefer SMBIOS DXE diver could support the whole SMBIOS table from 
> > PEI/bootloader.
> > But it is also possible to support individual records if required by 
> > checking AnchorString to know if it is whole table.
> >
> > Thanks,
> > Guo
> >
> > > -Original Message-
> > > From: Ni, Ray 
> > > Sent: Wednesday, March 3, 2021 3:04 AM
> > > To: Patrick Rudolph 
> > > Cc: devel@edk2.groups.io; Chaganty, Rangasai V
> > > ; Bi, Dandan ; Zeng,
> > > Star ; Gao, Zhichao ; You,
> > > Benjamin ;
> > > philipp.deppenwi...@9elements.com; Ma, Maurice
> > > ; Dong, Guo 
> > > Subject: RE: [edk2-devel] [PATCH - resend]
> > > MdeModulePkg/Universal/SmbiosDxe: Scan for existing tables
> > >
> > > >
> > > > Hi Ray,
> > > > thanks for your feedback.
> > > >
> > > > Currently a single HOB containing all the SMBIOS table is exported by
> > > > coreboot.
> > > > As coreboot doesn't support multiple HOBs with the same ID, #2 isn't a
> > > > solution.
> > >
> > > Hi Patrick,
> > > I checked the code in deep.
> > > The HOB is not created by coreboot. It's the PayloadEntry that creates the
> > > HOB.
> > > Can we update PayloadEntry to create multiple HOBs?
> > >
> > > Guo,
> > > Any comments?
> > >
> > > The reason I like this approach is it doesn't require the other 
> > > bootloaders to
> > > write
> > > a SMBIOS driver that merges all SMBIOS structures together into one table.
> > >
> > > Thanks,
> > > Ray


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




[edk2-devel] [PATCH v3] UefiCpuPkg/PiSmmCpu: Don't allocate Token for SmmStartupThisAp

2021-03-09 Thread Ni, Ray
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3199

When Token points to mSmmStartupThisApToken, this routine is called
from SmmStartupThisAp() in non-blocking mode due to
PcdCpuSmmBlockStartupThisAp == FALSE.

In this case, caller wants to startup AP procedure in non-blocking
mode and cannot get the completion status from the Token because there
is no way to return the Token to caller from SmmStartupThisAp().
Caller needs to use its specific way to query the completion status.

There is no need to allocate a token for such case so the 3 overheads
can be avoided:
1. Call AllocateTokenBuffer() when there is no free token.
2. Get a free token from the token buffer.
3. Call ReleaseToken() in APHandler().

Signed-off-by: Ray Ni 
Cc: Eric Dong 
Reviewed-by: Laszlo Ersek 
Cc: Rahul Kumar 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 30 ---
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 6227b2428a..4f3c5f60a1 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -1,7 +1,7 @@
 /** @file
 SMM MP service implementation
 
-Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.
 Copyright (c) 2017, AMD Incorporated. All rights reserved.
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -22,6 +22,7 @@ UINTN   mSemaphoreSize;
 SPIN_LOCK   *mPFLock = NULL;
 SMM_CPU_SYNC_MODE   mCpuSmmSyncMode;
 BOOLEAN mMachineCheckSupported = FALSE;
+MM_COMPLETION   mSmmStartupThisApToken;
 
 extern UINTN mSmmShadowStackSize;
 
@@ -1240,9 +1241,26 @@ InternalSmmStartupThisAp (
   mSmmMpSyncData->CpuData[CpuIndex].Procedure = Procedure;
   mSmmMpSyncData->CpuData[CpuIndex].Parameter = ProcArguments;
   if (Token != NULL) {
-ProcToken= GetFreeToken (1);
-mSmmMpSyncData->CpuData[CpuIndex].Token = ProcToken;
-*Token = (MM_COMPLETION)ProcToken->SpinLock;
+if (Token != ) {
+  //
+  // When Token points to mSmmStartupThisApToken, this routine is called
+  // from SmmStartupThisAp() in non-blocking mode 
(PcdCpuSmmBlockStartupThisAp == FALSE).
+  //
+  // In this case, caller wants to startup AP procedure in non-blocking
+  // mode and cannot get the completion status from the Token because there
+  // is no way to return the Token to caller from SmmStartupThisAp().
+  // Caller needs to use its implementation specific way to query the 
completion status.
+  //
+  // There is no need to allocate a token for such case so the 3 overheads
+  // can be avoided:
+  // 1. Call AllocateTokenBuffer() when there is no free token.
+  // 2. Get a free token from the token buffer.
+  // 3. Call ReleaseToken() in APHandler().
+  //
+  ProcToken= GetFreeToken (1);
+  mSmmMpSyncData->CpuData[CpuIndex].Token = ProcToken;
+  *Token = (MM_COMPLETION)ProcToken->SpinLock;
+}
   }
   mSmmMpSyncData->CpuData[CpuIndex].Status= CpuStatus;
   if (mSmmMpSyncData->CpuData[CpuIndex].Status != NULL) {
@@ -1474,8 +1492,6 @@ SmmStartupThisAp (
   IN OUT  VOID  *ProcArguments OPTIONAL
   )
 {
-  MM_COMPLETION   Token;
-
   gSmmCpuPrivate->ApWrapperFunc[CpuIndex].Procedure = Procedure;
   gSmmCpuPrivate->ApWrapperFunc[CpuIndex].ProcedureArgument = ProcArguments;
 
@@ -1486,7 +1502,7 @@ SmmStartupThisAp (
 ProcedureWrapper,
 CpuIndex,
 >ApWrapperFunc[CpuIndex],
-FeaturePcdGet (PcdCpuSmmBlockStartupThisAp) ? NULL : ,
+FeaturePcdGet (PcdCpuSmmBlockStartupThisAp) ? NULL : 
,
 0,
 NULL
 );
-- 
2.27.0.windows.1



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




[edk2-devel] [PATCH v2] UefiCpuPkg/PiSmmCpu: Don't allocate Token for SmmStartupThisAp

2021-03-09 Thread Ni, Ray
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3199

When Token points to mSmmStartupThisApToken, this routine is called
from SmmStartupThisAp() in non-blocking mode due to
PcdCpuSmmBlockStartupThisAp == FALSE.

In this case, caller wants to startup AP procedure in non-blocking
mode and cannot get the completion status from the Token because there
is no way to return the Token to caller from SmmStartupThisAp().
Caller needs to use its specific way to query the completion status.

There is no need to allocate a token for such case so the 3 overheads
can be avoided:
1. Call AllocateTokenBuffer() when there is no free token.
2. Get a free token from the token buffer.
3. Call ReleaseToken() in APHandler().

Signed-off-by: Ray Ni 
Cc: Eric Dong 
Reviewed-by: Laszlo Ersek 
Cc: Rahul Kumar 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 27 ---
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 6227b2428a..91452ec398 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -1,7 +1,7 @@
 /** @file
 SMM MP service implementation
 
-Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.
 Copyright (c) 2017, AMD Incorporated. All rights reserved.
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -22,6 +22,7 @@ UINTN   mSemaphoreSize;
 SPIN_LOCK   *mPFLock = NULL;
 SMM_CPU_SYNC_MODE   mCpuSmmSyncMode;
 BOOLEAN mMachineCheckSupported = FALSE;
+MM_COMPLETION   mSmmStartupThisApToken;
 
 extern UINTN mSmmShadowStackSize;
 
@@ -1240,9 +1241,23 @@ InternalSmmStartupThisAp (
   mSmmMpSyncData->CpuData[CpuIndex].Procedure = Procedure;
   mSmmMpSyncData->CpuData[CpuIndex].Parameter = ProcArguments;
   if (Token != NULL) {
-ProcToken= GetFreeToken (1);
-mSmmMpSyncData->CpuData[CpuIndex].Token = ProcToken;
-*Token = (MM_COMPLETION)ProcToken->SpinLock;
+if (Token != ) {
+  //
+  // When Token points to mSmmStartupThisApToken, this routine is called
+  // from SmmStartupThisAp() in non-blocking mode 
(PcdCpuSmmBlockStartupThisAp == FALSE).
+  //
+  // In this case, caller wants to startup AP procedure in non-blocking
+  // mode and cannot get the completion status from the Token because there
+  // is no way to return the Token to caller from SmmStartupThisAp().
+  // Caller needs to use its implementation specific way to query the 
completion status.
+  //
+  // There is no need to allocate a token for such case so the overhead of 
SMRAM and
+  // the allocation operation can be avoided.
+  //
+  ProcToken= GetFreeToken (1);
+  mSmmMpSyncData->CpuData[CpuIndex].Token = ProcToken;
+  *Token = (MM_COMPLETION)ProcToken->SpinLock;
+}
   }
   mSmmMpSyncData->CpuData[CpuIndex].Status= CpuStatus;
   if (mSmmMpSyncData->CpuData[CpuIndex].Status != NULL) {
@@ -1474,8 +1489,6 @@ SmmStartupThisAp (
   IN OUT  VOID  *ProcArguments OPTIONAL
   )
 {
-  MM_COMPLETION   Token;
-
   gSmmCpuPrivate->ApWrapperFunc[CpuIndex].Procedure = Procedure;
   gSmmCpuPrivate->ApWrapperFunc[CpuIndex].ProcedureArgument = ProcArguments;
 
@@ -1486,7 +1499,7 @@ SmmStartupThisAp (
 ProcedureWrapper,
 CpuIndex,
 >ApWrapperFunc[CpuIndex],
-FeaturePcdGet (PcdCpuSmmBlockStartupThisAp) ? NULL : ,
+FeaturePcdGet (PcdCpuSmmBlockStartupThisAp) ? NULL : 
,
 0,
 NULL
 );
-- 
2.27.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72579): https://edk2.groups.io/g/devel/message/72579
Mute This Topic: https://groups.io/mt/81197093/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 2/3] OvmfPkg: Add PCDs for TdxLib

2021-03-09 Thread Min Xu
Hi, Jiewen
See comments inline.

> -Original Message-
> From: Yao, Jiewen 
> Sent: Tuesday, March 9, 2021 2:44 PM
> To: Xu, Min M ; devel@edk2.groups.io
> Cc: Justen, Jordan L ; Laszlo Ersek
> ; Reiland, Doug 
> Subject: RE: [PATCH V3 2/3] OvmfPkg: Add PCDs for TdxLib
> 
> Hi
> May I understand why we need this : PcdUseTdxAcceptPage ?
> I think accepting page is always required.
This PCD is for test purpose in early development. It doesn't make sense
any more.  It will be removed in next version.


> 
> For PcdUseTdxEmulation, This is only for pre-production.
> For real production, I don't think we need this one, right?
This PCD is for test in SDV environment which emulate the Tdcall by calling
vmcall. It will be removed in next version.

 
> 
> > -Original Message-
> > From: Xu, Min M 
> > Sent: Tuesday, March 9, 2021 2:13 PM
> > To: devel@edk2.groups.io
> > Cc: Xu, Min M ; Justen, Jordan L
> > ; Laszlo Ersek ; Yao,
> > Jiewen ; Reiland, Doug 
> > Subject: [PATCH V3 2/3] OvmfPkg: Add PCDs for TdxLib
> >
> > TdxLib for OvmfPkg depends on the below PCDs
> >   - PcdUseTdxAcceptPage
> > Indicate whether TdCall(AcceptPage) is used.
> >   - PcdUseTdxEmulation
> > Indicate whether TdxEmulation is used.
> >
> > Cc: Jordan Justen 
> > Cc: Laszlo Ersek 
> > Cc: Jiewen Yao 
> >
> > Signed-off-by: Min Xu 
> > Signed-off-by: Doug Reiland 
> > ---
> >  OvmfPkg/OvmfPkg.dec | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec index
> > 4348bb45c64a..68b3fd86d516 100644
> > --- a/OvmfPkg/OvmfPkg.dec
> > +++ b/OvmfPkg/OvmfPkg.dec
> > @@ -317,6 +317,12 @@
> >
> gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase|0x0|UINT32|0x42
> >
> gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize|0x0|UINT32|0x43
> >
> > +  ## Indicate whether TdCall(AcceptPage) is used.
> > +
> gUefiOvmfPkgTokenSpaceGuid.PcdUseTdxAcceptPage|TRUE|BOOLEAN|0x44
> > +  ## Indicate whether TdxEmulation is used.
> > +  gUefiOvmfPkgTokenSpaceGuid.PcdUseTdxEmulation|0x1|UINT32|0x45
> > +
> > +
> >  [PcdsDynamic, PcdsDynamicEx]
> >gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
> >
> >
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLE
> AN
> > |0x10
> > --
> > 2.29.2.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72578): https://edk2.groups.io/g/devel/message/72578
Mute This Topic: https://groups.io/mt/81195557/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 1/3] MdePkg: Add Tdx support lib

2021-03-09 Thread Min Xu
Hi, Jiewen
See comments inline.

> -Original Message-
> From: Yao, Jiewen 
> Sent: Tuesday, March 9, 2021 2:25 PM
> To: Xu, Min M ; devel@edk2.groups.io
> Cc: Liming Gao ; Liu, Zhiguang
> ; Reiland, Doug 
> Subject: RE: [PATCH V3 1/3] MdePkg: Add Tdx support lib
> 
> Hi Min
> Some recommendation:
> 
> 1) Please separate 1 big patch to multiple smaller ones.
> 1 patch for TdxLib
> 1 patch for Tdx protocol.
> 1 patch for TDX event log ACPI table.
> 1 patch for TDX Library.
> 
The big patch will be separated to smaller ones in next version.

> 2) The ACPI definition from TDX protocol should be isolated to TdxAcpi.h
> 
> #define EFI_TDX_EVENT_DATA_SIGNATURE  SIGNATURE_32 ('T', 'D', 'E', 'L')
OK. TdxAcpi.h will be added in next version.
 
> 3) There is no description for TD protocol itself and TD event data ACPI 
> table.
> Please add them.
> 
> You may copy some content from the specification.
Description will be added in next version.

> 4) I think we are following TDX spec to provide TdxLib.
> I don't see the need to provide NULL version in MdePkg. We can put real
> TdxLib to MdePkg.
Agree. Will update in next version.

> 5) If possible, please provide TDX spec link in the file header comment
> session.
> As such, the reviewer can check the spec easily.
The TDX spec link will be added.
 
> 
> > -Original Message-
> > From: Xu, Min M 
> > Sent: Tuesday, March 9, 2021 2:13 PM
> > To: devel@edk2.groups.io
> > Cc: Xu, Min M ; Liming Gao
> > ; Liu, Zhiguang ;
> > Yao, Jiewen ; Reiland, Doug
> > 
> > Subject: [PATCH V3 1/3] MdePkg: Add Tdx support lib
> >
> > Intel Trust Domain Extension (Intel TDX) refers to an Intel technology
> > that extends Virtual Machines Extensions (VMX) and Multi-Key Total
> > Memory Encryption (MKTME) with a new kind of virtual machine guest
> > called a Trust Domain (TD).
> >
> > TdxLib is created with functions to perform the related Tdx operation.
> > This includes functions for:
> >   - TdCall : to cause a VM exit to the Intel TDX module
> >   - TdVmCall   : it is a leaf function 0 for TDCALL
> >   - TdVmCallCpuid  : enable the TD guest to request VMM to emulate CPUID
> >   - TdReport   : to retrieve TDREPORT_STRUCT
> >   - TdAcceptPages  : to accept pending private pages
> >   - TdExtendRtmr   : to extend one of the RTMR registers
> >
> > The base function in this dirver will not do anything and will return
> > an error if a return value is required. It is expected that other
> > packages (like OvmfPkg) will create a version of the library to fully
> > support a TD guest.
> >
> > Cc: Liming Gao 
> > Cc: Zhiguang Liu 
> > Cc: Jiewen Yao 
> >
> > Signed-off-by: Min Xu 
> > Signed-off-by: Doug Reiland 
> > ---
> >  MdePkg/Include/IndustryStandard/Tdx.h | 201
> ++
> >  MdePkg/Include/Library/TdxLib.h   | 165 +
> >  MdePkg/Include/Protocol/Tdx.h |  29 
> >  MdePkg/Library/TdxLib/TdxLibNull.c| 155 
> >  MdePkg/Library/TdxLib/TdxLibNull.inf  |  33 +
> >  5 files changed, 583 insertions(+)
> >  create mode 100644 MdePkg/Include/IndustryStandard/Tdx.h
> >  create mode 100644 MdePkg/Include/Library/TdxLib.h  create mode
> > 100644 MdePkg/Include/Protocol/Tdx.h  create mode 100644
> > MdePkg/Library/TdxLib/TdxLibNull.c
> >  create mode 100644 MdePkg/Library/TdxLib/TdxLibNull.inf
> >
> > diff --git a/MdePkg/Include/IndustryStandard/Tdx.h
> > b/MdePkg/Include/IndustryStandard/Tdx.h
> > new file mode 100644
> > index ..dbcc31c26528
> > --- /dev/null
> > +++ b/MdePkg/Include/IndustryStandard/Tdx.h
> > @@ -0,0 +1,201 @@
> > +/** @file
> > +  Intel Trust Domain Extension definitions
> > +
> > +  Copyright (c) 2020 - 2021, Intel Corporation. All rights
> > + reserved.  This program and the accompanying materials  are
> > + licensed and made available under the terms and conditions of the
> > + BSD
> > License
> > +  which accompanies this distribution.  The full text of the license
> > + may be found
> > at
> > +  http://opensource.org/licenses/bsd-license.php
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> > + BASIS,  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER
> > EXPRESS OR IMPLIED.
> > +
> > +**/
> > +
> > +#ifndef _TDX_H_
> > +#define _TDX_H_
> > +
> > +#define EXIT_REASON_EXTERNAL_INTERRUPT  1
> > +#define EXIT_REASON_TRIPLE_FAULT2
> > +
> > +#define EXIT_REASON_PENDING_INTERRUPT   7
> > +#define EXIT_REASON_NMI_WINDOW  8
> > +#define EXIT_REASON_TASK_SWITCH 9
> > +#define EXIT_REASON_CPUID   10
> > +#define EXIT_REASON_HLT 12
> > +#define EXIT_REASON_INVD13
> > +#define EXIT_REASON_INVLPG  14
> > +#define EXIT_REASON_RDPMC   15
> > +#define EXIT_REASON_RDTSC   16
> > +#define EXIT_REASON_VMCALL  18
> > +#define EXIT_REASON_VMCLEAR 19
> > +#define EXIT_REASON_VMLAUNCH20
> >