Re: [edk2-devel] [PATCH v2] MdeModulePkg/RegularExpressionDxe: Make oniguruma a submodule in edk2.

2020-04-12 Thread Zhang, Shenglei
Hi all,

With this patch checked in,  you may meet build failure with MdeModulePkg. 
Because the components in oniguruma can’t be found.
Please try command " git submodule update --init" to check out the source code 
before you build the package.

Thanks,
Shenglei

> -Original Message-
> From: Gao, Liming
> Sent: Thursday, March 12, 2020 11:12 AM
> To: Zhang, Shenglei ; devel@edk2.groups.io
> Cc: Wang, Jian J ; Wu, Hao A 
> Subject: RE: [PATCH v2] MdeModulePkg/RegularExpressionDxe: Make
> oniguruma a submodule in edk2.
> 
> Reviewed-by: Liming Gao 
> 
> -Original Message-
> From: Zhang, Shenglei 
> Sent: 2020年3月9日 15:28
> To: devel@edk2.groups.io
> Cc: Wang, Jian J ; Wu, Hao A ;
> Gao, Liming 
> Subject: [PATCH v2] MdeModulePkg/RegularExpressionDxe: Make
> oniguruma a submodule in edk2.
> 
> Use submodule way to access oniguruma. And upgrade oniguruma
> version from v6.9.3 to v6.9.4_mark1.
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2073
> 
> Cc: Jian J Wang 
> Cc: Hao A Wu 
> Cc: Liming Gao 
> Signed-off-by: Shenglei Zhang 
> ---
> 
> v2: Include wrapper files in RegularExpressionDxe.inf.
> Patch link: https://github.com/shenglei10/edk2/commits/oniguruma
> 
>  .../Oniguruma/OnigurumaUefiPort.c |26 -
>  .../RegularExpressionDxe/Oniguruma/ascii.c|   118 -
>  .../Oniguruma/onig_init.c |45 -
>  .../RegularExpressionDxe/Oniguruma/regcomp.c  |  6972 
>  .../RegularExpressionDxe/Oniguruma/regenc.c   |  1029 -
>  .../RegularExpressionDxe/Oniguruma/regerror.c |   402 -
>  .../RegularExpressionDxe/Oniguruma/regexec.c  |  5874 ---
>  .../RegularExpressionDxe/Oniguruma/reggnu.c   |   131 -
>  .../RegularExpressionDxe/Oniguruma/regparse.c |  8461 -
>  .../Oniguruma/regposerr.c |   108 -
>  .../RegularExpressionDxe/Oniguruma/regposix.c |   304 -
>  .../Oniguruma/regsyntax.c |   336 -
>  .../RegularExpressionDxe/Oniguruma/regtrav.c  |76 -
>  .../Oniguruma/regversion.c|57 -
>  .../RegularExpressionDxe/Oniguruma/st.c   |   588 -
>  .../RegularExpressionDxe/Oniguruma/unicode.c  |  1152 -
>  .../Oniguruma/unicode_egcb_data.c |  1374 -
>  .../Oniguruma/unicode_fold1_key.c |  2995 --
>  .../Oniguruma/unicode_fold2_key.c |   222 -
>  .../Oniguruma/unicode_fold3_key.c |   133 -
>  .../Oniguruma/unicode_fold_data.c |  1522 -
>  .../Oniguruma/unicode_property_data.c | 30388 
>  .../Oniguruma/unicode_property_data_posix.c   |  5347 ---
>  .../Oniguruma/unicode_unfold_key.c|  3299 --
>  .../Oniguruma/unicode_wb_data.c   |  1023 -
>  .../RegularExpressionDxe/Oniguruma/utf16_le.c |   309 -
>  .../{Oniguruma => }/OnigurumaIntrinsics.c | 1 +
>  .../RegularExpressionDxe/OnigurumaUefiPort.c  |90 +
>  .gitmodules   | 3 +
>  .../RegularExpressionDxe/Oniguruma/AUTHORS| 1 -
>  .../RegularExpressionDxe/Oniguruma/COPYING|26 -
>  .../RegularExpressionDxe/Oniguruma/README |   195 -
>  .../RegularExpressionDxe/Oniguruma/oniggnu.h  |87 -
>  .../Oniguruma/onigposix.h |   172 -
>  .../Oniguruma/oniguruma.h |  1014 -
>  .../RegularExpressionDxe/Oniguruma/regenc.h   |   279 -
>  .../RegularExpressionDxe/Oniguruma/regint.h   |  1117 -
>  .../RegularExpressionDxe/Oniguruma/regparse.h |   455 -
>  .../RegularExpressionDxe/Oniguruma/st.h   |69 -
>  .../{Oniguruma => }/OnigurumaUefiPort.h   |44 +-
>  .../RegularExpressionDxe.h| 3 +-
>  .../RegularExpressionDxe.inf  |73 +-
>  .../Universal/RegularExpressionDxe/config.h   | 9 +
>  .../Universal/RegularExpressionDxe/oniguruma  | 1 +
>  .../Universal/RegularExpressionDxe/stdarg.h   | 9 +
>  .../Universal/RegularExpressionDxe/stddef.h   | 9 +
>  .../Universal/RegularExpressionDxe/stdio.h| 9 +
>  .../Universal/RegularExpressionDxe/stdlib.h   | 9 +
>  .../Universal/RegularExpressionDxe/string.h   | 9 +
>  49 files changed, 230 insertions(+), 75745 deletions(-)
>  delete mode 100644
> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/OnigurumaUe
> fiPort.c
>  delete mode 100644
> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/ascii.c
>  delete mode 100644
> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/onig_init.c
>  delete mode 100644
> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regcomp.c
>  delete mode 100644
> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regenc.c
>  delete mode 100644
> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regerror.c
>  delete mode 100644
> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regexec.c
>  delete mode 100644
> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/reggnu.c
>  delete mode 100644
> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regparse.c
>  delete 

Re: [edk2-devel] [PATCH V4] BaseTools:Add the spare space FV image size checker

2020-04-12 Thread Bob Feng
Reviewed-by: Bob Feng 

-Original Message-
From: Fan, ZhijuX 
Sent: Monday, April 13, 2020 11:04 AM
To: devel@edk2.groups.io
Cc: Gao, Liming ; Feng, Bob C ; 
Fan, ZhijuX 
Subject: [PATCH V4] BaseTools:Add the spare space FV image size checker

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

If FV is placed in FD region, its FV image size is fixed.
When FV image size exceeds it, it will trig the build break.
To alert the developer to adjust FV image size earlier, I request to add new 
checker for the the spare FV space.
When the spare FV space is less than the specified threshold, build tool will 
report the error.

This checker is the optional.
It can be enabled by -D FV_SPARE_SPACE_THRESHOLD=1.
Macro is the value of the spare space threshold size.
It can be decimal or hex format. If it is enabled, BaseTools will check every 
FV with the fixed size.
If FV doesn't meet with the size requirement, Build tool will report error 
message to say there is no enough spare space.

Cc: Liming Gao 
Cc: Bob Feng 
Signed-off-by: Zhiju.Fan 
---
 Determine the value of Threshold and FvDir

 BaseTools/Source/Python/Common/BuildToolError.py |  2 +
 BaseTools/Source/Python/build/build.py   | 47 ++
 2 files changed, 49 insertions(+)

diff --git a/BaseTools/Source/Python/Common/BuildToolError.py 
b/BaseTools/Source/Python/Common/BuildToolError.py
index ecc83d0f48..21549683cd 100644
--- a/BaseTools/Source/Python/Common/BuildToolError.py
+++ b/BaseTools/Source/Python/Common/BuildToolError.py
@@ -64,6 +64,8 @@ COMMAND_FAILURE = 0x7000
 
 PERMISSION_FAILURE = 0x8000
 
+FV_FREESIZE_ERROR = 0x9000
+
 CODE_ERROR = 0xC0DE
 
 AUTOGEN_ERROR = 0xF000
diff --git a/BaseTools/Source/Python/build/build.py 
b/BaseTools/Source/Python/build/build.py
index bec848a7b2..c19be67254 100755
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -25,6 +25,7 @@ import traceback
 import multiprocessing
 from threading import Thread,Event,BoundedSemaphore  import threading
+from linecache import getlines
 from subprocess import Popen,PIPE, STDOUT  from collections import 
OrderedDict, defaultdict
 
@@ -1413,6 +1414,9 @@ class Build():
 if Target == 'fds':
 if GenFdsApi(AutoGenObject.GenFdsCommandDict, self.Db):
 EdkLogger.error("build", COMMAND_FAILURE)
+Threshold = self.GetFreeSizeThreshold()
+if Threshold:
+self.CheckFreeSizeThreshold(Threshold, 
+ AutoGenObject.FvDir)
 return True
 
 # run
@@ -2311,6 +2315,9 @@ class Build():
 GenFdsStart = time.time()
 if GenFdsApi(Wa.GenFdsCommandDict, self.Db):
 EdkLogger.error("build", COMMAND_FAILURE)
+Threshold = self.GetFreeSizeThreshold()
+if Threshold:
+self.CheckFreeSizeThreshold(Threshold, 
+ Wa.FvDir)
 
 #
 # Create MAP file for all platform FVs after GenFds.
@@ -2322,6 +2329,46 @@ class Build():
 #
 self._SaveMapFile(MapBuffer, Wa)
 self.CreateGuidedSectionToolsFile(Wa)
+
+## GetFreeSizeThreshold()
+#
+#   @retval int Threshold value
+#
+def GetFreeSizeThreshold(self):
+Threshold = None
+Threshold_Str = 
GlobalData.gCommandLineDefines.get('FV_SPARE_SPACE_THRESHOLD')
+if Threshold_Str:
+try:
+if Threshold_Str.lower().startswith('0x'):
+Threshold = int(Threshold_Str, 16)
+else:
+Threshold = int(Threshold_Str)
+except:
+EdkLogger.warn("build", 'incorrect value for 
FV_SPARE_SPACE_THRESHOLD %s. It can be decimal or hex format' % Threshold_Str)
+return Threshold
+
+def CheckFreeSizeThreshold(self, Threshold=None, FvDir=None):
+if not isinstance(Threshold, int):
+return
+if not isinstance(FvDir, str) or not FvDir:
+return
+FdfParserObject = GlobalData.gFdfParser
+FvRegionNameList = [FvName for FvName in 
FdfParserObject.Profile.FvDict if 
FdfParserObject.Profile.FvDict[FvName].FvRegionInFD]
+for FvName in FdfParserObject.Profile.FvDict:
+if FvName in FvRegionNameList:
+FvSpaceInfoFileName = os.path.join(FvDir, FvName.upper() + 
'.Fv.map')
+if os.path.exists(FvSpaceInfoFileName):
+FileLinesList = getlines(FvSpaceInfoFileName)
+for Line in FileLinesList:
+NameValue = Line.split('=')
+if len(NameValue) == 2 and NameValue[0].strip() == 
'EFI_FV_SPACE_SIZE':
+FreeSizeValue = int(NameValue[1].strip(), 0)
+if FreeSizeValue < Threshold:
+

[edk2-devel] BaseTools Win32 binaries repo hasn't been updated for over a year

2020-04-12 Thread Rebecca Cran
Should the BaseTools Win32 repo still work against edk2 master? I just 
noticed it hasn't been updated for over a year and no longer works, 
since GenFfs has new options.



https://github.com/tianocore/tianocore.github.io/wiki/Edk2-buildtools says:


"Prebuilt Windows tools are available at 
https://github.com/tianocore/edk2-BaseTools-win32.git;



--
Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#57276): https://edk2.groups.io/g/devel/message/57276
Mute This Topic: https://groups.io/mt/72980714/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH V4] BaseTools:Add the spare space FV image size checker

2020-04-12 Thread Fan, ZhijuX
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2654

If FV is placed in FD region, its FV image size is fixed.
When FV image size exceeds it, it will trig the build break.
To alert the developer to adjust FV image size earlier,
I request to add new checker for the the spare FV space.
When the spare FV space is less than the specified threshold,
build tool will report the error.

This checker is the optional.
It can be enabled by -D FV_SPARE_SPACE_THRESHOLD=1.
Macro is the value of the spare space threshold size.
It can be decimal or hex format. If it is enabled,
BaseTools will check every FV with the fixed size.
If FV doesn't meet with the size requirement,
Build tool will report error message to say there is no
enough spare space.

Cc: Liming Gao 
Cc: Bob Feng 
Signed-off-by: Zhiju.Fan 
---
 Determine the value of Threshold and FvDir

 BaseTools/Source/Python/Common/BuildToolError.py |  2 +
 BaseTools/Source/Python/build/build.py   | 47 ++
 2 files changed, 49 insertions(+)

diff --git a/BaseTools/Source/Python/Common/BuildToolError.py 
b/BaseTools/Source/Python/Common/BuildToolError.py
index ecc83d0f48..21549683cd 100644
--- a/BaseTools/Source/Python/Common/BuildToolError.py
+++ b/BaseTools/Source/Python/Common/BuildToolError.py
@@ -64,6 +64,8 @@ COMMAND_FAILURE = 0x7000
 
 PERMISSION_FAILURE = 0x8000
 
+FV_FREESIZE_ERROR = 0x9000
+
 CODE_ERROR = 0xC0DE
 
 AUTOGEN_ERROR = 0xF000
diff --git a/BaseTools/Source/Python/build/build.py 
b/BaseTools/Source/Python/build/build.py
index bec848a7b2..c19be67254 100755
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -25,6 +25,7 @@ import traceback
 import multiprocessing
 from threading import Thread,Event,BoundedSemaphore
 import threading
+from linecache import getlines
 from subprocess import Popen,PIPE, STDOUT
 from collections import OrderedDict, defaultdict
 
@@ -1413,6 +1414,9 @@ class Build():
 if Target == 'fds':
 if GenFdsApi(AutoGenObject.GenFdsCommandDict, self.Db):
 EdkLogger.error("build", COMMAND_FAILURE)
+Threshold = self.GetFreeSizeThreshold()
+if Threshold:
+self.CheckFreeSizeThreshold(Threshold, AutoGenObject.FvDir)
 return True
 
 # run
@@ -2311,6 +2315,9 @@ class Build():
 GenFdsStart = time.time()
 if GenFdsApi(Wa.GenFdsCommandDict, self.Db):
 EdkLogger.error("build", COMMAND_FAILURE)
+Threshold = self.GetFreeSizeThreshold()
+if Threshold:
+self.CheckFreeSizeThreshold(Threshold, Wa.FvDir)
 
 #
 # Create MAP file for all platform FVs after GenFds.
@@ -2322,6 +2329,46 @@ class Build():
 #
 self._SaveMapFile(MapBuffer, Wa)
 self.CreateGuidedSectionToolsFile(Wa)
+
+## GetFreeSizeThreshold()
+#
+#   @retval int Threshold value
+#
+def GetFreeSizeThreshold(self):
+Threshold = None
+Threshold_Str = 
GlobalData.gCommandLineDefines.get('FV_SPARE_SPACE_THRESHOLD')
+if Threshold_Str:
+try:
+if Threshold_Str.lower().startswith('0x'):
+Threshold = int(Threshold_Str, 16)
+else:
+Threshold = int(Threshold_Str)
+except:
+EdkLogger.warn("build", 'incorrect value for 
FV_SPARE_SPACE_THRESHOLD %s. It can be decimal or hex format' % Threshold_Str)
+return Threshold
+
+def CheckFreeSizeThreshold(self, Threshold=None, FvDir=None):
+if not isinstance(Threshold, int):
+return
+if not isinstance(FvDir, str) or not FvDir:
+return
+FdfParserObject = GlobalData.gFdfParser
+FvRegionNameList = [FvName for FvName in 
FdfParserObject.Profile.FvDict if 
FdfParserObject.Profile.FvDict[FvName].FvRegionInFD]
+for FvName in FdfParserObject.Profile.FvDict:
+if FvName in FvRegionNameList:
+FvSpaceInfoFileName = os.path.join(FvDir, FvName.upper() + 
'.Fv.map')
+if os.path.exists(FvSpaceInfoFileName):
+FileLinesList = getlines(FvSpaceInfoFileName)
+for Line in FileLinesList:
+NameValue = Line.split('=')
+if len(NameValue) == 2 and NameValue[0].strip() == 
'EFI_FV_SPACE_SIZE':
+FreeSizeValue = int(NameValue[1].strip(), 0)
+if FreeSizeValue < Threshold:
+EdkLogger.error("build", FV_FREESIZE_ERROR,
+'Freespace value %d of %s FV 
is smaller than threshold, The value of FV_SPARE_SPACE_THRESHOLD is %d' % (
+FreeSizeValue, 

Re: [edk2-devel] [PATCH v2 07/28] Silicon/NXP: Implement SerialUartClockLib

2020-04-12 Thread Pankaj Bansal
> > ---
> >  .../SerialUartClockLib/SerialUartClockLib.c   | 27 +++
> >  .../SerialUartClockLib/SerialUartClockLib.inf | 26 ++
> 
> I requested after the initial submission that you "either follow the
> manual git setup steps from
> https://github.com/tianocore/tianocore.github.io/wiki/Laszlo%27s-unkempt-
> git-guide-for-edk2-contributors-and-maintainers
> or execute edk2/BaseTools/Scripts/SetupGit.py in each of the tianocore
> repositories"
> 
> Now, neither appears to have happened, as .c still comes before .inf
> and paths are still truncated.
> 
> But in addition to that, we realised that git happily ignores settings
> for --stat. So, please, execute aforementioned script, but then
> generate v3 with --stat=1000 --stat-graph-width=20.
> 

I am preparing the v3 of patches, and I am still seeing this issue after 
running SetupGit.py again.
After some tinkering I figured out the problem.
The git diff is ignoring the diff.Orderfile
The reason is line ending of edk2/BaseTools/Conf/diff.order
The current line ending of diff.order is windows, while I am using Linux to 
prepare patches.
If I change it to unix (dos2unix), then git is able to show proper diff
I wonder if same problem is with edk2/BaseTools/Conf/gitattributes ?
Looks like SetupGit.py needs to be modified to take into account the git 
running environment.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#57274): https://edk2.groups.io/g/devel/message/57274
Mute This Topic: https://groups.io/mt/72077432/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Update Edk2ToolsBuild.py to use multiple threads on Linux

2020-04-12 Thread Liming Gao
Reviewed-by: Liming Gao 

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Michael Kubacki
> Sent: Friday, April 3, 2020 1:05 AM
> To: devel@edk2.groups.io
> Cc: Feng, Bob C ; Gao, Liming 
> Subject: [edk2-devel] [PATCH v1 1/1] BaseTools: Update Edk2ToolsBuild.py to 
> use multiple threads on Linux
> 
> From: Sean Brogan 
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2640
> 
> Azure Pipelines agents have 2 threads. This commit has been shown to
> reduce the build time in half on those agents.
> 
> Cc: Bob C Feng 
> Cc: Liming Gao 
> Signed-off-by: Michael Kubacki 
> ---
>  BaseTools/Edk2ToolsBuild.py | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Edk2ToolsBuild.py b/BaseTools/Edk2ToolsBuild.py
> index 057d2e9e0633..1ea8187de693 100644
> --- a/BaseTools/Edk2ToolsBuild.py
> +++ b/BaseTools/Edk2ToolsBuild.py
> @@ -11,6 +11,7 @@ import os
>  import sys
>  import logging
>  import argparse
> +import multiprocessing
>  from edk2toolext import edk2_logging
>  from edk2toolext.environment import self_describing_environment
>  from edk2toolext.base_abstract_invocable import BaseAbstractInvocable
> @@ -141,7 +142,8 @@ class Edk2ToolsBuild(BaseAbstractInvocable):
>  return ret
> 
>  elif self.tool_chain_tag.lower().startswith("gcc"):
> -ret = RunCmd("make", "-C .", 
> workingdir=shell_env.get_shell_var("EDK_TOOLS_PATH"))
> +cpu_count = self.GetCpuThreads()
> +ret = RunCmd("make", f"-C .  -j {cpu_count}", 
> workingdir=shell_env.get_shell_var("EDK_TOOLS_PATH"))
>  if ret != 0:
>  raise Exception("Failed to build.")
> 
> @@ -154,6 +156,18 @@ class Edk2ToolsBuild(BaseAbstractInvocable):
>  logging.critical("Tool Chain not supported")
>  return -1
> 
> +def GetCpuThreads(self) -> int:
> +''' Function to return number of cpus. If error return 1'''
> +cpus = 1
> +try:
> +cpus = multiprocessing.cpu_count()
> +except:
> +# from the internet there are cases where cpu_count is not 
> implemented.
> +# will handle error by just doing single proc build
> +pass
> +return cpus
> +
> +
> 
>  def main():
>  Edk2ToolsBuild().Invoke()
> --
> 2.16.3.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#57273): https://edk2.groups.io/g/devel/message/57273
Mute This Topic: https://groups.io/mt/72729664/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page

2020-04-12 Thread Ard Biesheuvel

On 4/12/20 9:55 AM, Bi, Dandan wrote:

Hi Ard,

It seems that the root cause is the 'Network Device List' in the device manager 
menu is crated before EfiBootManagerConnectAll () is called in UiEntry function.
If we choose to add the EfiBootManagerConnectAll() in DeviceManagerUiLib with 
this patch, could we don't call the EfiBootManagerConnectAll() in UiEntry to 
avoid it's called twice when enter UiApp?



DeviceManagerUiLib is optional - this seems to be the purpose of the 
modular nature of UiApp with the NULL library class resolution.


Removing EfiBootManagerConnectAll() from UiApp itself means it may not 
ever be called.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#57272): https://edk2.groups.io/g/devel/message/57272
Mute This Topic: https://groups.io/mt/72879609/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page

2020-04-12 Thread Hot Tian
EfiBootManagerConnectAll is kind of BDS policy. Should it be controlled by Ui 
App or Ui Lib?

Thanks,
Hot

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Dandan Bi
Sent: Sunday, April 12, 2020 15:56
To: devel@edk2.groups.io; ard.biesheu...@arm.com; ler...@redhat.com
Cc: Gao, Zhichao ; Ni, Ray ; Wang, 
Jian J ; Wu, Hao A 
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all 
before creating menu page

Hi Ard,

It seems that the root cause is the 'Network Device List' in the device manager 
menu is crated before EfiBootManagerConnectAll () is called in UiEntry function.
If we choose to add the EfiBootManagerConnectAll() in DeviceManagerUiLib with 
this patch, could we don't call the EfiBootManagerConnectAll() in UiEntry to 
avoid it's called twice when enter UiApp?


Thanks,
Dandan
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of 
> Ard Biesheuvel
> Sent: Thursday, April 9, 2020 5:37 PM
> To: devel@edk2.groups.io; ler...@redhat.com
> Cc: Gao, Zhichao ; Ni, Ray ; 
> Wang, Jian J ; Wu, Hao A ; 
> Bi, Dandan 
> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib:
> connect all before creating menu page
> 
> On 4/9/20 11:29 AM, Laszlo Ersek via groups.io wrote:
> > On 04/08/20 19:28, Ard Biesheuvel wrote:
> >> The device manager UI library creates a UiApp submenu that contains 
> >> a list of network devices in the system. The logic that creates 
> >> this menu assumes that all handles have been connected to their 
> >> drivers, but this is not guaranteed in the general case.
> >>
> >> So work around this by doing an explicit ConnectAll() before 
> >> populating the pages.
> >>
> >> Cc: Zhichao Gao 
> >> Cc: Ray Ni 
> >> Cc: Jian J Wang 
> >> Cc: Hao A Wu 
> >> Cc: Dandan Bi 
> >> Signed-off-by: Ard Biesheuvel 
> >> ---
> >>   MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c| 7
> +++
> >>   MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h| 1 +
> >>   MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf |
> 1 +
> >>   3 files changed, 9 insertions(+)
> >
> > Can you describe one example that's (a) relevant to you and (b) 
> > affected by this issue?
> >
> 
> Yes. The SynQuacer platform has a NIC driver that waits up to 5 
> seconds for the link to come up after it resets the PHY, which never 
> happens if you boot without a network cable connected. But in the 
> general case, connecting all network drivers on a typical BootOrder based 
> boot should be be necessary.
> 
> Currently, ArmPkg's PlatformBootManagerLib does a 
> EfiBootManagerConnectAll () in its AfterConsole() callback, and so 
> this delay is always induced. I intend to 'fix' this platform (and 
> this library in general) to only do a 
> EfiBootManagerConnectAllDefaultConsoles
> () at this point, which is sufficient to interrupt the boot if 
> necessary, and otherwise, boot as fast as possible, without connecting 
> any devices that are not needed for this.
> 
> This works surprisingly well, with the exception of the 'Network 
> Device List' in the device manager UiApp menu, which no longer lists 
> the network interface, even though UiApp does a connect-all as it starts up.
> As you guessed, the constructor ordering versus the connect-all call 
> results in the 'network device list' menu to be populated before any 
> of the non- connected handles are connected. This is obviously against 
> the intent of doing a connect-all and enumerating all devices, and so this is 
> a bug in UiApp.
> 
> > EfiBootManagerConnectAll() feels quite heavy-weight; I'd *vaguely* 
> > prefer adding it elsewhere, instead of a "UI lib"'s constructor. I 
> > don't have a particular suggestion however.
> >
> > I guess part of the issue must be that the 
> > EfiBootManagerConnectAll() calls in BootManagerMenuApp / UiApp are 
> > not reached, somehow. Maybe those are the calls that DeviceManagerUiLib 
> > takes for granted.
> >
> 
> I agree that this approach is not the most elegant, but the way UiApp 
> is constructed makes it very hard to find a better way without some 
> major refactoring.
> 
> 
> >> diff --git
> a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c
> >> b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c
> >> index 0540e6fa8a44..3bc13d340775 100644
> >> --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c
> >> +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c
> >> @@ -892,6 +892,13 @@ DeviceManagerUiLibConstructor (
> >> );
> >> ASSERT (gDeviceManagerPrivate.HiiHandle != NULL);
> >>
> >> +  //
> >> +  // The device manager form contains a page listing all the 
> >> + network // controllers in the system. This list can only be 
> >> + populated if all  // handles have been connected, so do it here.
> >> +  //
> >> +  EfiBootManagerConnectAll ();
> >> +
> >> //
> >> // Update boot manager page
> >> //
> >> diff --git
> 

Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib: connect all before creating menu page

2020-04-12 Thread Dandan Bi
Hi Ard,

It seems that the root cause is the 'Network Device List' in the device manager 
menu is crated before EfiBootManagerConnectAll () is called in UiEntry function.
If we choose to add the EfiBootManagerConnectAll() in DeviceManagerUiLib with 
this patch, could we don't call the EfiBootManagerConnectAll() in UiEntry to 
avoid it's called twice when enter UiApp?


Thanks,
Dandan
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Ard Biesheuvel
> Sent: Thursday, April 9, 2020 5:37 PM
> To: devel@edk2.groups.io; ler...@redhat.com
> Cc: Gao, Zhichao ; Ni, Ray ;
> Wang, Jian J ; Wu, Hao A ; Bi,
> Dandan 
> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/DeviceManagerUiLib:
> connect all before creating menu page
> 
> On 4/9/20 11:29 AM, Laszlo Ersek via groups.io wrote:
> > On 04/08/20 19:28, Ard Biesheuvel wrote:
> >> The device manager UI library creates a UiApp submenu that contains a
> >> list of network devices in the system. The logic that creates this
> >> menu assumes that all handles have been connected to their drivers,
> >> but this is not guaranteed in the general case.
> >>
> >> So work around this by doing an explicit ConnectAll() before
> >> populating the pages.
> >>
> >> Cc: Zhichao Gao 
> >> Cc: Ray Ni 
> >> Cc: Jian J Wang 
> >> Cc: Hao A Wu 
> >> Cc: Dandan Bi 
> >> Signed-off-by: Ard Biesheuvel 
> >> ---
> >>   MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c| 7
> +++
> >>   MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h| 1 +
> >>   MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf |
> 1 +
> >>   3 files changed, 9 insertions(+)
> >
> > Can you describe one example that's (a) relevant to you and (b)
> > affected by this issue?
> >
> 
> Yes. The SynQuacer platform has a NIC driver that waits up to 5 seconds for
> the link to come up after it resets the PHY, which never happens if you boot
> without a network cable connected. But in the general case, connecting all
> network drivers on a typical BootOrder based boot should be be necessary.
> 
> Currently, ArmPkg's PlatformBootManagerLib does a
> EfiBootManagerConnectAll () in its AfterConsole() callback, and so this delay
> is always induced. I intend to 'fix' this platform (and this library in 
> general) to
> only do a EfiBootManagerConnectAllDefaultConsoles
> () at this point, which is sufficient to interrupt the boot if necessary, and
> otherwise, boot as fast as possible, without connecting any devices that are
> not needed for this.
> 
> This works surprisingly well, with the exception of the 'Network Device List' 
> in
> the device manager UiApp menu, which no longer lists the network interface,
> even though UiApp does a connect-all as it starts up.
> As you guessed, the constructor ordering versus the connect-all call results 
> in
> the 'network device list' menu to be populated before any of the non-
> connected handles are connected. This is obviously against the intent of
> doing a connect-all and enumerating all devices, and so this is a bug in 
> UiApp.
> 
> > EfiBootManagerConnectAll() feels quite heavy-weight; I'd *vaguely*
> > prefer adding it elsewhere, instead of a "UI lib"'s constructor. I
> > don't have a particular suggestion however.
> >
> > I guess part of the issue must be that the EfiBootManagerConnectAll()
> > calls in BootManagerMenuApp / UiApp are not reached, somehow. Maybe
> > those are the calls that DeviceManagerUiLib takes for granted.
> >
> 
> I agree that this approach is not the most elegant, but the way UiApp is
> constructed makes it very hard to find a better way without some major
> refactoring.
> 
> 
> >> diff --git
> a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c
> >> b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c
> >> index 0540e6fa8a44..3bc13d340775 100644
> >> --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c
> >> +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c
> >> @@ -892,6 +892,13 @@ DeviceManagerUiLibConstructor (
> >> );
> >> ASSERT (gDeviceManagerPrivate.HiiHandle != NULL);
> >>
> >> +  //
> >> +  // The device manager form contains a page listing all the network
> >> + // controllers in the system. This list can only be populated if
> >> + all  // handles have been connected, so do it here.
> >> +  //
> >> +  EfiBootManagerConnectAll ();
> >> +
> >> //
> >> // Update boot manager page
> >> //
> >> diff --git
> a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h
> >> b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h
> >> index 22fe12d2a5e8..c53c2a1a0e1a 100644
> >> --- a/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h
> >> +++ b/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.h
> >> @@ -23,6 +23,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >>   #include 
> >>   #include 
> >>   #include 
> >> +#include 
> >>   #include 
> >>
> >>   //
> >> diff --git
> >> 

Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/ReportStatusCodeRouter: Revert end pointer on out of resources

2020-04-12 Thread Dandan Bi
Reviewed-by: Dandan Bi 


Thanks,
Dandan
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Michael Kubacki
> Sent: Saturday, April 11, 2020 4:50 AM
> To: devel@edk2.groups.io
> Cc: Bi, Dandan ; Wu, Hao A ;
> Wang, Jian J ; Kun Qin ;
> Gao, Liming 
> Subject: [edk2-devel] [PATCH v1 1/1]
> MdeModulePkg/ReportStatusCodeRouter: Revert end pointer on out of
> resources
> 
> From: Michael Kubacki 
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2665
> 
> ReportDispatcher() is called by a software module to report a status code.
> The interface is generic and can be called frequently throughout the boot
> under various conditions. A certain set of conditions can cause the currently
> implemented algorithm for resource exhaustion to fail. A sample
> scenario:
> 
> 1. ReportStatusCode() is called at a TPL higher than one of the registered
>status code listeners making the call to the listener deferred until
>TPL is lowered.
> 2. Additional calls to ReportStatusCode() occur, so the data buffer
>continues to expand.
> 3. A call to ReportStatusCode() is made from within a memory allocation
>call (e.g. CoreAllocatePoolPages ()) which is protected from re-
>entrancy with mPoolMemoryLock. This will cause the ReallocatePool()
>call in ReportDispatcher() to fail. Because the end pointer was already
>moved to account for the data size, the end pointer is now moved
>beyond the buffer and invalid.
> 
> This commit saves the original end pointer value into a local variable called
> "FailSafeEndPointer" which tracks a safe end pointer to revert to in the case
> the allocated buffer size (CallbackEntry->EndPointer -
> CallbackEntry->StatusCodeDataBuffer) is still not large enough for the
> data.
> 
> Cc: Dandan Bi 
> Cc: Hao A Wu 
> Cc: Jian J Wang 
> Cc: Kun Qin 
> Cc: Liming Gao 
> Signed-off-by: Michael Kubacki 
> ---
> 
> MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportSt
> atusCodeRouterRuntimeDxe.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git
> a/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/Report
> StatusCodeRouterRuntimeDxe.c
> b/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/Report
> StatusCodeRouterRuntimeDxe.c
> index 6ca7e180ebb3..d7dc0a75ac83 100644
> ---
> a/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/Report
> StatusCodeRouterRuntimeDxe.c
> +++
> b/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/Report
> Sta
> +++ tusCodeRouterRuntimeDxe.c
> @@ -3,6 +3,7 @@
>and Status Code Runtime Protocol.
> 
>Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
> +  Copyright (c) Microsoft Corporation.
>SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -237,6 +238,7 @@ ReportDispatcher (
>RSC_DATA_ENTRY*RscData;
>EFI_STATUSStatus;
>VOID  *NewBuffer;
> +  EFI_PHYSICAL_ADDRESS  FailSafeEndPointer;
> 
>//
>// Use atom operation to avoid the reentant of report.
> @@ -267,6 +269,7 @@ ReportDispatcher (
>  // If callback is registered with TPL lower than TPL_HIGH_LEVEL, event
> must be signaled at boot time to possibly wait for
>  // allowed TPL to report status code. Related data should also be stored 
> in
> data buffer.
>  //
> +FailSafeEndPointer = CallbackEntry->EndPointer;
>  CallbackEntry->EndPointer  = ALIGN_VARIABLE (CallbackEntry-
> >EndPointer);
>  RscData = (RSC_DATA_ENTRY *) (UINTN) CallbackEntry->EndPointer;
>  CallbackEntry->EndPointer += sizeof (RSC_DATA_ENTRY); @@ -285,6
> +288,7 @@ ReportDispatcher (
>(VOID *) (UINTN) CallbackEntry->StatusCodeDataBuffer
>);
>  if (NewBuffer != NULL) {
> +  FailSafeEndPointer = (EFI_PHYSICAL_ADDRESS) (UINTN) NewBuffer
> + + (FailSafeEndPointer - CallbackEntry->StatusCodeDataBuffer);
>CallbackEntry->EndPointer = (EFI_PHYSICAL_ADDRESS) (UINTN)
> NewBuffer + (CallbackEntry->EndPointer - CallbackEntry-
> >StatusCodeDataBuffer);
>CallbackEntry->StatusCodeDataBuffer = (EFI_PHYSICAL_ADDRESS)
> (UINTN) NewBuffer;
>CallbackEntry->BufferSize *= 2; @@ -296,6 +300,7 @@
> ReportDispatcher (
>  // If data buffer is used up, do not report for this time.
>  //
>  if (CallbackEntry->EndPointer > (CallbackEntry->StatusCodeDataBuffer +
> CallbackEntry->BufferSize)) {
> +  CallbackEntry->EndPointer = FailSafeEndPointer;
>continue;
>  }
> 
> --
> 2.16.3.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#57269): https://edk2.groups.io/g/devel/message/57269
Mute This Topic: https://groups.io/mt/72931170/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/ReportStatusCodeRouter: Update RSC Data on reallocation

2020-04-12 Thread Dandan Bi
Reviewed-by: Dandan Bi 


Thanks,
Dandan

> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Michael Kubacki
> Sent: Thursday, April 9, 2020 8:05 AM
> To: devel@edk2.groups.io
> Cc: Bi, Dandan ; Wu, Hao A ;
> Wang, Jian J ; Kun Qin ;
> Gao, Liming 
> Subject: [edk2-devel] [PATCH v1 1/1]
> MdeModulePkg/ReportStatusCodeRouter: Update RSC Data on reallocation
> 
> From: Michael Kubacki 
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2670
> 
> The RSC router data buffer may be reallocated when the buffer is nearing
> exhaustion (7/8 portion of the buffer used).
> 
> While several pointers are updated to point to the newly allocated buffer,
> the RscData is not updated. This commit updates the RSC data pointer to the
> same offset in the reallocated data buffer.
> 
> Cc: Dandan Bi 
> Cc: Hao A Wu 
> Cc: Jian J Wang 
> Cc: Kun Qin 
> Cc: Liming Gao 
> Signed-off-by: Michael Kubacki 
> ---
> 
> MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportSt
> atusCodeRouterRuntimeDxe.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git
> a/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/Report
> StatusCodeRouterRuntimeDxe.c
> b/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/Report
> StatusCodeRouterRuntimeDxe.c
> index 6ca7e180ebb3..82fa2e025466 100644
> ---
> a/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/Report
> StatusCodeRouterRuntimeDxe.c
> +++
> b/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/Report
> Sta
> +++ tusCodeRouterRuntimeDxe.c
> @@ -3,6 +3,7 @@
>and Status Code Runtime Protocol.
> 
>Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
> +  Copyright (c) Microsoft Corporation.
>SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -286,6 +287,7 @@ ReportDispatcher (
>);
>  if (NewBuffer != NULL) {
>CallbackEntry->EndPointer = (EFI_PHYSICAL_ADDRESS) (UINTN)
> NewBuffer + (CallbackEntry->EndPointer - CallbackEntry-
> >StatusCodeDataBuffer);
> +  RscData = (RSC_DATA_ENTRY *) (UINTN) ((UINTN) NewBuffer +
> + ((UINTN) RscData - CallbackEntry->StatusCodeDataBuffer));
>CallbackEntry->StatusCodeDataBuffer = (EFI_PHYSICAL_ADDRESS)
> (UINTN) NewBuffer;
>CallbackEntry->BufferSize *= 2;
>  }
> --
> 2.16.3.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#57268): https://edk2.groups.io/g/devel/message/57268
Mute This Topic: https://groups.io/mt/72887077/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/ReportStatusCodeRouter: Clear RSC Data buffer if Data is NULL

2020-04-12 Thread Dandan Bi
Reviewed-by: Dandan Bi 


Thanks,
Dandan
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Michael Kubacki
> Sent: Thursday, April 9, 2020 6:12 AM
> To: devel@edk2.groups.io
> Cc: Bi, Dandan ; Wu, Hao A ;
> Wang, Jian J ; Kun Qin ;
> Gao, Liming 
> Subject: [edk2-devel] [PATCH v1 1/1]
> MdeModulePkg/ReportStatusCodeRouter: Clear RSC Data buffer if Data is
> NULL
> 
> From: Michael Kubacki 
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1969
> 
> ReportDispatcher() may be invoked with a NULL Data argument. When TPL is
> less than TPL_HIGH_LEVEL and Data is NULL, the EFI_STATUS_CODE_DATA
> structure inside RscData should be cleared so listeners will not receive data
> from a previous operation.
> 
> Cc: Dandan Bi 
> Cc: Hao A Wu 
> Cc: Jian J Wang 
> Cc: Kun Qin 
> Cc: Liming Gao 
> Signed-off-by: Michael Kubacki 
> ---
> 
> MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportSt
> atusCodeRouterRuntimeDxe.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git
> a/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/Report
> StatusCodeRouterRuntimeDxe.c
> b/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/Report
> StatusCodeRouterRuntimeDxe.c
> index 6ca7e180ebb3..5df83027c62d 100644
> ---
> a/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/Report
> StatusCodeRouterRuntimeDxe.c
> +++
> b/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/Report
> Sta
> +++ tusCodeRouterRuntimeDxe.c
> @@ -3,6 +3,7 @@
>and Status Code Runtime Protocol.
> 
>Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
> +  Copyright (c) Microsoft Corporation.
>SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -307,6 +308,9 @@ ReportDispatcher (
>  }
>  if (Data != NULL) {
>CopyMem (>Data, Data, Data->HeaderSize + Data->Size);
> +} else {
> +  ZeroMem (>Data, sizeof (RscData->Data));
> +  RscData->Data.HeaderSize = sizeof (RscData->Data);
>  }
> 
>  Status = gBS->SignalEvent (CallbackEntry->Event);
> --
> 2.16.3.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#57267): https://edk2.groups.io/g/devel/message/57267
Mute This Topic: https://groups.io/mt/72885147/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/ReportStatusCodeRouter: Take HeaderSize into Consideration

2020-04-12 Thread Dandan Bi
Reviewed-by: Dandan Bi 

Thanks,
Dandan
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Michael Kubacki
> Sent: Thursday, April 9, 2020 5:45 AM
> To: devel@edk2.groups.io
> Cc: Bi, Dandan ; Wu, Hao A ;
> Wang, Jian J ; Kun Qin ;
> Gao, Liming 
> Subject: [edk2-devel] [PATCH v1 1/1]
> MdeModulePkg/ReportStatusCodeRouter: Take HeaderSize into
> Consideration
> 
> From: Michael Kubacki 
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2669
> 
> Updates ReportDispatcher() to take the size in the HeaderSize field in a
> EFI_STATUS_CODE_DATA element into account when walking the data
> buffer.
> This size will cause the header size to differ from the compiled sizeof 
> header.
> 
> Cc: Dandan Bi 
> Cc: Hao A Wu 
> Cc: Jian J Wang 
> Cc: Kun Qin 
> Cc: Liming Gao 
> Signed-off-by: Michael Kubacki 
> ---
> 
> MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportSt
> atusCodeRouterRuntimeDxe.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/Report
> StatusCodeRouterRuntimeDxe.c
> b/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/Report
> StatusCodeRouterRuntimeDxe.c
> index 6ca7e180ebb3..920191cb3a8c 100644
> ---
> a/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/Report
> StatusCodeRouterRuntimeDxe.c
> +++
> b/MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/Report
> Sta
> +++ tusCodeRouterRuntimeDxe.c
> @@ -3,6 +3,7 @@
>and Status Code Runtime Protocol.
> 
>Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
> +  Copyright (c) Microsoft Corporation.
>SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -64,7 +65,7 @@ RscHandlerNotification (
>   >Data
>   );
> 
> -Address += (sizeof (RSC_DATA_ENTRY) + RscData->Data.Size);
> +Address += (OFFSET_OF (RSC_DATA_ENTRY, Data) +
> + RscData->Data.HeaderSize + RscData->Data.Size);
>  Address  = ALIGN_VARIABLE (Address);
>}
> 
> @@ -271,7 +272,7 @@ ReportDispatcher (
>  RscData = (RSC_DATA_ENTRY *) (UINTN) CallbackEntry->EndPointer;
>  CallbackEntry->EndPointer += sizeof (RSC_DATA_ENTRY);
>  if (Data != NULL) {
> -  CallbackEntry->EndPointer += Data->Size;
> +  CallbackEntry->EndPointer += (Data->Size + Data->HeaderSize -
> + sizeof (EFI_STATUS_CODE_DATA));
>  }
> 
>  //
> --
> 2.16.3.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#57266): https://edk2.groups.io/g/devel/message/57266
Mute This Topic: https://groups.io/mt/72884670/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-