Given how the meaning of the attribute bits for page table entry types
is slightly awkward, and changes between levels, add some helpers to
abstract from this.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
Reviewed-by: Ashish Singhal <ashishsingha@nvidia.com>
Tested-by: Ashish Singhal <ashishsingha@nvidia.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
FreePageTablesRecursive () traverses the page table tree depth first
to free all pages that it finds, without taking into account the
level at which it is operating.
Since TT_TYPE_TABLE_ENTRY aliases TT_TYPE_BLOCK_ENTRY_LEVEL3, we cannot
distinguish table entries from block entries unless we take the level
into account, and so we may be dereferencing garbage if we happen to
try and free a hierarchy of page tables that has level 3 pages in it.
Let's fix this by passing the level into FreePageTablesRecursive (),
and limit the recursion to levels < 3.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
Reviewed-by: Ashish Singhal <ashishsingha@nvidia.com>
Tested-by: Ashish Singhal <ashishsingha@nvidia.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Older GCC (<= 4.9) fail to infer that Parent is never used unless it
has been assigned before, and may throw an error like
/work/git/edk2/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c:
In function ‘PlatformPeim’:
/work/git/edk2/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c:132:24:
error: ‘Parent’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
RangesProp = fdt_getprop (Base, Parent, "ranges", &RangesLen);
Set Parent to 0 at the start of the sequence to work around this.
Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2601
Fixes: 82662a3b5f ("ArmVirtPkg/PlatformPeiLib: discover the TPM base ...")
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
GCC 4.8 or 4.9 may throw the following error when building OVMF:
Edk2/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c:
In function ‘QemuLoadKernelImage’:
Edk2/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c:416:30:
error: ‘CommandLine’ may be used uninitialized in this function
[-Werror=maybe-uninitialized]
UnicodeSPrintAsciiFormat (
cc1: all warnings being treated as errors
This is due to the fact that older GCCs fail to infer that CommandLine is
never actually used unless it has been assigned. So add a redundant NULL
assignment to help these older GCCs understand this.
Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2630
Fixes: 7c47d89003 ("OvmfPkg: implement QEMU loader library for X86 with ...")
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2575
The following components are currently missing from the [Components]
section of ArmPlatformPkg.dsc:
* ArmPlatformPkg/Library/HdLcd/HdLcd.inf
* ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.inf
This commit includes the components in the package DSC build.
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2574
The following components are currently missing from the [Components]
section of ArmPkg.dsc:
* ArmPkg/Drivers/ArmCrashDumpDxe/ArmCrashDumpDxe.inf
* ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
* ArmPkg/Library/ArmMtlNullLib/ArmMtlNullLib.inf
* ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
This commit includes the components in the package DSC build.
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2610
We meet a case that the DEC file declaring the PCD isn't
included in the INF.it cause build tools report Traceback error.
Remove raise statements that generate Tracebacks that were only
intended for development/debug. With the raise statements removed
proper error messages are shown.
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Zhiju.Fan <zhijux.fan@intel.com>
Reviewed-by: Bob Feng <bob.c.feng@intel.com>
BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=2365
This patch is to fix a build tool regression issue which was introduced
by commit 8ddec24dea.
compiler output message includes localized string.
So build failed when code decode the stdout/stderr byte arrays.
The cause of the build failed is that Commit 8ddec24dea
removed "errors='ignore'".
The build tool does not need to deal with localized string,
so we need to add "errors='ignore'".
this function is only invoked for structure PCDs.
Build failed if structurePcd is used in platform dsc file.
The patch is going to fixed this issue
Cc: Liming Gao <liming.gao@intel.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Signed-off-by: Zhiju.Fan <zhijux.fan@intel.com>
Reviewed-by: Bob Feng <bob.c.feng@intel.com>
Because of a bug, current EL gets passed to DC IVAC instruction instead
of the VA entry that needs to be invalidated.
Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2598
All registers definition of DVSEC are defined as per the PCI Express Base
Specification 4.0 chapter 7.9.6.
Signed-off-by: Ashraf Javeed <ashraf.javeed@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
ArmVirtGicArchLib was originally implemented before virtualization
emulation was implemented in QEMU, and the GICv2 model implemented only
the physical copy of control registers.
Enabling virtualization emulation to QEMU adds also the virtual copy,
doubling the RegSize returned by FindCompatibleNodeReg () in
ArmVirtGicArchLibConstructor (). This triggered an ASSERT when running
QEMU with -M virt,virtualization=on. Address this by testing for both
possible valid values of RegSize.
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2588
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Leif Lindholm <leif@nuviainc.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2582
warning C4244: '=': conversion from 'UINTN' to 'UINT32', possible loss of data
With this fix, OvmfIa32, OvmfX64 and OvmfIa32X64 can pass build.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Liming Gao <liming.gao@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Leif suggested that FDF include files should preferably refer with their
names to the FDF file sections from which they are included.
Therefore
- rename "OvmfPkg.fdf.inc" to "OvmfPkgDefines.fdf.inc" (included from the
[Defines] section),
- rename "DecomprScratchEnd.fdf.inc" to "FvmainCompactScratchEnd.fdf.inc"
(included under the [FV.FVMAIN_COMPACT] section).
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien@xen.org>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: http://mid.mail-archive.com/20200312142006.GG23627@bivouac.eciton.net
Ref: https://edk2.groups.io/g/devel/message/55812
Suggested-by: Leif Lindholm <leif@nuviainc.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200312223555.29267-3-lersek@redhat.com>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
Add a code comment that explains the nature of the NumberOfPages field
values. Including this kind of historical information was suggested by
Leif in <https://edk2.groups.io/g/devel/message/55797> (alternative link:
<http://mid.mail-archive.com/20200312104006.GB23627@bivouac.eciton.net>).
Right now, the most recent commit updating the page counts has been commit
991d956362 ("[...] Update default memory type information to reduce EFI
Memory Map fragmentation.", 2010-07-16).
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Suggested-by: Leif Lindholm <leif@nuviainc.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200312223555.29267-2-lersek@redhat.com>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
* In the Intel whitepaper:
--v--
A Tour Beyond BIOS -- Secure SMM Communication
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Security-White-Papershttps://github.com/tianocore-docs/Docs/raw/master/White_Papers/A_Tour_Beyond_BIOS_Secure_SMM_Communication.pdf
--^--
bullet#3 in section "Assumption and Recommendation", and bullet#4 in "Call
for action", recommend enabling the (adaptive) Memory Type Information
feature.
* In the Intel whitepaper:
--v--
A Tour Beyond BIOS -- Memory Map and Practices in UEFI BIOS
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-white-papershttps://github.com/tianocore-docs/Docs/raw/master/White_Papers/A_Tour_Beyond_BIOS_Memory_Map_And_Practices_in_UEFI_BIOS_V2.pdf
--^--
figure#6 describes the Memory Type Information feature in detail; namely
as a feedback loop between the Platform PEIM, the DXE IPL PEIM, the DXE
Core, and BDS.
Implement the missing PlatformPei functionality in OvmfPkg, for fulfilling
the Secure SMM Communication recommendation.
In the longer term, OVMF should install the WSMT ACPI table, and this
patch contributes to that.
Notes:
- the step in figure#6 where the UEFI variable is copied into the HOB is
covered by the DXE IPL PEIM, in the DxeLoadCore() function,
- "PcdResetOnMemoryTypeInformationChange" must be reverted to the DEC
default TRUE value, because both whitepapers indicate that BDS needs to
reset the system if the Memory Type Information changes.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=386
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200310222739.26717-6-lersek@redhat.com>
Acked-by: Leif Lindholm <leif@nuviainc.com>
FaultTolerantWritePei consumes:
- PcdFlashNvStorageFtwWorkingBase,
- PcdFlashNvStorageFtwSpareBase.
VariablePei consumes:
- PcdFlashNvStorageVariableBase64.
Due to the previous patches in this series, the above PCDs are available
in the PEI phase, in the SMM_REQUIRE build.
FaultTolerantWritePei produces a GUID-ed HOB with
FAULT_TOLERANT_WRITE_LAST_WRITE_DATA as contents. It also installs a Null
PPI that carries the same gEdkiiFaultTolerantWriteGuid as the HOB.
VariablePei depends on the Null PPI mentioned above with a DEPEX, consumes
the HOB (which is safe due to the DEPEX), and produces
EFI_PEI_READ_ONLY_VARIABLE2_PPI.
This enables read-only access to non-volatile UEFI variables in the PEI
phase, in the SMM_REQUIRE build.
For now, the DxeLoadCore() function in
"MdeModulePkg/Core/DxeIplPeim/DxeLoad.c" will not access the
"MemoryTypeInformation" variable, because OVMF's PlatformPei always
produces the MemoryTypeInformation HOB.
(Note: when the boot mode is BOOT_ON_S3_RESUME, PlatformPei doesn't build
the HOB, but that's in sync with DxeLoadCore() also not looking for either
the HOB or the UEFI variable.)
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=386
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200310222739.26717-5-lersek@redhat.com>
Acked-by: Leif Lindholm <leif@nuviainc.com>
The following flash-related base addresses:
- PcdFlashNvStorageVariableBase64,
- PcdFlashNvStorageFtwWorkingBase,
- PcdFlashNvStorageFtwSpareBase,
are always set to constant (invariable) values in the "-D SMM_REQUIRE"
build of OVMF. (That's because in the SMM build, actual pflash is a hard
requirement, and the RAM-based emulation is never available.)
Set said PCDs statically, at build. This will allow us to depend on their
values in the PEI phase.
When SMM_REQUIRE is FALSE, this change has no effect (confirmed by report
file comparison).
When SMM_REQUIRE is TRUE, the report file shows the following changes:
- "PcdOvmfFlashNvStorageFtwSpareBase" and
"PcdOvmfFlashNvStorageFtwWorkingBase" are no longer consumed by any
module directly,
- for "PcdFlashNvStorageFtwSpareBase", "PcdFlashNvStorageFtwWorkingBase"
and "PcdFlashNvStorageVariableBase64", the access method changes from
DYN to FIXED,
- for the latter PCDs, the zero (dynamic default) values are replaced with
the desired constants.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=386
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200310222739.26717-4-lersek@redhat.com>
Acked-by: Leif Lindholm <leif@nuviainc.com>
Extract the dynamic setting of the
- PcdFlashNvStorageVariableBase64
- PcdFlashNvStorageFtwWorkingBase
- PcdFlashNvStorageFtwSpareBase
addresses to a helper function.
For now, the helper function is identical (duplicated) between the SMM
flash driver and the runtime DXE flash driver. In subsequent patches, this
will change.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=386
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200310222739.26717-3-lersek@redhat.com>
Acked-by: Leif Lindholm <leif@nuviainc.com>
The only two OvmfPkg references to "PcdFlashNvStorageVariableBase" are the
spurious ones in the runtime DXE driver and the SMM driver INF files of
the QEMU flash driver. Remove these references.
The flash driver does not access "PcdOvmfFlashNvStorageEventLogBase"
either, so remove that from the INF files too.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=386
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200310222739.26717-2-lersek@redhat.com>
Acked-by: Leif Lindholm <leif@nuviainc.com>
From the function description of GetIfrBinaryData(), FormSetGuid can be
NULL. However, FormSetGuid is passed to IsZeroGuid(). This causes exception
when FormSetGuid is NULL.
Signed-off-by: Nickle Wang <nickle.wang@hpe.com>
Reviewed-by: Dandan Bi <dandan.bi@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2580
Ovmf build failed on Windows with VS2017 tool chain.
The error message like:
OvmfPkg\LinuxInitrdDynamicShellCommand\LinuxInitr
dDynamicShellCommand.c(199): error C2220: warning treated as error -
no 'object' file generated
OvmfPkg\LinuxInitrdDynamicShellCommand\LinuxInitrdDynamicShellCommand.c(199):
warning C4244: '=': conversion from 'UINT64' to 'UINTN',
possible loss of data
This patch is to cast UINT64 type to UINTN type
when doing the variable assignment.
Signed-off-by: Bob Feng <bob.c.feng@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2568
PcdValueInit shares the same Edk2\BaseTools\Source\C\PcdValueCommon.c.
To avoid the conflict, it should copy this file to its output directory,
If so, PcdValueCommon.c file will be private for PcdValueInit
Cc: Liming Gao <liming.gao@intel.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Signed-off-by: Zhiju.Fan <zhijux.fan@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
Reviewed-by: Bob Feng <bob.c.feng@intel.com>
Similarly to the "cadence" mentioned in commit d272449d9e ("OvmfPkg:
raise DXEFV size to 11 MB", 2018-05-29), it's been ~1.75 years, and we've
outgrown DXEFV again. Increase the DXEFV size to 12MB now.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Gary Lin <glin@suse.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2585
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200310175025.18849-1-lersek@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
This is enabling a future EVP implementation to utilize the
EVP_get_digestbyname() function.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Signed-off-by: Christopher J Zurcher <christopher.j.zurcher@intel.com>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
This prevents the .inf files from being randomized after every run
of process_files.pl.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Signed-off-by: Christopher J Zurcher <christopher.j.zurcher@intel.com>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Some cosmetic fixups to the AArch64 MMU code:
- reflow overly long lines unless it hurts legibility
- add/remove whitespace according to the [de facto] coding style
- use camel case for goto labels
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Message-Id: <20200307091008.14918-3-ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
This is the AARCH64 counterpart of commit 1f3b1eb308, to remove
a pointless check against the memory type of the allocations that the
page tables happened to land in. On ArmV8, we use writeback cacheable
exclusively for all memory.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Message-Id: <20200307091008.14918-2-ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
As it turns out, ARMv8 also permits accesses made with the MMU and
caches off to hit in the caches, so to ensure that any modifications
we make before enabling the MMU are visible afterwards as well, we
should invalidate page tables right after allocation like we do now on
ARM, if the MMU is still disabled at that point.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
Message-Id: <20200307083849.8940-3-ard.biesheuvel@linaro.org>
Replace the slightly overcomplicated page table management code with
a simplified, recursive implementation that should be far easier to
reason about.
Note that, as a side effect, this extends the per-entry cache invalidation
that we do on page table entries to block and page entries, whereas the
previous change inadvertently only affected the creation of table entries.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Message-Id: <20200307083849.8940-2-ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
When the MDE_CPU_IA32 macro is not defined, there is no access to the
"KernelImageHandle" local variable in QemuStartKernelImage(). This breaks
the OvmfPkgIa32X64 and OvmfPkgX64 platform builds, at least with gcc-8.
Move the local variable to the inner scope, where declaration and usage
are inseparable.
(Note that such inner-scope declarations are frowned upon in the wider
edk2 codebase, but we use them liberally in ArmVirtPkg and OvmfPkg anyway,
because they help us reason about variable lifetime and visibility.)
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Fixes: 7c47d89003
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2572
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Bob reports that VS2017 chokes on a tentative definition of the const
object 'mEfiFileProtocolTemplate', with the following error:
OvmfPkg\QemuKernelLoaderFsDxe\QemuKernelLoaderFsDxe.c(130):
error C2220: warning treated as error - no 'object' file generated
OvmfPkg\QemuKernelLoaderFsDxe\QemuKernelLoaderFsDxe.c(130):
warning C4132: 'mEfiFileProtocolTemplate': const object should be initialized
Let's turn the only function that relies on this tentative definition
into a forward declaration itself, and move its definition after the
external definition of the object. That allows us to drop the tentative
definition of the const object, and hopefully make VS2017 happy.
Cc: "Feng, Bob C" <bob.c.feng@intel.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Commit 859b55443a ("OvmfPkg/PlatformBootManagerLib: switch to
QemuLoadImageLib") replaced a dependency on LoadLinuxLib with one on
QemuLoadImageLib in the PlatformBootManagerLib implementation that is
shared between all OVMF builds, without taking into account that even
the Xen targeted builds incorporate this code, which is only used to
load kernels passed via the QEMU command line.
Since this is dead code on Xen, we can satisfy the dependency using
the generic version of QemuLoadImageLib, which does not rely on
LoadLinuxLib, which we can therefore drop from OvmfXen.dsc.
Fixes: 859b55443a
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
We already expect normal memory to be mapped writeback cacheable if
EDK2 itself is to make use of it, so doing an early sanity check on
the memory type of the allocation that the page tables happened to
land in isn't very useful. So let's drop it.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
The expression passed into ArmSetTTBR0 () in ArmConfigureMmu() is
sub-optimal at several levels:
- TranslationTable is already aligned, and if it wasn't, doing it
here wouldn't help
- TTBRAttributes is guaranteed not to have any bits set outside of
the 0x7f mask, so the mask operation is pointless as well,
- an additional (UINTN) cast for good measure is also not needed.
So simplify the expression.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
On ARMv7 and up, doing cache maintenance by set/way is only
permitted in the context of on/offlining a core, and any other
uses should be avoided. Add ASSERT()s in the right place to
ensure that any uses with the MMU enabled are caught in DEBUG
builds.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
ArmLib is a BASE type library, which should not depend or
even be aware on DXE type protocols. So drop the reference
to gEfiCpuArchProtocolGuid.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
Suspiciously, ArmLib's INF does not contain a [LibraryClasses]
section at all, but it turns out that all the library includes
it contains (except for ArmLib.h itself) are actually bogus so
let's just drop all of them. While at it, replace <Uefi.h> with
the more accurate <Base.h> for a BASE type module, and put the
includes in a consistent order.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
The clean/invalidate helper functions that operate on a single cache
line identified by set, way and level in a special, architected format
are only used by the implementations of the clean/invalidate routines
that operate on the entire cache hierarchy, as exposed by ArmLib.
The latter routines will be deprecated soon, so move the helpers out
of ArmLib.h and into a private header so they are safe from abuse.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
In the AARCH64 version of ArmMmuLib, we are currently relying on
set/way invalidation to ensure that the caches are in a consistent
state with respect to main memory once we turn the MMU on. Even if
set/way operations were the appropriate method to achieve this, doing
an invalidate-all first and then populating the page table entries
creates a window where page table entries could be loaded speculatively
into the caches before we modify them, and shadow the new values that
we write there.
So let's get rid of the blanket clean/invalidate operations, and
instead, update ArmUpdateTranslationTableEntry () to invalidate each
page table entry *after* it is written if the MMU is still disabled
at this point.
On ARMv8, it is guaranteed that memory accesses done by the page table
walker are cache coherent, and so we can ignore the case where the
MMU is on.
Since the MMU and D-cache are already off when we reach this point, we
can drop the MMU and D-cache disables as well. Maintenance of the I-cache
is unnecessary, since we are not modifying any code, and the installed
mapping is guaranteed to be 1:1. This means we can also leave it enabled
while the page table population code is running.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
In the ARM version of ArmMmuLib, we are currently relying on set/way
invalidation to ensure that the caches are in a consistent state with
respect to main memory once we turn the MMU on. Even if set/way
operations were the appropriate method to achieve this, doing an
invalidate-all first and then populating the page table entries creates
a window where page table entries could be loaded speculatively into
the caches before we modify them, and shadow the new values that we
write there.
So let's get rid of the blanket clean/invalidate operations, and instead,
invalidate each page table right after allocating it, and each section
entry after it is updated (to address all the little corner cases that the
ARMv7 spec permits), and invalidate sets of level 2 entries in blocks,
using the generic invalidation routine from CacheMaintenanceLib
On ARMv7, cache maintenance may be required also when the MMU is
enabled, in case the page table walker is not cache coherent. However,
the code being updated here is guaranteed to run only when the MMU is
still off, and so we can disregard the case when the MMU and caches
are on.
Since the MMU and D-cache are already off when we reach this point, we
can drop the MMU and D-cache disables as well. Maintenance of the I-cache
is unnecessary, since we are not modifying any code, and the installed
mapping is guaranteed to be 1:1. This means we can also leave it enabled
while the page table population code is running.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
Instead of overallocating memory and align the resulting base address
manually, use the AllocateAlignedPages () helper, which achieves the
same, and might even manage that without leaking a chunk of memory of
the same size as the allocation itself.
While at it, fix up a variable declaration in the same hunk, and drop
a comment whose contents add nothing to the following line of code.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Unlike the AArch64 implementation of ArmMmuLib, which combines the
initial page table population code with the code that runs at later
stages to manage permission attributes in the page tables, ARM uses
two completely separate sets of routines for this.
Since ArmMmuLib is a static library, we can prevent duplication of
this code between different users, which usually only need one or
the other. (Note that LTO should also achieve the same.)
This also makes it easier to reason about modifying the cache
maintenance handling, and replace the set/way ops with by-VA
ops, since the code that performs the set/way ops only executes
when the MMU is still off.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
Make the CONSTRUCTOR define in the .INF AARCH64 only, so we can drop
the empty stub that exists for ARM.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
Cache maintenance operations by set/way are only intended to be used
in the context of on/offlining a core, while it has been taken out of
the coherency domain. Any use intended to ensure that the contents of
the cache have made it to main memory is unreliable, since cacheline
migration and non-architected system caches may cause these contents
to linger elsewhere, without being visible in main memory once the
MMU and caches are disabled.
In KVM on Linux, there are horrid hacks in place to ensure that such
set/way operations are trapped, and replaced with a single by-VA
clean/invalidate of the entire guest VA space once the MMU state
changes, which can be costly, and is unnecessary if we manage the
caches a bit more carefully, and perform maintenance by virtual
address only.
So let's get rid of the call to ArmInvalidateDataCache () in the
PrePeiCore startup code, and instead, invalidate the UEFI memory
region by virtual address, which is the only memory region we will
be touching with the caches and MMU both disabled and enabled.
(This will lead to data corruption if data written with the MMU off
is shadowed by clean, stale cachelines that stick around when the
MMU is enabled again.)
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Pete Batard <pete@akeo.ie>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
This wasn't correctly testing for FD to be outside RAM,
when RAM base immediately follows the FD.
This is part of some cleanup for RPi4 in edk2-platform.
Signed-off-by: Andrei Warkentin <awarkentin@vmware.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
The QemuLoadImageLib implementation we currently use for all OVMF
builds copies the behavior of the QEMU loader code that precedes it,
which is to disregard UEFI secure boot policies entirely when it comes
to loading kernel images that have been specified on the QEMU command
line. This behavior deviates from ArmVirtQemu based builds, which do
take UEFI secure boot policies into account, and refuse to load images
from the command line that cannot be authenticated.
The disparity was originally due to the fact that the QEMU command line
kernel loader did not use LoadImage and StartImage at all, but this
changed recently, and now, there are only a couple of reasons left to
stick with the legacy loader:
- it permits loading images that lack a valid PE/COFF header,
- it permits loading X64 kernels on IA32 firmware running on a X64
capable system.
Since every non-authentic PE/COFF image can trivially be converted into
an image that lacks a valid PE/COFF header, the former case can simply
not be supported in a UEFI secure boot context. The latter case is highly
theoretical, given that one could easily switch to native X64 firmware in
a VM scenario.
That leaves us with little justification to use the legacy loader at all
when UEFI secure boot policies are in effect, so let's switch to the
generic loader for UEFI secure boot enabled builds.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Linux v5.7 will introduce a new method to load the initial ramdisk
(initrd) from the loader, using the LoadFile2 protocol installed on a
special vendor GUIDed media device path.
Add support for this to our QEMU command line kernel/initrd loader.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Replace the open coded sequence to load Linux on x86 with a short and
generic sequence invoking QemuLoadImageLib, which can be provided by
a generic version that only supports the LoadImage and StartImage boot
services, and one that incorporates the entire legacy loading sequence
as well.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Add the components that expose the QEMU abstract loader file system so
that we can switch over our PlatformBmLib over to it in a subsequent
patch.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Ard Biesheuvel
Michael Kubacki
Fan, ZhijuX
Ashish Singhal
Javeed, Ashraf
Leif Lindholm
Liming Gao
Laszlo Ersek
Nickle Wang
Daniel Schaefer
Bob Feng
Zurcher, Christopher J
Andrei Warkentin