Date   

Re: [edk2-platforms: PATCH 05/14] Marvell/Armada7k8k: MvBoardDesc: Extend protocol with PCIE support

Ard Biesheuvel
 

On Thu, 9 May 2019 at 11:54, Marcin Wojtas <mw@...> wrote:

Introduce new callback that can provide information about PCIE
controllers, which are used on the platform. According ArmadaSoCDescLib
ArmadaBoardDescLib routines are used for obtaining required data.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@...>
---
Silicon/Marvell/Include/Protocol/BoardDesc.h | 22 +++++
Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c | 86 ++++++++++++++++++++
2 files changed, 108 insertions(+)

diff --git a/Silicon/Marvell/Include/Protocol/BoardDesc.h b/Silicon/Marvell/Include/Protocol/BoardDesc.h
index 02905ea..c38ad86 100644
--- a/Silicon/Marvell/Include/Protocol/BoardDesc.h
+++ b/Silicon/Marvell/Include/Protocol/BoardDesc.h
@@ -90,6 +90,27 @@ EFI_STATUS
IN OUT MV_BOARD_XHCI_DESC **XhciDesc
);

+/**
+ Return the description of PCIE controllers used on the platform.
+
+ @param[in out] *This Pointer to board description protocol.
+ @param[in out] **PcieDescription Array containing PCIE controllers'
+ description.
+
+ @retval EFI_SUCCESS The data were obtained successfully.
+ @retval EFI_NOT_FOUND None of the controllers is used.
+ @retval EFI_INVALID_PARAMETER Description wrongly defined.
+ @retval EFI_OUT_OF_RESOURCES Lack of resources.
+ @retval Other Return error status.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *MV_BOARD_PCIE_DESCRIPTION_GET) (
+ IN MARVELL_BOARD_DESC_PROTOCOL *This,
+ IN OUT MV_BOARD_PCIE_DESCRIPTION **PcieDescription
+ );
+
typedef
EFI_STATUS
(EFIAPI *MV_BOARD_DESC_PP2_GET) (
@@ -121,6 +142,7 @@ struct _MARVELL_BOARD_DESC_PROTOCOL {
MV_BOARD_DESC_XHCI_GET BoardDescXhciGet;
MV_BOARD_DESC_FREE BoardDescFree;
MV_BOARD_GPIO_DESCRIPTION_GET GpioDescriptionGet;
+ MV_BOARD_PCIE_DESCRIPTION_GET PcieDescriptionGet;
};

#endif // __MARVELL_BOARD_DESC_PROTOCOL_H__
diff --git a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
index 973c362..9cd95bd 100644
--- a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
+++ b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
@@ -36,6 +36,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
MV_BOARD_DESC *mBoardDescInstance;

STATIC MV_BOARD_GPIO_DESCRIPTION *mGpioDescription;
+STATIC MV_BOARD_PCIE_DESCRIPTION *mPcieDescription;

STATIC
EFI_STATUS
@@ -444,6 +445,90 @@ MvBoardDescXhciGet (
return EFI_SUCCESS;
}

+/**
+ Return the description of PCIE controllers used on the platform.
+
+ @param[in out] *This Pointer to board description protocol.
+ @param[in out] **PcieDescription Array containing PCIE controllers'
+ description.
+
+ @retval EFI_SUCCESS The data were obtained successfully.
+ @retval EFI_NOT_FOUND None of the controllers is used.
+ @retval EFI_INVALID_PARAMETER Description wrongly defined.
+ @retval EFI_OUT_OF_RESOURCES Lack of resources.
+ @retval Other Return error status.
+
+**/
+STATIC
+EFI_STATUS
+MvBoardPcieDescriptionGet (
+ IN MARVELL_BOARD_DESC_PROTOCOL *This,
+ IN OUT MV_BOARD_PCIE_DESCRIPTION **PcieDescription
+ )
+{
+ UINTN SoCPcieControllerCount, BoardPcieControllerCount, SoCIndex, BoardIndex;
+ EFI_PHYSICAL_ADDRESS *PcieBaseAddresses;
+ MV_PCIE_CONTROLLER *PcieControllers;
+ EFI_STATUS Status;
+
+ /* Use existing structure if already created. */
+ if (mPcieDescription != NULL) {
+ *PcieDescription = mPcieDescription;
Since you are returning a cached copy here, I'd prefer it if the
prototype of the OUT parameter was MV_BOARD_PCIE_DESCRIPTION CONST**
rather than MV_BOARD_PCIE_DESCRIPTION**, since now, you are returning
a pointer to internal state of this driver.
(I guess this might apply to other methods as well)

+ return EFI_SUCCESS;
+ }
+
+ /* Get SoC data about all available PCIE controllers. */
+ Status = ArmadaSoCPcieGet (&PcieBaseAddresses, &SoCPcieControllerCount);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ /* Get per-board information about all used PCIE controllers. */
+ Status = ArmadaBoardPcieControllerGet (&PcieControllers,
+ &BoardPcieControllerCount);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ /* Sanity check of the board description. */
+ if (BoardPcieControllerCount > SoCPcieControllerCount) {
+ DEBUG ((DEBUG_ERROR, "%a: Too many controllers described\n", __FUNCTION__));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ for (BoardIndex = 0; BoardIndex < BoardPcieControllerCount; BoardIndex++) {
+ for (SoCIndex = 0; SoCIndex < SoCPcieControllerCount; SoCIndex++) {
+ if (PcieControllers[BoardIndex].PcieBaseAddress ==
+ PcieBaseAddresses[SoCIndex]) {
+ /* Match found */
+ break;
+ }
+ }
+ if (SoCIndex == SoCPcieControllerCount) {
+ DEBUG ((DEBUG_ERROR,
+ "%a: Controller #%d base address invalid: 0x%x\n",
+ __FUNCTION__,
+ BoardIndex,
+ PcieControllers[BoardIndex].PcieBaseAddress));
+ return EFI_INVALID_PARAMETER;
+ }
+ }
+
+ /* Allocate and fill board description. */
+ mPcieDescription = AllocateZeroPool (sizeof (MV_BOARD_PCIE_DESCRIPTION));
+ if (mPcieDescription == NULL) {
+ DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__));
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ mPcieDescription->PcieControllers = PcieControllers;
+ mPcieDescription->PcieControllerCount = BoardPcieControllerCount;
+
+ *PcieDescription = mPcieDescription;
+
+ return EFI_SUCCESS;
+}
+
STATIC
EFI_STATUS
MvBoardDescPp2Get (
@@ -621,6 +706,7 @@ MvBoardDescInitProtocol (
BoardDescProtocol->BoardDescXhciGet = MvBoardDescXhciGet;
BoardDescProtocol->BoardDescFree = MvBoardDescFree;
BoardDescProtocol->GpioDescriptionGet = MvBoardGpioDescriptionGet;
+ BoardDescProtocol->PcieDescriptionGet = MvBoardPcieDescriptionGet;

return EFI_SUCCESS;
}
--
2.7.4


Re: [PATCH 1/1] OvmfPkg/EnrollDefaultKeys: import the non-default key into db

Laszlo Ersek
 

On 05/16/19 05:08, Gary Lin wrote:
For QA test and development, we may need to test Secure Boot with a
devel key instead of UEFI CA.

This commit adds an argument, "--no-default", to EnrollDefaultKeys.efi.
With the argument, the key from SMBIOS Type 11 will also be enrolled
into db. Besides, the keys in AuthData.c, i.e. Microsoft KEK CA,
Microsoft PCA, and Microsoft UEFI CA will be excluded, so the developer
can easily create a varstore template for a specific key.

Cc: Laszlo Ersek <lersek@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Jordan Justen <jordan.l.justen@...>
Signed-off-by: Gary Lin <glin@...>
---
OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c | 53 ++++++++++++++-----
1 file changed, 39 insertions(+), 14 deletions(-)

diff --git a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
index 75f2749dc84a..f45cb799f726 100644
--- a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
+++ b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
@@ -538,6 +538,13 @@ ShellAppMain (
SETTINGS Settings;
UINT8 *PkKek1;
UINTN SizeOfPkKek1;
+ BOOLEAN NoDefault;
+
+ if (Argc == 2 && StrCmp (Argv[1], L"--no-default") == 0) {
+ NoDefault = TRUE;
+ } else {
+ NoDefault = FALSE;
+ }

//
// Prepare for failure.
@@ -594,13 +601,22 @@ ShellAppMain (
//
// Enroll db.
//
- Status = EnrollListOfCerts (
- EFI_IMAGE_SECURITY_DATABASE,
- &gEfiImageSecurityDatabaseGuid,
- &gEfiCertX509Guid,
- mMicrosoftPca, mSizeOfMicrosoftPca, &gMicrosoftVendorGuid,
- mMicrosoftUefiCa, mSizeOfMicrosoftUefiCa, &gMicrosoftVendorGuid,
- NULL);
+ if (NoDefault) {
+ Status = EnrollListOfCerts (
+ EFI_IMAGE_SECURITY_DATABASE,
+ &gEfiImageSecurityDatabaseGuid,
+ &gEfiCertX509Guid,
+ PkKek1, SizeOfPkKek1, &gEfiCallerIdGuid,
+ NULL);
+ } else {
+ Status = EnrollListOfCerts (
+ EFI_IMAGE_SECURITY_DATABASE,
+ &gEfiImageSecurityDatabaseGuid,
+ &gEfiCertX509Guid,
+ mMicrosoftPca, mSizeOfMicrosoftPca, &gMicrosoftVendorGuid,
+ mMicrosoftUefiCa, mSizeOfMicrosoftUefiCa, &gMicrosoftVendorGuid,
+ NULL);
+ }
if (EFI_ERROR (Status)) {
goto FreePkKek1;
}
@@ -621,13 +637,22 @@ ShellAppMain (
//
// Enroll KEK.
//
- Status = EnrollListOfCerts (
- EFI_KEY_EXCHANGE_KEY_NAME,
- &gEfiGlobalVariableGuid,
- &gEfiCertX509Guid,
- PkKek1, SizeOfPkKek1, &gEfiCallerIdGuid,
- mMicrosoftKek, mSizeOfMicrosoftKek, &gMicrosoftVendorGuid,
- NULL);
+ if (NoDefault) {
+ Status = EnrollListOfCerts (
+ EFI_KEY_EXCHANGE_KEY_NAME,
+ &gEfiGlobalVariableGuid,
+ &gEfiCertX509Guid,
+ PkKek1, SizeOfPkKek1, &gEfiCallerIdGuid,
+ NULL);
+ } else {
+ Status = EnrollListOfCerts (
+ EFI_KEY_EXCHANGE_KEY_NAME,
+ &gEfiGlobalVariableGuid,
+ &gEfiCertX509Guid,
+ PkKek1, SizeOfPkKek1, &gEfiCallerIdGuid,
+ mMicrosoftKek, mSizeOfMicrosoftKek, &gMicrosoftVendorGuid,
+ NULL);
+ }
if (EFI_ERROR (Status)) {
goto FreePkKek1;
}
Reviewed-by: Laszlo Ersek <lersek@...>

Pushed as commit 89d7c543cf71.

Thanks,
Laszlo


Re: [PATCH edk2-platform] Platform/PurleyOpenBoardPkg: Add the missing copyright and license

Zhou, Bowen <bowen.zhou@...>
 

Reviewed-by: Xiaohu Zhou <bowen.zhou@...>

-----Original Message-----
From: Gao, Liming
Sent: Thursday, May 16, 2019 8:27 AM
To: devel@edk2.groups.io
Cc: Lu, Shifei A <shifei.a.lu@...>; Zhou, Bowen <bowen.zhou@...>
Subject: [PATCH edk2-platform] Platform/PurleyOpenBoardPkg: Add the missing copyright and license

Signed-off-by: Liming Gao <liming.gao@...>
cc: Shifei A Lu <shifei.a.lu@...>
cc: Xiaohu Zhou <bowen.zhou@...>
---
.../Intel/PurleyOpenBoardPkg/Acpi/BoardAcpiDxe/AmlOffsetTable.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/Platform/Intel/PurleyOpenBoardPkg/Acpi/BoardAcpiDxe/AmlOffsetTable.c b/Platform/Intel/PurleyOpenBoardPkg/Acpi/BoardAcpiDxe/AmlOffsetTable.c
index 5d8714f589..abb484172e 100644
--- a/Platform/Intel/PurleyOpenBoardPkg/Acpi/BoardAcpiDxe/AmlOffsetTable.c
+++ b/Platform/Intel/PurleyOpenBoardPkg/Acpi/BoardAcpiDxe/AmlOffsetTable.c
@@ -1,3 +1,9 @@
+/** @file
+ Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
/*
*
* Intel ACPI Component Architecture
--
2.13.0.windows.1


Re: [PATCH edk2-platform] Silicon/Intel/PurleySktPkg: Add the missing copyright and license

Zhou, Bowen <bowen.zhou@...>
 

Reviewed-by: Xiaohu Zhou <bowen.zhou@...>

-----Original Message-----
From: Gao, Liming
Sent: Thursday, May 16, 2019 8:27 AM
To: devel@edk2.groups.io
Cc: Lu, Shifei A <shifei.a.lu@...>; Zhou, Bowen <bowen.zhou@...>
Subject: [PATCH edk2-platform] Silicon/Intel/PurleySktPkg: Add the missing copyright and license

Signed-off-by: Liming Gao <liming.gao@...>
cc: Shifei A Lu <shifei.a.lu@...>
cc: Xiaohu Zhou <bowen.zhou@...>
---
.../PurleySktPkg/Library/ProcMemInit/Chip/Include/Rc_Revision.h | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/Silicon/Intel/PurleySktPkg/Library/ProcMemInit/Chip/Include/Rc_Revision.h b/Silicon/Intel/PurleySktPkg/Library/ProcMemInit/Chip/Include/Rc_Revision.h
index df4d9a2ecf..2c5e74a059 100644
--- a/Silicon/Intel/PurleySktPkg/Library/ProcMemInit/Chip/Include/Rc_Revision.h
+++ b/Silicon/Intel/PurleySktPkg/Library/ProcMemInit/Chip/Include/Rc_Revision.h
@@ -1,3 +1,9 @@
+/** @file
+ Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
// Declarations and directives
// -------------------------------------
// Reference Code (RC) revision in BCD format:
--
2.13.0.windows.1


Re: [Patch 0/7] Add new CLANG8ELF tool chain for new LLVM/CLANG8

Liming Gao
 

This topic is discussed in edk2 design meeting. Here is the minutes https://edk2.groups.io/g/announce/topic/31575273

I also collect the image size for OvmfIa32x64 (Bytes). GCC and CLANG enables LTO, VS2015 enables GL.
OvmfIa32X64 (Bytes) GCC5 VS2015x86 CLANG8ELF CLANG8PE
PEIFV (IA32) 0x2ff28 0x2dfe8 0x2a5a8 0x57028
DXEFV (X64) 0x418528 0x429650 0x3ba6f8 0x502900
FVCOMPACT(Compress) 0x1372e8 0x1204d8 0x1177f0 0x116110

The image size shows new CLANG8ELF tool chain to get the smaller image size than GCC5 and VS2015x86 tool chain. So, I prefer to add CLANG8ELF tool chain first. Then, I will continue to investigate CLANG8PE tool chain and how to generate the different format debug symbol. If no other comments, I plan to add this tool chain for Q2 stable tag.

Thanks
Liming

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Liming Gao
Sent: Sunday, May 5, 2019 2:19 PM
To: devel@edk2.groups.io; leif.lindholm@...
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Subject: Re: [edk2-devel] [Patch 0/7] Add new CLANG8ELF tool chain for new LLVM/CLANG8

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Leif
Lindholm
Sent: Tuesday, April 30, 2019 7:01 PM
To: devel@edk2.groups.io; Gao, Liming <liming.gao@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Subject: Re: [edk2-devel] [Patch 0/7] Add new CLANG8ELF tool chain for new
LLVM/CLANG8

On Tue, Apr 30, 2019 at 04:21:29AM +0000, Liming Gao wrote:
This series confuses me. The existing CLANGxx toolchains already use
GenFw and ELF to PE/COFF conversion, so the name CLANG8ELF is
misleading.
LLVM/CLANG8.0 compiler supports to generate PE image or ELF
image. This tool chain is to generate ELF image and be converted to
PE image.
Which is what CLANG38 does - so why do we need a completely new
toolchain profile? (Shortly after we got rid of a bunch of unneeded
ones.)
CLANG38 depends on GNU binutils linker.
Yes.

It supports Linux only.
Really?
I mean, I haven't tested it on Windows, but I don't think there is any
fundamental limitation that should prevent it from working?
It may work on Windows. But, no one try the step.

It requires CLANG source code to be compiled, and be used.
OK, that is inconvenient.
I think you can get it through cygwin, but that creates other problems.

CLANG8ELF depends on LLVM LLD.
I would flip that statement.
It enables the use of LLD.
Yes.

LLVM/CLANG release provides the prebuilt binaries
for Windows/Linux/Mac. It is easy for user to setup the
environment. User can also use this tool chain in the different OS.
It was always my understanding that this was the intent of the CLANG##
profiles. So I do not see this as an added benefit.
Yes. Easy use single tool chain is the main purpose.
I am investigating another tool chain with CLANG8.0 to
directly generate PE image. To differentiate them, I use the tool
chain name CLANG8ELF and CLANG8PE for them.
Why do we want two different toolchain profiles that generate
identical output in different ways, using the same tools?
Generate the different debug symbols (DWARF, PDB) for the different
debugger. Windows user may use WinDbg for the source level
debug.
OK, this is a big deal, and I wish this had been mentioned both in the
https://bugzilla.tianocore.org/show_bug.cgi?id=1603 and the patch
submission.

The bugzilla entry reads to me only like "add CLANG8 profile" or "make
sure CLANG38 profile works with clang 8"..
Sorry for this confuse. I add such information in BZ.
Generate the different executable image to run Emulator in Windows
or Linux.

I need that CLANG8 tool chain provides the same functionality to
VS2015, GCC and XCODE tool chain. If so, the developer can use the
single tool chain for his development.
Again, I don't see this as being any different from what CLANG38
already gives us.
The difference is linker LLD or LD.

Also, it seems that the primary difference is using LLD instead of GNU
ld, but this has nothing to do with the Clang version.

What is the benefit of using LLD over GNU ld? It seems we are working
around various incompatibilities, and I think this is only justified
if LLD has some benefit over GNU ld.
LLD is part of LLVM/CLANG8 tool set. User can get all required
compilers and linkers from
http://releases.llvm.org/download.html#8.0.0.
LLVM8 release includes Windows/Linux/Mac version. User can
download
it and install them together. This tool chain is the unified tool
chain to be used in Windows/Linux/Mac OS.
Can we note already build under all of these operating systems with
the GNU binutils linker?
I am not sure. Now, I use VS2015 on Windows OS, use GCC5 on Linux
OS, and XCODE5 on Mac OS.
VS2015 and XCODE5 doesn't use GNU binutils linker.
Indeed.

So, to summarise - I am all for adding a toolchain profile that uses
clang with lld (this is also available with Linux distribution
packaged toolchains). But that is what we're doing - the fact that it's
version 8 of clang is beside the point.
If we cannot do this with a profile called CLANG8, then I would prefer
if we can call it LLDCLANG#.
Yes. New tool chain will use LLD linker. I find previous version LLD has one issue
https://bugs.llvm.org/show_bug.cgi?id=39810. It causes the build failure in edk2 build.
This issue is fixed in LLVM8.0 release. So, I name this tool chain as CLANG8.
I am OK to add LLD in tool chain name. So, new tool chain will be LLDCLANG8ELF.

I think if we are able to add another profile for native PE (and PDB),
that would be excellent. But the name ought to emphasise what the
functional difference in the output is rather than what the
intermediate steps are.
Yes. This is also in my plan.

/
Leif



Re: [PATCH] BaseTools:Update mailing list address in BaseTools error messages

Laszlo Ersek
 

On 05/16/19 07:11, Fan, ZhijuX wrote:
BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1815

The edk2 source tree contains four instances of the outdated mailing
list address "edk2-devel@...".
I factoring these out to a new constant, and then setting the constant
to "devel@edk2.groups.io".
(1) The commit message should be cleaned up: "Factor these out ... and
then set ..."

Furthermore:


Cc: Bob Feng <bob.c.feng@...>
Cc: Liming Gao <liming.gao@...>
Signed-off-by: Zhiju.Fan <zhijux.fan@...>
---
BaseTools/Source/Python/Common/DataType.py | 1 +
BaseTools/Source/Python/GenFds/GenFds.py | 2 +-
BaseTools/Source/Python/Trim/Trim.py | 2 +-
BaseTools/Source/Python/UPT/Logger/StringTable.py | 2 +-
BaseTools/Source/Python/build/build.py | 2 +-
5 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/Python/Common/DataType.py b/BaseTools/Source/Python/Common/DataType.py
index 780711bf8e..7f10533345 100644
--- a/BaseTools/Source/Python/Common/DataType.py
+++ b/BaseTools/Source/Python/Common/DataType.py
@@ -88,6 +88,7 @@ EDK_COMPONENT_TYPE_SAL_RT_DRIVER = 'SAL_RT_DRIVER'
EDK_COMPONENT_TYPE_APPLICATION = 'APPLICATION'
EDK_NAME = 'EDK'
EDKII_NAME = 'EDKII'
+EDKII_MAIL_ADDR = 'devel@edk2.groups.io'

COMPONENT_TO_MODULE_MAP_DICT = {
EDK_COMPONENT_TYPE_LIBRARY : SUP_MODULE_BASE,
diff --git a/BaseTools/Source/Python/GenFds/GenFds.py b/BaseTools/Source/Python/GenFds/GenFds.py
index 21ae9c4d4c..bd70aac9f5 100644
--- a/BaseTools/Source/Python/GenFds/GenFds.py
+++ b/BaseTools/Source/Python/GenFds/GenFds.py
@@ -388,7 +388,7 @@ def GenFdsApi(FdsCommandDict, WorkSpaceDataBase=None):
"\nPython",
CODE_ERROR,
"Tools code failure",
- ExtraData="Please send email to edk2-devel@... for help, attaching following call stack trace!\n",
+ ExtraData="Please send email to %s for help, attaching following call stack trace!\n" % EDKII_MAIL_ADDR,
RaiseError=False
)
EdkLogger.quiet(traceback.format_exc())
diff --git a/BaseTools/Source/Python/Trim/Trim.py b/BaseTools/Source/Python/Trim/Trim.py
index 6f29f1a35a..573f33772f 100644
--- a/BaseTools/Source/Python/Trim/Trim.py
+++ b/BaseTools/Source/Python/Trim/Trim.py
@@ -533,7 +533,7 @@ def Main():
"\nTrim",
CODE_ERROR,
"Unknown fatal error when trimming [%s]" % InputFile,
- ExtraData="\n(Please send email to edk2-devel@... for help, attaching following call stack trace!)\n",
+ ExtraData="\n(Please send email to %s for help, attaching following call stack trace!)\n" % EDKII_MAIL_ADDR,
RaiseError=False
)
EdkLogger.quiet("(Python %s on %s) " % (platform.python_version(), sys.platform) + traceback.format_exc())
diff --git a/BaseTools/Source/Python/UPT/Logger/StringTable.py b/BaseTools/Source/Python/UPT/Logger/StringTable.py
index 59f71d390b..c1eda61d05 100644
--- a/BaseTools/Source/Python/UPT/Logger/StringTable.py
+++ b/BaseTools/Source/Python/UPT/Logger/StringTable.py
@@ -317,7 +317,7 @@ MSG_NEW_FILE_NAME_FOR_DIST = _(
MSG_UPDATE_PACKAGE_DATABASE = _("Update Distribution Package Database ...")
MSG_PYTHON_ON = _("(Python %s on %s) ")
MSG_SEARCH_FOR_HELP = _(
- "\n(Please send email to edk2-devel@... for\n"
+ "\n(Please send email to devel@edk2.groups.io for\n"
" help, attach the following call stack trace.)\n")
(2) When I filed the BZ, I didn't realize that this instance of the old
list address was in a localized string / message.

However, the example of MSG_PYTHON_ON shows that such messages can take
parameters as well. This means that all code locations referencing
MSG_SEARCH_FOR_HELP will have to be updated -- MSG_SEARCH_FOR_HELP
should use %s, and all uses of MSG_SEARCH_FOR_HELP should pass in
EDKII_MAIL_ADDR as a parameter:

$ git grep -l -w MSG_SEARCH_FOR_HELP

Source/Python/UPT/InstallPkg.py
Source/Python/UPT/InventoryWs.py
Source/Python/UPT/Logger/StringTable.py
Source/Python/UPT/MkPkg.py
Source/Python/UPT/ReplacePkg.py
Source/Python/UPT/RmPkg.py
Source/Python/UPT/TestInstall.py

For example,

ExtraData=ST.MSG_SEARCH_FOR_HELP % EDKII_MAIL_ADDR

Thanks
Laszlo

MSG_REMOVE_TEMP_FILE_STARTED = _("Removing temp files started ... ")
MSG_REMOVE_TEMP_FILE_DONE = _("Removing temp files ... Done.")
diff --git a/BaseTools/Source/Python/build/build.py b/BaseTools/Source/Python/build/build.py
index 7271570d29..0071581c80 100644
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -2480,7 +2480,7 @@ def Main():
"\nbuild",
CODE_ERROR,
"Unknown fatal error when processing [%s]" % MetaFile,
- ExtraData="\n(Please send email to edk2-devel@... for help, attaching following call stack trace!)\n",
+ ExtraData="\n(Please send email to %s for help, attaching following call stack trace!)\n" % EDKII_MAIL_ADDR,
RaiseError=False
)
EdkLogger.quiet("(Python %s on %s) " % (platform.python_version(), sys.platform) + traceback.format_exc())


Re: [Patch] Nt32Pkg: Remove it

Liming Gao
 

Ray:
I have no comments on this change. Please also send the patch to update maintainer.txt.

Reviewed-by: Liming Gao <liming.gao@...>

Thanks
Liming

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ni, Ray
Sent: Wednesday, May 15, 2019 6:12 PM
To: Wu, Hao A <hao.a.wu@...>
Cc: devel@edk2.groups.io
Subject: [edk2-devel][Patch] Nt32Pkg: Remove it

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

Now since EmulatorPkg supports to run in Windows environment,
this patch removes Nt32Pkg to remove duplicate code in edk2 repo.

Signed-off-by: Ray Ni <ray.ni@...>
Cc: Hao A Wu <hao.a.wu@...>

--------------
NOTE: This patch is to completely remove Nt32Pkg folder in edk2 repo.
I don't want to flood people's inbox with a big patch just removing
every line of code in Nt32Pkg.

The patch that modifies Maintainers.txt will be sent out later after
Nt32Pkg is removed.


Re: [PATCH] UefiCpuPkg CpuCommonFeaturesLib: Remove CPU generation check

Laszlo Ersek
 

Hi Star,

On 05/16/19 12:33, Star Zeng wrote:
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1679

The checking to CpuInfo->CpuIdVersionInfoEcx.Bits.AESNI is enough,
the checking to CPU generation could be removed, then the code
could be reused by more platforms.

Cc: Laszlo Ersek <lersek@...>
Cc: Eric Dong <eric.dong@...>
Cc: Ruiyu Ni <ruiyu.ni@...>
Cc: Chandana Kumar <chandana.c.kumar@...>
Signed-off-by: Star Zeng <star.zeng@...>
---
UefiCpuPkg/Library/CpuCommonFeaturesLib/Aesni.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/Aesni.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/Aesni.c
index b79446ba3ca9..4a56eec1b267 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/Aesni.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/Aesni.c
@@ -57,15 +57,9 @@ AesniSupport (
MSR_SANDY_BRIDGE_FEATURE_CONFIG_REGISTER *MsrFeatureConfig;

if (CpuInfo->CpuIdVersionInfoEcx.Bits.AESNI == 1) {
- if (IS_SANDY_BRIDGE_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) ||
- IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) ||
- IS_XEON_5600_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) ||
- IS_XEON_E7_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) ||
- IS_XEON_PHI_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)) {
- MsrFeatureConfig = (MSR_SANDY_BRIDGE_FEATURE_CONFIG_REGISTER *) ConfigData;
- ASSERT (MsrFeatureConfig != NULL);
- MsrFeatureConfig[ProcessorNumber].Uint64 = AsmReadMsr64 (MSR_SANDY_BRIDGE_FEATURE_CONFIG);
- }
+ MsrFeatureConfig = (MSR_SANDY_BRIDGE_FEATURE_CONFIG_REGISTER *) ConfigData;
+ ASSERT (MsrFeatureConfig != NULL);
+ MsrFeatureConfig[ProcessorNumber].Uint64 = AsmReadMsr64 (MSR_SANDY_BRIDGE_FEATURE_CONFIG);
return TRUE;
}
return FALSE;
the patch and the bugzilla ticket claim that the AESNI bit's presence in
CPUID guarantees that MSR 0x13C is available.

I don't see what guarantees this. According to the latest Intel SDM Vol
4, which I just downloaded (335592-069US, January 2019),
MSR_FEATURE_CONFIG is available on the following (DisplayFamily,
DisplayModel) pairs:

- 06_37H, 06_4AH, 06_4DH, 06_5AH, 06_5DH, 06_5CH, 06_7AH
- 06_25H, 06_2CH
- 06_2FH
- 06_2AH, 06_2DH
- 06_57H

Which seems to indicate that at least *the approach* of the original
code -- i.e. the family/model checking -- is correct. (It's possible
that the family/model list has to be extended from time to time, of course.)

Anyway, I don't intend to block this patch; OVMF does not use
CpuCommonFeaturesLib, so this change cannot regress it. I will let other
UefiCpuPkg reviewers decide about this patch.

Thanks!
Laszlo


Re: [PATCH 3/4] OvmfPkg/PlatformPei: reorder the 32-bit PCI hole vs. the PCIEXBAR on q35

Philippe Mathieu-Daudé
 

Hi Laszlo,

On 5/16/19 2:18 PM, Laszlo Ersek wrote:
On 05/16/19 10:00, Ard Biesheuvel wrote:
On Sat, 4 May 2019 at 02:07, Laszlo Ersek <lersek@...> wrote:

Commit 7b8fe63561b4 ("OvmfPkg: PlatformPei: enable PCIEXBAR (aka MMCONFIG
/ ECAM) on Q35", 2016-03-10) claimed that,

On Q35 machine types that QEMU intends to support in the long term, QEMU
never lets the RAM below 4 GB exceed 2 GB.

Alas, this statement came from a misunderstanding that occurred while we
worked out the interface contract. In fact QEMU does allow the 32-bit RAM
extend up to 0xB000_0000 (exclusive), in case the RAM size falls in the
range (0x8000_0000, 0xB000_0000) (i.e., the RAM size is greater than
2048MB and smaller than 2816MB).

In turn, such a RAM size (justifiedly) triggers

ASSERT (TopOfLowRam <= PciExBarBase);

in MemMapInitialization(), because we placed the 256MB PCIEXBAR at
0x8000_0000 (2GB) exactly, relying on the interface contract. (And, the
32-bit PCI hole would follow the PCIEXBAR, covering the [0x9000_0000,
0xFC00_0000) range.)

In order to fix this, reorder the 32-bit PCI hole against the PCIEXBAR, as
follows:

- start the 32-bit PCI hole where it starts on i440fx as well, that is, at
2GB or TopOfLowRam, whichever is higher;

- unlike on i440fx, where the 32-bit PCI hole extends up to 0xFC00_0000,
stop it at 0xE000_0000 on q35,

- place the PCIEXBAR at 0xE000_0000.

(We cannot place the PCIEXBAR at 0xF000_0000 because the 256MB MMIO area
that starts there is not entirely free.)

Before this patch, the 32-bit PCI hole used to only *end* at the same spot
(namely, 0xFC00_0000) between i440fx and q35; now it will only *start* at
the same spot (namely, 2GB or TopOfLowRam, whichever is higher) between
both boards.

On q35, the maximal hole shrinks from

0xFC00_0000 - 0x9000_0000 = 0x6C00_0000 == 1728 MB

to

0xE000_0000 - 0x8000_0000 == 1536 MB.

We lose 192 MB of the aperture; however, the aperture is now aligned at
1GB, rather than 256 MB, and so it could fit a 1GB BAR even.

Regarding the minimal hole (triggered by RAM size 2815MB), its size is

0xE000_0000 - 0xAFF0_0000 = 769 MB

which is not great, but probably better than a failed ASSERT.

Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Gerd Hoffmann <kraxel@...>
Cc: Jordan Justen <jordan.l.justen@...>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1666941
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1701710
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@...>
I take it the 32-bit PCI 'hole' is in fact the 32-bit MMIO BAR window?
Yes, that's correct. We also call it "32-bit PCI MMIO aperture",
sometimes. The interval that PciHostBridgeLib passes to
PciHostBridgeDxe, to allocate BARs out of, when PciBusDxe asks.
Maybe you can amend that comment, ...


That could do with a clarification. I guess it may be an x86-ism to
consider any physical memory region not used by RAM to be a hole, but
it is not terribly helpful.
The "PCI hole" expressions is a QEMU-ism; it is frequently used in
... and this one in the commit description.

Regardless:
Reviewed-by: Philippe Mathieu-Daude <philmd@...>

connection with the i440fx and q35 (x86) machine types, and not much
elsewhere:

$ git grep -c pci_hole

hw/i386/acpi-build.c:10
hw/i386/pc.c:1
hw/pci-host/grackle.c:3
hw/pci-host/piix.c:23
hw/pci-host/q35.c:24
hw/pci-host/uninorth.c:4
include/hw/i386/pc.h:1
include/hw/pci-host/q35.h:3
include/hw/pci-host/uninorth.h:1

In any case,

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@...>
Thank you!
Laszlo


---
OvmfPkg/OvmfPkgIa32.dsc | 5 +----
OvmfPkg/OvmfPkgIa32X64.dsc | 5 +----
OvmfPkg/OvmfPkgX64.dsc | 5 +----
OvmfPkg/PlatformPei/Platform.c | 9 ++++-----
4 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 36a0f87258dd..bb55b6fa58e1 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -491,10 +491,7 @@ [PcdsFixedAtBuild]
# This PCD is used to set the base address of the PCI express hierarchy. It
# is only consulted when OVMF runs on Q35. In that case it is programmed into
# the PCIEXBAR register.
- #
- # On Q35 machine types that QEMU intends to support in the long term, QEMU
- # never lets the RAM below 4 GB exceed 2 GB.
- gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x80000000
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xE0000000

!ifdef $(SOURCE_DEBUG_ENABLE)
gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 9b341e17d7ff..06c394a6fb1f 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -496,10 +496,7 @@ [PcdsFixedAtBuild]
# This PCD is used to set the base address of the PCI express hierarchy. It
# is only consulted when OVMF runs on Q35. In that case it is programmed into
# the PCIEXBAR register.
- #
- # On Q35 machine types that QEMU intends to support in the long term, QEMU
- # never lets the RAM below 4 GB exceed 2 GB.
- gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x80000000
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xE0000000

!ifdef $(SOURCE_DEBUG_ENABLE)
gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index a0f87f74dab9..5e0eb043fab9 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -496,10 +496,7 @@ [PcdsFixedAtBuild]
# This PCD is used to set the base address of the PCI express hierarchy. It
# is only consulted when OVMF runs on Q35. In that case it is programmed into
# the PCIEXBAR register.
- #
- # On Q35 machine types that QEMU intends to support in the long term, QEMU
- # never lets the RAM below 4 GB exceed 2 GB.
- gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x80000000
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xE0000000

!ifdef $(SOURCE_DEBUG_ENABLE)
gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 9c013613a1a0..fd8eccaf3e50 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -184,14 +184,13 @@ MemMapInitialization (
PciBase = (TopOfLowRam < BASE_2GB) ? BASE_2GB : TopOfLowRam;
if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) {
//
- // The MMCONFIG area is expected to fall between the top of low RAM and
- // the base of the 32-bit PCI host aperture.
+ // The 32-bit PCI host aperture is expected to fall between the top of
+ // low RAM and the base of the MMCONFIG area.
//
PciExBarBase = FixedPcdGet64 (PcdPciExpressBaseAddress);
- ASSERT (TopOfLowRam <= PciExBarBase);
+ ASSERT (PciBase < PciExBarBase);
ASSERT (PciExBarBase <= MAX_UINT32 - SIZE_256MB);
- PciBase = (UINT32)(PciExBarBase + SIZE_256MB);
- PciSize = 0xFC000000 - PciBase;
+ PciSize = (UINT32)(PciExBarBase - PciBase);
} else {
PciSize = 0xFC000000 - PciBase;
}
--
2.19.1.3.g30247aa5d201



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

View/Reply Online (#39968): https://edk2.groups.io/g/devel/message/39968
Mute This Topic: https://groups.io/mt/31489698/1761188
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [ard.biesheuvel@...]
------------



Re: [PATCH 3/4] OvmfPkg/PlatformPei: reorder the 32-bit PCI hole vs. the PCIEXBAR on q35

Laszlo Ersek
 

On 05/16/19 10:00, Ard Biesheuvel wrote:
On Sat, 4 May 2019 at 02:07, Laszlo Ersek <lersek@...> wrote:

Commit 7b8fe63561b4 ("OvmfPkg: PlatformPei: enable PCIEXBAR (aka MMCONFIG
/ ECAM) on Q35", 2016-03-10) claimed that,

On Q35 machine types that QEMU intends to support in the long term, QEMU
never lets the RAM below 4 GB exceed 2 GB.

Alas, this statement came from a misunderstanding that occurred while we
worked out the interface contract. In fact QEMU does allow the 32-bit RAM
extend up to 0xB000_0000 (exclusive), in case the RAM size falls in the
range (0x8000_0000, 0xB000_0000) (i.e., the RAM size is greater than
2048MB and smaller than 2816MB).

In turn, such a RAM size (justifiedly) triggers

ASSERT (TopOfLowRam <= PciExBarBase);

in MemMapInitialization(), because we placed the 256MB PCIEXBAR at
0x8000_0000 (2GB) exactly, relying on the interface contract. (And, the
32-bit PCI hole would follow the PCIEXBAR, covering the [0x9000_0000,
0xFC00_0000) range.)

In order to fix this, reorder the 32-bit PCI hole against the PCIEXBAR, as
follows:

- start the 32-bit PCI hole where it starts on i440fx as well, that is, at
2GB or TopOfLowRam, whichever is higher;

- unlike on i440fx, where the 32-bit PCI hole extends up to 0xFC00_0000,
stop it at 0xE000_0000 on q35,

- place the PCIEXBAR at 0xE000_0000.

(We cannot place the PCIEXBAR at 0xF000_0000 because the 256MB MMIO area
that starts there is not entirely free.)

Before this patch, the 32-bit PCI hole used to only *end* at the same spot
(namely, 0xFC00_0000) between i440fx and q35; now it will only *start* at
the same spot (namely, 2GB or TopOfLowRam, whichever is higher) between
both boards.

On q35, the maximal hole shrinks from

0xFC00_0000 - 0x9000_0000 = 0x6C00_0000 == 1728 MB

to

0xE000_0000 - 0x8000_0000 == 1536 MB.

We lose 192 MB of the aperture; however, the aperture is now aligned at
1GB, rather than 256 MB, and so it could fit a 1GB BAR even.

Regarding the minimal hole (triggered by RAM size 2815MB), its size is

0xE000_0000 - 0xAFF0_0000 = 769 MB

which is not great, but probably better than a failed ASSERT.

Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Gerd Hoffmann <kraxel@...>
Cc: Jordan Justen <jordan.l.justen@...>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1666941
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1701710
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@...>
I take it the 32-bit PCI 'hole' is in fact the 32-bit MMIO BAR window?
Yes, that's correct. We also call it "32-bit PCI MMIO aperture",
sometimes. The interval that PciHostBridgeLib passes to
PciHostBridgeDxe, to allocate BARs out of, when PciBusDxe asks.

That could do with a clarification. I guess it may be an x86-ism to
consider any physical memory region not used by RAM to be a hole, but
it is not terribly helpful.
The "PCI hole" expressions is a QEMU-ism; it is frequently used in
connection with the i440fx and q35 (x86) machine types, and not much
elsewhere:

$ git grep -c pci_hole

hw/i386/acpi-build.c:10
hw/i386/pc.c:1
hw/pci-host/grackle.c:3
hw/pci-host/piix.c:23
hw/pci-host/q35.c:24
hw/pci-host/uninorth.c:4
include/hw/i386/pc.h:1
include/hw/pci-host/q35.h:3
include/hw/pci-host/uninorth.h:1

In any case,

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@...>
Thank you!
Laszlo


---
OvmfPkg/OvmfPkgIa32.dsc | 5 +----
OvmfPkg/OvmfPkgIa32X64.dsc | 5 +----
OvmfPkg/OvmfPkgX64.dsc | 5 +----
OvmfPkg/PlatformPei/Platform.c | 9 ++++-----
4 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 36a0f87258dd..bb55b6fa58e1 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -491,10 +491,7 @@ [PcdsFixedAtBuild]
# This PCD is used to set the base address of the PCI express hierarchy. It
# is only consulted when OVMF runs on Q35. In that case it is programmed into
# the PCIEXBAR register.
- #
- # On Q35 machine types that QEMU intends to support in the long term, QEMU
- # never lets the RAM below 4 GB exceed 2 GB.
- gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x80000000
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xE0000000

!ifdef $(SOURCE_DEBUG_ENABLE)
gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 9b341e17d7ff..06c394a6fb1f 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -496,10 +496,7 @@ [PcdsFixedAtBuild]
# This PCD is used to set the base address of the PCI express hierarchy. It
# is only consulted when OVMF runs on Q35. In that case it is programmed into
# the PCIEXBAR register.
- #
- # On Q35 machine types that QEMU intends to support in the long term, QEMU
- # never lets the RAM below 4 GB exceed 2 GB.
- gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x80000000
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xE0000000

!ifdef $(SOURCE_DEBUG_ENABLE)
gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index a0f87f74dab9..5e0eb043fab9 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -496,10 +496,7 @@ [PcdsFixedAtBuild]
# This PCD is used to set the base address of the PCI express hierarchy. It
# is only consulted when OVMF runs on Q35. In that case it is programmed into
# the PCIEXBAR register.
- #
- # On Q35 machine types that QEMU intends to support in the long term, QEMU
- # never lets the RAM below 4 GB exceed 2 GB.
- gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x80000000
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xE0000000

!ifdef $(SOURCE_DEBUG_ENABLE)
gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 9c013613a1a0..fd8eccaf3e50 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -184,14 +184,13 @@ MemMapInitialization (
PciBase = (TopOfLowRam < BASE_2GB) ? BASE_2GB : TopOfLowRam;
if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) {
//
- // The MMCONFIG area is expected to fall between the top of low RAM and
- // the base of the 32-bit PCI host aperture.
+ // The 32-bit PCI host aperture is expected to fall between the top of
+ // low RAM and the base of the MMCONFIG area.
//
PciExBarBase = FixedPcdGet64 (PcdPciExpressBaseAddress);
- ASSERT (TopOfLowRam <= PciExBarBase);
+ ASSERT (PciBase < PciExBarBase);
ASSERT (PciExBarBase <= MAX_UINT32 - SIZE_256MB);
- PciBase = (UINT32)(PciExBarBase + SIZE_256MB);
- PciSize = 0xFC000000 - PciBase;
+ PciSize = (UINT32)(PciExBarBase - PciBase);
} else {
PciSize = 0xFC000000 - PciBase;
}
--
2.19.1.3.g30247aa5d201



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

View/Reply Online (#39968): https://edk2.groups.io/g/devel/message/39968
Mute This Topic: https://groups.io/mt/31489698/1761188
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [ard.biesheuvel@...]
------------


[Patch] BaseTools: Fix private includes for FILE_GUID override

Bob Feng
 

From: Michael D Kinney <michael.d.kinney@...>

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1648

When a module in a DSC file uses a FILE_GUID override in the
module scoped <Defines> section, a copy of the modified INF file
is placed in the Conf/.cache directory. The check for private
includes uses the INF path to determine if the module is allowed
to use the private includes. Since the INF path in this case is
not in any package, this check always fails, and no private
include paths are possible.

The fix is to keep both the OriginalPath and the new Path in
the PathClass object, and always use the OriginalPath to see if
the module INF is in the package with private includes.

Signed-off-by: Michael D Kinney <michael.d.kinney@...>
Cc: Bob Feng <bob.c.feng@...>
Cc: Liming Gao <liming.gao@...>
---
BaseTools/Source/Python/AutoGen/AutoGen.py | 2 +-
BaseTools/Source/Python/Common/Misc.py | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 31721a6f9f..c174b5a0bb 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -3414,11 +3414,11 @@ class ModuleAutoGen(AutoGen):
PackageDir = mws.join(self.WorkspaceDir, Package.MetaFile.Dir)
if PackageDir not in RetVal:
RetVal.append(PackageDir)
IncludesList = Package.Includes
if Package._PrivateIncludes:
- if not self.MetaFile.Path.startswith(PackageDir):
+ if not self.MetaFile.OriginalPath.Path.startswith(PackageDir):
IncludesList = list(set(Package.Includes).difference(set(Package._PrivateIncludes)))
for Inc in IncludesList:
if Inc not in RetVal:
RetVal.append(str(Inc))
return RetVal
diff --git a/BaseTools/Source/Python/Common/Misc.py b/BaseTools/Source/Python/Common/Misc.py
index 0e0cb45ebe..d082c58bef 100644
--- a/BaseTools/Source/Python/Common/Misc.py
+++ b/BaseTools/Source/Python/Common/Misc.py
@@ -279,10 +279,11 @@ def ProcessDuplicatedInf(Path, BaseName, Workspace):
# The BaseName is the FILE_GUID which is also the output directory name.
#
#
RtPath.Path = TempFullPath
RtPath.BaseName = BaseName
+ RtPath.OriginalPath = Path
#
# If file exists, compare contents
#
if os.path.exists(TempFullPath):
with open(str(Path), 'rb') as f1, open(TempFullPath, 'rb') as f2:
@@ -1403,10 +1404,11 @@ class PathClass(object):
self.IsBinary = IsBinary
self.Target = Target
self.TagName = TagName
self.ToolCode = ToolCode
self.ToolChainFamily = ToolChainFamily
+ self.OriginalPath = self

## Convert the object of this class to a string
#
# Convert member Path of the class to a string
#
--
2.20.1.windows.1


Re: [PATCH] BaseTools: Library hashing fix and optimization for --hash feature

Bob Feng
 

Hi Christian,

+ # Return a Boolean based on if can skip by hash, either from memory or from IO.
+ if self.Name not in GlobalData.gBuildHashSkipTracking[self.Arch]:
+ # If hashes are the same, SaveFileOnChange() will return False.
+ GlobalData.gBuildHashSkipTracking[self.Arch][self.Name] = not SaveFileOnChange(HashFile, self.GenModuleHash(), False)
+ return GlobalData.gBuildHashSkipTracking[self.Arch][self.Name]
+ else:
+ return


For SaveFileOnChange(HashFile, self.GenModuleHash(), False), the third parameter False should be True since the self.GenModuleHash() return byte types not str.

Thanks,
Bob

-----Original Message-----
From: Rodriguez, Christian
Sent: Wednesday, May 15, 2019 1:56 AM
To: devel@edk2.groups.io
Cc: Feng, Bob C <bob.c.feng@...>; Gao, Liming <liming.gao@...>; Zhu, Yonghong <yonghong.zhu@...>
Subject: [PATCH] BaseTools: Library hashing fix and optimization for --hash feature

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1788
Library hashing is now supported by the --hash feature. The --hash feature implementation assumed that the hashing could be done in place once per module, but that isn't true for libraries due to the fact that they are built as dependencies. So on a clean build, we now generate the .hash after the library dependencies are complete.
Added early escape as optimization, if hash already exists in memory.

Signed-off-by: Christian Rodriguez <christian.rodriguez@...>
Cc: Bob Feng <bob.c.feng@...>
Cc: Liming Gao <liming.gao@...>
Cc: Yonghong Zhu <yonghong.zhu@...>
---
BaseTools/Source/Python/AutoGen/AutoGen.py | 52 +++++++++++++++++++++++++++++++++++++++-------------
BaseTools/Source/Python/Common/GlobalData.py | 6 ++++++
BaseTools/Source/Python/build/build.py | 7 ++++++-
3 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 31721a6f9f..cf92d07be1 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -4093,13 +4093,20 @@ class ModuleAutoGen(AutoGen):
return RetVal

def GenModuleHash(self):
+ # Initialize a dictionary for each arch type
if self.Arch not in GlobalData.gModuleHash:
GlobalData.gModuleHash[self.Arch] = {}
- if self.Name in GlobalData.gModuleHash[self.Arch] and GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
- return False
+
+ # Early exit if module or library has been hashed and is in memory
+ if self.Name in GlobalData.gModuleHash[self.Arch]:
+ return
+ GlobalData.gModuleHash[self.Arch][self.Name].encode('utf-8')
+
+ # Initialze hash object
m = hashlib.md5()
+
# Add Platform level hash
m.update(GlobalData.gPlatformHash.encode('utf-8'))
+
# Add Package level hash
if self.DependentPackageList:
for Pkg in sorted(self.DependentPackageList, key=lambda x: x.PackageName):
@@ -4118,6 +4125,7 @@ class ModuleAutoGen(AutoGen):
Content = f.read()
f.close()
m.update(Content)
+
# Add Module's source files
if self.SourceFileList:
for File in sorted(self.SourceFileList, key=lambda x: str(x)):
@@ -4126,27 +4134,45 @@ class ModuleAutoGen(AutoGen):
f.close()
m.update(Content)

- ModuleHashFile = path.join(self.BuildDir, self.Name + ".hash")
- if self.Name not in GlobalData.gModuleHash[self.Arch]:
- GlobalData.gModuleHash[self.Arch][self.Name] = m.hexdigest()
- if GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
- return False
- return SaveFileOnChange(ModuleHashFile, m.hexdigest(), False)
+ GlobalData.gModuleHash[self.Arch][self.Name] = m.hexdigest()
+
+ return
+ GlobalData.gModuleHash[self.Arch][self.Name].encode('utf-8')

## Decide whether we can skip the ModuleAutoGen process
def CanSkipbyHash(self):
+ # Hashing feature is off
+ if not GlobalData.gUseHashCache:
+ return False
+
+ # Initialize a dictionary for each arch type
+ if self.Arch not in GlobalData.gBuildHashSkipTracking:
+ GlobalData.gBuildHashSkipTracking[self.Arch] = dict()
+
# If library or Module is binary do not skip by hash
if self.IsBinaryModule:
return False
+
# .inc is contains binary information so do not skip by hash as well
for f_ext in self.SourceFileList:
if '.inc' in str(f_ext):
return False
- if GlobalData.gUseHashCache:
- # If there is a valid hash or function generated a valid hash; function will return False
- # and the statement below will return True
- return not self.GenModuleHash()
- return False
+
+ # Use Cache, if exists and if Module has a copy in cache
+ if GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
+ return True
+
+ # Early exit for libraries that haven't yet finished building
+ HashFile = path.join(self.BuildDir, self.Name + ".hash")
+ if self.IsLibrary and not os.path.exists(HashFile):
+ return False
+
+ # Return a Boolean based on if can skip by hash, either from memory or from IO.
+ if self.Name not in GlobalData.gBuildHashSkipTracking[self.Arch]:
+ # If hashes are the same, SaveFileOnChange() will return False.
+ GlobalData.gBuildHashSkipTracking[self.Arch][self.Name] = not SaveFileOnChange(HashFile, self.GenModuleHash(), False)
+ return GlobalData.gBuildHashSkipTracking[self.Arch][self.Name]
+ else:
+ return
+ GlobalData.gBuildHashSkipTracking[self.Arch][self.Name]

## Decide whether we can skip the ModuleAutoGen process
# If any source file is newer than the module than we cannot skip diff --git a/BaseTools/Source/Python/Common/GlobalData.py b/BaseTools/Source/Python/Common/GlobalData.py
index 79f23c892d..95e28a988f 100644
--- a/BaseTools/Source/Python/Common/GlobalData.py
+++ b/BaseTools/Source/Python/Common/GlobalData.py
@@ -112,3 +112,9 @@ gSikpAutoGenCache = set() # Dictionary for tracking Module build status as success or failure # False -> Fail : True -> Success gModuleBuildTracking = dict()
+
+# Dictionary of booleans that dictate whether a module or # library can
+be skiped
+# Top Dict: Key: Arch Type Value: Dictionary
+# Second Dict: Key: Module\Library Name Value: True\False
+gBuildHashSkipTracking = dict()
diff --git a/BaseTools/Source/Python/build/build.py b/BaseTools/Source/Python/build/build.py
index 7271570d29..97a4dc8ee2 100644
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -593,7 +593,7 @@ class BuildTask:
#
def AddDependency(self, Dependency):
for Dep in Dependency:
- if not Dep.BuildObject.IsBinaryModule:
+ if not Dep.BuildObject.IsBinaryModule and not Dep.BuildObject.CanSkipbyHash():
self.DependencyList.append(BuildTask.New(Dep)) # BuildTask list

## The thread wrapper of LaunchCommand function @@ -605,6 +605,11 @@ class BuildTask:
try:
self.BuildItem.BuildObject.BuildTime = LaunchCommand(Command, WorkingDir)
self.CompleteFlag = True
+
+ # Run hash operation post dependency, to account for libs
+ if GlobalData.gUseHashCache and self.BuildItem.BuildObject.IsLibrary:
+ HashFile = path.join(self.BuildItem.BuildObject.BuildDir, self.BuildItem.BuildObject.Name + ".hash")
+ SaveFileOnChange(HashFile,
+ self.BuildItem.BuildObject.GenModuleHash(), False)
except:
#
# TRICK: hide the output of threads left running, so that the user can
--
2.19.1.windows.1


[PATCH] UefiCpuPkg CpuCommonFeaturesLib: Remove CPU generation check

Zeng, Star
 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1679

The checking to CpuInfo->CpuIdVersionInfoEcx.Bits.AESNI is enough,
the checking to CPU generation could be removed, then the code
could be reused by more platforms.

Cc: Laszlo Ersek <lersek@...>
Cc: Eric Dong <eric.dong@...>
Cc: Ruiyu Ni <ruiyu.ni@...>
Cc: Chandana Kumar <chandana.c.kumar@...>
Signed-off-by: Star Zeng <star.zeng@...>
---
UefiCpuPkg/Library/CpuCommonFeaturesLib/Aesni.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/Aesni.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/Aesni.c
index b79446ba3ca9..4a56eec1b267 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/Aesni.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/Aesni.c
@@ -57,15 +57,9 @@ AesniSupport (
MSR_SANDY_BRIDGE_FEATURE_CONFIG_REGISTER *MsrFeatureConfig;

if (CpuInfo->CpuIdVersionInfoEcx.Bits.AESNI == 1) {
- if (IS_SANDY_BRIDGE_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) ||
- IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) ||
- IS_XEON_5600_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) ||
- IS_XEON_E7_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel) ||
- IS_XEON_PHI_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayModel)) {
- MsrFeatureConfig = (MSR_SANDY_BRIDGE_FEATURE_CONFIG_REGISTER *) ConfigData;
- ASSERT (MsrFeatureConfig != NULL);
- MsrFeatureConfig[ProcessorNumber].Uint64 = AsmReadMsr64 (MSR_SANDY_BRIDGE_FEATURE_CONFIG);
- }
+ MsrFeatureConfig = (MSR_SANDY_BRIDGE_FEATURE_CONFIG_REGISTER *) ConfigData;
+ ASSERT (MsrFeatureConfig != NULL);
+ MsrFeatureConfig[ProcessorNumber].Uint64 = AsmReadMsr64 (MSR_SANDY_BRIDGE_FEATURE_CONFIG);
return TRUE;
}
return FALSE;
--
2.21.0.windows.1


[PATCH v1 3/3] DynamicTablesPkg: Test for duplicate GT Block frame numbers

Krzysztof Koch
 

Check for duplicate frame numbers when populating the GT Block Timer
Frames inside the GTDT table generator.

Signed-off-by: Krzysztof Koch <krzysztof.koch@...>
---

Notes:
v1:
- Detect duplicate GT Frame Numbers in GTDT [Krzysztof]

DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c | 66 +++++++++++++++++++-
1 file changed, 64 insertions(+), 2 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
index 8d9ddcf9244b9f8b795edf7a53dd8a071bb121bc..d1ac9110cc5c8df8506b6db09cc362fdb0df8d76 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
@@ -179,6 +179,55 @@ AddGenericWatchdogList (
} // for
}

+/**
+ Function to test if two Generic Timer Block Frame Info structures have the
+ same frame number.
+
+ @param [in] Frame1 Pointer to the first GT Block Frame Info
+ structure.
+ @param [in] Frame2 Pointer to the second GT Block Frame Info
+ structure.
+ @param [in] Index1 Index of Frame1 in the shared GT Block Frame
+ Information List.
+ @param [in] Index2 Index of Frame2 in the shared GT Block Frame
+ Information List.
+
+ @retval TRUE Frame1 and Frame2 have the same frame number.
+ @return FALSE Frame1 and Frame2 have different frame numbers.
+
+**/
+BOOLEAN
+EFIAPI
+IsGtFrameNumberEqual (
+ IN CONST VOID * Frame1,
+ IN CONST VOID * Frame2,
+ IN UINTN Index1,
+ IN UINTN Index2
+ )
+{
+ UINT8 FrameNumber1;
+ UINT8 FrameNumber2;
+
+ ASSERT ((Frame1 != NULL) && (Frame2 != NULL));
+
+ FrameNumber1 = ((CM_ARM_GTBLOCK_TIMER_FRAME_INFO*)Frame1)->FrameNumber;
+ FrameNumber2 = ((CM_ARM_GTBLOCK_TIMER_FRAME_INFO*)Frame2)->FrameNumber;
+
+ if (FrameNumber1 == FrameNumber2) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: GTDT: GT Block Frame Info Structures %d and %d have the same " \
+ "frame number: 0x%x.\n",
+ Index1,
+ Index2,
+ FrameNumber1
+ ));
+ return TRUE;
+ }
+
+ return FALSE;
+}
+
/** Update the GT Block Timer Frame lists in the GTDT Table.

@param [in] GtBlockFrame Pointer to the GT Block Frames
@@ -187,8 +236,8 @@ AddGenericWatchdogList (
Information List.
@param [in] GTBlockFrameCount Number of GT Block Frames.

- @retval EFI_SUCCESS Table generated successfully.
- @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_SUCCESS Table generated successfully.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
**/
STATIC
EFI_STATUS
@@ -198,6 +247,8 @@ AddGTBlockTimerFrames (
IN UINT32 GTBlockFrameCount
)
{
+ BOOLEAN IsFrameNumberDuplicated;
+
ASSERT (GtBlockFrame != NULL);
ASSERT (GTBlockTimerFrameList != NULL);

@@ -211,6 +262,17 @@ AddGTBlockTimerFrames (
return EFI_INVALID_PARAMETER;
}

+ IsFrameNumberDuplicated = FindDuplicateValue (
+ GTBlockTimerFrameList,
+ GTBlockFrameCount,
+ sizeof (CM_ARM_GTBLOCK_TIMER_FRAME_INFO),
+ IsGtFrameNumberEqual
+ );
+ // Duplicate entry was found so timer frame numbers provided are invalid
+ if (IsFrameNumberDuplicated) {
+ return EFI_INVALID_PARAMETER;
+ }
+
while (GTBlockFrameCount-- != 0) {
DEBUG ((
DEBUG_INFO,
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[PATCH v1 2/3] DynamicTablesPkg: Test for duplicate UIDs in MADT generator

Krzysztof Koch
 

Check for duplicate ACPI Processor UIDs when populating the GIC CPU
(GICC) Interface structures inside the MADT table generator.

Signed-off-by: Krzysztof Koch <krzysztof.koch@...>
---

Notes:
v1:
- Detect duplicate ACPI Processor UIDs in GICCs [Krzysztof]

DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c | 90 ++++++++++++++++++--
1 file changed, 83 insertions(+), 7 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c
index ab9734fb31480f1b653227d1d56abf60bb04f98a..613bf665d9cecc6495f5ac84c2515ea89f476194 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c
@@ -140,28 +140,96 @@ AddGICC (
Gicc->Reserved2[2] = EFI_ACPI_RESERVED_BYTE;
}

+/**
+ Function to test if two GIC CPU Interface information structures have the
+ same ACPI Processor UID.
+
+ @param [in] GicCInfo1 Pointer to the first GICC info structure.
+ @param [in] GicCInfo2 Pointer to the second GICC info structure.
+ @param [in] Index1 Index of GicCInfo1 in the shared list of GIC
+ CPU Interface Info structures.
+ @param [in] Index2 Index of GicCInfo2 in the shared list of GIC
+ CPU Interface Info structures.
+
+ @retval TRUE GicCInfo1 and GicCInfo2 have the same UID.
+ @retval FALSE GicCInfo1 and GicCInfo2 have different UIDs.
+**/
+BOOLEAN
+EFIAPI
+IsAcpiUidEqual (
+ IN CONST VOID * GicCInfo1,
+ IN CONST VOID * GicCInfo2,
+ IN UINTN Index1,
+ IN UINTN Index2
+ )
+{
+ UINT32 Uid1;
+ UINT32 Uid2;
+
+ ASSERT ((GicCInfo1 != NULL) && (GicCInfo2 != NULL));
+
+ Uid1 = ((CM_ARM_GICC_INFO*)GicCInfo1)->AcpiProcessorUid;
+ Uid2 = ((CM_ARM_GICC_INFO*)GicCInfo2)->AcpiProcessorUid;
+
+ if (Uid1 == Uid2) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MADT: GICC Info Structures %d and %d have the same ACPI " \
+ "Processor UID: 0x%x.\n",
+ Index1,
+ Index2,
+ Uid1
+ ));
+ return TRUE;
+ }
+
+ return FALSE;
+}
+
/** Add the GIC CPU Interface Information to the MADT Table.

- @param [in] Gicc Pointer to GIC CPU Interface
- structure list.
- @param [in] GicCInfo Pointer to the GIC CPU
- Information list.
- @param [in] GicCCount Count of GIC CPU Interfaces.
+ This function also checks for duplicate ACPI Processor UIDs.
+
+ @param [in] Gicc Pointer to GIC CPU Interface structure list.
+ @param [in] GicCInfo Pointer to the GIC CPU Information list.
+ @param [in] GicCCount Count of GIC CPU Interfaces.
+
+ @retval EFI_SUCCESS GIC CPU Interface Information was added
+ successfully.
+ @retval EFI_INVALID_PARAMETER One or more invalid GIC CPU Info values were
+ provided and the generator failed to add the
+ information to the table.
**/
STATIC
-VOID
+EFI_STATUS
AddGICCList (
IN EFI_ACPI_6_2_GIC_STRUCTURE * Gicc,
IN CONST CM_ARM_GICC_INFO * GicCInfo,
IN UINT32 GicCCount
)
{
+ BOOLEAN IsAcpiProcUidDuplicated;
+
ASSERT (Gicc != NULL);
ASSERT (GicCInfo != NULL);

+ IsAcpiProcUidDuplicated = FindDuplicateValue (
+ GicCInfo,
+ GicCCount,
+ sizeof (CM_ARM_GICC_INFO),
+ IsAcpiUidEqual
+ );
+ // Duplicate ACPI Processor UID was found so the GICC info provided
+ // is invalid
+ if (IsAcpiProcUidDuplicated) {
+ return EFI_INVALID_PARAMETER;
+ }
+
while (GicCCount-- != 0) {
AddGICC (Gicc++, GicCInfo++);
}
+
+ return EFI_SUCCESS;
}

/** Update the GIC Distributor Information in the MADT Table.
@@ -577,11 +645,19 @@ BuildMadtTable (
goto error_handler;
}

- AddGICCList (
+ Status = AddGICCList (
(EFI_ACPI_6_2_GIC_STRUCTURE*)((UINT8*)Madt + GicCOffset),
GicCInfo,
GicCCount
);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: MADT: Failed to add GICC structures. Status = %r\n",
+ Status
+ ));
+ goto error_handler;
+ }

AddGICD (
(EFI_ACPI_6_2_GIC_DISTRIBUTOR_STRUCTURE*)((UINT8*)Madt + GicDOffset),
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[PATCH v1 1/3] DynamicTablesPkg: Add code for finding duplicate values in arrays

Krzysztof Koch
 

Added generic function for detecting duplicate values in an array.

Also defined a function prototype to test if two objects are equal.
The prototype is used as an argument to the 'FindDuplicateValues'
function.

Signed-off-by: Krzysztof Koch <krzysztof.koch@...>
---
Notes:
v1:
- Add generic code for duplicate detection [Krzysztof]

DynamicTablesPkg/Include/Library/TableHelperLib.h | 48 +++++++++++++++
DynamicTablesPkg/Library/Common/TableHelperLib/TableHelper.c | 64 ++++++++++++++++++++
2 files changed, 112 insertions(+)

diff --git a/DynamicTablesPkg/Include/Library/TableHelperLib.h b/DynamicTablesPkg/Include/Library/TableHelperLib.h
index 9c5b3835413f8768ef81ababa932c9f9be7ced82..e4a8dfa046bd97d89f0297ccad521f317bed5c36 100644
--- a/DynamicTablesPkg/Include/Library/TableHelperLib.h
+++ b/DynamicTablesPkg/Include/Library/TableHelperLib.h
@@ -4,6 +4,9 @@

SPDX-License-Identifier: BSD-2-Clause-Patent

+ @par Glossary:
+ - PFN - Pointer to a Function
+
**/

#ifndef TABLE_HELPER_LIB_H_
@@ -59,4 +62,49 @@ AddAcpiHeader (
IN CONST UINT32 Length
);

+/**
+ Function prototype for testing if two arbitrary objects are equal.
+
+ @param [in] Object1 Pointer to the first object to compare.
+ @param [in] Object2 Pointer to the second object to compare.
+ @param [in] Index1 Index of Object1. This value is optional and
+ can be ignored by the specified implementation.
+ @param [in] Index2 Index of Object2. This value is optional and
+ can be ignored by the specified implementation.
+
+ @retval TRUE Object1 and Object2 are equal.
+ @retval FALSE Object1 and Object2 are NOT equal.
+**/
+typedef
+BOOLEAN
+(EFIAPI *PFN_IS_EQUAL)(
+ IN CONST VOID * Object1,
+ IN CONST VOID * Object2,
+ IN UINTN Index1 OPTIONAL,
+ IN UINTN Index2 OPTIONAL
+ );
+
+/**
+ Test and report if a duplicate entry exists in the given array of comparable
+ elements.
+
+ @param [in] Array Array of elements to test for duplicates.
+ @param [in] Count Number of elements in Array.
+ @param [in] ElementSize Size of an element in bytes
+ @param [in] EqualTestFunction The function to call to check if any two
+ elements are equal.
+
+ @retval TRUE A duplicate element was found or one of
+ the input arguments is invalid.
+ @retval FALSE Every element in Array is unique.
+**/
+BOOLEAN
+EFIAPI
+FindDuplicateValue (
+ IN CONST VOID * Array,
+ IN CONST UINTN Count,
+ IN CONST UINTN ElementSize,
+ IN PFN_IS_EQUAL EqualTestFunction
+ );
+
#endif // TABLE_HELPER_LIB_H_
diff --git a/DynamicTablesPkg/Library/Common/TableHelperLib/TableHelper.c b/DynamicTablesPkg/Library/Common/TableHelperLib/TableHelper.c
index 3938302b6d770c5bed2bc6c55a51c96ea8316dff..fc6cf3b088da1f7ad89dd4356b414bede9e80575 100644
--- a/DynamicTablesPkg/Library/Common/TableHelperLib/TableHelper.c
+++ b/DynamicTablesPkg/Library/Common/TableHelperLib/TableHelper.c
@@ -13,6 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
// Module specific include files.
#include <AcpiTableGenerator.h>
#include <ConfigurationManagerObject.h>
+#include <Library/TableHelperLib.h>
#include <Protocol/ConfigurationManagerProtocol.h>

/** The GetCgfMgrInfo function gets the CM_STD_OBJ_CONFIGURATION_MANAGER_INFO
@@ -180,3 +181,66 @@ AddAcpiHeader (
error_handler:
return Status;
}
+
+/**
+ Test and report if a duplicate entry exists in the given array of comparable
+ elements.
+
+ @param [in] Array Array of elements to test for duplicates.
+ @param [in] Count Number of elements in Array.
+ @param [in] ElementSize Size of an element in bytes
+ @param [in] EqualTestFunction The function to call to check if any two
+ elements are equal.
+
+ @retval TRUE A duplicate element was found or one of
+ the input arguments is invalid.
+ @retval FALSE Every element in Array is unique.
+**/
+BOOLEAN
+EFIAPI
+FindDuplicateValue (
+ IN CONST VOID * Array,
+ IN CONST UINTN Count,
+ IN CONST UINTN ElementSize,
+ IN PFN_IS_EQUAL EqualTestFunction
+ )
+{
+ UINTN Index1;
+ UINTN Index2;
+ UINT8 * Element1;
+ UINT8 * Element2;
+
+ if (Array == NULL) {
+ DEBUG ((DEBUG_ERROR, "ERROR: FindDuplicateValues: Array is NULL.\n"));
+ return TRUE;
+ }
+
+ if (ElementSize == 0) {
+ DEBUG ((DEBUG_ERROR, "ERROR: FindDuplicateValues: ElementSize is 0.\n"));
+ return TRUE;
+ }
+
+ if (EqualTestFunction == NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: FindDuplicateValues: EqualTestFunction is NULL.\n"
+ ));
+ return TRUE;
+ }
+
+ if (Count < 2) {
+ return FALSE;
+ }
+
+ for (Index1 = 0; Index1 < Count - 1; Index1++) {
+ for (Index2 = Index1 + 1; Index2 < Count; Index2++) {
+ Element1 = (UINT8*)Array + (Index1 * ElementSize);
+ Element2 = (UINT8*)Array + (Index2 * ElementSize);
+
+ if (EqualTestFunction (Element1, Element2, Index1, Index2)) {
+ return TRUE;
+ }
+ }
+ }
+ return FALSE;
+}
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[PATCH v1 0/3] Detect duplicate field values in ACPI tables

Krzysztof Koch
 

This patch set introduces generic code for finding duplicate elements
in an array and uses it to validate two ACPI tables: MADT and GTDT.

This change is motivated by the need for certain ACPI table field
to have unique values across the entire table.

Changes can be seen at: https://github.com/KrzysztofKoch1/edk2/tree/479_find_duplicate_ids_v1


Krzysztof Koch (3):
DynamicTablesPkg: Add code for finding duplicate values in arrays
DynamicTablesPkg: Test for duplicate UIDs in MADT generator
DynamicTablesPkg: Test for duplicate GT Block frame numbers

DynamicTablesPkg/Include/Library/TableHelperLib.h | 48 +++++++++++
DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c | 66 +++++++++++++-
DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c | 90 ++++++++++++++++++--
DynamicTablesPkg/Library/Common/TableHelperLib/TableHelper.c | 64 ++++++++++++++
4 files changed, 259 insertions(+), 9 deletions(-)

--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[PATCH v2 1/1] ShellPkg: acpiview: Add GT Frame Number validation to GTDT parser

Krzysztof Koch
 

The ACPI 6.2 specification mandates that the Generic Timer (GT) Block
Timer Structures must have a frame number in the range 0-7.

Update the GTDT parser to warn if this condition is violated.

Signed-off-by: Krzysztof Koch <krzysztof.koch@...>
---

The changes can be seen at: https://github.com/KrzysztofKoch1/edk2/tree/528_acpiview_gt_frame_validate_v2

Notes:
v2:
- Change FrameNumber data type to UINT8 [Zhichao]
- Modify the patch based on upstream review [Krzysztof]

ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c | 47 +++++++++++++++++++-
1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
index f31c4a2761751c58d4b1d3eb75084e24ec318e7f..1b7e56486c8fb98a8fe063ae5fa25d86500a58a9 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
@@ -1,7 +1,7 @@
/** @file
GTDT table parser

- Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
+ Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent

@par Reference(s):
@@ -38,6 +38,21 @@ ValidateGtBlockTimerCount (
IN VOID* Context
);

+/**
+ This function validates the GT Frame Number.
+
+ @param [in] Ptr Pointer to the start of the field data.
+ @param [in] Context Pointer to context specific information e.g. this
+ could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidateGtFrameNumber (
+ IN UINT8* Ptr,
+ IN VOID* Context
+ );
+
/**
An ACPI_PARSER array describing the ACPI GTDT Table.
**/
@@ -92,7 +107,7 @@ STATIC CONST ACPI_PARSER GtBlockParser[] = {
An ACPI_PARSER array describing the GT Block timer.
**/
STATIC CONST ACPI_PARSER GtBlockTimerParser[] = {
- {L"Frame Number", 1, 0, L"%d", NULL, NULL, NULL, NULL},
+ {L"Frame Number", 1, 0, L"%d", NULL, NULL, ValidateGtFrameNumber, NULL},
{L"Reserved", 3, 1, L"%x %x %x", Dump3Chars, NULL, NULL, NULL},
{L"Physical address (CntBaseX)", 8, 4, L"0x%lx", NULL, NULL, NULL, NULL},
{L"Physical address (CntEL0BaseX)", 8, 12, L"0x%lx", NULL, NULL, NULL,
@@ -145,6 +160,34 @@ ValidateGtBlockTimerCount (
}
}

+/**
+ This function validates the GT Frame Number.
+
+ @param [in] Ptr Pointer to the start of the field data.
+ @param [in] Context Pointer to context specific information e.g. this
+ could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidateGtFrameNumber (
+ IN UINT8* Ptr,
+ IN VOID* Context
+ )
+{
+ UINT8 FrameNumber;
+
+ FrameNumber = *(UINT8*)Ptr;
+
+ if (FrameNumber > 7) {
+ IncrementErrorCount ();
+ Print (
+ L"\nERROR: GT Frame Number = %d. GT Frame Number must be in range 0-7.",
+ FrameNumber
+ );
+ }
+}
+
/**
This function parses the Platform GT Block.

--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[Patch] IntelFspPkg&IntelFspWrapperPkg: Remove them

Ni, Ray
 

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

Since there are V2 FSP packages (IntelFsp2Pkg, IntelFsp2WrapperPkg),
this patch removes IntelFspPkg, IntelFspWrapperPkg to remove
obsolete code in edk2 repo.

Signed-off-by: Ray Ni <ray.ni@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <@natedesimone>
Cc: Star Zeng <star.zeng@...>
--------------
NOTE: This patch is to completely remove IntelFspPkg and
IntelFspWrapperPkg folder in edk2 repo.
I don't want to flood people's inbox with a big patch just
removing every line of code in this two packages.

The patch that modifies Maintainers.txt will be sent out later
after IntelFspPkg and IntelFspWrapperPkg are removed.


[PATCH v2 4/4] EmulatorPkg: Update DSC/FDF to use NetworkPkg's include fragment file.

Zhang, Shenglei
 

From: Fu Siyuan <@sfu5>

This patch updates the platform DSC/FDF files to use the include fragment
files provided by NetworkPkg.
The feature enabling flags in [Defines] section have been updated to use
the NetworkPkg's terms, and the value has been overridden with the original
default value on this platform.

Cc: Jordan Justen <jordan.l.justen@...>
Cc: Andrew Fish <afish@...>
Cc: Ruiyu Ni <ruiyu.ni@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
Reviewed-by: Ray Ni <ray.ni@...>
---
EmulatorPkg/EmulatorPkg.dsc | 29 ++++++++++++-----------------
EmulatorPkg/EmulatorPkg.fdf | 10 +---------
2 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc
index de20f81046f1..ea8b6ce76e24 100644
--- a/EmulatorPkg/EmulatorPkg.dsc
+++ b/EmulatorPkg/EmulatorPkg.dsc
@@ -23,6 +23,16 @@ [Defines]
SKUID_IDENTIFIER = DEFAULT
FLASH_DEFINITION = EmulatorPkg/EmulatorPkg.fdf

+
+ #
+ # Network definition
+ #
+ DEFINE NETWORK_SNP_ENABLE = FALSE
+ DEFINE NETWORK_IP6_ENABLE = FALSE
+ DEFINE NETWORK_TLS_ENABLE = FALSE
+ DEFINE NETWORK_HTTP_BOOT_ENABLE = FALSE
+ DEFINE NETWORK_ISCSI_ENABLE = FALSE
+
[SkuIds]
0|DEFAULT

@@ -68,10 +78,6 @@ [LibraryClasses]
# Generic Modules
#
UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
- NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
- IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
- UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
- DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
@@ -357,19 +363,6 @@ [Components]

MdeModulePkg/Application/HelloWorld/HelloWorld.inf

- #
- # Network stack drivers
- #
- MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
- MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
- MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
- MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
- MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
- MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
- MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
- MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
- NetworkPkg/TcpDxe/TcpDxe.inf
-
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
@@ -412,6 +405,8 @@ [Components]

!endif

+!include NetworkPkg/Network.dsc.inc
+
[BuildOptions]
MSFT:DEBUG_*_*_CC_FLAGS = /Od /Oy-
MSFT:NOOPT_*_*_CC_FLAGS = /Od /Oy-
diff --git a/EmulatorPkg/EmulatorPkg.fdf b/EmulatorPkg/EmulatorPkg.fdf
index 94a060c99938..ec411e82b427 100644
--- a/EmulatorPkg/EmulatorPkg.fdf
+++ b/EmulatorPkg/EmulatorPkg.fdf
@@ -190,15 +190,7 @@ [FV.FvRecovery]
!if $(NETWORK_SUPPORT)
INF EmulatorPkg/EmuSnpDxe/EmuSnpDxe.inf
!endif
-INF MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-INF MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
-INF MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
-INF MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
-INF MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
-INF MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
-INF MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
-INF NetworkPkg/TcpDxe/TcpDxe.inf
-INF MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
+!include NetworkPkg/Network.fdf.inc

INF FatPkg/EnhancedFatDxe/Fat.inf

--
2.18.0.windows.1