[PATCH 1/1] BaseTools:Add the spare space FV image size checker

[PATCH 1/1] BaseTools:Add the spare space FV image size checker

[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=10000.
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 <liming.gao@intel.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Signed-off-by: Zhiju.Fan <zhijux.fan@intel.com>
---
 BaseTools/Source/Python/Common/BuildToolError.py |  2 ++
 BaseTools/Source/Python/GenFds/GenFds.py         | 32 +++++++++++++++++++++---
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/Python/Common/BuildToolError.py b/BaseTools/Source/Python/Common/BuildToolError.py
index ecc83d0f48bd..21549683cd19 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/GenFds/GenFds.py b/BaseTools/Source/Python/GenFds/GenFds.py
index d5511f4c40e5..c4782e89111b 100644
--- a/BaseTools/Source/Python/GenFds/GenFds.py
+++ b/BaseTools/Source/Python/GenFds/GenFds.py
@@ -29,7 +29,7 @@ from Common.Misc import DirCache, PathClass, GuidStructureStringToGuidString
 from Common.Misc import SaveFileOnChange, ClearDuplicatedInf
 from Common.BuildVersion import gBUILD_VERSION
 from Common.MultipleWorkspace import MultipleWorkspace as mws
-from Common.BuildToolError import FatalError, GENFDS_ERROR, CODE_ERROR, FORMAT_INVALID, RESOURCE_NOT_AVAILABLE, FILE_NOT_FOUND, OPTION_MISSING, FORMAT_NOT_SUPPORTED, OPTION_VALUE_INVALID, PARAMETER_INVALID
+from Common.BuildToolError import FatalError, GENFDS_ERROR, CODE_ERROR, FORMAT_INVALID, RESOURCE_NOT_AVAILABLE, FILE_NOT_FOUND, OPTION_MISSING, FORMAT_NOT_SUPPORTED, OPTION_VALUE_INVALID, PARAMETER_INVALID, FV_FREESIZE_ERROR
 from Workspace.WorkspaceDatabase import WorkspaceDatabase
 
 from .FdfParser import FdfParser, Warning
@@ -374,7 +374,8 @@ def GenFdsApi(FdsCommandDict, WorkSpaceDataBase=None):
         GenFds.GenerateGuidXRefFile(BuildWorkSpace, ArchList, FdfParserObj)
 
         """Display FV space info."""
-        GenFds.DisplayFvSpaceInfo(FdfParserObj)
+        Threshold = GenFds.GetFreeSizeThreshold()
+        GenFds.DisplayFvSpaceInfo(FdfParserObj, Threshold)
 
     except Warning as X:
         EdkLogger.error(X.ToolName, FORMAT_INVALID, File=X.FileName, Line=X.LineNumber, ExtraData=X.Message, RaiseError=False)
@@ -584,13 +585,31 @@ class GenFds(object):
                                     return ElementRegion.BlockSizeOfRegion(ElementFd.BlockSizeList)
             return DefaultBlockSize
 
+    ## GetFreeSizeThreshold()
+    #
+    #   @retval int             Threshold value
+    #
+    @staticmethod
+    def GetFreeSizeThreshold():
+        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("GenFds", 'incorrect value for FV_SPARE_SPACE_THRESHOLD %s. It can be decimal or hex format' % Threshold_Str)
+        return Threshold
+
     ## DisplayFvSpaceInfo()
     #
     #   @param  FvObj           Whose block size to get
     #   @retval None
     #
     @staticmethod
-    def DisplayFvSpaceInfo(FdfParserObject):
+    def DisplayFvSpaceInfo(FdfParserObject, Threshold):
 
         FvSpaceInfoList = []
         MaxFvNameLength = 0
@@ -623,6 +642,10 @@ class GenFds(object):
                     FvSpaceInfoList.append((FvName, Total, Used, Free))
 
         GenFdsGlobalVariable.InfLogger('\nFV Space Information')
+        if Threshold:
+            FvRegionNameList = [FvName for FvName in FdfParserObject.Profile.FvDict if FdfParserObject.Profile.FvDict[FvName].FvRegionInFD]
+        else:
+            FvRegionNameList = []
         for FvSpaceInfo in FvSpaceInfoList:
             Name = FvSpaceInfo[0]
             TotalSizeValue = int(FvSpaceInfo[1], 0)
@@ -634,6 +657,9 @@ class GenFds(object):
                 Percentage = str((UsedSizeValue + 0.0) / TotalSizeValue)[0:4].lstrip('0.')
 
             GenFdsGlobalVariable.InfLogger(Name + ' ' + '[' + Percentage + '%Full] ' + str(TotalSizeValue) + ' total, ' + str(UsedSizeValue) + ' used, ' + str(FreeSizeValue) + ' free')
+            if Threshold and Name in FvRegionNameList and FreeSizeValue < Threshold:
+                EdkLogger.error("GenFds", FV_FREESIZE_ERROR, 'Freespace of %s FV is smaller than threshold, The value of FV_SPARE_SPACE_THRESHOLD is %d' % (
+                                Name, Threshold))
 
     ## PreprocessImage()
     #
-- 
2.14.1.windows.1

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

[Sean]

[-- Attachment #1: Type: text/plain, Size: 195 bytes --]

I would have much rather seen this implemented independent of the GenFds tool as this is a nice feature but shouldn't be tightly coupled into the tool that does building and packaging firmware.

[-- Attachment #2: Type: text/html, Size: 207 bytes --]

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

[Fan, ZhijuX]

[-- Attachment #1: Type: text/plain, Size: 606 bytes --]

Hi,

It can be implemented independent of the GenFds tool
It accepts the parameters passed by building and GenFds Tool

Best Regards
Fan Zhiju



From: sean.brogan via [] <sean.brogan=microsoft.com@[]>
Sent: Friday, April 3, 2020 10:51 AM
To: Fan; Fan, ZhijuX <zhijux.fan@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH 1/1] BaseTools:Add the spare space FV image size checker

I would have much rather seen this implemented independent of the GenFds tool as this is a nice feature but shouldn't be tightly coupled into the tool that does building and packaging firmware.

[-- Attachment #2: Type: text/html, Size: 3583 bytes --]

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

[Sean]

[-- Attachment #1: Type: text/plain, Size: 259 bytes --]

Not sure I follow.  The changes are in BaseTools/Source/Python/GenFds/GenFds.py
I am suggesting this functionality to be written outside the GenFds.py files so that we don't add complexity to the critical tools used for building and packaging FW binaries.

[-- Attachment #2: Type: text/html, Size: 317 bytes --]