Integrate major branch changes into main [REBASE & FF] - #1671
Conversation
QEMU Validation FailedQEMU validation did not complete successfully or did not shutdown as expected. Workflow run: https://github.com/OpenDevicePartnership/patina/actions/runs/30662770960
|
| Job | Result |
|---|---|
| Gather Incoming PR Metadata | ✅ |
| Run Patina QEMU Validation / Post In-Progress Notification | ✅ |
| Run Patina QEMU Validation / Preflight Checks | ✅ |
| Run Patina QEMU Validation / Get Constants / Get Repository Constants | ✅ |
| Run Patina QEMU Validation / Validate QEMU Q35 (Windows) | ❌ |
| Run Patina QEMU Validation / Validate QEMU - ArmVirt (Linux) | ❌ |
| Run Patina QEMU Validation / Validate QEMU - Q35 (Linux) | ❌ |
| Run Patina QEMU Validation / Emit PR Metadata | ✅ |
Error Details
qemu-validation-logs-Windows-Q35/q35-windows.log (5 error/warning sections)
error[E0432]: unresolved import `patina::boot_services`
--> src\q35\component\service\smbios_test.rs:17:13
|
17 | use patina::boot_services::{BootServices, StandardBootServices};
| ^^^^^^^^^^^^^ could not find `boot_services` in `patina`
error[E0603]: module `base` is private
--> src\q35\component\service\mm_config_provider.rs:96:50
|
96 | hob.pages as usize * patina::base::UEFI_PAGE_SIZE,
| ^^^^ private module
|
note: the module `base` is defined here
--> D:\a\patina\patina\sdk\patina\src\lib.rs:21:1
|
21 | mod base;
| ^^^^^^^^
help: consider importing this constant instead
|
96 - hob.pages as usize * patina::base::UEFI_PAGE_SIZE,
96 + hob.pages as usize * patina::UEFI_PAGE_SIZE,
|
error: could not compile `qemu_dxe_core` (lib) due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
[cargo-make] ERROR - Error while running duckscript: Source: Unknown Line: 113 - Error while executing command, exit code: 101
qemu-validation-logs-Linux-ArmVirt/armvirt-linux.log (5 error/warning sections)
error[E0432]: unresolved import `patina::boot_services`
--> src/q35/component/service/smbios_test.rs:17:13
|
17 | use patina::boot_services::{BootServices, StandardBootServices};
| ^^^^^^^^^^^^^ could not find `boot_services` in `patina`
error[E0603]: module `base` is private
--> src/q35/component/service/mm_config_provider.rs:96:50
|
96 | hob.pages as usize * patina::base::UEFI_PAGE_SIZE,
| ^^^^ private module
|
note: the module `base` is defined here
--> /__w/patina/patina/sdk/patina/src/lib.rs:21:1
|
21 | mod base;
| ^^^^^^^^
help: consider importing this constant instead
|
96 - hob.pages as usize * patina::base::UEFI_PAGE_SIZE,
96 + hob.pages as usize * patina::UEFI_PAGE_SIZE,
|
error: could not compile `qemu_dxe_core` (lib) due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
[cargo-make] ERROR - Error while running duckscript: Source: Unknown Line: 113 - Error while executing command, exit code: 101
qemu-validation-logs-Linux-Q35/q35-linux.log (5 error/warning sections)
error[E0432]: unresolved import `patina::boot_services`
--> src/q35/component/service/smbios_test.rs:17:13
|
17 | use patina::boot_services::{BootServices, StandardBootServices};
| ^^^^^^^^^^^^^ could not find `boot_services` in `patina`
error[E0603]: module `base` is private
--> src/q35/component/service/mm_config_provider.rs:96:50
|
96 | hob.pages as usize * patina::base::UEFI_PAGE_SIZE,
| ^^^^ private module
|
note: the module `base` is defined here
--> /__w/patina/patina/sdk/patina/src/lib.rs:21:1
|
21 | mod base;
| ^^^^^^^^
help: consider importing this constant instead
|
96 - hob.pages as usize * patina::base::UEFI_PAGE_SIZE,
96 + hob.pages as usize * patina::UEFI_PAGE_SIZE,
|
error: could not compile `qemu_dxe_core` (lib) due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
[cargo-make] ERROR - Error while running duckscript: Source: Unknown Line: 113 - Error while executing command, exit code: 101
Dependencies
| Repository | Ref |
|---|---|
| patina | 1e631a0 |
| patina-dxe-core-qemu | f11ec63 |
| patina-fw-patcher | d06ba7e |
| patina-qemu firmware | v4.0.5 |
| patina-qemu build script | d629f65 |
This comment was automatically generated by the Patina QEMU PR Validation Post workflow.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
Since we've already reviewed the content changes, I was mostly looking at commit organization and high-level changes.
|
|
@makubacki, I've implemented your suggestions. |
The current `SerialIO` implementation takes `&self` instead of `&mut self` for its method, forcing all serial implementations to implement interior mutability. However, because of the nature of the serial port use cases, the typical locking mechanisms that avoid deadlocks, such as the `TplMutex` is not readily accessible. Because of this, internal implementations have one-off, and at times questionable, interior mutability implementations. This commit removes the expectation on the serial port to ensure this, and instead provides a re-usable `SharedSerial` wrapper that implements interior mutability in a suitable way for serial port use cases.
This commit takes advantage of the new mutable references to cleanup the serial port implementations. Namely by dropping re-initialization of interior references used as a workaround for the lack of mutability in the `SerialIO` trait.
This commit splits the null, 16550, and pl011 UART implementations into their own submodules under the `uart` module. This better aligns to general code organization practices. This additionally removes some unnecessary conditional complication. This commit also adds some basic tests for the uart implementations.
…penDevicePartnership#1643) This commit removes the deprecated `without_transport_init` function. This function is now a no-op and the inverse function `with_transport_init` is now the control for this behavior.
This commit addresses multiple issues with the current performance structure 1. Performance data is not available until the component is dispatched. This can cause significant misrepresentation of the DXE execution time and cause events from images to be lost if not dispatched in the first loop. 2. Performance implementations in the SDK require SDK statics, which is generally an SDK antipattern. This is address by the following: 1. Moves the core performance implementation into the core, into a new struct names `CorePerformance`, following the pattern established by the timer and memory management. 2. Creates a new service trait `PerformanceMeasurement` that exposes this core support. 3. Updates the component to now use this core service to produce the protocol & publish tables. 4. Change initialization and configuration of performance measurements to be done by the core. 5. Updates all documentation according to these changes 6. Some smaller rework and restructuring
This commit introduces a new `standard` module in the Patina SDK, which re-exports the `r_efi` crate. This is in it's own module to be able to communicate the fact that this is contains standard without Patina-specific extension. This commit also updates all uses of r_efi in the patina repro to instead use the r_efi republished from the patina crate, and removing r_efi from individual cargo dependencies.
…lity This commit creates well-defined top-level modules, documents them, and moves existing modules and implementations into their new appropriate locations. This commit additionally clarifies comments in various modules for this refactor.
This updates the name of the error type from `IncompatibleError` to `IncompatibleVersion` to match the specification name.
This commit re-organizes the SDK to better align definitions to their respective modules, and normalized the naming and paths of definitions. All of the `Patina::base::*` is republished from the Patina root. This flattens callers' imports and makes accessing common base types easier.
…ip#1660) Moves several protocol definitions from the `uefi` module in the SDK and move to their appropriate location: - `DecompressProtocol`: A duplicate definition with `r_efi` that carried implementation details that were not needed in the SDK. - Switched to use the `r_efi` version directly - moved implementation to dxe_core - `EdkiiPerformanceMeasurementProtocol`: An EDKII protocol, that should only be produced and not consumed by Patina code - Moved the definition to `patina_performance` that produces the protocol. - `StatusCodeRuntimeProtocol`: A wrapper around the PI definition providing a convenience function - Moved the convenience function directly onto the `pi` module definition
Moves the global `#![allow(static_mut_refs)]` attributes to the test modules that require it, rather than having it at the crate level. This change improves code safety and clarity by limiting the scope of this allowance to only where it's needed.
This is a breaking change to public APIs in the SDK to use the new UEFI string types where appropriate. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
This is a breaking change to use `&Char16Str` as the filename. Some other minor integrating changes in patina_dxe_core for the new Patina SDK UEFI string types are included. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Updates the code to use the new string types modified in public APIs used by components that were made as breaking changes. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
## Description Closes OpenDevicePartnership#1673 Updates the Minimum Supported Rust Version (MSRV) to 1.90.0. This update is being made at this time to allow dependencies with an MSRV of 1.90.0 that have security advisories to be updated. - [x] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [ ] Includes tests? - [ ] Includes documentation? ## How This Was Tested - `cargo make all` ## Integration Instructions - N/A Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
|
Overriding Patina QEMU PR Validation since build failure in patina-dxe-core-qemu is expected as this is merging breaking changes in the |
Description
Merged the following changes from the major branch
PR #1628 — [major] Refactor performance into core service
Description
This PR addresses multiple issues with the current performance structure:
misrepresentation of the DXE execution time and cause events from images to be lost if not
dispatched in the first loop.
This PR does the following:
CorePerformance,following the pattern established by the timer and memory management.
PerformanceMeasurementthat exposes this core support.Resolves: #1483
Reviewing this PR
This PR is intended to be merged as a single commit. The individual commits do attempt to follow a
basic progression of the code rework but may be incomplete at the partial steps. Recommended review flow:
performance_measurement.md) to understand the intended structure.PerformanceMeasurement).PR #1630 — [major] Remove
SerialIOinterior mutability and cleanup serial implementations [REBASE & FF]Description
Change SerialIO trait to be mutable
The current
SerialIOimplementation takes&selfinstead of&mut selffor its methods, forcingall serial implementations to implement interior mutability. However, because of the nature of the
serial port use cases, the typical locking mechanisms that avoid deadlocks, such as the
TplMutex,are not readily accessible. Because of this, internal implementations have one-off, and at times
questionable, interior mutability implementations.
This commit removes the expectation on the serial port to ensure this, and instead provides a
re-usable
SharedSerialwrapper that implements interior mutability in a suitable way for serialport use cases.
patina: Simplify serial implementations
This commit takes advantage of the new mutable references to cleanup the serial port implementations.
Namely by dropping re-initialization of interior references used as a workaround for the lack of
mutability in the
SerialIOtrait.patina: split uart module submodules
This commit splits the null, 16550, and pl011 UART implementations into their own submodules under
the
uartmodule. This better aligns to general code organization practices. This additionallyremoves some unnecessary conditional complication.
PR #1643 — [major] patina_debugger: remove deprecated
without_transport_initfunctionDescription
This PR removes the deprecated
without_transport_initfunction for the next major release. Thisfunction is now a no-op and the inverse function
with_transport_initis now the control for this behavior.PR #1646 — [major] Refactor Patina SDK for stronger code organization [REBASE & FF]
Description
This PR organizes the SDK into well-defined modules, and moves code and modules into their appropriate location.
The main goals of this refactor are to:
Reviewing this PR
The commits have been organized into a basic pattern for the sake of review. All the Patina SDK
changes are kept in their own commits and all the mass-edits for consumers are kept in separate
squash: ...commits. This is to keep the core changes clear and reviewable; these are squashedinto their respective change before merging.
There are three significant steps in this PR:
Top level modules
A detailed before/after view of the "New Patina Structure" is included in the original PR, showing the
reorganization of files (e.g.
serial.rs→standard.rs,uefi_protocol→protocol,protocols→protocol, newperipheralanddebugmodules, r_efi re-export viastandard, etc.).PR #1660 — [major] Remove unnecessary protocol definitions from SDK
Description
Moves several protocol definitions from the
uefimodule in the SDK to their appropriate location:DecompressProtocol: A duplicate definition withr_efithat carried implementation details that were not needed in the SDK.r_efiversion directly.EdkiiPerformanceMeasurementProtocol: An EDKII protocol, that should only be produced and not consumed by Patina code.patina_performancethat produces the protocol.StatusCodeRuntimeProtocol: A wrapper around the PI definition providing a convenience function.pimodule definition.PR #1666 — [major] SDK Refactor: Republish base, update consumers, and cleanup [REBASE & FF]
Description
Second phase of large Patina SDK refactor, including:
Add
CorePerformanceto servicesThe performance refactor left out declaring the
CorePerformanceservice in the derive service macrofor
CorePerformance. This commit simply adds the missing service declaration.SDK Refactor: Consume r_efi republish from patina crate
This commit changes all uses of r_efi in the patina repo to instead use the r_efi republished from
the patina crate, and removes r_efi from individual cargo dependencies.
SDK Refactor: Publish all of base from Patina root
This commit republishes all of
Patina::base::*from the Patina root. This flattens callers' importsand makes accessing common base types easier.
patina: Scope allow(static_mut_refs) only to necessary tests
Moves the global
#![allow(static_mut_refs)]attributes to the test modules that require it, ratherthan having it at the crate level. This change improves code safety and clarity by limiting the scope
of this allowance to only where it's needed.
PR #1669 — [major] Breaking changes for Patina SDK UEFI Strings [Rebase & FF]
Description
Breaking changes to use the new UEFI string
types that have been deferred until this PR to the
majorbranch.sdk: Use Patina SDK UEFI string types in public APIs
This is a breaking change to public APIs in the SDK to use the new UEFI string types where appropriate.
patina_dxe_core: Use
Char16Strinfilesystems::SimpleFile::openThis is a breaking change to use
&Char16Stras the filename. Some other minor integrating changes inpatina_dxe_core for the new Patina SDK UEFI string types are included.
components: Integrate Patina SDK UEFI string changes
Updates the code to use the new string types modified in public APIs used by components that were made
as breaking changes.
PR #1674 — [major] Update the MSRV to 1.90.0
Description
Closes #1673
Updates the Minimum Supported Rust Version (MSRV) to 1.90.0. This update is being made at this time to
allow dependencies with an MSRV of 1.90.0 that have security advisories to be updated.
How This Was Tested
Integration Instructions
Performance Refactor
Platforms must change use of with_measurements from the patina_performance to instead override PlatformInfo::default_performance_config()
SerialIOTrait ChangesDebugger Changes
Callers should remove use of
without_transport_initwhich is already deprecatedPatina SDK Refactor
This change consists of major refactoring to the SDK. Consumers should ensure the following
Consume standard::efi instead of using r_efi to ensure consistent compilation.
It is not practical to enumerate all possible changes here, but if there is any ambiguity, reach out to the core Patina team.
String Based Interface Changes
Review the public API changes and adjust consuming code accordingly. Any callers in the patina repo are updated in this PR.
Rust Version
MSRV Updated to 1.9. Consumers must update as well.