patina-v22.2.0
What's Changed
-
SDK: Migrate to lzma-rust2 instead of patina-lzma-rs @os-d (#1611)
Change Details
## Description
patina-lzma-rs is a fork of the unmaintained lzma-rs crate that adds no_std support for it.
lzma-rust2 is a maintained fork of lzma-rs that already contains no_std support and large performance improvements over lzma-rs.
On an example AARCH64 platform, decompressing an LZMA compressed FV went from 2500 ms with patina-lzma-rs to 1314 ms with lzma-rust2.
Host based benchmarking shows similar performance improvements.
This commit moves patina over to the maintained crate with better performance that doesn't require a custom override.
The optimization feature is not enabled because it is primarily for encoding (which patina does not do) and does not support the FORMAT_ONLY compression type that our FV use, so the performance is the same but it adds unsafe code.
- Impacts functionality?
- Impacts security?
- Breaking change?
- Includes tests?
- Includes documentation?
How This Was Tested
Tested with local benchmarks and running on an AARCH64 platform.
Integration Instructions
N/A. The changed dependency will be picked up automatically. patina-lzma-rs will be deprecated.
-
Consistently apply LF line endings to .stderr files @makubacki (#1609)
Change Details
## Description
Currently LF is enforced for .rs files with rustfmt.
.stderrfiles are used to hold reference text for compile-time test messages.Some
.stderrfiles are checked in with CRLF line endings. Others with LF line endings. This change converts them to LF line endings and updates.gitattributesto convert.stderrfiles to LF when committed going forward.- Impacts functionality?
- Impacts security?
- Breaking change?
- Includes tests?
- Includes documentation?
How This Was Tested
cargo make all
Integration Instructions
- N/A
-
sdk: Prevent re-entrant lock acquisition in add\_fbpt\_record() @makubacki (#1595)
Change Details
## Description
Today, the FBPT
TplMutexcan be acquired re-entrantly, which can lead to a panic with "Re-entrant lock" when dropping a performance measurement's lock guard restores the TPL and dispatches a pending event notification whose callback creates another measurement before the guard finishes releasing the lock.This change modifies calls to acquire the FBPT
TplMutexto attempt the lock and, on contention, drop the record. Because the edk2 implementation ofcreate_performance_measurement()also returnsEfiError::InvalidParameterwhen the record is not added, this change does that as well so this scenario will be handled similarly in any pre-existing caller status code handling logic.- Impacts functionality?
- Impacts security?
- Breaking change?
- Includes tests?
- Includes documentation?
How This Was Tested
cargo make all- QEMU ArmVirt & Q35 boot
- Intel physical platform boot (w/ perf enabled)
Integration Instructions
- N/A
🐛 Bug Fixes
-
[REBASE \& FF] Always Allocate FV Regions @os-d (#1608)
Change Details
## Description
This PR contains five commits, two of which are set up/clean up for the third commit which is a bug fix. The final two commits are bugs noticed doing this work.
dxe: GCD: Get Descriptors With Closure, Not Filter
Currently, get_memory_descriptor_for_address will return any descriptor for an address. get_memory_descriptors accepts a filter to determine which descs to return.
This commit updates both fns to accept a closure so that custom logic can be applied to determine when a desc should
be returned. This removes increasingly complicated and use case specific descriptor filters in favor of a more flexible mechanism.dxe: GCD: Consolidate get_existent_memory_descriptor_for_address
Now that get_memory_descriptor_for_address accepts a closure, convert users of get_existent_memory_descriptor_for_address and drop the function.
dxe: GCD: Always Allocate FV Regions
Currently, the DXE Core initialization only attempts to allocate FV regions if they are marked as MMIO in the GCD. However, many platforms mark these regions as system memory (particularly for FVs that were decompressed into memory).
These regions are not allocated and so are free to allocate by regular allocations. This causes corruption in the FV and failure to parse it.
This commit changes the behavior from the EDK II behavior to always allocate FV regions if they are not already allocated (some platforms produce memory allocation HOBs for the FV region; also three FVs are produced for each extracted FV, so we only allocate it once).
This ensures that the FVs do not get corrupted.
dxe: GCD: Account For Direct Allocated GCD Memory in EFI_MEMORY_MAP
is_efi_memory_map_descriptor() currently assumes all system memory is efi::ConventionalMemory because it assumes that the allocators have been checked for this descriptor before it runs.
However, this misses direct allocated GCD memory (calling through DXE services).
This commit does three things:
- Move is_efi_memory_map_descriptor() from the SDK to the core so that it can directly call memory_type_for_handle() and be a complete function. Given any memory descriptor it will correctly report what the EFI_MEMORY_MAP type will be, instead of relying on memory_type_handle() being called first. It is not a useful function in the SDK, only intended for core use. It cannot be used by SDK consumers because it relies on a core internal function.
- Update the system memory logic in that function to check whether it is allocated and if the runtime flag is set.
- Add tests.
This was noticed because the FVs are directly allocated in the GCD.
dxe: GCD: Don't Assert on Non-Page Aligned GCD Allocations
This updates the EFI_MEMORY_MAP building logic to not assert on non-page aligned GCD allocations. FVs are expected to not be page aligned, but must be allocated to avoid being corrupted by other allocations.
This is a temporary workaround to log and drop the allocations. A longer term solution is tracked in #1614.
- Impacts functionality?
- Impacts security?
- Breaking change?
- Includes tests?
- Includes documentation?
How This Was Tested
Tested on a platform that had an FV starting at 0x1000. When the page table was allocated bottom up, the root landed at 0x1000. This overwrote the FV signature, causing Patina to fail to parse the FV and therefore could not dispatch any drivers. Obviously, it is incredibly dangerous to have the page table conflict with any other memory as well, though the FV should be read only.
After this change, the FV is marked as allocated and the page table lands on a different page and the system is able to read the FV and dispatch drivers.
Also booted to Windows on a physical Intel platform to confirm no regression.
Integration Instructions
N/A.
-
[REBASE \& FF] Check EL Before Setting HCR\_EL2 @os-d (#1603)
Change Details
## Description
This PR contains two commits:
SDK: Move get_current_el() to SDK and Introduce EL Enum
get_current_el() is a common AARCH64 function. It is currently being shared in patina_internal_cpu, but is broadly applicable.
This moves it to the SDK and introduces an EL enum for it to return instead of each caller needing to know the register values.
aarch64 intr: Only Set TGE if Running in EL2
When initializing exceptions, TGE is always set in HCR_EL2. However, this will hang the system when running at EL1.
Trapping general exceptions is not required at EL1, we are only getting the EL1 exceptions. As such, this commit just changes the code to check the current EL and only set the bit if we are running at EL2.
- Impacts functionality?
- Impacts security?
- Breaking change?
- Includes tests?
- Includes documentation?
How This Was Tested
Tested on a platform that runs at EL1. Before this change, it hangs, after this, it boots past this point.
Integration Instructions
N/A.
-
Prevent ldmxcsr from using absolute address @apop5 (#1593)
Change Details
## Description
Closes #1601
ldmxcsr was attempting to use an absolute address. On a X86 system, this resulted in a 0xF extended address
i.e. ldmxcsr dword ptr [0xffffffffa240bac8] when the value was at 0xa240bac8
Switching the syntax resulted in using a relative address (rip + n).
The absolute address was resulting in a GP# fault when executing.
- Impacts functionality?
- Impacts security?
- Breaking change?
- Includes tests?
- Includes documentation?
How This Was Tested
Virtual x86 system. GP fault before change, boot past after the change.
Integration Instructions
No integration necessary.
📖 Documentation Updates
-
Makefile.toml: Add `install-tools` task @makubacki (#1615)
Change Details
## Description
Resolves #1613
Patina maintains a list of project tools in the
[tools]section ofrust-toolchain.toml. While CI directly installs these tools, local developers have to do so manually. That was raised as a pain point as noted in the referenced GitHub issue.This commit adds a new
install-toolstask to the Makefile that will automatically install all tools listed in the[tools]section ofrust-toolchain.toml. The task first verifies thatcargo-binstallis installed, and if not, it installs it. Then it parses the[tools]section ofrust-toolchain.tomland installs each tool usingcargo binstall.- Impacts functionality?
- Impacts security?
- Breaking change?
- Includes tests?
- Includes documentation?
How This Was Tested
-
Run
cargo make install-toolswith some intentional tool issues: a tool missing (cargo-vet) and a tool on the wrong version (cargo-mdbook).
-
Run
cargo make install-toolswith everything installed on the correct versions:
Integration Instructions
- N/A - Local workspace task
-
Create `patina_internal_core` crate to merge common core code @cfernald (#1597)
Change Details
## Description
Merges existing
patina_internal_depexandpatina_internal_collectionsinto a new more commonpatina_internal_coreto reduce overhead and process for adding new shared code for the Patina DXE, MM, and supervisor cores.Implementation for RFC: #1594
- Impacts functionality?
- Impacts security?
- Breaking change?
- Includes tests?
- Includes documentation?
How This Was Tested
- Unit test pass
- Boot to shell on Q35
- Boot to shell on ArmVirt
Integration Instructions
N/A
-
Switch to the stable toolchain (and RUSTC\_BOOTSTRAP=1) @makubacki (#1612)
Change Details
## Description
Per RFC 0030, switch to the stable toolchain and set RUSTC_BOOTSTRAP=1 to allow the stable toolchain to accept the unstable feature gates Patina still depends on.
Sets
allow-featuresto the list of unstable features Patina uses.Update associated documentation to reflect the change.
Plan is to get a single change into this repo (this PR) that includes the exact changes from patina-devops + doc updates then the patina-devops changes will not make any subsequent updates to the files.
- patina-devops PR: OpenDevicePartnership/patina-devops#169
- Impacts functionality?
- Impacts security?
- Breaking change?
- Includes tests?
- Includes documentation?
How This Was Tested
cargo make all
Integration Instructions
- See RFC 0030 - Build on Stable Rust with RUSTC_BOOTSTRAP and the documentation updates in this change.
-
[FCP] RFC: Consolidate common core code into a single internal library crate @cfernald (#1594)
Change Details
## Description
This RFC proposes creating an internal crate (
core/patina_internal_lib) for code shared by core implementations. This single crate would remove overhead and simplify changing internal code and interfaces. This will become increasingly important with the introduction of the Standalone MM core.This RFC builds on the crate organization and naming conventions established by RFC: Categorizing and Renaming Patina Crates for Consistency.
- Impacts functionality?
- Impacts security?
- Breaking change?
- Includes tests?
- Includes documentation?
How This Was Tested
N/A
Integration Instructions
N/A
-
RFC: Build on Stable Rust with `RUSTC_BOOTSTRAP` @makubacki (#1592)
Change Details
## Description
Status: Accepted
Proposes pinning a stable toolchain (initially 1.95.0) and setting
RUSTC_BOOTSTRAP=1instead of building on nightly, to align with the stable release downstream projects ship on.The long-term plan to drop unstable features, and
RUSTC_BOOTSTRAPwith them, is unchanged.- Impacts functionality?
- Impacts security?
- Breaking change?
- Includes tests?
- Includes documentation?
How This Was Tested
- Markdown lint
Integration Instructions
- N/A
-
Docs: Update Adv Logger Doc @os-d (#1600)
Change Details
## Description
This corrects the adv logger doc to say that Patina cannot currently be configured to create the memory log and that hardware port log level is controlled by the creator of the log.
- Impacts functionality?
- Impacts security?
- Breaking change?
- Includes tests?
- Includes documentation?
How This Was Tested
N/A.
Integration Instructions
N/A.
Full Changelog: patina-v22.1.0...v22.2.0