[edk2] [Patch] BaseTools: Fixed Pcd from command line issue.

[BobCF]

Save the pcd command line value in Pcd object

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng 
Cc: Liming Gao 
---
 BaseTools/Source/Python/AutoGen/GenC.py| 15 +++
 BaseTools/Source/Python/AutoGen/GenMake.py | 28 +++
 .../Source/Python/Workspace/BuildClassObject.py|  4 +--
 BaseTools/Source/Python/Workspace/DscBuildData.py  | 31 +++---
 4 files changed, 31 insertions(+), 47 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenC.py 
b/BaseTools/Source/Python/AutoGen/GenC.py
index 3e98506cc8..481c4dda14 100644
--- a/BaseTools/Source/Python/AutoGen/GenC.py
+++ b/BaseTools/Source/Python/AutoGen/GenC.py
@@ -914,15 +914,12 @@ def CreateModulePcdCode(Info, AutoGenC, AutoGenH, Pcd):
 PcdTokenName = '_PCD_TOKEN_' + TokenCName
 PatchPcdSizeTokenName = '_PCD_PATCHABLE_' + TokenCName +'_SIZE'
 PatchPcdSizeVariableName = '_gPcd_BinaryPatch_Size_' + TokenCName
 FixPcdSizeTokenName = '_PCD_SIZE_' + TokenCName
 
-if GlobalData.BuildOptionPcd:
-for PcdItem in GlobalData.BuildOptionPcd:
-if (Pcd.TokenSpaceGuidCName, TokenCName) == (PcdItem[0], 
PcdItem[1]):
-Pcd.DefaultValue = PcdItem[2]
-break
+if Pcd.PcdValueFromComm:
+Pcd.DefaultValue = Pcd.PcdValueFromComm
 
 if Pcd.Type in gDynamicExPcd:
 TokenNumber = int(Pcd.TokenValue, 0)
 # Add TokenSpaceGuidValue value to PcdTokenName to discriminate the 
DynamicEx PCDs with 
 # different Guids but same TokenCName
@@ -1213,16 +1210,12 @@ def CreateLibraryPcdCode(Info, AutoGenC, AutoGenH, Pcd):
 PcdTokenName = '_PCD_TOKEN_' + TokenCName
 FixPcdSizeTokenName = '_PCD_SIZE_' + TokenCName
 PatchPcdSizeTokenName = '_PCD_PATCHABLE_' + TokenCName +'_SIZE'
 PatchPcdSizeVariableName = '_gPcd_BinaryPatch_Size_' + TokenCName
 
-if GlobalData.BuildOptionPcd:
-for PcdItem in GlobalData.BuildOptionPcd:
-if (Pcd.TokenSpaceGuidCName, TokenCName) == (PcdItem[0], 
PcdItem[1]):
-Pcd.DefaultValue = PcdItem[2]
-break
-
+if Pcd.PcdValueFromComm:
+Pcd.DefaultValue = Pcd.PcdValueFromComm
 #
 # Write PCDs
 #
 if Pcd.Type in gDynamicExPcd:
 TokenNumber = int(Pcd.TokenValue, 0)
diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 1b0cf17e25..60bd625cd2 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -1549,29 +1549,19 @@ class TopLevelMakefile(BuildFile):
 if GlobalData.gEnableGenfdsMultiThread:
 ExtraOption += " --genfds-multi-thread"
 if GlobalData.gIgnoreSource:
 ExtraOption += " --ignore-sources"
 
-for index, option in enumerate(GlobalData.gCommand):
-if "--pcd" == option and GlobalData.gCommand[index+1]:
-pcdName, pcdValue = GlobalData.gCommand[index+1].split('=')
-for Item in GlobalData.BuildOptionPcd:
-if '.'.join(Item[0:2]) == pcdName:
-pcdValue = Item[2]
-if pcdValue.startswith('L') or 
pcdValue.startswith('"'):
-pcdValue, Size = ParseFieldValue(pcdValue)
-NewVal = '{'
-for S in range(Size):
-NewVal = NewVal + '0x%02X' % ((pcdValue >> S * 
8) & 0xff)
-NewVal += ','
-pcdValue =  NewVal[:-1] + '}'
-break
-if pcdValue.startswith('{'):
-pcdValue = 'H' + '"' + pcdValue + '"'
-ExtraOption += " --pcd " + pcdName + '=' + pcdValue
-else:
-ExtraOption += " --pcd " + GlobalData.gCommand[index+1]
+for pcd in GlobalData.BuildOptionPcd:
+if pcd[2]:
+pcdname = '.'.join(pcd[0:3])
+else:
+pcdname = '.'.join(pcd[0:2])
+if pcd[3].startswith('{'):
+ExtraOption += " --pcd " + pcdname + '=' + 'H' + '"' + pcd[3] 
+ '"'
+else:
+ExtraOption += " --pcd " + pcdname + '=' + pcd[3]
 
 MakefileName = self._FILE_NAME_[self._FileType]
 SubBuildCommandList = []
 for A in PlatformInfo.ArchList:
 Command = self._MAKE_TEMPLATE_[self._FileType] % 
{"file":os.path.join("$(BUILD_DIR)", A, MakefileName)}
diff --git a/BaseTools/Source/Python/Workspace/BuildClassObject.py 
b/BaseTools/Source/Python/Workspace/BuildClassObject.py
index 711ba492ef..1352fa21c8 100644
--- a/BaseTools/Source/Python/Workspace/BuildClassObject.py
+++ b/BaseTools/Source/Python/Workspace/BuildClassObject.py
@@ -66,10 +66,11 @@ class PcdClassObject(object):
 self.expressions = 

[edk2] [Patch] BaseTools: Fixed Pcd from command line issue.

[BobCF]

Save the pcd command line value in Pcd object

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng 
Cc: Liming Gao 
---
 BaseTools/Source/Python/AutoGen/GenC.py| 15 +++
 BaseTools/Source/Python/AutoGen/GenMake.py | 25 +
 .../Source/Python/Workspace/BuildClassObject.py|  4 +--
 BaseTools/Source/Python/Workspace/DscBuildData.py  | 31 +++---
 4 files changed, 28 insertions(+), 47 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenC.py 
b/BaseTools/Source/Python/AutoGen/GenC.py
index 3e98506cc8..481c4dda14 100644
--- a/BaseTools/Source/Python/AutoGen/GenC.py
+++ b/BaseTools/Source/Python/AutoGen/GenC.py
@@ -914,15 +914,12 @@ def CreateModulePcdCode(Info, AutoGenC, AutoGenH, Pcd):
 PcdTokenName = '_PCD_TOKEN_' + TokenCName
 PatchPcdSizeTokenName = '_PCD_PATCHABLE_' + TokenCName +'_SIZE'
 PatchPcdSizeVariableName = '_gPcd_BinaryPatch_Size_' + TokenCName
 FixPcdSizeTokenName = '_PCD_SIZE_' + TokenCName
 
-if GlobalData.BuildOptionPcd:
-for PcdItem in GlobalData.BuildOptionPcd:
-if (Pcd.TokenSpaceGuidCName, TokenCName) == (PcdItem[0], 
PcdItem[1]):
-Pcd.DefaultValue = PcdItem[2]
-break
+if Pcd.PcdValueFromComm:
+Pcd.DefaultValue = Pcd.PcdValueFromComm
 
 if Pcd.Type in gDynamicExPcd:
 TokenNumber = int(Pcd.TokenValue, 0)
 # Add TokenSpaceGuidValue value to PcdTokenName to discriminate the 
DynamicEx PCDs with 
 # different Guids but same TokenCName
@@ -1213,16 +1210,12 @@ def CreateLibraryPcdCode(Info, AutoGenC, AutoGenH, Pcd):
 PcdTokenName = '_PCD_TOKEN_' + TokenCName
 FixPcdSizeTokenName = '_PCD_SIZE_' + TokenCName
 PatchPcdSizeTokenName = '_PCD_PATCHABLE_' + TokenCName +'_SIZE'
 PatchPcdSizeVariableName = '_gPcd_BinaryPatch_Size_' + TokenCName
 
-if GlobalData.BuildOptionPcd:
-for PcdItem in GlobalData.BuildOptionPcd:
-if (Pcd.TokenSpaceGuidCName, TokenCName) == (PcdItem[0], 
PcdItem[1]):
-Pcd.DefaultValue = PcdItem[2]
-break
-
+if Pcd.PcdValueFromComm:
+Pcd.DefaultValue = Pcd.PcdValueFromComm
 #
 # Write PCDs
 #
 if Pcd.Type in gDynamicExPcd:
 TokenNumber = int(Pcd.TokenValue, 0)
diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 1b0cf17e25..0a3cc32155 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -1549,29 +1549,16 @@ class TopLevelMakefile(BuildFile):
 if GlobalData.gEnableGenfdsMultiThread:
 ExtraOption += " --genfds-multi-thread"
 if GlobalData.gIgnoreSource:
 ExtraOption += " --ignore-sources"
 
-for index, option in enumerate(GlobalData.gCommand):
-if "--pcd" == option and GlobalData.gCommand[index+1]:
-pcdName, pcdValue = GlobalData.gCommand[index+1].split('=')
-for Item in GlobalData.BuildOptionPcd:
-if '.'.join(Item[0:2]) == pcdName:
-pcdValue = Item[2]
-if pcdValue.startswith('L') or 
pcdValue.startswith('"'):
-pcdValue, Size = ParseFieldValue(pcdValue)
-NewVal = '{'
-for S in range(Size):
-NewVal = NewVal + '0x%02X' % ((pcdValue >> S * 
8) & 0xff)
-NewVal += ','
-pcdValue =  NewVal[:-1] + '}'
-break
-if pcdValue.startswith('{'):
-pcdValue = 'H' + '"' + pcdValue + '"'
-ExtraOption += " --pcd " + pcdName + '=' + pcdValue
-else:
-ExtraOption += " --pcd " + GlobalData.gCommand[index+1]
+for pcd in GlobalData.BuildOptionPcd:
+if pcd[2]:
+pcdname = '.'.join(pcd[0:3])
+else:
+pcdname = '.'.join(pcd[0:2])
+ExtraOption += " --pcd " + pcdname + '=' + pcd[3]
 
 MakefileName = self._FILE_NAME_[self._FileType]
 SubBuildCommandList = []
 for A in PlatformInfo.ArchList:
 Command = self._MAKE_TEMPLATE_[self._FileType] % 
{"file":os.path.join("$(BUILD_DIR)", A, MakefileName)}
diff --git a/BaseTools/Source/Python/Workspace/BuildClassObject.py 
b/BaseTools/Source/Python/Workspace/BuildClassObject.py
index 711ba492ef..1352fa21c8 100644
--- a/BaseTools/Source/Python/Workspace/BuildClassObject.py
+++ b/BaseTools/Source/Python/Workspace/BuildClassObject.py
@@ -66,10 +66,11 @@ class PcdClassObject(object):
 self.expressions = expressions
 self.DscDefaultValue = None
 self.DscRawValue = None
 if IsDsc:
 self.DscDefaultValue = Value
+

[edk2] [Patch] BaseTools: Update --pcd parser to support flexible pcd format

[Yonghong Zhu]

This patch update --pcd parser to support flexible pcd format.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/Common/Expression.py   |  21 +-
 BaseTools/Source/Python/Common/Misc.py |  41 ++-
 BaseTools/Source/Python/GenFds/FdfParser.py|   1 +
 BaseTools/Source/Python/GenFds/FfsInfStatement.py  |  18 +-
 BaseTools/Source/Python/GenFds/GenFds.py   |  49 
 BaseTools/Source/Python/Workspace/DscBuildData.py  | 285 -
 .../Source/Python/Workspace/MetaFileParser.py  |   1 +
 BaseTools/Source/Python/build/BuildReport.py   |   4 +-
 8 files changed, 152 insertions(+), 268 deletions(-)

diff --git a/BaseTools/Source/Python/Common/Expression.py 
b/BaseTools/Source/Python/Common/Expression.py
index 5a0ade9..79dc83e 100644
--- a/BaseTools/Source/Python/Common/Expression.py
+++ b/BaseTools/Source/Python/Common/Expression.py
@@ -13,11 +13,11 @@
 ## Import Modules
 #
 from Common.GlobalData import *
 from CommonDataClass.Exceptions import BadExpression
 from CommonDataClass.Exceptions import WrnExpression
-from Misc import GuidStringToGuidStructureString, ParseFieldValue
+from Misc import GuidStringToGuidStructureString, ParseFieldValue, 
IsFieldValueAnArray
 import Common.EdkLogger as EdkLogger
 import copy
 
 ERR_STRING_EXPR = 'This operator cannot be used in string expression: 
[%s].'
 ERR_SNYTAX  = 'Syntax error, the rest of expression cannot be 
evaluated: [%s].'
@@ -123,10 +123,29 @@ def IsValidCString(Str):
 ValidString = re.compile(r'[_a-zA-Z][_0-9a-zA-Z]*$')
 if not ValidString.match(Str):
 return False
 return True
 
+def BuildOptionValue(PcdValue, GuidDict):
+IsArray = False
+if PcdValue.startswith('H'):
+InputValue = PcdValue[1:]
+elif PcdValue.startswith("L'") or PcdValue.startswith("'"):
+InputValue = PcdValue
+elif PcdValue.startswith('L'):
+InputValue = 'L"' + PcdValue[1:] + '"'
+else:
+InputValue = PcdValue
+if IsFieldValueAnArray(InputValue):
+IsArray = True
+if IsArray:
+try:
+PcdValue = ValueExpressionEx(InputValue, 'VOID*', GuidDict)(True)
+except:
+pass
+return PcdValue
+
 ## ReplaceExprMacro
 #
 def ReplaceExprMacro(String, Macros, ExceptionList = None):
 StrList = SplitString(String)
 for i, String in enumerate(StrList):
diff --git a/BaseTools/Source/Python/Common/Misc.py 
b/BaseTools/Source/Python/Common/Misc.py
index af374d8..2086b4c 100644
--- a/BaseTools/Source/Python/Common/Misc.py
+++ b/BaseTools/Source/Python/Common/Misc.py
@@ -1439,10 +1439,26 @@ def ParseConsoleLog(Filename):
 Opw.write('%s\n' % Line)
 
 Opr.close()
 Opw.close()
 
+def IsFieldValueAnArray (Value):
+Value = Value.strip()
+if Value.startswith('GUID') and Value.endswith(')'):
+return True
+if Value.startswith('L"') and Value.endswith('"')  and 
len(list(Value[2:-1])) > 1:
+return True
+if Value[0] == '"' and Value[-1] == '"' and len(list(Value[1:-1])) > 1:
+return True
+if Value[0] == '{' and Value[-1] == '}':
+return True
+if Value.startswith("L'") and Value.endswith("'") and 
len(list(Value[2:-1])) > 1:
+return True
+if Value[0] == "'" and Value[-1] == "'" and len(list(Value[1:-1])) > 1:
+return True
+return False
+
 def AnalyzePcdExpression(Setting):
 Setting = Setting.strip()
 # There might be escaped quote in a string: \", \\\" , \', \\\'
 Data = Setting
 # There might be '|' in string and in ( ... | ... ), replace it with '-'
@@ -2375,35 +2391,10 @@ def PackRegistryFormatGuid(Guid):
 int(Guid[4][-6:-4], 16),
 int(Guid[4][-4:-2], 16),
 int(Guid[4][-2:], 16)
 )
 
-def BuildOptionPcdValueFormat(TokenSpaceGuidCName, TokenCName, PcdDatumType, 
Value):
-if PcdDatumType not in [TAB_UINT8, TAB_UINT16, TAB_UINT32, 
TAB_UINT64,'BOOLEAN']:
-if Value.startswith('L') or Value.startswith('"'):
-if not Value[1]:
-EdkLogger.error("build", FORMAT_INVALID, 'For Void* type PCD, 
when specify the Value in the command line, please use the following format: 
"string", L"string", H"{...}"')
-Value = Value
-elif Value.startswith('H'):
-if not Value[1]:
-EdkLogger.error("build", FORMAT_INVALID, 'For Void* type PCD, 
when specify the Value in the command line, please use the following format: 
"string", L"string", H"{...}"')
-Value = Value[1:]
-else:
-if not Value[0]:
-EdkLogger.error("build", FORMAT_INVALID, 'For Void* type PCD, 
when specify the Value in the command line, please use the following format: 
"string", L"string", H"{...}"')
-Value = '"' + Value + '"'
-
-IsValid, Cause = 

Re: [edk2] [PATCH 1/1] MdeModulePkg/UefiBootManagerLib: limit recursive call depth

[Ni, Ruiyu]

On 3/7/2018 9:54 AM, Guo Heyi wrote:

Hi Ray,

Sorry to disturb, but I didn't find the patch committed. Could you help to do
that?

Thanks,
Heyi


On Thu, Mar 01, 2018 at 12:46:32PM +0800, Ni, Ruiyu wrote:

On 3/1/2018 10:39 AM, Heyi Guo wrote:

Function BmRepairAllControllers may recursively call itself if some
driver health protocol returns EfiDriverHealthStatusReconnectRequired.
However, driver health protocol of some buggy third party driver may
always return such status even after one and another reconnect. The
endless iteration will cause stack overflow and then system exception,
and it may be not easy to find that the exception is actually caused
by stack overflow.

So we limit the number of reconnect retry to 10 to improve code
robustness, and DEBUG_CODE is moved ahead before recursive repair to
track the repair result.

We also remove a duplicated declaration of BmRepairAllControllers() in
InternalBm.h in this patch, for it is only a trivial change.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
---

Notes:
 v2
 - Use argument instead of global variable to record the recursive
   count [Ray]
 - Move DEBUG_CODE before recursively calling BmRepairAllControllers()
   to track the change of each reconnect [Ray]
 - Remove a duplicated declaration of BmRepairAllControllers() in
   InternalBm.h.

  MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h | 19 
++-
  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c |  2 +-
  MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c | 18 
+-
  3 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h 
b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
index 25a1d522fe84..21ecd8584d24 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
+++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
@@ -108,6 +108,12 @@ CHAR16 *
  #define BM_OPTION_NAME_LEN  sizeof 
("PlatformRecovery")
  extern CHAR16  *mBmLoadOptionName[];
+//
+// Maximum number of reconnect retry to repair controller; it is to limit the
+// number of recursive call of BmRepairAllControllers.
+//
+#define MAX_RECONNECT_REPAIR10
+
  /**
Visitor function to be called by BmForEachVariable for each variable
in variable storage.
@@ -145,10 +151,13 @@ typedef struct {
  /**
Repair all the controllers according to the Driver Health status queried.
+
+  @param ReconnectRepairCount To record the number of recursive call of
+  this function itself.
  **/
  VOID
  BmRepairAllControllers (
-  VOID
+  UINTN   ReconnectRepairCount
);
  #define BM_HOTKEY_SIGNATURE SIGNATURE_32 ('b', 'm', 'h', 'k')
@@ -328,14 +337,6 @@ BmDelPartMatchInstance (
);
  /**
-  Repair all the controllers according to the Driver Health status queried.
-**/
-VOID
-BmRepairAllControllers (
-  VOID
-  );
-
-/**
Print the device path info.
@param DevicePath   The device path need to print.
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index ce19ae400660..b842d5824aed 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1767,7 +1767,7 @@ EfiBootManagerBoot (
  //
  // 4. Repair system through DriverHealth protocol
  //
-BmRepairAllControllers ();
+BmRepairAllControllers (0);
}
PERF_START_EX (gImageHandle, "BdsAttempt", NULL, 0, (UINT32) OptionNumber);
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
index ddcee8b0676f..db2f859ae73d 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
@@ -423,10 +423,13 @@ EfiBootManagerFreeDriverHealthInfo (
  /**
Repair all the controllers according to the Driver Health status queried.
+
+  @param ReconnectRepairCount To record the number of recursive call of
+  this function itself.
  **/
  VOID
  BmRepairAllControllers (
-  VOID
+  UINTN   ReconnectRepairCount
)
  {
EFI_STATUS  Status;
@@ -548,10 +551,6 @@ BmRepairAllControllers (
EfiBootManagerFreeDriverHealthInfo (DriverHealthInfo, Count);
-  if (ReconnectRequired) {
-BmRepairAllControllers ();
-  }
-
DEBUG_CODE (
  CHAR16 *ControllerName;
@@ -576,6 +575,15 @@ BmRepairAllControllers (
  EfiBootManagerFreeDriverHealthInfo (DriverHealthInfo, Count);
  );
+  if (ReconnectRequired) {
+if (ReconnectRepairCount < MAX_RECONNECT_REPAIR) {
+  BmRepairAllControllers 

Re: [edk2] [Patch] BaseTools: Fix a bug for --pcd used in ConditionalStatement calculate

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Yonghong Zhu
> Sent: Wednesday, March 7, 2018 4:34 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch] BaseTools: Fix a bug for --pcd used in 
> ConditionalStatement calculate
> 
> Move the GlobalData.BuildOptionPcd before FdfParser() function
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Source/Python/GenFds/GenFds.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Source/Python/GenFds/GenFds.py 
> b/BaseTools/Source/Python/GenFds/GenFds.py
> index 4c56cbb..e3cc77f 100644
> --- a/BaseTools/Source/Python/GenFds/GenFds.py
> +++ b/BaseTools/Source/Python/GenFds/GenFds.py
> @@ -268,10 +268,12 @@ def main():
> 
>  if not os.path.exists(OutputDir):
>  EdkLogger.error("GenFds", FILE_NOT_FOUND, 
> ExtraData=OutputDir)
>  GenFdsGlobalVariable.OutputDirDict[Key] = OutputDir
> 
> +GlobalData.BuildOptionPcd = Options.OptionPcd if 
> Options.OptionPcd else {}
> +
>  """ Parse Fdf file, has to place after build Workspace as FDF may 
> contain macros from DSC file """
>  FdfParserObj = FdfParser.FdfParser(FdfFilename)
>  FdfParserObj.ParseFile()
> 
>  if FdfParserObj.CycleReferenceCheck():
> @@ -324,11 +326,10 @@ def main():
>  EdkLogger.error("GenFds", 
> FORMAT_INVALID, "The FV %s's region is specified in
> multiple FD with different value." %FvObj.UiFvName)
>  else:
>  FvObj.FvRegionInFD = RegionObj.Size
>  
> RegionObj.BlockInfoOfRegion(FdObj.BlockSizeList, FvObj)
> 
> -GlobalData.BuildOptionPcd = Options.OptionPcd if 
> Options.OptionPcd else {}
>  """Call GenFds"""
>  GenFds.GenFd('', FdfParserObj, BuildWorkSpace, ArchList)
> 
>  """Generate GUID cross reference file"""
>  GenFds.GenerateGuidXRefFile(BuildWorkSpace, ArchList, FdfParserObj)
> --
> 2.6.1.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] RFC: Proposal to halt automatic builds of Windows BaseTools executables

[Gao, Liming]

Hi, all

  Here is wiki 
https://github.com/tianocore/tianocore.github.io/wiki/Windows-systems#compile-tools
 on Compile BaseTools in windows. 

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Bjorge, Erik C
> Sent: Thursday, March 8, 2018 1:57 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] RFC: Proposal to halt automatic builds of Windows BaseTools 
> executables
> 
> I would like to propose that the automatic builds of Windows BaseTools 
> executables be halted.  This implies there will no longer be
> updates to the edk2-BaseTools-win32 repository.
> 
> With this change, developers using Windows must install Python 2.7.x and 
> configure their environment to build C tools  and run
> python scripts from sources.  This matches the development experience for 
> non-Windows environments.
> 
> Please respond with comments by 03/23/2018.
> 
> Thanks,
> -Erik
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/3] Hisilicon/D0x: Set ACPI GTDT always-on flag

[Guo Heyi]

Yes it is. Sorry for missing to do that. Will keep in mind next time :)

Thanks,
Heyi


On Wed, Mar 07, 2018 at 10:36:48AM +0100, Laszlo Ersek wrote:
> Hello Heyi,
> 
> On 03/07/18 07:55, Heyi Guo wrote:
> > From: Jason Zhang 
> > 
> > Timer is always working on Hisilicon D0x, even system enters WFI/WFE,
> > and there is no other low power status, so we set "always-on" flag in
> > ACPI GTDT.
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jason Zhang 
> > Signed-off-by: Heyi Guo 
> > Cc: Ard Biesheuvel 
> > Cc: Leif Lindholm 
> > Cc: Graeme Gregory 
> > ---
> >  Silicon/Hisilicon/Hi1610/Hi1610AcpiTables/Gtdt.aslc | 3 ++-
> >  Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc| 2 +-
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> this is for edk2-platforms, isn't it?
> 
> If so, next time please add "edk2-platforms" to the subject prefix, as in:
> 
>   [PATCH edk2-platforms]
> 
> or
> 
>   [edk2-platforms PATCH]
> 
> Thanks!
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] HELP: Qemu not able to load Duet image

[david moheban]

Hi,

I am having difficulty loading Duet image via Qemu x86-64 or Bochs in Windows 
10-64. Command line I type ‘Qemu -fda floppy.img’ and does not get past 
‘Welcome to Efi World’ but if I burn the image to a flash usb drive it will 
boot up just fine. Are there any settings or drivers that need to be included 
in the compilation process? 

Thank you


Sent from my iPad
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch v2] EmbeddedPkg: Correct the way of handling sections with a large size

[Ge Song]

Correct the way of handling EFI_SECTION_GUID_DEFINED type sections
with a large size

Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ge Song 
Reviewed-by: Ard Biesheuvel 
---
 
EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.c 
| 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git 
a/EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.c
 
b/EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.c
index 7b08de8ab9fe..e94f5424ef1d 100644
--- 
a/EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.c
+++ 
b/EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.c
@@ -123,6 +123,7 @@ ExtractGuidedSectionGetInfo (
 {
   PRE_PI_EXTRACT_GUIDED_SECTION_DATA  *SavedData;
   UINT32  Index;
+  EFI_GUID*SectionDefinitionGuid;
 
   if (InputSection == NULL) {
 return RETURN_INVALID_PARAMETER;
@@ -134,11 +135,17 @@ ExtractGuidedSectionGetInfo (
 
   SavedData = GetSavedData();
 
+  if (IS_SECTION2 (InputSection)) {
+SectionDefinitionGuid = &(((EFI_GUID_DEFINED_SECTION2 *) 
InputSection)->SectionDefinitionGuid);
+  } else {
+SectionDefinitionGuid = &(((EFI_GUID_DEFINED_SECTION *) 
InputSection)->SectionDefinitionGuid);
+  }
+
   //
   // Search the match registered GetInfo handler for the input guided section.
   //
   for (Index = 0; Index < SavedData->NumberOfExtractHandler; Index ++) {
-if (CompareGuid (>ExtractHandlerGuidTable[Index], 
&(((EFI_GUID_DEFINED_SECTION *) InputSection)->SectionDefinitionGuid))) {
+if (CompareGuid (>ExtractHandlerGuidTable[Index], 
SectionDefinitionGuid)) {
   break;
 }
   }
@@ -172,6 +179,7 @@ ExtractGuidedSectionDecode (
 {
   PRE_PI_EXTRACT_GUIDED_SECTION_DATA  *SavedData;
   UINT32  Index;
+  EFI_GUID*SectionDefinitionGuid;
 
   if (InputSection == NULL) {
 return RETURN_INVALID_PARAMETER;
@@ -182,11 +190,17 @@ ExtractGuidedSectionDecode (
 
   SavedData = GetSavedData();
 
+  if (IS_SECTION2 (InputSection)) {
+SectionDefinitionGuid = &(((EFI_GUID_DEFINED_SECTION2 *) 
InputSection)->SectionDefinitionGuid);
+  } else {
+SectionDefinitionGuid = &(((EFI_GUID_DEFINED_SECTION *) 
InputSection)->SectionDefinitionGuid);
+  }
+
   //
   // Search the match registered GetInfo handler for the input guided section.
   //
   for (Index = 0; Index < SavedData->NumberOfExtractHandler; Index ++) {
-if (CompareGuid (>ExtractHandlerGuidTable[Index], 
&(((EFI_GUID_DEFINED_SECTION *) InputSection)->SectionDefinitionGuid))) {
+if (CompareGuid (>ExtractHandlerGuidTable[Index], 
SectionDefinitionGuid)) {
   break;
 }
   }
-- 
2.11.0


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 1/8] SecurityPkg: also clear HashInterfaceHob.SupportedHashMask

[Zeng, Star]

Reviewed-by: Star Zeng 

Thanks,
Star
-Original Message-
From: Zhang, Chao B 
Sent: Thursday, March 8, 2018 8:35 AM
To: marcandre.lur...@redhat.com; edk2-devel@lists.01.org
Cc: pjo...@redhat.com; Yao, Jiewen ; 
stef...@linux.vnet.ibm.com; ler...@redhat.com; qemu-de...@nongnu.org; 
javi...@redhat.com; Zeng, Star 
Subject: RE: [PATCH v2 1/8] SecurityPkg: also clear 
HashInterfaceHob.SupportedHashMask

Reviewed-by: Chao Zhang

-Original Message-
From: marcandre.lur...@redhat.com [mailto:marcandre.lur...@redhat.com] 
Sent: Wednesday, March 7, 2018 11:58 PM
To: edk2-devel@lists.01.org
Cc: pjo...@redhat.com; Yao, Jiewen ; 
stef...@linux.vnet.ibm.com; ler...@redhat.com; qemu-de...@nongnu.org; 
javi...@redhat.com; Marc-André Lureau ; Zhang, 
Chao B ; Zeng, Star 
Subject: [PATCH v2 1/8] SecurityPkg: also clear 
HashInterfaceHob.SupportedHashMask

From: Marc-André Lureau 

Commit 4cc2b63bd829426b05bad0d8952f1855a10d6ed7 fixed an out of bounds
ZeroMem() call. However, as Laszlo Ersek pointed out, the intent was to clear 
all but the Identifier (to revert the effect of RegisterHashInterfaceLib()). 
For that, it should clear the SupportedHashMask too.

Cc: Jiewen Yao 
Cc: Chao Zhang 
Cc: Star Zeng 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marc-André Lureau 
---
 .../Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c | 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c 
b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
index 361a4f6508a0..bf6e1336ee76 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRoute
+++ rPei.c
@@ -426,6 +426,7 @@ HashLibBaseCryptoRouterPeiConstructor (
 //
 ZeroMem (>HashInterface, sizeof 
(HashInterfaceHob->HashInterface));
 HashInterfaceHob->HashInterfaceCount = 0;
+HashInterfaceHob->SupportedHashMask = 0;
   }
 
   //
--
2.16.2.346.g9779355e34

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v1 1/1] BaseTools: GlobalData remove unused variable

[Jaben Carsey]

gWideStringPattern is not used.

Cc: Yonghong Zhu 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/Common/GlobalData.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/Common/GlobalData.py 
b/BaseTools/Source/Python/Common/GlobalData.py
index f7d4d577f994..8b36a1b77366 100644
--- a/BaseTools/Source/Python/Common/GlobalData.py
+++ b/BaseTools/Source/Python/Common/GlobalData.py
@@ -47,8 +47,7 @@ gDefaultStores = []
 gMacroRefPattern = re.compile("\$\(([A-Z][_A-Z0-9]*)\)", re.UNICODE)
 gMacroDefPattern = re.compile("^(DEFINE|EDK_GLOBAL)[ \t]+")
 gMacroNamePattern = re.compile("^[A-Z][A-Z0-9_]*$")
-# C-style wide string pattern
-gWideStringPattern = re.compile('(\W|\A)L"')
+
 #
 # A global variable for whether current build in AutoGen phase or not.
 #
-- 
2.14.1.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v1 0/1] BaseTools: remove unused variable

[Jaben Carsey]

delete a variable never uised and the comment

Cc: Yonghong Zhu 
Cc: Liming Gao 

Jaben Carsey (1):
  BaseTools: GlobalData remove unused variable

 BaseTools/Source/Python/Common/GlobalData.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

-- 
2.14.1.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 2/8] SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from Depex

[Zhang, Chao B]

Hi Lureau:
   I think we can remove same dependency in TcgPei. 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
marcandre.lur...@redhat.com
Sent: Wednesday, March 7, 2018 11:58 PM
To: edk2-devel@lists.01.org
Cc: qemu-de...@nongnu.org; javi...@redhat.com; pjo...@redhat.com; Yao, Jiewen 
; ler...@redhat.com
Subject: [edk2] [PATCH v2 2/8] SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable 
from Depex

From: Marc-André Lureau 

The module doesn't use read-only variable.

Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marc-André Lureau 
---
 SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 1 -
 1 file changed, 1 deletion(-)

diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf 
b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
index bc910c3baf97..a4aae1488ff8 100644
--- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
+++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
@@ -91,7 +91,6 @@ [Pcd]
 
 [Depex]
   gEfiPeiMasterBootModePpiGuid AND
-  gEfiPeiReadOnlyVariable2PpiGuid AND
   gEfiTpmDeviceSelectedGuid
 
 [UserExtensions.TianoCore."ExtraFiles"]
-- 
2.16.2.346.g9779355e34

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 1/8] SecurityPkg: also clear HashInterfaceHob.SupportedHashMask

[Zhang, Chao B]

Reviewed-by: Chao Zhang

-Original Message-
From: marcandre.lur...@redhat.com [mailto:marcandre.lur...@redhat.com] 
Sent: Wednesday, March 7, 2018 11:58 PM
To: edk2-devel@lists.01.org
Cc: pjo...@redhat.com; Yao, Jiewen ; 
stef...@linux.vnet.ibm.com; ler...@redhat.com; qemu-de...@nongnu.org; 
javi...@redhat.com; Marc-André Lureau ; Zhang, 
Chao B ; Zeng, Star 
Subject: [PATCH v2 1/8] SecurityPkg: also clear 
HashInterfaceHob.SupportedHashMask

From: Marc-André Lureau 

Commit 4cc2b63bd829426b05bad0d8952f1855a10d6ed7 fixed an out of bounds
ZeroMem() call. However, as Laszlo Ersek pointed out, the intent was to clear 
all but the Identifier (to revert the effect of RegisterHashInterfaceLib()). 
For that, it should clear the SupportedHashMask too.

Cc: Jiewen Yao 
Cc: Chao Zhang 
Cc: Star Zeng 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marc-André Lureau 
---
 .../Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c | 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c 
b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
index 361a4f6508a0..bf6e1336ee76 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRoute
+++ rPei.c
@@ -426,6 +426,7 @@ HashLibBaseCryptoRouterPeiConstructor (
 //
 ZeroMem (>HashInterface, sizeof 
(HashInterfaceHob->HashInterface));
 HashInterfaceHob->HashInterfaceCount = 0;
+HashInterfaceHob->SupportedHashMask = 0;
   }
 
   //
--
2.16.2.346.g9779355e34

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] BaseTools/header.makefile: revert gcc-8 "-Wno-xxx" options on OSX

[Laszlo Ersek]

On 03/07/18 21:42, Ard Biesheuvel wrote:
> On 7 March 2018 at 20:28, Laszlo Ersek  wrote:
>> On 03/07/18 12:51, Gao, Liming wrote:
>>> Reviewed-by: Liming Gao 
>>
>> Thank you, Liming.
>>
>> Ard, are you OK with this patch as well?
>>
> 
> Sure
> 
> Acked-by: Ard Biesheuvel 

Thank you both; commit 777f4aa083e9.

Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 1/1] SecurityPkg: fix ZeroMem HashInterfaceHob

[Laszlo Ersek]

On 03/07/18 16:35, Zhang, Chao B wrote:
> Star:
>Why do we need to add HashInterfaceHob->SupportedHashMask = 0?  
> HashInterfaceHob is internally maintained and accessed by HashLibRouterPei.
> There is no impact to leave the value after module has been re-shadowed.

There seems to be no functional requirement for clearing the
SupportedHashMask field, except the original (buggy) ZeroMem() call
looked like it *intended* to clear the field. So now that we have fixed
the buffer overflow, we should decide whether we want to stick with the
original intent (that would mean continuing to clear the field, one way
or another), or to depart from the original intent -- but that would
merit a comment in the code (or, it would have deserved a comment in the
commit message).

In short, it's not great to do two independent things in the same patch,
namely (a) fix the buffer overflow, (b) silently diverge from the
original intent, even if that's functionally justified.

Such changes should be split to two patches, or else explained in detail.

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/1] RFC: SecurityPkg: only clear HashInterface informations

[Laszlo Ersek]

On 03/07/18 10:42, Zeng, Star wrote:
> Hi Laszlo,
> 
> Good analysis.
> 
> Yes, the SupportedHashMask field in HashInterfaceHob will have stale value, 
> but that does not impact the functionality since the code have.
> So the patch could fix the problem.
> 
> HashLibBaseCryptoRouterPeiConstructor():
>   Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, 0);
> 
> RegisterHashInterfaceLib():
>   HashInterfaceHob->SupportedHashMask = PcdGet32 (PcdTcg2HashAlgorithmBitmap) 
> | HashMask;
> 
> HashStart()/HashUpdate()/HashCompleteAndExtend()/HashAndExtend():
>   if (HashInterfaceHob->HashInterfaceCount == 0) {
> return EFI_UNSUPPORTED;
>   }
> 
> RegisterHashInterfaceLib():
>   if (HashInterfaceHob->HashInterfaceCount >= HASH_COUNT) {
> return EFI_OUT_OF_RESOURCES;
>   }

Ugh, this is a bit too complex for me to digest right now, but I'll
trust you if you say the value doesn't matter :)

> As I know, Chao has helped push the patch.

Yes, that's correct, it's commit 4cc2b63bd829 ("SecurityPkg: only clear
HashInterface information", 2018-03-07).

> But I am fine with keeping this patch or continue refining the code. :)

I would slightly prefer clearing the SupportedHashMask field as well (or
adding a comment explaining why it's not necessary), because then the
uninitiated reader, like me, wouldn't have to ask themselves why the
field isn't being cleared :)

But, given your response, I don't feel strongly about it any longer;
I'll leave it up to Marc-André.

(Also, if we have to choose between extending the ZeroMem() and writing
the comment, I think the former is easier :) I could write that too, but
I couldn't write the comment.)

> Is it working with " ZeroMem (HashInterfaceHob, sizeof (*HashInterfaceHob) - 
> sizeof (EFI_GUID)) " if to refine the code?

I'll skip this question based on your later followup.

Thanks!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] BaseTools/header.makefile: revert gcc-8 "-Wno-xxx" options on OSX

[Ard Biesheuvel]

On 7 March 2018 at 20:28, Laszlo Ersek  wrote:
> On 03/07/18 12:51, Gao, Liming wrote:
>> Reviewed-by: Liming Gao 
>
> Thank you, Liming.
>
> Ard, are you OK with this patch as well?
>

Sure

Acked-by: Ard Biesheuvel 

>>> -Original Message-
>>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>>> Sent: Wednesday, March 7, 2018 5:33 PM
>>> To: edk2-devel-01 
>>> Cc: Gao, Liming ; Zhu, Yonghong 
>>> 
>>> Subject: [PATCH] BaseTools/header.makefile: revert gcc-8 "-Wno-xxx" options 
>>> on OSX
>>>
>>> I recently added the gcc-8 specific "-Wno-stringop-truncation" and
>>> "-Wno-restrict" options to BUILD_CFLAGS, both for "Darwin" (XCODE5 /
>>> clang, OSX) and otherwise (gcc, Linux / Cygwin).
>>>
>>> I also regression-tested the change with gcc-4.8 on Linux -- gcc-4.8 does
>>> not know either of the (gcc-8 specific) "-Wno-stringop-truncation" and
>>> "-Wno-restrict" options, yet the build completed fine (by GCC design).
>>>
>>> Regarding OSX, my expectation was that
>>>
>>> - XCODE5 / clang would either recognize these warnings options (because
>>>   clang does recognize most -W options of gcc),
>>>
>>> - or, similarly to gcc, clang would simply ignore the "-Wno-xxx" flags
>>>   that it didn't recognize.
>>>
>>> Neither is the case; the new flags have broken the BaseTools build on OSX.
>>> Revert them (for OSX only).
>>>
>>> Cc: Liming Gao 
>>> Cc: Yonghong Zhu 
>>> Reported-by: Liming Gao 
>>> Fixes: 1d212a83df0eaf32a6f5d4159beb2d77832e0231
>>> Fixes: 9222154ae7b3eef75ae88cdb56158256227cb929
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Laszlo Ersek 
>>> ---
>>>  BaseTools/Source/C/Makefiles/header.makefile | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/BaseTools/Source/C/Makefiles/header.makefile 
>>> b/BaseTools/Source/C/Makefiles/header.makefile
>>> index 065a998bf5de..db436773cf40 100644
>>> --- a/BaseTools/Source/C/Makefiles/header.makefile
>>> +++ b/BaseTools/Source/C/Makefiles/header.makefile
>>> @@ -71,7 +71,7 @@ INCLUDE = $(TOOL_INCLUDE) -I $(MAKEROOT) -I 
>>> $(MAKEROOT)/Include/Common -I $(MAKE
>>>  BUILD_CPPFLAGS = $(INCLUDE) -O2
>>>  ifeq ($(DARWIN),Darwin)
>>>  # assume clang or clang compatible flags on OS X
>>> -BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror 
>>> -Wno-deprecated-declarations -Wno-stringop-truncation
>>> -Wno-restrict -Wno-self-assign -Wno-unused-result -nostdlib -c -g
>>> +BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror 
>>> -Wno-deprecated-declarations -Wno-self-assign
>>> -Wno-unused-result -nostdlib -c -g
>>>  else
>>>  BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror 
>>> -Wno-deprecated-declarations -Wno-stringop-truncation
>>> -Wno-restrict -Wno-unused-result -nostdlib -c -g
>>>  endif
>>> --
>>> 2.14.1.3.gb7cf6e02401b
>>
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] BaseTools/header.makefile: revert gcc-8 "-Wno-xxx" options on OSX

[Laszlo Ersek]

On 03/07/18 12:51, Gao, Liming wrote:
> Reviewed-by: Liming Gao 

Thank you, Liming.

Ard, are you OK with this patch as well?

Thanks
Laszlo


>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Wednesday, March 7, 2018 5:33 PM
>> To: edk2-devel-01 
>> Cc: Gao, Liming ; Zhu, Yonghong 
>> 
>> Subject: [PATCH] BaseTools/header.makefile: revert gcc-8 "-Wno-xxx" options 
>> on OSX
>>
>> I recently added the gcc-8 specific "-Wno-stringop-truncation" and
>> "-Wno-restrict" options to BUILD_CFLAGS, both for "Darwin" (XCODE5 /
>> clang, OSX) and otherwise (gcc, Linux / Cygwin).
>>
>> I also regression-tested the change with gcc-4.8 on Linux -- gcc-4.8 does
>> not know either of the (gcc-8 specific) "-Wno-stringop-truncation" and
>> "-Wno-restrict" options, yet the build completed fine (by GCC design).
>>
>> Regarding OSX, my expectation was that
>>
>> - XCODE5 / clang would either recognize these warnings options (because
>>   clang does recognize most -W options of gcc),
>>
>> - or, similarly to gcc, clang would simply ignore the "-Wno-xxx" flags
>>   that it didn't recognize.
>>
>> Neither is the case; the new flags have broken the BaseTools build on OSX.
>> Revert them (for OSX only).
>>
>> Cc: Liming Gao 
>> Cc: Yonghong Zhu 
>> Reported-by: Liming Gao 
>> Fixes: 1d212a83df0eaf32a6f5d4159beb2d77832e0231
>> Fixes: 9222154ae7b3eef75ae88cdb56158256227cb929
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek 
>> ---
>>  BaseTools/Source/C/Makefiles/header.makefile | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/BaseTools/Source/C/Makefiles/header.makefile 
>> b/BaseTools/Source/C/Makefiles/header.makefile
>> index 065a998bf5de..db436773cf40 100644
>> --- a/BaseTools/Source/C/Makefiles/header.makefile
>> +++ b/BaseTools/Source/C/Makefiles/header.makefile
>> @@ -71,7 +71,7 @@ INCLUDE = $(TOOL_INCLUDE) -I $(MAKEROOT) -I 
>> $(MAKEROOT)/Include/Common -I $(MAKE
>>  BUILD_CPPFLAGS = $(INCLUDE) -O2
>>  ifeq ($(DARWIN),Darwin)
>>  # assume clang or clang compatible flags on OS X
>> -BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror 
>> -Wno-deprecated-declarations -Wno-stringop-truncation
>> -Wno-restrict -Wno-self-assign -Wno-unused-result -nostdlib -c -g
>> +BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror 
>> -Wno-deprecated-declarations -Wno-self-assign
>> -Wno-unused-result -nostdlib -c -g
>>  else
>>  BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror 
>> -Wno-deprecated-declarations -Wno-stringop-truncation
>> -Wno-restrict -Wno-unused-result -nostdlib -c -g
>>  endif
>> --
>> 2.14.1.3.gb7cf6e02401b
> 

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/3] BaseTools: let the C-language build utils compile with gcc-8

[Laszlo Ersek]

On 03/07/18 10:12, Ard Biesheuvel wrote:
> On 7 March 2018 at 09:11, Laszlo Ersek  wrote:
>> On 03/07/18 07:43, Gao, Liming wrote:
>>> Laszlo:
>>>   We just find this change causes XCODE5 build failure, because XCODE 
>>> compiler doesn't know these two options. So, could you provide the patch to 
>>> remove the change in BUILD_CFLAGS for MAC?
>>
>> Yep, I was a bit concerned about that.
>>
>> Ultimately I assumed that
>> - clang would either recognize these warnings options (because it does
>>   recognize most -W options of gcc),
>> - or, similarly to gcc, clang would simply ignore -Wno-* flags that it
>>   does not recognize.
>>
>> Neither appears to be the case, then. I'll send a patch soon.
>>
> 
> What is the reason we don't pass -Wno-unknown-warning-option to XCODE ?

My personal reason is that I simply didn't think of
"-Wno-unknown-warning-option", for either XCODE or GCC. I just added the
two new -Wno-xxx options to the GCC command line, tested them with
gcc-4.8 (which knows neither of those options), and gcc-4.8 didn't
complain, without needing any further massaging from my side. I assumed
"ignore unknown -Wno-xxx options" was the default (vaguely recalling
something like this from an earlier patch of yours -- apparently
incorrectly?), and that the same would apply to XCODE too.

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/4] Virtio non-discoverable devices

[Laszlo Ersek]

On 03/07/18 09:03, Ard Biesheuvel wrote:
> (+ Laszlo)
> 
> Hello Daniil,
> 
> On 7 March 2018 at 01:36, Daniil Egranov  wrote:
>> This is an attempt to add MMIO Virtio devices into the
>> non-discoverable device registration procedure and allow
>> Virtio PCI drivers to recognize and program such devices
>> correctly.
> 
> Why? The purpose of the non-discoverable device layer is to make
> non-PCI controllers that can be driven by PCI class drivers appear as
> PCI devices. We have started using the base non-discoverable device
> protocol for other devices as well, but the PCI wrapper is really only
> intended for PCI class drivers.
> 
> For VirtIO MMIO, we already have a driver model driver, and given that
> you need to patch up differences between MMIO and PCI based virtio in
> your code, I am reluctant to incorporate modifications in to the PCI
> driver to support MMIO devices.

I'm impressed and thankful that you managed to say it in two paragraphs
-- I needed ten or more :)

Thanks,
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] RFC: Proposal to halt automatic builds of Windows BaseTools executables

[Bjorge, Erik C]

I would like to propose that the automatic builds of Windows BaseTools 
executables be halted.  This implies there will no longer be updates to the 
edk2-BaseTools-win32 repository.

With this change, developers using Windows must install Python 2.7.x and 
configure their environment to build C tools  and run python scripts from 
sources.  This matches the development experience for non-Windows environments.

Please respond with comments by 03/23/2018.

Thanks,
-Erik

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 2/8] SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from Depex

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: marcandre.lur...@redhat.com [mailto:marcandre.lur...@redhat.com]
> Sent: Wednesday, March 7, 2018 11:58 PM
> To: edk2-devel@lists.01.org
> Cc: pjo...@redhat.com; Yao, Jiewen ;
> stef...@linux.vnet.ibm.com; ler...@redhat.com; qemu-de...@nongnu.org;
> javi...@redhat.com; Marc-André Lureau 
> Subject: [PATCH v2 2/8] SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from
> Depex
> 
> From: Marc-André Lureau 
> 
> The module doesn't use read-only variable.
> 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau 
> ---
>  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> index bc910c3baf97..a4aae1488ff8 100644
> --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> @@ -91,7 +91,6 @@ [Pcd]
> 
>  [Depex]
>gEfiPeiMasterBootModePpiGuid AND
> -  gEfiPeiReadOnlyVariable2PpiGuid AND
>gEfiTpmDeviceSelectedGuid
> 
>  [UserExtensions.TianoCore."ExtraFiles"]
> --
> 2.16.2.346.g9779355e34

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 3/3] Hisilicon/D05: Support SBSA watchdog

[Ard Biesheuvel]

On 7 March 2018 at 16:10, Ard Biesheuvel  wrote:
> On 7 March 2018 at 06:55, Heyi Guo  wrote:
>> From: Chenhui Sun 
>>
>> Add description of SBSA watchdogs to ACPI GTDT on D05.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Chenhui Sun 
>> Signed-off-by: Heyi Guo 
>> Cc: Ard Biesheuvel 
>> Cc: Leif Lindholm 
>> Cc: Graeme Gregory 
>> ---
>>  Platform/Hisilicon/D05/D05.dsc  |  4 
>>  Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf |  2 ++
>>  Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc| 19 
>> +++
>>  3 files changed, 13 insertions(+), 12 deletions(-)
>>
>> diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
>> index 0792b0814ea1..22eaf356224d 100644
>> --- a/Platform/Hisilicon/D05/D05.dsc
>> +++ b/Platform/Hisilicon/D05/D05.dsc
>> @@ -418,6 +418,10 @@ [PcdsFixedAtBuild.common]
>>
>>gHisiTokenSpaceGuid.Pcdsoctype|0x1610
>>
>> +  # SBSA watchdog on Hi1616
>> +  gArmTokenSpaceGuid.PcdGenericWatchdogControlBase|0x4050
>> +  gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase|0x4060
>> +
>>  
>> 
>>  #
>>  # Components Section - list of all EDK II Modules needed by this Platform
>> diff --git a/Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf 
>> b/Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf
>> index bb279c8e428e..6955e6145c30 100644
>> --- a/Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf
>> +++ b/Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf
>> @@ -55,5 +55,7 @@ [FixedPcd]
>>gArmTokenSpaceGuid.PcdArmArchTimerIntrNum
>>gArmTokenSpaceGuid.PcdArmArchTimerHypIntrNum
>>gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum
>> +  gArmTokenSpaceGuid.PcdGenericWatchdogControlBase
>> +  gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase
>>gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase
>>
>> diff --git a/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc 
>> b/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc
>> index 2a9d209c00f0..6bc1bde2a490 100644
>> --- a/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc
>> +++ b/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc
>> @@ -29,6 +29,7 @@
>>  #define GTDT_TIMER_ALWAYS_ON_CAPABILITY 
>> EFI_ACPI_6_1_GTDT_TIMER_FLAG_ALWAYS_ON_CAPABILITY
>>
>>  #define GTDT_GTIMER_FLAGS   (GTDT_TIMER_ALWAYS_ON_CAPABILITY | 
>> GTDT_TIMER_ACTIVE_LOW | GTDT_TIMER_LEVEL_TRIGGERED)
>> +#define WATCHDOG_SPAN  0x2000
>>
>
> Please don't use
>
> gArmTokenSpaceGuid.PcdGenericWatchdogXXXBase
>
> to describe two different instances of the IP that are %!@ MB apart.
>

I don't know what happened there :-) That should be 512 MB, obviously.

> Instead, you could introduce your own PCDs in the HiSilicon token
> space, but I am also fine with creating local #defines in this file if
> the watchdog is not used anywhere else.
>
>
>>  #pragma pack (1)
>>
>> @@ -57,22 +58,16 @@ EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
>>  FixedPcdGet32 (PcdArmArchTimerHypIntrNum),// UINT32  
>> NonSecurePL2TimerGSIV
>>  GTDT_GTIMER_FLAGS,// UINT32  
>> NonSecurePL2TimerFlags
>>  0x,   // UINT64  
>> CntReadBasePhysicalAddress
>> -#ifdef notyet
>> -PV660_WATCHDOG_COUNT,  // UINT32  
>> PlatformTimerCount
>> +HI1616_WATCHDOG_COUNT,// UINT32  
>> PlatformTimerCount
>>  sizeof (EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLE) // UINT32 
>> PlatfromTimerOffset
>>},
>>{
>> -EFI_ACPI_6_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
>> -//FixedPcdGet32 (PcdGenericWatchdogRefreshBase), FixedPcdGet32 
>> (PcdGenericWatchdogControlBase), 93, 0),
>> -0, 0, 0, 0),
>> -EFI_ACPI_6_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
>> -//FixedPcdGet32 (PcdGenericWatchdogRefreshBase), FixedPcdGet32 
>> (PcdGenericWatchdogControlBase), 94, 
>> EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_FLAG_SECURE_TIMER)
>> -0, 0, 0, 0)
>> +EFI_ACPI_5_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
>> +  FixedPcdGet32 (PcdGenericWatchdogRefreshBase), FixedPcdGet32 
>> (PcdGenericWatchdogControlBase), 400, 0),
>> +EFI_ACPI_5_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
>> +  FixedPcdGet32 (PcdGenericWatchdogRefreshBase) + WATCHDOG_SPAN, 
>> FixedPcdGet32 (PcdGenericWatchdogControlBase) + WATCHDOG_SPAN, 496, 0)
>> +
>>}
>> -#else /* !notyet */
>> -  0, 0
>> -  }
>> -#endif
>>};
>>
>>  //
>> --
>> 2.7.4
>>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 3/3] Hisilicon/D05: Support SBSA watchdog

[Ard Biesheuvel]

On 7 March 2018 at 06:55, Heyi Guo  wrote:
> From: Chenhui Sun 
>
> Add description of SBSA watchdogs to ACPI GTDT on D05.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chenhui Sun 
> Signed-off-by: Heyi Guo 
> Cc: Ard Biesheuvel 
> Cc: Leif Lindholm 
> Cc: Graeme Gregory 
> ---
>  Platform/Hisilicon/D05/D05.dsc  |  4 
>  Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf |  2 ++
>  Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc| 19 
> +++
>  3 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
> index 0792b0814ea1..22eaf356224d 100644
> --- a/Platform/Hisilicon/D05/D05.dsc
> +++ b/Platform/Hisilicon/D05/D05.dsc
> @@ -418,6 +418,10 @@ [PcdsFixedAtBuild.common]
>
>gHisiTokenSpaceGuid.Pcdsoctype|0x1610
>
> +  # SBSA watchdog on Hi1616
> +  gArmTokenSpaceGuid.PcdGenericWatchdogControlBase|0x4050
> +  gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase|0x4060
> +
>  
> 
>  #
>  # Components Section - list of all EDK II Modules needed by this Platform
> diff --git a/Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf 
> b/Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf
> index bb279c8e428e..6955e6145c30 100644
> --- a/Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf
> +++ b/Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf
> @@ -55,5 +55,7 @@ [FixedPcd]
>gArmTokenSpaceGuid.PcdArmArchTimerIntrNum
>gArmTokenSpaceGuid.PcdArmArchTimerHypIntrNum
>gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum
> +  gArmTokenSpaceGuid.PcdGenericWatchdogControlBase
> +  gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase
>gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase
>
> diff --git a/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc 
> b/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc
> index 2a9d209c00f0..6bc1bde2a490 100644
> --- a/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc
> +++ b/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc
> @@ -29,6 +29,7 @@
>  #define GTDT_TIMER_ALWAYS_ON_CAPABILITY 
> EFI_ACPI_6_1_GTDT_TIMER_FLAG_ALWAYS_ON_CAPABILITY
>
>  #define GTDT_GTIMER_FLAGS   (GTDT_TIMER_ALWAYS_ON_CAPABILITY | 
> GTDT_TIMER_ACTIVE_LOW | GTDT_TIMER_LEVEL_TRIGGERED)
> +#define WATCHDOG_SPAN  0x2000
>

Please don't use

gArmTokenSpaceGuid.PcdGenericWatchdogXXXBase

to describe two different instances of the IP that are %!@ MB apart.

Instead, you could introduce your own PCDs in the HiSilicon token
space, but I am also fine with creating local #defines in this file if
the watchdog is not used anywhere else.


>  #pragma pack (1)
>
> @@ -57,22 +58,16 @@ EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
>  FixedPcdGet32 (PcdArmArchTimerHypIntrNum),// UINT32  
> NonSecurePL2TimerGSIV
>  GTDT_GTIMER_FLAGS,// UINT32  
> NonSecurePL2TimerFlags
>  0x,   // UINT64  
> CntReadBasePhysicalAddress
> -#ifdef notyet
> -PV660_WATCHDOG_COUNT,  // UINT32  
> PlatformTimerCount
> +HI1616_WATCHDOG_COUNT,// UINT32  
> PlatformTimerCount
>  sizeof (EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLE) // UINT32 
> PlatfromTimerOffset
>},
>{
> -EFI_ACPI_6_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
> -//FixedPcdGet32 (PcdGenericWatchdogRefreshBase), FixedPcdGet32 
> (PcdGenericWatchdogControlBase), 93, 0),
> -0, 0, 0, 0),
> -EFI_ACPI_6_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
> -//FixedPcdGet32 (PcdGenericWatchdogRefreshBase), FixedPcdGet32 
> (PcdGenericWatchdogControlBase), 94, 
> EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_FLAG_SECURE_TIMER)
> -0, 0, 0, 0)
> +EFI_ACPI_5_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
> +  FixedPcdGet32 (PcdGenericWatchdogRefreshBase), FixedPcdGet32 
> (PcdGenericWatchdogControlBase), 400, 0),
> +EFI_ACPI_5_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
> +  FixedPcdGet32 (PcdGenericWatchdogRefreshBase) + WATCHDOG_SPAN, 
> FixedPcdGet32 (PcdGenericWatchdogControlBase) + WATCHDOG_SPAN, 496, 0)
> +
>}
> -#else /* !notyet */
> -  0, 0
> -  }
> -#endif
>};
>
>  //
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/2] Hisilicon/D0x: Enable tftp command by default

[Ard Biesheuvel]

On 7 March 2018 at 03:03, Heyi Guo  wrote:
> Since D0x platforms always have network enabled, we would like to
> enable tftp command by default so that we can download something in
> EFI Shell.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Heyi Guo 
> Cc: Ard Biesheuvel 
> Cc: Leif Lindholm 

The first patch looks fine to me, but I would like to give Leif a
chance to comment on the policy side of this patch.

Please ping us by the end of next week if we haven't responded by then.

> ---
>  Platform/Hisilicon/D03/D03.dsc | 2 ++
>  Platform/Hisilicon/D05/D05.dsc | 1 +
>  2 files changed, 3 insertions(+)
>
> diff --git a/Platform/Hisilicon/D03/D03.dsc b/Platform/Hisilicon/D03/D03.dsc
> index cb0669d639d1..fce1e60b1275 100644
> --- a/Platform/Hisilicon/D03/D03.dsc
> +++ b/Platform/Hisilicon/D03/D03.dsc
> @@ -29,6 +29,8 @@ [Defines]
>SKUID_IDENTIFIER   = DEFAULT
>FLASH_DEFINITION   = 
> Platform/Hisilicon/$(PLATFORM_NAME)/$(PLATFORM_NAME).fdf
>
> +  DEFINE INCLUDE_TFTP_COMMAND= TRUE
> +
>  !include Silicon/Hisilicon/Hisilicon.dsc.inc
>
>  [LibraryClasses.common]
> diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
> index 8373a821a496..f007f3d2d7e8 100644
> --- a/Platform/Hisilicon/D05/D05.dsc
> +++ b/Platform/Hisilicon/D05/D05.dsc
> @@ -29,6 +29,7 @@ [Defines]
>SKUID_IDENTIFIER   = DEFAULT
>FLASH_DEFINITION   = 
> Platform/Hisilicon/$(PLATFORM_NAME)/$(PLATFORM_NAME).fdf
>DEFINE EDK2_SKIP_PEICORE=0
> +  DEFINE INCLUDE_TFTP_COMMAND= TRUE
>DEFINE NETWORK_IP6_ENABLE  = FALSE
>DEFINE HTTP_BOOT_ENABLE= FALSE
>
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 7/8] ovmf: link with Tcg2Dxe module

[marcandre . lureau]

From: Marc-André Lureau 

This module measures and log the boot environment. It also produces
the Tcg2 protocol, which allows for example to read the log from OS.

The linux kernel doesn't yet read the EFI_TCG2_EVENT_LOG_FORMAT_TCG_2,
which is required for crypto-agile log. In fact, only upcoming 4.16
adds support EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2:

[0.00] efi: EFI v2.70 by EDK II
[0.00] efi:  SMBIOS=0x3fa1f000  ACPI=0x3fbb6000  ACPI 2.0=0x3fbb6014  
MEMATTR=0x3e7d4318  TPMEventLog=0x3db21018

$ python chipsec_util.py tpm parse_log binary_bios_measurements

[CHIPSEC] Version 1.3.5.dev2
[CHIPSEC] API mode: using OS native API (not using CHIPSEC kernel module)
[CHIPSEC] Executing command 'tpm' with args ['parse_log', 
'/tmp/binary_bios_measurements']

PCR: 0  type: EV_S_CRTM_VERSION size: 0x2   digest: 
1489f923c4dca729178b3e3233458550d8dddf29
+ version:
PCR: 0  type: EV_EFI_PLATFORM_FIRMWARE_BLOB size: 0x10  digest: 
fd39ced7c0d2a61f6830c78c7625f94826b05bcc
+ base: 0x82length: 0xe
PCR: 0  type: EV_EFI_PLATFORM_FIRMWARE_BLOB size: 0x10  digest: 
39ebc6783b72bc1e73c7d5bcfeb5f54a3f105d4c
+ base: 0x90length: 0xa0
PCR: 7  type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x35  digest: 
57cd4dc19442475aa82743484f3b1caa88e142b8
PCR: 7  type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x24  digest: 
9b1387306ebb7ff8e795e7be77563666bbf4516e
PCR: 7  type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x26  digest: 
9afa86c507419b8570c62167cb9486d9fc809758
PCR: 7  type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x24  digest: 
5bf8faa078d40ffbd03317c93398b01229a0e1e0
PCR: 7  type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x26  digest: 
734424c9fe8fc71716c42096f4b74c88733b175e
PCR: 7  type: EV_SEPARATOR  size: 0x4   digest: 
9069ca78e7450a285173431b3e52c5c25299e473
PCR: 1  type: EV_EFI_VARIABLE_BOOT  size: 0x3e  digest: 
252f8ebb85340290b64f4b06a001742be8e5cab6
PCR: 1  type: EV_EFI_VARIABLE_BOOT  size: 0x6e  digest: 
22a4f6ee9af6dba01d3528deb64b74b582fc182b
PCR: 1  type: EV_EFI_VARIABLE_BOOT  size: 0x80  digest: 
b7811d5bf30a7efd4e385c6179fe10d9290bb9e8
PCR: 1  type: EV_EFI_VARIABLE_BOOT  size: 0x84  digest: 
425e502c24fc924e231e0a62327b6b7d1f704573
PCR: 1  type: EV_EFI_VARIABLE_BOOT  size: 0x9a  digest: 
0b5d2c98ac5de6148a4a1490ff9d5df69039f04e
PCR: 1  type: EV_EFI_VARIABLE_BOOT  size: 0xbd  digest: 
20bd5f402271d57a88ea314fe35c1705956b1f74
PCR: 1  type: EV_EFI_VARIABLE_BOOT  size: 0x88  digest: 
df5d6605cb8f4366d745a8464cfb26c1efdc305c
PCR: 4  type: EV_EFI_ACTION size: 0x28  digest: 
cd0fdb4531a6ec41be2753ba042637d6e5f7f256
PCR: 0  type: EV_SEPARATOR  size: 0x4   digest: 
9069ca78e7450a285173431b3e52c5c25299e473
PCR: 1  type: EV_SEPARATOR  size: 0x4   digest: 
9069ca78e7450a285173431b3e52c5c25299e473
PCR: 2  type: EV_SEPARATOR  size: 0x4   digest: 
9069ca78e7450a285173431b3e52c5c25299e473
PCR: 3  type: EV_SEPARATOR  size: 0x4   digest: 
9069ca78e7450a285173431b3e52c5c25299e473
PCR: 4  type: EV_SEPARATOR  size: 0x4   digest: 
9069ca78e7450a285173431b3e52c5c25299e473
PCR: 5  type: EV_SEPARATOR  size: 0x4   digest: 
9069ca78e7450a285173431b3e52c5c25299e473

$ tpm2_pcrlist
sha1 :
  0  : 35bd1786b6909daad610d7598b1d620352d33b8a
  1  : ec0511e860206e0af13c31da2f9e943fb6ca353d
  2  : b2a83b0ebf2f8374299a5b2bdfc31ea955ad7236
  3  : b2a83b0ebf2f8374299a5b2bdfc31ea955ad7236
  4  : 45a323382bd933f08e7f0e256bc8249e4095b1ec
  5  : d16d7e629fd8d08ca256f9ad3a3a1587c9e6cc1b
  6  : b2a83b0ebf2f8374299a5b2bdfc31ea955ad7236
  7  : 518bd167271fbb64589c61e43d8c0165861431d8
  8  : 
  9  : 
  10 : 
  11 : 
  12 : 
  13 : 
  14 : 
  15 : 
  16 : 
  17 : 
  18 : 
  19 : 
  20 : 
  21 : 
  22 : 
  23 : 
sha256 :
  0  : 9ae903dbae3357ac00d223660bac19ea5c021499a56201104332ab966631ce2c
  1  : acc611d90245cf04e77b0ca94901f90e7fa54770f0426f53c3049b532243d1b8
  2  : 3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969
  3  : 

[edk2] [PATCH v2 6/8] ovmf: link with Tcg2Pei module

[marcandre . lureau]

From: Marc-André Lureau 

This module will initialize TPM device, measure reported FVs and BIOS
version. We keep both SHA-1 and SHA-256 for the TCG 1.2 log format
compatibility, but the SHA-256 measurements and TCG 2 log format are
now recommended.

Cc: Laszlo Ersek 
Cc: Stefan Berger 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marc-André Lureau 
---
 OvmfPkg/OvmfPkgX64.dsc | 7 +++
 OvmfPkg/OvmfPkgX64.fdf | 1 +
 2 files changed, 8 insertions(+)

diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 64bd6b6a9f08..3fa1a31f4c37 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -278,6 +278,8 @@ [LibraryClasses.common.PEIM]
   QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
 
 !if $(TPM2_ENABLE)
+  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
+  
HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf
   Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
 !endif
 
@@ -615,6 +617,11 @@ [Components]
 
 !if $(TPM2_ENABLE) == TRUE
   OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {
+
+  NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
+  NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
+  }
 !endif
 
   #
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index dbafada5226b..c0173e7adf5f 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -167,6 +167,7 @@ [FV.PEIFV]
 
 !if $(TPM2_ENABLE) == TRUE
 INF  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+INF  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
 !endif
 
 

-- 
2.16.2.346.g9779355e34

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 8/8] ovmf: add DxeTpm2MeasureBootLib

[marcandre . lureau]

From: Marc-André Lureau 

The library registers a security management handler, to measure images
that are not measure in PEI phase.

This seems to work for example with the qemu PXE rom:

Loading driver at 0x0003E6C2000 EntryPoint=0x0003E6C9076 8086100e.efi

And the following binary_bios_measurements log entry seems to be
added:

PCR: 2  type: EV_EFI_BOOT_SERVICES_DRIVER   size: 0x4e  digest: 
70a22475e9f18806d2ed9193b48d80d26779d9a4

Cc: Laszlo Ersek 
Cc: Stefan Berger 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marc-André Lureau 
---
 OvmfPkg/OvmfPkgX64.dsc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 7753852144fb..9db1712e3623 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -662,6 +662,9 @@ [Components]
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   
NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
+!endif
+!if $(TPM2_ENABLE) == TRUE
+  NULL|SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
 !endif
   }
 
-- 
2.16.2.346.g9779355e34

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 5/8] ovmf: add OvmfPkg Tcg2ConfigPei module

[marcandre . lureau]

From: Marc-André Lureau 

The Tcg2ConfigPei module informs the firmware globally about the TPM
device type, by setting the PcdTpmInstanceGuid PCD to the appropriate
GUID value. The original module under SecurityPkg can perform device
detection, or read a cached value from a non-volatile UEFI variable.

OvmfPkg's clone of the module only performs the TPM2 hardware detection.

This is what the module does:

- Check the QEMU hardware for TPM2 availability only

- If found, set the dynamic PCD "PcdTpmInstanceGuid" to
   This is what informs the rest of
  the firmware about the TPM type.

- Install the gEfiTpmDeviceSelectedGuid PPI. This action permits the
  PEI_CORE to dispatch the Tcg2Pei module, which consumes the above PCD.
  In effect, the gEfiTpmDeviceSelectedGuid PPI serializes the setting
  and the consumption of the "TPM type" PCD.

- If no TPM2 was found, install gPeiTpmInitializationDonePpiGuid.
  (Normally this is performed by Tcg2Pei, but Tcg2Pei doesn't do it if
  no TPM2 is available. So in that case our Tcg2ConfigPei must do it.)

Cc: Laszlo Ersek 
Cc: Stefan Berger 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marc-André Lureau 
---
 OvmfPkg/OvmfPkgX64.dsc   |  17 
 OvmfPkg/OvmfPkgX64.fdf   |   4 +
 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf |  57 +++
 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c  | 124 +++
 OvmfPkg/Tcg/Tcg2Config/TpmDetection.c|  46 +
 5 files changed, 248 insertions(+)
 create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
 create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
 create mode 100644 OvmfPkg/Tcg/Tcg2Config/TpmDetection.c

diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index a8e89276c0b2..64bd6b6a9f08 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -39,6 +39,7 @@ [Defines]
   DEFINE HTTP_BOOT_ENABLE= FALSE
   DEFINE SMM_REQUIRE = FALSE
   DEFINE TLS_ENABLE  = FALSE
+  DEFINE TPM2_ENABLE = FALSE
 
   #
   # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to
@@ -208,6 +209,10 @@ [LibraryClasses]
   
OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
   XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
 
+!if $(TPM2_ENABLE) == TRUE
+  Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
+!endif
+
 [LibraryClasses.common]
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
 
@@ -272,6 +277,10 @@ [LibraryClasses.common.PEIM]
   PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
   QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
 
+!if $(TPM2_ENABLE)
+  Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
+!endif
+
 [LibraryClasses.common.DXE_CORE]
   HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
   DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
@@ -554,6 +563,10 @@ [PcdsDynamicDefault]
 
   gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00
 
+!if $(TPM2_ENABLE) == TRUE
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
+!endif
+
 

 #
 # Components Section - list of all EDK II Modules needed by this Platform.
@@ -600,6 +613,10 @@ [Components]
 !endif
   UefiCpuPkg/CpuMpPei/CpuMpPei.inf
 
+!if $(TPM2_ENABLE) == TRUE
+  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+!endif
+
   #
   # DXE Phase modules
   #
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 2fc17810eb23..dbafada5226b 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -165,6 +165,10 @@ [FV.PEIFV]
 !endif
 INF  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
 
+!if $(TPM2_ENABLE) == TRUE
+INF  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+!endif
+
 

 
 [FV.DXEFV]
diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf 
b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
new file mode 100644
index ..201e4f24d5f4
--- /dev/null
+++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
@@ -0,0 +1,57 @@
+## @file
+#  Set TPM device type
+#
+#  This module initializes TPM device type based on variable and detection.
+#  This OvmfPkg of SecurityPkg only does TPM2 detection.
+#
+# Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
+# Copyright (C) 2018, Red Hat, Inc.
+#
+# 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 

[edk2] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support

[marcandre . lureau]

From: Marc-André Lureau 

Hi,

The following series adds basic TPM2 support for OVMF-on-QEMU (I
haven't tested TPM1, for lack of interest). It links with the modules
to initializes the device in PEI phase, and do measurements (both PEI
and DXE). The Tcg2Dxe module provides the Tcg2 protocol which allows
the guest to access the measurement log and other facilities.

DxeTpm2MeasureBootLib seems to do its job at measuring images that are
not measured in PEI phase (such as PCI PXE rom)

Tcg2ConfigDxe is not included due to its integration with edk2 own PPI
implementation which conflicts with qemu design. PPI design is still
being discussed & experimented at this point.

Linux guests seem to work fine. But windows guest generally complains
about the lack of PPI interface (most HLK tests require it, tpm.msc
admin interactions too). I haven't done "real" use-cases tests, as I
lack experience with TPM usage. Any help appreciated to test the TPM.

I build edk2 with:

$ build -DTPM2_ENABLE -DSECURE_BOOT_ENABLE  -DMEM_VARSTORE_EMU_ENABLE=FALSE

I test with qemu & swtpm/libtpms (tpm2 branches, swtpm_setup.sh --tpm2 
--tpm-state tpmstatedir)

$ swtpm socket --tpmstate tpmstatedir --ctrl type=unixio,path=tpmsock  --tpm2 &
$ qemu .. -chardev socket,id=chrtpm,path=tpmsock -tpmdev 
emulator,id=tpm0,chardev=chrtpm -device tpm-crb,tpmdev=tpm0

Thanks

Github tree:
https://github.com/elmarco/edk2/tree/tpm2 (tpm2-v2 tag)

Related bug:
https://bugzilla.tianocore.org/show_bug.cgi?id=594

v2:
- the series can now be applied to master directly, thanks to dropping
  PeiReadOnlyVariable requirement
- remove the HOB list workaround, the main fix is now upstream. Add a
  preliminary patch to complete it.
- removed traces of TPM1.2 support
- add own OvmfPkg Tcg2ConfigPei, which performs only TPM2 detection
- make PcdTpmInstanceGuid default all-bits-zero
- drop unneeded Pcd values
- explain why SHA1 is still nice to have (for 1.2 log format)
- drop Tcg2ConfigDxe
- more detailed commit messages, thanks to Laszlo explanations!
- rebased

TODO:
- modify Ia32 and Ia32X64 builds

Marc-André Lureau (8):
  SecurityPkg: also clear HashInterfaceHob.SupportedHashMask
  SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from Depex
  MdeModulePkg: fix REGISITER -> REGISTER
  ovmf: simplify SecurityStubDxe.inf inclusion
  ovmf: add OvmfPkg Tcg2ConfigPei module
  ovmf: link with Tcg2Pei module
  ovmf: link with Tcg2Dxe module
  ovmf: add DxeTpm2MeasureBootLib

 MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c |  18 +--
 MdeModulePkg/Core/Pei/Image/Image.c   |   4 +-
 MdeModulePkg/Core/Pei/PeiMain.h   |   2 +-
 MdeModulePkg/Core/Pei/PeiMain/PeiMain.c   |   2 +-
 OvmfPkg/OvmfPkgIa32.dsc   |   6 +-
 OvmfPkg/OvmfPkgIa32X64.dsc|   6 +-
 OvmfPkg/OvmfPkgX64.dsc|  49 ++-
 OvmfPkg/OvmfPkgX64.fdf|   9 ++
 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf  |  57 
 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c   | 124 ++
 OvmfPkg/Tcg/Tcg2Config/TpmDetection.c |  46 +++
 .../HashLibBaseCryptoRouterPei.c  |   1 +
 SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf   |   1 -
 13 files changed, 299 insertions(+), 26 deletions(-)
 create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
 create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
 create mode 100644 OvmfPkg/Tcg/Tcg2Config/TpmDetection.c

-- 
2.16.2.346.g9779355e34

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 3/8] MdeModulePkg: fix REGISITER -> REGISTER

[marcandre . lureau]

From: Marc-André Lureau 

Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marc-André Lureau 
---
 MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 18 +-
 MdeModulePkg/Core/Pei/Image/Image.c   |  4 ++--
 MdeModulePkg/Core/Pei/PeiMain.h   |  2 +-
 MdeModulePkg/Core/Pei/PeiMain/PeiMain.c   |  2 +-
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c 
b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
index 79f2e5cebcbe..027176d872c7 100644
--- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
@@ -970,7 +970,7 @@ PeiDispatcher (
   if ((Private->PeiMemoryInstalled) && 
(Private->HobList.HandoffInformationTable->BootMode != BOOT_ON_S3_RESUME || 
PcdGetBool (PcdShadowPeimOnS3Boot))) {
 //
 // Once real memory is available, shadow the RegisterForShadow modules. 
And meanwhile
-// update the modules' status from PEIM_STATE_REGISITER_FOR_SHADOW to 
PEIM_STATE_DONE.
+// update the modules' status from PEIM_STATE_REGISTER_FOR_SHADOW to 
PEIM_STATE_DONE.
 //
 SaveCurrentPeimCount  = Private->CurrentPeimCount;
 SaveCurrentFvCount= Private->CurrentPeimFvCount;
@@ -978,7 +978,7 @@ PeiDispatcher (
 
 for (Index1 = 0; Index1 <= SaveCurrentFvCount; Index1++) {
   for (Index2 = 0; (Index2 < PcdGet32 (PcdPeiCoreMaxPeimPerFv)) && 
(Private->Fv[Index1].FvFileHandles[Index2] != NULL); Index2++) {
-if (Private->Fv[Index1].PeimState[Index2] == 
PEIM_STATE_REGISITER_FOR_SHADOW) {
+if (Private->Fv[Index1].PeimState[Index2] == 
PEIM_STATE_REGISTER_FOR_SHADOW) {
   PeimFileHandle = Private->Fv[Index1].FvFileHandles[Index2];
   Private->CurrentFileHandle   = PeimFileHandle;
   Private->CurrentPeimFvCount  = Index1;
@@ -986,13 +986,13 @@ PeiDispatcher (
   Status = PeiLoadImage (
 (CONST EFI_PEI_SERVICES **) >Ps,
 PeimFileHandle,
-PEIM_STATE_REGISITER_FOR_SHADOW,
+PEIM_STATE_REGISTER_FOR_SHADOW,
 ,
 
 );
   if (Status == EFI_SUCCESS) {
 //
-// PEIM_STATE_REGISITER_FOR_SHADOW move to PEIM_STATE_DONE
+// PEIM_STATE_REGISTER_FOR_SHADOW move to PEIM_STATE_DONE
 //
 Private->Fv[Index1].PeimState[Index2]++;
 //
@@ -1165,7 +1165,7 @@ PeiDispatcher (
 //
 PeiCheckAndSwitchStack (SecCoreData, Private);
 
-if ((Private->PeiMemoryInstalled) && 
(Private->Fv[FvCount].PeimState[PeimCount] == PEIM_STATE_REGISITER_FOR_SHADOW) 
&&   \
+if ((Private->PeiMemoryInstalled) && 
(Private->Fv[FvCount].PeimState[PeimCount] == PEIM_STATE_REGISTER_FOR_SHADOW) 
&&   \
 (Private->HobList.HandoffInformationTable->BootMode != 
BOOT_ON_S3_RESUME || PcdGetBool (PcdShadowPeimOnS3Boot))) {
   //
   // If memory is available we shadow images by default for 
performance reasons.
@@ -1179,7 +1179,7 @@ PeiDispatcher (
 Status = PeiLoadImage (
PeiServices,
PeimFileHandle,
-   PEIM_STATE_REGISITER_FOR_SHADOW,
+   PEIM_STATE_REGISTER_FOR_SHADOW,
,

);
@@ -1192,7 +1192,7 @@ PeiDispatcher (
   //PERF_END (PeiServices, L"PEIM", PeimFileHandle, 0);
 
   //
-  // PEIM_STATE_REGISITER_FOR_SHADOW move to PEIM_STATE_DONE
+  // PEIM_STATE_REGISTER_FOR_SHADOW move to PEIM_STATE_DONE
   //
   Private->Fv[FvCount].PeimState[PeimCount]++;
 
@@ -1356,14 +1356,14 @@ PeiRegisterForShadow (
 return EFI_NOT_FOUND;
   }
 
-  if 
(Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPeimCount] 
>= PEIM_STATE_REGISITER_FOR_SHADOW) {
+  if 
(Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPeimCount] 
>= PEIM_STATE_REGISTER_FOR_SHADOW) {
 //
 // If the PEIM has already entered the PEIM_STATE_REGISTER_FOR_SHADOW or 
PEIM_STATE_DONE then it's already been started
 //
 return EFI_ALREADY_STARTED;
   }
 
-  
Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPeimCount] = 
PEIM_STATE_REGISITER_FOR_SHADOW;
+  
Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPeimCount] = 
PEIM_STATE_REGISTER_FOR_SHADOW;
 
   return EFI_SUCCESS;
 }
diff --git a/MdeModulePkg/Core/Pei/Image/Image.c 
b/MdeModulePkg/Core/Pei/Image/Image.c
index f41d3acac77e..f07f48823117 100644
--- a/MdeModulePkg/Core/Pei/Image/Image.c
+++ b/MdeModulePkg/Core/Pei/Image/Image.c
@@ -387,7 +387,7 @@ LoadAndRelocatePeCoffImage (
   }
   

Re: [edk2] [PATCH v1] Platform/ARM/VExpressPkg: Fix RSDT pointer in RSDP

[Ard Biesheuvel]

On 6 March 2018 at 14:15, Sami Mujawar  wrote:
> According to the SBBR Specification (ARM DEN 0044B), Section 4.2.1.1
>   "Within the RSDP, the RsdtAddress field must be null (zero)
>and the XsdtAddresss MUST be a valid, non-null, 64-bit value."
>
> The PcdAcpiExposedTableVersions is used to indicate the ACPI versions
> that are supported. The default value for PcdAcpiExposedTableVersions
> is 0x3E which indicates that the ACPI versions 1.0B and above are
> supported.
>
> For ACPI 1.0B the RSDT pointer is set in the RSDP table. However for
> ACPI versions greater than ACPI 1.0B the AcpiTableDxe populates
> the RSDP with the RSDT address set to NULL.
>
> Therefore set the PcdAcpiExposedTableVersions to 0x20 indicating
> support for ACPI 5.0 and above.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sami Mujawar 
> Signed-off-by: Evan Lloyd 

I am changing the subject into

Platform/ARM/VExpressPkg: Provide only 64-bit ACPI entrypoint

but the patch looks fine to me.

Reviewed-by: Ard Biesheuvel 

Pushed as 9422cb8c75b3

Thanks


> ---
>
> The changes can be seen at
> https://github.com/samimujawar/edk2-platforms/tree/212_fvp_rsdt_fix_v1
>
> Notes:
> v1:
> - Setup PcdAcpiExposedTableVersions correctly so that the
>   AcpiTableDxe populates the RSDP with the RSDT address set
>   to NULL. [SAMI]
>
>  Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc 
> b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> index 
> 295e9a126b0ebaa4653d7e25e2f7d8c396403764..cdf9e2d49784d542701dc84eb511f592e77ec106
>  100644
> --- a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> +++ b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> @@ -1,5 +1,5 @@
>  #
> -#  Copyright (c) 2011-2017, ARM Limited. All rights reserved.
> +#  Copyright (c) 2011-2018, ARM Limited. All rights reserved.
>  #
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions of the BSD 
> License
> @@ -162,6 +162,11 @@ [PcdsFixedAtBuild.common]
># the entire FVP address space can be covered by 36 bit VAs
>gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36
>
> +  #
> +  # ACPI Table Version
> +  #
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x20
> +
>  
> 
>  #
>  # Components Section - list of all EDK II Modules needed by this Platform
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 1/1] SecurityPkg: fix ZeroMem HashInterfaceHob

[Zeng, Star]

Yes, since the V1 has been pushed.
Just adding one line like below based on V1 should be ok.

HashInterfaceHob->SupportedHashMask = 0;


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
Marc-André Lureau
Sent: Wednesday, March 7, 2018 10:56 PM
To: edk2-devel@lists.01.org
Cc: Laszlo Ersek ; Yao, Jiewen ; 
Zhang, Chao B ; Zeng, Star 
Subject: Re: [edk2] [PATCH v2 1/1] SecurityPkg: fix ZeroMem HashInterfaceHob

Hi

On Wed, Mar 7, 2018 at 12:24 PM,   wrote:
> From: Marc-André Lureau 
>
> The ZeroMem() call goes beyond the HashInterfaceHob structure, causing 
> HOB list corruption. The intent was to clear all but the Identifier, 
> that is starting from HashInterfaceCount.
>

I see v1 is already applied. I'll send another patch to clear the mask in the 
Ovmf/TPM series.

Thanks

> Quoting Laszlo Ersek:
>   Therefore I think the *first* approach to actually fixing this bug would
>   be to simply add the "Count" suffix:
>
>   > diff --git 
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c 
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
>   > index dbee0f2531bc..a869fffae3fe 100644
>   > --- 
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
>   > +++ 
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
>   > @@ -424,7 +424,10 @@ HashLibBaseCryptoRouterPeiConstructor (
>   >  // This is the second execution of this module, clear the hash 
> interface
>   >  // information registered at its first execution.
>   >  //
>   > -ZeroMem (>HashInterface, sizeof 
> (*HashInterfaceHob) - sizeof (EFI_GUID));
>   > +ZeroMem (
>   > +  >HashInterfaceCount,
>   > +  sizeof (*HashInterfaceHob) - sizeof (EFI_GUID)
>   > +  );
>   >}
>   >
>   >//
>
>   On the other hand, I don't think such a fix would be great. First of
>   all, the HASH_INTERFACE_HOB struct definition is not wrapped in
>
>   #pragma pack (1)
>   ...
>   #pragma pack ()
>
>   in other words, HASH_INTERFACE_HOB is not packed, hence there could be
>   an unspecified amount of padding between "Identifier" and
>   "HashInterfaceCount". That would lead to the same kind of buffer
>   overflow, because "Length" includes the padding, but the "Buffer"
>   argument doesn't.
>
>   Second, even if there were no padding (and thus the byte count would be
>   a perfect match for the full HOB structure), in the "Buffer" argument
>   we take the address of the "HashInterfaceCount" field, and with the
>   "Length" field, we still overflow that *individual* field. Although we
>   wouldn't overflow the containing HOB structure, this is still (arguably)
>   undefined behavior -- I seem to remember that some compilers actually
>   blow up on this when you use the standard ISO C memcpy() or memset()
>   functions in such contexts.
>
> This patch takes Laszlo suggestion to fix the issue.
>
> Cc: Jiewen Yao 
> Cc: Chao Zhang 
> Cc: Star Zeng 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau 
> ---
>  .../HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c   | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git 
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterP
> ei.c 
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterP
> ei.c index dbee0f2531bc..84c1aaae70eb 100644
> --- 
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterP
> ei.c
> +++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRou
> +++ terPei.c
> @@ -397,6 +397,7 @@ HashLibBaseCryptoRouterPeiConstructor (  {
>EFI_STATUSStatus;
>HASH_INTERFACE_HOB*HashInterfaceHob;
> +  UINTN ClearStartOffset;
>
>HashInterfaceHob = InternalGetHashInterfaceHob ();
>if (HashInterfaceHob == NULL) {
> @@ -424,7 +425,11 @@ HashLibBaseCryptoRouterPeiConstructor (
>  // This is the second execution of this module, clear the hash interface
>  // information registered at its first execution.
>  //
> -ZeroMem (>HashInterface, sizeof (*HashInterfaceHob) - 
> sizeof (EFI_GUID));
> +ClearStartOffset = OFFSET_OF (HASH_INTERFACE_HOB, HashInterfaceCount);
> +ZeroMem (
> +  (UINT8 *)HashInterfaceHob + ClearStartOffset,
> +  sizeof (*HashInterfaceHob) - ClearStartOffset
> +  );
>}
>
>//
> --
> 2.16.2.346.g9779355e34
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel



-- 
Marc-André Lureau
___

Re: [edk2] [PATCH 1/1] RFC: SecurityPkg: only clear HashInterface informations

[Zeng, Star]

My head was stuck to say using " ZeroMem (HashInterfaceHob, sizeof 
(*HashInterfaceHob) - sizeof (EFI_GUID)) ", that is wrong. :(

Thanks,
Star
-Original Message-
From: Zeng, Star 
Sent: Wednesday, March 7, 2018 5:42 PM
To: Laszlo Ersek ; marcandre.lur...@redhat.com
Cc: edk2-devel@lists.01.org; Yao, Jiewen ; Zhang, Chao B 
; Zeng, Star 
Subject: RE: [edk2] [PATCH 1/1] RFC: SecurityPkg: only clear HashInterface 
informations

Hi Laszlo,

Good analysis.

Yes, the SupportedHashMask field in HashInterfaceHob will have stale value, but 
that does not impact the functionality since the code have.
So the patch could fix the problem.

HashLibBaseCryptoRouterPeiConstructor():
  Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, 0);

RegisterHashInterfaceLib():
  HashInterfaceHob->SupportedHashMask = PcdGet32 (PcdTcg2HashAlgorithmBitmap) | 
HashMask;

HashStart()/HashUpdate()/HashCompleteAndExtend()/HashAndExtend():
  if (HashInterfaceHob->HashInterfaceCount == 0) {
return EFI_UNSUPPORTED;
  }

RegisterHashInterfaceLib():
  if (HashInterfaceHob->HashInterfaceCount >= HASH_COUNT) {
return EFI_OUT_OF_RESOURCES;
  }

As I know, Chao has helped push the patch.
But I am fine with keeping this patch or continue refining the code. :)

Is it working with " ZeroMem (HashInterfaceHob, sizeof (*HashInterfaceHob) - 
sizeof (EFI_GUID)) " if to refine the code?


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Wednesday, March 7, 2018 5:06 PM
To: marcandre.lur...@redhat.com
Cc: edk2-devel@lists.01.org; Yao, Jiewen ; Zeng, Star 
; Zhang, Chao B 
Subject: Re: [edk2] [PATCH 1/1] RFC: SecurityPkg: only clear HashInterface 
informations

Hi Marc-André,

On 03/06/18 21:27, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
>
> The ZeroMem() call goes beyond the HashInterfaceHob structure, causing 
> HOB list corruption. Instead, just clear the HashInterface fields, as 
> I suppose was originally intended.
>
> Cc: Jiewen Yao 
> Cc: Chao Zhang 
> Cc: Star Zeng 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau 
> ---
>  .../HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c   | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterP
> ei.c
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterP
> ei.c index dbee0f2531bc..361a4f6508a0 100644
> ---
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterP
> ei.c
> +++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRou
> +++ terPei.c
> @@ -424,7 +424,8 @@ HashLibBaseCryptoRouterPeiConstructor (
>  // This is the second execution of this module, clear the hash interface
>  // information registered at its first execution.
>  //
> -ZeroMem (>HashInterface, sizeof (*HashInterfaceHob) - 
> sizeof (EFI_GUID));
> +ZeroMem (>HashInterface, sizeof 
> (HashInterfaceHob->HashInterface));
> +HashInterfaceHob->HashInterfaceCount = 0;
>}
>
>//
>

Excellent catch! :)

I do have a question however, for the other reviewers on this patch, in line 
with your commit message saying, "clear the HashInterface fields, as *I 
suppose* was originally intended".

I don't think the proposed update matches the original intent 100%.
Namely, this is the definition of the HASH_INTERFACE_HOB type (from the same 
source file):

> typedef struct {
>   //
>   // If gZeroGuid, SupportedHashMask is 0 for FIRST module which consumes 
> HashLib
>   //   or the hash algorithm bitmap of LAST module which consumes HashLib.
>   //   HashInterfaceCount and HashInterface are all 0.
>   // If gEfiCallerIdGuid, HashInterfaceCount, HashInterface and 
> SupportedHashMask
>   //   are the hash interface information of CURRENT module which consumes 
> HashLib.
>   //
>   EFI_GUID Identifier;
>   UINTNHashInterfaceCount;
>   HASH_INTERFACE   HashInterface[HASH_COUNT];
>   UINT32   SupportedHashMask;
> } HASH_INTERFACE_HOB;

Note that it says, if Identifier equals "gEfiCallerIdGuid", then *all
three* subsequent fields carry information of the current module that consumes 
HashLib. Those three fields include "SupportedHashMask".

Furthermore, the comment just preceding the bug / ZeroMem() call says,

> //
> // In PEI phase, some modules may call RegisterForShadow and will be
> // shadowed and executed again after memory is discovered.
> // This is the second execution of this module, clear the hash interface
> // information registered at its first execution.
> 

Re: [edk2] [PATCH v2 1/1] SecurityPkg: fix ZeroMem HashInterfaceHob

[Marc-André Lureau]

Hi

On Wed, Mar 7, 2018 at 12:24 PM,   wrote:
> From: Marc-André Lureau 
>
> The ZeroMem() call goes beyond the HashInterfaceHob structure, causing
> HOB list corruption. The intent was to clear all but the Identifier,
> that is starting from HashInterfaceCount.
>

I see v1 is already applied. I'll send another patch to clear the mask
in the Ovmf/TPM series.

Thanks

> Quoting Laszlo Ersek:
>   Therefore I think the *first* approach to actually fixing this bug would
>   be to simply add the "Count" suffix:
>
>   > diff --git 
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c 
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
>   > index dbee0f2531bc..a869fffae3fe 100644
>   > --- 
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
>   > +++ 
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
>   > @@ -424,7 +424,10 @@ HashLibBaseCryptoRouterPeiConstructor (
>   >  // This is the second execution of this module, clear the hash 
> interface
>   >  // information registered at its first execution.
>   >  //
>   > -ZeroMem (>HashInterface, sizeof 
> (*HashInterfaceHob) - sizeof (EFI_GUID));
>   > +ZeroMem (
>   > +  >HashInterfaceCount,
>   > +  sizeof (*HashInterfaceHob) - sizeof (EFI_GUID)
>   > +  );
>   >}
>   >
>   >//
>
>   On the other hand, I don't think such a fix would be great. First of
>   all, the HASH_INTERFACE_HOB struct definition is not wrapped in
>
>   #pragma pack (1)
>   ...
>   #pragma pack ()
>
>   in other words, HASH_INTERFACE_HOB is not packed, hence there could be
>   an unspecified amount of padding between "Identifier" and
>   "HashInterfaceCount". That would lead to the same kind of buffer
>   overflow, because "Length" includes the padding, but the "Buffer"
>   argument doesn't.
>
>   Second, even if there were no padding (and thus the byte count would be
>   a perfect match for the full HOB structure), in the "Buffer" argument
>   we take the address of the "HashInterfaceCount" field, and with the
>   "Length" field, we still overflow that *individual* field. Although we
>   wouldn't overflow the containing HOB structure, this is still (arguably)
>   undefined behavior -- I seem to remember that some compilers actually
>   blow up on this when you use the standard ISO C memcpy() or memset()
>   functions in such contexts.
>
> This patch takes Laszlo suggestion to fix the issue.
>
> Cc: Jiewen Yao 
> Cc: Chao Zhang 
> Cc: Star Zeng 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau 
> ---
>  .../HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c   | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git 
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c 
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> index dbee0f2531bc..84c1aaae70eb 100644
> --- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> +++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> @@ -397,6 +397,7 @@ HashLibBaseCryptoRouterPeiConstructor (
>  {
>EFI_STATUSStatus;
>HASH_INTERFACE_HOB*HashInterfaceHob;
> +  UINTN ClearStartOffset;
>
>HashInterfaceHob = InternalGetHashInterfaceHob ();
>if (HashInterfaceHob == NULL) {
> @@ -424,7 +425,11 @@ HashLibBaseCryptoRouterPeiConstructor (
>  // This is the second execution of this module, clear the hash interface
>  // information registered at its first execution.
>  //
> -ZeroMem (>HashInterface, sizeof (*HashInterfaceHob) - 
> sizeof (EFI_GUID));
> +ClearStartOffset = OFFSET_OF (HASH_INTERFACE_HOB, HashInterfaceCount);
> +ZeroMem (
> +  (UINT8 *)HashInterfaceHob + ClearStartOffset,
> +  sizeof (*HashInterfaceHob) - ClearStartOffset
> +  );
>}
>
>//
> --
> 2.16.2.346.g9779355e34
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel



-- 
Marc-André Lureau
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH V2 7/7] SecurityPkg OpalPasswordExtraInfoVariable.h: Remove it

[Star Zeng]

Remove OpalPasswordExtraInfoVariable.h as it is not been used
anymore.

Cc: Jiewen Yao 
Cc: Eric Dong 
Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
Reviewed-by: Jiewen Yao 
---
 .../Include/Guid/OpalPasswordExtraInfoVariable.h   | 27 --
 1 file changed, 27 deletions(-)
 delete mode 100644 SecurityPkg/Include/Guid/OpalPasswordExtraInfoVariable.h

diff --git a/SecurityPkg/Include/Guid/OpalPasswordExtraInfoVariable.h 
b/SecurityPkg/Include/Guid/OpalPasswordExtraInfoVariable.h
deleted file mode 100644
index f16d0a4ac360..
--- a/SecurityPkg/Include/Guid/OpalPasswordExtraInfoVariable.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/** @file
-  Defines Name GUIDs to represent an Opal device variable guid for Opal 
Security Feature.
-
-Copyright (c) 2016, 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 _OPAL_PASSWORD_EXTRA_INFO_VARIABLE_H_
-#define _OPAL_PASSWORD_EXTRA_INFO_VARIABLE_H_
-
-#define OPAL_EXTRA_INFO_VAR_NAME L"OpalExtraInfo"
-
-typedef struct {
-  UINT8   EnableBlockSid;
-} OPAL_EXTRA_INFO_VAR;
-
-extern EFI_GUID gOpalExtraInfoVariableGuid;
-
-#endif // _OPAL_PASSWORD_SECURITY_VARIABLE_H_
-
-- 
2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH V2 6/7] SecurityPkg OpalPasswordSupportLib: Remove it

[Star Zeng]

Remove OpalPasswordSupportLib as it is not been used
anymore.

Cc: Jiewen Yao 
Cc: Eric Dong 
Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
Reviewed-by: Jiewen Yao 
---
 .../Include/Library/OpalPasswordSupportLib.h   | 289 
 .../OpalPasswordSupportLib.c   | 781 -
 .../OpalPasswordSupportLib.inf |  55 --
 .../OpalPasswordSupportNotify.h|  55 --
 SecurityPkg/SecurityPkg.dec|   4 -
 SecurityPkg/SecurityPkg.dsc|   2 -
 6 files changed, 1186 deletions(-)
 delete mode 100644 SecurityPkg/Include/Library/OpalPasswordSupportLib.h
 delete mode 100644 
SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportLib.c
 delete mode 100644 
SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportLib.inf
 delete mode 100644 
SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportNotify.h

diff --git a/SecurityPkg/Include/Library/OpalPasswordSupportLib.h 
b/SecurityPkg/Include/Library/OpalPasswordSupportLib.h
deleted file mode 100644
index e616c763f05c..
--- a/SecurityPkg/Include/Library/OpalPasswordSupportLib.h
+++ /dev/null
@@ -1,289 +0,0 @@
-/** @file
-  Header file of Opal password support library.
-
-Copyright (c) 2016, 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 _OPAL_PASSWORD_SUPPORT_LIB_H_
-#define _OPAL_PASSWORD_SUPPORT_LIB_H_
-
-#include 
-#include 
-
-
-#pragma pack(1)
-
-//
-// Structure that is used to represent the available actions for an OpalDisk.
-// The data can then be utilized to expose/hide certain actions available to 
an end user
-// by the consumer of this library.
-//
-typedef struct {
-//
-// Indicates if the disk can support PSID Revert action.  should verify 
disk supports PSID authority
-//
-UINT16 PsidRevert : 1;
-
-//
-// Indicates if the disk can support Revert action
-//
-UINT16 Revert : 1;
-
-//
-// Indicates if the user must keep data for revert action.  It is true if 
no media encryption is supported.
-//
-UINT16 RevertKeepDataForced : 1;
-
-//
-// Indicates if the disk can support set Admin password
-//
-UINT16 AdminPass : 1;
-
-//
-// Indicates if the disk can support set User password.  This action 
requires that a user
-// password is first enabled.
-//
-UINT16 UserPass : 1;
-
-//
-// Indicates if unlock action is available.  Requires disk to be currently 
locked.
-//
-UINT16 Unlock : 1;
-
-//
-// Indicates if Secure Erase action is available.  Action requires admin 
credentials and media encryption support.
-//
-UINT16 SecureErase : 1;
-
-//
-// Indicates if Disable User action is available.  Action requires admin 
credentials.
-//
-UINT16 DisableUser : 1;
-} OPAL_DISK_ACTIONS;
-
-//
-// Structure that is used to represent the Opal device with password info.
-//
-typedef struct {
-  LIST_ENTRY Link;
-
-  UINT8  Password[32];
-  UINT8  PasswordLength;
-
-  EFI_DEVICE_PATH_PROTOCOL   OpalDevicePath;
-} OPAL_DISK_AND_PASSWORD_INFO;
-
-#pragma pack()
-
-/**
-
-  The function performs determines the available actions for the OPAL_DISK 
provided.
-
-  @param[in]   SupportedAttributes   The support attribute for the device.
-  @param[in]   LockingFeatureThe locking status for the device.
-  @param[in]   OwnerShip The ownership for the device.
-  @param[out]  AvalDiskActions   Pointer to fill-out with appropriate disk 
actions.
-
-**/
-TCG_RESULT
-EFIAPI
-OpalSupportGetAvailableActions(
-  IN  OPAL_DISK_SUPPORT_ATTRIBUTE  *SupportedAttributes,
-  IN  TCG_LOCKING_FEATURE_DESCRIPTOR   *LockingFeature,
-  IN  UINT16   OwnerShip,
-  OUT OPAL_DISK_ACTIONS*AvalDiskActions
-  );
-
-/**
-  Enable Opal Feature for the input device.
-
-  @param[in]  SessionThe opal session for the opal device.
-  @param[in]  Msid   Msid
-  @param[in]  MsidLength Msid Length
-  @param[in]  Password   Admin password
-  @param[in]  PassLength Length of password in bytes
-  @param[in]  DevicePath The device path for the opal devcie.
-
-**/
-TCG_RESULT
-EFIAPI
-OpalSupportEnableOpalFeature(
-  IN OPAL_SESSION  *Session,
-  IN VOID 

[edk2] [PATCH V2 0/7] OpalPassword: New solution without SMM device code

[Star Zeng]

The patch series is also at
https://github.com/lzeng14/edk2 OpalPasswordNewV2 branch.

After IOMMU is enabled in S3, original solution with SMM device
code (OpalPasswordSmm) to unlock OPAL device for S3 will not work
as the DMA operation will be aborted without granted DMA buffer.
Instead, this solution is to add OpalPasswordPei to eliminate
SMM device code, and OPAL setup UI produced by OpalPasswordDxe
will be updated to send requests (set password, update password,
and etc), and then the requests will be processed in next boot
before SmmReadyToLock, password and device info will be saved to
lock box used by OpalPasswordPei to unlock OPAL device for S3.

The old solution related codes are also removed.

V2: Thanks for Jiewen's great comments.
1. Still use suppressif for unavailabe requests, and use grayoutif
for conflict requests.
2. Zero DevInfo in BuildOpalDeviceInfoAta() and BuildOpalDeviceInfoNvme().
3. Zero Mask and Unicode in OpalDriverPopUpPsidInput() and
OpalDriverPopUpPasswordInput() even for NULL return path.
4. Do some HII related refinement.
5. Do more NULL pointer check.

Cc: Jiewen Yao 
Cc: Eric Dong 
Cc: Chao Zhang 

Star Zeng (7):
  MdeModulePkg LockBoxLib: Support LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY
  SecurityPkg TcgStorageOpalLib: Make it be base type really
  SecurityPkg TcgStorageCoreLib: Make it be base type really
  SecurityPkg OpalPassword: Add solution without SMM device code
  SecurityPkg OpalPassword: Remove old solution
  SecurityPkg OpalPasswordSupportLib: Remove it
  SecurityPkg OpalPasswordExtraInfoVariable.h: Remove it

 MdeModulePkg/Include/Library/LockBoxLib.h  |   14 +-
 .../Library/SmmLockBoxLib/SmmLockBoxDxeLib.c   |4 +-
 .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.c   |  227 +-
 .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf |   10 +-
 .../Include/Guid/OpalPasswordExtraInfoVariable.h   |   27 -
 .../Include/Library/OpalPasswordSupportLib.h   |  289 --
 .../OpalPasswordSupportLib.c   |  781 -
 .../OpalPasswordSupportLib.inf |   55 -
 .../OpalPasswordSupportNotify.h|   55 -
 .../TcgStorageCoreLib/TcgStorageCoreLib.inf|4 +-
 .../TcgStorageOpalLib/TcgStorageOpalLib.inf|8 +-
 SecurityPkg/SecurityPkg.dec|4 -
 SecurityPkg/SecurityPkg.dsc|6 +-
 .../ComponentName.c|0
 .../OpalAhciMode.c |  492 ++--
 .../OpalAhciMode.h |   93 +-
 SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c | 3003 
 .../{OpalPasswordDxe => OpalPassword}/OpalDriver.h |  215 +-
 .../{OpalPasswordDxe => OpalPassword}/OpalHii.c|  895 ++
 .../OpalHiiPrivate.h => OpalPassword/OpalHii.h}|  150 +-
 .../OpalHiiCallbacks.c |6 +-
 .../OpalHiiFormStrings.uni |   49 +-
 .../OpalHiiFormValues.h|   97 +-
 .../OpalNvmeMode.c |   95 +-
 .../OpalNvmeMode.h |   19 +-
 .../OpalNvmeReg.h  |5 +-
 .../Tcg/Opal/OpalPassword/OpalPasswordCommon.h |   65 +
 .../OpalPasswordDxe.inf|   25 +-
 .../OpalPasswordForm.vfr   |  279 +-
 .../Tcg/Opal/OpalPassword/OpalPasswordPei.c|  940 ++
 .../Tcg/Opal/OpalPassword/OpalPasswordPei.h|  133 +
 .../Tcg/Opal/OpalPassword/OpalPasswordPei.inf  |   63 +
 SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c  | 1091 ---
 .../Tcg/Opal/OpalPasswordDxe/OpalDriverPrivate.h   |  102 -
 SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.h |  146 -
 SecurityPkg/Tcg/Opal/OpalPasswordSmm/OpalIdeMode.c |  767 -
 SecurityPkg/Tcg/Opal/OpalPasswordSmm/OpalIdeMode.h |  173 --
 .../Tcg/Opal/OpalPasswordSmm/OpalPasswordSmm.c | 1088 ---
 .../Tcg/Opal/OpalPasswordSmm/OpalPasswordSmm.h |  299 --
 .../Tcg/Opal/OpalPasswordSmm/OpalPasswordSmm.inf   |   77 -
 40 files changed, 5656 insertions(+), 6195 deletions(-)
 delete mode 100644 SecurityPkg/Include/Guid/OpalPasswordExtraInfoVariable.h
 delete mode 100644 SecurityPkg/Include/Library/OpalPasswordSupportLib.h
 delete mode 100644 
SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportLib.c
 delete mode 100644 
SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportLib.inf
 delete mode 100644 
SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportNotify.h
 rename SecurityPkg/Tcg/Opal/{OpalPasswordDxe => OpalPassword}/ComponentName.c 
(100%)
 rename SecurityPkg/Tcg/Opal/{OpalPasswordSmm => OpalPassword}/OpalAhciMode.c 
(68%)
 rename SecurityPkg/Tcg/Opal/{OpalPasswordSmm => OpalPassword}/OpalAhciMode.h 
(85%)
 create mode 100644 SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
 rename 

[edk2] [PATCH V2 1/7] MdeModulePkg LockBoxLib: Support LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY

[Star Zeng]

With this flag, the LockBox can be restored in S3 resume only.
The LockBox can not be restored after SmmReadyToLock in normal boot
and after EndOfS3Resume in S3 resume.
It can not be set together with LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE.

Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
Reviewed-by: Jiewen Yao 
---
 MdeModulePkg/Include/Library/LockBoxLib.h  |  14 +-
 .../Library/SmmLockBoxLib/SmmLockBoxDxeLib.c   |   4 +-
 .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.c   | 227 -
 .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf |  10 +-
 4 files changed, 247 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Include/Library/LockBoxLib.h 
b/MdeModulePkg/Include/Library/LockBoxLib.h
index db7fd05def58..80beb4d0f880 100644
--- a/MdeModulePkg/Include/Library/LockBoxLib.h
+++ b/MdeModulePkg/Include/Library/LockBoxLib.h
@@ -2,7 +2,7 @@
   This library is only intended to be used by DXE modules that need save
   confidential information to LockBox and get it by PEI modules in S3 phase.
 
-Copyright (c) 2010 - 2011, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions
@@ -62,9 +62,17 @@ SetLockBoxAttributes (
   );
 
 //
-// With this flag, this LockBox can be restored to this Buffer with 
RestoreAllLockBoxInPlace()
+// With this flag, this LockBox can be restored to this Buffer
+// with RestoreAllLockBoxInPlace()
 //
-#define LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE  BIT0
+#define LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE BIT0
+//
+// With this flag, this LockBox can be restored in S3 resume only.
+// This LockBox can not be restored after SmmReadyToLock in normal boot
+// and after EndOfS3Resume in S3 resume.
+// It can not be set together with LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE.
+//
+#define LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY   BIT1
 
 /**
   This function will update confidential information to lockbox.
diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
index b75f81e69e04..9b6f0bedbd4f 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
@@ -1,6 +1,6 @@
 /** @file
 
-Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions
@@ -241,7 +241,7 @@ SetLockBoxAttributes (
   // Basic check
   //
   if ((Guid == NULL) ||
-  ((Attributes & ~LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE) != 0)) {
+  ((Attributes & ~(LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE | 
LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY)) != 0)) {
 return EFI_INVALID_PARAMETER;
   }
 
diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
index 4960df755534..af75a4cb9cd1 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
@@ -1,6 +1,6 @@
 /** @file
 
-Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions
@@ -20,6 +20,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
+#include 
 
 #include "SmmLockBoxLibPrivate.h"
 
@@ -31,6 +35,11 @@ SMM_LOCK_BOX_CONTEXT mSmmLockBoxContext;
 LIST_ENTRY   mLockBoxQueue = INITIALIZE_LIST_HEAD_VARIABLE 
(mLockBoxQueue);
 
 BOOLEAN  mSmmConfigurationTableInstalled = FALSE;
+VOID *mRegistrationSmmEndOfDxe = NULL;
+VOID *mRegistrationSmmReadyToLock = NULL;
+VOID *mRegistrationEndOfS3Resume = NULL;
+BOOLEAN  mSmmLockBoxSmmReadyToLock = FALSE;
+BOOLEAN  mSmmLockBoxDuringS3Resume = FALSE;
 
 /**
   This function return SmmLockBox context from SMST.
@@ -64,6 +73,128 @@ InternalGetSmmLockBoxContext (
 }
 
 /**
+  Notification for SMM ReadyToLock protocol.
+
+  @param[in] Protocol   Points to the protocol's unique identifier.
+  @param[in] Interface  Points to the interface instance.
+  @param[in] Handle The handle on which the interface was installed.
+
+  @retval EFI_SUCCESS   Notification runs successfully.
+**/
+EFI_STATUS
+EFIAPI
+SmmLockBoxSmmReadyToLockNotify (
+  IN CONST EFI_GUID  *Protocol,
+  IN VOID*Interface,
+  IN EFI_HANDLE  Handle
+  )
+{
+  mSmmLockBoxSmmReadyToLock = TRUE;
+  return EFI_SUCCESS;
+}
+
+/**
+  Main entry point for an SMM 

[edk2] [PATCH V2 3/7] SecurityPkg TcgStorageCoreLib: Make it be base type really

[Star Zeng]

Cc: Jiewen Yao 
Cc: Eric Dong 
Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
Reviewed-by: Jiewen Yao 
---
 SecurityPkg/Library/TcgStorageCoreLib/TcgStorageCoreLib.inf | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/SecurityPkg/Library/TcgStorageCoreLib/TcgStorageCoreLib.inf 
b/SecurityPkg/Library/TcgStorageCoreLib/TcgStorageCoreLib.inf
index a80cb00a9ce3..78a0557bfe42 100644
--- a/SecurityPkg/Library/TcgStorageCoreLib/TcgStorageCoreLib.inf
+++ b/SecurityPkg/Library/TcgStorageCoreLib/TcgStorageCoreLib.inf
@@ -3,7 +3,7 @@
 #
 #  This module is used to provide API used by Opal password solution.
 #
-# Copyright (c) 2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
 # This program and the accompanying materials
 # are licensed and made available under the terms and conditions of the BSD 
License
 # which accompanies this distribution. The full text of the license may be 
found at
@@ -18,7 +18,7 @@ [Defines]
   FILE_GUID  = ad63b09b-1fc9-4789-af0c-5af8a3fb1f9c
   VERSION_STRING = 1.0
   MODULE_TYPE= BASE
-  LIBRARY_CLASS  = TcgStorageCoreLib|DXE_DRIVER DXE_CORE 
DXE_SMM_DRIVER
+  LIBRARY_CLASS  = TcgStorageCoreLib
 #
 # The following information is for reference only and not required by the 
build tools.
 #
-- 
2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH V2 2/7] SecurityPkg TcgStorageOpalLib: Make it be base type really

[Star Zeng]

Cc: Jiewen Yao 
Cc: Eric Dong 
Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
Reviewed-by: Jiewen Yao 
---
 SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf 
b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf
index ec26111ee2c3..78e47387a981 100644
--- a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf
+++ b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf
@@ -3,7 +3,7 @@
 #
 #  This module is used to provide API used by Opal password solution.
 #
-# Copyright (c) 2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
 # This program and the accompanying materials
 # are licensed and made available under the terms and conditions of the BSD 
License
 # which accompanies this distribution. The full text of the license may be 
found at
@@ -18,7 +18,7 @@ [Defines]
   FILE_GUID  = F8B56221-FD5D-4215-B578-C3574AD1E253
   VERSION_STRING = 1.0
   MODULE_TYPE= BASE
-  LIBRARY_CLASS  = TcgStorageOpalLib|DXE_DRIVER DXE_CORE 
DXE_SMM_DRIVER
+  LIBRARY_CLASS  = TcgStorageOpalLib
 
 #
 # The following information is for reference only and not required by the 
build tools.
@@ -37,11 +37,7 @@ [LibraryClasses]
   DebugLib
   TimerLib
   TcgStorageCoreLib
-  UefiLib
 
 [Packages]
   MdePkg/MdePkg.dec
   SecurityPkg/SecurityPkg.dec
-
-[Protocols]
-  gEfiStorageSecurityCommandProtocolGuid ## CONSUMES
-- 
2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 1/1] SecurityPkg: fix ZeroMem HashInterfaceHob

[marcandre . lureau]

From: Marc-André Lureau 

The ZeroMem() call goes beyond the HashInterfaceHob structure, causing
HOB list corruption. The intent was to clear all but the Identifier,
that is starting from HashInterfaceCount.

Quoting Laszlo Ersek:
  Therefore I think the *first* approach to actually fixing this bug would
  be to simply add the "Count" suffix:

  > diff --git 
a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c 
b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
  > index dbee0f2531bc..a869fffae3fe 100644
  > --- 
a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
  > +++ 
b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
  > @@ -424,7 +424,10 @@ HashLibBaseCryptoRouterPeiConstructor (
  >  // This is the second execution of this module, clear the hash 
interface
  >  // information registered at its first execution.
  >  //
  > -ZeroMem (>HashInterface, sizeof (*HashInterfaceHob) 
- sizeof (EFI_GUID));
  > +ZeroMem (
  > +  >HashInterfaceCount,
  > +  sizeof (*HashInterfaceHob) - sizeof (EFI_GUID)
  > +  );
  >}
  >
  >//

  On the other hand, I don't think such a fix would be great. First of
  all, the HASH_INTERFACE_HOB struct definition is not wrapped in

  #pragma pack (1)
  ...
  #pragma pack ()

  in other words, HASH_INTERFACE_HOB is not packed, hence there could be
  an unspecified amount of padding between "Identifier" and
  "HashInterfaceCount". That would lead to the same kind of buffer
  overflow, because "Length" includes the padding, but the "Buffer"
  argument doesn't.

  Second, even if there were no padding (and thus the byte count would be
  a perfect match for the full HOB structure), in the "Buffer" argument
  we take the address of the "HashInterfaceCount" field, and with the
  "Length" field, we still overflow that *individual* field. Although we
  wouldn't overflow the containing HOB structure, this is still (arguably)
  undefined behavior -- I seem to remember that some compilers actually
  blow up on this when you use the standard ISO C memcpy() or memset()
  functions in such contexts.

This patch takes Laszlo suggestion to fix the issue.

Cc: Jiewen Yao 
Cc: Chao Zhang 
Cc: Star Zeng 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marc-André Lureau 
---
 .../HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c   | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git 
a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c 
b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
index dbee0f2531bc..84c1aaae70eb 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
@@ -397,6 +397,7 @@ HashLibBaseCryptoRouterPeiConstructor (
 {
   EFI_STATUSStatus;
   HASH_INTERFACE_HOB*HashInterfaceHob;
+  UINTN ClearStartOffset;
 
   HashInterfaceHob = InternalGetHashInterfaceHob ();
   if (HashInterfaceHob == NULL) {
@@ -424,7 +425,11 @@ HashLibBaseCryptoRouterPeiConstructor (
 // This is the second execution of this module, clear the hash interface
 // information registered at its first execution.
 //
-ZeroMem (>HashInterface, sizeof (*HashInterfaceHob) - 
sizeof (EFI_GUID));
+ClearStartOffset = OFFSET_OF (HASH_INTERFACE_HOB, HashInterfaceCount);
+ZeroMem (
+  (UINT8 *)HashInterfaceHob + ClearStartOffset,
+  sizeof (*HashInterfaceHob) - ClearStartOffset
+  );
   }
 
   //
-- 
2.16.2.346.g9779355e34

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/4] Virtio non-discoverable devices

[Laszlo Ersek]

Hi Daniil,

On 03/07/18 02:36, Daniil Egranov wrote:
> This is an attempt to add MMIO Virtio devices into the
> non-discoverable device registration procedure and allow
> Virtio PCI drivers to recognize and program such devices 
> correctly.
> The main issue is that the set of MMIO registers is different
> from PCI, plus the width of similar registers are not 
> always the same. The code implements the translation of 
> the PCI IO registers to MMIO registers. 
> Another difference between PCI and MMIO Virtio devices found 
> during the testing is that MMIO devices may require more 
> registers to be programmed compared to PCI. The VirtioPciDeviceDxe
> was patched to detect non-discoverable MMIO devices and allow
> calling a PCI MemIo protocol function.
> 
> This set of patches was tested with MMIO Virtio Block and
> Virtio Net devices.

I'm commenting on this series because of patch 4/4, which is for OvmfPkg.

OvmfPkg supports the following virtio transports:

- virtio-pci, spec version 0.9.5 ("legacy"):

  OvmfPkg/VirtioPciDeviceDxe consumes EFI_PCI_IO_PROTOCOL,
  and produces VIRTIO_DEVICE_PROTOCOL;

- virtio-pci, spec version 1.0 ("modern"):

  OvmfPkg/Virtio10Dxe consumes EFI_PCI_IO_PROTOCOL and produces
  VIRTIO_DEVICE_PROTOCOL;

- virtio-mmio, spec version 0.9.5 ("legacy"):

  OvmfPkg/Library/VirtioMmioDeviceLib takes (a) an EFI_HANDLE that
  carries a vendor-specific device path protocol instance, (b) the base
  address of the virtio-mmio transport (register block), and produces
  VIRTIO_DEVICE_PROTOCOL.

The individual virtio device drivers under OvmfPkg -- such as
VirtioBlkDxe, VirtioGpuDxe, VirtioNetDxe, VirtioRngDxe and VirtioScsiDxe
-- consume VIRTIO_DEVICE_PROTOCOL, and produce the corresponding (device
type specific) UEFI protocols on top. They perform a *union* of the
steps that are required for generic device configuration over either
virtio transport (going through VIRTIO_DEVICE_PROTOCOL), and the
transport drivers take care of mapping the actions to the actual
transports. In some cases, this means simply ignoring an action (because
it has no mapping defined for the given transport type).

Now, this patch set:

(1) extends NonDiscoverableDeviceRegistrationLib, so that a platform
DXE_DRIVER can register a new kind of non-discoverable device,

(2) extends the NonDiscoverablePciDeviceDxe PCI emulation driver in
MdeModulePkg to recognize the new kind of device.

However: the PciIo protocol instances that are consequently produced
represent virtio devices that do *not* conform to the PCI binding of
either virtio specification (0.9.5 or 1.0). Namely,

- The QueueAlign register (at offset 0x3C in the MMIO register block)
  and the GuestPageSize register (at offset 0x28) are only defined for
  the virtio-mmio binding, spec version 0.9.5 ("legacy").

- The QueueNum register (at offset 0x38) is:

  - defined in the virtio-mmio binding, spec version 0.9.5 ("legacy"),

  - defined in the virtio-mmio binding, spec version 1.0 ("modern"),

  - "sort of defined" in the virtio-pci binding, spec version 1.0
only ("modern" only). (By "sort of defined", I mean the fact that
the "queue_size" register of the PCI binding is read-write in spec
version 1.0.)

Therefore adding any actual handling for these registers to
VirtioPciDeviceDxe, which implements the 0.9.5 PCI binding, is wrong.

If your platform provides a virtio device over an MMIO transport, then
the right way to expose the device to the firmware is *not* to

(a) simulate a PciIo interface that does not conform to the PCI binding
of the virtio-0.9.5 spec,

(b) then patch that shortcoming up in VirtioPciDeviceDxe, which already
conforms to the PCI binding of the virtio-0.9.5 spec.

Instead, the right way is to use "OvmfPkg/Library/VirtioMmioDeviceLib"
in a platform driver, for producing VIRTIO_DEVICE_PROTOCOL instances on
top of the virtio-mmio transports (MMIO register blocks) directly.

You must already have a platform DXE_DRIVER that produces the
non-discoverable device protocol instances described in (1). I suggest
that you modify that driver to use VirtioMmioDeviceLib instead:
enumerate the same set of virtio-mmio transports that you must already
be doing, and call VirtioMmioInstallDevice() on each. (You can see an
example in "ArmVirtPkg/VirtioFdtDxe".)

This will give you VIRTIO_DEVICE_PROTOCOL instances right off the bat
that can be driven by VirtioBlkDxe, VirtioGpuDxe, etc.

Thanks,
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/1] RFC: SecurityPkg: only clear HashInterface informations

[Marc-André Lureau]

Hi

On Wed, Mar 7, 2018 at 10:06 AM, Laszlo Ersek  wrote:
> Hi Marc-André,
>
> On 03/06/18 21:27, marcandre.lur...@redhat.com wrote:
>> From: Marc-André Lureau 
>>
>> The ZeroMem() call goes beyond the HashInterfaceHob structure, causing
>> HOB list corruption. Instead, just clear the HashInterface fields, as
>> I suppose was originally intended.
>>
>> Cc: Jiewen Yao 
>> Cc: Chao Zhang 
>> Cc: Star Zeng 
>> Cc: Laszlo Ersek 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marc-André Lureau 
>> ---
>>  .../HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c   | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git 
>> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c 
>> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
>> index dbee0f2531bc..361a4f6508a0 100644
>> --- 
>> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
>> +++ 
>> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
>> @@ -424,7 +424,8 @@ HashLibBaseCryptoRouterPeiConstructor (
>>  // This is the second execution of this module, clear the hash interface
>>  // information registered at its first execution.
>>  //
>> -ZeroMem (>HashInterface, sizeof (*HashInterfaceHob) - 
>> sizeof (EFI_GUID));
>> +ZeroMem (>HashInterface, sizeof 
>> (HashInterfaceHob->HashInterface));
>> +HashInterfaceHob->HashInterfaceCount = 0;
>>}
>>
>>//
>>
>
> Excellent catch! :)
>
> I do have a question however, for the other reviewers on this patch, in
> line with your commit message saying, "clear the HashInterface fields,
> as *I suppose* was originally intended".
>
> I don't think the proposed update matches the original intent 100%.
> Namely, this is the definition of the HASH_INTERFACE_HOB type (from the
> same source file):
>
>> typedef struct {
>>   //
>>   // If gZeroGuid, SupportedHashMask is 0 for FIRST module which consumes 
>> HashLib
>>   //   or the hash algorithm bitmap of LAST module which consumes HashLib.
>>   //   HashInterfaceCount and HashInterface are all 0.
>>   // If gEfiCallerIdGuid, HashInterfaceCount, HashInterface and 
>> SupportedHashMask
>>   //   are the hash interface information of CURRENT module which consumes 
>> HashLib.
>>   //
>>   EFI_GUID Identifier;
>>   UINTNHashInterfaceCount;
>>   HASH_INTERFACE   HashInterface[HASH_COUNT];
>>   UINT32   SupportedHashMask;
>> } HASH_INTERFACE_HOB;
>
> Note that it says, if Identifier equals "gEfiCallerIdGuid", then *all
> three* subsequent fields carry information of the current module that
> consumes HashLib. Those three fields include "SupportedHashMask".
>
> Furthermore, the comment just preceding the bug / ZeroMem() call says,
>
>> //
>> // In PEI phase, some modules may call RegisterForShadow and will be
>> // shadowed and executed again after memory is discovered.
>> // This is the second execution of this module, clear the hash interface
>> // information registered at its first execution.
>> //
>
> And note that here we are just past the check
>
>>   HashInterfaceHob = InternalGetHashInterfaceHob ();
>>   if (HashInterfaceHob != NULL) {
>
> in other words, the "Identifier equals gEfiCallerIdGuid" branch from the
> type definition applies; meaning that "SupportedHashMask" is defined
> too.
>
> Thus, I'd like the other reviewers to confirm whether we should clear
> "SupportedHashMask".
>
> From the original (buggy) code, I think we *should* clear
> SupportedHashMask as well. The "Length" argument of the original
> ZeroMem() call implies precisely that:
>
>   sizeof (*HashInterfaceHob) - sizeof (EFI_GUID)
>
> This stands for "all fields in *HashInterfaceHob except the leading
> EFI_GUID Identifier field".
>
> So, the actual bug is in the "Buffer" argument of the ZeroMem() call: it
> is a typo. Certainly the intent must have been "start clearing from the
> first field right after the EFI_GUID Identifier field", namely
> "HashInterfaceCount":
>
>   >HashInterfaceCount
>   ^
>
> Therefore I think the *first* approach to actually fixing this bug would
> be to simply add the "Count" suffix:
>
>> diff --git 
>> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c 
>> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
>> index dbee0f2531bc..a869fffae3fe 100644
>> --- 
>> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
>> +++ 
>> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
>> @@ -424,7 +424,10 @@ HashLibBaseCryptoRouterPeiConstructor (
>>  // This is the second execution of this module, clear the hash interface
>>  // information registered at its first 

Re: [edk2] [PATCH 1/1] RFC: SecurityPkg: only clear HashInterface informations

[Zeng, Star]

Hi Laszlo,

Good analysis.

Yes, the SupportedHashMask field in HashInterfaceHob will have stale value, but 
that does not impact the functionality since the code have.
So the patch could fix the problem.

HashLibBaseCryptoRouterPeiConstructor():
  Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, 0);

RegisterHashInterfaceLib():
  HashInterfaceHob->SupportedHashMask = PcdGet32 (PcdTcg2HashAlgorithmBitmap) | 
HashMask;

HashStart()/HashUpdate()/HashCompleteAndExtend()/HashAndExtend():
  if (HashInterfaceHob->HashInterfaceCount == 0) {
return EFI_UNSUPPORTED;
  }

RegisterHashInterfaceLib():
  if (HashInterfaceHob->HashInterfaceCount >= HASH_COUNT) {
return EFI_OUT_OF_RESOURCES;
  }

As I know, Chao has helped push the patch.
But I am fine with keeping this patch or continue refining the code. :)

Is it working with " ZeroMem (HashInterfaceHob, sizeof (*HashInterfaceHob) - 
sizeof (EFI_GUID)) " if to refine the code?


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Wednesday, March 7, 2018 5:06 PM
To: marcandre.lur...@redhat.com
Cc: edk2-devel@lists.01.org; Yao, Jiewen ; Zeng, Star 
; Zhang, Chao B 
Subject: Re: [edk2] [PATCH 1/1] RFC: SecurityPkg: only clear HashInterface 
informations

Hi Marc-André,

On 03/06/18 21:27, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
>
> The ZeroMem() call goes beyond the HashInterfaceHob structure, causing 
> HOB list corruption. Instead, just clear the HashInterface fields, as 
> I suppose was originally intended.
>
> Cc: Jiewen Yao 
> Cc: Chao Zhang 
> Cc: Star Zeng 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau 
> ---
>  .../HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c   | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git 
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterP
> ei.c 
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterP
> ei.c index dbee0f2531bc..361a4f6508a0 100644
> --- 
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterP
> ei.c
> +++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRou
> +++ terPei.c
> @@ -424,7 +424,8 @@ HashLibBaseCryptoRouterPeiConstructor (
>  // This is the second execution of this module, clear the hash interface
>  // information registered at its first execution.
>  //
> -ZeroMem (>HashInterface, sizeof (*HashInterfaceHob) - 
> sizeof (EFI_GUID));
> +ZeroMem (>HashInterface, sizeof 
> (HashInterfaceHob->HashInterface));
> +HashInterfaceHob->HashInterfaceCount = 0;
>}
>
>//
>

Excellent catch! :)

I do have a question however, for the other reviewers on this patch, in line 
with your commit message saying, "clear the HashInterface fields, as *I 
suppose* was originally intended".

I don't think the proposed update matches the original intent 100%.
Namely, this is the definition of the HASH_INTERFACE_HOB type (from the same 
source file):

> typedef struct {
>   //
>   // If gZeroGuid, SupportedHashMask is 0 for FIRST module which consumes 
> HashLib
>   //   or the hash algorithm bitmap of LAST module which consumes HashLib.
>   //   HashInterfaceCount and HashInterface are all 0.
>   // If gEfiCallerIdGuid, HashInterfaceCount, HashInterface and 
> SupportedHashMask
>   //   are the hash interface information of CURRENT module which consumes 
> HashLib.
>   //
>   EFI_GUID Identifier;
>   UINTNHashInterfaceCount;
>   HASH_INTERFACE   HashInterface[HASH_COUNT];
>   UINT32   SupportedHashMask;
> } HASH_INTERFACE_HOB;

Note that it says, if Identifier equals "gEfiCallerIdGuid", then *all
three* subsequent fields carry information of the current module that consumes 
HashLib. Those three fields include "SupportedHashMask".

Furthermore, the comment just preceding the bug / ZeroMem() call says,

> //
> // In PEI phase, some modules may call RegisterForShadow and will be
> // shadowed and executed again after memory is discovered.
> // This is the second execution of this module, clear the hash interface
> // information registered at its first execution.
> //

And note that here we are just past the check

>   HashInterfaceHob = InternalGetHashInterfaceHob ();
>   if (HashInterfaceHob != NULL) {

in other words, the "Identifier equals gEfiCallerIdGuid" branch from the type 
definition applies; meaning that "SupportedHashMask" is defined too.

Thus, I'd like the other reviewers to confirm whether we should clear 
"SupportedHashMask".

From the original (buggy) code, I think we *should* clear SupportedHashMask as 
well. The "Length" argument of the original
ZeroMem() 

Re: [edk2] [PATCH 1/3] Hisilicon/D0x: Set ACPI GTDT always-on flag

[Laszlo Ersek]

Hello Heyi,

On 03/07/18 07:55, Heyi Guo wrote:
> From: Jason Zhang 
> 
> Timer is always working on Hisilicon D0x, even system enters WFI/WFE,
> and there is no other low power status, so we set "always-on" flag in
> ACPI GTDT.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jason Zhang 
> Signed-off-by: Heyi Guo 
> Cc: Ard Biesheuvel 
> Cc: Leif Lindholm 
> Cc: Graeme Gregory 
> ---
>  Silicon/Hisilicon/Hi1610/Hi1610AcpiTables/Gtdt.aslc | 3 ++-
>  Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc| 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)

this is for edk2-platforms, isn't it?

If so, next time please add "edk2-platforms" to the subject prefix, as in:

  [PATCH edk2-platforms]

or

  [edk2-platforms PATCH]

Thanks!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] BaseTools/header.makefile: revert gcc-8 "-Wno-xxx" options on OSX

[Laszlo Ersek]

I recently added the gcc-8 specific "-Wno-stringop-truncation" and
"-Wno-restrict" options to BUILD_CFLAGS, both for "Darwin" (XCODE5 /
clang, OSX) and otherwise (gcc, Linux / Cygwin).

I also regression-tested the change with gcc-4.8 on Linux -- gcc-4.8 does
not know either of the (gcc-8 specific) "-Wno-stringop-truncation" and
"-Wno-restrict" options, yet the build completed fine (by GCC design).

Regarding OSX, my expectation was that

- XCODE5 / clang would either recognize these warnings options (because
  clang does recognize most -W options of gcc),

- or, similarly to gcc, clang would simply ignore the "-Wno-xxx" flags
  that it didn't recognize.

Neither is the case; the new flags have broken the BaseTools build on OSX.
Revert them (for OSX only).

Cc: Liming Gao 
Cc: Yonghong Zhu 
Reported-by: Liming Gao 
Fixes: 1d212a83df0eaf32a6f5d4159beb2d77832e0231
Fixes: 9222154ae7b3eef75ae88cdb56158256227cb929
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 BaseTools/Source/C/Makefiles/header.makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/Makefiles/header.makefile 
b/BaseTools/Source/C/Makefiles/header.makefile
index 065a998bf5de..db436773cf40 100644
--- a/BaseTools/Source/C/Makefiles/header.makefile
+++ b/BaseTools/Source/C/Makefiles/header.makefile
@@ -71,7 +71,7 @@ INCLUDE = $(TOOL_INCLUDE) -I $(MAKEROOT) -I 
$(MAKEROOT)/Include/Common -I $(MAKE
 BUILD_CPPFLAGS = $(INCLUDE) -O2
 ifeq ($(DARWIN),Darwin)
 # assume clang or clang compatible flags on OS X
-BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror 
-Wno-deprecated-declarations -Wno-stringop-truncation -Wno-restrict 
-Wno-self-assign -Wno-unused-result -nostdlib -c -g
+BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror 
-Wno-deprecated-declarations -Wno-self-assign -Wno-unused-result -nostdlib -c -g
 else
 BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror 
-Wno-deprecated-declarations -Wno-stringop-truncation -Wno-restrict 
-Wno-unused-result -nostdlib -c -g
 endif
-- 
2.14.1.3.gb7cf6e02401b

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/3] BaseTools: let the C-language build utils compile with gcc-8

[Ard Biesheuvel]

On 7 March 2018 at 09:11, Laszlo Ersek  wrote:
> On 03/07/18 07:43, Gao, Liming wrote:
>> Laszlo:
>>   We just find this change causes XCODE5 build failure, because XCODE 
>> compiler doesn't know these two options. So, could you provide the patch to 
>> remove the change in BUILD_CFLAGS for MAC?
>
> Yep, I was a bit concerned about that.
>
> Ultimately I assumed that
> - clang would either recognize these warnings options (because it does
>   recognize most -W options of gcc),
> - or, similarly to gcc, clang would simply ignore -Wno-* flags that it
>   does not recognize.
>
> Neither appears to be the case, then. I'll send a patch soon.
>

What is the reason we don't pass -Wno-unknown-warning-option to XCODE ?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/3] BaseTools: let the C-language build utils compile with gcc-8

[Laszlo Ersek]

On 03/07/18 07:43, Gao, Liming wrote:
> Laszlo:
>   We just find this change causes XCODE5 build failure, because XCODE 
> compiler doesn't know these two options. So, could you provide the patch to 
> remove the change in BUILD_CFLAGS for MAC?

Yep, I was a bit concerned about that.

Ultimately I assumed that
- clang would either recognize these warnings options (because it does
  recognize most -W options of gcc),
- or, similarly to gcc, clang would simply ignore -Wno-* flags that it
  does not recognize.

Neither appears to be the case, then. I'll send a patch soon.

Thanks, and sorry about the regression.
Laszlo

> ifeq ($(DARWIN),Darwin)
> # assume clang or clang compatible flags on OS X
> BUILD_CFLAGS =
> else
> 
> Thanks
> Liming
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Gao, 
>> Liming
>> Sent: Monday, March 5, 2018 10:14 PM
>> To: Laszlo Ersek ; edk2-devel-01 
>> Cc: Paolo Bonzini ; Cole Robinson 
>> ; Ard Biesheuvel 
>> Subject: Re: [edk2] [PATCH 0/3] BaseTools: let the C-language build utils 
>> compile with gcc-8
>>
>> Reviewed-by: Liming Gao 
>>
>>> -Original Message-
>>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>>> Sent: Saturday, March 03, 2018 2:09 AM
>>> To: edk2-devel-01 
>>> Cc: Ard Biesheuvel ; Cole Robinson
>>> ; Gao, Liming ; Paolo Bonzini
>>> ; Zhu, Yonghong 
>>> Subject: [PATCH 0/3] BaseTools: let the C-language build utils compile with
>>> gcc-8
>>>
>>> Repo:   https://github.com/lersek/edk2.git
>>> Branch: basetools_gcc8
>>>
>>> Once these patches are applied to the build flags and the source code of
>>> the build utilities themselves, OVMF builds fine with gcc-8, using the
>>> GCC5 toolchain settings without any changes.
>>>
>>> Regression-tested with gcc-4.8 / x86_64.
>>>
>>> Cc: Ard Biesheuvel 
>>> Cc: Cole Robinson 
>>> Cc: Liming Gao 
>>> Cc: Paolo Bonzini 
>>> Cc: Yonghong Zhu 
>>>
>>> Thanks
>>> Laszlo
>>>
>>> Laszlo Ersek (3):
>>>  BaseTools/header.makefile: add "-Wno-stringop-truncation"
>>>  BaseTools/header.makefile: add "-Wno-restrict"
>>>  BaseTools/GenVtf: silence false "stringop-overflow" warning with
>>>memcpy()
>>>
>>> BaseTools/Source/C/Makefiles/header.makefile | 4 ++--
>>> BaseTools/Source/C/GenVtf/GenVtf.c   | 4 ++--
>>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> --
>>> 2.14.1.3.gb7cf6e02401b
>>
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/1] RFC: SecurityPkg: only clear HashInterface informations

[Laszlo Ersek]

Hi Marc-André,

On 03/06/18 21:27, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
>
> The ZeroMem() call goes beyond the HashInterfaceHob structure, causing
> HOB list corruption. Instead, just clear the HashInterface fields, as
> I suppose was originally intended.
>
> Cc: Jiewen Yao 
> Cc: Chao Zhang 
> Cc: Star Zeng 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau 
> ---
>  .../HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c   | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git 
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c 
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> index dbee0f2531bc..361a4f6508a0 100644
> --- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> +++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> @@ -424,7 +424,8 @@ HashLibBaseCryptoRouterPeiConstructor (
>  // This is the second execution of this module, clear the hash interface
>  // information registered at its first execution.
>  //
> -ZeroMem (>HashInterface, sizeof (*HashInterfaceHob) - 
> sizeof (EFI_GUID));
> +ZeroMem (>HashInterface, sizeof 
> (HashInterfaceHob->HashInterface));
> +HashInterfaceHob->HashInterfaceCount = 0;
>}
>
>//
>

Excellent catch! :)

I do have a question however, for the other reviewers on this patch, in
line with your commit message saying, "clear the HashInterface fields,
as *I suppose* was originally intended".

I don't think the proposed update matches the original intent 100%.
Namely, this is the definition of the HASH_INTERFACE_HOB type (from the
same source file):

> typedef struct {
>   //
>   // If gZeroGuid, SupportedHashMask is 0 for FIRST module which consumes 
> HashLib
>   //   or the hash algorithm bitmap of LAST module which consumes HashLib.
>   //   HashInterfaceCount and HashInterface are all 0.
>   // If gEfiCallerIdGuid, HashInterfaceCount, HashInterface and 
> SupportedHashMask
>   //   are the hash interface information of CURRENT module which consumes 
> HashLib.
>   //
>   EFI_GUID Identifier;
>   UINTNHashInterfaceCount;
>   HASH_INTERFACE   HashInterface[HASH_COUNT];
>   UINT32   SupportedHashMask;
> } HASH_INTERFACE_HOB;

Note that it says, if Identifier equals "gEfiCallerIdGuid", then *all
three* subsequent fields carry information of the current module that
consumes HashLib. Those three fields include "SupportedHashMask".

Furthermore, the comment just preceding the bug / ZeroMem() call says,

> //
> // In PEI phase, some modules may call RegisterForShadow and will be
> // shadowed and executed again after memory is discovered.
> // This is the second execution of this module, clear the hash interface
> // information registered at its first execution.
> //

And note that here we are just past the check

>   HashInterfaceHob = InternalGetHashInterfaceHob ();
>   if (HashInterfaceHob != NULL) {

in other words, the "Identifier equals gEfiCallerIdGuid" branch from the
type definition applies; meaning that "SupportedHashMask" is defined
too.

Thus, I'd like the other reviewers to confirm whether we should clear
"SupportedHashMask".

From the original (buggy) code, I think we *should* clear
SupportedHashMask as well. The "Length" argument of the original
ZeroMem() call implies precisely that:

  sizeof (*HashInterfaceHob) - sizeof (EFI_GUID)

This stands for "all fields in *HashInterfaceHob except the leading
EFI_GUID Identifier field".

So, the actual bug is in the "Buffer" argument of the ZeroMem() call: it
is a typo. Certainly the intent must have been "start clearing from the
first field right after the EFI_GUID Identifier field", namely
"HashInterfaceCount":

  >HashInterfaceCount
  ^

Therefore I think the *first* approach to actually fixing this bug would
be to simply add the "Count" suffix:

> diff --git 
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c 
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> index dbee0f2531bc..a869fffae3fe 100644
> --- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> +++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> @@ -424,7 +424,10 @@ HashLibBaseCryptoRouterPeiConstructor (
>  // This is the second execution of this module, clear the hash interface
>  // information registered at its first execution.
>  //
> -ZeroMem (>HashInterface, sizeof (*HashInterfaceHob) - 
> sizeof (EFI_GUID));
> +ZeroMem (
> +  >HashInterfaceCount,
> +  sizeof (*HashInterfaceHob) - sizeof (EFI_GUID)
> +  );
>}
>
>//

On the 

Re: [edk2] [Patch V2] Build spec: update PCD value and SKU, DefaultStore info in report

[Gao, Liming]

Reviewed-by: Liming Gao 

>-Original Message-
>From: Zhu, Yonghong
>Sent: Friday, February 23, 2018 1:21 PM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming ; Kinney, Michael D
>; Shaw, Kevin W 
>Subject: [Patch V2] Build spec: update PCD value and SKU, DefaultStore info in
>report
>
>V2: Add *B info for structure pcd field value that from build command
>
>Cc: Liming Gao 
>Cc: Michael Kinney 
>Cc: Kevin W Shaw 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Yonghong Zhu 
>---
> 13_build_reports/134_platform_summary.md   | 10 --
> 13_build_reports/136_global_pcd_section.md | 49
>--
> 13_build_reports/138_module_section.md |  9 +-
> 3 files changed, 63 insertions(+), 5 deletions(-)
>
>diff --git a/13_build_reports/134_platform_summary.md
>b/13_build_reports/134_platform_summary.md
>index 083d714..09b8db6 100644
>--- a/13_build_reports/134_platform_summary.md
>+++ b/13_build_reports/134_platform_summary.md
>@@ -37,11 +37,13 @@ following items:
> * Platform Name : %Platform UI name: '`PLATFORM_NAME`' in DSC
>`[Defines]`
>   section%
> * Platform DSC Path: %Path of platform DSC file%
> * Architectures : %List string of all architectures used in build%
> * Tool Chain : %Tool chain string%
>-* Target : %Target String"
>+* Target : %Target String%
>+* SKUID: %Platform SKUID String%
>+* DefaultStore: %Platform DefaultStore String%
> * Output Path : %Path to platform build directory%
> * Build Environment : %Environment string reported by Python%
> * Build Duration : %Build duration time string%
> * AutoGen Duration : %AutoGen duration time string if it exists%
> * Make Duration : %Make duration time string if it exists%
>@@ -54,10 +56,12 @@ following items:
> Platform Name:  NT32
> Platform DSC Path:  s:\edk2\Nt32Pkg\Nt32Pkg.dsc
> Architectures:  IA32
> Tool Chain: VS2008x86
> Target: DEBUG
>+SKUID:  DEFAULT
>+DefaultStore:   STANDARD
> Output Path:s:\edk2\Build\NT32IA32
> Build Environment:  Windows-7-6.1.7601-SP1
> Build Duration: 00:01:29
> AutoGen Duration:   00:00:10
> Make Duration:  00:01:02
>@@ -104,10 +108,11 @@ The first line is required:
> `[*P|*F|*B] :  () = `
>
> * `*P` means the Pcd's value was obtained from the DSC file
> * `*F` means the PCD's value was obtained from the FDF file.
> * `*B` means the PCD's value set by a build option.
>+**Note:** If the Pcd is a Structure PCD,  is the Struct Name.
>
> Additional lines may be displayed showing default values when the value is
>not a
> default value.
>
> ### Example
>@@ -135,15 +140,16 @@ not used PCD section is generated.
> PCD values derived from expressions or other PCDs are not differentiated in
>the
> report. Only the final value is displayed.
>
> The first line is required:
>
>-`[*P|*F|*B] :  () = `
>+`[*P|*F|*B] :  ()
>[()][()] = `
>
> * `*P` means the Pcd's value was obtained from the DSC file
> * `*F` means the PCD's value was obtained from the FDF file.
> * `*B` means the PCD's value set by a build option.
>+**Note:** If the Pcd is a Structure PCD,  is the Struct Name.
>
> Additional lines may be displayed showing default values when the value is
>not a
> default value.
>
> Since the PCDs in this section are not used by any module, the PCD value is
>not
>diff --git a/13_build_reports/136_global_pcd_section.md
>b/13_build_reports/136_global_pcd_section.md
>index d768488..c300920 100644
>--- a/13_build_reports/136_global_pcd_section.md
>+++ b/13_build_reports/136_global_pcd_section.md
>@@ -52,18 +52,19 @@ Each global PCD item contains one or more lines:
>
> ### 13.6.1 Required line
>
> The first line is required:
>
>-`[*P|*F|*B] :  () = `
>+`[*P|*F|*B] :  ()
>[()][()] = `
>
>-* `*P` means the Pcd's value was obtained from the DSC file
>+* `*P` means the PCD's value was obtained from the DSC file
> * `*F` means the PCD's value was obtained from the FDF file.
> * `*B` means the PCD's value was obtained from a build option.
> * If no `*P`, `*F` or `*B` is shown, the PCD's value comes from DEC file. If 
> the
>   value obtained from either a build option, the DSC or FDF is the same as the
>   value in the DEC, then `*B`, `*P` or `*F` will not be shown in the report.
>+**Note:** If the Pcd is a Structure PCD,  is the Struct Name.
>
>  Examples
>
> ```
> *P PcdWinNtFirmwareVolume   : FIXED   (VOID*) = L"..\\Fv\\Nt32.fd"
>@@ -71,10 +72,14 @@ The first line is required:
>   DEC DEFAULT = 0x0
>
> gTokenSpaceGuid
> *B LogEnable: FIXED   (UNIT32) = 0x1
>   DEC DEFAULT = 0x0
>+*P TestDynamic  :  DYN(VOID*) (DEFAULT) = 
>L"COM1!COM2"
>+  

[edk2] [Patch] MdeModulePkg PCD: Remove unused PCD attribute PCD_TYPE_SKU_ENABLED

[Liming Gao]

PcdDb optimization has handled PCD DB for each SKU, not for single PCD.
So, this PCD attribute is not used any more.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Reviewed-by: Liming Gao 
---
 MdeModulePkg/Include/Guid/PcdDataBaseSignatureGuid.h | 3 +--
 MdeModulePkg/Universal/PCD/Dxe/Pcd.inf   | 3 +--
 MdeModulePkg/Universal/PCD/Dxe/PcdDxe.uni| 3 +--
 MdeModulePkg/Universal/PCD/Pei/Pcd.inf   | 3 +--
 MdeModulePkg/Universal/PCD/Pei/PcdPeim.uni   | 3 +--
 5 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/MdeModulePkg/Include/Guid/PcdDataBaseSignatureGuid.h 
b/MdeModulePkg/Include/Guid/PcdDataBaseSignatureGuid.h
index d670af1..5767ac8 100644
--- a/MdeModulePkg/Include/Guid/PcdDataBaseSignatureGuid.h
+++ b/MdeModulePkg/Include/Guid/PcdDataBaseSignatureGuid.h
@@ -1,7 +1,7 @@
 /** @file
   Guid for Pcd DataBase Signature.
 
-Copyright (c) 2012 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.
 This program and the accompanying materials are licensed and made available 
under
 the terms and conditions of the BSD License that accompanies this distribution.
 The full text of the license may be found at
@@ -30,7 +30,6 @@ typedef UINT64 SKU_ID;
 #define PCD_TYPE_DATA (0x0U << PCD_TYPE_SHIFT)
 #define PCD_TYPE_HII  (0x8U << PCD_TYPE_SHIFT)
 #define PCD_TYPE_VPD  (0x4U << PCD_TYPE_SHIFT)
-#define PCD_TYPE_SKU_ENABLED  (0x2U << PCD_TYPE_SHIFT)
 #define PCD_TYPE_STRING   (0x1U << PCD_TYPE_SHIFT)
 
 #define PCD_TYPE_ALL_SET  (PCD_TYPE_DATA | PCD_TYPE_HII | PCD_TYPE_VPD | 
PCD_TYPE_SKU_ENABLED | PCD_TYPE_STRING)
diff --git a/MdeModulePkg/Universal/PCD/Dxe/Pcd.inf 
b/MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
index 54cd8d9..a20b645 100644
--- a/MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
+++ b/MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
@@ -174,7 +174,6 @@
 # PCD_TYPE_DATA
 # PCD_TYPE_HII
 # PCD_TYPE_VPD
-# PCD_TYPE_SKU_ENABLED
 # PCD_TYPE_STRING
 #  Datum Type   : indicate PCD vaue type from following macro:
 # PCD_DATUM_TYPE_POINTER
@@ -279,7 +278,7 @@
 #- Variable GUID for HII type PCD
 #- Token space GUID for dynamicex type PCD 
 #
-#  Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
diff --git a/MdeModulePkg/Universal/PCD/Dxe/PcdDxe.uni 
b/MdeModulePkg/Universal/PCD/Dxe/PcdDxe.uni
index 922391c..0883ad8 100644
--- a/MdeModulePkg/Universal/PCD/Dxe/PcdDxe.uni
+++ b/MdeModulePkg/Universal/PCD/Dxe/PcdDxe.uni
@@ -174,7 +174,6 @@
 // PCD_TYPE_DATA
 // PCD_TYPE_HII
 // PCD_TYPE_VPD
-// PCD_TYPE_SKU_ENABLED
 // PCD_TYPE_STRING
 // Datum Type   : indicate PCD vaue type from following macro:
 // PCD_DATUM_TYPE_POINTER
@@ -279,7 +278,7 @@
 // - Variable GUID for HII type PCD
 // - Token space GUID for dynamicex type PCD
 //
-// Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
+// Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
 //
 // This program and the accompanying materials
 // are licensed and made available under the terms and conditions of the BSD 
License
diff --git a/MdeModulePkg/Universal/PCD/Pei/Pcd.inf 
b/MdeModulePkg/Universal/PCD/Pei/Pcd.inf
index e1ea5be..3cba289 100644
--- a/MdeModulePkg/Universal/PCD/Pei/Pcd.inf
+++ b/MdeModulePkg/Universal/PCD/Pei/Pcd.inf
@@ -173,7 +173,6 @@
 # PCD_TYPE_DATA
 # PCD_TYPE_HII
 # PCD_TYPE_VPD
-# PCD_TYPE_SKU_ENABLED
 # PCD_TYPE_STRING
 #  Datum Type   : indicate PCD vaue type from following macro:
 # PCD_DATUM_TYPE_POINTER
@@ -278,7 +277,7 @@
 #- Variable GUID for HII type PCD
 #- Token space GUID for dynamicex type PCD 
 #
-#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
+#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
diff --git a/MdeModulePkg/Universal/PCD/Pei/PcdPeim.uni 
b/MdeModulePkg/Universal/PCD/Pei/PcdPeim.uni
index b0d3f1f..3e7c556 100644
--- a/MdeModulePkg/Universal/PCD/Pei/PcdPeim.uni
+++ b/MdeModulePkg/Universal/PCD/Pei/PcdPeim.uni
@@ -173,7 +173,6 @@
 // PCD_TYPE_DATA
 // PCD_TYPE_HII
 // PCD_TYPE_VPD
-// PCD_TYPE_SKU_ENABLED
 // PCD_TYPE_STRING
 // Datum Type   : indicate PCD vaue type from following macro:
 // PCD_DATUM_TYPE_POINTER
@@ -278,7 +277,7 

Re: [edk2] [PATCH 0/4] Virtio non-discoverable devices

[Ard Biesheuvel]

(+ Laszlo)

Hello Daniil,

On 7 March 2018 at 01:36, Daniil Egranov  wrote:
> This is an attempt to add MMIO Virtio devices into the
> non-discoverable device registration procedure and allow
> Virtio PCI drivers to recognize and program such devices
> correctly.

Why? The purpose of the non-discoverable device layer is to make
non-PCI controllers that can be driven by PCI class drivers appear as
PCI devices. We have started using the base non-discoverable device
protocol for other devices as well, but the PCI wrapper is really only
intended for PCI class drivers.

For VirtIO MMIO, we already have a driver model driver, and given that
you need to patch up differences between MMIO and PCI based virtio in
your code, I am reluctant to incorporate modifications in to the PCI
driver to support MMIO devices.

Is this related to virtio 1.0 support?

Also, could you please ensure next time that you cc all the relevant
people? (Please check the Maintainers file)

> The main issue is that the set of MMIO registers is different
> from PCI, plus the width of similar registers are not
> always the same. The code implements the translation of
> the PCI IO registers to MMIO registers.
> Another difference between PCI and MMIO Virtio devices found
> during the testing is that MMIO devices may require more
> registers to be programmed compared to PCI. The VirtioPciDeviceDxe
> was patched to detect non-discoverable MMIO devices and allow
> calling a PCI MemIo protocol function.
>
> This set of patches was tested with MMIO Virtio Block and
> Virtio Net devices.
>
> Daniil Egranov (4):
>   MdeModulePkg: Added new Virtio non-discoverable type and GUID
>   NonDiscoverableDeviceRegistrationLib: Added Virtio support
>   NonDiscoverablePciDeviceDxe: Added MMIO Virtio support
>   VirtioPciDeviceDxe: Added non-discoverable Virtio support
>
>  .../NonDiscoverablePciDeviceDxe.c  |   3 +-
>  .../NonDiscoverablePciDeviceDxe.inf|   5 +-
>  .../NonDiscoverablePciDeviceIo.c   | 240 
> -
>  MdeModulePkg/Include/Guid/NonDiscoverableDevice.h  |   3 +
>  .../Library/NonDiscoverableDeviceRegistrationLib.h |   1 +
>  .../NonDiscoverableDeviceRegistrationLib.c |   3 +
>  .../NonDiscoverableDeviceRegistrationLib.inf   |   1 +
>  MdeModulePkg/MdeModulePkg.dec  |   1 +
>  OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.c   | 143 +++-
>  OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h   |  21 +-
>  OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf  |   4 +-
>  OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c| 117 +-
>  12 files changed, 528 insertions(+), 14 deletions(-)
>
> --
> 2.11.0
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel