Re: [edk2] [Patch] MdeModulePkg PCD: Add more debug message to show SkuId update

2018-02-26 Thread Zeng, Star
Reviewed-by: Star Zeng 

Thanks,
Star
-Original Message-
From: Gao, Liming 
Sent: Tuesday, February 27, 2018 1:38 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star 
Subject: [Patch] MdeModulePkg PCD: Add more debug message to show SkuId update

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Star Zeng 
---
 MdeModulePkg/Universal/PCD/Dxe/Pcd.c | 6 +-  
MdeModulePkg/Universal/PCD/Pei/Pcd.c | 6 +-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/PCD/Dxe/Pcd.c 
b/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
index 26485f1..1b19e38 100644
--- a/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
+++ b/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
@@ -286,10 +286,13 @@ DxePcdSetSku (
   UINTN  Index;
   EFI_STATUS Status;
 
+  DEBUG ((DEBUG_INFO, "PcdDxe - SkuId 0x%lx is to be set.\n", (SKU_ID) 
+ SkuId));
+
   if (SkuId == mPcdDatabase.DxeDb->SystemSkuId) {
 //
 // The input SKU Id is equal to current SKU Id, return directly.
 //
+DEBUG ((DEBUG_INFO, "PcdDxe - SkuId is same to current system 
+ Sku.\n"));
 return;
   }
 
@@ -308,6 +311,7 @@ DxePcdSetSku (
   SkuIdTable = (SKU_ID *) ((UINT8 *) mPcdDatabase.DxeDb + 
mPcdDatabase.DxeDb->SkuIdTableOffset);
   for (Index = 0; Index < SkuIdTable[0]; Index++) {
 if (SkuId == SkuIdTable[Index + 1]) {
+  DEBUG ((DEBUG_INFO, "PcdDxe - SkuId is found in SkuId 
+ table.\n"));
   Status = UpdatePcdDatabase (SkuId, TRUE);
   if (!EFI_ERROR (Status)) {
 mPcdDatabase.DxeDb->SystemSkuId = (SKU_ID) SkuId; @@ -320,7 +324,7 @@ 
DxePcdSetSku (
   //
   // Invalid input SkuId, the default SKU Id will be still used for the system.
   //
-  DEBUG ((DEBUG_INFO, "PcdDxe - Invalid input SkuId, the default SKU Id will 
be still used.\n"));
+  DEBUG ((DEBUG_ERROR, "PcdDxe - Invalid input SkuId, the default SKU 
+ Id will be still used.\n"));
   return;
 }
 
diff --git a/MdeModulePkg/Universal/PCD/Pei/Pcd.c 
b/MdeModulePkg/Universal/PCD/Pei/Pcd.c
index f821395..8d9328b 100644
--- a/MdeModulePkg/Universal/PCD/Pei/Pcd.c
+++ b/MdeModulePkg/Universal/PCD/Pei/Pcd.c
@@ -475,12 +475,15 @@ PeiPcdSetSku (
   PCD_DATABASE_SKU_DELTA *SkuDelta;
   PCD_DATA_DELTA *SkuDeltaData;
 
+  DEBUG ((DEBUG_INFO, "PcdPei - SkuId 0x%lx is to be set.\n", (SKU_ID) 
+ SkuId));
+
   PeiPcdDb = GetPcdDatabase();
 
   if (SkuId == PeiPcdDb->SystemSkuId) {
 //
 // The input SKU Id is equal to current SKU Id, return directly.
 //
+DEBUG ((DEBUG_INFO, "PcdPei - SkuId is same to current system 
+ Sku.\n"));
 return;
   }
 
@@ -499,6 +502,7 @@ PeiPcdSetSku (
   SkuIdTable = (SKU_ID *) ((UINT8 *) PeiPcdDb + PeiPcdDb->SkuIdTableOffset);
   for (Index = 0; Index < SkuIdTable[0]; Index++) {
 if (SkuId == SkuIdTable[Index + 1]) {
+  DEBUG ((DEBUG_INFO, "PcdPei - SkuId is found in SkuId 
+ table.\n"));
   break;
 }
   }
@@ -570,7 +574,7 @@ PeiPcdSetSku (
   //
   // Invalid input SkuId, the default SKU Id will be still used for the system.
   //
-  DEBUG ((EFI_D_INFO, "PcdPei - Invalid input SkuId, the default SKU Id will 
be still used.\n"));
+  DEBUG ((DEBUG_ERROR, "PcdPei - Invalid input SkuId, the default SKU 
+ Id will be still used.\n"));
 
   return;
 }
--
2.8.0.windows.1

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


Re: [edk2] [Patch] MdeModulePkg PCD: Fix the issue to set the big SkuId

2018-02-26 Thread Zeng, Star
Reviewed-by: Star Zeng 

Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Liming 
Gao
Sent: Tuesday, February 27, 2018 1:37 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star 
Subject: [edk2] [Patch] MdeModulePkg PCD: Fix the issue to set the big SkuId

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Star Zeng 
---
 MdeModulePkg/Universal/PCD/Dxe/Service.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/PCD/Dxe/Service.c 
b/MdeModulePkg/Universal/PCD/Dxe/Service.c
index 68ad5b7..cdfb4fb 100644
--- a/MdeModulePkg/Universal/PCD/Dxe/Service.c
+++ b/MdeModulePkg/Universal/PCD/Dxe/Service.c
@@ -790,7 +790,7 @@ UpdatePcdDatabase (
 SkuDelta = NULL;
 while (Index < mPeiPcdDbSize) {
   SkuDelta = (PCD_DATABASE_SKU_DELTA *) ((UINT8 *) mPeiPcdDbBinary + 
Index);
-  if (SkuDelta->SkuId == (UINT16) SkuId && SkuDelta->SkuIdCompared == 0) {
+  if (SkuDelta->SkuId == SkuId && SkuDelta->SkuIdCompared == 0) {
 break;
   }
   Index = (Index + SkuDelta->Length + 7) & (~7);
-- 
2.8.0.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] Add Platform Include path in modules

2018-02-26 Thread Pankaj Bansal
Hi Laszlo/Michael,

Thanks for your feedback on this proposal.
I looked at the structured PCDs and UEFI Platform Initialization Distribution 
Packaging Specification.
Here is my take on these.

1. structured PCDs are good if we want to declare single complex structure.
But consider a case where I want to keep device information in structure. 
(e.g. hardware settings, limitations etc)
And we may want to tweak this information based on platform revision being 
used.
And different platforms can have different number of such devices.
In this case, when we want to add a new platform, we might need to 
introduce new PCDs in .dec files, which will not be needed for others.
I don't know, will this even increase the PCD database size for existing 
platforms or not ?

2. To mitigate the "hidden" dependency of a module on platform, we can 
explicitly declare this dependency in module inf file.
 I am thinking something like gEfiCallerIdGuid, i.e. module can declare 
that that platform building(using) the module, supply this information.

3. Using Libraries and Protocols can also solve such use cases. I just felt 
that it's less cumbersome to use include files, and it also avoids code 
replication.
Anyway, this is just my suggestion to have such mechanism in edk2 build 
process. I am more than happy to stick to platform libraries.

Thanks & Regards,
Pankaj Bansal

> -Original Message-
> From: Kinney, Michael D [mailto:michael.d.kin...@intel.com]
> Sent: Monday, February 26, 2018 10:56 PM
> To: Laszlo Ersek ; Pankaj Bansal
> ; edk2-devel@lists.01.org; Kinney, Michael D
> 
> Cc: Gao, Liming 
> Subject: RE: [edk2] [RFC] Add Platform Include path in modules
> 
> Hi Pankaj,
> 
> I agree with Laszlo that you should evaluate use of PCDs.  There are a few
> methods for a driver to use platform specific values/behavior.  These are:
> 
> * PCDs
> * Library class/Library Instance
> * Protocol/PPI
> 
> One issue with the proposal is that it adds a hidden dependency to modules.
> An EDK II INF file describes the external interfaces of a module along with
> produces/consumes usage.  This information is aligned with the XML schema
> that is documented in the UEFI Platform Initialization Distribution Packaging
> Specification.
> 
> 
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fuefi.
> org%2Fspecifications&data=02%7C01%7Cpankaj.bansal%40nxp.com%7C96c4
> 2dd7271d449d56e908d57d3df2d4%7C686ea1d3bc2b4c6fa92cd99c5c301635
> %7C0%7C0%7C636552627376357192&sdata=sS7KT3haANF5TLHSeRGjIQHnLQ
> BtnTDLIZWntUhsk78%3D&reserved=0
> 
> If two modules have the same GUID/Version, then the external interfaces to
> those two modules are expected to be identical.
> With your proposal, two modules built for 2 different platforms would have
> the same GUID/Version but would not have the same external interfaces
> because a hidden dependency on a platform package was added.
> 
> If a module really needs to use content from a platform package, then a new
> copy of the module should be created with a new GUID/Version and the
> platform package added to the [Packages] section.  The other option is to use
> one of the supported interfaces (PCDs, Lib, Protocol, PPI).
> 
> Please let us know if any of these exiting methods do not work for your use
> case.
> 
> Thanks,
> 
> Mike
> 
> > -Original Message-
> > From: Laszlo Ersek [mailto:ler...@redhat.com]
> > Sent: Monday, February 26, 2018 7:55 AM
> > To: Pankaj Bansal ; Kinney, Michael D
> > ; edk2- de...@lists.01.org
> > Cc: Gao, Liming 
> > Subject: Re: [edk2] [RFC] Add Platform Include path in modules
> >
> > On 02/26/18 11:55, Pankaj Bansal wrote:
> > > Hi,
> > >
> > > Consider a simple driver which needs that some data
> > structures be
> > > filled by the Platform, which is using the driver.
> > >
> > > Driver.c #include 
> > >
> > > Struct a = platformVal;
> > >
> > > We can define platformVal in Platform.h, which would
> > be unique to the
> > > platform being built. This Platform.h can be placed in
> > include
> > > directories, whose path would be defined in
> > Platform.dec file.
> > >
> > > Now, whenever we build driver for each unique
> > platform, we need not
> > > to mention Platform.dec file in driver.inf [packages]
> > section. We can
> > > append Platform.dec include paths to each driver. i.e.
> > look for the
> > > include files in [packages] section as well as in
> > Platform include
> > > directories.
> > >
> > > For this, I am looking for Platform.dec file in same
> > directory as
> > > Platform.dsc and using same name as Platform.dsc
> > >
> > > We can refine this change further. i.e. add Platform
> > include
> > > directories to driver's include paths based on some
> > condition in
> > > driver.inf file.
> >
> > (Apologies in advance if I failed to grasp the use
> > case.)
> >
> > If I understand correctly, you have multiple platforms (defined by DSC
> > and FDF files), and you build a given driver for several of these
> > platf

[edk2] [PATCH] BaseTools: Fix report not used --pcd value incorrectly

2018-02-26 Thread Feng, YunhuaX
Argument --pcd gUefiOvmfPkgTokenSpaceGuid.test10=H"{1}",
If the PCD is not used, report value {0x01, 0x00}, is incorrect.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: Yunhua Feng 
---
 BaseTools/Source/Python/build/BuildReport.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/BaseTools/Source/Python/build/BuildReport.py 
b/BaseTools/Source/Python/build/BuildReport.py
index 21144991bf..58595d62b3 100644
--- a/BaseTools/Source/Python/build/BuildReport.py
+++ b/BaseTools/Source/Python/build/BuildReport.py
@@ -976,10 +976,11 @@ class PcdReport(object):
 BuildOptionMatch = False
 if GlobalData.BuildOptionPcd:
 for pcd in GlobalData.BuildOptionPcd:
 if (Pcd.TokenSpaceGuidCName, Pcd.TokenCName) == 
(pcd[0], pcd[1]):
 PcdValue = pcd[2]
+Pcd.DefaultValue = PcdValue
 BuildOptionMatch = True
 break
 
 if First:
 if ModulePcdSet == None:
-- 
2.12.2.windows.2

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


Re: [edk2] [Patch] BaseTools:Override the MAKE_FLAGS by BuildOptions in DSC

2018-02-26 Thread Gao, Liming
Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Yonghong Zhu
> Sent: Saturday, February 24, 2018 9:54 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch] BaseTools:Override the MAKE_FLAGS by BuildOptions in 
> DSC
> 
> The issue that *_*_*_MAKE_FLAGS doesn't work in DSC [BuildOptions]
> section. It means MAKE flags can't be set in platform DSC file.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Source/Python/AutoGen/AutoGen.py | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
> b/BaseTools/Source/Python/AutoGen/AutoGen.py
> index 405bfa1..1787dec 100644
> --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> @@ -1905,10 +1905,17 @@ class PlatformAutoGen(AutoGen):
>  self._BuildCommand += 
> SplitOption(self.ToolDefinition["MAKE"]["PATH"])
>  if "FLAGS" in self.ToolDefinition["MAKE"]:
>  NewOption = self.ToolDefinition["MAKE"]["FLAGS"].strip()
>  if NewOption != '':
>  self._BuildCommand += SplitOption(NewOption)
> +if "MAKE" in self.EdkIIBuildOption:
> +if "FLAGS" in self.EdkIIBuildOption["MAKE"]:
> +Flags = self.EdkIIBuildOption["MAKE"]["FLAGS"]
> +if Flags.startswith('='):
> +self._BuildCommand = [self._BuildCommand[0]] + 
> [Flags[1:]]
> +else:
> +self._BuildCommand += [Flags]
>  return self._BuildCommand
> 
>  ## Get tool chain definition
>  #
>  #  Get each tool defition for given tool chain from tools_def.txt and 
> platform
> --
> 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


[edk2] [patch 1/2] BaseTool/VfrCompile: make delete[] match with new[]

2018-02-26 Thread Dandan Bi
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=764

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 BaseTools/Source/C/VfrCompile/VfrCompiler.cpp   | 14 +++---
 BaseTools/Source/C/VfrCompile/VfrError.cpp  |  2 +-
 BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp|  6 +++---
 BaseTools/Source/C/VfrCompile/VfrSyntax.g   | 14 +++---
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp |  8 
 5 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp 
b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
index ff2a837..84c0e59 100644
--- a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
@@ -1,10 +1,10 @@
 /** @file
   
   VfrCompiler main class and main function.
 
-Copyright (c) 2004 - 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.
 This program and the accompanying materials  
 are licensed and made available under the terms and conditions of the BSD 
License 
 which accompanies this distribution.  The full text of the license may be 
found at
 http://opensource.org/licenses/bsd-license.php 
   

   
@@ -282,11 +282,11 @@ CVfrCompiler::AppendIncludePath (
 strcat (IncludePaths, mOptions.IncludePaths);
   }
   strcat (IncludePaths, " -I ");
   strcat (IncludePaths, PathStr);
   if (mOptions.IncludePaths != NULL) {
-delete mOptions.IncludePaths;
+delete[] mOptions.IncludePaths;
   }
   mOptions.IncludePaths = IncludePaths;
 }
 
 VOID
@@ -311,11 +311,11 @@ CVfrCompiler::AppendCPreprocessorOptions (
 strcat (Opt, mOptions.CPreprocessorOptions);
   }
   strcat (Opt, " ");
   strcat (Opt, Options);
   if (mOptions.CPreprocessorOptions != NULL) {
-delete mOptions.CPreprocessorOptions;
+delete[] mOptions.CPreprocessorOptions;
   }
   mOptions.CPreprocessorOptions = Opt;
 }
 
 INT8
@@ -529,16 +529,16 @@ CVfrCompiler::~CVfrCompiler (
 free (mOptions.RecordListFile);
 mOptions.RecordListFile = NULL;
   }
 
   if (mOptions.IncludePaths != NULL) {
-delete mOptions.IncludePaths;
+delete[] mOptions.IncludePaths;
 mOptions.IncludePaths = NULL;
   }
 
   if (mOptions.CPreprocessorOptions != NULL) {
-delete mOptions.CPreprocessorOptions;
+delete[] mOptions.CPreprocessorOptions;
 mOptions.CPreprocessorOptions = NULL;
   }
 
   SET_RUN_STATUS(STATUS_DEAD);
 }
@@ -963,15 +963,15 @@ main (
   if ((Status == STATUS_DEAD) || (Status == STATUS_FAILED)) {
 return 2;
   }
 
   if (gCBuffer.Buffer != NULL) {
-delete gCBuffer.Buffer;
+delete[] gCBuffer.Buffer;
   }
   
   if (gRBuffer.Buffer != NULL) {
-delete gRBuffer.Buffer;
+delete[] gRBuffer.Buffer;
   }
 
   return GetUtilityStatus ();
 }
 
diff --git a/BaseTools/Source/C/VfrCompile/VfrError.cpp 
b/BaseTools/Source/C/VfrCompile/VfrError.cpp
index 2366fac..14771a2 100644
--- a/BaseTools/Source/C/VfrCompile/VfrError.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrError.cpp
@@ -75,11 +75,11 @@ CVfrErrorHandle::~CVfrErrorHandle (
   )
 {
   SVfrFileScopeRecord *pNode = NULL;
 
   if (mInputFileName != NULL) {
-delete mInputFileName;
+delete[] mInputFileName;
   }
 
   while (mScopeRecordListHead != NULL) {
 pNode = mScopeRecordListHead;
 mScopeRecordListHead = mScopeRecordListHead->mNext;
diff --git a/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp 
b/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
index 090ee13..b40bcdf 100644
--- a/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
@@ -142,11 +142,11 @@ CFormPkg::~CFormPkg ()
 
   while (mBufferNodeQueueHead != NULL) {
 pBNode = mBufferNodeQueueHead;
 mBufferNodeQueueHead = mBufferNodeQueueHead->mNext;
 if (pBNode->mBufferStart != NULL) {
-  delete pBNode->mBufferStart;
+  delete[] pBNode->mBufferStart;
   delete pBNode;
 }
   }
   mBufferNodeQueueTail = NULL;
   mCurrBufferNode  = NULL;
@@ -1150,11 +1150,11 @@ CIfrRecordInfoDB::IfrRecordOutput (
 {
   CHAR8  *Temp;
   SIfrRecord *pNode; 
 
   if (TBuffer.Buffer != NULL) {
-delete TBuffer.Buffer;
+delete[] TBuffer.Buffer;
   }
 
   TBuffer.Size = 0;
   TBuffer.Buffer = NULL;
 
@@ -2257,11 +2257,11 @@ CIfrObj::_EMIT_PENDING_OBJ (
   
   //
   // update bin buffer to package data buffer
   //
   if (mObjBinBuf != NULL) {
-delete mObjBinBuf;
+delete[] mObjBinBuf;
 mObjBinBuf = ObjBinBuf;
   }
   
   mDelayEmit = FALSE;
 }
diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g 
b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
index 4c7c6f2..d48072a 100644
--- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
+++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
@@ -967,11 +967,11 @@ vfrExtensionData[U

[edk2] [patch 2/2] BaseTool/VfrCompile: Fix potential memory leak issue

2018-02-26 Thread Dandan Bi
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=771

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp | 4 
 1 file changed, 4 insertions(+)

diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp 
b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
index 9bdc544..5cab7bb 100644
--- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
@@ -3502,10 +3502,14 @@ CVfrStringDB::SetStringFileName(IN CHAR8 
*StringFileName)
 
   if (StringFileName == NULL) {
 return;
   }
 
+  if (mStringFileName != NULL) {
+delete[] mStringFileName;
+  }
+
   FileLen = strlen (StringFileName) + 1;
   mStringFileName = new CHAR8[FileLen];
   if (mStringFileName == NULL) {
 return;
   }
-- 
1.9.5.msysgit.1

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


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

2018-02-26 Thread Ni, Ruiyu

On 2/27/2018 8:48 AM, Guo Heyi wrote:

Hi Laszlo,

I agree the current patch makes the code ugly, and turning the logic into a
normal loop should be the perfect solution. If Ray also agrees on it, I can try
to do that.

Thanks and regards,

Heyi

On Mon, Feb 26, 2018 at 05:23:29PM +0100, Laszlo Ersek wrote:

On 02/26/18 09:29, 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.


Not really my place to comment on this, but how about removing the
recursion entirely, and turning the logic into a normal (iterative) loop
instead?

(1) Rename the current function to:

STATIC
VOID
BmRepairAllControllersWorker (
   OUT BOOLEAN *ReconnectRequired,
   OUT BOOLEAN *RebootRequired
   );


(2) The worker function should end right before

   if (ReconnectRequired) {
 BmRepairAllControllers ();
   }


(3) The worker function should not contain

   RebootRequired= FALSE;
   ReconnectRequired = FALSE;

Such initialization should be left to the caller.


(4) The worker function should be called in a loop from a new
BmRepairAllControllers() function, like this:

   Reconnect = 0;
   RebootRequired = FALSE;
   do {
 ReconnectRequired = FALSE;
 BmRepairAllControllersWorker (&ReconnectRequired, &RebootRequired);
 ++Reconnect;
   } while (ReconnectRequired && Reconnect < MAX_RECONNECT_REPAIR);

   DEBUG_CODE (...);

   if (RebootRequired) {
 ...
   }


In addition to eliminating the shoddy recursive call (and the shoddier
global counter, ewww :) ), this would fix the following other warts with
the code:

- When a nested call chain is unwound, we currently dump a series of
"driver health info" lists (assuming no reboot is required), in the
DEBUG_CODE section. I would argue that we should do that only once, at
the end. (Even if we have to do it multiple times, it can be moved into
the worker function, to the end.)

- It seems to be sufficient to accumulate RebootRequired into one
variable (i.e. not multiple instances of the same local variable on the
stack) and to act upon it at the very end.


Feel free to ignore my comments -- I just think we should be moving in
the opposite direction; that is, away from recursion (especially from
recursion combined with global variables -- that's one difficult pattern
to reason about).


How about to just remove the global variable?
I prefer to change BmRepairAllControllers in the following prototype:
VOID
BmRepairAllControllers (
  UINTN  ReconnectRepairCount
  );
And start to call this like BmRepairAllControllers (0).

I am neutral between recursive call and while loop.
But I am afraid such a big change may introduce some bugs.
And I also like to move the DEBUG_CODE to before:
if (ReconnectRequired) {
  BmRepairAllControllers (ReconnectRepairCount + 1);
}
So that we can dump the health info for every reconnect repair.





Thanks
Laszlo


Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Ruiyu Ni 
---
  MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h |  6 ++
  MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c | 17 
-
  2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h 
b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
index 25a1d522fe84..9aa86b096525 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.
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
index ddcee8b0676f..30d70f32af84 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
@@ -26,6 +26,12 @@ GLOBAL_REMOVE_IF_UNREFERENCED
  L"Reboot Required"
  };
  
+//

+// Counter of reconnect retry to repair controller; it is to limit the
+// number of recursive call of BmRepairAllControllers.
+//
+STATIC UINTN mReconnectRepairCount;
+
  /**
Return the controller name.
  
@@ -549,7 +555

[edk2] [Patch] MdeModulePkg PCD: Add more debug message to show SkuId update

2018-02-26 Thread Liming Gao
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Star Zeng 
---
 MdeModulePkg/Universal/PCD/Dxe/Pcd.c | 6 +-
 MdeModulePkg/Universal/PCD/Pei/Pcd.c | 6 +-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/PCD/Dxe/Pcd.c 
b/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
index 26485f1..1b19e38 100644
--- a/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
+++ b/MdeModulePkg/Universal/PCD/Dxe/Pcd.c
@@ -286,10 +286,13 @@ DxePcdSetSku (
   UINTN  Index;
   EFI_STATUS Status;
 
+  DEBUG ((DEBUG_INFO, "PcdDxe - SkuId 0x%lx is to be set.\n", (SKU_ID) SkuId));
+
   if (SkuId == mPcdDatabase.DxeDb->SystemSkuId) {
 //
 // The input SKU Id is equal to current SKU Id, return directly.
 //
+DEBUG ((DEBUG_INFO, "PcdDxe - SkuId is same to current system Sku.\n"));
 return;
   }
 
@@ -308,6 +311,7 @@ DxePcdSetSku (
   SkuIdTable = (SKU_ID *) ((UINT8 *) mPcdDatabase.DxeDb + 
mPcdDatabase.DxeDb->SkuIdTableOffset);
   for (Index = 0; Index < SkuIdTable[0]; Index++) {
 if (SkuId == SkuIdTable[Index + 1]) {
+  DEBUG ((DEBUG_INFO, "PcdDxe - SkuId is found in SkuId table.\n"));
   Status = UpdatePcdDatabase (SkuId, TRUE);
   if (!EFI_ERROR (Status)) {
 mPcdDatabase.DxeDb->SystemSkuId = (SKU_ID) SkuId;
@@ -320,7 +324,7 @@ DxePcdSetSku (
   //
   // Invalid input SkuId, the default SKU Id will be still used for the system.
   //
-  DEBUG ((DEBUG_INFO, "PcdDxe - Invalid input SkuId, the default SKU Id will 
be still used.\n"));
+  DEBUG ((DEBUG_ERROR, "PcdDxe - Invalid input SkuId, the default SKU Id will 
be still used.\n"));
   return;
 }
 
diff --git a/MdeModulePkg/Universal/PCD/Pei/Pcd.c 
b/MdeModulePkg/Universal/PCD/Pei/Pcd.c
index f821395..8d9328b 100644
--- a/MdeModulePkg/Universal/PCD/Pei/Pcd.c
+++ b/MdeModulePkg/Universal/PCD/Pei/Pcd.c
@@ -475,12 +475,15 @@ PeiPcdSetSku (
   PCD_DATABASE_SKU_DELTA *SkuDelta;
   PCD_DATA_DELTA *SkuDeltaData;
 
+  DEBUG ((DEBUG_INFO, "PcdPei - SkuId 0x%lx is to be set.\n", (SKU_ID) SkuId));
+
   PeiPcdDb = GetPcdDatabase();
 
   if (SkuId == PeiPcdDb->SystemSkuId) {
 //
 // The input SKU Id is equal to current SKU Id, return directly.
 //
+DEBUG ((DEBUG_INFO, "PcdPei - SkuId is same to current system Sku.\n"));
 return;
   }
 
@@ -499,6 +502,7 @@ PeiPcdSetSku (
   SkuIdTable = (SKU_ID *) ((UINT8 *) PeiPcdDb + PeiPcdDb->SkuIdTableOffset);
   for (Index = 0; Index < SkuIdTable[0]; Index++) {
 if (SkuId == SkuIdTable[Index + 1]) {
+  DEBUG ((DEBUG_INFO, "PcdPei - SkuId is found in SkuId table.\n"));
   break;
 }
   }
@@ -570,7 +574,7 @@ PeiPcdSetSku (
   //
   // Invalid input SkuId, the default SKU Id will be still used for the system.
   //
-  DEBUG ((EFI_D_INFO, "PcdPei - Invalid input SkuId, the default SKU Id will 
be still used.\n"));
+  DEBUG ((DEBUG_ERROR, "PcdPei - Invalid input SkuId, the default SKU Id will 
be still used.\n"));
 
   return;
 }
-- 
2.8.0.windows.1

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


[edk2] [Patch] MdeModulePkg PCD: Fix the issue to set the big SkuId

2018-02-26 Thread Liming Gao
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Star Zeng 
---
 MdeModulePkg/Universal/PCD/Dxe/Service.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/PCD/Dxe/Service.c 
b/MdeModulePkg/Universal/PCD/Dxe/Service.c
index 68ad5b7..cdfb4fb 100644
--- a/MdeModulePkg/Universal/PCD/Dxe/Service.c
+++ b/MdeModulePkg/Universal/PCD/Dxe/Service.c
@@ -790,7 +790,7 @@ UpdatePcdDatabase (
 SkuDelta = NULL;
 while (Index < mPeiPcdDbSize) {
   SkuDelta = (PCD_DATABASE_SKU_DELTA *) ((UINT8 *) mPeiPcdDbBinary + 
Index);
-  if (SkuDelta->SkuId == (UINT16) SkuId && SkuDelta->SkuIdCompared == 0) {
+  if (SkuDelta->SkuId == SkuId && SkuDelta->SkuIdCompared == 0) {
 break;
   }
   Index = (Index + SkuDelta->Length + 7) & (~7);
-- 
2.8.0.windows.1

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


Re: [edk2] [Patch] EmulatorPkg: Undefine CR3 macro in Host.h

2018-02-26 Thread Jordan Justen
Reviewed-by: Jordan Justen 

On 2018-02-22 22:22:18, Liming Gao wrote:
> CR3 has been used as structure field name in BaseLib IA32_TASK_STATE_SEGMENT.
> Undefine CR3 to make sure there is no conflict to it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Liming Gao 
> Cc: Andrew Fish 
> ---
>  EmulatorPkg/Unix/Host/Host.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/EmulatorPkg/Unix/Host/Host.h b/EmulatorPkg/Unix/Host/Host.h
> index 9d6d36e..f7db46c 100644
> --- a/EmulatorPkg/Unix/Host/Host.h
> +++ b/EmulatorPkg/Unix/Host/Host.h
> @@ -82,7 +82,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>  #undef NTOHS
>  #undef HTONS
>  #undef B0
> -
> +#undef CR3
> 
>  #include 
>  #include 
> --
> 2.8.0.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 v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks

2018-02-26 Thread Ni, Ruiyu
I don't prefer to add PCD, unless we cannot find:
1. spec content  to describe the max/min blocks
2. error handling when the blocks number is bigger than HW expects.

Thanks/Ray

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ming Huang
> Sent: Saturday, February 24, 2018 5:30 PM
> To: leif.lindh...@linaro.org; linaro-u...@lists.linaro.org; edk2-
> de...@lists.01.org; graeme.greg...@linaro.org; Zeng, Star
> ; Dong, Eric 
> Cc: huangmin...@huawei.com; ard.biesheu...@linaro.org; Ming Huang
> ; Gao, Liming ;
> mengfanr...@huawei.com; guoh...@huawei.com;
> zhangjinso...@huawei.com; Kinney, Michael D
> ; wai...@126.com;
> wanghuiqi...@huawei.com; huangda...@hisilicon.com
> Subject: [edk2] [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for
> UsbBootIoBlocks
> 
> Booting from USB may fail while the macro USB_BOOT_IO_BLOCKS set to 128
> because 128 blocks is exceeded the maximun blocks of some USB
> devices,like some virtual CD-ROM from BMC. So, give a chance to set the
> value of USB_BOOT_IO_BLOCKS by adding a Pcd.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang 
> ---
>  MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h | 7
> +--
>  MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf | 4
> 
>  MdeModulePkg/MdeModulePkg.dec| 4 
>  MdeModulePkg/MdeModulePkg.uni| 4 
>  4 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
> index 5ee50ac52a21..ca9240adbd5f 100644
> --- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
> +++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
> @@ -16,6 +16,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #ifndef _EFI_USB_MASS_BOOT_H_
>  #define _EFI_USB_MASS_BOOT_H_
> 
> +#include 
> +
>  //
>  // The opcodes of various USB boot commands:
>  // INQUIRY/REQUEST_SENSE are "No Timeout Commands" as specified @@
> -66,9 +68,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #define USB_PDT_SIMPLE_DIRECT   0x0E   ///< Simplified direct 
> access
> device
> 
>  //
> -// Other parameters, Max carried size is 512B * 128 = 64KB
> +// Other parameters, Max carried size is depanded on Pcd.
> +// The default of PcdUsbBootIoBlocks is 128. 512B * 128 = 64KB
>  //
> -#define USB_BOOT_IO_BLOCKS  128
> +#define USB_BOOT_IO_BLOCKS  (FixedPcdGet32
> (PcdUsbBootIoBlocks))
> 
>  //
>  // Retry mass command times, set by experience diff --git
> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
> index 26d15c7679bf..40426512f884 100644
> ---
> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
> +++
> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
> @@ -60,6 +60,7 @@ [Sources]
>UsbMassDiskInfo.c
> 
>  [Packages]
> +  MdeModulePkg/MdeModulePkg.dec
>MdePkg/MdePkg.dec
> 
>  [LibraryClasses]
> @@ -83,5 +84,8 @@ [Protocols]
>  # EVENT_TYPE_RELATIVE_TIMER## CONSUMES
>  #
> 
> +[FixedPcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdUsbBootIoBlocks
> +
>  [UserExtensions.TianoCore."ExtraFiles"]
>UsbMassStorageDxeExtra.uni
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec index 455979386e3f..fc40745315a0
> 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -999,6 +999,10 @@ [PcdsFixedAtBuild]
># @Prompt Enable UEFI Stack Guard.
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0
> x30001055
> 
> +## The Max blocks of usb transfer. The default is 128.
> +# @Prompt The Max blocks of usb transfer
> +gEfiMdeModulePkgTokenSpaceGuid.PcdUsbBootIoBlocks|128|UINT32|0x0
> 10B
> +
>  [PcdsFixedAtBuild, PcdsPatchableInModule]
>## Dynamic type PCD can be registered callback function for Pcd setting
> action.
>#  PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum
> number of callback function diff --git a/MdeModulePkg/MdeModulePkg.uni
> b/MdeModulePkg/MdeModulePkg.uni index f3fa616438b0..c996d6b4ebe0
> 100644
> --- a/MdeModulePkg/MdeModulePkg.uni
> +++ b/MdeModulePkg/MdeModulePkg.uni
> @@ -1243,3 +1243,7 @@
>  #string
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdEdkiiFpdtStringRecordEnableO
> nly_HELP#language en-US "Control which FPDT record format will be used
> to store the performance entry.\n"
>   
>  "On TRUE, the string FPDT
> record will be used to store every performance entry.\n"
>   
>  "On FALSE, the different
> FPDT record will be used to store the different performanc

[edk2] [PATCH] UefiCpuPkg/CpuExceptionHandlerLib: fix incorrect init of exception stack

2018-02-26 Thread Jian J Wang
The field KnownGoodStackTop in CPU_EXCEPTION_INIT_DATA is initialized to
the start address of array mNewStack. This is wrong. It must be the end
of mNewStack. This patch fixes this mistake.

Cc: Ruiyu Ni 
Cc: Eric Dong 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c 
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
index 6d1b54d31d..2a090782fc 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
@@ -270,7 +270,7 @@ InitializeCpuExceptionHandlersEx (
 AsmReadGdtr (&Gdtr);
 
 EssData.X64.Revision = CPU_EXCEPTION_INIT_DATA_REV;
-EssData.X64.KnownGoodStackTop = (UINTN)mNewStack;
+EssData.X64.KnownGoodStackTop = (UINTN)mNewStack + sizeof (mNewStack);
 EssData.X64.KnownGoodStackSize = CPU_KNOWN_GOOD_STACK_SIZE;
 EssData.X64.StackSwitchExceptions = CPU_STACK_SWITCH_EXCEPTION_LIST;
 EssData.X64.StackSwitchExceptionNumber = 
CPU_STACK_SWITCH_EXCEPTION_NUMBER;
-- 
2.15.1.windows.2

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


Re: [edk2] [PATCH v2 0/2] Update memory test driver to handle more reliable memory

2018-02-26 Thread Gao, Liming
Reviewed-by: Liming Gao 

Please add the bugzilar link in commit log. 

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Ruiyu Ni
>Sent: Saturday, February 10, 2018 10:41 PM
>To: edk2-devel@lists.01.org
>Subject: [edk2] [PATCH v2 0/2] Update memory test driver to handle more
>reliable memory
>
>v2: Fix GenericMemoryTest to count more reliable memory into total memory.
>
>Ruiyu Ni (2):
>  MdeModulePkg/GenericMemoryTest: Handle more reliable memory
>  MdeModulePkg/NullMemoryTest: Handle more reliable memory
>
> .../GenericMemoryTestDxe/LightMemoryTest.c | 75 ++--
>--
> .../MemoryTest/NullMemoryTestDxe/NullMemoryTest.c  | 75
>+-
> 2 files changed, 96 insertions(+), 54 deletions(-)
>
>--
>2.16.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] [Patch] NetworkPkg/HttpDxe: Support HTTP Delete Method.

2018-02-26 Thread Fu, Siyuan
Reviewed-by: Fu Siyuan 

> -Original Message-
> From: Wu, Jiaxin
> Sent: Tuesday, February 27, 2018 11:53 AM
> To: edk2-devel@lists.01.org
> Cc: Karunakar P ; Ye, Ting ;
> Fu, Siyuan ; Wu, Jiaxin 
> Subject: [Patch] NetworkPkg/HttpDxe: Support HTTP Delete Method.
> 
> Per the request to support HttpMethodDelete:
> https://bugzilla.tianocore.org/show_bug.cgi?id=879,
> This patch is to enable the HTTP Delete Method.
> 
> Cc: Karunakar P 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin 
> ---
>  NetworkPkg/HttpDxe/HttpImpl.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
> index b3a64cf516..a2af59674a 100644
> --- a/NetworkPkg/HttpDxe/HttpImpl.c
> +++ b/NetworkPkg/HttpDxe/HttpImpl.c
> @@ -1,9 +1,9 @@
>  /** @file
>Implementation of EFI_HTTP_PROTOCOL protocol interfaces.
> 
> -  Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
> +  Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
>(C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP
> 
>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
> @@ -279,15 +279,16 @@ EfiHttpRequest (
>}
> 
>Request = HttpMsg->Data.Request;
> 
>//
> -  // Only support GET, HEAD, PATCH, PUT and POST method in current
> implementation.
> +  // Only support GET, HEAD, DELETE, PATCH, PUT and POST method in
> current implementation.
>//
>if ((Request != NULL) && (Request->Method != HttpMethodGet) &&
> -  (Request->Method != HttpMethodHead) && (Request->Method !=
> HttpMethodPut) &&
> -  (Request->Method != HttpMethodPost) && (Request->Method !=
> HttpMethodPatch)) {
> +  (Request->Method != HttpMethodHead) && (Request->Method !=
> HttpMethodDelete) &&
> +  (Request->Method != HttpMethodPut) && (Request->Method !=
> HttpMethodPost) &&
> +  (Request->Method != HttpMethodPatch)) {
>  return EFI_UNSUPPORTED;
>}
> 
>HttpInstance = HTTP_INSTANCE_FROM_PROTOCOL (This);
> 
> --
> 2.16.2.windows.1

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


[edk2] [Patch] NetworkPkg/HttpDxe: Support HTTP Delete Method.

2018-02-26 Thread Jiaxin Wu
Per the request to support HttpMethodDelete:
https://bugzilla.tianocore.org/show_bug.cgi?id=879,
This patch is to enable the HTTP Delete Method.

Cc: Karunakar P 
Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
---
 NetworkPkg/HttpDxe/HttpImpl.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index b3a64cf516..a2af59674a 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -1,9 +1,9 @@
 /** @file
   Implementation of EFI_HTTP_PROTOCOL protocol interfaces.
 
-  Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
+  Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
   (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP
 
   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
@@ -279,15 +279,16 @@ EfiHttpRequest (
   }
 
   Request = HttpMsg->Data.Request;
 
   //
-  // Only support GET, HEAD, PATCH, PUT and POST method in current 
implementation.
+  // Only support GET, HEAD, DELETE, PATCH, PUT and POST method in current 
implementation.
   //
   if ((Request != NULL) && (Request->Method != HttpMethodGet) &&
-  (Request->Method != HttpMethodHead) && (Request->Method != 
HttpMethodPut) && 
-  (Request->Method != HttpMethodPost) && (Request->Method != 
HttpMethodPatch)) {
+  (Request->Method != HttpMethodHead) && (Request->Method != 
HttpMethodDelete) && 
+  (Request->Method != HttpMethodPut) && (Request->Method != 
HttpMethodPost) && 
+  (Request->Method != HttpMethodPatch)) {
 return EFI_UNSUPPORTED;
   }
 
   HttpInstance = HTTP_INSTANCE_FROM_PROTOCOL (This);
 
-- 
2.16.2.windows.1

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


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

2018-02-26 Thread Wang, Sunny (HPS SW)
Hi Heyi, 
Thanks for looking into my suggestion. You already mentioned the value I 
thought. :) The value is to get better boot performance by setting the 
MAX_RECONNECT_REPAIR to a smaller number. 10 times reconnect may be suitable 
for consumer products like laptop. However, it may be not suitable for server. 
For example, user installs a problematic NIC 4-port card and its OPROM produced 
4 driver handles to manage 4 ports. For this case, 10 times reconnect may take 
much longer time.  
If you think it's still not worth adding a PCD for this or have other concern 
about adding a PCD, I'm fine with dropping this suggestion.   

Regards,
Sunny Wang

-Original Message-
From: Guo Heyi [mailto:heyi@linaro.org] 
Sent: Monday, February 26, 2018 7:34 PM
To: Wang, Sunny (HPS SW) 
Cc: Heyi Guo ; edk2-devel@lists.01.org; Ruiyu Ni 
; Eric Dong ; Star Zeng 

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

Hi Sunny,

I didn't consider it as a value necessary for platform override, for the retry 
count should only have some impact on boot performance and it only happens when 
there is something wrong.

May I know what value you will use for your platform and why?

Thanks and regards,

Gary (Heyi Guo)

On Mon, Feb 26, 2018 at 08:56:50AM +, Wang, Sunny (HPS SW) wrote:
> Hi Heyi,
> Just a suggestion. 
> Is it better to use a PCD instead of a define for MAX_RECONNECT_REPAIR? So 
> that we can easily override it in our platform dsc file.
> 
> Regards,
> Sunny Wang
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Heyi Guo
> Sent: Monday, February 26, 2018 4:30 PM
> To: edk2-devel@lists.01.org
> Cc: Ruiyu Ni ; Heyi Guo ; 
> Eric Dong ; Star Zeng 
> Subject: [edk2] [PATCH 1/1] MdeModulePkg/UefiBootManagerLib: limit 
> recursive call depth
> 
> 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.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Heyi Guo 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Ruiyu Ni 
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h |  6 ++
>  MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c | 17 
> -
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h 
> b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> index 25a1d522fe84..9aa86b096525 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.
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c 
> b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
> index ddcee8b0676f..30d70f32af84 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
> @@ -26,6 +26,12 @@ GLOBAL_REMOVE_IF_UNREFERENCED
>  L"Reboot Required"
>  };
>  
> +//
> +// Counter of reconnect retry to repair controller; it is to limit 
> +the // number of recursive call of BmRepairAllControllers.
> +//
> +STATIC UINTN mReconnectRepairCount;
> +
>  /**
>Return the controller name.
>  
> @@ -549,7 +555,16 @@ BmRepairAllControllers (
>  
>  
>if (ReconnectRequired) {
> -BmRepairAllControllers ();
> +if (mReconnectRepairCount < MAX_RECONNECT_REPAIR) {
> +  mReconnectRepairCount++;
> +  BmRepairAllControllers ();
> +} else {
> +  DEBUG ((DEBUG_ERROR, "[%a:%d] Repair failed after %d retries.\n",
> +__FUNCTION__, __LINE__, mReconnectRepairCount));
> +  // Reset counter so that it will not affect calling
> +  // BmRepairAllControllers() somewhere else
> +  mReconnectRepairCount = 0;
> +}
>}
>  
>DEBUG_CODE (
> --
> 2.7.4
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel

Re: [edk2] [RFC v3 1/3] MdeModulePkg/PciHostBridgeDxe: Add support for address translation

2018-02-26 Thread Guo Heyi
Hi Laszlo,

Just a note that comment [6] has not been fully applied in my RFC v4, that I
didn't touch the outer ALIGN_VALUE() yet. As you said, it is an independent
issue and I think it can be fixed before or after my proposed change. I can do
something after Ray comments on it.

Regards,

Heyi

On Fri, Feb 23, 2018 at 04:05:17PM +0100, Laszlo Ersek wrote:
> Thanks for writing up the details!
> 
> Comments:
> 
> On 02/23/18 09:53, Heyi Guo wrote:
> > PCI address translation is necessary for some non-x86 platforms. On
> > such platforms, address value (denoted as "device address" or "address
> > in PCI view") set to PCI BAR registers in configuration space might be
> > different from the address which is used by CPU to access the
> > registers in memory BAR or IO BAR spaces (denoted as "host address" or
> > "address in CPU view"). The difference between the two addresses is
> > called "Address Translation Offset" or simply "translation", and can
> > be represented by "Address Translation Offset" in ACPI QWORD Address
> > Space Descriptor (Offset 0x1E). However UEFI and ACPI differs on the
> > definitions of QWORD Address Space Descriptor, and we will follow UEFI
> > definition on UEFI protocols, such as PCI root bridge IO protocol and
> > PCI IO protocol. In UEFI 2.7, "Address Translation Offset" is "Offset
> > to apply to the Starting address to convert it to a PCI address". This
> > means:
> >
> > 1. Translation = device address - host address.
> 
> OK, this looks like a sensible choice to me. It means that the "apply"
> term used in the UEFI spec means "add", which (I think) is the "natural"
> interpretation.
> 
> > 2. PciRootBridgeIo->Configuration should return CPU view address, as
> > well as PciIo->GetBarAttributes.
> 
> OK.
> 
> >
> > Summary of addresses used:
> >
> > 1. Base and Limit in PCI_ROOT_BRIDGE_APERTURE are device address, for
> > it is easy to check whether the address is below 4G or above 4G.
> 
> I agree that PciHostBridgeLib instances that do not set a nonzero
> Translation need not care about the difference.
> 
> (1) Still, can you confirm this is a "natural" choice? Were the
> DmaAbove4G, MemAbove4G and PMemAbove4G fields originally introduced with
> the device (PCI) view in mind?
> 
> ... I also meant to raise a concern about the InitializePciHostBridge()
> function where we call AddIoSpace() and AddMemoryMappedIoSpace() --
> which end up manipulating GCD -- with data from
> PCI_ROOT_BRIDGE_APERTURE. I can see you modify those call sites in the
> patch, so that's good. (I do have more comments on the actual
> implementation.)
> 
> >
> > 2. Address returned by
> > EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL::GetProposedResources
> > is device address, or else PCI bus driver cannot set correct address
> > into PCI BAR registers.
> >
> > 3. Address returned by PciRootBridgeIo->Configuration is host address
> > per UEFI 2.7 definition.
> >
> > 4. Addresses used in GCD manipulation are host address.
> >
> > 5. Addresses in ResAllocNode of PCI_ROOT_BRIDGE_INSTANCE are host
> > address, for they are allocated from GCD.
> >
> > 6. Address passed to PciHostBridgeResourceConflict is host address,
> > for it comes from ResAllocNode.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Heyi Guo 
> > Cc: Ruiyu Ni 
> > Cc: Ard Biesheuvel 
> > Cc: Star Zeng 
> > Cc: Eric Dong 
> > Cc: Laszlo Ersek 
> > Cc: Michael D Kinney 
> > ---
> >  .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c   |  74 +++---
> >  .../Bus/Pci/PciHostBridgeDxe/PciHostResource.h |   2 +
> >  .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 112 
> > ++---
> >  MdeModulePkg/Include/Library/PciHostBridgeLib.h|   8 ++
> >  4 files changed, 167 insertions(+), 29 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c 
> > b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> > index 1494848..e8979eb 100644
> > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> > @@ -32,6 +32,29 @@ EDKII_IOMMU_PROTOCOL*mIoMmuProtocol;
> >  EFI_EVENT   mIoMmuEvent;
> >  VOID*mIoMmuRegistration;
> >
> > +STATIC
> > +UINT64
> > +GetTranslationByResourceType (
> > +  IN  PCI_ROOT_BRIDGE_INSTANCE *RootBridge,
> > +  IN  PCI_RESOURCE_TYPEResourceType
> > +  )
> > +{
> > +  switch (ResourceType) {
> > +case TypeIo:
> > +  return RootBridge->Io.Translation;
> > +case TypeMem32:
> > +  return RootBridge->Mem.Translation;
> > +case TypePMem32:
> > +  return RootBridge->PMem.Translation;
> > +case TypeMem64:
> > +  return RootBridge->MemAbove4G.Translation;
> > +case TypePMem64:
> > +  return RootBridge->PMemAbove4G.Translation;
> > +default:
> 
> (2) How about we return zero for TypeBus, explicitly, and then
> ASSERT(FALSE) and return zero for "default"?
> 
> > +  retu

[edk2] [RFC v4 1/3] MdeModulePkg/PciHostBridgeDxe: Add support for address translation

2018-02-26 Thread Heyi Guo
PCI address translation is necessary for some non-x86 platforms. On
such platforms, address value (denoted as "device address" or "address
in PCI view") set to PCI BAR registers in configuration space might be
different from the address which is used by CPU to access the
registers in memory BAR or IO BAR spaces (denoted as "host address" or
"address in CPU view"). The difference between the two addresses is
called "Address Translation Offset" or simply "translation", and can
be represented by "Address Translation Offset" in ACPI QWORD Address
Space Descriptor (Offset 0x1E). However UEFI and ACPI differs on the
definitions of QWORD Address Space Descriptor, and we will follow UEFI
definition on UEFI protocols, such as PCI root bridge IO protocol and
PCI IO protocol. In UEFI 2.7, "Address Translation Offset" is "Offset
to apply to the Starting address to convert it to a PCI address". This
means:

1. Translation = device address - host address.

2. PciRootBridgeIo->Configuration should return CPU view address, as
well as PciIo->GetBarAttributes.

Summary of addresses used:

1. Base and Limit in PCI_ROOT_BRIDGE_APERTURE are device address, for
it is easy to check whether the address is below 4G or above 4G.

2. Address returned by
EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL::GetProposedResources
is device address, or else PCI bus driver cannot set correct address
into PCI BAR registers.

3. Address returned by PciRootBridgeIo->Configuration is host address
per UEFI 2.7 definition.

4. Addresses used in GCD manipulation are host address.

5. Addresses in ResAllocNode of PCI_ROOT_BRIDGE_INSTANCE are host
address, for they are allocated from GCD.

6. Address passed to PciHostBridgeResourceConflict is host address,
for it comes from ResAllocNode.

RESTRICTION: to simplify the situation, we require the alignment of
Translation must be larger than any BAR alignment in the same root
bridge, so that resource allocation alignment can be applied to both
device address and host address.

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

Notes:
v4:
- Add ASSERT (FALSE) to default branch in GetTranslationByResourceType
  [Laszlo]
- Fix bug when passing BaseAddress to gDS->AllocateIoSpace and
  gDS->AllocateMemorySpace [Laszlo]
- Add comment for applying alignment to both device address and host
  address, and add NOTE for the alignment requirement of Translation,
  as well as in commit message [Laszlo][Ray]
- Improve indention for the code in CreateRootBridge [Laszlo]
- Improve comment for Translation in PCI_ROOT_BRIDGE_APERTURE
  definition [Laszlo]
- Ignore translation of bus in CreateRootBridge

 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostResource.h |   2 +
 MdeModulePkg/Include/Library/PciHostBridgeLib.h |  14 +++
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c   |  85 +++---
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 122 
+---
 4 files changed, 194 insertions(+), 29 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostResource.h 
b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostResource.h
index 8612c0c3251b..662c2dd59529 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostResource.h
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostResource.h
@@ -38,6 +38,8 @@ typedef enum {
 
 typedef struct {
   PCI_RESOURCE_TYPE Type;
+  // Base is a host address instead of a device address when address 
translation
+  // exists and host address != device address
   UINT64Base;
   UINT64Length;
   UINT64Alignment;
diff --git a/MdeModulePkg/Include/Library/PciHostBridgeLib.h 
b/MdeModulePkg/Include/Library/PciHostBridgeLib.h
index d42e9ecdd763..c842a4152e85 100644
--- a/MdeModulePkg/Include/Library/PciHostBridgeLib.h
+++ b/MdeModulePkg/Include/Library/PciHostBridgeLib.h
@@ -20,8 +20,22 @@
 // (Base > Limit) indicates an aperture is not available.
 //
 typedef struct {
+  // Base and Limit are the device address instead of host address when
+  // Translation is not zero
   UINT64 Base;
   UINT64 Limit;
+  // According to UEFI 2.7, Device Address = Host Address + Translation,
+  // so Translation = Device Address - Host Address.
+  // On platforms where Translation is not zero, the subtraction is probably to
+  // be performed with UINT64 wrap-around semantics, for we may translate an
+  // above-4G host address into a below-4G device address for legacy PCIe 
device
+  // compatibility.
+  // NOTE: The alignment of Translation is required to be larger than any BAR
+  // alignment in the same root bridge, so that the same alignment can be
+  // applied to both device address and host address, which simplifies the
+  // situation and makes the current resource allocation code in generic PCI
+  // host bridge driver still work.
+  UI

[edk2] [RFC v4 0/3] Add translation support to generic PciHostBridge

2018-02-26 Thread Heyi Guo
Please note: this is still *RFC* version, so we have not gone thru all the code
in EDK2 for applying the change of PciSegmentLib definition.

v4:
- Modify the code according to the comments from Ray, Laszlo and Ard (Please see
  the notes of Patch 1/3)
- Ignore translation of bus in CreateRootBridge.


v3:
- Keep definition of Translation consistent in EDKII code: Translation = device
  address - host address.
- Patch 2/2 is split into 2 patches (2/3 and 3/3).
- Refine comments and commit messages to make the code easier to understand.


v2:
Changs are made according to the discussion on the mailing list, including:

- PciRootBridgeIo->Configuration should return CPU view address, as well as
  PciIo->GetBarAttributes, and Translation Offset should be equal to PCI view
  address - CPU view address.
- Add translation offset to PCI_ROOT_BRIDGE_APERTURE structure definition.
- PciHostBridge driver internally used Base Address is still based on PCI view
  address, and translation offset = CPU view - PCI view, which follows the
  definition in ACPI, and not the same as that in UEFI spec.

Cc: Ruiyu Ni 
Cc: Ard Biesheuvel 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Laszlo Ersek 
Cc: Michael D Kinney 

Heyi Guo (3):
  MdeModulePkg/PciHostBridgeDxe: Add support for address translation
  MdeModulePkg/PciBus: convert host address to device address
  MdeModulePkg/PciBus: return CPU address for GetBarAttributes

 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostResource.h |   2 +
 MdeModulePkg/Include/Library/PciHostBridgeLib.h |  14 +++
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c  |  12 +-
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c   |  85 +++---
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 122 
+---
 5 files changed, 204 insertions(+), 31 deletions(-)

-- 
2.7.4

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


[edk2] [RFC v4 3/3] MdeModulePkg/PciBus: return CPU address for GetBarAttributes

2018-02-26 Thread Heyi Guo
According to UEFI spec 2.7, PciIo->GetBarAttributes should return host
address (CPU view ddress) rather than device address (PCI view
address), and
device address = host address + address translation offset,
so we subtract translation from device address before returning.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo 
Cc: Ruiyu Ni 
Cc: Ard Biesheuvel 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Laszlo Ersek 
Cc: Michael D Kinney 
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
index fef3eceb7f62..62179eb44bbd 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
@@ -1972,6 +1972,10 @@ PciIoGetBarAttributes (
 return EFI_UNSUPPORTED;
   }
 }
+
+// According to UEFI spec 2.7, we need return host address for
+// PciIo->GetBarAttributes, and host address = device address - 
translation.
+Descriptor->AddrRangeMin -= Descriptor->AddrTranslationOffset;
   }
 
   return EFI_SUCCESS;
-- 
2.7.4

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


[edk2] [RFC v4 2/3] MdeModulePkg/PciBus: convert host address to device address

2018-02-26 Thread Heyi Guo
According to UEFI spec 2.7, PciRootBridgeIo->Configuration() should
return host address (CPU view ddress) rather than device address
(PCI view address), so in function GetMmioAddressTranslationOffset we
need to convert the range to device address before comparing.

And device address = host address + translation offset.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo 
Cc: Ruiyu Ni 
Cc: Ard Biesheuvel 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Laszlo Ersek 
Cc: Michael D Kinney 
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
index 190f4b0dc7ed..fef3eceb7f62 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
@@ -1812,10 +1812,14 @@ GetMmioAddressTranslationOffset (
 return (UINT64) -1;
   }
 
+  // According to UEFI 2.7, EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL::Configuration()
+  // returns host address instead of device address, while 
AddrTranslationOffset
+  // is not zero, and device address = host address + AddrTranslationOffset, so
+  // we convert host address to device address for range compare.
   while (Configuration->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR) {
 if ((Configuration->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) &&
-(Configuration->AddrRangeMin <= AddrRangeMin) &&
-(Configuration->AddrRangeMin + Configuration->AddrLen >= AddrRangeMin 
+ AddrLen)
+(Configuration->AddrRangeMin + Configuration->AddrTranslationOffset <= 
AddrRangeMin) &&
+(Configuration->AddrRangeMin + Configuration->AddrLen + 
Configuration->AddrTranslationOffset >= AddrRangeMin + AddrLen)
 ) {
   return Configuration->AddrTranslationOffset;
 }
-- 
2.7.4

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


Re: [edk2] [PATCH edk2-platforms 1/2] Platform, Silicon: use DxeRuntimeDebugLibSerialPort for runtime DXE drivers

2018-02-26 Thread Guo Heyi
Hi Ard,

Sorry for the late of seeing this patch. I have one question: why don't we
implement a runtime serial port lib, which will switch UART base address in
virtual address map change? I think this will be useful when we want to debug
runtime driver in OS stage. And if we have a runtime version of SerialPortLib,
then we don't need a runtime version of DebugLib which just disable touching
serial port.

Thanks,

Heyi


On Sat, Feb 24, 2018 at 02:25:14PM +, Ard Biesheuvel wrote:
> Commit 4bf95a9f361e ("MdeModulePkg/ResetSystemRuntimeDxe: Add more
> debug message") broke the DEBUG build for all platforms that rely on
> MMIO mapped UART devices, since it introduces a DEBUG() print that
> may trigger at runtime, at which such UART devices are usually not
> mapped, resulting in an OS crash.
> 
> Given that this mostly only affects ARM and AARCH64, it is not unlikely
> that similar inadvertent breakage will occur again in the future, so
> let's fix this once and for all by switching affected platforms to the
> new DxeRuntimeDebugLibSerialPort DebugLib implementation that takes care
> not to touch the UART hardware after ExitBootServices().
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  Platform/AMD/OverdriveBoard/OverdriveBoard.dsc   | 3 +++
>  Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 3 +++
>  Platform/LeMaker/CelloBoard/CelloBoard.dsc   | 3 +++
>  Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 3 +++
>  Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 3 +++
>  Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc  | 3 +++
>  Silicon/Hisilicon/Hisilicon.dsc.inc  | 3 +++
>  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc| 3 +++
>  8 files changed, 24 insertions(+)
> 
> diff --git a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc 
> b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
> index 7d85b78642da..348828e18d44 100644
> --- a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
> +++ b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
> @@ -227,6 +227,9 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
>
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
> +!if $(TARGET) != RELEASE
> +  
> DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
> +!endif
>  
>  [LibraryClasses.common.UEFI_APPLICATION]
>
> UefiDecompressLib|IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.inf
> diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc 
> b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> index b026ce3a420a..7cb47937329e 100644
> --- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> +++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> @@ -227,6 +227,9 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>  !if $(SECURE_BOOT_ENABLE) == TRUE
>BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>  !endif
> +!if $(TARGET) != RELEASE
> +  
> DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
> +!endif
>  
>  [LibraryClasses.ARM, LibraryClasses.AARCH64]
>#
> diff --git a/Platform/LeMaker/CelloBoard/CelloBoard.dsc 
> b/Platform/LeMaker/CelloBoard/CelloBoard.dsc
> index 2468583c0daa..569e444f6b26 100644
> --- a/Platform/LeMaker/CelloBoard/CelloBoard.dsc
> +++ b/Platform/LeMaker/CelloBoard/CelloBoard.dsc
> @@ -211,6 +211,9 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>
> ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
>CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
> +!if $(TARGET) != RELEASE
> +  
> DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
> +!endif
>  
>  [LibraryClasses.common.UEFI_APPLICATION]
>
> UefiDecompressLib|IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.inf
> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc 
> b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> index 8450d7800e43..925ce36d278b 100644
> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> @@ -208,6 +208,9 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
>
> ResetSystemLib|ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
>BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> +!if $(TARGET) != RELEASE
> +  
> DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
> +!endif
>  
>  
> 

[edk2] [patch v2] MdePkg/BaseSafeIntLib: Fix VS2015 IA32 NOOPT build failure

2018-02-26 Thread Dandan Bi
v2: Add [LibraryClasses] section in INF file and refine coding style.

There are VS2015 NOOPT IA32 build failure like below in BaseSafeIntLib.
XXX.lib(XXX.obj): error LNK2001: unresolved external symbol __allmul
XXX.lib(XXX.obj): error LNK2001: unresolved external symbol __allshl
XXX.lib(XXX.obj): error LNK2001: unresolved external symbol __aullshr

This patch replaces direct shift/multiplication of 64-bit integer
with related function call to fix these failure.

Cc: Laszlo Ersek 
Cc: Liming Gao 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf | 3 +++
 MdePkg/Library/BaseSafeIntLib/SafeIntLib.c   | 9 +
 MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c | 3 ++-
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf 
b/MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
index 20a83ed..8fbdafe 100644
--- a/MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
+++ b/MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
@@ -54,5 +54,8 @@
 [Sources.EBC]
   SafeIntLibEbc.c
 
 [Packages]
   MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  BaseLib
diff --git a/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c 
b/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
index c5f13d7..e96327d 100644
--- a/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
+++ b/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
@@ -26,10 +26,11 @@
 
 **/
 
 #include 
 #include 
+#include 
 
 
 //
 // Magnitude of MIN_INT64 as expressed by a UINT64 number.
 //
@@ -3371,12 +3372,12 @@ SafeUint64Mult (
   // a * c must be 0 or there would be bits in the high 64-bits
   // a * d must be less than 2^32 or there would be bits in the high 64-bits
   // b * c must be less than 2^32 or there would be bits in the high 64-bits
   // then there must be no overflow of the resulting values summed up.
   //
-  DwordA = (UINT32)(Multiplicand >> 32);
-  DwordC = (UINT32)(Multiplier >> 32);
+  DwordA = (UINT32)RShiftU64 (Multiplicand, 32);
+  DwordC = (UINT32)RShiftU64 (Multiplier, 32);
 
   //
   // common case -- if high dwords are both zero, no chance for overflow
   //
   if ((DwordA == 0) && (DwordC == 0)) {
@@ -3407,11 +3408,11 @@ SafeUint64Mult (
 if ((ProductBC & 0x) == 0) {
   //
   // now sum them all up checking for overflow.
   // shifting is safe because we already checked for overflow above
   //
-  if (!RETURN_ERROR (SafeUint64Add (ProductBC << 32, ProductAD << 32, 
&UnsignedResult))) {
+  if (!RETURN_ERROR (SafeUint64Add (LShiftU64 (ProductBC, 32), 
LShiftU64 (ProductAD, 32), &UnsignedResult))) {
 //
 // b * d
 //
 ProductBD = (((UINT64)DwordB) *(UINT64)DwordD);
 
@@ -4073,11 +4074,11 @@ SafeInt32Mult (
   IN  INT32  Multiplicand,
   IN  INT32  Multiplier,
   OUT INT32  *Result
   )
 {
-  return SafeInt64ToInt32 (((INT64)Multiplicand) *((INT64)Multiplier), Result);
+  return SafeInt64ToInt32 (MultS64x64 (Multiplicand, Multiplier), Result);
 }
 
 /**
   INT64 multiplication
 
diff --git a/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c 
b/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c
index 18bfb9e..ce66a92 100644
--- a/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c
+++ b/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c
@@ -26,10 +26,11 @@
 
 **/
 
 #include 
 #include 
+#include 
 
 /**
   INT32 -> UINTN conversion
 
   Converts the value specified by Operand to a value specified by Result type
@@ -547,8 +548,8 @@ SafeIntnMult (
   IN  INTN  Multiplicand,
   IN  INTN  Multiplier,
   OUT INTN  *Result
   )
 {
-  return SafeInt64ToIntn (((INT64)Multiplicand) *((INT64)Multiplier), Result);
+  return SafeInt64ToIntn (MultS64x64 (Multiplicand, Multiplier), Result);
 }
 
-- 
1.9.5.msysgit.1

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


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

2018-02-26 Thread Guo Heyi
Hi Laszlo,

I agree the current patch makes the code ugly, and turning the logic into a
normal loop should be the perfect solution. If Ray also agrees on it, I can try
to do that.

Thanks and regards,

Heyi

On Mon, Feb 26, 2018 at 05:23:29PM +0100, Laszlo Ersek wrote:
> On 02/26/18 09:29, 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.
> 
> Not really my place to comment on this, but how about removing the
> recursion entirely, and turning the logic into a normal (iterative) loop
> instead?
> 
> (1) Rename the current function to:
> 
> STATIC
> VOID
> BmRepairAllControllersWorker (
>   OUT BOOLEAN *ReconnectRequired,
>   OUT BOOLEAN *RebootRequired
>   );
> 
> 
> (2) The worker function should end right before
> 
>   if (ReconnectRequired) {
> BmRepairAllControllers ();
>   }
> 
> 
> (3) The worker function should not contain
> 
>   RebootRequired= FALSE;
>   ReconnectRequired = FALSE;
> 
> Such initialization should be left to the caller.
> 
> 
> (4) The worker function should be called in a loop from a new
> BmRepairAllControllers() function, like this:
> 
>   Reconnect = 0;
>   RebootRequired = FALSE;
>   do {
> ReconnectRequired = FALSE;
> BmRepairAllControllersWorker (&ReconnectRequired, &RebootRequired);
> ++Reconnect;
>   } while (ReconnectRequired && Reconnect < MAX_RECONNECT_REPAIR);
> 
>   DEBUG_CODE (...);
> 
>   if (RebootRequired) {
> ...
>   }
> 
> 
> In addition to eliminating the shoddy recursive call (and the shoddier
> global counter, ewww :) ), this would fix the following other warts with
> the code:
> 
> - When a nested call chain is unwound, we currently dump a series of
> "driver health info" lists (assuming no reboot is required), in the
> DEBUG_CODE section. I would argue that we should do that only once, at
> the end. (Even if we have to do it multiple times, it can be moved into
> the worker function, to the end.)
> 
> - It seems to be sufficient to accumulate RebootRequired into one
> variable (i.e. not multiple instances of the same local variable on the
> stack) and to act upon it at the very end.
> 
> 
> Feel free to ignore my comments -- I just think we should be moving in
> the opposite direction; that is, away from recursion (especially from
> recursion combined with global variables -- that's one difficult pattern
> to reason about).
> 
> Thanks
> Laszlo
> 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Heyi Guo 
> > Cc: Star Zeng 
> > Cc: Eric Dong 
> > Cc: Ruiyu Ni 
> > ---
> >  MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h |  6 ++
> >  MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c | 17 
> > -
> >  2 files changed, 22 insertions(+), 1 deletion(-)
> > 
> > diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h 
> > b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> > index 25a1d522fe84..9aa86b096525 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.
> > diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c 
> > b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
> > index ddcee8b0676f..30d70f32af84 100644
> > --- a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
> > +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
> > @@ -26,6 +26,12 @@ GLOBAL_REMOVE_IF_UNREFERENCED
> >  L"Reboot Required"
> >  };
> >  
> > +//
> > +// Counter of reconnect retry to repair controller; it is to limit the
> > +// number of recursive call of BmRepairAllControllers.
> > +//
> > +STATIC UINTN mReconnectRepairCount;
> > +
> >  /**
> >Return the controller name.
> >  
> > @@ -549,7 +555,16 @@ BmRepairAllControllers (
> >  
> >  
> >if (ReconnectRequired) {
> > -BmRepairAllControllers ();
> > +if (mReconnectRepairCount < MAX_RECONNECT_REPAIR) {
> > +  mReconnectRepairCount++;
> > +  BmRepairAllControllers (

Re: [edk2] [RFC] Add Platform Include path in modules

2018-02-26 Thread Kinney, Michael D
Hi Pankaj,

I agree with Laszlo that you should evaluate use of 
PCDs.  There are a few methods for a driver to use
platform specific values/behavior.  These are:

* PCDs
* Library class/Library Instance
* Protocol/PPI

One issue with the proposal is that it adds a hidden
dependency to modules.  An EDK II INF file describes
the external interfaces of a module along with
produces/consumes usage.  This information is aligned
with the XML schema that is documented in the UEFI
Platform Initialization Distribution Packaging Specification.

  http://uefi.org/specifications

If two modules have the same GUID/Version, then the external
interfaces to those two modules are expected to be identical.
With your proposal, two modules built for 2 different platforms
would have the same GUID/Version but would not have the same
external interfaces because a hidden dependency on a platform
package was added.

If a module really needs to use content from a platform
package, then a new copy of the module should be created
with a new GUID/Version and the platform package added to
the [Packages] section.  The other option is to use one
of the supported interfaces (PCDs, Lib, Protocol, PPI).

Please let us know if any of these exiting methods do not
work for your use case.

Thanks,

Mike

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Monday, February 26, 2018 7:55 AM
> To: Pankaj Bansal ; Kinney,
> Michael D ; edk2-
> de...@lists.01.org
> Cc: Gao, Liming 
> Subject: Re: [edk2] [RFC] Add Platform Include path in
> modules
> 
> On 02/26/18 11:55, Pankaj Bansal wrote:
> > Hi,
> >
> > Consider a simple driver which needs that some data
> structures be
> > filled by the Platform, which is using the driver.
> >
> > Driver.c #include 
> >
> > Struct a = platformVal;
> >
> > We can define platformVal in Platform.h, which would
> be unique to the
> > platform being built. This Platform.h can be placed in
> include
> > directories, whose path would be defined in
> Platform.dec file.
> >
> > Now, whenever we build driver for each unique
> platform, we need not
> > to mention Platform.dec file in driver.inf [packages]
> section. We can
> > append Platform.dec include paths to each driver. i.e.
> look for the
> > include files in [packages] section as well as in
> Platform include
> > directories.
> >
> > For this, I am looking for Platform.dec file in same
> directory as
> > Platform.dsc and using same name as Platform.dsc
> >
> > We can refine this change further. i.e. add Platform
> include
> > directories to driver's include paths based on some
> condition in
> > driver.inf file.
> 
> (Apologies in advance if I failed to grasp the use
> case.)
> 
> If I understand correctly, you have multiple platforms
> (defined by DSC
> and FDF files), and you build a given driver for several
> of these
> platforms, separately. And, when building the driver for
> the separate
> platforms, you'd like the driver to get different
> initializers for
> various static (global) structure variables.
> 
> Have you tried the structured PCD format? I think that
> could cover your
> use case.
> 
> Unfortunately I couldn't find anything about structured
> PCDs in the edk2
> specs, but there are several BZ references in the
> following mailing list
> message:
> 
> [edk2] [Patch 00/14] Enable Structure PCD support in
> edk2
> 
> http://mid.mail-archive.com/1512140335-6932-1-git-send-
> email-liming@intel.com
> 
> Thanks
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


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

2018-02-26 Thread Laszlo Ersek
On 02/26/18 09:29, 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.

Not really my place to comment on this, but how about removing the
recursion entirely, and turning the logic into a normal (iterative) loop
instead?

(1) Rename the current function to:

STATIC
VOID
BmRepairAllControllersWorker (
  OUT BOOLEAN *ReconnectRequired,
  OUT BOOLEAN *RebootRequired
  );


(2) The worker function should end right before

  if (ReconnectRequired) {
BmRepairAllControllers ();
  }


(3) The worker function should not contain

  RebootRequired= FALSE;
  ReconnectRequired = FALSE;

Such initialization should be left to the caller.


(4) The worker function should be called in a loop from a new
BmRepairAllControllers() function, like this:

  Reconnect = 0;
  RebootRequired = FALSE;
  do {
ReconnectRequired = FALSE;
BmRepairAllControllersWorker (&ReconnectRequired, &RebootRequired);
++Reconnect;
  } while (ReconnectRequired && Reconnect < MAX_RECONNECT_REPAIR);

  DEBUG_CODE (...);

  if (RebootRequired) {
...
  }


In addition to eliminating the shoddy recursive call (and the shoddier
global counter, ewww :) ), this would fix the following other warts with
the code:

- When a nested call chain is unwound, we currently dump a series of
"driver health info" lists (assuming no reboot is required), in the
DEBUG_CODE section. I would argue that we should do that only once, at
the end. (Even if we have to do it multiple times, it can be moved into
the worker function, to the end.)

- It seems to be sufficient to accumulate RebootRequired into one
variable (i.e. not multiple instances of the same local variable on the
stack) and to act upon it at the very end.


Feel free to ignore my comments -- I just think we should be moving in
the opposite direction; that is, away from recursion (especially from
recursion combined with global variables -- that's one difficult pattern
to reason about).

Thanks
Laszlo

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Heyi Guo 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Ruiyu Ni 
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h |  6 ++
>  MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c | 17 
> -
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h 
> b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> index 25a1d522fe84..9aa86b096525 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.
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c 
> b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
> index ddcee8b0676f..30d70f32af84 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
> @@ -26,6 +26,12 @@ GLOBAL_REMOVE_IF_UNREFERENCED
>  L"Reboot Required"
>  };
>  
> +//
> +// Counter of reconnect retry to repair controller; it is to limit the
> +// number of recursive call of BmRepairAllControllers.
> +//
> +STATIC UINTN mReconnectRepairCount;
> +
>  /**
>Return the controller name.
>  
> @@ -549,7 +555,16 @@ BmRepairAllControllers (
>  
>  
>if (ReconnectRequired) {
> -BmRepairAllControllers ();
> +if (mReconnectRepairCount < MAX_RECONNECT_REPAIR) {
> +  mReconnectRepairCount++;
> +  BmRepairAllControllers ();
> +} else {
> +  DEBUG ((DEBUG_ERROR, "[%a:%d] Repair failed after %d retries.\n",
> +__FUNCTION__, __LINE__, mReconnectRepairCount));
> +  // Reset counter so that it will not affect calling
> +  // BmRepairAllControllers() somewhere else
> +  mReconnectRepairCount = 0;
> +}
>}
>  
>DEBUG_CODE (
> 

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


Re: [edk2] [RFC] Add Platform Include path in modules

2018-02-26 Thread Laszlo Ersek
On 02/26/18 11:55, Pankaj Bansal wrote:
> Hi,
> 
> Consider a simple driver which needs that some data structures be
> filled by the Platform, which is using the driver.
> 
> Driver.c #include 
> 
> Struct a = platformVal;
> 
> We can define platformVal in Platform.h, which would be unique to the
> platform being built. This Platform.h can be placed in include
> directories, whose path would be defined in Platform.dec file.
> 
> Now, whenever we build driver for each unique platform, we need not
> to mention Platform.dec file in driver.inf [packages] section. We can
> append Platform.dec include paths to each driver. i.e. look for the
> include files in [packages] section as well as in Platform include
> directories.
> 
> For this, I am looking for Platform.dec file in same directory as
> Platform.dsc and using same name as Platform.dsc
> 
> We can refine this change further. i.e. add Platform include
> directories to driver's include paths based on some condition in
> driver.inf file.

(Apologies in advance if I failed to grasp the use case.)

If I understand correctly, you have multiple platforms (defined by DSC
and FDF files), and you build a given driver for several of these
platforms, separately. And, when building the driver for the separate
platforms, you'd like the driver to get different initializers for
various static (global) structure variables.

Have you tried the structured PCD format? I think that could cover your
use case.

Unfortunately I couldn't find anything about structured PCDs in the edk2
specs, but there are several BZ references in the following mailing list
message:

[edk2] [Patch 00/14] Enable Structure PCD support in edk2

http://mid.mail-archive.com/1512140335-6932-1-git-send-email-liming.gao@intel.com

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


Re: [edk2] [patch] MdePkg/BaseSafeIntLib: Fix VS2015 IA32 NOOPT build failure

2018-02-26 Thread Laszlo Ersek
On 02/26/18 07:22, Dandan Bi wrote:
> There are VS2015 NOOPT IA32 build failure like below in BaseSafeIntLib.
> XXX.lib(XXX.obj): error LNK2001: unresolved external symbol __allmul
> XXX.lib(XXX.obj): error LNK2001: unresolved external symbol __allshl
> XXX.lib(XXX.obj): error LNK2001: unresolved external symbol __aullshr
> 
> This patch replaces direct shift/multiplication of 64-bit integer
> with related function call to fix these failure.
> 
> Cc: Liming Gao 
> Cc: Michael Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi 
> ---
>  MdePkg/Library/BaseSafeIntLib/SafeIntLib.c   | 9 +
>  MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c | 3 ++-
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c 
> b/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
> index c5f13d7..ccffbb6 100644
> --- a/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
> +++ b/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
> @@ -26,10 +26,11 @@
>  
>  **/
>  
>  #include 
>  #include 
> +#include 

(1) Can you introduce a [LibraryClasses] section to the INF file, and
list BaseLib there?

>  
>  
>  //
>  // Magnitude of MIN_INT64 as expressed by a UINT64 number.
>  //
> @@ -3371,12 +3372,12 @@ SafeUint64Mult (
>// a * c must be 0 or there would be bits in the high 64-bits
>// a * d must be less than 2^32 or there would be bits in the high 64-bits
>// b * c must be less than 2^32 or there would be bits in the high 64-bits
>// then there must be no overflow of the resulting values summed up.
>//
> -  DwordA = (UINT32)(Multiplicand >> 32);
> -  DwordC = (UINT32)(Multiplier >> 32);
> +  DwordA = (UINT32)RShiftU64 (Multiplicand, 32);
> +  DwordC = (UINT32)RShiftU64 (Multiplier, 32);
>  
>//
>// common case -- if high dwords are both zero, no chance for overflow
>//
>if ((DwordA == 0) && (DwordC == 0)) {
> @@ -3407,11 +3408,11 @@ SafeUint64Mult (
>  if ((ProductBC & 0x) == 0) {
>//
>// now sum them all up checking for overflow.
>// shifting is safe because we already checked for overflow above
>//
> -  if (!RETURN_ERROR (SafeUint64Add (ProductBC << 32, ProductAD << 
> 32, &UnsignedResult))) {
> +  if (!RETURN_ERROR (SafeUint64Add (LShiftU64(ProductBC, 32), 
> LShiftU64(ProductAD, 32), &UnsignedResult))) {

(2) This change is correct, but the style is not perfect. Please insert
a space character right after each "LShiftU64" identifier.


>  //
>  // b * d
>  //
>  ProductBD = (((UINT64)DwordB) *(UINT64)DwordD);
>  
> @@ -4073,11 +4074,11 @@ SafeInt32Mult (
>IN  INT32  Multiplicand,
>IN  INT32  Multiplier,
>OUT INT32  *Result
>)
>  {
> -  return SafeInt64ToInt32 (((INT64)Multiplicand) *((INT64)Multiplier), 
> Result);
> +  return SafeInt64ToInt32 (MultS64x64((INT64)Multiplicand, 
> (INT64)Multiplier), Result);
>  }

(3) This can be simplified. The prototype of MultS64x64() is visible
here, so you can drop the explicit INT64 casts.

(4) Also, please insert a space character after "MultS64x64".

(5) Yet another comment: I'm surprised that, while this 64-bit
multiplication is flagged, several 64-bit (unsigned) multiplications are
(apparently) not, in SafeUint64Mult():

  3386  *Result = (((UINT64)DwordB) *(UINT64)DwordD);

  3399ProductAD = (((UINT64)DwordA) *(UINT64)DwordD);

  3406  ProductBC = (((UINT64)DwordB) *(UINT64)DwordC);

  3416  ProductBD = (((UINT64)DwordB) *(UINT64)DwordD);

>  
>  /**
>INT64 multiplication
>  
> diff --git a/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c 
> b/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c
> index 18bfb9e..5e30db4 100644
> --- a/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c
> +++ b/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c
> @@ -26,10 +26,11 @@
>  
>  **/
>  
>  #include 
>  #include 
> +#include 
>  
>  /**
>INT32 -> UINTN conversion
>  
>Converts the value specified by Operand to a value specified by Result type
> @@ -547,8 +548,8 @@ SafeIntnMult (
>IN  INTN  Multiplicand,
>IN  INTN  Multiplier,
>OUT INTN  *Result
>)
>  {
> -  return SafeInt64ToIntn (((INT64)Multiplicand) *((INT64)Multiplier), 
> Result);
> +  return SafeInt64ToIntn (MultS64x64((INT64)Multiplicand, 
> (INT64)Multiplier), Result);
>  }
>  
> 

(6) Comments (3) and (4) apply here.

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


Re: [edk2] [PATCH edk2-platforms 1/2] Platform, Silicon: use DxeRuntimeDebugLibSerialPort for runtime DXE drivers

2018-02-26 Thread Laszlo Ersek
On 02/24/18 15:25, Ard Biesheuvel wrote:
> Commit 4bf95a9f361e ("MdeModulePkg/ResetSystemRuntimeDxe: Add more
> debug message") broke the DEBUG build for all platforms that rely on
> MMIO mapped UART devices, since it introduces a DEBUG() print that
> may trigger at runtime, at which such UART devices are usually not
> mapped, resulting in an OS crash.
> 
> Given that this mostly only affects ARM and AARCH64, it is not unlikely
> that similar inadvertent breakage will occur again in the future, so
> let's fix this once and for all by switching affected platforms to the
> new DxeRuntimeDebugLibSerialPort DebugLib implementation that takes care
> not to touch the UART hardware after ExitBootServices().
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  Platform/AMD/OverdriveBoard/OverdriveBoard.dsc   | 3 +++
>  Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 3 +++
>  Platform/LeMaker/CelloBoard/CelloBoard.dsc   | 3 +++
>  Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 3 +++
>  Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 3 +++
>  Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc  | 3 +++
>  Silicon/Hisilicon/Hisilicon.dsc.inc  | 3 +++
>  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc| 3 +++
>  8 files changed, 24 insertions(+)
> 
> diff --git a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc 
> b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
> index 7d85b78642da..348828e18d44 100644
> --- a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
> +++ b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
> @@ -227,6 +227,9 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
>
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
> +!if $(TARGET) != RELEASE
> +  
> DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
> +!endif
>  
>  [LibraryClasses.common.UEFI_APPLICATION]
>
> UefiDecompressLib|IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.inf
> diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc 
> b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> index b026ce3a420a..7cb47937329e 100644
> --- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> +++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> @@ -227,6 +227,9 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>  !if $(SECURE_BOOT_ENABLE) == TRUE
>BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>  !endif
> +!if $(TARGET) != RELEASE
> +  
> DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
> +!endif
>  
>  [LibraryClasses.ARM, LibraryClasses.AARCH64]
>#
> diff --git a/Platform/LeMaker/CelloBoard/CelloBoard.dsc 
> b/Platform/LeMaker/CelloBoard/CelloBoard.dsc
> index 2468583c0daa..569e444f6b26 100644
> --- a/Platform/LeMaker/CelloBoard/CelloBoard.dsc
> +++ b/Platform/LeMaker/CelloBoard/CelloBoard.dsc
> @@ -211,6 +211,9 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>
> ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
>CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
> +!if $(TARGET) != RELEASE
> +  
> DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
> +!endif
>  
>  [LibraryClasses.common.UEFI_APPLICATION]
>
> UefiDecompressLib|IntelFrameworkModulePkg/Library/BaseUefiTianoCustomDecompressLib/BaseUefiTianoCustomDecompressLib.inf
> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc 
> b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> index 8450d7800e43..925ce36d278b 100644
> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> @@ -208,6 +208,9 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
>
> ResetSystemLib|ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
>BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> +!if $(TARGET) != RELEASE
> +  
> DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
> +!endif
>  
>  
> 
>  #
> diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc 
> b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> index 45ab2afc4069..2d5a94ed1dab 100644
> --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> @@ -203,6 +203,9 @@ [LibraryClasses.c

[edk2] [PATCH] MdePkg/Include/IndustryStandard: Add PCI Express 4.0 header file

2018-02-26 Thread Felix Polyudov
The header includes Physical Layer PCI Express Extended Capability definitions
described in section 7.7.5 of PCI Express Base Specification rev. 4.0.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Felix Polyudov 
---
 MdePkg/Include/IndustryStandard/PciExpress40.h | 69 ++
 1 file changed, 69 insertions(+)
 create mode 100644 MdePkg/Include/IndustryStandard/PciExpress40.h

diff --git a/MdePkg/Include/IndustryStandard/PciExpress40.h 
b/MdePkg/Include/IndustryStandard/PciExpress40.h
new file mode 100644
index 000..e0f4323
--- /dev/null
+++ b/MdePkg/Include/IndustryStandard/PciExpress40.h
@@ -0,0 +1,69 @@
+/** @file
+Support for the PCI Express 4.0 standard.
+
+This header file may not define all structures.  Please extend as required.
+
+Copyright (c) 2018, American Megatrends, Inc. 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 _PCIEXPRESS40_H_
+#define _PCIEXPRESS40_H_
+
+#include 
+
+#pragma pack(1)
+
+/// The Physical Layer PCI Express Extended Capability definitions.
+///
+/// Based on section 7.7.5 of PCI Express Base Specification 4.0.
+///@{
+#define PCI_EXPRESS_EXTENDED_CAPABILITY_PHYSICAL_LAYER_16_0_ID0x0026
+#define PCI_EXPRESS_EXTENDED_CAPABILITY_PHYSICAL_LAYER_16_0_VER1  0x1
+
+// Register offsets from Physical Layer PCI-E Ext Cap Header
+#define PCI_EXPRESS_REG_PHYSICAL_LAYER_16_0_CAPABILITIES_OFFSET
 0x04 
+#define PCI_EXPRESS_REG_PHYSICAL_LAYER_16_0_CONTROL_OFFSET 
 0x08
+#define PCI_EXPRESS_REG_PHYSICAL_LAYER_16_0_STATUS_OFFSET  
 0x0C
+#define PCI_EXPRESS_REG_PHYSICAL_LAYER_16_0_LOCAL_DATA_PARITY_STATUS_OFFSET
 0x10
+#define 
PCI_EXPRESS_REG_PHYSICAL_LAYER_16_0_FIRST_RETIMER_DATA_PARITY_STATUS_OFFSET 
0x14
+#define 
PCI_EXPRESS_REG_PHYSICAL_LAYER_16_0_SECOND_RETIMER_DATA_PARITY_STATUS_OFFSET
0x18
+#define PCI_EXPRESS_REG_PHYSICAL_LAYER_16_0_LANE_EQUALIZATION_CONTROL_OFFSET   
 0x20
+
+typedef union {
+  struct {
+UINT32 EqualizationComplete  : 1; // bit 0
+UINT32 EqualizationPhase1Success : 1; // bit 1
+UINT32 EqualizationPhase2Success : 1; // bit 2
+UINT32 EqualizationPhase3Success : 1; // bit 3
+UINT32 LinkEqualizationRequest   : 1; // bit 4
+UINT32 Reserved  : 27; // Reserved bit 5:31
+  } Bits;
+  UINT32   Uint32;
+} PCI_EXPRESS_REG_PHYSICAL_LAYER_16_0_STATUS;
+
+typedef union {
+  struct {
+UINT8 DownstreamPortTransmitterPreset : 4; //bit 0..3
+UINT8 UpstreamPortTransmitterPreset   : 4; //bit 4..7
+  } Bits;
+  UINT8   Uint8;
+} PCI_EXPRESS_REG_PHYSICAL_LAYER_16_0_LANE_EQUALIZATION_CONTROL;
+
+typedef struct {
+  PCI_EXPRESS_EXTENDED_CAPABILITIES_HEADER   Header;
+  PCI_EXPRESS_REG_PHYSICAL_LAYER_16_0_STATUS Status;
+  PCI_EXPRESS_REG_PHYSICAL_LAYER_16_0_LANE_EQUALIZATION_CONTROL   Control;
+} PCI_EXPRESS_EXTENDED_CAPABILITIES_PHYSICAL_LAYER_16_0;
+///@}
+
+#pragma pack()
+
+#endif
-- 
2.10.0.windows.1



Please consider the environment before printing this email.

The information contained in this message may be confidential and proprietary 
to American Megatrends, Inc.  This communication is intended to be read only by 
the individual or entity to whom it is addressed or by their designee. If the 
reader of this message is not the intended recipient, you are on notice that 
any distribution of this message, in any form, is strictly prohibited.  Please 
promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and 
then delete or destroy all copies of the transmission.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [RFC v2] Add Platform Include path in modules

2018-02-26 Thread Pankaj Bansal
When we are writing the drivers for IP modules, then sometimes we want
that Platform specific customizations or platform dependent values be
supplied to IP module driver. normally we achieve this using Pcd values.

But sometimes we want to use header files for such data.e.g. if the
values are complex structures.

we need a mechanism that platform be able to supply these header files
to a module, without changing module code.

This patch is an attempt to achieve this functionality.

Cc: Yonghong Zhu 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Pankaj Bansal 
---

Notes:
V2:
 -check if .dec file exist before appending include paths to include path 
list

 BaseTools/Source/Python/AutoGen/AutoGen.py | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 405bfa1..4b281d8 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -3783,6 +3783,20 @@ class ModuleAutoGen(AutoGen):
 for Inc in IncludesList:
 if Inc not in self._IncludePathList:
 self._IncludePathList.append(str(Inc))
+PackageFilePath = os.path.join(self.PlatformInfo.MetaFile.SubDir, 
self.PlatformInfo.MetaFile.BaseName + '.dec')
+if 
os.path.isfile(os.path.normpath(mws.join(self.PlatformInfo.MetaFile.Root, 
PackageFilePath))):
+PackageFile = PathClass(PackageFilePath, 
self.PlatformInfo.MetaFile.Root)
+Package = self.BuildDatabase[PackageFile, self.Arch, 
self.BuildTarget, self.ToolChain]
+PackageDir = mws.join(self.WorkspaceDir, Package.MetaFile.Dir)
+if PackageDir not in self._IncludePathList:
+self._IncludePathList.append(PackageDir)
+IncludesList = Package.Includes
+if Package._PrivateIncludes:
+if not self.MetaFile.Path.startswith(PackageDir):
+IncludesList = 
list(set(Package.Includes).difference(set(Package._PrivateIncludes)))
+for Inc in IncludesList:
+if Inc not in self._IncludePathList:
+self._IncludePathList.append(str(Inc))
 return self._IncludePathList
 
 def _GetIncludePathLength(self):
-- 
2.7.4

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


[edk2] [PATCH] BaseTools: Fix flexible PCD single quote and double quote bugs

2018-02-26 Thread Feng, YunhuaX
1. Argument --pcd format as below:
  Some examples that to match --pcd format and DSC format
  --pcd DSC format
  L"ABC"L"ABC"
  "AB\\\"C" "AB\"C"
  "AB\tC"  "AB\tC"
  "AB\\\'C" "AB\'C"
  "\'ABC\'" 'ABC’
  L"\'AB\\\"C\'"L'AB\"C'
  "\'AB\\\'C\'" 'AB\'C'
  "\'AB\tC\'"  'AB\tC'
  H"{0, L\"AB\\\"B\", \'ab\\\"c\'}" {0, L"AB\"B", 'ab\"c'}
  H"{0, L\"AB\\\"B\",\'ab\\\'c\'}"  {0, L"AB\"B", 'ab\'c'}
2. {'#', '|'} split string incorrectly.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: Yunhua Feng 
---
 BaseTools/Source/Python/AutoGen/GenMake.py| 20 +---
 BaseTools/Source/Python/Common/Expression.py  | 60 ---
 BaseTools/Source/Python/Common/Misc.py| 31 +++-
 BaseTools/Source/Python/Common/String.py  | 33 -
 BaseTools/Source/Python/Workspace/DscBuildData.py | 37 +++---
 5 files changed, 115 insertions(+), 66 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
b/BaseTools/Source/Python/AutoGen/GenMake.py
index afe6f2f99c..4b924d21e0 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -1553,17 +1553,23 @@ class TopLevelMakefile(BuildFile):
 
 if GlobalData.BuildOptionPcd:
 for index, option in enumerate(GlobalData.gCommand):
 if "--pcd" == option and GlobalData.gCommand[index+1]:
 pcdName, pcdValue = GlobalData.gCommand[index+1].split('=')
-if pcdValue.startswith('H'):
-pcdValue = 'H' + '"' + pcdValue[1:] + '"'
-ExtraOption += " --pcd " + pcdName + '=' + pcdValue
-elif pcdValue.startswith("L'"):
-ExtraOption += "--pcd " + pcdName + '=' + pcdValue
-elif pcdValue.startswith('L'):
-pcdValue = 'L' + '"' + pcdValue[1:] + '"'
+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]
 
 MakefileName = self._FILE_NAME_[self._FileType]
diff --git a/BaseTools/Source/Python/Common/Expression.py 
b/BaseTools/Source/Python/Common/Expression.py
index edb0a60de6..f1516d5c7b 100644
--- a/BaseTools/Source/Python/Common/Expression.py
+++ b/BaseTools/Source/Python/Common/Expression.py
@@ -43,28 +43,41 @@ ERR_IN_OPERAND  = 'Macro after IN operator can only 
be: $(FAMILY), $(ARC
 ## SplitString
 #  Split string to list according double quote
 #  For example: abc"de\"f"ghi"jkl"mn will be: ['abc', '"de\"f"', 'ghi', 
'"jkl"', 'mn']
 #
 def SplitString(String):
-# There might be escaped quote: "abc\"def\\\"ghi"
-Str = String.replace('', '//').replace('\\\"', '\\\'')
+# There might be escaped quote: "abc\"def\\\"ghi", 'abc\'def\\\'ghi'
+Str = String
 RetList = []
-InQuote = False
+InSingleQuote = False
+InDoubleQuote = False
 Item = ''
 for i, ch in enumerate(Str):
-if ch == '"':
-InQuote = not InQuote
-if not InQuote:
+if ch == '"' and not InSingleQuote:
+if Str[i - 1] != '\\':
+InDoubleQuote = not InDoubleQuote
+if not InDoubleQuote:
+Item += String[i]
+RetList.append(Item)
+Item = ''
+continue
+if Item:
+RetList.append(Item)
+Item = ''
+elif ch == "'" and not InDoubleQuote:
+if Str[i - 1] != '\\':
+InSingleQuote = not InSingleQuote
+if not InSingleQuote:
 Item += String[i]
 RetList.append(Item)
 Item = ''
 continue
 if Item:
 RetList.append(Item)
 Item = ''
 Item += String[i]
-if InQuote:
+if InSin

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

2018-02-26 Thread Guo Heyi
Hi Sunny,

I didn't consider it as a value necessary for platform override, for the retry
count should only have some impact on boot performance and it only happens when
there is something wrong.

May I know what value you will use for your platform and why?

Thanks and regards,

Gary (Heyi Guo)

On Mon, Feb 26, 2018 at 08:56:50AM +, Wang, Sunny (HPS SW) wrote:
> Hi Heyi, 
> Just a suggestion. 
> Is it better to use a PCD instead of a define for MAX_RECONNECT_REPAIR? So 
> that we can easily override it in our platform dsc file.
> 
> Regards,
> Sunny Wang
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Heyi 
> Guo
> Sent: Monday, February 26, 2018 4:30 PM
> To: edk2-devel@lists.01.org
> Cc: Ruiyu Ni ; Heyi Guo ; Eric Dong 
> ; Star Zeng 
> Subject: [edk2] [PATCH 1/1] MdeModulePkg/UefiBootManagerLib: limit recursive 
> call depth
> 
> 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.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Heyi Guo 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Ruiyu Ni 
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h |  6 ++
>  MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c | 17 
> -
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h 
> b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> index 25a1d522fe84..9aa86b096525 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.
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c 
> b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
> index ddcee8b0676f..30d70f32af84 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
> @@ -26,6 +26,12 @@ GLOBAL_REMOVE_IF_UNREFERENCED
>  L"Reboot Required"
>  };
>  
> +//
> +// Counter of reconnect retry to repair controller; it is to limit the 
> +// number of recursive call of BmRepairAllControllers.
> +//
> +STATIC UINTN mReconnectRepairCount;
> +
>  /**
>Return the controller name.
>  
> @@ -549,7 +555,16 @@ BmRepairAllControllers (
>  
>  
>if (ReconnectRequired) {
> -BmRepairAllControllers ();
> +if (mReconnectRepairCount < MAX_RECONNECT_REPAIR) {
> +  mReconnectRepairCount++;
> +  BmRepairAllControllers ();
> +} else {
> +  DEBUG ((DEBUG_ERROR, "[%a:%d] Repair failed after %d retries.\n",
> +__FUNCTION__, __LINE__, mReconnectRepairCount));
> +  // Reset counter so that it will not affect calling
> +  // BmRepairAllControllers() somewhere else
> +  mReconnectRepairCount = 0;
> +}
>}
>  
>DEBUG_CODE (
> --
> 2.7.4
> 
> ___
> 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] Add Platform Include path in modules

2018-02-26 Thread Pankaj Bansal
Hi,

Consider a simple driver which needs that some data structures be filled by the
Platform, which is using the driver.

Driver.c
#include 

Struct a = platformVal;

We can define platformVal in Platform.h, which would be unique to the platform 
being built.
This Platform.h can be placed in include directories, whose path would be 
defined in Platform.dec file.

Now, whenever we build driver for each unique platform, we need not to mention 
Platform.dec file in driver.inf [packages] section.
We can append Platform.dec include paths to each driver. i.e. look for the 
include files in [packages] section as well as in Platform include directories.

For this, I am looking for Platform.dec file in same directory as Platform.dsc 
and using same name as Platform.dsc

We can refine this change further. i.e. add Platform include directories to 
driver's include paths based on some condition in driver.inf file.

Thanks & Regards,
Pankaj Bansal

> -Original Message-
> From: Kinney, Michael D [mailto:michael.d.kin...@intel.com]
> Sent: Monday, February 26, 2018 1:13 PM
> To: Pankaj Bansal ; edk2-devel@lists.01.org;
> Kinney, Michael D 
> Cc: Gao, Liming 
> Subject: RE: [edk2] [RFC] Add Platform Include path in modules
> 
> Hi,
> 
> Can you provide a simple example that shows how this feature is used and
> how it works?
> 
> Thanks,
> 
> Mike
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-
> > boun...@lists.01.org] On Behalf Of Pankaj Bansal
> > Sent: Sunday, February 25, 2018 10:29 PM
> > To: edk2-devel@lists.01.org
> > Cc: Gao, Liming 
> > Subject: [edk2] [RFC] Add Platform Include path in modules
> >
> > When we are writing the drivers for IP modules, then sometimes we want
> > that Platform specific customizations or platform dependent values be
> > supplied to IP module driver. normally we achieve this using Pcd
> > values.
> >
> > But sometimes we want to use header files for such data.e.g. if the
> > values are complex structures.
> >
> > we need a mechanism that platform be able to supply these header files
> > to a module, without changing module code.
> >
> > This patch is an attempt to achieve this functionality.
> >
> > Cc: Yonghong Zhu 
> > Cc: Liming Gao 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Pankaj Bansal 
> > ---
> >  BaseTools/Source/Python/AutoGen/AutoGen.py | 12
> > 
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> > b/BaseTools/Source/Python/AutoGen/AutoGen.py
> > index 405bfa1..de4a17c 100644
> > --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> > +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> > @@ -3783,6 +3783,18 @@ class ModuleAutoGen(AutoGen):
> >  for Inc in IncludesList:
> >  if Inc not in
> > self._IncludePathList:
> >
> > self._IncludePathList.append(str(Inc))
> > +PackageFile =
> > PathClass(os.path.join(self.PlatformInfo.MetaFile.SubDir
> > , self.PlatformInfo.MetaFile.BaseName + '.dec'),
> > self.PlatformInfo.MetaFile.Root)
> > +Package = self.BuildDatabase[PackageFile,
> > self.Arch, self.BuildTarget, self.ToolChain]
> > +PackageDir = mws.join(self.WorkspaceDir,
> > Package.MetaFile.Dir)
> > +if PackageDir not in self._IncludePathList:
> > +
> > self._IncludePathList.append(PackageDir)
> > +IncludesList = Package.Includes
> > +if Package._PrivateIncludes:
> > +if not
> > self.MetaFile.Path.startswith(PackageDir):
> > +IncludesList =
> > list(set(Package.Includes).difference(set(Package._Priva
> > teIncludes)))
> > +for Inc in IncludesList:
> > +if Inc not in self._IncludePathList:
> > +
> > self._IncludePathList.append(str(Inc))
> >  return self._IncludePathList
> >
> >  def _GetIncludePathLength(self):
> > --
> > 2.7.4
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> > ts.01.org%2Fmailman%2Flistinfo%2Fedk2-
> devel&data=02%7C01%7Cpankaj.bans
> >
> al%40nxp.com%7Ccaec1b6b479b4111c26308d57cec86ac%7C686ea1d3bc2b4
> c6fa92c
> >
> d99c5c301635%7C0%7C0%7C636552277685045614&sdata=G3VP7FkkVcBKN4
> bkA5RRdJ
> > FzfpdIBhuxmRmBMcFvMY8%3D&reserved=0
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 7/7] ovmf: add DxeTpm2MeasureBootLib

2018-02-26 Thread Laszlo Ersek
On 02/23/18 14:23, marcandre.lur...@redhat.com wrote:
> 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: 2type: 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 | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 2281bd5ff8..92ed9f3b0c 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -677,7 +677,10 @@
>MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
>  
>
> NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> - }
> +!if $(TPM2_ENABLE) == TRUE
> +  
> NULL|SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
> +!endif
> +  }
>  !else
>MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
>  !endif
>

This looks OK to me.

First, can you please clean up the SecurityStubDxe stanza as follows (as
a separate patch):

> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 96fc7b82e708..f4288b625cba 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -634,14 +634,12 @@ [Components]
>
>MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
>
> -!if $(SECURE_BOOT_ENABLE) == TRUE
>MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
>  
> +!if $(SECURE_BOOT_ENABLE) == TRUE
>
> NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> -   }
> -!else
> -  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
>  !endif
> +  }
>
>MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
>PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf

The idea is that "SecurityStubDxe.inf" should be included
unconditionally; only its plug-in libs should be conditional on various
build flags. While the current (pre-patch) code does that -- in effect
-- for SECURE_BOOT_ENABLE already, your patch (as-is) can only add
TPM2_ENABLE *within* SECURE_BOOT_ENABLE.

I don't think that's for the best -- first we should make
DxeImageVerificationLib the *only* bit that's conditional on
SECURE_BOOT_ENABLE, and then we can add DxeTpm2MeasureBootLib
independently. (If neither build option is specified, we'll have a
 list that's empty, but that's perfectly fine.)

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


Re: [edk2] [PATCH 6/7] ovmf: link with Tcg2ConfigDxe module

2018-02-26 Thread Laszlo Ersek
On 02/23/18 14:23, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> The module allows to tweak and interact with the TPM. Note that many
> actions are broken due to implementation of qemu TPM (providing it's
> own ACPI table), and the lack of PPI implementation.
> 
> CC: Laszlo Ersek 
> CC: Stefan Berger 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Marc-André Lureau 
> ---
>  OvmfPkg/OvmfPkgX64.dsc | 2 ++
>  OvmfPkg/OvmfPkgX64.fdf | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 9bd0709f98..2281bd5ff8 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -669,6 +669,8 @@
>NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
>
> NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
>}
> +
> +  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
>  !endif
>  
>  !if $(SECURE_BOOT_ENABLE) == TRUE
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index b8dd7ecae4..985404850f 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -399,6 +399,7 @@ INF  
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
>  
>  !if $(TPM2_ENABLE) == TRUE
>  INF  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
> +INF  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
>  !endif
>  
>  
> 
> 

Please drop this patch.

In my earlier investigation I wrote, Tcg2ConfigDxe "[p]rovides a Setup
TUI interface to configure the TPM. IIUC, it can also save the
configured TPM type for subsequent boots (see Tcg2ConfigPei.inf above)".

The INF file itself says "This module is only for reference only, each
platform should have its own setup page."

And Jiewen wrote earlier, "Tcg2ConfigPei/Dxe are platform sample driver.
A platform may have its own version based upon platform requirement. For
example, if a platform supports fTPM, it may use another Tcg2Config driver."

Given that OVMF lacks PEI-phase variable access, and that I consequently
suggested cloning, and seriously trimming, Tcg2ConfigPei, it makes no
sense to include an HII dialog that sets a variable for PEI phase
consumption. Also, as you say, many of the exposed operations are broken
due to lack of PPI support. So let's just postpone the inclusion of this
driver, for now.

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


Re: [edk2] [PATCH 5/7] ovmf: link with Tcg2Dxe module

2018-02-26 Thread Laszlo Ersek
On 02/23/18 14:23, marcandre.lur...@redhat.com wrote:
> 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:
> 
> [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: 0type: EV_S_CRTM_VERSION size: 0x2   digest: 
> 1489f923c4dca729178b3e3233458550d8dddf29
>   + version:
> PCR: 0type: EV_EFI_PLATFORM_FIRMWARE_BLOB size: 0x10  digest: 
> fd39ced7c0d2a61f6830c78c7625f94826b05bcc
>   + base: 0x82length: 0xe
> PCR: 0type: EV_EFI_PLATFORM_FIRMWARE_BLOB size: 0x10  digest: 
> 39ebc6783b72bc1e73c7d5bcfeb5f54a3f105d4c
>   + base: 0x90length: 0xa0
> PCR: 7type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x35  digest: 
> 57cd4dc19442475aa82743484f3b1caa88e142b8
> PCR: 7type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x24  digest: 
> 9b1387306ebb7ff8e795e7be77563666bbf4516e
> PCR: 7type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x26  digest: 
> 9afa86c507419b8570c62167cb9486d9fc809758
> PCR: 7type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x24  digest: 
> 5bf8faa078d40ffbd03317c93398b01229a0e1e0
> PCR: 7type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x26  digest: 
> 734424c9fe8fc71716c42096f4b74c88733b175e
> PCR: 7type: EV_SEPARATOR  size: 0x4   digest: 
> 9069ca78e7450a285173431b3e52c5c25299e473
> PCR: 1type: EV_EFI_VARIABLE_BOOT  size: 0x3e  digest: 
> 252f8ebb85340290b64f4b06a001742be8e5cab6
> PCR: 1type: EV_EFI_VARIABLE_BOOT  size: 0x6e  digest: 
> 22a4f6ee9af6dba01d3528deb64b74b582fc182b
> PCR: 1type: EV_EFI_VARIABLE_BOOT  size: 0x80  digest: 
> b7811d5bf30a7efd4e385c6179fe10d9290bb9e8
> PCR: 1type: EV_EFI_VARIABLE_BOOT  size: 0x84  digest: 
> 425e502c24fc924e231e0a62327b6b7d1f704573
> PCR: 1type: EV_EFI_VARIABLE_BOOT  size: 0x9a  digest: 
> 0b5d2c98ac5de6148a4a1490ff9d5df69039f04e
> PCR: 1type: EV_EFI_VARIABLE_BOOT  size: 0xbd  digest: 
> 20bd5f402271d57a88ea314fe35c1705956b1f74
> PCR: 1type: EV_EFI_VARIABLE_BOOT  size: 0x88  digest: 
> df5d6605cb8f4366d745a8464cfb26c1efdc305c
> PCR: 4type: EV_EFI_ACTION size: 0x28  digest: 
> cd0fdb4531a6ec41be2753ba042637d6e5f7f256
> PCR: 0type: EV_SEPARATOR  size: 0x4   digest: 
> 9069ca78e7450a285173431b3e52c5c25299e473
> PCR: 1type: EV_SEPARATOR  size: 0x4   digest: 
> 9069ca78e7450a285173431b3e52c5c25299e473
> PCR: 2type: EV_SEPARATOR  size: 0x4   digest: 
> 9069ca78e7450a285173431b3e52c5c25299e473
> PCR: 3type: EV_SEPARATOR  size: 0x4   digest: 
> 9069ca78e7450a285173431b3e52c5c25299e473
> PCR: 4type: EV_SEPARATOR  size: 0x4   digest: 
> 9069ca78e7450a285173431b3e52c5c25299e473
> PCR: 5type: 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  : acc611d90245cf04e77b0ca94901f90e7f

Re: [edk2] [PATCH 4/7] ovmf: link with Tcg2Pei module

2018-02-26 Thread Laszlo Ersek
On 02/23/18 14:23, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> This module will initialize TPM device, measure reported FVs and BIOS
> version.
> 
> 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 b5cbe8430f..34a7c2778e 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -279,6 +279,8 @@
>PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
>QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
>  !if $(TPM2_ENABLE)
> +  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
> +  
> HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf
>
> Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceLibDTpm.inf
>Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
>  !endif
> @@ -647,6 +649,11 @@
>  
>  !if $(TPM2_ENABLE) == TRUE
>SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> +  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {
> +
> +  NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
> +  
> NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
> +  }
>  !endif
>  
>  !if $(SECURE_BOOT_ENABLE) == TRUE
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index dc35d0a1f7..9558142a42 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -170,6 +170,7 @@ INF  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
>  !endif
>  !if $(TPM2_ENABLE) == TRUE
>  INF  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> +INF  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
>  !endif
>  
>  
> 
> 

Would it be possible to drop SHA1 (include SHA256 only) by setting
PcdTpm2HashMask to value 2? Or SHA1 required for some other reason? (If
so please mention it in the commit message.)

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


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

2018-02-26 Thread Wang, Sunny (HPS SW)
Hi Heyi, 
Just a suggestion. 
Is it better to use a PCD instead of a define for MAX_RECONNECT_REPAIR? So that 
we can easily override it in our platform dsc file.

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Heyi Guo
Sent: Monday, February 26, 2018 4:30 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni ; Heyi Guo ; Eric Dong 
; Star Zeng 
Subject: [edk2] [PATCH 1/1] MdeModulePkg/UefiBootManagerLib: limit recursive 
call depth

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.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Ruiyu Ni 
---
 MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h |  6 ++
 MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c | 17 -
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h 
b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
index 25a1d522fe84..9aa86b096525 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.
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
index ddcee8b0676f..30d70f32af84 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
@@ -26,6 +26,12 @@ GLOBAL_REMOVE_IF_UNREFERENCED
 L"Reboot Required"
 };
 
+//
+// Counter of reconnect retry to repair controller; it is to limit the 
+// number of recursive call of BmRepairAllControllers.
+//
+STATIC UINTN mReconnectRepairCount;
+
 /**
   Return the controller name.
 
@@ -549,7 +555,16 @@ BmRepairAllControllers (
 
 
   if (ReconnectRequired) {
-BmRepairAllControllers ();
+if (mReconnectRepairCount < MAX_RECONNECT_REPAIR) {
+  mReconnectRepairCount++;
+  BmRepairAllControllers ();
+} else {
+  DEBUG ((DEBUG_ERROR, "[%a:%d] Repair failed after %d retries.\n",
+__FUNCTION__, __LINE__, mReconnectRepairCount));
+  // Reset counter so that it will not affect calling
+  // BmRepairAllControllers() somewhere else
+  mReconnectRepairCount = 0;
+}
   }
 
   DEBUG_CODE (
--
2.7.4

___
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


[edk2] [PATCH] EmbeddedPkg/Drivers: add virtual keyboard driver

2018-02-26 Thread Haojian Zhuang
The virtual keyboard could simulate a keyboard. User could simulate
a key value when pattern is matched.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Haojian Zhuang 
---
 .../Drivers/VirtualKeyboardDxe/ComponentName.c |  184 
 .../Drivers/VirtualKeyboardDxe/ComponentName.h |  154 +++
 .../Drivers/VirtualKeyboardDxe/VirtualKeyboard.c   | 1117 
 .../Drivers/VirtualKeyboardDxe/VirtualKeyboard.h   |  544 ++
 .../VirtualKeyboardDxe/VirtualKeyboardDxe.dec  |   39 +
 .../VirtualKeyboardDxe/VirtualKeyboardDxe.inf  |   61 ++
 .../Include/Protocol/PlatformVirtualKeyboard.h |   65 ++
 7 files changed, 2164 insertions(+)
 create mode 100644 EmbeddedPkg/Drivers/VirtualKeyboardDxe/ComponentName.c
 create mode 100644 EmbeddedPkg/Drivers/VirtualKeyboardDxe/ComponentName.h
 create mode 100644 EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.c
 create mode 100644 EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.h
 create mode 100644 
EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboardDxe.dec
 create mode 100644 
EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboardDxe.inf
 create mode 100644 EmbeddedPkg/Include/Protocol/PlatformVirtualKeyboard.h

diff --git a/EmbeddedPkg/Drivers/VirtualKeyboardDxe/ComponentName.c 
b/EmbeddedPkg/Drivers/VirtualKeyboardDxe/ComponentName.c
new file mode 100644
index 000..2935307
--- /dev/null
+++ b/EmbeddedPkg/Drivers/VirtualKeyboardDxe/ComponentName.c
@@ -0,0 +1,184 @@
+/** @file
+
+Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.
+Copyright (c) 2018, Linaro Ltd. All rights reserved.
+
+This program and the accompanying materials
+are licensed and made available under the terms and conditions
+of the BSD License which accompanies this distribution.  The
+full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include "VirtualKeyboard.h"
+
+//
+// EFI Component Name Protocol
+//
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_COMPONENT_NAME_PROTOCOL  
gVirtualKeyboardComponentName = {
+  VirtualKeyboardComponentNameGetDriverName,
+  VirtualKeyboardComponentNameGetControllerName,
+  "eng"
+};
+
+//
+// EFI Component Name 2 Protocol
+//
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_COMPONENT_NAME2_PROTOCOL 
gVirtualKeyboardComponentName2 = {
+  (EFI_COMPONENT_NAME2_GET_DRIVER_NAME) 
VirtualKeyboardComponentNameGetDriverName,
+  (EFI_COMPONENT_NAME2_GET_CONTROLLER_NAME) 
VirtualKeyboardComponentNameGetControllerName,
+  "en"
+};
+
+
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_UNICODE_STRING_TABLE 
mVirtualKeyboardDriverNameTable[] = {
+  {
+"eng;en",
+L"RAM Keyboard Driver"
+  },
+  {
+NULL,
+NULL
+  }
+};
+
+/**
+  Retrieves a Unicode string that is the user readable name of the driver.
+
+  This function retrieves the user readable name of a driver in the form of a
+  Unicode string. If the driver specified by This has a user readable name in
+  the language specified by Language, then a pointer to the driver name is
+  returned in DriverName, and EFI_SUCCESS is returned. If the driver specified
+  by This does not support the language specified by Language,
+  then EFI_UNSUPPORTED is returned.
+
+  @param  This[in]  A pointer to the EFI_COMPONENT_NAME2_PROTOCOL 
or
+EFI_COMPONENT_NAME_PROTOCOL instance.
+
+  @param  Language[in]  A pointer to a Null-terminated ASCII string
+array indicating the language. This is the
+language of the driver name that the caller is
+requesting, and it must match one of the
+languages specified in SupportedLanguages. The
+number of languages supported by a driver is up
+to the driver writer. Language is specified
+in RFC 4646 or ISO 639-2 language code format.
+
+  @param  DriverName[out]   A pointer to the Unicode string to return.
+This Unicode string is the name of the
+driver specified by This in the language
+specified by Language.
+
+  @retval EFI_SUCCESS   The Unicode string for the Driver specified by
+This and the language specified by Language was
+returned in DriverName.
+
+  @retval EFI_INVALID_PARAMETER Language is NULL.
+
+  @retval EFI_INVALID_PARAMETER DriverName is NULL.
+
+  @retval EFI_UNSUPPORTED   The driver specified by This does not support
+the language specified by Language.
+
+**/
+EFI_STATUS
+EFIAPI
+VirtualKeyboardComponentNameGetDriverName (
+  IN  EFI_COMPONENT_NAME_PR

[edk2] [platform][PATCH 2/4] Platform/Hisilicon/HiKey960: enable virtual keyboard

2018-02-26 Thread Haojian Zhuang
Enable virtual keyboard on HiKey960 platform. The platform
driver read pattern from memory or GPIO pin. When the value
is matched, it simulates a hotkey that is used to adjust
sequence of boot options.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Haojian Zhuang 
---
 Platform/Hisilicon/HiKey960/HiKey960.dsc   |  10 +
 Platform/Hisilicon/HiKey960/HiKey960.fdf   |   7 +
 .../Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c   | 491 +
 .../Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf |  55 +++
 Silicon/Hisilicon/Hi3660/Include/Hi3660.h  | 147 ++
 Silicon/Hisilicon/Hi3660/Include/Hkadc.h   |  66 +++
 6 files changed, 776 insertions(+)
 create mode 100644 Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c
 create mode 100644 Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf
 create mode 100644 Silicon/Hisilicon/Hi3660/Include/Hi3660.h
 create mode 100644 Silicon/Hisilicon/Hi3660/Include/Hkadc.h

diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dsc 
b/Platform/Hisilicon/HiKey960/HiKey960.dsc
index 0316e2d..082e12b 100644
--- a/Platform/Hisilicon/HiKey960/HiKey960.dsc
+++ b/Platform/Hisilicon/HiKey960/HiKey960.dsc
@@ -68,6 +68,9 @@
   PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
   
PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
 
+[BuildOptions]
+  GCC:*_*_*_PLATFORM_FLAGS = -I$(WORKSPACE)/Silicon/Hisilicon/Hi3660/Include
+
 

 #
 # Pcd Section - list of all EDK II PCD Entries defined by this Platform
@@ -186,6 +189,13 @@
   ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf
 
   #
+  # Virtual Keyboard
+  #
+  EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboardDxe.inf
+
+  Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf
+
+  #
   # USB Host Support
   #
   MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
diff --git a/Platform/Hisilicon/HiKey960/HiKey960.fdf 
b/Platform/Hisilicon/HiKey960/HiKey960.fdf
index f67ed72..fa8e145 100644
--- a/Platform/Hisilicon/HiKey960/HiKey960.fdf
+++ b/Platform/Hisilicon/HiKey960/HiKey960.fdf
@@ -361,6 +361,13 @@ READ_LOCK_STATUS   = TRUE
   INF ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf
 
   #
+  # Virtual Keyboard
+  #
+  INF EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboardDxe.inf
+
+  INF Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf
+
+  #
   # USB Host Support
   #
   INF MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
diff --git a/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c 
b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c
new file mode 100644
index 000..473d61e
--- /dev/null
+++ b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c
@@ -0,0 +1,491 @@
+/** @file
+*
+*  Copyright (c) 2018, Linaro Ltd. All rights reserved.
+*
+*  This program and the accompanying materials
+*  are licensed and made available under the terms and conditions of the BSD 
License
+*  which accompanies this distribution.  The full text of the license may be 
found at
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+*
+**/
+
+#include 
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#define ADC_ADCIN0   0
+#define ADC_ADCIN1   1
+#define ADC_ADCIN2   2
+
+#define HKADC_DATA_GRADE00
+#define HKADC_DATA_GRADE1100
+#define HKADC_DATA_GRADE2300
+#define HKADC_DATA_GRADE3500
+#define HKADC_DATA_GRADE4700
+#define HKADC_DATA_GRADE5900
+#define HKADC_DATA_GRADE61100
+#define HKADC_DATA_GRADE71300
+#define HKADC_DATA_GRADE81500
+#define HKADC_DATA_GRADE91700
+#define HKADC_DATA_GRADE10   1800
+
+#define BOARDID_VALUE0   0
+#define BOARDID_VALUE1   1
+#define BOARDID_VALUE2   2
+#define BOARDID_VALUE3   3
+#define BOARDID_VALUE4   4
+#define BOARDID_VALUE5   5
+#define BOARDID_VALUE6   6
+#define BOARDID_VALUE7   7
+#define BOARDID_VALUE8   8
+#define BOARDID_VALUE9   9
+#define BOARDID_UNKNOW   0xF
+
+#define BOARDID3_BASE5
+
+#define HIKEY960_BOARDID_V1  5300
+#define HIKEY960_BOARDID_V2  5301
+
+#define HIKEY960_COMPATIBLE_LEDS_V1  "gpio-leds_v1"
+#define HIKEY960_COMPATIBLE_LEDS_V2  "gpio-leds_v2"
+#define HIKEY960_COMPATIBLE_HUB_V1   "hisil

[edk2] [platform][PATCH 4/4] Platform/Hisilicon/HiKey: enable virtual keyboard

2018-02-26 Thread Haojian Zhuang
Enable virtual keyboard driver on HiKey platform. The platform
driver reads pattern from memory or GPIO pin. When the value
is matched, it simulates a key value that is used to adjust
the sequence of boot options.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Haojian Zhuang 
---
 Platform/Hisilicon/HiKey/HiKey.dsc|   8 +
 Platform/Hisilicon/HiKey/HiKey.fdf|   8 +
 Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.c  | 229 ++
 Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf|  58 ++
 Silicon/Hisilicon/Hi6220/Include/Hi6220RegsPeri.h |  48 +
 5 files changed, 351 insertions(+)
 create mode 100644 Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.c
 create mode 100644 Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf
 create mode 100644 Silicon/Hisilicon/Hi6220/Include/Hi6220RegsPeri.h

diff --git a/Platform/Hisilicon/HiKey/HiKey.dsc 
b/Platform/Hisilicon/HiKey/HiKey.dsc
index 2dfdced..37ad6da 100644
--- a/Platform/Hisilicon/HiKey/HiKey.dsc
+++ b/Platform/Hisilicon/HiKey/HiKey.dsc
@@ -192,9 +192,17 @@
   #
   # GPIO
   #
+  Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.inf
   ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf
 
   #
+  # Virtual Keyboard
+  #
+  EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboardDxe.inf
+
+  Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf
+
+  #
   # MMC/SD
   #
   EmbeddedPkg/Universal/MmcDxe/MmcDxe.inf
diff --git a/Platform/Hisilicon/HiKey/HiKey.fdf 
b/Platform/Hisilicon/HiKey/HiKey.fdf
index 80b272c..e7cea73 100644
--- a/Platform/Hisilicon/HiKey/HiKey.fdf
+++ b/Platform/Hisilicon/HiKey/HiKey.fdf
@@ -355,9 +355,17 @@ READ_LOCK_STATUS   = TRUE
   #
   # GPIO
   #
+  INF Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.inf
   INF ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf
 
   #
+  # Virtual Keyboard
+  #
+  INF EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboardDxe.inf
+
+  INF Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf
+
+  #
   # Multimedia Card Interface
   #
   INF EmbeddedPkg/Universal/MmcDxe/MmcDxe.inf
diff --git a/Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.c 
b/Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.c
new file mode 100644
index 000..65e8001
--- /dev/null
+++ b/Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.c
@@ -0,0 +1,229 @@
+/** @file
+*
+*  Copyright (c) 2018, Linaro Ltd. All rights reserved.
+*
+*  This program and the accompanying materials
+*  are licensed and made available under the terms and conditions of the BSD 
License
+*  which accompanies this distribution.  The full text of the license may be 
found at
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+*
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define SERIAL_NUMBER_SIZE   17
+#define SERIAL_NUMBER_BLOCK_SIZE EFI_PAGE_SIZE
+#define SERIAL_NUMBER_LBA1024
+#define RANDOM_MAX   0x7FFF
+#define RANDOM_MAGIC 0x9A4DBEAF
+
+#define DETECT_J15_FASTBOOT  24   // GPIO3_0
+
+#define ADB_REBOOT_ADDRESS   0x05F01000
+#define ADB_REBOOT_BOOTLOADER0x77665500
+#define ADB_REBOOT_NONE  0x77665501
+
+
+typedef struct {
+  UINT64Magic;
+  UINT64Data;
+  CHAR16UnicodeSN[SERIAL_NUMBER_SIZE];
+} RANDOM_SERIAL_NUMBER;
+
+STATIC EMBEDDED_GPIO*mGpio;
+
+STATIC
+VOID
+UartInit (
+  IN VOID
+  )
+{
+  UINT32 Val;
+
+  /* make UART1 out of reset */
+  MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_RSTDIS3, PERIPH_RST3_UART1);
+  MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CLKEN3, PERIPH_RST3_UART1);
+  /* make UART2 out of reset */
+  MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_RSTDIS3, PERIPH_RST3_UART2);
+  MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CLKEN3, PERIPH_RST3_UART2);
+  /* make UART3 out of reset */
+  MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_RSTDIS3, PERIPH_RST3_UART3);
+  MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CLKEN3, PERIPH_RST3_UART3);
+  /* make UART4 out of reset */
+  MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_RSTDIS3, PERIPH_RST3_UART4);
+  MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_CLKEN3, PERIPH_RST3_UART4);
+
+  /* make DW_MMC2 out of reset */
+  MmioWrite32 (PERI_CTRL_BASE + SC_PERIPH_RSTDIS0, PERIPH_RST0_MMC2);
+
+  /* enable clock for BT/WIFI */
+  Val = MmioRead32 (PMUSSI_REG(0x1c)) | 0x40;
+  MmioWrite32 (PMUSSI_REG(0x1c), Val);
+}
+
+STATIC
+VOID
+MtcmosInit (
+  IN VOID
+  )
+{
+  UINT32 Data;
+
+  /* enable MTCMOS for GPU */
+  MmioWrite32 (AO_CTRL_BASE + SC_PW_MTCMOS_EN0, PW_EN0_G3D);
+  do {
+Data = MmioRead32 (AO_CTRL_BASE + SC_PW_MTCMOS_ACK_STAT0);
+  } while ((Data & PW_EN0_G3D) == 0);
+}
+
+EFI_STATUS
+HiKeyInitPeripherals (
+  IN VOID
+  

[edk2] [platform][PATCH 3/4] Platform/Hisilicon/HiKey: add gpio platform driver

2018-02-26 Thread Haojian Zhuang
Add gpio platform driver to enable GPIO in HiKey platform.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Haojian Zhuang 
---
 .../Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.c| 68 ++
 .../Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.inf  | 36 
 2 files changed, 104 insertions(+)
 create mode 100644 Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.c
 create mode 100644 Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.inf

diff --git a/Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.c 
b/Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.c
new file mode 100644
index 000..543f65d
--- /dev/null
+++ b/Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.c
@@ -0,0 +1,68 @@
+/** @file
+*
+*  Copyright (c) 2018, Linaro. All rights reserved.
+*
+*  This program and the accompanying materials
+*  are licensed and made available under the terms and conditions of the BSD 
License
+*  which accompanies this distribution.  The full text of the license may be 
found at
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+*
+**/
+
+#include 
+
+#include 
+
+GPIO_CONTROLLER gGpioDevice[]= {
+  { 0xf8011000, 0, 8 },// GPIO0
+  { 0xf8012000, 8, 8 },// GPIO1
+  { 0xf8013000, 16, 8 },   // GPIO2
+  { 0xf8014000, 24, 8 },   // GPIO3
+  { 0xf702, 32, 8 },   // GPIO4
+  { 0xf7021000, 40, 8 },   // GPIO5
+  { 0xf7022000, 48, 8 },   // GPIO6
+  { 0xf7023000, 56, 8 },   // GPIO7
+  { 0xf7024000, 64, 8 },   // GPIO8
+  { 0xf7025000, 72, 8 },   // GPIO9
+  { 0xf7026000, 80, 8 },   // GPIO10
+  { 0xf7027000, 88, 8 },   // GPIO11
+  { 0xf7028000, 96, 8 },   // GPIO12
+  { 0xf7029000, 104, 8 },  // GPIO13
+  { 0xf702a000, 112, 8 },  // GPIO14
+  { 0xf702b000, 120, 8 },  // GPIO15
+  { 0xf702c000, 128, 8 },  // GPIO16
+  { 0xf702d000, 136, 8 },  // GPIO17
+  { 0xf702e000, 144, 8 },  // GPIO18
+  { 0xf702f000, 152, 8 }   // GPIO19
+};
+
+PLATFORM_GPIO_CONTROLLER gPlatformGpioDevice = {
+  160, 20, gGpioDevice
+};
+
+EFI_STATUS
+EFIAPI
+HiKeyGpioEntryPoint (
+  IN EFI_HANDLE ImageHandle,
+  IN EFI_SYSTEM_TABLE   *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+  EFI_HANDLE  Handle;
+
+  // Install the Embedded Platform GPIO Protocol onto a new handle
+  Handle = NULL;
+  Status = gBS->InstallMultipleProtocolInterfaces(
+  &Handle,
+  &gPlatformGpioProtocolGuid, &gPlatformGpioDevice,
+  NULL
+ );
+  if (EFI_ERROR(Status)) {
+Status = EFI_OUT_OF_RESOURCES;
+  }
+
+  return Status;
+}
diff --git a/Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.inf 
b/Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.inf
new file mode 100644
index 000..272ed1c
--- /dev/null
+++ b/Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.inf
@@ -0,0 +1,36 @@
+#
+#  Copyright (c) 2018, Linaro. All rights reserved.
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD 
License
+#  which accompanies this distribution.  The full text of the license may be 
found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+
+[Defines]
+  INF_VERSION= 0x00010019
+  BASE_NAME  = HiKeyGpio
+  FILE_GUID  = b51a851c-7bf7-463f-b261-cfb158b7f699
+  MODULE_TYPE= DXE_DRIVER
+  VERSION_STRING = 1.0
+  ENTRY_POINT= HiKeyGpioEntryPoint
+
+[Sources.common]
+  HiKeyGpioDxe.c
+
+[Packages]
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  DebugLib
+  UefiDriverEntryPoint
+
+[Protocols]
+  gPlatformGpioProtocolGuid
+
+[Depex]
+  TRUE
-- 
2.7.4

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


[edk2] [platform][PATCH 0/4] enable virtual keyboard

2018-02-26 Thread Haojian Zhuang
Changelog:
v1:
  * Enable GPIO driver.
  * Enable virtual keyboard driver.

Haojian Zhuang (4):
  Platform/Hisilicon/HiKey960: add gpio platform driver
  Platform/Hisilicon/HiKey960: enable virtual keyboard
  Platform/Hisilicon/HiKey: add gpio platform driver
  Platform/Hisilicon/HiKey: enable virtual keyboard

 Platform/Hisilicon/HiKey/HiKey.dsc |   8 +
 Platform/Hisilicon/HiKey/HiKey.fdf |   8 +
 Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.c   | 229 ++
 Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf |  58 +++
 .../Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.c|  68 +++
 .../Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.inf  |  36 ++
 Platform/Hisilicon/HiKey960/HiKey960.dsc   |  11 +
 Platform/Hisilicon/HiKey960/HiKey960.fdf   |   8 +
 .../Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c   | 491 +
 .../Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf |  55 +++
 .../HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.c |  77 
 .../HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf   |  35 ++
 Silicon/Hisilicon/Hi3660/Include/Hi3660.h  | 147 ++
 Silicon/Hisilicon/Hi3660/Include/Hkadc.h   |  66 +++
 Silicon/Hisilicon/Hi6220/Include/Hi6220RegsPeri.h  |  48 ++
 15 files changed, 1345 insertions(+)
 create mode 100644 Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.c
 create mode 100644 Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf
 create mode 100644 Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.c
 create mode 100644 Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.inf
 create mode 100644 Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c
 create mode 100644 Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf
 create mode 100644 
Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.c
 create mode 100644 
Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf
 create mode 100644 Silicon/Hisilicon/Hi3660/Include/Hi3660.h
 create mode 100644 Silicon/Hisilicon/Hi3660/Include/Hkadc.h
 create mode 100644 Silicon/Hisilicon/Hi6220/Include/Hi6220RegsPeri.h

-- 
2.7.4

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


[edk2] [platform][PATCH 1/4] Platform/Hisilicon/HiKey960: add gpio platform driver

2018-02-26 Thread Haojian Zhuang
Add gpio platform driver to enable GPIO in HiKey960 platform.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Haojian Zhuang 
---
 Platform/Hisilicon/HiKey960/HiKey960.dsc   |  1 +
 Platform/Hisilicon/HiKey960/HiKey960.fdf   |  1 +
 .../HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.c | 77 ++
 .../HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf   | 35 ++
 4 files changed, 114 insertions(+)
 create mode 100644 
Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.c
 create mode 100644 
Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf

diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dsc 
b/Platform/Hisilicon/HiKey960/HiKey960.dsc
index 69c6c98..0316e2d 100644
--- a/Platform/Hisilicon/HiKey960/HiKey960.dsc
+++ b/Platform/Hisilicon/HiKey960/HiKey960.dsc
@@ -182,6 +182,7 @@
   #
   # GPIO
   #
+  Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf
   ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf
 
   #
diff --git a/Platform/Hisilicon/HiKey960/HiKey960.fdf 
b/Platform/Hisilicon/HiKey960/HiKey960.fdf
index af10430..f67ed72 100644
--- a/Platform/Hisilicon/HiKey960/HiKey960.fdf
+++ b/Platform/Hisilicon/HiKey960/HiKey960.fdf
@@ -357,6 +357,7 @@ READ_LOCK_STATUS   = TRUE
   #
   # GPIO
   #
+  INF Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf
   INF ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf
 
   #
diff --git a/Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.c 
b/Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.c
new file mode 100644
index 000..986fece
--- /dev/null
+++ b/Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.c
@@ -0,0 +1,77 @@
+/** @file
+*
+*  Copyright (c) 2018, Linaro. All rights reserved.
+*
+*  This program and the accompanying materials
+*  are licensed and made available under the terms and conditions of the BSD 
License
+*  which accompanies this distribution.  The full text of the license may be 
found at
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+*
+**/
+
+#include 
+
+#include 
+
+GPIO_CONTROLLER gGpioDevice[]= {
+  { 0xe8a0b000, 0, 8 },// GPIO0
+  { 0xe8a0c000, 8, 8 },// GPIO1
+  { 0xe8a0d000, 16, 8 },   // GPIO2
+  { 0xe8a0e000, 24, 8 },   // GPIO3
+  { 0xe8a0f000, 32, 8 },   // GPIO4
+  { 0xe8a1, 40, 8 },   // GPIO5
+  { 0xe8a11000, 48, 8 },   // GPIO6
+  { 0xe8a12000, 56, 8 },   // GPIO7
+  { 0xe8a13000, 64, 8 },   // GPIO8
+  { 0xe8a14000, 72, 8 },   // GPIO9
+  { 0xe8a15000, 80, 8 },   // GPIO10
+  { 0xe8a16000, 88, 8 },   // GPIO11
+  { 0xe8a17000, 96, 8 },   // GPIO12
+  { 0xe8a18000, 104, 8 },  // GPIO13
+  { 0xe8a19000, 112, 8 },  // GPIO14
+  { 0xe8a1a000, 120, 8 },  // GPIO15
+  { 0xe8a1b000, 128, 8 },  // GPIO16
+  { 0xe8a1c000, 136, 8 },  // GPIO17
+  { 0xff3b4000, 144, 8 },  // GPIO18
+  { 0xff3b5000, 152, 8 },  // GPIO19
+  { 0xe8a1f000, 160, 8 },  // GPIO20
+  { 0xe8a2, 168, 8 },  // GPIO21
+  { 0xfff0b000, 176, 8 },  // GPIO22
+  { 0xfff0c000, 184, 8 },  // GPIO23
+  { 0xfff0d000, 192, 8 },  // GPIO24
+  { 0xfff0e000, 200, 8 },  // GPIO25
+  { 0xfff0f000, 208, 8 },  // GPIO26
+  { 0xfff1, 216, 8 },  // GPIO27
+  { 0xfff1d000, 224, 8 },  // GPIO28
+};
+
+PLATFORM_GPIO_CONTROLLER gPlatformGpioDevice = {
+  232, 29, gGpioDevice
+};
+
+EFI_STATUS
+EFIAPI
+HiKey960GpioEntryPoint (
+  IN EFI_HANDLE ImageHandle,
+  IN EFI_SYSTEM_TABLE   *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+  EFI_HANDLE  Handle;
+
+  // Install the Embedded Platform GPIO Protocol onto a new handle
+  Handle = NULL;
+  Status = gBS->InstallMultipleProtocolInterfaces(
+  &Handle,
+  &gPlatformGpioProtocolGuid, &gPlatformGpioDevice,
+  NULL
+ );
+  if (EFI_ERROR(Status)) {
+Status = EFI_OUT_OF_RESOURCES;
+  }
+
+  return Status;
+}
diff --git a/Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf 
b/Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf
new file mode 100644
index 000..a16213f
--- /dev/null
+++ b/Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf
@@ -0,0 +1,35 @@
+#
+#  Copyright (c) 2018, Linaro. All rights reserved.
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD 
License
+#  which accompanies this distribution.  The full text of the license may be 
found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+
+[Defines]
+  INF_VERSION= 0x00010019
+  BASE_NAME  = HiKey960GpioDxe
+  FILE_GUID  = 6aa12592-7e36-4aec-acf8-2ac2fd13815c
+

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

2018-02-26 Thread Heyi Guo
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.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Ruiyu Ni 
---
 MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h |  6 ++
 MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c | 17 -
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h 
b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
index 25a1d522fe84..9aa86b096525 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.
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
index ddcee8b0676f..30d70f32af84 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
@@ -26,6 +26,12 @@ GLOBAL_REMOVE_IF_UNREFERENCED
 L"Reboot Required"
 };
 
+//
+// Counter of reconnect retry to repair controller; it is to limit the
+// number of recursive call of BmRepairAllControllers.
+//
+STATIC UINTN mReconnectRepairCount;
+
 /**
   Return the controller name.
 
@@ -549,7 +555,16 @@ BmRepairAllControllers (
 
 
   if (ReconnectRequired) {
-BmRepairAllControllers ();
+if (mReconnectRepairCount < MAX_RECONNECT_REPAIR) {
+  mReconnectRepairCount++;
+  BmRepairAllControllers ();
+} else {
+  DEBUG ((DEBUG_ERROR, "[%a:%d] Repair failed after %d retries.\n",
+__FUNCTION__, __LINE__, mReconnectRepairCount));
+  // Reset counter so that it will not affect calling
+  // BmRepairAllControllers() somewhere else
+  mReconnectRepairCount = 0;
+}
   }
 
   DEBUG_CODE (
-- 
2.7.4

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


[edk2] [Patch V3] DSC spec: Add flexible PCD value format into spec

2018-02-26 Thread Yonghong Zhu
V3: Update the Pcd value format in [Components] section
V2: update EBNF for Array format.

Cc: Liming Gao 
Cc: Michael Kinney 
Cc: Kevin W Shaw 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 3_edk_ii_dsc_file_format/310_pcd_sections.md   | 160 +
 .../311_[components]_sections.md   |  23 +--
 .../33_platform_dsc_definition.md  |  78 +++---
 3 files changed, 174 insertions(+), 87 deletions(-)

diff --git a/3_edk_ii_dsc_file_format/310_pcd_sections.md 
b/3_edk_ii_dsc_file_format/310_pcd_sections.md
index 2af42cc..18a243d 100644
--- a/3_edk_ii_dsc_file_format/310_pcd_sections.md
+++ b/3_edk_ii_dsc_file_format/310_pcd_sections.md
@@ -98,13 +98,11 @@ is permissible to list multiple architectures in a single 
method section as in:
 It is permissible to list a PCD in a common architecture section and also list
 it in an architecturally modified section. In this case, the value in the
 architectural section overrides the value specified in the common section.
 
 The PCD values must match the datum type declared for a given PCD in the DEC
-file. While a PCD of datum type `BOOLEAN` is permitted to have a `1` or a `0`
-(instead of TRUE or FALSE) in the value field, a PCD of type UINT* cannot use
-`TRUE` or `FALSE` for values.
+file.
 
 PCDs with a data type of `VOID`* can optionally provide the maximum size of the
 value. If not provided, the maximum length will be calculated as the largest of
 the size of the data in the DSC file, the size of the data in the INF file or
 the size of the data in the DEC file that declares the PCD.
@@ -220,21 +218,24 @@ fields that are separated by the pipe character, "|".
 ::=  [ ]*
::= 
 ::= 
  ::= {} {} {}
   ::=   [ ] 
-  ::= if (pcddatumtype == "BOOLEAN"): {} {}
-elif (pcddatumtype == "UINT8"): {}
-{} elif (pcddatumtype == "UINT16"):
-{} {} elif (pcddatumtype ==
-"UINT32"): {} {} elif
-(pcddatumtype == "UINT64"): {} {}
+  ::= if (pcddatumtype == "BOOLEAN"):
+  {} {}
+elif (pcddatumtype == "UINT8"):
+  {} {}
+elif (pcddatumtype == "UINT16"):
+  {} {}
+elif (pcddatumtype == "UINT32"):
+  {} {}
+elif (pcddatumtype == "UINT64"):
+  {} {}
 else:
- []
+   []
::=  "VOID*"  {} {}
-   ::= {} {} {}
-{} {}
+   ::= {} {} {}
 ```
 
  Parameters
 
 **_Expression_**
@@ -325,21 +326,24 @@ of the DSC file.
 ::=  [ ]*
::= 
 ::= 
  ::= {} {} {}
   ::=   [ ] 
-  ::= if (pcddatumtype == "BOOLEAN"): {} {}
-elif (pcddatumtype == "UINT8"): {}
-{} elif (pcddatumtype == "UINT16"):
-{} {} elif (pcddatumtype ==
-"UINT32"): {} {} elif
-(pcddatumtype == "UINT64"): {} {}
+  ::= if (pcddatumtype == "BOOLEAN"):
+  {} {}
+elif (pcddatumtype == "UINT8"):
+  {} {}
+elif (pcddatumtype == "UINT16"):
+  {} {}
+elif (pcddatumtype == "UINT32"):
+  {} {}
+elif (pcddatumtype == "UINT64"):
+  {} {}
 else:
- []
+   []
::=  {} {}
-   ::= {} {} {}
-{} {}
+   ::= {} {} {}
 ```
 
  Parameters
 
 **_Expression_**
@@ -458,41 +462,58 @@ sections of the DSC file.
   ::= "."  ["." ]
  ::=  [ ]*
 ::= 
  ::= 
::=  [ ] 
-   ::= if (pcddatumtype == "BOOLEAN"): {} {}
- elif (pcddatumtype == "UINT8"): {}
- {} elif (pcddatumtype == "UINT16"):
- {} {} elif (pcddatumtype ==
- "UINT32"): {} {} elif
- (pcddatumtype == "UINT64"): {}
- {} else:
-  []
+   ::= if (pcddatumtype == "BOOLEAN"):
+   {} {}
+ elif (pcddatumtype == "UINT8"):
+   {} {}
+ elif (pcddatumtype == "UINT16"):
+   {} {}
+ elif (pcddatumtype == "UINT32"):
+   {} {}
+ elif (pcddatumtype == "UINT64"):
+   {} {}
+ else:
+[]
 ::=  "VOID*" [ ]
   ::= {} {}
-::= {} {} {} {}
- {}
+::= {} {} {}
::=[ ] 
   ::= {} {"*"}
-::= if (pcddatumtype == "BOOLEAN"): {} {}
- elif (pcddatumtype == "UINT8"): {}
-