Date   

[edk2-staging/UEFI_PCI_ENHANCE-2 PATCH 00/12] New PCI features - MPS, MRRS, RO, NS, CTO

Javeed, Ashraf
 

The EDK2 Kernel PciBusDxe driver is enhanced to enable the configuration=0D
of PCI features like=0D
(1) Max_Payload_Size=0D
(2) Max_Read_Req_Size=0D
(3) Relax Ordering=0D
(4) No-Snoop=0D
(5) Completion Timeout=0D
=0D
Max_Payload_Size:- The PCI Device Control register provides this feature=0D
register field which controls the maximum data packet (TLP) size that a=0D
PCI device should maintain as a requester. The PCI Bus driver is required=0D
to maintain a highest common value supported by all the PCI devices in a=0D
PCIe hierarchy, especially in case of isochronous applications.=0D
=0D
Max_Read_Req_Size:- The PCI Device Control register provides this feature=0D
register field which controls the maximum memory read request size that a=0D
PCI device should maintain as a requester. The PCI Bus driver is required=0D
to maintain a common value, same as Max_Payload_Size, in case of=0D
isochronous applications only; or else, it should maintain the user=0D
requested value uniformly in a PCIe hierarchy (PCI root port and its=0D
downstream devices).=0D
=0D
Relax Ordering:- The PCI Device Control register has the enabling of=0D
Relax Ordering functionality register field (bit 4). If this bit is Set,=0D
the PCI Function is permitted to set the Relaxed Ordering bit in the=0D
Attributes field of transactions it initiates that do not require strong=0D
write ordering (see PCI Base Specification 4, Section 2.2.6.4 and Sect-=0D
ion 2.4). Any supporting PCI function is expected have this bit enabled=0D
by as per its hardware default; the code enhancement is to enable / dis-=0D
able as per the PCI device policy provided by the platform firmware. If=0D
no device policy override is provided than it shall be ignored by the PCI=0D
Bus driver for that PCI function.=0D
=0D
No-Snoop:- The PCI Device Control register has the enabling of No-Snoop=0D
functionality register field (bit 11). If this bit is Set, the PCI=0D
Function is permitted to Set the No Snoop bit in the Requester Attributes=0D
of transactions it initiates that do not require hardware enforced cache=0D
coherency (see PCI Base Specification 4, Section 2.2.6.5). Any supporting=0D
PCI function is expected have this bit enabled by as per its hardware=0D
default; the code enhancement is to enable / disable as per the PCI=0D
device policy provided by the platform firmware. If no device policy=0D
override is provided than it shall be ignored by the PCI Bus driver for=0D
that PCI function.=0D
=0D
Completion Timeout:- The PCI Device Control 2 register provides two=0D
register fields based on its Device Capability 2 register; the CTO Ranges=0D
(bits [3:0]) and the disabling of CTO detection mechanism (bit 4). The=0D
software is permitted to change the CTO ranges and enable/disable the CTO=0D
detection mechanism any time. The code enhancement here is to override=0D
these register fields as per the platform device policy. If no device=0D
policy override is provided than it shall be ignored by the PCI Bus=0D
driver for that PCI function.=0D
=0D
The PCI Base Specification 4 Revision 1 contains detailed information=0D
about these features. The EDK2 PCI Bus driver needs to enable the=0D
configuration of these features as per the PCI Base specification.=0D
=0D
The EDK2 PCI Bus driver also needs to take the PCI device-specific=0D
platform policy into the consideration while programming these features;=0D
thus the code changes to support these, is explicitly dependent on the=0D
new PCI Platform Protocol interface definition defined in the below=0D
record:-=0D
https://bugzilla.tianocore.org/show_bug.cgi?id=3D1954=0D
=0D
=0D
Signed-off-by: Ashraf Javeed <ashraf.javeed@...>=0D
Cc: Jian J Wang <jian.j.wang@...>=0D
Cc: Hao A Wu <hao.a.wu@...>=0D
Cc: Ray Ni <ray.ni@...>=0D
---

Ashraf Javeed (12):
MdeModulePkg/PciBusDxe:New PCI features separation with PCD
PciBusDxe: Reorganize the PCI Platform Protocol usage code
PciBusDxe: Separation of the PCI device registration and start
PciBusDxe: Inclusion of new PCI Platform Protocol 2
PciBusDxe: Setup sub-phases for PCI feature enumeration
PciBusDxe: Integration of setup for PCI feature enumeration
PciBusDxe: Record the PCI-Express Capability Structure
PciBusDxe: New PCI feature Max_Payload_Size
PciBusDxe: New PCI feature Max_Read_Req_Size
PciBusDxe: New PCI feature Relax Ordering
PciBusDxe: New PCI feature No-Snoop
PciBusDxe: New PCI feature Completion Timeout

MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c | 23 +----------
MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 20 ++++++++--
MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 9 ++++-
MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c | 233 +++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------=
--------------------
MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c | 139 +++++++++++++=
+++-------------------------------------------------
MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 34 ++++++++++---=
---
MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c | 2030 +++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
++++++++++++++++++++++++
MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h | 223 +++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++
MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c | 749 +++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++++++
MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.h | 191 +++++++++++++=
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c | 15 +------
MdeModulePkg/MdeModulePkg.dec | 22 +++++++++++
12 files changed, 3450 insertions(+), 238 deletions(-)
create mode 100644 MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.c
create mode 100644 MdeModulePkg/Bus/Pci/PciBusDxe/PciFeatureSupport.h
create mode 100644 MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.c
create mode 100644 MdeModulePkg/Bus/Pci/PciBusDxe/PciPlatformSupport.h

--=20
2.21.0.windows.1


Re: Question regarding MMIO address space exposed from GetMemoryMap

Jon Nettleton
 

On Fri, Nov 1, 2019 at 6:57 AM Jon Nettleton via Groups.Io
<jon=solid-run.com@groups.io> wrote:

On Thu, Oct 31, 2019 at 5:44 PM Andrew Fish via Groups.Io
<afish=apple.com@groups.io> wrote:

Jon,

Its a little confusing but gBS->GetMemoryMap () only returns information about DRAM and any address that requires a kernel virtual address mapping in EFI. The OS calls EFI Runtime Services from a kernel virtual mapping so the memory map is also involved in the hand shake to communicate the virtual address mappings to EFI. Thus any MMIO region in the EFI Memory Map should always have the EFI_MEMORY_RUNTIME attribute set. The most common MMIO region to be mapped is the NOR FLASH to support the EFI Runtime Variable services.

On a UEFI system MMIO regions are described to the OS via ACPI. Processing the ACPI tables requires an interpreter, and source to an interpreter is available at the ACPI CA site [1].

The PI (Platform Initialization) Spec has a concept called the Global Coherency Domain (GCD) [2]. The GCD is the map for who owns the CPU address and IO (x86 in/out instructions not MMIO) space. You can dump the GCD info via gDS->GetMemorySpaceMap(). The idea is the DRAM controller would carve out a EfiGcdMemoryTypeSystemMemory range, and the PCI Host Bridge would carve out a EfiGcdMemoryTypeMemoryMappedIo range, etc.

Note: A UEFI implementation is not required to be constructed out of PI Spec components, but the EDKII UEFI implementation is constructed using the PI Specification.

[1] https://acpica.org/downloads/uefi-support
[2] Sorry I could not make up a better name at the time.

Thanks,

Andrew Fish

On Oct 31, 2019, at 1:32 AM, Jon Nettleton <jon@...> wrote:

I am working on sorting out a failure on test 605 of the SBSA test.
The test is "Where a memory access is to an unpopulated part of the
addressable memory space, accesses must be terminated in a manner that
is presented to the PE as either a precise Data Abort or that causes a
system error interrupt or an SPI or LPI interrupt to be delivered to
the GIC." The issue is that the random test address that was chosen
0x04200000 falls directly in the middle of the Qoriq configuration,
control, and status register (CCSR) address space. This is an area in
the memory space that provides CPU access to the device registers.

An entry is added for this region in the VirtualMemoryMap, and
registered with the attribute, ARM_MEMORY_REGION_ATTRIBUTE_DEVICE.
However only a handful of devices are being registered so only a
couple of ranges are showing up in memmap as MMIO.

The SBSA test in question gets the memorymap and starts at the address
mentioned above and looks for the first chunk of memory that is free
and then attempts to access the memory and is looking for a Data
Abort. Of course a data abort is not generated because 0x04200000 is
a valid address. If you offset this to 0x42000000 then a DA is
generated as expected and the test passes.

There is a very old thread started by Ard, "MMIO regions in
GetMemoryMap ()", in which he questions if all the MMIO regions should
be reported by GetMemoryMap() or only the ones being used by
initialized devices, which is what the current implementation does.
The end result of the thread was the current implementation is
correct. That leaves me with the question, "What is the proper
solution for the current implementation that I am working with?"

To me it seems like I need a special Library that on boot goes through
and claims and maps every valid MMIO slot in this region, and then
have the drivers use this library for proxying requests to
gDS->AddMemorySpace (), gDS->SetMemorySpaceAttributes () etc. If
there is a standardized way to deal with this configuration I would
much rather follow that.

Thanks for any input,
Jon




Andrew,

Thanks for clearing this up. I definitely understand the
relationships much better now. I am still uncertain of the proper way
to fix this issue regarding the SBSA peripheral test. The test is
using gBS->GetMemoryMap in order to look for available memory
segments, which according to the earlier thread and your confirmation
"gBS->GetMemoryMap () only returns information about DRAM and any
address that requires a kernel virtual address mapping in EFI.", which
means that this test will never work on this platform because the
address that was randomly chosen just happens to fall right into the
address range for the device addresses that are valid so won't produce
a DA, but aren't being used by EFI therefore don't require a virtual
address mapping in EFI.

Is this something I should bring up with the SBSA group?

-Jon
I have followed up with ARM and they agree it is an issue with the
test and not a configuration error. They will provide a waiver and
look into refining the test in the future.

Thanks for your help.


Re: [PATCH v4] CryptoPkg: Upgrade OpenSSL to 1.1.1d

Wang, Jian J
 

Hi Laszlo,

I did simple ovmf boot tests (shell, linux, windows) and all passed. Let me know if you have
any comments or want to do more tests against v4 before check in.

Based on my review and tests,
Reviewed-by: Jian J Wang <jian.j.wang@...>

Regards,
Jian

-----Original Message-----
From: Zhang, Shenglei <shenglei.zhang@...>
Sent: Friday, November 01, 2019 2:56 PM
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...>;
Gao, Liming <liming.gao@...>
Subject: [PATCH v4] CryptoPkg: Upgrade OpenSSL to 1.1.1d

Update openssl from 1.1.1b to 1.1.1d.
Something needs to be noticed is that, there is a bug existing in the
released 1_1_1d version(894da2fb7ed5d314ee5c2fc9fd2d9b8b74111596),
which causes build failure. So we switch the code base to a usable
version, which is 2 commits later than the stable tag.
Now we use the version c3656cc594daac8167721dde7220f0e59ae146fc.
This log is to fix the build failure.
https://bugzilla.tianocore.org/show_bug.cgi?id=2226

Besides, the absense of "DSO_NONE" in dso_conf.h causes build failure
in OvmfPkg. So update process_files.pl to generate information from
"crypto/include/internal/dso_conf.h.in".

shm.h and utsname.h are added to avoid GCC build failure.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Xiaoyu Lu <xiaoyux.lu@...>
Cc: Liming Gao <liming.gao@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
---
v2: Revert the changes in OpensslLib.inf and OpensslLibCrypto.inf.
The removed header files could be auto-generated by process_files.pl now.

v3: Add display information for dso_conf.h.

v4: Add shm.h and utsname.h to avoid GCC build failure.

CryptoPkg/Library/Include/internal/dso_conf.h | 16 ++++++++++++++++
CryptoPkg/Library/Include/sys/shm.h | 9 +++++++++
CryptoPkg/Library/Include/sys/utsname.h | 10 ++++++++++
CryptoPkg/Library/OpensslLib/openssl | 2 +-
CryptoPkg/Library/OpensslLib/process_files.pl | 15 ++++++++++++++-
5 files changed, 50 insertions(+), 2 deletions(-)
create mode 100644 CryptoPkg/Library/Include/sys/shm.h
create mode 100644 CryptoPkg/Library/Include/sys/utsname.h

diff --git a/CryptoPkg/Library/Include/internal/dso_conf.h
b/CryptoPkg/Library/Include/internal/dso_conf.h
index e69de29bb2d1..43c891588bc2 100644
--- a/CryptoPkg/Library/Include/internal/dso_conf.h
+++ b/CryptoPkg/Library/Include/internal/dso_conf.h
@@ -0,0 +1,16 @@
+/* WARNING: do not edit! */
+/* Generated from crypto/include/internal/dso_conf.h.in */
+/*
+ * Copyright 2016-2019 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the OpenSSL license (the "License"). You may not use
+ * this file except in compliance with the License. You can obtain a copy
+ * in the file LICENSE in the source distribution or at
+ * https://www.openssl.org/source/license.html
+ */
+
+#ifndef HEADER_DSO_CONF_H
+# define HEADER_DSO_CONF_H
+# define DSO_NONE
+# define DSO_EXTENSION ".so"
+#endif
diff --git a/CryptoPkg/Library/Include/sys/shm.h
b/CryptoPkg/Library/Include/sys/shm.h
new file mode 100644
index 000000000000..dc0b8e81c8b0
--- /dev/null
+++ b/CryptoPkg/Library/Include/sys/shm.h
@@ -0,0 +1,9 @@
+/** @file
+ Include file to support building the third-party cryptographic library.
+
+Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <CrtLibSupport.h>
diff --git a/CryptoPkg/Library/Include/sys/utsname.h
b/CryptoPkg/Library/Include/sys/utsname.h
new file mode 100644
index 000000000000..75955b0a4eb6
--- /dev/null
+++ b/CryptoPkg/Library/Include/sys/utsname.h
@@ -0,0 +1,10 @@
+/** @file
+ Include file to support building the third-party cryptographic library.
+
+Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <CrtLibSupport.h>
+
diff --git a/CryptoPkg/Library/OpensslLib/openssl
b/CryptoPkg/Library/OpensslLib/openssl
index 50eaac9f3337..c3656cc594da 160000
--- a/CryptoPkg/Library/OpensslLib/openssl
+++ b/CryptoPkg/Library/OpensslLib/openssl
@@ -1 +1 @@
-Subproject commit 50eaac9f3337667259de725451f201e784599687
+Subproject commit c3656cc594daac8167721dde7220f0e59ae146fc
diff --git a/CryptoPkg/Library/OpensslLib/process_files.pl
b/CryptoPkg/Library/OpensslLib/process_files.pl
index 4fe54cd808a5..dd93bd84da22 100755
--- a/CryptoPkg/Library/OpensslLib/process_files.pl
+++ b/CryptoPkg/Library/OpensslLib/process_files.pl
@@ -106,6 +106,14 @@ BEGIN {
) == 0 ||
die "Failed to generate opensslconf.h!\n";

+ # Generate dso_conf.h per config data
+ system(
+ "perl -I. -Mconfigdata util/dofile.pl " .
+ "crypto/include/internal/dso_conf.h.in " .
+ "> include/internal/dso_conf.h"
+ ) == 0 ||
+ die "Failed to generate dso_conf.h!\n";
+
chdir($basedir) ||
die "Cannot change to base directory \"" . $basedir . "\"";

@@ -249,12 +257,17 @@ rename( $new_inf_file, $inf_file ) ||
print "Done!";

#
-# Copy opensslconf.h generated from OpenSSL Configuration
+# Copy opensslconf.h and dso_conf.h generated from OpenSSL Configuration
#
print "\n--> Duplicating opensslconf.h into Include/openssl ... ";
copy($OPENSSL_PATH . "/include/openssl/opensslconf.h",
$OPENSSL_PATH . "/../../Include/openssl/") ||
die "Cannot copy opensslconf.h!";
+print "Done!";
+print "\n--> Duplicating dso_conf.h into Include/internal ... ";
+copy($OPENSSL_PATH . "/include/internal/dso_conf.h",
+ $OPENSSL_PATH . "/../../Include/internal/") ||
+ die "Cannot copy dso_conf.h!";
print "Done!\n";

print "\nProcessing Files Done!\n";
--
2.18.0.windows.1


[PATCH v4] CryptoPkg: Upgrade OpenSSL to 1.1.1d

Zhang, Shenglei
 

Update openssl from 1.1.1b to 1.1.1d.
Something needs to be noticed is that, there is a bug existing in the
released 1_1_1d version(894da2fb7ed5d314ee5c2fc9fd2d9b8b74111596),
which causes build failure. So we switch the code base to a usable
version, which is 2 commits later than the stable tag.
Now we use the version c3656cc594daac8167721dde7220f0e59ae146fc.
This log is to fix the build failure.
https://bugzilla.tianocore.org/show_bug.cgi?id=2226

Besides, the absense of "DSO_NONE" in dso_conf.h causes build failure
in OvmfPkg. So update process_files.pl to generate information from
"crypto/include/internal/dso_conf.h.in".

shm.h and utsname.h are added to avoid GCC build failure.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Xiaoyu Lu <xiaoyux.lu@...>
Cc: Liming Gao <liming.gao@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
---
v2: Revert the changes in OpensslLib.inf and OpensslLibCrypto.inf.
The removed header files could be auto-generated by process_files.pl now.

v3: Add display information for dso_conf.h.

v4: Add shm.h and utsname.h to avoid GCC build failure.

CryptoPkg/Library/Include/internal/dso_conf.h | 16 ++++++++++++++++
CryptoPkg/Library/Include/sys/shm.h | 9 +++++++++
CryptoPkg/Library/Include/sys/utsname.h | 10 ++++++++++
CryptoPkg/Library/OpensslLib/openssl | 2 +-
CryptoPkg/Library/OpensslLib/process_files.pl | 15 ++++++++++++++-
5 files changed, 50 insertions(+), 2 deletions(-)
create mode 100644 CryptoPkg/Library/Include/sys/shm.h
create mode 100644 CryptoPkg/Library/Include/sys/utsname.h

diff --git a/CryptoPkg/Library/Include/internal/dso_conf.h b/CryptoPkg/Library/Include/internal/dso_conf.h
index e69de29bb2d1..43c891588bc2 100644
--- a/CryptoPkg/Library/Include/internal/dso_conf.h
+++ b/CryptoPkg/Library/Include/internal/dso_conf.h
@@ -0,0 +1,16 @@
+/* WARNING: do not edit! */
+/* Generated from crypto/include/internal/dso_conf.h.in */
+/*
+ * Copyright 2016-2019 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the OpenSSL license (the "License"). You may not use
+ * this file except in compliance with the License. You can obtain a copy
+ * in the file LICENSE in the source distribution or at
+ * https://www.openssl.org/source/license.html
+ */
+
+#ifndef HEADER_DSO_CONF_H
+# define HEADER_DSO_CONF_H
+# define DSO_NONE
+# define DSO_EXTENSION ".so"
+#endif
diff --git a/CryptoPkg/Library/Include/sys/shm.h b/CryptoPkg/Library/Include/sys/shm.h
new file mode 100644
index 000000000000..dc0b8e81c8b0
--- /dev/null
+++ b/CryptoPkg/Library/Include/sys/shm.h
@@ -0,0 +1,9 @@
+/** @file
+ Include file to support building the third-party cryptographic library.
+
+Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <CrtLibSupport.h>
diff --git a/CryptoPkg/Library/Include/sys/utsname.h b/CryptoPkg/Library/Include/sys/utsname.h
new file mode 100644
index 000000000000..75955b0a4eb6
--- /dev/null
+++ b/CryptoPkg/Library/Include/sys/utsname.h
@@ -0,0 +1,10 @@
+/** @file
+ Include file to support building the third-party cryptographic library.
+
+Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <CrtLibSupport.h>
+
diff --git a/CryptoPkg/Library/OpensslLib/openssl b/CryptoPkg/Library/OpensslLib/openssl
index 50eaac9f3337..c3656cc594da 160000
--- a/CryptoPkg/Library/OpensslLib/openssl
+++ b/CryptoPkg/Library/OpensslLib/openssl
@@ -1 +1 @@
-Subproject commit 50eaac9f3337667259de725451f201e784599687
+Subproject commit c3656cc594daac8167721dde7220f0e59ae146fc
diff --git a/CryptoPkg/Library/OpensslLib/process_files.pl b/CryptoPkg/Library/OpensslLib/process_files.pl
index 4fe54cd808a5..dd93bd84da22 100755
--- a/CryptoPkg/Library/OpensslLib/process_files.pl
+++ b/CryptoPkg/Library/OpensslLib/process_files.pl
@@ -106,6 +106,14 @@ BEGIN {
) == 0 ||
die "Failed to generate opensslconf.h!\n";

+ # Generate dso_conf.h per config data
+ system(
+ "perl -I. -Mconfigdata util/dofile.pl " .
+ "crypto/include/internal/dso_conf.h.in " .
+ "> include/internal/dso_conf.h"
+ ) == 0 ||
+ die "Failed to generate dso_conf.h!\n";
+
chdir($basedir) ||
die "Cannot change to base directory \"" . $basedir . "\"";

@@ -249,12 +257,17 @@ rename( $new_inf_file, $inf_file ) ||
print "Done!";

#
-# Copy opensslconf.h generated from OpenSSL Configuration
+# Copy opensslconf.h and dso_conf.h generated from OpenSSL Configuration
#
print "\n--> Duplicating opensslconf.h into Include/openssl ... ";
copy($OPENSSL_PATH . "/include/openssl/opensslconf.h",
$OPENSSL_PATH . "/../../Include/openssl/") ||
die "Cannot copy opensslconf.h!";
+print "Done!";
+print "\n--> Duplicating dso_conf.h into Include/internal ... ";
+copy($OPENSSL_PATH . "/include/internal/dso_conf.h",
+ $OPENSSL_PATH . "/../../Include/internal/") ||
+ die "Cannot copy dso_conf.h!";
print "Done!\n";

print "\nProcessing Files Done!\n";
--
2.18.0.windows.1


[PATCH 2/2] BaseTools: Add support for parseing map files generated by CLANG9 in GenFv

Zhiguang Liu
 

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

Add support for parseing map files generated by CLANG9 in GenFv

Cc: Bob Feng <bob.c.feng@...>
Cc: Liming Gao <liming.gao@...>

Signed-off-by: Zhiguang Liu <zhiguang.liu@...>
---
BaseTools/Source/C/GenFv/GenFvInternalLib.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
index 908740de50..daebfe894c 100644
--- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
+++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
@@ -791,6 +791,7 @@ Returns:
FILE *PeMapFile;
CHAR8 Line [MAX_LINE_LEN];
CHAR8 KeyWord [MAX_LINE_LEN];
+ CHAR8 KeyWord2 [MAX_LINE_LEN];
CHAR8 FunctionName [MAX_LINE_LEN];
EFI_PHYSICAL_ADDRESS FunctionAddress;
UINT32 FunctionType;
@@ -805,6 +806,7 @@ Returns:
UINT32 TextVirtualAddress;
UINT32 DataVirtualAddress;
EFI_PHYSICAL_ADDRESS LinkTimeBaseAddress;
+ BOOLEAN IsUseClang;

//
// Init local variable
@@ -932,6 +934,7 @@ Returns:
// Output Functions information into Fv Map file
//
LinkTimeBaseAddress = 0;
+ IsUseClang = FALSE;
while (fgets (Line, MAX_LINE_LEN, PeMapFile) != NULL) {
//
// Skip blank line
@@ -946,6 +949,12 @@ Returns:
if (FunctionType == 0) {
sscanf (Line, "%s", KeyWord);
if (stricmp (KeyWord, "Address") == 0) {
+ sscanf (Line, "%s %s", KeyWord, KeyWord2);
+ if (stricmp (KeyWord2, "Size") == 0) {
+ IsUseClang = TRUE;
+ FunctionType = 1;
+ continue;
+ }
//
// function list
//
@@ -967,11 +976,20 @@ Returns:
// Printf Function Information
//
if (FunctionType == 1) {
- sscanf (Line, "%s %s %llx %s", KeyWord, FunctionName, &TempLongAddress, FunctionTypeName);
- FunctionAddress = (UINT64) TempLongAddress;
- if (FunctionTypeName [1] == '\0' && (FunctionTypeName [0] == 'f' || FunctionTypeName [0] == 'F')) {
- fprintf (FvMapFile, " 0x%010llx ", (unsigned long long) (ImageBaseAddress + FunctionAddress - LinkTimeBaseAddress));
- fprintf (FvMapFile, "%s\n", FunctionName);
+ if (IsUseClang) {
+ sscanf (Line, "%llx %s %s %s", &TempLongAddress, KeyWord, KeyWord2, FunctionTypeName);
+ FunctionAddress = (UINT64) TempLongAddress;
+ if (FunctionTypeName [0] == '_' ) {
+ fprintf (FvMapFile, " 0x%010llx ", (unsigned long long) (ImageBaseAddress + FunctionAddress - LinkTimeBaseAddress));
+ fprintf (FvMapFile, "%s\n", FunctionTypeName);
+ }
+ } else {
+ sscanf (Line, "%s %s %llx %s", KeyWord, FunctionName, &TempLongAddress, FunctionTypeName);
+ FunctionAddress = (UINT64) TempLongAddress;
+ if (FunctionTypeName [1] == '\0' && (FunctionTypeName [0] == 'f' || FunctionTypeName [0] == 'F')) {
+ fprintf (FvMapFile, " 0x%010llx ", (unsigned long long) (ImageBaseAddress + FunctionAddress - LinkTimeBaseAddress));
+ fprintf (FvMapFile, "%s\n", FunctionName);
+ }
}
} else if (FunctionType == 2) {
sscanf (Line, "%s %s %llx %s", KeyWord, FunctionName, &TempLongAddress, FunctionTypeName);
--
2.16.2.windows.1


[PATCH 1/2] BaseTools: Add map file parsing support for CLANG9

Zhiguang Liu
 

Cc: Bob Feng <bob.c.feng@...>
Cc: Liming Gao <liming.gao@...>

Signed-off-by: Zhiguang Liu <zhiguang.liu@...>
---
BaseTools/Source/Python/Common/Misc.py | 9 ++++++---
BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py | 9 ++++++---
2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/BaseTools/Source/Python/Common/Misc.py b/BaseTools/Source/Python/Common/Misc.py
index 714eb840ea..403715e6cc 100755
--- a/BaseTools/Source/Python/Common/Misc.py
+++ b/BaseTools/Source/Python/Common/Misc.py
@@ -81,19 +81,22 @@ def GetVariableOffset(mapfilepath, efifilepath, varnames):

if len(lines) == 0: return None
firstline = lines[0].strip()
+ if re.match('^\s*Address\s*Size\s*Align\s*Out\s*In\s*Symbol\s*$', firstline):
+ return _parseForXcodeAndClang9(lines, efifilepath, varnames)
if (firstline.startswith("Archive member included ") and
firstline.endswith(" file (symbol)")):
return _parseForGCC(lines, efifilepath, varnames)
if firstline.startswith("# Path:"):
- return _parseForXcode(lines, efifilepath, varnames)
+ return _parseForXcodeAndClang9(lines, efifilepath, varnames)
return _parseGeneral(lines, efifilepath, varnames)

-def _parseForXcode(lines, efifilepath, varnames):
+def _parseForXcodeAndClang9(lines, efifilepath, varnames):
status = 0
ret = []
for line in lines:
line = line.strip()
- if status == 0 and line == "# Symbols:":
+ if status == 0 and (re.match('^\s*Address\s*Size\s*Align\s*Out\s*In\s*Symbol\s*$', line) \
+ or line == "# Symbols:"):
status = 1
continue
if status == 1 and len(line) != 0:
diff --git a/BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py b/BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py
index 7d5e4fc34a..d962ab0add 100644
--- a/BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py
+++ b/BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py
@@ -49,20 +49,23 @@ def parsePcdInfoFromMapFile(mapfilepath, efifilepath):

if len(lines) == 0: return None
firstline = lines[0].strip()
+ if re.match('^\s*Address\s*Size\s*Align\s*Out\s*In\s*Symbol\s*$', firstline):
+ return _parseForXcodeAndClang9(lines, efifilepath)
if (firstline.startswith("Archive member included ") and
firstline.endswith(" file (symbol)")):
return _parseForGCC(lines, efifilepath)
if firstline.startswith("# Path:"):
- return _parseForXcode(lines, efifilepath)
+ return _parseForXcodeAndClang9(lines, efifilepath)
return _parseGeneral(lines, efifilepath)

-def _parseForXcode(lines, efifilepath):
+def _parseForXcodeAndClang9(lines, efifilepath):
valuePattern = re.compile('^([\da-fA-FxX]+)([\s\S]*)([_]*_gPcd_BinaryPatch_([\w]+))')
status = 0
pcds = []
for line in lines:
line = line.strip()
- if status == 0 and line == "# Symbols:":
+ if status == 0 and (re.match('^\s*Address\s*Size\s*Align\s*Out\s*In\s*Symbol\s*$', line) \
+ or line == "# Symbols:"):
status = 1
continue
if status == 1 and len(line) != 0:
--
2.16.2.windows.1


[PATCH v3 4/4] MdeModulePkg/SetupBrowserDxe: ASSERT GetBufferForValue(&Value)

Zhang, Shenglei
 

Before called by GetBufferForValue(), Value has already been called
function IsTypeInBuffer to make sure the value must be buffer type.
So GetBufferForValue can not return NULL.
This commit adds ASSERT to assume (GetBufferForValue (&Value) is not
NULL.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
---

v3: Add ASSERT instead of using error handling.

MdeModulePkg/Universal/SetupBrowserDxe/Expression.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Expression.c b/MdeModulePkg/Universal/SetupBrowserDxe/Expression.c
index 7f4929c2fcd9..138912e00823 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/Expression.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/Expression.c
@@ -1281,7 +1281,10 @@ IfrToUint (
Result->Type = EFI_IFR_TYPE_UNDEFINED;
return EFI_SUCCESS;
}
+
+ ASSERT ((GetBufferForValue (&Value) != NULL);
Result->Value.u64 = *(UINT64*) GetBufferForValue (&Value);
+
if (Value.Type == EFI_IFR_TYPE_BUFFER) {
FreePool (Value.Buffer);
}
--
2.18.0.windows.1


[PATCH v3 3/4] MdeModulePkg/EsrtDxe: Add check for EsrtRepository

Zhang, Shenglei
 

EsrtRepository might be NULL. So return EFI_OUT_OF_RESOURCES
when it is NULL.

Cc: Hao A Wu <hao.a.wu@...>
Cc: Liming Gao <liming.gao@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
Reviewed-by: Hao A Wu <hao.a.wu@...>
---
MdeModulePkg/Universal/EsrtDxe/EsrtImpl.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/MdeModulePkg/Universal/EsrtDxe/EsrtImpl.c b/MdeModulePkg/Universal/EsrtDxe/EsrtImpl.c
index f48125382dbc..fff17b98fa3d 100644
--- a/MdeModulePkg/Universal/EsrtDxe/EsrtImpl.c
+++ b/MdeModulePkg/Universal/EsrtDxe/EsrtImpl.c
@@ -239,6 +239,11 @@ DeleteEsrtEntry(
goto EXIT;
}

+ if (EsrtRepository == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto EXIT;
+ }
+
if ((RepositorySize % sizeof(EFI_SYSTEM_RESOURCE_ENTRY)) != 0) {
DEBUG((EFI_D_ERROR, "Repository Corrupt. Need to rebuild Repository.\n"));
//
@@ -332,6 +337,11 @@ UpdateEsrtEntry(
&RepositorySize
);

+ if (EsrtRepository == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto EXIT;
+ }
+
if (!EFI_ERROR(Status)) {
//
// if exist, update Esrt cache repository
--
2.18.0.windows.1


[PATCH v3 2/4] MdeModulePkg/HiiDatabaseDxe: ASSERT StringPtr

Zhang, Shenglei
 

The caller of CompareAndMergeDefaultString has checked that
AltCfgResp must contain AltConfigHdr. So we add ASSERT to assume
StringPtr is not NULL.

Cc: Dandan Bi <dandan.bi@...>
Cc: Eric Dong <eric.dong@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
Reviewed-by: Dandan Bi <dandan.bi@...>
---
MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
index 71ea25bc19bf..2cad6d29f455 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
@@ -909,6 +909,7 @@ CompareAndMergeDefaultString (
// To find the <AltResp> with AltConfigHdr in AltCfgResp, ignore other <AltResp> which follow it.
//
StringPtr = StrStr (*AltCfgResp, AltConfigHdr);
+ ASSERT (StringPtr != NULL);
StringPtrNext = StrStr (StringPtr + 1, L"&GUID");
if (StringPtrNext != NULL) {
TempCharA = *StringPtrNext;
--
2.18.0.windows.1


[PATCH v3 1/4] MdeModulePkg/EbcDebugger: Add check for Entry and RetEntry

Zhang, Shenglei
 

Entry and RetEntry might be NULL before used.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
Reviewed-by: Hao A Wu <hao.a.wu@...>
---
MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdSymbol.c | 2 +-
MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdSymbol.c b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdSymbol.c
index 8e305e4243a5..7b453fa98c2b 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdSymbol.c
+++ b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCmdSymbol.c
@@ -143,7 +143,7 @@ DebuggerDisplaySymbolAccrodingToAddress (
// Find the nearest symbol address
//
CandidateAddress = EbdFindSymbolAddress (Address, EdbMatchSymbolTypeNearestAddress, &Object, &Entry);
- if (CandidateAddress == 0 || CandidateAddress == (UINTN) -1) {
+ if (CandidateAddress == 0 || CandidateAddress == (UINTN) -1 || Entry == NULL) {
EDBPrint (L"Symbole at Address not found!\n");
return EFI_DEBUG_CONTINUE;
} else if (Address != CandidateAddress) {
diff --git a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c
index 85cc275c114b..90a9b9fbd7ee 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c
+++ b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c
@@ -2062,7 +2062,7 @@ EdbPrintSource (
&RetObject,
&RetEntry
);
- if (SymbolAddress == 0) {
+ if (SymbolAddress == 0 || RetEntry == NULL) {
return 0 ;
}

--
2.18.0.windows.1


[PATCH v 3 0/4] MdeModulePkg: Add check and ASSERT for variables

Zhang, Shenglei
 

Add error handling and ASSERT to ensure the variables are
usable when called.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Dandan Bi <dandan.bi@...>
Cc: Eric Dong <eric.dong@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Liming Gao <liming.gao@...>
Shenglei Zhang (4):

v2: Update the checking method in 02/04.

v3: Add ASSERT instead of error handling in 04/04.

Shenglei Zhang (4):
MdeModulePkg/EbcDebugger: Add check for Entry and RetEntry
MdeModulePkg/HiiDatabaseDxe: Add check for StringPtr
MdeModulePkg/EsrtDxe: Add check for EsrtRepository
MdeModulePkg/SetupBrowserDxe: ASSERT GetBufferForValue(&Value)

.../Universal/EbcDxe/EbcDebugger/EdbCmdSymbol.c | 2 +-
MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSymbol.c | 2 +-
MdeModulePkg/Universal/EsrtDxe/EsrtImpl.c | 10 ++++++++++
MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c | 1 +
MdeModulePkg/Universal/SetupBrowserDxe/Expression.c | 3 +++
5 files changed, 16 insertions(+), 2 deletions(-)

--
2.18.0.windows.1


Re: Question regarding MMIO address space exposed from GetMemoryMap

Jon Nettleton
 

On Thu, Oct 31, 2019 at 5:44 PM Andrew Fish via Groups.Io
<afish=apple.com@groups.io> wrote:

Jon,

Its a little confusing but gBS->GetMemoryMap () only returns information about DRAM and any address that requires a kernel virtual address mapping in EFI. The OS calls EFI Runtime Services from a kernel virtual mapping so the memory map is also involved in the hand shake to communicate the virtual address mappings to EFI. Thus any MMIO region in the EFI Memory Map should always have the EFI_MEMORY_RUNTIME attribute set. The most common MMIO region to be mapped is the NOR FLASH to support the EFI Runtime Variable services.

On a UEFI system MMIO regions are described to the OS via ACPI. Processing the ACPI tables requires an interpreter, and source to an interpreter is available at the ACPI CA site [1].

The PI (Platform Initialization) Spec has a concept called the Global Coherency Domain (GCD) [2]. The GCD is the map for who owns the CPU address and IO (x86 in/out instructions not MMIO) space. You can dump the GCD info via gDS->GetMemorySpaceMap(). The idea is the DRAM controller would carve out a EfiGcdMemoryTypeSystemMemory range, and the PCI Host Bridge would carve out a EfiGcdMemoryTypeMemoryMappedIo range, etc.

Note: A UEFI implementation is not required to be constructed out of PI Spec components, but the EDKII UEFI implementation is constructed using the PI Specification.

[1] https://acpica.org/downloads/uefi-support
[2] Sorry I could not make up a better name at the time.

Thanks,

Andrew Fish

On Oct 31, 2019, at 1:32 AM, Jon Nettleton <jon@...> wrote:

I am working on sorting out a failure on test 605 of the SBSA test.
The test is "Where a memory access is to an unpopulated part of the
addressable memory space, accesses must be terminated in a manner that
is presented to the PE as either a precise Data Abort or that causes a
system error interrupt or an SPI or LPI interrupt to be delivered to
the GIC." The issue is that the random test address that was chosen
0x04200000 falls directly in the middle of the Qoriq configuration,
control, and status register (CCSR) address space. This is an area in
the memory space that provides CPU access to the device registers.

An entry is added for this region in the VirtualMemoryMap, and
registered with the attribute, ARM_MEMORY_REGION_ATTRIBUTE_DEVICE.
However only a handful of devices are being registered so only a
couple of ranges are showing up in memmap as MMIO.

The SBSA test in question gets the memorymap and starts at the address
mentioned above and looks for the first chunk of memory that is free
and then attempts to access the memory and is looking for a Data
Abort. Of course a data abort is not generated because 0x04200000 is
a valid address. If you offset this to 0x42000000 then a DA is
generated as expected and the test passes.

There is a very old thread started by Ard, "MMIO regions in
GetMemoryMap ()", in which he questions if all the MMIO regions should
be reported by GetMemoryMap() or only the ones being used by
initialized devices, which is what the current implementation does.
The end result of the thread was the current implementation is
correct. That leaves me with the question, "What is the proper
solution for the current implementation that I am working with?"

To me it seems like I need a special Library that on boot goes through
and claims and maps every valid MMIO slot in this region, and then
have the drivers use this library for proxying requests to
gDS->AddMemorySpace (), gDS->SetMemorySpaceAttributes () etc. If
there is a standardized way to deal with this configuration I would
much rather follow that.

Thanks for any input,
Jon




Andrew,

Thanks for clearing this up. I definitely understand the
relationships much better now. I am still uncertain of the proper way
to fix this issue regarding the SBSA peripheral test. The test is
using gBS->GetMemoryMap in order to look for available memory
segments, which according to the earlier thread and your confirmation
"gBS->GetMemoryMap () only returns information about DRAM and any
address that requires a kernel virtual address mapping in EFI.", which
means that this test will never work on this platform because the
address that was randomly chosen just happens to fall right into the
address range for the device addresses that are valid so won't produce
a DA, but aren't being used by EFI therefore don't require a virtual
address mapping in EFI.

Is this something I should bring up with the SBSA group?

-Jon


Re: [Patch v3 00/22] Enable Phase 1 of EDK II CI

Michael D Kinney
 

Hi Laszlo,

===================================================
Note for all reviewers:
---------------------------------------------------
Pull requests against edk2-staging/edk2-ci are not
being processed right now. We are working on some
configuration changes after noticing that all the
checks were not being shown on the Web UI. We will
let you know when it is back up.
===================================================

Yes. You need to create a fork of the tianocore/edk2-staging repo.

You can do this with the WebUI or the hub command line utility.

* https://github.com/github/hub
* https://github.com/github/hub/releases
* https://hub.github.com/hub.1.html

The hub command also supports creating a pull request.
I have used it extensively to write some unit tests
for edk2-ci this week.

Once you create a branch with changes to submit in your
own fork of tianocore/edk2-staging the WebUI will show
that this pull request is possible and guide you through
it.

The use of edk2-staging/edk2-ci is only for the review
and unit testing. Once the review is approved, it will
be enabled on edk2/master and you will be able to use
your own fork of edk2 to make branches and submit pull
requests.

Mike

-----Original Message-----
From: Laszlo Ersek <lersek@...>
Sent: Thursday, October 31, 2019 2:55 AM
To: devel@edk2.groups.io; Kinney, Michael D
<michael.d.kinney@...>
Cc: Sean Brogan <sean.brogan@...>; Bret
Barkelew <Bret.Barkelew@...>; Gao, Liming
<liming.gao@...>; Feng, Bob C
<bob.c.feng@...>; Andrew Fish <afish@...>;
Leif Lindholm <leif.lindholm@...>; Wang, Jian J
<jian.j.wang@...>; Lu, XiaoyuX
<xiaoyux.lu@...>; Ni, Ray <ray.ni@...>; Wu,
Hao A <hao.a.wu@...>; Wu, Jiaxin
<jiaxin.wu@...>; Fu, Siyuan <@sfu5>;
Yao, Jiewen <jiewen.yao@...>; Zhang, Chao B
<chao.b.zhang@...>; Gao, Zhichao
<zhichao.gao@...>; Dong, Eric
<eric.dong@...>
Subject: Re: [edk2-devel] [Patch v3 00/22] Enable Phase
1 of EDK II CI

On 10/29/19 20:54, Michael D Kinney wrote:

Active branch for testing/evaluation:
* https://github.com/tianocore/edk2-staging/tree/edk2-
ci
* To test, fork edk2-staging repo, create a branch
with a change, and submit
a pull request targeting edk2-staging/edk2-ci.
NOTE: the default branch for
the edk2-staging is 'about'. You must select the
'edk2-ci' branch when
a pull request is opened. Set the 'push' label to
require commit if all
checks pass.
The edk2-staging repository has been added as a "git
remote" to my local
edk2 clone for a long while now. Using the local
identifier "staging".
(This makes perfect sense as edk2-staging is itself a
fork of edk2, with branches that are supposed to be
rebased to edk2/master periodically.)

Furthermore, the identifier by which I refer to the
remote at <https://github.com/lersek/edk2.git> is
"lersek".

I've now run the following commands:

$ git fetch staging
$ git checkout -b ci-test-1 staging/edk2-ci

[modify "SampleFile.txt"]

$ git add -p
$ git commit
$ git push lersek ci-test-1

Questions:

(a) How can I submit a pull request for the staging
repo's edk2-ci branch using the command line (and set
the "push" label)?

(b) How can I submit a PR for the staging repo's edk2-ci
branch (regardless of command line vs. WebUI usage)
against my <https://github.com/lersek/edk2.git>
repository?

When I go to the WebUI, the PR view does not offer
"tianocore/edk2-staging" as "base repository", it only
offers "tianocore/edk2". I thought I'd be able to pick
any destination repository at all.

By the instruction "fork edk2-staging repo", did you
mean we should fork edk2-staging *on github*? (Using the
WebUI?)

Thanks!
Laszlo


Re: How /sys/firmware/fdt getting created

Prabhakar Kushwaha
 

Hi Bhupesh,

On Fri, Nov 1, 2019 at 1:59 AM Bhupesh Sharma <bhsharma@...> wrote:

Hi Prabhakar,

On Wed, Oct 30, 2019 at 1:47 PM Prabhakar Kushwaha
<prabhakar.pkin@...> wrote:

On Wed, Oct 30, 2019 at 1:14 PM Ard Biesheuvel
<ard.biesheuvel@...> wrote:

On Wed, 30 Oct 2019 at 08:36, Prabhakar Kushwaha
<prabhakar.pkin@...> wrote:

On Wed, Oct 30, 2019 at 12:43 PM Ard Biesheuvel
<ard.biesheuvel@...> wrote:

On Tue, 29 Oct 2019 at 18:17, Prabhakar Kushwaha
<prabhakar.pkin@...> wrote:

Hi All,

I am working on Ubuntu-18.04 with UEFI on ARM64(64 bit) platform. The
UEFI used is having ACPI tables.

I am trying to understand where and how /sys/firmware/fdt is getting
created. is it created by UEFI or grub and passed to Linux?
Neither. It is created by Linux itself.


Thanks Ard,

Can you please point me the code where it is getting created.
I want to add below in /sys/firmware/fdt.

#size-cells = <0x02>;
#address-cells = <0x02>;
Actually, in your case it is GRUB not the kernel that creates the FDT.
It does this to pass the initrd information.

So if you want to add these properties, you should add them there.

Can you explain why doing this is necessary?
I am trying to test kexec -p (kdump feature) on CentOS-release
7.7.1908 and Ubuntu-18.04 distributions.

"kexec -p" command show error on Ubuntu. While no error on CentOS

CentOS:
$ kexec -p /boot/vmlinuz-`uname -r` --initrd=/boot/initramfs-`uname
-r`.img --reuse-cmdline
$ ==> No error

Ubuntu
$ kexec -p /boot/vmlinuz-`uname -r` --initrd=/boot/initrd.img-`uname
-r` --reuse-cmdline
$ kexec: elfcorehdr doesn't fit cells-size.
$ kexec: setup_2nd_dtb failed.
$ kexec: load failed.
$ Cannot load /boot/vmlinuz-5.4.0-rc4+

Note: Both CentOS and Ubuntu has Linux-5.4-rc4 tag.

When i debugged further reason for Ubuntu error is due to
address-cells and size-cells as "1"
log from kexec tool :-
load_crashdump_segments: elfcorehdr 0x7f7cbfc000-0x7f7cbff7ff
read_1st_dtb: found name =dtb_sys /sys/firmware/fdt
get_cells_size: #address-cells:1 #size-cells:1

On CentOS both values are "2".
log from kexec tool :-
load_crashdump_segments: elfcorehdr 0xbf98bf0000-0xbf98bf33ff
read_1st_dtb: found nmae=dtb_sys /sys/firmware/fdt
get_cells_size: #address-cells:2 #size-cells:2

Note: Kexec tool read values from /sys/firmware/fdt.

I am trying to figure out why 2 distributions showing different values.
There are a couple of things I can suggest:

1. Try to see if it is a kexec-tools specific issue or is the kernel
itself passed an incorrectly fixed DTB (by grub?) with incorrect
#address-cells and #size-cells values (in the past I have seen
kexec-tools sometimes reports incorrect #address-cells and #size-cells
values, but they should be fixed in the newer kexec-tools versions):

a). Can you check the kexec-tools version and share the same:
$ kexec -v

b). Using 'dtc' tool, you can confirm if it reports a correct
#address-cells and #size-cells values:
# dtc -I dtb -O dts /sys/firmware/fdt | grep cells | less

For e.g on my fedora arm64 system, it reports:
#address-cells = <0x2>;
#size-cells = <0x2>;

2a). If its not a kexec-tools specific issue, it is most probably a
bootloader (grub?) issue in your case:

For e.g. I use the following grub2 on my Fedora arm64 board:
<https://github.com/rhboot/grub2>
and <https://github.com/rhboot/grub2/blob/master/grub-core/loader/efi/fdt.c#L34>
contains the changes to send the correct #address-cells and
#size-cells values to Linux (and hence user-space tools like
kexec-tools later).

I believe the same grub2 is used (backported) for CentOS, so things
should be fine there.

2b). I see that the latest devel branch of ubuntu grub2
(<https://code.launchpad.net/ubuntu/+source/grub2>) also contains this
fix, but I am not sure which grub2 version you have on your ubuntu
machine.
Ubuntu 18.04 has grub 2.02. When i migrated to grub 2.05, this issue
is not there.
Most probably the patch which is mentioned by Laszlo done the fix.

http://git.savannah.gnu.org/cgit/grub.git/commit/?id=347210a5d5ce655b95315f320faa515afb723c11

Thanks
--pk


Re: How /sys/firmware/fdt getting created

Prabhakar Kushwaha
 

On Fri, Nov 1, 2019 at 2:04 AM dann frazier <dann.frazier@...> wrote:

On Thu, Oct 31, 2019 at 12:55:10PM -0600, dann frazier wrote:
On Thu, Oct 31, 2019 at 6:16 AM Leif Lindholm <leif.lindholm@...> wrote:

On Thu, Oct 31, 2019 at 11:07:52AM +0100, Laszlo Ersek wrote:
+Leif, comment at bottom
Thanks Laszlo. +Dann, -kexec.

On 10/30/19 09:16, Prabhakar Kushwaha wrote:
So if you want to add these properties, you should add them there.

Can you explain why doing this is necessary?
I am trying to test kexec -p (kdump feature) on CentOS-release
7.7.1908 and Ubuntu-18.04 distributions.

"kexec -p" command show error on Ubuntu. While no error on CentOS

CentOS:
$ kexec -p /boot/vmlinuz-`uname -r` --initrd=/boot/initramfs-`uname
-r`.img --reuse-cmdline
$ ==> No error

Ubuntu
$ kexec -p /boot/vmlinuz-`uname -r` --initrd=/boot/initrd.img-`uname
-r` --reuse-cmdline
$ kexec: elfcorehdr doesn't fit cells-size.
$ kexec: setup_2nd_dtb failed.
$ kexec: load failed.
$ Cannot load /boot/vmlinuz-5.4.0-rc4+

Note: Both CentOS and Ubuntu has Linux-5.4-rc4 tag.

When i debugged further reason for Ubuntu error is due to
address-cells and size-cells as "1"
log from kexec tool :-
load_crashdump_segments: elfcorehdr 0x7f7cbfc000-0x7f7cbff7ff
read_1st_dtb: found name =dtb_sys /sys/firmware/fdt
get_cells_size: #address-cells:1 #size-cells:1

On CentOS both values are "2".
log from kexec tool :-
load_crashdump_segments: elfcorehdr 0xbf98bf0000-0xbf98bf33ff
read_1st_dtb: found nmae=dtb_sys /sys/firmware/fdt
get_cells_size: #address-cells:2 #size-cells:2

Note: Kexec tool read values from /sys/firmware/fdt.

I am trying to figure out why 2 distributions showing different values.
http://git.savannah.gnu.org/cgit/grub.git/commit/?id=347210a5d5ce655b95315f320faa515afb723c11

Ubuntu probably ships a grub version that lacks this commit.
Yes, it came after the 18.04 release.
Dann: given that 18.04 is LTS, would it be reasonable to cherry-pick
this grub patch? I would consider the behaviour without it to be a
bug.
Leif,

Likely - I'll run some tests and get back to you...
Can you help me understand the scenario in which the above patch is
required? I tested an 18.04 VM w/ AAVMF (ACPI-mode), and kexec -p
worked fine. I also verified that 18.04 does not carry the above patch.
This issue is only visible when crashkernel region reserved beyond 4GB
by kernel.
I reproduced this issue by providing "crashkernel=2G" in bootargs.

if kernel is not reserving memory due to crashkernel region
overlapping with "reserved regions", you can apply below patch.
https://patchwork.kernel.org/patch/10144305/

Update: When I migrated ubuntu grub to 2.05 this issue is gone.

--pk


Re: [PATCH v2] MdeModulePkg/HiiDatabaseDxe: Add check for 'Private->Attribute >> 4'

Dandan Bi
 

-----Original Message-----
From: Zhang, Shenglei
Sent: Wednesday, October 30, 2019 9:46 PM
To: devel@edk2.groups.io
Cc: Bi, Dandan <dandan.bi@...>; Dong, Eric <eric.dong@...>
Subject: [PATCH v2] MdeModulePkg/HiiDatabaseDxe: Add check for 'Private-
Attribute >> 4'
The size of mHiiEfiColors is 16.
mHiiEfiColors[Private->Attribute >> 4] may be out of boundary.
So add a check for that.

Cc: Dandan Bi <dandan.bi@...>
Cc: Eric Dong <eric.dong@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
---
v2: Instead of returing value, we add ASSERT to ensure
"Private->Attribute >> 4" is not out of boundary.

MdeModulePkg/Universal/HiiDatabaseDxe/Font.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c
b/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c
index ca63df168c94..1eee5ec76bb0 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Font.c
@@ -999,6 +999,7 @@ GetSystemFont (
}

Info->ForegroundColor = mHiiEfiColors[Private->Attribute & 0x0f];
+ ASSERT ((Private->Attribute >> 4) < 16);
Hi Shenglei,
1) I think we can add ASSERT ((Private->Attribute >> 4) < 8); here directly, the reason can refer my comments in V1 patch.
2) And please also update the subject and commit message accordingly.
With above comments addressed, Reviewed-by: Dandan Bi <dandan.bi@...>

Thanks,
Dandan
Info->BackgroundColor = mHiiEfiColors[Private->Attribute >> 4];
Info->FontInfoMask = EFI_FONT_INFO_SYS_FONT |
EFI_FONT_INFO_SYS_SIZE | EFI_FONT_INFO_SYS_STYLE;
Info->FontInfo.FontStyle = 0;
--
2.18.0.windows.1


Re: [PATCH v2 2/4] MdeModulePkg/HiiDatabaseDxe: Add check for StringPtr

Dandan Bi
 

Hi Shenglei,

Please update the commit message and subject before commit, since we have added ASSERT code for this case that the StringPtr cannot be NULL instead of adding check.
With the commit message updated, Reviewed-by: Dandan Bi <dandan.bi@...>.


Thanks,
Dandan

-----Original Message-----
From: Zhang, Shenglei
Sent: Wednesday, October 30, 2019 10:27 PM
To: devel@edk2.groups.io
Cc: Bi, Dandan <dandan.bi@...>; Dong, Eric <eric.dong@...>
Subject: [PATCH v2 2/4] MdeModulePkg/HiiDatabaseDxe: Add check for
StringPtr

If the target string doesn't appear in the searched string, StringPtr will be
NULL. So add a check for that.

Cc: Dandan Bi <dandan.bi@...>
Cc: Eric Dong <eric.dong@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
---
v2: Instead of returning a value, we add ASSERT to ensure
StringPtr is not NULL.

MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
index 71ea25bc19bf..19a23fcc951e 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
@@ -909,6 +909,7 @@ CompareAndMergeDefaultString (
// To find the <AltResp> with AltConfigHdr in AltCfgResp, ignore other
<AltResp> which follow it.
//
StringPtr = StrStr (*AltCfgResp, AltConfigHdr);
+ ASSERT (StringPtr != NULL);
StringPtrNext = StrStr (StringPtr + 1, L"&GUID");
if (StringPtrNext != NULL) {
TempCharA = *StringPtrNext;
--
2.18.0.windows.1


Re: [PATCH v2 3/3] MdeModulePkg/Mem: Initialize the variable MapMemory

Dandan Bi
 

Reviewed-by: Dandan Bi <dandan.bi@...>

Thanks,
Dandan

-----Original Message-----
From: Zhang, Shenglei
Sent: Wednesday, October 30, 2019 10:09 PM
To: devel@edk2.groups.io
Cc: Bi, Dandan <dandan.bi@...>; Gao, Liming <liming.gao@...>
Subject: [PATCH v2 3/3] MdeModulePkg/Mem: Initialize the variable
MapMemory

MapMemory is not initialized by FindGuardedMemoryMap or
CoreInternalAllocatePages which calls MapMemory.
So we give a 0 to it.

Cc: Dandan Bi <dandan.bi@...>
Cc: Liming Gao <liming.gao@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
---
MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
index 9477b94044ba..b4cb48843fb7 100644
--- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
+++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
@@ -225,6 +225,8 @@ FindGuardedMemoryMap (
UINTN BitsToUnitEnd;
EFI_STATUS Status;

+ MapMemory = 0;
+
//
// Adjust current map table depth according to the address to access
//
--
2.18.0.windows.1


Re: [PATCH V6 00/10] UEFI Variable SMI Reduction

Wang, Jian J
 

For this patch series,

Reviewed-by: Jian J Wang <jian.j.wang@...>

Regards,
Jian

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kubacki,
Michael A
Sent: Thursday, October 31, 2019 2:45 PM
To: devel@edk2.groups.io
Cc: Bi, Dandan <dandan.bi@...>; Ard Biesheuvel
<ard.biesheuvel@...>; Dong, Eric <eric.dong@...>; Laszlo Ersek
<lersek@...>; Gao, Liming <liming.gao@...>; Kinney, Michael D
<michael.d.kinney@...>; Ni, Ray <ray.ni@...>; Wang, Jian J
<jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>; Yao, Jiewen
<jiewen.yao@...>
Subject: [edk2-devel] [PATCH V6 00/10] UEFI Variable SMI Reduction

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

V6 Changes:
[PATCH V5 07/10] MdeModulePkg/Variable: Add RT GetVariable() cache support
The most significant change is:
* Free mVariableRuntimeHobCacheBuffer in CheckForRuntimeCacheSync () in
VariableSmmRuntimeDxe.c with FreePages () instead of FreePool ().
This issue was not found in earlier testing because on the initial set of
platforms tested, the variable HOB flush was finished prior to the variable
HOB runtime cache buffer being allocated so the FreePages () call was not
executed.

The remaining changes did not affect testing but are included for robustness:
* Pass EFI_OPTIONAL_PTR for the DebugDisposition type in the
EfiConvertPointer ()
calls for mVariableRuntimeHobCacheBuffer, mVariableRuntimeNvCacheBuffer,
and
mVariableRuntimeVolatileCacheBuffer in VariableAddressChangeEvent () in
VariableSmmRuntimeDxe.c as these buffers will not be allocated if the runtime
cache is disabled.
* In the
SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEXT case
in
SmmVariableHandler () in VariableSmm.c, explicitly verify that
VariableRuntimeHobCache.Store is not NULL in addition to checking that
VariableGlobal.HobVariableBase is not set to zero (variable HOB is flushed)
before writing to VariableRuntimeHobCache.Store.

V5 Changes:
[PATCH V4 07/10] MdeModulePkg/Variable: Add RT GetVariable() cache support
* Increased validation of the runtime buffers passed in the SMM comm buffer
SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT
structure to the
SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEXT
function in
SmmVariableHandler () in VariableSmm.c.
* Most notably, each runtime buffer given is checked to ensure its memory
range does not overlap with SMRAM ranges via
VariableSmmIsBufferOutsideSmmValid ().

V4 Changes:
[PATCH V3 7/9] MdeModulePkg/Variable: Add RT GetVariable() cache support
* Set gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache to
FALSE
by default in MdeModulePkg.dec.

* Added a new patch to set
gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache
to TRUE at the end of the patch series. This allows someone to bisect an issue
at
patch #7 or patch #8 in the series with no change in variable caching behavior.
The
runtime cache variable logic would be applied explicitly in V4 patch #10.

V3 Changes:
[PATCH V2 1/9] MdeModulePkg/Variable: Consolidate common parsing
functions
* Removed GUIDs added to VariableStandaloneMm.inf that are not required.
* Added more details to the commit message describing the criteria of
moving the chosen functions to VariableParsing.c.

[PATCH V2 2/9] MdeModulePkg/Variable: Parameterize GetNextVariableEx()
store list
* RenamedGetNextVariableEx () to VariableServiceGetNextVariableInternal ()
* Updated comments in VariableServiceGetNextVariableInternal () to refer to
"FindVariablEx ()" instead of "FindVariable ()" since FindVariableEx ()
is not used in the function.

[PATCH V2 3/9] MdeModulePkg/Variable: Parameterize
VARIABLE_INFO_ENTRY buffer
* Updated the commit message to clarify the message "structure can be
updated
outside the fixed global variable".

[edk2-devel] [PATCH V2 4/9] MdeModulePkg/Variable: Add local auth status in
VariableParsing
* Remove the function InitVariableParsing ()
* Remove the mAuthFormat global variable and instead add a BOOLEAN
parameter
to all functions that require variable authentication information to
indicate if authenticated variables are used.
* This allows the authenticated variable status to be tracked in one place in
each variable driver in the SMM variable solution (VariableSmmRuntimeDxe
and VariableSmm).

[edk2-devel] [PATCH V2 5/9] MdeModulePkg/Variable: Add a file for NV
variable functions
* Added the following non-volatile related functions to VariableNonVolatile.c
from Variable.c:
* InitRealNonVolatileVariableStore ()
* InitEmuNonVolatileVariableStore ()
* InitNonVolatileVariableStore ()

[edk2-devel] [PATCH V2 7/9] MdeModulePkg/Variable: Add RT GetVariable()
cache support
* Added a FeaturePCD to control enabling the runtime variable cache -
gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache.
* Removed usage of the TimerLib and the wait to acquire
mVariableRuntimeCacheReadLock. Can rely on the UEFI specification
restrictions on Runtime Services callers.
* Removed the EFIAPI keyword from internal functions.
* Removed PCDs in VariableSmmRuntimeDxe.inf not required.
* Removed the HobVariableBackupBase variable no longer required.
* Renamed SynchronizeRuntimeVariableCacheEx () to better reflect usage.
* Renamed functions in VariableRuntimeCache.c to better reflect usage.
* Introduced a local variable in FlushPendingRuntimeVariableCacheUpdates ()
to reduce duplication of
mVariableModuleGlobal->VariableGlobal.VariableRuntimeCacheContext.
* Corrected the macro used in SmmVariableHandler () to
SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT.
* Remove usage of the EDKII_PI_SMM_COMMUNICATION_REGION_TABLE to
acquire a
CommBuffer from the EFI System Table and use the same runtime
CommBuffer
allocated for variable SMM communicate calls.

[PATCH V2 8/9] MdeModulePkg/Variable: Add RT GetNextVariableName()
cache support
* Removed usage of the TimerLib and the wait to acquire
mVariableRuntimeCacheReadLock. Can rely on the UEFI specification
restrictions
on Runtime Services callers.

* Added a new patch to disable
gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache
for all OvmfPkg package builds as requested by maintainer Laszlo Ersek.

V2 Changes:

Patch #1 in V1 both moved functions to VariableParsing.c and modified some
functionality in those functions. In V2, the functions are first moved and
then functionality is modified in subsequent patches. This resulted in the
following new patches in the V2 patch series:

1. MdeModulePkg/Variable: Parameterize GetNextVariableEx() store list
2. MdeModulePkg/Variable: Parameterize VARIABLE_INFO_ENTRY buffer
3. MdeModulePkg/Variable: Add local auth status in VariableParsing
4. MdeModulePkg/Variable: Add a file for NV variable functions

Apart from this refactor in the patches, no functionally impacting changes
were made.

Overview
---------
This patch series reduces SMM usage when using VariableSmmRuntimeDxe with
VariableSmm. It does so by eliminating SMM usage for runtime service
GetVariable () and GetNextVariableName () invocations. Most UEFI variable
usage in typical systems after the variable store is initialized
(e.g. manufacturing boots) is due to GetVariable ( ) and
GetNextVariableName () not SetVariable (). GetVariable () calls can regularly
exceed 100 per boot while SetVariable () calls typically remain less than 10
per boot. By focusing on the common case, the majority of overhead associated
with SMM can be avoided while still using existing and proven code for
operations such as variable authentication that require an isolated execution
environment.

* Advantage: Reduces overall system SMM usage
* Disadvantage: Requires more Runtime data memory usage

The runtime cache behavior described for this patch series is enabled by
default but can be disabled with a new FeaturePCD added to
MdeModulePkg.dec:
gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache

The reaminder of this blurb describes the changes and behavior when the PCD
is set to TRUE.

Initial Performance Observations
---------------------------------
* With these proposed changes, an Intel Atom based SoC saw GetVariable ( )
time for an existing variable reduce from ~220us to ~5us.

Major Changes
--------------
1. Two UEFI variable caches will be maintained.
a. "Runtime Cache" - Maintained in VariableSmmRuntimeDxe. Used to serve
runtime service GetVariable () and GetNextVariableName () callers.
b. "SMM cache" - Maintained in VariableSmm to service SMM GetVariable ()
and GetNextVariableName () callers.
i. A cache in SMRAM is retained so SMM modules do not operate on data
outside SMRAM.
2. A new UEFI variable read and write flow will be used as described below.

At any given time, the two caches would be coherent. On a variable write, the
runtime cache is only updated after validation in SMM and, in the case of a
non-volatile UEFI variable, the variable must also be successfully written to
non-volatile storage.

Prior RFC Feedback Addressed
-----------------------------
RFC sent Sept. 5, 2019: https://edk2.groups.io/g/devel/message/46939

1. UEFI variable data retrieval from a ring 0 buffer

A common concern with this proposed set of changes is the potential security
threat presented by serving runtime services callers from a ring 0 memory
buffer of EfiRuntimeServicesData type. The conclusion was that this change
does not fundamentally alter the attack surface. The UEFI variable Runtime
Services are invoked from ring 0 and the data already travels through ring
0 buffers (such as the SMM communicate buffer) to reach the caller. Even
today if ring 0 is assumed to be malicious, the malicious code may keep one
AP in a loop to monitor the communication data, when the BSP gets an
(authenticated) variable. When the communication buffer is updated and the
status is set to EFI_SUCCESS, the AP may modify the communication buffer
contents such the tampered data is returned to the BSP caller. Or an
interrupt handler on the BSP may alter the communication buffer contents
before the data is returned to the caller. In summary, this was not found to
introduce any attack not possible today.

2. VarCheckLib impact

VarCheckLib plays a role in SetVariable () calls. This patch series only
changes GetVariable () behavior. Therefore, VarCheckLib is expected to
have no impact due to these changes.

Testing Performed
------------------
This code was tested with the master branch of edk2 on an Intel Kaby Lake U
and Intel Whiskey Lake U reference validation platform. The set of tests
performed included:

1. Boot from S5 to Windows 10 OS with SMM variables enabled and
the variable runtime cache enabled.
2. Boot from S5 to Ubuntu 18.04.1 LTS with SMM variable enabled
and the variable runtime cache enabled.
3. Boot from S5 to Windows 10 OS with SMM variables enabled and
the variable runtime cache disabled.
4. Boot from S5 to Ubuntu 18.04.1 LTS with SMM variable enabled
and the variable runtime cache disabled.
5. Boot from S5 to EFI shell with DXE variables enabled.
(commit 2de1f611be broke OvmfPkgIa32X64 boot regardless of
this patch series; testing without this commit was successful)
6. Dump UEFI variable store at shell with dmpstore to verify contents.
7. Dump NvStorage FV from SPI flash after boot to verify contents written.
8. Dump UEFI variable statistics with VariableInfo at shell.
9. Boot with emulated variables enabled.
10. Cycles of adding and deleting a UEFI variable to verify cache correctness.
11. Set OsIndications to stop at FW UI to verify cache load of non-volatile
contents.

Why Keep SMM on Variable Writes
--------------------------------
* SMM provides a ubiquitous isolated execution environment in x86 for
authenticated UEFI variables.
* BIOS region SPI flash write restrictions to SMM in platforms today can
be retained.

Today's UEFI Variable Cache (for reference)
--------------------------------------------
* Maintained in SMRAM via VariableSmm.
* A "write-through" cache of variable data in the form of a UEFI variable
store.
* Non-volatile and volatile variables are maintained in separate buffers
(variable stores).

Runtime & SMM Cache Coherency
------------------------------
The non-volatile cache should always accurately reflect non-volatile storage
contents (done today) and the "SMM cache" and "Runtime cache" should always
be
coherent on access. The runtime cache is updated by VariableSmm.

Updating both caches from within a SMM SetVariable () operation is fairly
straightforward but a race condition can occur if an SMI occurs during the
execution of runtime code reading from the runtime cache. To handle this case,
a runtime cache read lock is introduced that explicitly moves pending updates
from SMM to the runtime cache if an SMM update occurs while the runtime
cache
is locked. Note that it is not expected a Runtime services call will interrupt
SMM processing since all CPU cores rendezvous in SMM.

New Key Elements for Coherence
-------------------------------
Runtime DXE (VariableSmmRuntimeDxe)
1. RuntimeCacheReadLock - A global lock used to lock read access to the
runtime cache.
2. RuntimeCachePendingUpdate - A global flag used to notify runtime code of a
pending cache update in SMM.

SMM (VariableSmm)
1. FlushRuntimeCachePendingUpdate SMI - A SW SMI handler that synchronizes
the runtime cache buffer with the SMM
cache buffer.

Proposed Runtime DXE Read Flow
-------------------------------
1. Acquire RuntimeCacheReadLock
2. If RuntimeCachePendingUpdate flag (rare) is set then:
2.a. Trigger FlushRuntimeCachePendingUpdate SMI
2.b. Verify RuntimeCachePendingUpdate flag is cleared
3. Perform read from RuntimeCache
4. Release RuntimeCacheReadLock

Proposed FlushRuntimeCachePendingUpdate SMI
--------------------------------------------
1. If RuntimeCachePendingUpdate flag is not set:
1.a. Return
2. Copy the data at RuntimeCachePendingOffset of
RuntimeCachePendingLength to
RuntimeCache
3. Clear the RuntimeCachePendingUpdate flag

Proposed SMM Write Flow
------------------------
1. Perform variable authentication and non-volatile write. If either fail,
return an error to the caller.
2. If RuntimeCacheReadLock is set then:
2.a. Set RuntimeCachePendingUpdate flag
2.b. Update RuntimeCachePendingOffset and RuntimeCachePendingLength
to
cover the a superset of the pending chunk (for simplicity, the
entire variable store is currently synchronized).
3. Else:
3.a. Update RuntimeCache
4. Update SmmCache
- Note: RT read cannot occur during SMI processing since all cores are
locked in SMM.

Cc: Dandan Bi <dandan.bi@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Eric Dong <eric.dong@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Liming Gao <liming.gao@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Ray Ni <ray.ni@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Signed-off-by: Michael Kubacki <michael.a.kubacki@...>

Michael Kubacki (10):
MdeModulePkg/Variable: Consolidate common parsing functions
MdeModulePkg/Variable: Parameterize GetNextVariableInternal () stores
MdeModulePkg/Variable: Parameterize VARIABLE_INFO_ENTRY buffer
MdeModulePkg/Variable: Parameterize auth status in VariableParsing
MdeModulePkg/Variable: Add a file for NV variable functions
MdeModulePkg VariableInfo: Always consider RT DXE and SMM stats
MdeModulePkg/Variable: Add RT GetVariable() cache support
MdeModulePkg/Variable: Add RT GetNextVariableName() cache support
OvmfPkg: Disable variable runtime cache
MdeModulePkg: Enable variable runtime cache by default

MdeModulePkg/MdeModulePkg.dec | 12 +
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf | 6
+
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf | 6 +
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf |
20 +-
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf |
6 +
MdeModulePkg/Include/Guid/SmmVariableCommon.h | 29 +-
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h | 151 +--
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.h | 67
+
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h | 347
+++++
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.h |
51 +
MdeModulePkg/Application/VariableInfo/VariableInfo.c | 37 +-
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 1373
++++----------------
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableExLib.c | 24 +-
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c | 334
+++++
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c | 786
+++++++++++
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.c |
153 +++
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 195
++-
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c |
655 +++++++++-
21 files changed, 2926 insertions(+), 1329 deletions(-)
create mode 100644
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.h
create mode 100644
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h
create mode 100644
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.h
create mode 100644
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c
create mode 100644
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c
create mode 100644
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.c

--
2.16.2.windows.1



Re: [edk2-platforms: PATCH v3 1/6] MinPlatformPkg: Add SetCacheMtrrLib library class.

Chiu, Chasel
 

Thanks Michael!
I will update function description.

-----Original Message-----
From: Kubacki, Michael A <michael.a.kubacki@...>
Sent: Friday, November 1, 2019 1:02 AM
To: devel@edk2.groups.io; Chiu, Chasel <chasel.chiu@...>
Cc: Desimone, Nathaniel L <@natedesimone>; Gao, Liming
<liming.gao@...>
Subject: RE: [edk2-devel] [edk2-platforms: PATCH v3 1/6] MinPlatformPkg:
Add SetCacheMtrrLib library class.

Thanks for updating the name.

Platform\Intel\MinPlatformPkg\Include\Library\SetCacheMtrrLib.h:
* The function description for SetCacheMtrrAfterEndOfPei () is
constraining the implementation in a way that I don't believe
is required:
/**
Update MTRR setting and set write back as default memory attribute.

@retval EFI_SUCCESS The function completes successfully.
@retval Others Some error occurs.
**/

While it is typical to set the default memory type to WB, I don't
think this API should care whether that is the case.

* "SetCacheMtrrAtEndOfPei ()" better describes the way this
is actually being used but I don't think it is a must change.

With the function description updated:
Reviewed-by: Michael Kubacki <michael.a.kubacki@...>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chiu,
Chasel
Sent: Thursday, October 31, 2019 3:28 AM
To: devel@edk2.groups.io
Cc: Kubacki, Michael A <michael.a.kubacki@...>; Desimone,
Nathaniel L <@natedesimone>; Gao, Liming
<liming.gao@...>
Subject: [edk2-devel] [edk2-platforms: PATCH v3 1/6] MinPlatformPkg:
Add SetCacheMtrrLib library class.

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

MinPlatformPkg should contain the library class header (API) and the
NULL library class instance.

Cc: Michael Kubacki <michael.a.kubacki@...>
Cc: Nate DeSimone <@natedesimone>
Cc: Liming Gao <liming.gao@...>
Signed-off-by: Chasel Chiu <chasel.chiu@...>
---

Platform/Intel/MinPlatformPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.
c
| 327
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+++++++++++++++++++++++++++++++++++++

Platform/Intel/MinPlatformPkg/Library/SetCacheMtrrLib/SetCacheMtrrLibN
ull.c | 37 +++++++++++++++++++++++++++++++++++++
Platform/Intel/MinPlatformPkg/Include/Library/SetCacheMtrrLib.h
|
34 ++++++++++++++++++++++++++++++++++

Platform/Intel/MinPlatformPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.in
f | 46 ++++++++++++++++++++++++++++++++++++++++++++++

Platform/Intel/MinPlatformPkg/Library/SetCacheMtrrLib/SetCacheMtrrLibN
ull.inf | 29 +++++++++++++++++++++++++++++
Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
| 6
++++--
6 files changed, 477 insertions(+), 2 deletions(-)

diff --git
a/Platform/Intel/MinPlatformPkg/Library/SetCacheMtrrLib/SetCacheMtrrLi
b
.c
b/Platform/Intel/MinPlatformPkg/Library/SetCacheMtrrLib/SetCacheMtrrLi
b
.c
new file mode 100644
index 0000000000..26f06321f7
--- /dev/null
+++
b/Platform/Intel/MinPlatformPkg/Library/SetCacheMtrrLib/SetCacheMtrr
+++ Lib.c
@@ -0,0 +1,327 @@
+/** @file
+
+SetCacheMtrr library functions.
+This implementation is for typical platforms and may not be needed
+when cache MTRR will be initialized by FSP.
+
+Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi.h>
+#include <PiPei.h>
+#include <Library/HobLib.h>
+#include <Library/PcdLib.h>
+#include <Library/DebugLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/MtrrLib.h>
+#include <Library/PeiServicesLib.h>
+#include <Guid/SmramMemoryReserve.h>
+
+/**
+ Set Cache Mtrr.
+**/
+VOID
+EFIAPI
+SetCacheMtrr (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ EFI_PEI_HOB_POINTERS Hob;
+ MTRR_SETTINGS MtrrSetting;
+ UINT64 MemoryBase;
+ UINT64 MemoryLength;
+ UINT64 LowMemoryLength;
+ UINT64 HighMemoryLength;
+ EFI_BOOT_MODE BootMode;
+ EFI_RESOURCE_ATTRIBUTE_TYPE ResourceAttribute;
+ UINT64 CacheMemoryLength;
+
+ ///
+ /// Reset all MTRR setting.
+ ///
+ ZeroMem(&MtrrSetting, sizeof(MTRR_SETTINGS));
+
+ ///
+ /// Cache the Flash area as WP to boost performance /// Status =
+ MtrrSetMemoryAttributeInMtrrSettings (
+ &MtrrSetting,
+ (UINTN) PcdGet32 (PcdFlashAreaBaseAddress),
+ (UINTN) PcdGet32 (PcdFlashAreaSize),
+ CacheWriteProtected
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ ///
+ /// Update MTRR setting from MTRR buffer for Flash Region to be WP
+ to boost performance /// MtrrSetAllMtrrs (&MtrrSetting);
+
+ ///
+ /// Set low to 1 MB. Since 1MB cacheability will always be set ///
+ until override by CSM.
+ /// Initialize high memory to 0.
+ ///
+ LowMemoryLength = 0x100000;
+ HighMemoryLength = 0;
+ ResourceAttribute = (
+ EFI_RESOURCE_ATTRIBUTE_PRESENT |
+ EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
+ EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
+
EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
+
EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
+
EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE
+ );
+
+ Status = PeiServicesGetBootMode (&BootMode); ASSERT_EFI_ERROR
+ (Status);
+
+ if (BootMode != BOOT_ON_S3_RESUME) {
+ ResourceAttribute |= EFI_RESOURCE_ATTRIBUTE_TESTED; }
+
+ Status = PeiServicesGetHobList ((VOID **) &Hob.Raw); while
+ (!END_OF_HOB_LIST (Hob)) {
+ if (Hob.Header->HobType == EFI_HOB_TYPE_RESOURCE_DESCRIPTOR)
{
+ if ((Hob.ResourceDescriptor->ResourceType ==
EFI_RESOURCE_SYSTEM_MEMORY) ||
+ ((Hob.ResourceDescriptor->ResourceType ==
EFI_RESOURCE_MEMORY_RESERVED) &&
+ (Hob.ResourceDescriptor->ResourceAttribute ==
ResourceAttribute))
+ ) {
+ if (Hob.ResourceDescriptor->PhysicalStart >= 0x100000000ULL) {
+ HighMemoryLength +=
Hob.ResourceDescriptor->ResourceLength;
+ } else if (Hob.ResourceDescriptor->PhysicalStart >= 0x100000) {
+ LowMemoryLength +=
Hob.ResourceDescriptor->ResourceLength;
+ }
+ }
+ }
+
+ Hob.Raw = GET_NEXT_HOB (Hob);
+ }
+
+ DEBUG ((DEBUG_INFO, "Memory Length (Below 4GB) = %lx.\n",
+ LowMemoryLength)); DEBUG ((DEBUG_INFO, "Memory Length (Above
4GB) =
+ %lx.\n", HighMemoryLength));
+
+ ///
+ /// Assume size of main memory is multiple of 256MB ///
+ MemoryLength = (LowMemoryLength + 0xFFFFFFF) & 0xF0000000;
MemoryBase
+ = 0;
+
+ CacheMemoryLength = MemoryLength;
+ ///
+ /// Programming MTRRs to avoid override SPI region with UC when
MAX
+ TOLUD Length >= 3.5GB /// if (MemoryLength > 0xDC000000) {
+ CacheMemoryLength = 0xC0000000;
+ Status = MtrrSetMemoryAttributeInMtrrSettings (
+ &MtrrSetting,
+ MemoryBase,
+ CacheMemoryLength,
+ CacheWriteBack
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ MemoryBase = 0xC0000000;
+ CacheMemoryLength = MemoryLength - 0xC0000000;
+ if (MemoryLength > 0xE0000000) {
+ CacheMemoryLength = 0x20000000;
+ Status = MtrrSetMemoryAttributeInMtrrSettings (
+ &MtrrSetting,
+ MemoryBase,
+ CacheMemoryLength,
+ CacheWriteBack
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ MemoryBase = 0xE0000000;
+ CacheMemoryLength = MemoryLength - 0xE0000000;
+ }
+ }
+
+ Status = MtrrSetMemoryAttributeInMtrrSettings (
+ &MtrrSetting,
+ MemoryBase,
+ CacheMemoryLength,
+ CacheWriteBack
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ if (LowMemoryLength != MemoryLength) {
+ MemoryBase = LowMemoryLength;
+ MemoryLength -= LowMemoryLength;
+ Status = MtrrSetMemoryAttributeInMtrrSettings (
+ &MtrrSetting,
+ MemoryBase,
+ MemoryLength,
+ CacheUncacheable
+ );
+ ASSERT_EFI_ERROR (Status);
+ }
+
+ ///
+ /// VGA-MMIO - 0xA0000 to 0xC0000 to be UC /// Status =
+ MtrrSetMemoryAttributeInMtrrSettings (
+ &MtrrSetting,
+ 0xA0000,
+ 0x20000,
+ CacheUncacheable
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ ///
+ /// Update MTRR setting from MTRR buffer /// MtrrSetAllMtrrs
+ (&MtrrSetting);
+
+ return ;
+}
+
+/**
+ Update MTRR setting and set write back as default memory attribute.
+
+ @retval EFI_SUCCESS The function completes successfully.
+ @retval Others Some error occurs.
+**/
+EFI_STATUS
+EFIAPI
+SetCacheMtrrAfterEndOfPei (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ MTRR_SETTINGS MtrrSetting;
+ EFI_PEI_HOB_POINTERS Hob;
+ UINT64 MemoryBase;
+ UINT64 MemoryLength;
+ UINT64 Power2Length;
+ EFI_BOOT_MODE BootMode;
+ UINTN Index;
+ UINT64 SmramSize;
+ UINT64 SmramBase;
+ EFI_SMRAM_HOB_DESCRIPTOR_BLOCK
*SmramHobDescriptorBlock;
+ Status = PeiServicesGetBootMode (&BootMode);
+ ASSERT_EFI_ERROR (Status);
+
+ if (BootMode == BOOT_ON_S3_RESUME) {
+ return EFI_SUCCESS;
+ }
+ //
+ // Clear the CAR Settings
+ //
+ ZeroMem(&MtrrSetting, sizeof(MTRR_SETTINGS));
+
+ //
+ // Default Cachable attribute will be set to WB to support large
+ memory size/hot plug memory // MtrrSetting.MtrrDefType &=
+ ~((UINT64)(0xFF)); MtrrSetting.MtrrDefType |= (UINT64)
+ CacheWriteBack;
+
+ //
+ // Set fixed cache for memory range below 1MB // Status =
+ MtrrSetMemoryAttributeInMtrrSettings (
+ &MtrrSetting,
+ 0x0,
+ 0xA0000,
+ CacheWriteBack
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ Status = MtrrSetMemoryAttributeInMtrrSettings (
+ &MtrrSetting,
+ 0xA0000,
+ 0x20000,
+ CacheUncacheable
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ Status = MtrrSetMemoryAttributeInMtrrSettings (
+ &MtrrSetting,
+ 0xC0000,
+ 0x40000,
+ CacheWriteProtected
+ );
+ ASSERT_EFI_ERROR ( Status);
+
+ //
+ // PI SMM IPL can't set SMRAM to WB because at that time CPU ARCH
protocol is not available.
+ // Set cacheability of SMRAM to WB here to improve SMRAM
+ initialization
performance.
+ //
+ SmramSize = 0;
+ SmramBase = 0;
+ Status = PeiServicesGetHobList ((VOID **) &Hob.Raw); while
+ (!END_OF_HOB_LIST (Hob)) {
+ if (Hob.Header->HobType == EFI_HOB_TYPE_GUID_EXTENSION) {
+ if (CompareGuid (&Hob.Guid->Name,
&gEfiSmmSmramMemoryGuid)) {
+ SmramHobDescriptorBlock =
(EFI_SMRAM_HOB_DESCRIPTOR_BLOCK
*) (Hob.Guid + 1);
+ for (Index = 0; Index < SmramHobDescriptorBlock-
NumberOfSmmReservedRegions; Index++) {
+ if
+ (SmramHobDescriptorBlock->Descriptor[Index].PhysicalStart >
0x100000) {
+ SmramSize += SmramHobDescriptorBlock-
Descriptor[Index].PhysicalSize;
+ if (SmramBase == 0 || SmramBase >
+ SmramHobDescriptorBlock-
Descriptor[Index].CpuStart) {
+ SmramBase = SmramHobDescriptorBlock-
Descriptor[Index].CpuStart;
+ }
+ }
+ }
+ break;
+ }
+ }
+ Hob.Raw = GET_NEXT_HOB (Hob);
+ }
+
+ //
+ // Set non system memory as UC
+ //
+ MemoryBase = 0x100000000;
+
+ //
+ // Add IED size to set whole SMRAM as WB to save MTRR count //
+ MemoryLength = MemoryBase - (SmramBase + SmramSize); while
+ (MemoryLength != 0) {
+ Power2Length = GetPowerOfTwo64 (MemoryLength);
+ MemoryBase -= Power2Length;
+ Status = MtrrSetMemoryAttributeInMtrrSettings (
+ &MtrrSetting,
+ MemoryBase,
+ Power2Length,
+ CacheUncacheable
+ );
+ ASSERT_EFI_ERROR (Status);
+ MemoryLength -= Power2Length;
+ }
+
+ DEBUG ((DEBUG_INFO, "PcdPciReservedMemAbove4GBLimit - 0x%lx\n",
+ PcdGet64 (PcdPciReservedMemAbove4GBLimit)));
+ DEBUG ((DEBUG_INFO, "PcdPciReservedMemAbove4GBBase - 0x%lx\n",
+ PcdGet64 (PcdPciReservedMemAbove4GBBase)));
+ if (PcdGet64 (PcdPciReservedMemAbove4GBLimit) > PcdGet64
(PcdPciReservedMemAbove4GBBase)) {
+ Status = MtrrSetMemoryAttributeInMtrrSettings (
+ &MtrrSetting,
+ PcdGet64
(PcdPciReservedMemAbove4GBBase),
+ PcdGet64
(PcdPciReservedMemAbove4GBLimit)
+ - PcdGet64
(PcdPciReservedMemAbove4GBBase) + 1,
+ CacheUncacheable
+ );
+ ASSERT_EFI_ERROR ( Status);
+ }
+
+ DEBUG ((DEBUG_INFO, "PcdPciReservedPMemAbove4GBLimit -
0x%lx\n",
+ PcdGet64 (PcdPciReservedPMemAbove4GBLimit)));
+ DEBUG ((DEBUG_INFO, "PcdPciReservedPMemAbove4GBBase -
0x%lx\n",
+ PcdGet64 (PcdPciReservedPMemAbove4GBBase)));
+ if (PcdGet64 (PcdPciReservedPMemAbove4GBLimit) > PcdGet64
(PcdPciReservedPMemAbove4GBBase)) {
+ Status = MtrrSetMemoryAttributeInMtrrSettings (
+ &MtrrSetting,
+ PcdGet64
(PcdPciReservedPMemAbove4GBBase),
+ PcdGet64
(PcdPciReservedPMemAbove4GBLimit)
+ - PcdGet64
(PcdPciReservedPMemAbove4GBBase) + 1,
+ CacheUncacheable
+ );
+ ASSERT_EFI_ERROR ( Status);
+ }
+
+ //
+ // Update MTRR setting from MTRR buffer // MtrrSetAllMtrrs
+ (&MtrrSetting);
+
+ return Status;
+}
diff --git
a/Platform/Intel/MinPlatformPkg/Library/SetCacheMtrrLib/SetCacheMtrrLi
b
Null.c
b/Platform/Intel/MinPlatformPkg/Library/SetCacheMtrrLib/SetCacheMtrrLi
b
Null.c
new file mode 100644
index 0000000000..4f40de35f4
--- /dev/null
+++
b/Platform/Intel/MinPlatformPkg/Library/SetCacheMtrrLib/SetCacheMtrr
+++ LibNull.c
@@ -0,0 +1,37 @@
+/** @file
+
+NULL instances of SetCacheMtrr library functions.
+
+Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#include <Uefi.h>
+#include <Library/PcdLib.h>
+#include <Library/DebugLib.h>
+
+/**
+ Set Cache Mtrr.
+**/
+VOID
+EFIAPI
+SetCacheMtrr (
+ VOID
+ )
+{
+ return;
+}
+
+/**
+ Update MTRR setting and set write back as default memory attribute.
+
+ @retval EFI_SUCCESS The function completes successfully.
+**/
+EFI_STATUS
+EFIAPI
+SetCacheMtrrAfterEndOfPei (
+ VOID
+ )
+{
+ return EFI_SUCCESS;
+}
diff --git
a/Platform/Intel/MinPlatformPkg/Include/Library/SetCacheMtrrLib.h
b/Platform/Intel/MinPlatformPkg/Include/Library/SetCacheMtrrLib.h
new file mode 100644
index 0000000000..0fb566dfcc
--- /dev/null
+++ b/Platform/Intel/MinPlatformPkg/Include/Library/SetCacheMtrrLib.h
@@ -0,0 +1,34 @@
+/** @file
+
+Header for SetCacheMtrr library functions.
+
+Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _SET_CACHE_MTRR_LIB_H_
+#define _SET_CACHE_MTRR_LIB_H_
+
+/**
+ Set Cache Mtrr.
+**/
+VOID
+EFIAPI
+SetCacheMtrr (
+ VOID
+ );
+
+/**
+ Update MTRR setting and set write back as default memory attribute.
+
+ @retval EFI_SUCCESS The function completes successfully.
+ @retval Others Some error occurs.
+**/
+EFI_STATUS
+EFIAPI
+SetCacheMtrrAfterEndOfPei (
+ VOID
+ );
+
+#endif
diff --git
a/Platform/Intel/MinPlatformPkg/Library/SetCacheMtrrLib/SetCacheMtrrLi
b
.inf
b/Platform/Intel/MinPlatformPkg/Library/SetCacheMtrrLib/SetCacheMtrrLi
b
.inf
new file mode 100644
index 0000000000..0cfdda414b
--- /dev/null
+++
b/Platform/Intel/MinPlatformPkg/Library/SetCacheMtrrLib/SetCacheMtrr
+++ Lib.inf
@@ -0,0 +1,46 @@
+## @file
+# Component information file for Platform SetCacheMtrr Library.
+# This library implementation is for typical platforms and may not be
+# needed when cache MTRR will be initialized by FSP.
+#
+# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> # #
+SPDX-License-Identifier: BSD-2-Clause-Patent # ##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = PeiSetCacheMtrrLib
+ FILE_GUID =
9F2A2899-3AD7-4176-9B89-33B3AC456A99
+ MODULE_TYPE = PEIM
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = SetCacheMtrrLib
+
+[LibraryClasses]
+ BaseLib
+ PcdLib
+ DebugLib
+ HobLib
+ MtrrLib
+ PeiServicesLib
+ BaseMemoryLib
+
+[Packages]
+ MinPlatformPkg/MinPlatformPkg.dec
+ MdePkg/MdePkg.dec
+ UefiCpuPkg/UefiCpuPkg.dec
+
+[Sources]
+ SetCacheMtrrLib.c
+
+[Guids]
+ gEfiSmmSmramMemoryGuid ##
CONSUMES
+
+[Pcd]
+ gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaBaseAddress
##
CONSUMES
+ gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize
##
CONSUMES
+ gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemAbove4GBBase
## CONSUMES
+ gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemAbove4GBLimit
##
+CONSUMES
+ gMinPlatformPkgTokenSpaceGuid.PcdPciReservedPMemAbove4GBBase
##
+CONSUMES
+ gMinPlatformPkgTokenSpaceGuid.PcdPciReservedPMemAbove4GBLimit
##
+CONSUMES
diff --git
a/Platform/Intel/MinPlatformPkg/Library/SetCacheMtrrLib/SetCacheMtrrLi
b
Null.inf
b/Platform/Intel/MinPlatformPkg/Library/SetCacheMtrrLib/SetCacheMtrrLi
b
Null.inf
new file mode 100644
index 0000000000..433bd47331
--- /dev/null
+++
b/Platform/Intel/MinPlatformPkg/Library/SetCacheMtrrLib/SetCacheMtrr
+++ LibNull.inf
@@ -0,0 +1,29 @@
+## @file
+# Component information file for Platform SetCacheMtrr Library.
+#
+# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> # #
+SPDX-License-Identifier: BSD-2-Clause-Patent # ##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = BaseSetCacheMtrrLibNull
+ FILE_GUID =
D1ED4CD7-AD20-4943-9192-3ABE766A9411
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = SetCacheMtrrLib
+
+[LibraryClasses]
+ BaseLib
+ PcdLib
+ DebugLib
+
+[Packages]
+ MinPlatformPkg/MinPlatformPkg.dec
+ MdePkg/MdePkg.dec
+
+[Sources]
+ SetCacheMtrrLibNull.c
+
diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
index d79f5ec1bd..a851021c0b 100644
--- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
+++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
@@ -65,6 +65,8 @@ SecBoardInitLib|Include/Library/SecBoardInitLib.h
TestPointLib|Include/Library/TestPointLib.h
TestPointCheckLib|Include/Library/TestPointCheckLib.h

+SetCacheMtrrLib|Include/Library/SetCacheMtrrLib.h
+
[PcdsFixedAtBuild, PcdsPatchableInModule]


gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperBootMode|FALSE|BOOLE
AN|0x80000008
@@ -204,11 +206,11 @@
gMinPlatformPkgTokenSpaceGuid.PcdPcIoApicEnable|0x0|UINT32|0x90000
019
gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemBase
|0x90000000 |UINT32|0x40010043
gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemLimit
|0x00000000 |UINT32|0x40010044
gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemAbove4GBBase
|0xFFFFFFFFFFFFFFFF |UINT64|0x40010045
- gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemAbove4GBLimit
|0x0000000000000000 |UINT64|0x40010046
+ gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemAbove4GBLimit
+ |0x0000000000000000 |UINT64|0x40010046
gMinPlatformPkgTokenSpaceGuid.PcdPciReservedPMemBase
|0xFFFFFFFF |UINT32|0x40010047
gMinPlatformPkgTokenSpaceGuid.PcdPciReservedPMemLimit
|0x00000000 |UINT32|0x40010048
gMinPlatformPkgTokenSpaceGuid.PcdPciReservedPMemAbove4GBBase
|0xFFFFFFFFFFFFFFFF |UINT64|0x40010049
-
gMinPlatformPkgTokenSpaceGuid.PcdPciReservedPMemAbove4GBLimit|0x
0000000000000000 |UINT64|0x4001004A
+
+
gMinPlatformPkgTokenSpaceGuid.PcdPciReservedPMemAbove4GBLimit|0x
000000
+ 0000000000 |UINT64|0x4001004A
gMinPlatformPkgTokenSpaceGuid.PcdPciDmaAbove4G
|FALSE|BOOLEAN|0x4001004B
gMinPlatformPkgTokenSpaceGuid.PcdPciNoExtendedConfigSpace
|FALSE|BOOLEAN|0x4001004C
gMinPlatformPkgTokenSpaceGuid.PcdPciResourceAssigned
|FALSE|BOOLEAN|0x4001004D
--
2.13.3.windows.1