Re: [edk2] Proposition of a BmEnumerateBootOptions() hook.

[Zeng, Star]

Cc Ray.

Thanks,
Star
From: Marvin H?user [mailto:marvin.haeu...@outlook.com]
Sent: Tuesday, May 15, 2018 3:00 AM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Dong, Eric 
Subject: Proposition of a BmEnumerateBootOptions() hook.

Hey Star, Eric and everyone else,

I have seen that some platforms add a Boot Option for the UEFI Shell in 
"PlatformBootManagerBeforeConsole()", which is called as part of the regular 
boot flow.
This is surely beneficial for development platforms that are supposed to boot 
to UEFI Shell by default when no other option has been registered, however for 
retail platforms it usually makes more sense to show the UEFI Boot Menu, which 
renders adding the Shell Boot Option as part of the regular boot flow obsolete 
and just adds up to the boot time. Meanwhile, there is a function in the 
UefiBootManagerLib, "BmEnumerateBootOptions()", which is called prior to 
entering the Boot Menu and, in my opinion, would be the perfect place to 
introduce another PlatformBootManagerLib hook, which retrieves 
platform-specific boot options, such as an UEFI Shell or other utilities like a 
Memory Test application.
If you have a few spare minutes, I'll be happy for feedback.

Thanks in advance for your time.

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

Re: [edk2] Query regarding hole in EFI Memory Map

[Prakhya, Sai Praneeth]

> Of all the gin joints in all the towns in all the world, Prakhya, Sai 
> Praneeth had to
> walk into mine at 16:30 on Monday 14 May 2018 and say:
> 
> > Hi All,
> >
> > Recently, I have observed that there was a hole in EFI Memory Map
> > passed by firmware to Linux kernel. So, wanted to check with you if
> > this is expected or not.
> >
> > My Test setup:
> > I usually boot qemu with OVMF and Linux kernel. I use below command to
> > boot kernel. "qemu-system-x86_64 -cpu host -hda  -serial
> > stdio -bios  -m 2G -enable-kvm -smp 2"
> >
> > I have noticed that the EFI Memory Map (printed by kernel) is almost
> > contiguous but with only one hole ranging from 0xA to 0x10. As
> > far as I know, kernel hasn't modified this EFI Memory Map, so I am
> > assuming that firmware has passed memory map with a hole. I have
> > looked at UEFI spec "GetMemoryMap()" definition, and it says "The map
> > describes all of memory, no matter how it is being used". So, I am
> > thinking that EFI Memory Map shouldn't have any holes, am I correct?
> > If not, could someone please explain me the reason for this hole in EFI
> Memory Map.
> 
> The map may describe all of physical RAM, however it is not necessarily the 
> case
> that all available RAM be physically contiguous.
> 
> With older IBM PCs based on the Intel 8088 processor, you could only have a
> 1MB address space. The first 640KB was available for RAM. The remaining space
> traditionally contained memory-mapped option ROMs, particularly for things
> like the video BIOS routines. The VGA text screen was also mapped to 0xB8000.
> 
> Obviously, later processors made it possible to have additional memory above
> 1MB (sometimes called "high memory"), but for backward compatibility
> purposes, the gap from 0xA to 0xF remained.
> 
> So basically, on Intel machines you will always see this gap in RAM due to
> "hysterical raisins." It's just an artifact of the platform design. (And for 
> that
> reason you'll see it both with the UEFI memory map facility and the legacy 
> E820
> BIOS facility).

Thanks a lot! for the explanation Bill. I really appreciate it :)

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

Re: [edk2] Query regarding hole in EFI Memory Map

[Tim Lewis]

And now you see it again with 64-bit machines, where the flash and PCI
config space (and MMIO) appear below 4GB, but there is DRAM above and below
4GB. 

Tim

-Original Message-
From: edk2-devel  On Behalf Of Bill Paul
Sent: Monday, May 14, 2018 6:13 PM
To: edk2-devel@lists.01.org
Cc: Neri, Ricardo 
Subject: Re: [edk2] Query regarding hole in EFI Memory Map

Of all the gin joints in all the towns in all the world, Prakhya, Sai
Praneeth had to walk into mine at 16:30 on Monday 14 May 2018 and say:

> Hi All,
> 
> Recently, I have observed that there was a hole in EFI Memory Map 
> passed by firmware to Linux kernel. So, wanted to check with you if 
> this is expected or not.
> 
> My Test setup:
> I usually boot qemu with OVMF and Linux kernel. I use below command to 
> boot kernel. "qemu-system-x86_64 -cpu host -hda  -serial 
> stdio -bios  -m 2G -enable-kvm -smp 2"
> 
> I have noticed that the EFI Memory Map (printed by kernel) is almost 
> contiguous but with only one hole ranging from 0xA to 0x10. As 
> far as I know, kernel hasn't modified this EFI Memory Map, so I am 
> assuming that firmware has passed memory map with a hole. I have 
> looked at UEFI spec "GetMemoryMap()" definition, and it says "The map 
> describes all of memory, no matter how it is being used". So, I am 
> thinking that EFI Memory Map shouldn't have any holes, am I correct? 
> If not, could someone please explain me the reason for this hole in EFI
Memory Map.

The map may describe all of physical RAM, however it is not necessarily the
case that all available RAM be physically contiguous.

With older IBM PCs based on the Intel 8088 processor, you could only have a
1MB address space. The first 640KB was available for RAM. The remaining
space traditionally contained memory-mapped option ROMs, particularly for
things like the video BIOS routines. The VGA text screen was also mapped to
0xB8000.

Obviously, later processors made it possible to have additional memory above
1MB (sometimes called "high memory"), but for backward compatibility
purposes, the gap from 0xA to 0xF remained.

So basically, on Intel machines you will always see this gap in RAM due to
"hysterical raisins." It's just an artifact of the platform design. (And for
that reason you'll see it both with the UEFI memory map facility and the
legacy E820 BIOS facility).

-Bill


> 
> 
> Please let me know if you want me to post the EFI Memory Map or E820 
> map that I am looking at.
> 
> Note: I have also observed the same hole in E820 map.
> 
> 
> 
> Regards,
> 
> Sai
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
--

=
-Bill Paul(510) 749-2329 | Senior Member of Technical Staff,
 wp...@windriver.com | Master of Unix-Fu - Wind River
Systems

=
   "I put a dollar in a change machine. Nothing changed." - George Carlin

=
___
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] Query regarding hole in EFI Memory Map

[Bill Paul]

Of all the gin joints in all the towns in all the world, Prakhya, Sai Praneeth 
had to walk into mine at 16:30 on Monday 14 May 2018 and say:

> Hi All,
> 
> Recently, I have observed that there was a hole in EFI Memory Map passed by
> firmware to Linux kernel. So, wanted to check with you if this is expected
> or not.
> 
> My Test setup:
> I usually boot qemu with OVMF and Linux kernel. I use below command to boot
> kernel. "qemu-system-x86_64 -cpu host -hda  -serial stdio
> -bios  -m 2G -enable-kvm -smp 2"
> 
> I have noticed that the EFI Memory Map (printed by kernel) is almost
> contiguous but with only one hole ranging from 0xA to 0x10. As far
> as I know, kernel hasn't modified this EFI Memory Map, so I am assuming
> that firmware has passed memory map with a hole. I have looked at UEFI
> spec "GetMemoryMap()" definition, and it says "The map describes all of
> memory, no matter how it is being used". So, I am thinking that EFI Memory
> Map shouldn't have any holes, am I correct? If not, could someone please
> explain me the reason for this hole in EFI Memory Map.

The map may describe all of physical RAM, however it is not necessarily the 
case that all available RAM be physically contiguous.

With older IBM PCs based on the Intel 8088 processor, you could only have a 
1MB address space. The first 640KB was available for RAM. The remaining space 
traditionally contained memory-mapped option ROMs, particularly for things 
like the video BIOS routines. The VGA text screen was also mapped to 0xB8000.

Obviously, later processors made it possible to have additional memory above 
1MB (sometimes called "high memory"), but for backward compatibility purposes, 
the gap from 0xA to 0xF remained.

So basically, on Intel machines you will always see this gap in RAM due to 
"hysterical raisins." It's just an artifact of the platform design. (And for 
that reason you'll see it both with the UEFI memory map facility and the 
legacy E820 BIOS facility).

-Bill


> 
> 
> Please let me know if you want me to post the EFI Memory Map or E820 map
> that I am looking at.
> 
> Note: I have also observed the same hole in E820 map.
> 
> 
> 
> Regards,
> 
> Sai
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
-- 
=
-Bill Paul(510) 749-2329 | Senior Member of Technical Staff,
 wp...@windriver.com | Master of Unix-Fu - Wind River Systems
=
   "I put a dollar in a change machine. Nothing changed." - George Carlin
=
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] Query regarding hole in EFI Memory Map

[Prakhya, Sai Praneeth]

Hi All,

Recently, I have observed that there was a hole in EFI Memory Map passed by 
firmware to Linux kernel. So, wanted to check with you if this is expected or 
not.

My Test setup:
I usually boot qemu with OVMF and Linux kernel. I use below command to boot 
kernel.
"qemu-system-x86_64 -cpu host -hda  -serial stdio -bios  
-m 2G -enable-kvm -smp 2"

I have noticed that the EFI Memory Map (printed by kernel) is almost contiguous 
but with only one hole ranging from 0xA to 0x10. As far as I know, 
kernel hasn't modified this EFI Memory Map, so I am assuming that firmware has 
passed memory map with a hole. I have looked at UEFI spec "GetMemoryMap()" 
definition, and it says "The map describes all of memory, no matter how it is 
being used". So, I am thinking that EFI Memory Map shouldn't have any holes, am 
I correct? If not, could someone please explain me the reason for this hole in 
EFI Memory Map.



Please let me know if you want me to post the EFI Memory Map or E820 map that I 
am looking at.

Note: I have also observed the same hole in E820 map.



Regards,

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

[edk2] Proposition of a BmEnumerateBootOptions() hook.

[Marvin H?user]

Hey Star, Eric and everyone else,

I have seen that some platforms add a Boot Option for the UEFI Shell in 
"PlatformBootManagerBeforeConsole()", which is called as part of the regular 
boot flow.
This is surely beneficial for development platforms that are supposed to boot 
to UEFI Shell by default when no other option has been registered, however for 
retail platforms it usually makes more sense to show the UEFI Boot Menu, which 
renders adding the Shell Boot Option as part of the regular boot flow obsolete 
and just adds up to the boot time. Meanwhile, there is a function in the 
UefiBootManagerLib, "BmEnumerateBootOptions()", which is called prior to 
entering the Boot Menu and, in my opinion, would be the perfect place to 
introduce another PlatformBootManagerLib hook, which retrieves 
platform-specific boot options, such as an UEFI Shell or other utilities like a 
Memory Test application.
If you have a few spare minutes, I'll be happy for feedback.

Thanks in advance for your time.

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

[edk2] [PATCH] EmulatorPkg/SmbiosLib: Declare the correct library class.

[Marvin Häuser]

Currently, SmbiosLib declares the PcdLib library class. Update the
declaration to declare SmbiosLib.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marvin Haeuser 
---
 EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf 
b/EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf
index adcd7ef08e20..a688fe02f225 100644
--- a/EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf
+++ b/EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf
@@ -2,7 +2,7 @@
 # SMBIOS Library 
 #
 # Copyright (c) 2012, Apple Inc. All rights reserved. 
-# Portions copyright (c) 2006 - 2010, Intel Corporation. All rights 
reserved.
+# Portions copyright (c) 2006 - 2018, Intel Corporation. All rights 
reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
@@ -20,7 +20,7 @@ [Defines]
   FILE_GUID  = 881863A2-09FD-3E44-8D62-7AE038D03747
   MODULE_TYPE= DXE_DRIVER
   VERSION_STRING = 1.0
-  LIBRARY_CLASS  = PcdLib|DXE_CORE DXE_DRIVER 
DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION 
UEFI_DRIVER 
+  LIBRARY_CLASS  = SmbiosLib|DXE_CORE DXE_DRIVER 
DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION 
UEFI_DRIVER 
 
   CONSTRUCTOR= SmbiosLibConstructor
 
-- 
2.17.0.windows.1

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

[edk2] [PATCH] MdePkg: Update MmSwDispatch.h's references to SmmSw2Dispatch.

[Marvin Häuser]

MmSwDispatch.h current refers to the deprecated SmmSw2Dispatch
protocol. Replace those references with the new MmSwDispatch name.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marvin Haeuser 
---
 MdePkg/Include/Protocol/MmSwDispatch.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Include/Protocol/MmSwDispatch.h 
b/MdePkg/Include/Protocol/MmSwDispatch.h
index d3acb64e6f3d..44ef69ed76c6 100644
--- a/MdePkg/Include/Protocol/MmSwDispatch.h
+++ b/MdePkg/Include/Protocol/MmSwDispatch.h
@@ -4,7 +4,7 @@
 
   This protocol provides the parent dispatch service for a given MMI source 
generator.
 
-  Copyright (c) 2017, Intel Corporation. All rights reserved.
+  Copyright (c) 2017 - 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
@@ -15,8 +15,8 @@
 
 **/
 
-#ifndef _MM_SW_DISPATCH2_H_
-#define _MM_SW_DISPATCH2_H_
+#ifndef _MM_SW_DISPATCH_H_
+#define _MM_SW_DISPATCH_H_
 
 #include 
 
@@ -117,7 +117,7 @@ EFI_STATUS
 ///
 /// Interface structure for the MM Software MMI Dispatch Protocol.
 ///
-/// The EFI_MM_SW_DISPATCH2_PROTOCOL provides the ability to install child 
handlers for the
+/// The EFI_MM_SW_DISPATCH_PROTOCOL provides the ability to install child 
handlers for the
 /// given software.  These handlers will respond to software interrupts, and 
the maximum software
 /// interrupt in the EFI_MM_SW_REGISTER_CONTEXT is denoted by MaximumSwiValue.
 ///
-- 
2.17.0.windows.1

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

[edk2] [PATCH v1 11/11] BaseTools: remove extra assignment

[Jaben Carsey]

there is no use to assign back to a variable.  just return the result.

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

diff --git a/BaseTools/Source/Python/Common/Expression.py 
b/BaseTools/Source/Python/Common/Expression.py
index 3133f610b4a7..3036af058c1d 100644
--- a/BaseTools/Source/Python/Common/Expression.py
+++ b/BaseTools/Source/Python/Common/Expression.py
@@ -197,8 +197,7 @@ def IntToStr(Value):
 while Value > 0:
 StrList.append(chr(Value & 0xff))
 Value = Value >> 8
-Value = '"{VAL}"'.format(VAL=''.join(StrList))
-return Value
+return '"{VAL}"'.format(VAL=''.join(StrList))
 
 SupportedInMacroList = ['TARGET', 'TOOL_CHAIN_TAG', 'ARCH', 'FAMILY']
 
-- 
2.16.2.windows.1

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

[edk2] [PATCH v1 07/11] BaseTools: refactor file opening/writing

[Jaben Carsey]

change mode minimal needed permissions
change to use with statement

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py   |  71 +---
 BaseTools/Source/Python/AutoGen/GenC.py  |   5 +-
 BaseTools/Source/Python/AutoGen/GenMake.py   |   5 +-
 BaseTools/Source/Python/AutoGen/IdfClassObject.py|  21 ++--
 BaseTools/Source/Python/AutoGen/StrGather.py |  18 +--
 BaseTools/Source/Python/AutoGen/UniClassObject.py|   5 +-
 BaseTools/Source/Python/AutoGen/ValidCheckingInfoObject.py   |   5 +-
 BaseTools/Source/Python/BPDG/GenVpd.py   |  30 +++--
 BaseTools/Source/Python/Common/Misc.py   |  39 +++
 BaseTools/Source/Python/Common/TargetTxtClassObject.py   |  13 +--
 BaseTools/Source/Python/Common/ToolDefClassObject.py |   4 +-
 BaseTools/Source/Python/Common/VpdInfoFile.py|   4 +-
 BaseTools/Source/Python/Ecc/Ecc.py   |   4 +-
 BaseTools/Source/Python/Eot/EotGlobalData.py |  14 +--
 BaseTools/Source/Python/Eot/FileProfile.py   |   8 +-
 BaseTools/Source/Python/Eot/Report.py|   6 +-
 BaseTools/Source/Python/GenFds/Capsule.py|   7 +-
 BaseTools/Source/Python/GenFds/CapsuleData.py|  10 +-
 BaseTools/Source/Python/GenFds/FdfParser.py  |  20 +---
 BaseTools/Source/Python/GenFds/FfsFileStatement.py   |   5 +-
 BaseTools/Source/Python/GenFds/Fv.py |  37 +++---
 BaseTools/Source/Python/GenFds/FvImageSection.py |   8 +-
 BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py   |  53 -
 BaseTools/Source/Python/GenFds/GuidSection.py|  45 
 BaseTools/Source/Python/GenFds/Region.py |  15 +--
 BaseTools/Source/Python/GenFds/Vtf.py| 122 
++--
 BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py |  21 ++--
 BaseTools/Source/Python/PatchPcdValue/PatchPcdValue.py   |  19 ++-
 BaseTools/Source/Python/Table/TableReport.py |  47 
 BaseTools/Source/Python/TargetTool/TargetTool.py |  97 

 BaseTools/Source/Python/Trim/Trim.py |  67 +--
 BaseTools/Source/Python/Workspace/DscBuildData.py|  17 +--
 BaseTools/Source/Python/build/BuildReport.py |  81 
+++--
 BaseTools/Source/Python/build/build.py   | 106 
-
 34 files changed, 453 insertions(+), 576 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 619e1e41e32b..009e5c56781d 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -653,10 +653,8 @@ class WorkspaceAutoGen(AutoGen):
 for files in AllWorkSpaceMetaFiles:
 if files.endswith('.dec'):
 continue
-f = open(files, 'r')
-Content = f.read()
-f.close()
-m.update(Content)
+with open(files, 'r') as f:
+m.update(f.read())
 SaveFileOnChange(os.path.join(self.BuildDir, 'AutoGen.hash'), 
m.hexdigest(), True)
 GlobalData.gPlatformHash = m.hexdigest()
 
@@ -664,11 +662,11 @@ class WorkspaceAutoGen(AutoGen):
 # Write metafile list to build directory
 #
 AutoGenFilePath = os.path.join(self.BuildDir, 'AutoGen')
-if os.path.exists (AutoGenFilePath):
-os.remove(AutoGenFilePath)
 if not os.path.exists(self.BuildDir):
 os.makedirs(self.BuildDir)
-with open(os.path.join(self.BuildDir, 'AutoGen'), 'w+') as file:
+elif os.path.exists (AutoGenFilePath):
+os.remove(AutoGenFilePath)
+with open(AutoGenFilePath, 'w') as file:
 for f in AllWorkSpaceMetaFiles:
 print >> file, f
 return True
@@ -679,20 +677,16 @@ class WorkspaceAutoGen(AutoGen):
 HashFile = os.path.join(PkgDir, Pkg.PackageName + '.hash')
 m = hashlib.md5()
 # Get .dec file's hash value
-f = open(Pkg.MetaFile.Path, 'r')
-Content = f.read()
-f.close()
-m.update(Content)
+with open(Pkg.MetaFile.Path, 'r') as f:
+m.update(f.read())
 # Get include files hash value
 if Pkg.Includes:
 for inc in Pkg.Includes:
 for Root, Dirs, Files in os.walk(str(inc)):
 for File in Files:
 File_Path = os.path.join(Root, File)
-f = 

[edk2] [PATCH v1 09/11] BaseTools: refactor to stop re-allocating strings

[Jaben Carsey]

strings are immutable.  allocate minimal duplication.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 94 
+++-
 BaseTools/Source/Python/AutoGen/GenC.py|  2 +-
 BaseTools/Source/Python/AutoGen/GenDepex.py| 15 ++--
 BaseTools/Source/Python/AutoGen/GenMake.py |  6 +-
 BaseTools/Source/Python/AutoGen/GenVar.py  |  2 +-
 BaseTools/Source/Python/AutoGen/IdfClassObject.py  |  2 +-
 BaseTools/Source/Python/AutoGen/StrGather.py   |  4 +-
 BaseTools/Source/Python/AutoGen/UniClassObject.py  |  3 +-
 BaseTools/Source/Python/AutoGen/ValidCheckingInfoObject.py | 17 ++--
 BaseTools/Source/Python/Common/Expression.py   | 42 +
 BaseTools/Source/Python/Common/Misc.py |  8 +-
 BaseTools/Source/Python/Common/String.py   |  2 +-
 BaseTools/Source/Python/CommonDataClass/CommonClass.py | 29 +++---
 BaseTools/Source/Python/GenFds/Capsule.py  | 19 ++--
 BaseTools/Source/Python/GenFds/FdfParser.py|  8 +-
 BaseTools/Source/Python/GenFds/FfsInfStatement.py  |  8 +-
 BaseTools/Source/Python/GenFds/Fv.py   | 72 +--
 BaseTools/Source/Python/GenFds/GenFds.py   |  4 +-
 BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py | 24 ++---
 BaseTools/Source/Python/GenFds/OptionRom.py|  2 -
 BaseTools/Source/Python/GenFds/Vtf.py  | 76 
+---
 BaseTools/Source/Python/Table/TableDataModel.py| 11 +--
 BaseTools/Source/Python/Workspace/DscBuildData.py  | 14 +--
 BaseTools/Source/Python/Workspace/InfBuildData.py  |  4 +-
 BaseTools/Source/Python/Workspace/MetaDataTable.py | 10 +--
 BaseTools/Source/Python/Workspace/MetaFileParser.py|  4 +-
 26 files changed, 196 insertions(+), 286 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 599331060187..4ccb50a0a0af 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -815,42 +815,46 @@ class WorkspaceAutoGen(AutoGen):
 _PcdName = FfsFile.NameGuid.lstrip("PCD(").rstrip(")")
 PcdFoundFlag = False
 for Pa in self.AutoGenObjectList:
-if not PcdFoundFlag:
-for PcdItem in Pa.AllPcdList:
-if (PcdItem.TokenSpaceGuidCName + "." + 
PcdItem.TokenCName) == _PcdName:
+#
+# once found, get out of the loop
+#
+if PcdFoundFlag:
+break
+for PcdItem in Pa.AllPcdList:
+if 
"{TSG}.{CN}".format(TSG=PcdItem.TokenSpaceGuidCName, CN=PcdItem.TokenCName) == 
_PcdName:
+#
+# First convert from CFormatGuid to GUID 
string
+#
+_PcdGuidString = 
GuidStructureStringToGuidString(PcdItem.DefaultValue)
+
+if not _PcdGuidString:
 #
-# First convert from CFormatGuid to 
GUID string
+# Then try Byte array.
 #
-_PcdGuidString = 
GuidStructureStringToGuidString(PcdItem.DefaultValue)
-
-if not _PcdGuidString:
-#
-# Then try Byte array.
-#
-_PcdGuidString = 
GuidStructureByteArrayToGuidString(PcdItem.DefaultValue)
+_PcdGuidString = 
GuidStructureByteArrayToGuidString(PcdItem.DefaultValue)
 
-if not _PcdGuidString:
-#
-# Not Byte array or CFormat GUID, 
raise error.
-#
-EdkLogger.error("build",
-FORMAT_INVALID,
-"The format of PCD 
value is incorrect. PCD: %s , Value: %s\n" % (_PcdName, PcdItem.DefaultValue),
- 

[edk2] [PATCH v1 08/11] BaseTools: refactor to change object types

[Jaben Carsey]

change to object types that are closer to use case.  for example:
when using a list as a double ended queue, use the built in object.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py   | 30 +++-
 BaseTools/Source/Python/GenFds/FdfParser.py  |  5 ++--
 BaseTools/Source/Python/Workspace/WorkspaceCommon.py | 20 ++---
 3 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 009e5c56781d..599331060187 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -45,10 +45,14 @@ import InfSectionParser
 import datetime
 import hashlib
 from GenVar import VariableMgr,var_info
-from collections import OrderedDict
-from collections import defaultdict
+from collections import OrderedDict,defaultdict,deque
 from abc import ABCMeta, abstractmethod
 
+class OrderedListDict(OrderedDict, defaultdict):
+def __init__(self, *args, **kwargs):
+super(OrderedListDict, self).__init__(*args, **kwargs)
+self.default_factory = list
+
 ## Regular expression for splitting Dependency Expression string into tokens
 gDepexTokenPattern = re.compile("(\(|\)|\w+| \S+\.inf)")
 
@@ -2172,8 +2176,8 @@ class PlatformAutoGen(AutoGen):
 
 # EdkII module
 LibraryConsumerList = [Module]
-Constructor = []
-ConsumedByList  = OrderedDict()
+Constructor = set()
+ConsumedByList = OrderedListDict()
 LibraryInstance = OrderedDict()
 
 EdkLogger.verbose("")
@@ -2219,10 +2223,8 @@ class PlatformAutoGen(AutoGen):
 continue
 
 if LibraryModule.ConstructorList != [] and LibraryModule not 
in Constructor:
-Constructor.append(LibraryModule)
+Constructor.add(LibraryModule)
 
-if LibraryModule not in ConsumedByList:
-ConsumedByList[LibraryModule] = []
 # don't add current module itself to consumer list
 if M != Module:
 if M in ConsumedByList[LibraryModule]:
@@ -2235,8 +2237,8 @@ class PlatformAutoGen(AutoGen):
 #
 # Q <- Set of all nodes with no incoming edges
 #
-LibraryList = [] #LibraryInstance.values()
-Q = []
+LibraryList = []
+Q = deque()
 for LibraryClassName in LibraryInstance:
 M = LibraryInstance[LibraryClassName]
 LibraryList.append(M)
@@ -2248,7 +2250,7 @@ class PlatformAutoGen(AutoGen):
 #
 while True:
 EdgeRemoved = True
-while Q == [] and EdgeRemoved:
+while not Q and EdgeRemoved:
 EdgeRemoved = False
 # for each node Item with a Constructor
 for Item in LibraryList:
@@ -2263,12 +2265,12 @@ class PlatformAutoGen(AutoGen):
 EdgeRemoved = True
 if ConsumedByList[Item] == []:
 # insert Item into Q
-Q.insert(0, Item)
+Q.appendleft(Item)
 break
-if Q != []:
+if Q:
 break
 # DAG is done if there's no more incoming edge for all nodes
-if Q == []:
+if not Q:
 break
 
 # remove node from Q
@@ -2286,7 +2288,7 @@ class PlatformAutoGen(AutoGen):
 if ConsumedByList[Item] != []:
 continue
 # insert Item into Q, if Item has no other incoming edges
-Q.insert(0, Item)
+Q.appendleft(Item)
 
 #
 # if any remaining node Item in the graph has a constructor and an 
incoming edge, then the graph has a cycle
diff --git a/BaseTools/Source/Python/GenFds/FdfParser.py 
b/BaseTools/Source/Python/GenFds/FdfParser.py
index 8d1a4b543f0e..d511cf4f9d5a 100644
--- a/BaseTools/Source/Python/GenFds/FdfParser.py
+++ b/BaseTools/Source/Python/GenFds/FdfParser.py
@@ -43,6 +43,7 @@ import OptionRom
 import OptRomInfStatement
 import OptRomFileStatement
 import string
+from collections import deque
 
 from GenFdsGlobalVariable import GenFdsGlobalVariable
 from Common.BuildToolError import *
@@ -89,7 +90,7 @@ BaseAddrValuePattern = re.compile('^0[xX][0-9a-fA-F]+')
 FileExtensionPattern = re.compile(r'([a-zA-Z][a-zA-Z0-9]*)')
 TokenFindPattern = 
re.compile(r'([a-zA-Z0-9\-]+|\$\(TARGET\)|\*)_([a-zA-Z0-9\-]+|\$\(TOOL_CHAIN_TAG\)|\*)_([a-zA-Z0-9\-]+|\$\(ARCH\)|\*)')
 
-AllIncludeFileList = []
+AllIncludeFileList = deque()
 
 # Get the closest parent
 def GetParentAtLine (Line):
@@ 

[edk2] [PATCH v1 06/11] BaseTools: refactor section generation

[Jaben Carsey]

use with for opening files
remove unneeded variables
dont seek to 0 for just opened file

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/Common/DataType.py   |   1 +
 BaseTools/Source/Python/GenFds/CompressSection.py|  18 +-
 BaseTools/Source/Python/GenFds/DataSection.py|  28 +--
 BaseTools/Source/Python/GenFds/DepexSection.py   |   2 +-
 BaseTools/Source/Python/GenFds/EfiSection.py | 109 +
 BaseTools/Source/Python/GenFds/FfsInfStatement.py|  10 +-
 BaseTools/Source/Python/GenFds/FvImageSection.py |  28 +--
 BaseTools/Source/Python/GenFds/GuidSection.py|   8 +-
 BaseTools/Source/Python/GenFds/OptRomInfStatement.py |   4 +-
 BaseTools/Source/Python/GenFds/Section.py| 231 
 BaseTools/Source/Python/GenFds/UiSection.py  |  18 +-
 BaseTools/Source/Python/GenFds/VerSection.py |  22 +-
 12 files changed, 206 insertions(+), 273 deletions(-)

diff --git a/BaseTools/Source/Python/Common/DataType.py 
b/BaseTools/Source/Python/Common/DataType.py
index a72c7e6f067f..93136dff0db2 100644
--- a/BaseTools/Source/Python/Common/DataType.py
+++ b/BaseTools/Source/Python/Common/DataType.py
@@ -50,6 +50,7 @@ TAB_EDK_SOURCE = '$(EDK_SOURCE)'
 TAB_EFI_SOURCE = '$(EFI_SOURCE)'
 TAB_WORKSPACE = '$(WORKSPACE)'
 TAB_FV_DIRECTORY = 'FV'
+TAB_SEC_DIRECTORY = 'SEC'
 
 TAB_ARCH_NULL = ''
 TAB_ARCH_COMMON = 'COMMON'
diff --git a/BaseTools/Source/Python/GenFds/CompressSection.py 
b/BaseTools/Source/Python/GenFds/CompressSection.py
index cdae74c52fd9..b7aa72e43992 100644
--- a/BaseTools/Source/Python/GenFds/CompressSection.py
+++ b/BaseTools/Source/Python/GenFds/CompressSection.py
@@ -56,7 +56,7 @@ class CompressSection (CompressSectionClassObject) :
 #
 def GenSection(self, OutputPath, ModuleName, SecNum, KeyStringList, FfsInf 
= None, Dict = {}, IsMakefile = False):
 
-if FfsInf is not None:
+if FfsInf:
 self.CompType = FfsInf.__ExtendMacro__(self.CompType)
 self.Alignment = FfsInf.__ExtendMacro__(self.Alignment)
 
@@ -68,13 +68,13 @@ class CompressSection (CompressSectionClassObject) :
 Index = Index + 1
 SecIndex = '%s.%d' %(SecNum, Index)
 ReturnSectList, AlignValue = Sect.GenSection(OutputPath, 
ModuleName, SecIndex, KeyStringList, FfsInf, Dict, IsMakefile=IsMakefile)
-if AlignValue is not None:
-if MaxAlign is None:
+if AlignValue:
+if not MaxAlign:
 MaxAlign = AlignValue
 if GenFdsGlobalVariable.GetAlignment (AlignValue) > 
GenFdsGlobalVariable.GetAlignment (MaxAlign):
 MaxAlign = AlignValue
 if ReturnSectList != []:
-if AlignValue is None:
+if not AlignValue:
 AlignValue = "1"
 for FileData in ReturnSectList:
 SectFiles += (FileData,)
@@ -83,17 +83,13 @@ class CompressSection (CompressSectionClassObject) :
 OutputFile = OutputPath + \
  os.sep + \
  ModuleName + \
- SUP_MODULE_SEC  + \
+ SUP_MODULE_SEC + \
  SecNum + \
  SectionSuffix['COMPRESS']
 OutputFile = os.path.normpath(OutputFile)
 DummyFile = OutputFile + '.dummy'
 GenFdsGlobalVariable.GenerateSection(DummyFile, SectFiles, 
InputAlign=SectAlign, IsMakefile=IsMakefile)
 
-GenFdsGlobalVariable.GenerateSection(OutputFile, [DummyFile], 
Section.Section.SectionType['COMPRESS'],
+GenFdsGlobalVariable.GenerateSection(OutputFile, [DummyFile], 
Section.SectionType['COMPRESS'],
  
CompressionType=self.CompTypeDict[self.CompType], IsMakefile=IsMakefile)
-OutputFileList = []
-OutputFileList.append(OutputFile)
-return OutputFileList, self.Alignment
-
-
+return [OutputFile], self.Alignment
diff --git a/BaseTools/Source/Python/GenFds/DataSection.py 
b/BaseTools/Source/Python/GenFds/DataSection.py
index f0e5efab4178..71c2796b0b39 100644
--- a/BaseTools/Source/Python/GenFds/DataSection.py
+++ b/BaseTools/Source/Python/GenFds/DataSection.py
@@ -50,39 +50,39 @@ class DataSection (DataSectionClassObject):
 #   @retval tuple   (Generated file name list, section alignment)
 #
 def GenSection(self, OutputPath, ModuleName, SecNum, keyStringList, 
FfsFile = None, Dict = {}, IsMakefile = False):
+
+self.SectFileName = 
GenFdsGlobalVariable.ReplaceWorkspaceMacro(self.SectFileName)
 #
 # Prepare the parameter of GenSection
 #
-if FfsFile is not None:
-self.SectFileName = 

[edk2] [PATCH v1 03/11] BaseTools: remove unused code

[Jaben Carsey]

delete commented out code
delete never used class/variable/function/import
refactor to remove Ffs class
dont construct class just for class attribute

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/AutoGen/GenMake.py | 17 ++--
 BaseTools/Source/Python/CommonDataClass/DataClass.py   | 47 +--
 BaseTools/Source/Python/Ecc/Database.py| 76 +
 BaseTools/Source/Python/Ecc/c.py   |  1 -
 BaseTools/Source/Python/GenFds/CapsuleData.py  |  1 -
 BaseTools/Source/Python/GenFds/CompressSection.py  |  6 +-
 BaseTools/Source/Python/GenFds/DataSection.py  |  4 +-
 BaseTools/Source/Python/GenFds/DepexSection.py |  1 -
 BaseTools/Source/Python/GenFds/EfiSection.py   | 16 ++--
 BaseTools/Source/Python/GenFds/Ffs.py  | 88 

 BaseTools/Source/Python/GenFds/FfsFileStatement.py |  4 +-
 BaseTools/Source/Python/GenFds/FfsInfStatement.py  | 10 +--
 BaseTools/Source/Python/GenFds/Fv.py   |  1 -
 BaseTools/Source/Python/GenFds/FvImageSection.py   |  6 +-
 BaseTools/Source/Python/GenFds/GuidSection.py  |  4 +-
 BaseTools/Source/Python/GenFds/OptRomInfStatement.py   |  7 +-
 BaseTools/Source/Python/GenFds/UiSection.py|  6 +-
 BaseTools/Source/Python/GenFds/VerSection.py   |  6 +-
 BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 18 ++--
 BaseTools/Source/Python/build/build.py |  2 +-
 20 files changed, 81 insertions(+), 240 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 68ec9a817133..1c8ab7fe1ec8 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -74,8 +74,6 @@ class BuildFile(object):
 ## template used to generate the build file (i.e. makefile if using make)
 _TEMPLATE_ = TemplateString('')
 
-_DEFAULT_FILE_NAME_ = "Makefile"
-
 ## default file name for each type of build file
 _FILE_NAME_ = {
 "nmake" :   "Makefile",
@@ -151,23 +149,18 @@ class BuildFile(object):
 "gmake" :   "test -f %(Src)s && $(CP) %(Src)s %(Dst)s"
 }
 
-_CD_TEMPLATE_ = {
-"nmake" :   'if exist %(dir)s cd %(dir)s',
-"gmake" :   "test -e %(dir)s && cd %(dir)s"
-}
-
 _MAKE_TEMPLATE_ = {
 "nmake" :   'if exist %(file)s "$(MAKE)" $(MAKE_FLAGS) -f %(file)s',
 "gmake" :   'test -e %(file)s && "$(MAKE)" $(MAKE_FLAGS) -f %(file)s'
 }
 
-_INCLUDE_CMD_ = {
-"nmake" :   '!INCLUDE',
-"gmake" :   "include"
+_INC_FLAG_ = {
+"MSFT" : "/I",
+"GCC" : "-I",
+"INTEL" : "-I",
+"RVCT" : "-I"
 }
 
-_INC_FLAG_ = {"MSFT" : "/I", "GCC" : "-I", "INTEL" : "-I", "RVCT" : "-I"}
-
 ## Constructor of BuildFile
 #
 #   @param  AutoGenObject   Object of AutoGen class
diff --git a/BaseTools/Source/Python/CommonDataClass/DataClass.py 
b/BaseTools/Source/Python/CommonDataClass/DataClass.py
index 31ed46c7ec56..ddf3270ad8a5 100644
--- a/BaseTools/Source/Python/CommonDataClass/DataClass.py
+++ b/BaseTools/Source/Python/CommonDataClass/DataClass.py
@@ -1,7 +1,7 @@
 ## @file
 # This file is used to define class for data structure used in ECC
 #
-# Copyright (c) 2008 - 2014, Intel Corporation. All rights reserved.
+# Copyright (c) 2008 - 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
@@ -290,51 +290,6 @@ class IdentifierClass(object):
 self.EndLine = EndLine
 self.EndColumn = EndColumn
 
-## PcdClass
-#
-# This class defines a structure of a Pcd
-#
-# @param ID:   ID of a Pcd
-# @param CName:CName of a Pcd
-# @param TokenSpaceGuidCName:  TokenSpaceGuidCName of a Pcd
-# @param Token:Token of a Pcd
-# @param DatumType:DatumType of a Pcd
-# @param Model:Model of a Pcd
-# @param BelongsToFile:The Pcd belongs to which file
-# @param BelongsToFunction:The Pcd belongs to which function
-# @param StartLine:StartLine of a Pcd
-# @param StartColumn:  StartColumn of a Pcd
-# @param EndLine:  EndLine of a Pcd
-# @param EndColumn:EndColumn of a Pcd
-#
-# @var ID: ID of a Pcd
-# @var CName:  CName of a Pcd
-# @var TokenSpaceGuidCName:TokenSpaceGuidCName of a Pcd
-# @var Token:  Token of a Pcd
-# @var DatumType:  DatumType of a Pcd
-# @var Model:  Model of a Pcd
-# @var BelongsToFile:  The 

[edk2] [PATCH v1 02/11] BaseTools: Workspace - create a base class

[Jaben Carsey]

refactor 3 classes and create a new base class for their shared functions.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/Workspace/BuildClassObject.py | 140 
+++-
 1 file changed, 50 insertions(+), 90 deletions(-)

diff --git a/BaseTools/Source/Python/Workspace/BuildClassObject.py 
b/BaseTools/Source/Python/Workspace/BuildClassObject.py
index 5f34e8e0bc69..db9518cdff17 100644
--- a/BaseTools/Source/Python/Workspace/BuildClassObject.py
+++ b/BaseTools/Source/Python/Workspace/BuildClassObject.py
@@ -253,6 +253,47 @@ class LibraryClassObject(object):
 if Type is not None:
 self.SupModList = CleanString(Type).split(DataType.TAB_SPACE_SPLIT)
 
+## BuildClassObjectBase
+#
+# This is a base class for classes later in this file. it simplifies by 
+# not requiring duplication of standard functions.
+# This class is not intended for use outside of being a base class.
+#
+class BuildClassObjectBase(object):
+__metaclass__ = ABCMeta
+# prevent this class from being accidentally instantiated
+@abstractmethod
+def __init__(self, *a,**k):
+super(BuildClassObjectBase,self).__init__(*a,**k)
+
+## Convert the class to a string
+#
+#  Convert member MetaFile of the class to a string
+#
+#  @retval string Formatted String
+#
+def __str__(self):
+return str(self.MetaFile)
+
+## Override __eq__ function
+#
+# Check whether ModuleBuildClassObjects are the same
+#
+# @retval False The two ModuleBuildClassObjects are different
+# @retval True  The two ModuleBuildClassObjects are the same
+#
+def __eq__(self, Other):
+return Other and self.MetaFile == Other
+
+## Override __hash__ function
+#
+# Use MetaFile as key in hash table
+#
+# @retval string Key for hash table
+#
+def __hash__(self):
+return hash(self.MetaFile)
+
 ## ModuleBuildClassObject
 #
 # This Class defines ModuleBuildClass
@@ -297,8 +338,9 @@ class LibraryClassObject(object):
 #  { [BuildOptionKey] : BuildOptionValue}
 # @var Depex:  To store value for Depex
 #
-class ModuleBuildClassObject(object):
-def __init__(self):
+class ModuleBuildClassObject(BuildClassObjectBase):
+def __init__(self, *a, **k):
+super(ModuleBuildClassObject,self).__init__(*a,**k)
 self.AutoGenVersion  = 0
 self.MetaFile= ''
 self.BaseName= ''
@@ -330,34 +372,6 @@ class ModuleBuildClassObject(object):
 self.BuildOptions= {}
 self.Depex   = {}
 
-## Convert the class to a string
-#
-#  Convert member MetaFile of the class to a string
-#
-#  @retval string Formatted String
-#
-def __str__(self):
-return str(self.MetaFile)
-
-## Override __eq__ function
-#
-# Check whether ModuleBuildClassObjects are the same
-#
-# @retval False The two ModuleBuildClassObjects are different
-# @retval True  The two ModuleBuildClassObjects are the same
-#
-def __eq__(self, Other):
-return self.MetaFile == Other
-
-## Override __hash__ function
-#
-# Use MetaFile as key in hash table
-#
-# @retval string Key for hash table
-#
-def __hash__(self):
-return hash(self.MetaFile)
-
 ## PackageBuildClassObject
 #
 # This Class defines PackageBuildClass
@@ -381,11 +395,12 @@ class ModuleBuildClassObject(object):
 # @var Pcds:To store value for Pcds, it is a set structure as
 #   { [(PcdCName, PcdGuidCName)] : PcdClassObject}
 #
-class PackageBuildClassObject(object):
+class PackageBuildClassObject(BuildClassObjectBase):
 __metaclass__ = ABCMeta
 # prevent this class from being accidentally instantiated
 @abstractmethod
-def __init__(self):
+def __init__(self, *a, **k):
+super(PackageBuildClassObject,self).__init__(*a,**k)
 self.MetaFile= ''
 self.PackageName = ''
 self.Guid= ''
@@ -398,34 +413,6 @@ class PackageBuildClassObject(object):
 self.LibraryClasses  = {}
 self.Pcds= {}
 
-## Convert the class to a string
-#
-#  Convert member MetaFile of the class to a string
-#
-#  @retval string Formatted String
-#
-def __str__(self):
-return str(self.MetaFile)
-
-## Override __eq__ function
-#
-# Check whether PackageBuildClassObjects are the same
-#
-# @retval False The two PackageBuildClassObjects are different
-# @retval True  The two PackageBuildClassObjects are the same
-#
-def __eq__(self, Other):
-return self.MetaFile == Other
-
-## Override __hash__ 

[edk2] [PATCH v1 00/11] BaseTools refactoring

[Jaben Carsey]

This patch cleans up BaseTools.
On classes that are not instantiated and are deisgned as purely base classes,
they get marked using the abstract base class to prevent instantiation.  This
prevents future errors.
Create a new shared base class to centralize some code that was identical in
multiple classes.
cleanup to pass tuples to startswith and endswith instead of calling multiple
times in a single expression.
when an statement used a fixed list or tuple change to a set to allow
hashing to speed up the operation.  if the order was important then make sure
that is a list.  creating on the fly tuples serves no purpose.
use with statements for file opening and make sure we dont over request file
privilidge.  this also seems to have cleaned up the error about the file
AutoGenTimeStamp which was causing errors previously.
lots of work to elimiate concatenating strings as this is a poor performing
operation since strings are immutable and must be completely reallocated and
moved for each concatenation.

Jaben Carsey (11):
  BaseTools: decorate base classes to prevent instantiation
  BaseTools: Workspace - create a base class
  BaseTools: remove unused code
  BaseTools: remove repeated calls to startswith/endswith
  BaseTools: use set presence instead of series of equality
  BaseTools: refactor section generation
  BaseTools: refactor file opening/writing
  BaseTools: refactor to change object types
  BaseTools: refactor to stop re-allocating strings
  BaseTools: change to set for membership testing
  BaseTools: remove extra assignment

 BaseTools/Source/Python/AutoGen/AutoGen.py  | 248 
++--
 BaseTools/Source/Python/AutoGen/GenC.py |  75 +++---
 BaseTools/Source/Python/AutoGen/GenDepex.py |  42 ++--
 BaseTools/Source/Python/AutoGen/GenMake.py  |  43 ++--
 BaseTools/Source/Python/AutoGen/GenPcdDb.py |  52 ++--
 BaseTools/Source/Python/AutoGen/GenVar.py   |   6 +-
 BaseTools/Source/Python/AutoGen/IdfClassObject.py   |  21 +-
 BaseTools/Source/Python/AutoGen/StrGather.py|  22 +-
 BaseTools/Source/Python/AutoGen/UniClassObject.py   |   8 +-
 BaseTools/Source/Python/AutoGen/ValidCheckingInfoObject.py  |  32 ++-
 BaseTools/Source/Python/BPDG/BPDG.py|   2 +-
 BaseTools/Source/Python/BPDG/GenVpd.py  |  38 ++-
 BaseTools/Source/Python/Common/DataType.py  |  37 ++-
 BaseTools/Source/Python/Common/Expression.py|  68 +++---
 BaseTools/Source/Python/Common/Misc.py  |  77 +++---
 BaseTools/Source/Python/Common/Parsing.py   |   2 +-
 BaseTools/Source/Python/Common/RangeExpression.py   |  10 +-
 BaseTools/Source/Python/Common/String.py|   2 +-
 BaseTools/Source/Python/Common/TargetTxtClassObject.py  |  21 +-
 BaseTools/Source/Python/Common/ToolDefClassObject.py|   4 +-
 BaseTools/Source/Python/Common/VariableAttributes.py|   6 +-
 BaseTools/Source/Python/Common/VpdInfoFile.py   |   4 +-
 BaseTools/Source/Python/CommonDataClass/CommonClass.py  |  29 +--
 BaseTools/Source/Python/CommonDataClass/DataClass.py|  47 +---
 BaseTools/Source/Python/Ecc/Check.py| 123 
+-
 BaseTools/Source/Python/Ecc/CodeFragmentCollector.py|   4 +-
 BaseTools/Source/Python/Ecc/Configuration.py|  10 +-
 BaseTools/Source/Python/Ecc/Database.py |  76 +-
 BaseTools/Source/Python/Ecc/Ecc.py  |   4 +-
 BaseTools/Source/Python/Ecc/MetaDataParser.py   |   4 +-
 BaseTools/Source/Python/Ecc/MetaFileWorkspace/MetaFileParser.py |  54 +++--
 BaseTools/Source/Python/Ecc/c.py|  42 ++--
 BaseTools/Source/Python/Eot/CodeFragmentCollector.py|   9 +-
 BaseTools/Source/Python/Eot/EotGlobalData.py|  14 +-
 BaseTools/Source/Python/Eot/FileProfile.py  |   8 +-
 BaseTools/Source/Python/Eot/Parser.py   |   4 +-
 BaseTools/Source/Python/Eot/Report.py   |  19 +-
 BaseTools/Source/Python/Eot/c.py|   2 +-
 BaseTools/Source/Python/GenFds/Capsule.py   |  26 +-
 BaseTools/Source/Python/GenFds/CapsuleData.py   |  11 +-
 BaseTools/Source/Python/GenFds/CompressSection.py   |  24 +-
 BaseTools/Source/Python/GenFds/DataSection.py   |  32 +--
 BaseTools/Source/Python/GenFds/DepexSection.py  |   8 +-
 BaseTools/Source/Python/GenFds/EfiSection.py| 111 -
 BaseTools/Source/Python/GenFds/FdfParser.py | 195 
---
 

[edk2] [PATCH v1 04/11] BaseTools: remove repeated calls to startswith/endswith

[Jaben Carsey]

As both can take a tuple, use that instead of calling repeatedly.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/AutoGen/GenMake.py  |  2 +-
 BaseTools/Source/Python/AutoGen/StrGather.py|  2 +-
 BaseTools/Source/Python/AutoGen/ValidCheckingInfoObject.py  |  2 +-
 BaseTools/Source/Python/BPDG/GenVpd.py  |  2 +-
 BaseTools/Source/Python/Common/Expression.py| 13 
++---
 BaseTools/Source/Python/Common/Misc.py  |  6 +++---
 BaseTools/Source/Python/Ecc/Check.py|  6 +++---
 BaseTools/Source/Python/Ecc/CodeFragmentCollector.py|  4 ++--
 BaseTools/Source/Python/Ecc/MetaFileWorkspace/MetaFileParser.py |  2 +-
 BaseTools/Source/Python/Ecc/c.py| 14 
+++---
 BaseTools/Source/Python/Eot/CodeFragmentCollector.py|  9 +++--
 BaseTools/Source/Python/GenFds/FdfParser.py | 10 --
 BaseTools/Source/Python/Trim/Trim.py|  2 +-
 BaseTools/Source/Python/Workspace/BuildClassObject.py   |  2 +-
 BaseTools/Source/Python/Workspace/DscBuildData.py   |  2 +-
 BaseTools/Source/Python/Workspace/InfBuildData.py   |  2 +-
 BaseTools/Source/Python/build/BuildReport.py|  2 +-
 17 files changed, 38 insertions(+), 44 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 1c8ab7fe1ec8..d70c5c26ffc8 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -741,7 +741,7 @@ cleanlib:
 index = index + 1
 if CmdName == 'Trim':
 
SecDepsFileList.append(os.path.join('$(DEBUG_DIR)', 
os.path.basename(OutputFile).replace('offset', 'efi')))
-if OutputFile.endswith('.ui') or 
OutputFile.endswith('.ver'):
+if OutputFile.endswith(('.ui','.ver')):
 
SecDepsFileList.append(os.path.join('$(MODULE_DIR)','$(MODULE_FILE)'))
 self.FfsOutputFileList.append((OutputFile, ' 
'.join(SecDepsFileList), SecCmdStr))
 if len(SecDepsFileList) > 0:
diff --git a/BaseTools/Source/Python/AutoGen/StrGather.py 
b/BaseTools/Source/Python/AutoGen/StrGather.py
index 73af1214eb0a..e5e4f25efd5d 100644
--- a/BaseTools/Source/Python/AutoGen/StrGather.py
+++ b/BaseTools/Source/Python/AutoGen/StrGather.py
@@ -290,7 +290,7 @@ def GetFilteredLanguage(UniLanguageList, 
LanguageFilterList):
 if DefaultTag not in UniLanguageListFiltered:
 # check whether language code with primary code equivalent 
with DefaultTag already in the list, if so, use that
 for UniLanguage in UniLanguageList:
-if UniLanguage.startswith('en-') or 
UniLanguage.startswith('eng-'):
+if UniLanguage.startswith(('eng-','en-')):
 if UniLanguage not in UniLanguageListFiltered:
 UniLanguageListFiltered += [UniLanguage]
 break
diff --git a/BaseTools/Source/Python/AutoGen/ValidCheckingInfoObject.py 
b/BaseTools/Source/Python/AutoGen/ValidCheckingInfoObject.py
index e2b4795129ef..3b54865000bf 100644
--- a/BaseTools/Source/Python/AutoGen/ValidCheckingInfoObject.py
+++ b/BaseTools/Source/Python/AutoGen/ValidCheckingInfoObject.py
@@ -254,7 +254,7 @@ class VAR_CHECK_PCD_VALID_LIST(VAR_CHECK_PCD_VALID_OBJ):
 for valid_num in valid_num_list:
 valid_num = valid_num.strip()
 
-if valid_num.startswith('0x') or valid_num.startswith('0X'):
+if valid_num.startswith(('0x','0X')):
 self.data.add(int(valid_num, 16))
 else:
 self.data.add(int(valid_num))
diff --git a/BaseTools/Source/Python/BPDG/GenVpd.py 
b/BaseTools/Source/Python/BPDG/GenVpd.py
index 69a9665f5a76..4fa12b7d59de 100644
--- a/BaseTools/Source/Python/BPDG/GenVpd.py
+++ b/BaseTools/Source/Python/BPDG/GenVpd.py
@@ -172,7 +172,7 @@ class PcdEntry:
 #  @param ValueString The Integer type string for pack.
 #   
 def _PackPtrValue(self, ValueString, Size):
-if ValueString.startswith('L"') or ValueString.startswith("L'"):
+if ValueString.startswith(("L'",'L"')):
 self._PackUnicode(ValueString, Size)
 elif ValueString.startswith('{') and ValueString.endswith('}'):
 self._PackByteArray(ValueString, Size)
diff --git a/BaseTools/Source/Python/Common/Expression.py 
b/BaseTools/Source/Python/Common/Expression.py
index 9fa07c6add16..e5d17e6b4de0 100644

[edk2] [PATCH v1 05/11] BaseTools: use set presence instead of series of equality

[Jaben Carsey]

Instead of testing each equality individually, just make a set and test once.

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

diff --git a/BaseTools/Source/Python/Ecc/Configuration.py 
b/BaseTools/Source/Python/Ecc/Configuration.py
index fee7ecb9703d..d37ce8e19e67 100644
--- a/BaseTools/Source/Python/Ecc/Configuration.py
+++ b/BaseTools/Source/Python/Ecc/Configuration.py
@@ -404,17 +404,9 @@ class Configuration(object):
 ErrorMsg = "Invalid configuration option '%s' was found" % 
List[0]
 EdkLogger.error("Ecc", EdkLogger.ECC_ERROR, ErrorMsg, File 
= Filepath, Line = LineNo)
 assert _ConfigFileToInternalTranslation[List[0]] in 
self.__dict__
-if List[0] == 'ModifierList':
-List[1] = GetSplitValueList(List[1], TAB_COMMA_SPLIT)
 if List[0] == 'MetaDataFileCheckPathOfGenerateFileList' and 
List[1] == "":
 continue
-if List[0] == 'SkipDirList':
-List[1] = GetSplitValueList(List[1], TAB_COMMA_SPLIT)
-if List[0] == 'SkipFileList':
-List[1] = GetSplitValueList(List[1], TAB_COMMA_SPLIT)
-if List[0] == 'BinaryExtList':
-List[1] = GetSplitValueList(List[1], TAB_COMMA_SPLIT)
-if List[0] == 'Copyright':
+if List[0] in 
{'ModifierList','SkipDirList','SkipFileList','BinaryExtList','Copyright'}:
 List[1] = GetSplitValueList(List[1], TAB_COMMA_SPLIT)
 self.__dict__[_ConfigFileToInternalTranslation[List[0]]] = 
List[1]
 
-- 
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] Enable using device address when programming BARs

[Laszlo Ersek]

Hi Roman,

On 05/14/18 19:35, Ard Biesheuvel wrote:
> On 14 May 2018 at 19:28, Roman Bacik  wrote:
>> Ard,
>>
>> Thank you very much for your comment.
>>
>>> -Original Message-
>>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>>> Sent: Sunday, May 13, 2018 3:25 AM
>>> To: Roman Bacik
>>> Cc: edk2-devel@lists.01.org; Ruiyu Ni; Vladimir Olovyannikov
>>> Subject: Re: [edk2] [PATCH] Enable using device address when
>>> programming BARs
>>>
>>> On 9 May 2018 at 22:17, Roman Bacik 
>>> wrote:
 I will upload v2 with the corrected subject - add package name
 MdeModulePkg/Bus .

>>>
>>> I don't think this is the correct approach. Please use the address
>>> translation
>>> support that has been added recently to PciHostBridgeDxe and
>>> PciHostBridgeLib.
>>>
>>
>> Would you like to see this change:
>>
>>   Address = Base + Node->Offset;
>> + if (UseDeviceAddress)
>> +  Address = TO_DEVICE_ADDRESS(Address, -Base);
>>
>> Instead of:
>>
>> -  Address = Base + Node->Offset;
>> +  Address = UseDeviceAddress? Node->Offset: Base + Node->Offset;
>>
>
> No.
>
> Programming BARs should always involve device addresses, never CPU
> addresses. If you wire up the MMIO translation support correctly, the
> existing code will already do what you want.

To clarify a bit, please look at the following commits:

  1  5bb1866e5383 MdeModulePkg/PciHostBridgeLib.h: add address Translation
  2  74d0a339b832 MdeModulePkg/PciHostBridgeDxe: Add support for address 
translation
  3  c03860d05233 MdeModulePkg/PciBus: convert host address to device address
  4  dc080d3b61e5 MdeModulePkg/PciBus: return CPU address for GetBarAttributes

in particular, as Ard's first response suggests, the new field
introduced in commit 5bb1866e5383. The PciHostBridgeLib instance that
your platform plugs into MdeModulePkg/PciHostBridgeDxe should populate
this field correctly. Then PciHostBridgeDxe and PciBusDxe will do the
right thing for you.

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

Re: [edk2] [PATCH] Enable using device address when programming BARs

[Roman Bacik]

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Monday, May 14, 2018 10:35 AM
> To: Roman Bacik
> Cc: edk2-devel@lists.01.org; Ruiyu Ni; Vladimir Olovyannikov
> Subject: Re: [edk2] [PATCH] Enable using device address when programming
> BARs
>
> On 14 May 2018 at 19:28, Roman Bacik  wrote:
> > Ard,
> >
> > Thank you very much for your comment.
> >
> >> -Original Message-
> >> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> >> Sent: Sunday, May 13, 2018 3:25 AM
> >> To: Roman Bacik
> >> Cc: edk2-devel@lists.01.org; Ruiyu Ni; Vladimir Olovyannikov
> >> Subject: Re: [edk2] [PATCH] Enable using device address when
> >> programming BARs
> >>
> >> On 9 May 2018 at 22:17, Roman Bacik 
> wrote:
> >> > I will upload v2 with the corrected subject - add package name
> >> > MdeModulePkg/Bus .
> >> >
> >>
> >> I don't think this is the correct approach. Please use the address
> >> translation support that has been added recently to PciHostBridgeDxe
> >> and PciHostBridgeLib.
> >>
> >
> > Would you like to see this change:
> >
> >   Address = Base + Node->Offset;
> > + if (UseDeviceAddress)
> > +  Address = TO_DEVICE_ADDRESS(Address, -Base);
> >
> > Instead of:
> >
> > -  Address = Base + Node->Offset;
> > +  Address = UseDeviceAddress? Node->Offset: Base + Node->Offset;
> >
>
> No.
>
> Programming BARs should always involve device addresses, never CPU
> addresses. If you wire up the MMIO translation support correctly, the
> existing code will already do what you want.
>

Our code has base address 0x6000 and offset 0x200. We need Address=0x200
to be passed to PciIo->Pci.Write() and Address=0x6200 to be stored in
Node->PciDev->PciBar[Node->Bar].BaseAddress. The code as is does not seem to
do that unless PciIo->Pci.Write() itself changes the content of the Address
from 0x200 to 0x6200 for us.

So if we modify MMIO and the existing code gets the correct Address 0x200
being passed to PciIo->Pci.Write() then it means Address = 0x200 will be
stored into Node->PciDev->PciBar[Node->Bar].BaseAddress, which would be
wrong in our case. It appears that regardless how we change MMIO translation
the existing code would not work for us as is. Or I may be missing
something.

> >> >
> >> >
> >> > *From:* Roman Bacik [mailto:roman.ba...@broadcom.com]
> >> > *Sent:* Thursday, May 3, 2018 3:55 PM
> >> > *To:* edk2-devel@lists.01.org
> >> > *Cc:* Ruiyu Ni; Vladimir Olovyannikov
> >> > *Subject:* [edk2] [PATCH] Enable using device address when
> >> > programming BARs
> >> >
> >> >
> >> >
> >> > Some SoCs require to use device address when BARs are programmed:
> >> > https://bugzilla.tianocore.org/show_bug.cgi?id=948
> >> >
> >> > Cc: Ruiyu Ni 
> >> > Cc: Vladimir Olovyannikov 
> >> > Contributed-under: TianoCore Contribution Agreement 1.1
> >> > Signed-off-by: Roman Bacik 
> >> > ---
> >> >  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf| 1 +
> >> >  MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c | 8 +---
> >> >  MdeModulePkg/MdeModulePkg.dec   | 3 +++
> >> >  MdeModulePkg/MdeModulePkg.dsc   | 1 +
> >> >  4 files changed, 10 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> >> > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> >> > index 97608bfcf245..1368e5068574 100644
> >> > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> >> > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> >> > @@ -110,6 +110,7 @@
> >> >gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport  ##
> >> CONSUMES
> >> >gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport##
> >> CONSUMES
> >> >gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
> ##
> >> > SOMETIMES_CONSUMES
> >> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdUseDeviceAddress##
> >> CONSUMES
> >> >
> >> >  [UserExtensions.TianoCore."ExtraFiles"]
> >> >PciBusDxeExtra.uni
> >> > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
> >> > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
> >> > index 2f713fcee95e..a23bd1e258ef 100644
> >> > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
> >> > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
> >> > @@ -1269,6 +1269,7 @@ ProgramBar (
> >> >EFI_PCI_IO_PROTOCOL *PciIo;
> >> >UINT64  Address;
> >> >UINT32  Address32;
> >> > +  BOOLEAN UseDeviceAddress;
> >> >
> >> >ASSERT (Node->Bar < PCI_MAX_BAR);
> >> >
> >> > @@ -1282,8 +1283,9 @@ ProgramBar (
> >> >
> >> >Address = 0;
> >> >PciIo   = &(Node->PciDev->PciIo);
> >> > +  UseDeviceAddress = FeaturePcdGet (PcdUseDeviceAddress);
> >> >
> >> > -  Address = Base + Node->Offset;
> >> > +  Address = UseDeviceAddress? Node->Offset: Base + Node->Offset;
> >> >
> >> >

Re: [edk2] [PATCH] Enable using device address when programming BARs

[Ard Biesheuvel]

On 14 May 2018 at 19:28, Roman Bacik  wrote:
> Ard,
>
> Thank you very much for your comment.
>
>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Sunday, May 13, 2018 3:25 AM
>> To: Roman Bacik
>> Cc: edk2-devel@lists.01.org; Ruiyu Ni; Vladimir Olovyannikov
>> Subject: Re: [edk2] [PATCH] Enable using device address when programming
>> BARs
>>
>> On 9 May 2018 at 22:17, Roman Bacik  wrote:
>> > I will upload v2 with the corrected subject - add package name
>> > MdeModulePkg/Bus .
>> >
>>
>> I don't think this is the correct approach. Please use the address
>> translation
>> support that has been added recently to PciHostBridgeDxe and
>> PciHostBridgeLib.
>>
>
> Would you like to see this change:
>
>   Address = Base + Node->Offset;
> + if (UseDeviceAddress)
> +  Address = TO_DEVICE_ADDRESS(Address, -Base);
>
> Instead of:
>
> -  Address = Base + Node->Offset;
> +  Address = UseDeviceAddress? Node->Offset: Base + Node->Offset;
>

No.

Programming BARs should always involve device addresses, never CPU
addresses. If you wire up the MMIO translation support correctly, the
existing code will already do what you want.

>> >
>> >
>> > *From:* Roman Bacik [mailto:roman.ba...@broadcom.com]
>> > *Sent:* Thursday, May 3, 2018 3:55 PM
>> > *To:* edk2-devel@lists.01.org
>> > *Cc:* Ruiyu Ni; Vladimir Olovyannikov
>> > *Subject:* [edk2] [PATCH] Enable using device address when programming
>> > BARs
>> >
>> >
>> >
>> > Some SoCs require to use device address when BARs are programmed:
>> > https://bugzilla.tianocore.org/show_bug.cgi?id=948
>> >
>> > Cc: Ruiyu Ni 
>> > Cc: Vladimir Olovyannikov 
>> > Contributed-under: TianoCore Contribution Agreement 1.1
>> > Signed-off-by: Roman Bacik 
>> > ---
>> >  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf| 1 +
>> >  MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c | 8 +---
>> >  MdeModulePkg/MdeModulePkg.dec   | 3 +++
>> >  MdeModulePkg/MdeModulePkg.dsc   | 1 +
>> >  4 files changed, 10 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
>> > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
>> > index 97608bfcf245..1368e5068574 100644
>> > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
>> > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
>> > @@ -110,6 +110,7 @@
>> >gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport  ##
>> CONSUMES
>> >gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport##
>> CONSUMES
>> >gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration##
>> > SOMETIMES_CONSUMES
>> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdUseDeviceAddress##
>> CONSUMES
>> >
>> >  [UserExtensions.TianoCore."ExtraFiles"]
>> >PciBusDxeExtra.uni
>> > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
>> > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
>> > index 2f713fcee95e..a23bd1e258ef 100644
>> > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
>> > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
>> > @@ -1269,6 +1269,7 @@ ProgramBar (
>> >EFI_PCI_IO_PROTOCOL *PciIo;
>> >UINT64  Address;
>> >UINT32  Address32;
>> > +  BOOLEAN UseDeviceAddress;
>> >
>> >ASSERT (Node->Bar < PCI_MAX_BAR);
>> >
>> > @@ -1282,8 +1283,9 @@ ProgramBar (
>> >
>> >Address = 0;
>> >PciIo   = &(Node->PciDev->PciIo);
>> > +  UseDeviceAddress = FeaturePcdGet (PcdUseDeviceAddress);
>> >
>> > -  Address = Base + Node->Offset;
>> > +  Address = UseDeviceAddress? Node->Offset: Base + Node->Offset;
>> >
>> >//
>> >// Indicate pci bus driver has allocated @@ -1308,7 +1310,7 @@
>> > ProgramBar (
>> >   
>> >   );
>> >
>> > -Node->PciDev->PciBar[Node->Bar].BaseAddress = Address;
>> > +Node->PciDev->PciBar[Node->Bar].BaseAddress = UseDeviceAddress?
>> > + Base +
>> > Address: Address;
>> >
>> >  break;
>> >
>> > @@ -1335,7 +1337,7 @@ ProgramBar (
>> >   
>> >   );
>> >
>> > -Node->PciDev->PciBar[Node->Bar].BaseAddress = Address;
>> > +Node->PciDev->PciBar[Node->Bar].BaseAddress = UseDeviceAddress?
>> > + Base +
>> > Address: Address;
>> >
>> >  break;
>> >
>> > diff --git a/MdeModulePkg/MdeModulePkg.dec
>> > b/MdeModulePkg/MdeModulePkg.dec index cc397185f7b9..58425ee0d57f
>> > 100644
>> > --- a/MdeModulePkg/MdeModulePkg.dec
>> > +++ b/MdeModulePkg/MdeModulePkg.dec
>> > @@ -1005,6 +1005,9 @@
>> ># @Prompt Enable UEFI Stack Guard.
>> >
>> >
>> gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0
>> x300010
>> > 55
>> >
>> > +  ## Indicates whether the device address should be used for BAR
>> > programming
>> > +
>> >
>> >
>> gEfiMdeModulePkgTokenSpaceGuid.PcdUseDeviceAddress|FALSE|BOOLEA
>> 

Re: [edk2] [PATCH] Enable using device address when programming BARs

[Roman Bacik]

Ard,

Thank you very much for your comment.

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Sunday, May 13, 2018 3:25 AM
> To: Roman Bacik
> Cc: edk2-devel@lists.01.org; Ruiyu Ni; Vladimir Olovyannikov
> Subject: Re: [edk2] [PATCH] Enable using device address when programming
> BARs
>
> On 9 May 2018 at 22:17, Roman Bacik  wrote:
> > I will upload v2 with the corrected subject - add package name
> > MdeModulePkg/Bus .
> >
>
> I don't think this is the correct approach. Please use the address
> translation
> support that has been added recently to PciHostBridgeDxe and
> PciHostBridgeLib.
>

Would you like to see this change:

  Address = Base + Node->Offset;
+ if (UseDeviceAddress)
+  Address = TO_DEVICE_ADDRESS(Address, -Base);

Instead of:

-  Address = Base + Node->Offset;
+  Address = UseDeviceAddress? Node->Offset: Base + Node->Offset;

> >
> >
> > *From:* Roman Bacik [mailto:roman.ba...@broadcom.com]
> > *Sent:* Thursday, May 3, 2018 3:55 PM
> > *To:* edk2-devel@lists.01.org
> > *Cc:* Ruiyu Ni; Vladimir Olovyannikov
> > *Subject:* [edk2] [PATCH] Enable using device address when programming
> > BARs
> >
> >
> >
> > Some SoCs require to use device address when BARs are programmed:
> > https://bugzilla.tianocore.org/show_bug.cgi?id=948
> >
> > Cc: Ruiyu Ni 
> > Cc: Vladimir Olovyannikov 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Roman Bacik 
> > ---
> >  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf| 1 +
> >  MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c | 8 +---
> >  MdeModulePkg/MdeModulePkg.dec   | 3 +++
> >  MdeModulePkg/MdeModulePkg.dsc   | 1 +
> >  4 files changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> > index 97608bfcf245..1368e5068574 100644
> > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> > @@ -110,6 +110,7 @@
> >gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport  ##
> CONSUMES
> >gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport##
> CONSUMES
> >gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration##
> > SOMETIMES_CONSUMES
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdUseDeviceAddress##
> CONSUMES
> >
> >  [UserExtensions.TianoCore."ExtraFiles"]
> >PciBusDxeExtra.uni
> > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
> > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
> > index 2f713fcee95e..a23bd1e258ef 100644
> > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
> > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
> > @@ -1269,6 +1269,7 @@ ProgramBar (
> >EFI_PCI_IO_PROTOCOL *PciIo;
> >UINT64  Address;
> >UINT32  Address32;
> > +  BOOLEAN UseDeviceAddress;
> >
> >ASSERT (Node->Bar < PCI_MAX_BAR);
> >
> > @@ -1282,8 +1283,9 @@ ProgramBar (
> >
> >Address = 0;
> >PciIo   = &(Node->PciDev->PciIo);
> > +  UseDeviceAddress = FeaturePcdGet (PcdUseDeviceAddress);
> >
> > -  Address = Base + Node->Offset;
> > +  Address = UseDeviceAddress? Node->Offset: Base + Node->Offset;
> >
> >//
> >// Indicate pci bus driver has allocated @@ -1308,7 +1310,7 @@
> > ProgramBar (
> >   
> >   );
> >
> > -Node->PciDev->PciBar[Node->Bar].BaseAddress = Address;
> > +Node->PciDev->PciBar[Node->Bar].BaseAddress = UseDeviceAddress?
> > + Base +
> > Address: Address;
> >
> >  break;
> >
> > @@ -1335,7 +1337,7 @@ ProgramBar (
> >   
> >   );
> >
> > -Node->PciDev->PciBar[Node->Bar].BaseAddress = Address;
> > +Node->PciDev->PciBar[Node->Bar].BaseAddress = UseDeviceAddress?
> > + Base +
> > Address: Address;
> >
> >  break;
> >
> > diff --git a/MdeModulePkg/MdeModulePkg.dec
> > b/MdeModulePkg/MdeModulePkg.dec index cc397185f7b9..58425ee0d57f
> > 100644
> > --- a/MdeModulePkg/MdeModulePkg.dec
> > +++ b/MdeModulePkg/MdeModulePkg.dec
> > @@ -1005,6 +1005,9 @@
> ># @Prompt Enable UEFI Stack Guard.
> >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0
> x300010
> > 55
> >
> > +  ## Indicates whether the device address should be used for BAR
> > programming
> > +
> >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdUseDeviceAddress|FALSE|BOOLEA
> N|0x300
> > 01056
> > +
> >  [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.dsc
> > b/MdeModulePkg/MdeModulePkg.dsc index
> ec24a50c7d0a..39b397cb13d9
> > 100644
> > --- 

Re: [edk2] [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library

[Laszlo Ersek]

On 05/14/18 17:06, Gao, Liming wrote:
> Laszlo:
>   I have not fully understood its goal and design. Could you give me one more 
> week to review it?

Sure! Thank you.

Laszlo

>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Monday, May 14, 2018 9:16 PM
>> To: edk2-devel-01 
>> Cc: Kinney, Michael D ; Justen, Jordan L 
>> ; Gao, Liming
>> ; Ard Biesheuvel 
>> Subject: Re: [edk2] [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use 
>> PCI(E) Capabilities Library
>>
>> ping
>>
>> On 05/04/18 23:36, Laszlo Ersek wrote:
>>> Repo:   https://github.com/lersek/edk2.git
>>> Branch: pci_cap
>>>
>>> In message [1], Jordan suggested a general "capabilities helper lib that
>>> could iterate & read the PCI capability structs". Patch #1 introduces
>>> that library (class and central BASE instance) to MdePkg; the library
>>> class is called PciCapLib.
>>>
>>> [1] 
>>> http://mid.mail-archive.com/150696619010.2454.9266470245604846575@jljusten-skl
>>>
>>> PciCapLib offers APIs to parse capabilities lists, and to locate,
>>> describe, read and write capabilities.
>>>
>>> The library class targets a wide range of client module types; for
>>> example, platform PEIMs, platform DXE drivers, and UEFI drivers that
>>> follow the UEFI Driver Model. For this reason, the library class is
>>> designed with PCI config access abstracted away, even beyond the PPI /
>>> Protocol level, since those cannot cross the PEI / DXE boundary.
>>>
>>> This (minimal) config space access method abstraction is called
>>> PCI_CAP_DEV. It is *conceptually* a protocol. Client modules plug
>>> PCI_CAP_DEV instances into PciCapLib for config space access, similarly
>>> to how DiskIo and BlockIo instances are plugged into filesystem drivers.
>>> Similarly to the filesystem driver example, PciCapLib is an example for
>>> the "strategy pattern".
>>>
>>> Patches #2 and #3 introduce small "factory" libraries that help client
>>> modules produce PCI_CAP_DEV instances:
>>>
>>> - PciCapPciSegmentLib -- incl. class and central BASE instance -- from
>>>   patch #2 produces PCI_CAP_DEV instances on top of PciSegmentLib, using
>>>   the "Segment:Bus:Device.Function" notation. This is meant for platform
>>>   modules (PEIMs and DXE drivers), which generally have a working
>>>   PciSegmentLib instance available.
>>>
>>> - PciCapPciIoLib -- incl. class and central UEFI instance -- from patch
>>>   #3 lets client modules produce PCI_CAP_DEV instances on top of
>>>   EFI_PCI_IO_PROTOCOL instances. This is meant for UEFI drivers.
>>>
>>> Patches #4 and #5 resolve the three new lib classes to their respective
>>> central instances in the OvmfPkg and ArmVirtPkg DSC files.
>>>
>>> Patch #6 converts OvmfPkg/PciHotPlugInitDxe to the new APIs, from manual
>>> capability list parsing. Because PciHotPlugInitDxe is a platform DXE
>>> driver, this conversion uses PciCapPciSegmentLib and PciCapLib.
>>>
>>> Patch #7 converts OvmfPkg/Virtio10Dxe to the new APIs, from manual
>>> capability list parsing. Because Virtio10Dxe is a UEFI driver, this
>>> conversion uses PciCapPciIoLib and PciCapLib.
>>>
>>> [ jump to the patches now, and return if you have questions: I
>>>   have answers, but you might find them too verbose in advance ]
>>>
>>> Q1: If PCI config access needs to be abstracted from PciCapLib, then why
>>> don't we provide multiple instances of PciCapLib? In other words,
>>> why the PCI_CAP_DEV "protocol" rather than multiple instances of
>>> PciCapLib?
>>>
>>> A1: The point of lib classes and instances is that the class provides a
>>> common interface, and the instances provide different
>>> implementations. We have the opposite use case here (hence the
>>> "strategy pattern"): the implementation of the higher level
>>> algorithm is common, and the objects that plug into it differ at the
>>> *interface* level ("Segment:Bus:Device.Function" notation versus
>>> PciIo protocol).
>>>
>>> None of those notations are suitable for inclusion in the PciCapLib
>>> *class*; a more abstract interface is needed for identifying the PCI
>>> device to the library, for parsing of capabilities lists.
>>>
>>> Q2: OK, let's say we can't (and shouldn't) include both notations in the
>>> PciCapLib class; can we settle on one of them, rather than
>>> PCI_CAP_DEV?
>>>
>>> A2: No, we can't. PciIo is obviously unusable in PEIMs (and in quite a
>>> few platform DXE drivers too).
>>>
>>> It takes a bit longer to explain why the other choice, i.e. adopting
>>> the "Segment:Bus:Device.Function" notation for the PciCapLib
>>> interface, is not good either. It has to do with the portability of
>>> UEFI drivers that bind PciIo protocol instances:
>>>
>>> It is indeed easy enough to call PciIo->GetLocation() and plug the
>>> output values into a 

Re: [edk2] [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library

[Gao, Liming]

Laszlo:
  I have not fully understood its goal and design. Could you give me one more 
week to review it?

Thanks
Liming
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Monday, May 14, 2018 9:16 PM
> To: edk2-devel-01 
> Cc: Kinney, Michael D ; Justen, Jordan L 
> ; Gao, Liming
> ; Ard Biesheuvel 
> Subject: Re: [edk2] [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use 
> PCI(E) Capabilities Library
> 
> ping
> 
> On 05/04/18 23:36, Laszlo Ersek wrote:
> > Repo:   https://github.com/lersek/edk2.git
> > Branch: pci_cap
> >
> > In message [1], Jordan suggested a general "capabilities helper lib that
> > could iterate & read the PCI capability structs". Patch #1 introduces
> > that library (class and central BASE instance) to MdePkg; the library
> > class is called PciCapLib.
> >
> > [1] 
> > http://mid.mail-archive.com/150696619010.2454.9266470245604846575@jljusten-skl
> >
> > PciCapLib offers APIs to parse capabilities lists, and to locate,
> > describe, read and write capabilities.
> >
> > The library class targets a wide range of client module types; for
> > example, platform PEIMs, platform DXE drivers, and UEFI drivers that
> > follow the UEFI Driver Model. For this reason, the library class is
> > designed with PCI config access abstracted away, even beyond the PPI /
> > Protocol level, since those cannot cross the PEI / DXE boundary.
> >
> > This (minimal) config space access method abstraction is called
> > PCI_CAP_DEV. It is *conceptually* a protocol. Client modules plug
> > PCI_CAP_DEV instances into PciCapLib for config space access, similarly
> > to how DiskIo and BlockIo instances are plugged into filesystem drivers.
> > Similarly to the filesystem driver example, PciCapLib is an example for
> > the "strategy pattern".
> >
> > Patches #2 and #3 introduce small "factory" libraries that help client
> > modules produce PCI_CAP_DEV instances:
> >
> > - PciCapPciSegmentLib -- incl. class and central BASE instance -- from
> >   patch #2 produces PCI_CAP_DEV instances on top of PciSegmentLib, using
> >   the "Segment:Bus:Device.Function" notation. This is meant for platform
> >   modules (PEIMs and DXE drivers), which generally have a working
> >   PciSegmentLib instance available.
> >
> > - PciCapPciIoLib -- incl. class and central UEFI instance -- from patch
> >   #3 lets client modules produce PCI_CAP_DEV instances on top of
> >   EFI_PCI_IO_PROTOCOL instances. This is meant for UEFI drivers.
> >
> > Patches #4 and #5 resolve the three new lib classes to their respective
> > central instances in the OvmfPkg and ArmVirtPkg DSC files.
> >
> > Patch #6 converts OvmfPkg/PciHotPlugInitDxe to the new APIs, from manual
> > capability list parsing. Because PciHotPlugInitDxe is a platform DXE
> > driver, this conversion uses PciCapPciSegmentLib and PciCapLib.
> >
> > Patch #7 converts OvmfPkg/Virtio10Dxe to the new APIs, from manual
> > capability list parsing. Because Virtio10Dxe is a UEFI driver, this
> > conversion uses PciCapPciIoLib and PciCapLib.
> >
> > [ jump to the patches now, and return if you have questions: I
> >   have answers, but you might find them too verbose in advance ]
> >
> > Q1: If PCI config access needs to be abstracted from PciCapLib, then why
> > don't we provide multiple instances of PciCapLib? In other words,
> > why the PCI_CAP_DEV "protocol" rather than multiple instances of
> > PciCapLib?
> >
> > A1: The point of lib classes and instances is that the class provides a
> > common interface, and the instances provide different
> > implementations. We have the opposite use case here (hence the
> > "strategy pattern"): the implementation of the higher level
> > algorithm is common, and the objects that plug into it differ at the
> > *interface* level ("Segment:Bus:Device.Function" notation versus
> > PciIo protocol).
> >
> > None of those notations are suitable for inclusion in the PciCapLib
> > *class*; a more abstract interface is needed for identifying the PCI
> > device to the library, for parsing of capabilities lists.
> >
> > Q2: OK, let's say we can't (and shouldn't) include both notations in the
> > PciCapLib class; can we settle on one of them, rather than
> > PCI_CAP_DEV?
> >
> > A2: No, we can't. PciIo is obviously unusable in PEIMs (and in quite a
> > few platform DXE drivers too).
> >
> > It takes a bit longer to explain why the other choice, i.e. adopting
> > the "Segment:Bus:Device.Function" notation for the PciCapLib
> > interface, is not good either. It has to do with the portability of
> > UEFI drivers that bind PciIo protocol instances:
> >
> > It is indeed easy enough to call PciIo->GetLocation() and plug the
> > output values into a theoretical "Segment:Bus:Device.Function"
> > interface. However, 

Re: [edk2] [Patch] BaseTools: Enhance error message when file is not exist for Gensec

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Yonghong Zhu
> Sent: Sunday, May 13, 2018 5:46 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch] BaseTools: Enhance error message when file is not
> exist for Gensec
> 
> When the file is not exist in workspace or packages path, current
> Gensec tool doesn't report exactly error message.
> 
> FILE FV_IMAGE = -4CF1-42D8-A0C3-B3F60779dF4D  {
>   SECTION GUIDED A7717414-C616-4977-9420-844712A735BF {
> SECTION FV_IMAGE = TestPkg/Test.fd
>   }
> }
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Source/Python/GenFds/FvImageSection.py | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/BaseTools/Source/Python/GenFds/FvImageSection.py
> b/BaseTools/Source/Python/GenFds/FvImageSection.py
> index 57ecea0..3a4d8fb 100644
> --- a/BaseTools/Source/Python/GenFds/FvImageSection.py
> +++ b/BaseTools/Source/Python/GenFds/FvImageSection.py
> @@ -131,10 +131,16 @@ class FvImageSection(FvImageSectionClassObject):
>  self.Alignment = str (FvAlignmentValue / 
> 0x400) + "K"
>  else:
>  # FvAlignmentValue is less than 1K
>  self.Alignment = str (FvAlignmentValue)
>  FvFileObj.close()
> +else:
> +if len (mws.getPkgPath()) == 0:
> +EdkLogger.error("GenFds", FILE_NOT_FOUND, "%s is 
> not
> found in WORKSPACE: %s" % self.FvFileName,
> GenFdsGlobalVariable.WorkSpaceDir)
> +else:
> +EdkLogger.error("GenFds", FILE_NOT_FOUND, "%s is 
> not
> found in packages path:\n\t%s" % (self.FvFileName,
> '\n\t'.join(mws.getPkgPath(
> +
>  else:
>  EdkLogger.error("GenFds", GENFDS_ERROR, "FvImageSection
> Failed! %s NOT found in FDF" % self.FvName)
> 
>  #
>  # Prepare the parameter of GenSection
> --
> 2.6.1.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] OvmfPkg/PlatformBootManagerLib: connect consoles unconditionally

[Laszlo Ersek]

On 05/14/18 12:28, Ard Biesheuvel wrote:
> On 13 May 2018 at 00:55, Laszlo Ersek  wrote:
>> If both ConIn and ConOut exist, but ConIn references none of the PS/2
>> keyboard, the USB wild-card keyboard, and any serial ports, then
>> PlatformInitializeConsole() currently allows the boot to proceed without
>> any input devices at all. This makes for a bad user experience -- the
>> firmware menu could only be entered through OsIndications, set by a guest
>> OS.
>>
>> Do what ArmVirtQemu does already, namely connect the consoles, and add
>> them to ConIn / ConOut / ErrOut, unconditionally. (The underlying
>> EfiBootManagerUpdateConsoleVariable() function checks for duplicates.)
>>
>> The issue used to be masked by the EfiBootManagerConnectAll() call that
>> got conditionalized in commit 245c643cc8b7.
>>
>> This patch is best viewed with "git show -b -W".
>>
>> Cc: Ard Biesheuvel 
>> Cc: Jordan Justen 
>> Fixes: 245c643cc8b73240c3b88cb55b2911b285a8c10d
>> Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1577546
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek 
> 
> Reviewed-by: Ard Biesheuvel 

Thank you (and welcome back :) ), commit f803c03cc2e0.

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

Re: [edk2] [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library

[Laszlo Ersek]

ping

On 05/04/18 23:36, Laszlo Ersek wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: pci_cap
> 
> In message [1], Jordan suggested a general "capabilities helper lib that
> could iterate & read the PCI capability structs". Patch #1 introduces
> that library (class and central BASE instance) to MdePkg; the library
> class is called PciCapLib.
> 
> [1] 
> http://mid.mail-archive.com/150696619010.2454.9266470245604846575@jljusten-skl
> 
> PciCapLib offers APIs to parse capabilities lists, and to locate,
> describe, read and write capabilities.
> 
> The library class targets a wide range of client module types; for
> example, platform PEIMs, platform DXE drivers, and UEFI drivers that
> follow the UEFI Driver Model. For this reason, the library class is
> designed with PCI config access abstracted away, even beyond the PPI /
> Protocol level, since those cannot cross the PEI / DXE boundary.
> 
> This (minimal) config space access method abstraction is called
> PCI_CAP_DEV. It is *conceptually* a protocol. Client modules plug
> PCI_CAP_DEV instances into PciCapLib for config space access, similarly
> to how DiskIo and BlockIo instances are plugged into filesystem drivers.
> Similarly to the filesystem driver example, PciCapLib is an example for
> the "strategy pattern".
> 
> Patches #2 and #3 introduce small "factory" libraries that help client
> modules produce PCI_CAP_DEV instances:
> 
> - PciCapPciSegmentLib -- incl. class and central BASE instance -- from
>   patch #2 produces PCI_CAP_DEV instances on top of PciSegmentLib, using
>   the "Segment:Bus:Device.Function" notation. This is meant for platform
>   modules (PEIMs and DXE drivers), which generally have a working
>   PciSegmentLib instance available.
> 
> - PciCapPciIoLib -- incl. class and central UEFI instance -- from patch
>   #3 lets client modules produce PCI_CAP_DEV instances on top of
>   EFI_PCI_IO_PROTOCOL instances. This is meant for UEFI drivers.
> 
> Patches #4 and #5 resolve the three new lib classes to their respective
> central instances in the OvmfPkg and ArmVirtPkg DSC files.
> 
> Patch #6 converts OvmfPkg/PciHotPlugInitDxe to the new APIs, from manual
> capability list parsing. Because PciHotPlugInitDxe is a platform DXE
> driver, this conversion uses PciCapPciSegmentLib and PciCapLib.
> 
> Patch #7 converts OvmfPkg/Virtio10Dxe to the new APIs, from manual
> capability list parsing. Because Virtio10Dxe is a UEFI driver, this
> conversion uses PciCapPciIoLib and PciCapLib.
> 
> [ jump to the patches now, and return if you have questions: I
>   have answers, but you might find them too verbose in advance ]
> 
> Q1: If PCI config access needs to be abstracted from PciCapLib, then why
> don't we provide multiple instances of PciCapLib? In other words,
> why the PCI_CAP_DEV "protocol" rather than multiple instances of
> PciCapLib?
> 
> A1: The point of lib classes and instances is that the class provides a
> common interface, and the instances provide different
> implementations. We have the opposite use case here (hence the
> "strategy pattern"): the implementation of the higher level
> algorithm is common, and the objects that plug into it differ at the
> *interface* level ("Segment:Bus:Device.Function" notation versus
> PciIo protocol).
> 
> None of those notations are suitable for inclusion in the PciCapLib
> *class*; a more abstract interface is needed for identifying the PCI
> device to the library, for parsing of capabilities lists.
> 
> Q2: OK, let's say we can't (and shouldn't) include both notations in the
> PciCapLib class; can we settle on one of them, rather than
> PCI_CAP_DEV?
> 
> A2: No, we can't. PciIo is obviously unusable in PEIMs (and in quite a
> few platform DXE drivers too).
> 
> It takes a bit longer to explain why the other choice, i.e. adopting
> the "Segment:Bus:Device.Function" notation for the PciCapLib
> interface, is not good either. It has to do with the portability of
> UEFI drivers that bind PciIo protocol instances:
> 
> It is indeed easy enough to call PciIo->GetLocation() and plug the
> output values into a theoretical "Segment:Bus:Device.Function"
> interface. However, about the only two things that could consume
> such location identifiers and actually *implement* config space
> access would be:
> 
> (a) the EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL.Pci.Read/Write member
> functions, where the member functions themselves take the
> "Bus:Device.Function" part, and the "Segment" part must be used
> for selecting the EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL instance
> itself, through the SegmentNumber member;
> 
> (b) the PciSegmentLib interfaces.
> 
> Building a PciCapLib instance that uses (a) into a UEFI driver is a
> bad idea, because there are platforms that provide PciIo instances
> -- hence the driver can generally run on them -- but don't 

[edk2] [Patch 3/3] MdeModulePkg Variable: Fix XCODE5 varargs warning

[Liming Gao]

https://bugzilla.tianocore.org/show_bug.cgi?id=741
Change VariableGetBestLanguage() parameter type from BOOLEAN to UINTN

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Steven Shi 
Cc: Liming Gao 
---
 MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c | 2 +-
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c 
b/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
index 6dee2b6add..8ad2c7168d 100644
--- a/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
+++ b/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
@@ -575,7 +575,7 @@ CHAR8 *
 EFIAPI
 VariableGetBestLanguage (
   IN CONST CHAR8  *SupportedLanguages, 
-  IN BOOLEAN  Iso639Language,
+  IN UINTNIso639Language,
   ...
   )
 {
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 6caf603b3d..12047bbfce 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -1560,7 +1560,7 @@ CHAR8 *
 EFIAPI
 VariableGetBestLanguage (
   IN CONST CHAR8  *SupportedLanguages,
-  IN BOOLEAN  Iso639Language,
+  IN UINTNIso639Language,
   ...
   )
 {
-- 
2.16.2.windows.1

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

[edk2] [Patch 0/3] Fix XCODE5 varargs warning in GetBestLanguage API

[Liming Gao]

https://bugzilla.tianocore.org/show_bug.cgi?id=741
Change GetBestLanguage() parameter from BOOLEAN to UINTN
PI S3SaveState protocol will be updated after new PI spec is public.

Liming Gao (3):
  MdePkg UefiLib: Fix XCODE5 varargs warning
  IntelFrameworkPkg UefiLib: Fix XCODE5 varargs warning
  MdeModulePkg Variable: Fix XCODE5 varargs warning

 IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c| 2 +-
 MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c | 2 +-
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c   | 2 +-
 MdePkg/Include/Library/UefiLib.h| 2 +-
 MdePkg/Library/UefiLib/UefiLib.c| 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

-- 
2.16.2.windows.1

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

[edk2] [Patch 2/3] IntelFrameworkPkg UefiLib: Fix XCODE5 varargs warning

[Liming Gao]

https://bugzilla.tianocore.org/show_bug.cgi?id=741
Change GetBestLanguage() parameter type from BOOLEAN to UINTN

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Steven Shi 
Cc: Liming Gao 
---
 IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c 
b/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c
index 895ff39fc1..aa9162b539 100644
--- a/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c
+++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c
@@ -1486,7 +1486,7 @@ CHAR8 *
 EFIAPI
 GetBestLanguage (
   IN CONST CHAR8  *SupportedLanguages, 
-  IN BOOLEAN  Iso639Language,
+  IN UINTNIso639Language,
   ...
   )
 {
-- 
2.16.2.windows.1

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

[edk2] [Patch 1/3] MdePkg UefiLib: Fix XCODE5 varargs warning

[Liming Gao]

https://bugzilla.tianocore.org/show_bug.cgi?id=741
Change GetBestLanguage() parameter type from BOOLEAN to UINTN

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Steven Shi 
Cc: Liming Gao 
---
 MdePkg/Include/Library/UefiLib.h | 2 +-
 MdePkg/Library/UefiLib/UefiLib.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
index 54bc2cc5a3..9d0cdec2e5 100644
--- a/MdePkg/Include/Library/UefiLib.h
+++ b/MdePkg/Include/Library/UefiLib.h
@@ -818,7 +818,7 @@ CHAR8 *
 EFIAPI
 GetBestLanguage (
   IN CONST CHAR8  *SupportedLanguages, 
-  IN BOOLEAN  Iso639Language,
+  IN UINTNIso639Language,
   ...
   );
 
diff --git a/MdePkg/Library/UefiLib/UefiLib.c b/MdePkg/Library/UefiLib/UefiLib.c
index f1a3f1c7af..c96c0137d7 100644
--- a/MdePkg/Library/UefiLib/UefiLib.c
+++ b/MdePkg/Library/UefiLib/UefiLib.c
@@ -1514,7 +1514,7 @@ CHAR8 *
 EFIAPI
 GetBestLanguage (
   IN CONST CHAR8  *SupportedLanguages, 
-  IN BOOLEAN  Iso639Language,
+  IN UINTNIso639Language,
   ...
   )
 {
-- 
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 5/5] UefiCpuPkg: Remove X86 ASM and S files

[Gao, Liming]

Laszlo:
 1) Yes. Bug 881 is for nasm update. I will refer it in the commit log.
 2) Agree
 3) I will add notes to describe .asm in Vtf0 directory are used as nasm 
source. I don't clean up them.

Thanks
Liming
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Monday, May 14, 2018 2:40 AM
> To: Gao, Liming 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [Patch 5/5] UefiCpuPkg: Remove X86 ASM and S files
> 
> On 05/13/18 16:31, Liming Gao wrote:
> > NASM has replaced ASM and S files.
> > Rmove ASM from all modules.
> > Remove S files from the drivers only.
> > After NASM is updated, S files can be removed from Library.
> 
> The patch looks good to me. I suggest a few commit message updates:
> 
> (1) If I remember correctly, we have a TianoCore BZ for this (also for
> the NASM update). Can you please reference those here?
> 
> (2) There's a typo in "Rmove".
> 
> (3) Technically, a number of *.asm files remain under UefiCpuPkg, namely
> under "ResetVector/Vtf0". None of those are built stand-alone -- they
> are all included by "Vtf0.nasmb", and "Vtf0.inf" only references
> "Vtf0.nasmb". So technically speaking these *.asm files are *.nasm[b]
> files. I suggest we add a note about this fact, or else file a TianoCore
> BZ about renaming the Vtf0 ASM files.
> 
> With the commit message updates:
> 
> Reviewed-by: Laszlo Ersek 
> 
> Thanks!
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] OvmfPkg/PlatformBootManagerLib: connect consoles unconditionally

[Ard Biesheuvel]

On 13 May 2018 at 00:55, Laszlo Ersek  wrote:
> If both ConIn and ConOut exist, but ConIn references none of the PS/2
> keyboard, the USB wild-card keyboard, and any serial ports, then
> PlatformInitializeConsole() currently allows the boot to proceed without
> any input devices at all. This makes for a bad user experience -- the
> firmware menu could only be entered through OsIndications, set by a guest
> OS.
>
> Do what ArmVirtQemu does already, namely connect the consoles, and add
> them to ConIn / ConOut / ErrOut, unconditionally. (The underlying
> EfiBootManagerUpdateConsoleVariable() function checks for duplicates.)
>
> The issue used to be masked by the EfiBootManagerConnectAll() call that
> got conditionalized in commit 245c643cc8b7.
>
> This patch is best viewed with "git show -b -W".
>
> Cc: Ard Biesheuvel 
> Cc: Jordan Justen 
> Fixes: 245c643cc8b73240c3b88cb55b2911b285a8c10d
> Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1577546
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 

Reviewed-by: Ard Biesheuvel 


> ---
>
> Notes:
> Repo:   https://github.com/lersek/edk2.git
> Branch: conn_cons_uncond
>
>  OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 127 
> +++-
>  1 file changed, 44 insertions(+), 83 deletions(-)
>
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c 
> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> index 862fa6ebb4e5..004b753f4d26 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -26,7 +26,6 @@ VOID  *mEfiDevPathNotifyReg;
>  EFI_EVENT mEfiDevPathEvent;
>  VOID  *mEmuVariableEventReg;
>  EFI_EVENT mEmuVariableEvent;
> -BOOLEAN   mDetectVgaOnly;
>  UINT16mHostBridgeDevId;
>
>  //
> @@ -830,35 +829,33 @@ DetectAndPreparePlatformPciDevicePath (
>  );
>ASSERT_EFI_ERROR (Status);
>
> -  if (!mDetectVgaOnly) {
> +  //
> +  // Here we decide whether it is LPC Bridge
> +  //
> +  if ((IS_PCI_LPC (Pci)) ||
> +  ((IS_PCI_ISA_PDECODE (Pci)) &&
> +   (Pci->Hdr.VendorId == 0x8086) &&
> +   (Pci->Hdr.DeviceId == 0x7000)
> +  )
> + ) {
>  //
> -// Here we decide whether it is LPC Bridge
> +// Add IsaKeyboard to ConIn,
> +// add IsaSerial to ConOut, ConIn, ErrOut
>  //
> -if ((IS_PCI_LPC (Pci)) ||
> -((IS_PCI_ISA_PDECODE (Pci)) &&
> - (Pci->Hdr.VendorId == 0x8086) &&
> - (Pci->Hdr.DeviceId == 0x7000)
> -)
> -   ) {
> -  //
> -  // Add IsaKeyboard to ConIn,
> -  // add IsaSerial to ConOut, ConIn, ErrOut
> -  //
> -  DEBUG ((EFI_D_INFO, "Found LPC Bridge device\n"));
> -  PrepareLpcBridgeDevicePath (Handle);
> -  return EFI_SUCCESS;
> -}
> +DEBUG ((EFI_D_INFO, "Found LPC Bridge device\n"));
> +PrepareLpcBridgeDevicePath (Handle);
> +return EFI_SUCCESS;
> +  }
> +  //
> +  // Here we decide which Serial device to enable in PCI bus
> +  //
> +  if (IS_PCI_16550SERIAL (Pci)) {
>  //
> -// Here we decide which Serial device to enable in PCI bus
> +// Add them to ConOut, ConIn, ErrOut.
>  //
> -if (IS_PCI_16550SERIAL (Pci)) {
> -  //
> -  // Add them to ConOut, ConIn, ErrOut.
> -  //
> -  DEBUG ((EFI_D_INFO, "Found PCI 16550 SERIAL device\n"));
> -  PreparePciSerialDevicePath (Handle);
> -  return EFI_SUCCESS;
> -}
> +DEBUG ((EFI_D_INFO, "Found PCI 16550 SERIAL device\n"));
> +PreparePciSerialDevicePath (Handle);
> +return EFI_SUCCESS;
>}
>
>//
> @@ -877,26 +874,6 @@ DetectAndPreparePlatformPciDevicePath (
>  }
>
>
> -/**
> -  Do platform specific PCI Device check and add them to ConOut, ConIn, ErrOut
> -
> -  @param[in]  DetectVgaOnly - Only detect VGA device if it's TRUE.
> -
> -  @retval EFI_SUCCESS - PCI Device check and Console variable update
> -successfully.
> -  @retval EFI_STATUS - PCI Device check or Console variable update fail.
> -
> -**/
> -EFI_STATUS
> -DetectAndPreparePlatformPciDevicePaths (
> -  BOOLEAN DetectVgaOnly
> -  )
> -{
> -  mDetectVgaOnly = DetectVgaOnly;
> -  return VisitAllPciInstances (DetectAndPreparePlatformPciDevicePath);
> -}
> -
> -
>  /**
>Connect the predefined platform default console device.
>
> @@ -910,50 +887,34 @@ PlatformInitializeConsole (
>)
>  {
>UINTN  Index;
> -  EFI_DEVICE_PATH_PROTOCOL   *VarConout;
> -  EFI_DEVICE_PATH_PROTOCOL   *VarConin;
>
>//
> -  // Connect RootBridge
> +  // Do platform specific PCI Device check and add them to ConOut, ConIn,
> +  // ErrOut
>//
> -  GetEfiGlobalVariable2 (EFI_CON_OUT_VARIABLE_NAME, (VOID **) ,
> -NULL);
> -  GetEfiGlobalVariable2 (EFI_CON_IN_VARIABLE_NAME, (VOID **) , 
> NULL);
> -
> -  if 

Re: [edk2] [Patch 0/2] Fix 2 regression caused by enabling pyrite 2.0 feature changes.

[Wu, Hao A]

Reviewed-by: Hao Wu 

Best Regards,
Hao Wu


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Eric
> Dong
> Sent: Monday, May 14, 2018 3:36 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch 0/2] Fix 2 regression caused by enabling pyrite 2.0
> feature changes.
> 
> Fix GCC build failure and PSID revert no hint message issues caused
> by enabling pyrite 2.0 feature.
> 
> Eric Dong (2):
>   SecurityPkg/TcgStorageOpalLib: Fix GCC build failure.
>   SecurityPkg/OpalPassword: Fix PSID revert no hint message.
> 
>  .../Library/TcgStorageOpalLib/TcgStorageOpalCore.c |  4 ---
>  .../TcgStorageOpalLib/TcgStorageOpalLibInternal.h  |  1 -
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c | 29 +++---
> 
>  3 files changed, 20 insertions(+), 14 deletions(-)
> 
> --
> 2.15.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

[edk2] [Patch 2/2] SecurityPkg/OpalPassword: Fix PSID revert no hint message.

[Eric Dong]

For no warning message when do the PSID revert action, the
message in the popup dialog is not enough. The error use
of NULL for CreatePopUp function caused this regression.
This change fixed it.

Passed Unit Test:
1. Check PSID revert with/without warning message cases.

Cc: Hao Wu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c | 29 ++
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c 
b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
index 8733564f00..5d1638d5cf 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
@@ -687,15 +687,26 @@ OpalDriverPopUpPsidInput (
   InputLength = 0;
   while (TRUE) {
 Mask[InputLength] = L'_';
-CreatePopUp (
-  EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
-  ,
-  PopUpString,
-  PopUpString2,
-  L"-",
-  Mask,
-  NULL
-);
+if (PopUpString2 == NULL) {
+  CreatePopUp (
+EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
+,
+PopUpString,
+L"-",
+Mask,
+NULL
+  );
+} else {
+  CreatePopUp (
+EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
+,
+PopUpString,
+PopUpString2,
+L"-",
+Mask,
+NULL
+  );
+}
 
 //
 // Check key.
-- 
2.15.0.windows.1

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

[edk2] [Patch 0/2] Fix 2 regression caused by enabling pyrite 2.0 feature changes.

[Eric Dong]

Fix GCC build failure and PSID revert no hint message issues caused 
by enabling pyrite 2.0 feature.

Eric Dong (2):
  SecurityPkg/TcgStorageOpalLib: Fix GCC build failure.
  SecurityPkg/OpalPassword: Fix PSID revert no hint message.

 .../Library/TcgStorageOpalLib/TcgStorageOpalCore.c |  4 ---
 .../TcgStorageOpalLib/TcgStorageOpalLibInternal.h  |  1 -
 SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c | 29 +++---
 3 files changed, 20 insertions(+), 14 deletions(-)

-- 
2.15.0.windows.1

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

[edk2] [Patch 1/2] SecurityPkg/TcgStorageOpalLib: Fix GCC build failure.

[Eric Dong]

Function definition different with function implementation
caused this build failure. Change code to make them
consistent to pass the build.

Done Unit Test:
1. Pass GCC build.

Cc: Hao Wu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c| 4 
 SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h | 1 -
 2 files changed, 5 deletions(-)

diff --git a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c 
b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
index d794a91aad..cf37cad004 100644
--- a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
+++ b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
@@ -334,7 +334,6 @@ OpalPsidRevert(
 
 **/
 TCG_RESULT
-EFIAPI
 OpalPyrite2PsidRevert(
   OPAL_SESSION  *AdminSpSession,
   UINT32EstimateTimeCost
@@ -667,7 +666,6 @@ OpalGetMsid(
 
 **/
 TCG_RESULT
-EFIAPI
 OpalPyrite2GetActiveDataRemovalMechanism (
   IN  OPAL_SESSION*AdminSpSession,
   OUT UINT8   *ActiveDataRemovalMechanism
@@ -845,7 +843,6 @@ OpalAdminRevert(
 
 **/
 TCG_RESULT
-EFIAPI
 OpalPyrite2AdminRevert(
   OPAL_SESSION*LockingSpSession,
   BOOLEAN KeepUserData,
@@ -1866,7 +1863,6 @@ OpalGetLockingInfo(
 
 **/
 TCG_RESULT
-EFIAPI
 OpalGetFeatureDescriptor (
   IN OPAL_SESSION  *Session,
   IN UINT16FeatureCode,
diff --git a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h 
b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h
index cd16c51c3b..9a2f60454f 100644
--- a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h
+++ b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h
@@ -27,7 +27,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 **/
 TCG_RESULT
-EFIAPI
 OpalPyrite2GetActiveDataRemovalMechanism (
   OPAL_SESSION*AdminSpSession,
   UINT8   *ActiveDataRemovalMechanism
-- 
2.15.0.windows.1

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