[major] Simplify patina_performance config, Add SMBIOS types to Patina, R-EFI 6.0 Migration#1548
Conversation
QEMU Validation FailedQEMU validation did not complete successfully or did not shutdown as expected. Workflow run: https://github.com/OpenDevicePartnership/patina/actions/runs/27040535014
|
| 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 - Q35 (Linux) | ❌ |
| Run Patina QEMU Validation / Validate QEMU - SBSA (Linux) | ❌ |
| Run Patina QEMU Validation / Emit PR Metadata | ✅ |
Error Details
qemu-validation-logs-Windows-Q35/q35-windows.log (15 error/warning sections)
… (truncated)
error[E0308]: mismatched types
--> src\q35\component\service\smbios_platform.rs:125:30
|
125 | security_status: 0x02,
| ^^^^ expected `SecurityStatus`, found integer
error[E0308]: mismatched types
--> src\q35\component\service\smbios_platform.rs:155:28
|
155 | feature_flags: 0x01, // Board is a hosting board
| ^^^^ expected `FeatureFlags`, found integer
|
help: call `Into::into` on this expression to convert `{integer}` into `FeatureFlags`
|
155 | feature_flags: 0x01.into(), // Board is a hosting board
| +++++++
error[E0308]: mismatched types
--> src\q35\component\service\smbios_platform.rs:158:25
|
158 | board_type: 0x0A, // Motherboard
| ^^^^ expected `BoardType`, found integer
error[E0308]: mismatched types
--> src\q35\component\service\smbios_test.rs:45:49
|
45 | boot_services.locate_protocol_unchecked(&SMBIOS_PROTOCOL_GUID, core::ptr::null_mut()).map_err(|e| {
| ------------------------- ^^^^^^^^^^^^^^^^^^^^^ expected `r_efi::base::Guid`, found a different `r_efi::base::Guid`
| |
| arguments to this method are incorrect
|
note: there are multiple different versions of crate `r_efi` in the dependency graph
--> C:\Users\runneradmin\.cargo\registry\src\index.crates.io-1949cf8c6b5b557f\r-efi-6.0.0\src\base.rs:392:1
|
392 | pub struct Guid {
| ^^^^^^^^^^^^^^^ this is the expected type
|
::: C:\Users\runneradmin\.cargo\registry\src\index.crates.io-1949cf8c6b5b557f\r-efi-5.3.0\src\base.rs:392:1
|
392 | pub struct Guid {
| --------------- this is the found type
= help: you can use `cargo tree` to explore your dependency tree
note: method defined here
--> D:\a\patina\patina\sdk\patina\src\boot_services.rs:826:15
|
826 | unsafe fn locate_protocol_unchecked(
| ^^^^^^^^^^^^^^^^^^^^^^^^^
For more information about this error, try `rustc --explain E0308`.
error: could not compile `qemu_dxe_core` (lib) due to 12 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 (15 error/warning sections)
… (truncated)
error[E0308]: mismatched types
--> src/q35/component/service/smbios_platform.rs:125:30
|
125 | security_status: 0x02,
| ^^^^ expected `SecurityStatus`, found integer
error[E0308]: mismatched types
--> src/q35/component/service/smbios_platform.rs:155:28
|
155 | feature_flags: 0x01, // Board is a hosting board
| ^^^^ expected `FeatureFlags`, found integer
|
help: call `Into::into` on this expression to convert `{integer}` into `FeatureFlags`
|
155 | feature_flags: 0x01.into(), // Board is a hosting board
| +++++++
error[E0308]: mismatched types
--> src/q35/component/service/smbios_platform.rs:158:25
|
158 | board_type: 0x0A, // Motherboard
| ^^^^ expected `BoardType`, found integer
error[E0308]: mismatched types
--> src/q35/component/service/smbios_test.rs:45:49
|
45 | boot_services.locate_protocol_unchecked(&SMBIOS_PROTOCOL_GUID, core::ptr::null_mut()).map_err(|e| {
| ------------------------- ^^^^^^^^^^^^^^^^^^^^^ expected `r_efi::base::Guid`, found a different `r_efi::base::Guid`
| |
| arguments to this method are incorrect
|
note: there are multiple different versions of crate `r_efi` in the dependency graph
--> /.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/r-efi-6.0.0/src/base.rs:392:1
|
392 | pub struct Guid {
| ^^^^^^^^^^^^^^^ this is the expected type
|
::: /.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/r-efi-5.3.0/src/base.rs:392:1
|
392 | pub struct Guid {
| --------------- this is the found type
= help: you can use `cargo tree` to explore your dependency tree
note: method defined here
--> /__w/patina/patina/sdk/patina/src/boot_services.rs:826:15
|
826 | unsafe fn locate_protocol_unchecked(
| ^^^^^^^^^^^^^^^^^^^^^^^^^
For more information about this error, try `rustc --explain E0308`.
error: could not compile `qemu_dxe_core` (lib) due to 12 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-SBSA/sbsa-linux.log (15 error/warning sections)
… (truncated)
error[E0308]: mismatched types
--> src/q35/component/service/smbios_platform.rs:125:30
|
125 | security_status: 0x02,
| ^^^^ expected `SecurityStatus`, found integer
error[E0308]: mismatched types
--> src/q35/component/service/smbios_platform.rs:155:28
|
155 | feature_flags: 0x01, // Board is a hosting board
| ^^^^ expected `FeatureFlags`, found integer
|
help: call `Into::into` on this expression to convert `{integer}` into `FeatureFlags`
|
155 | feature_flags: 0x01.into(), // Board is a hosting board
| +++++++
error[E0308]: mismatched types
--> src/q35/component/service/smbios_platform.rs:158:25
|
158 | board_type: 0x0A, // Motherboard
| ^^^^ expected `BoardType`, found integer
error[E0308]: mismatched types
--> src/q35/component/service/smbios_test.rs:45:49
|
45 | boot_services.locate_protocol_unchecked(&SMBIOS_PROTOCOL_GUID, core::ptr::null_mut()).map_err(|e| {
| ------------------------- ^^^^^^^^^^^^^^^^^^^^^ expected `r_efi::base::Guid`, found a different `r_efi::base::Guid`
| |
| arguments to this method are incorrect
|
note: there are multiple different versions of crate `r_efi` in the dependency graph
--> /.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/r-efi-6.0.0/src/base.rs:392:1
|
392 | pub struct Guid {
| ^^^^^^^^^^^^^^^ this is the expected type
|
::: /.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/r-efi-5.3.0/src/base.rs:392:1
|
392 | pub struct Guid {
| --------------- this is the found type
= help: you can use `cargo tree` to explore your dependency tree
note: method defined here
--> /__w/patina/patina/sdk/patina/src/boot_services.rs:826:15
|
826 | unsafe fn locate_protocol_unchecked(
| ^^^^^^^^^^^^^^^^^^^^^^^^^
For more information about this error, try `rustc --explain E0308`.
error: could not compile `qemu_dxe_core` (lib) due to 12 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 | 4f3f455 |
| patina-dxe-core-qemu | 2a7a2bb |
| patina-fw-patcher | 1958fd4 |
| patina-qemu firmware | v3.0.0 |
| patina-qemu build script | b9cb783 |
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! |
os-d
left a comment
There was a problem hiding this comment.
I don't mind too much since there is no upstream we are tracking this against, but looks like the commits for r-efi 6.0 got a couple extra in there from review feedback and whatnot.
d501a41 to
c753e7e
Compare
952f652 to
42d0f54
Compare
|
"Patina QEMU PR Validation" failures are expected. Once this PR is merged manually (Rebase & FF). A new version is released and consumed in |
a36f54b to
43616b8
Compare
|
Note: I will override this PR and merge it Friday morning since it will require immediate integration changes in patina-dxe-core qemu after being merged to not block other changes. |
320ae28 to
1d3ccc2
Compare
## Description
patina_performance has a confusing configuration story. Prior to this
change, configuration was done via patina `Config`, A platform could
optionally register a secondary component that would read a HOB (if
provided) and overwrite any `Config` with the HOB configuration.
This commit works to simplify the configuration story by making a couple
of changes:
1. Removal of the `Config` object. As stated in documentation throughout
patina, `Config` is meant for configuration that is expected to be
shared between multiple components. This particular configuration does
not need to be shared, so it is moved to be private configuration of the
component.
2. Removal of the optional HOB reader driver. Now, a private
configuration of the component will allow you to specify if the HOB is
able to override local configuration or not.
This new story reduces patina_performance setup from three different
definitions (Config, performance component, and hob component) to only
one, the performance component.
- [ ] Impacts functionality?
- [ ] Impacts security?
- [x] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?
## How This Was Tested
Reading final configuration of the performance component on Q35 with
various configuration settings both in the component and via the HOB
## Integration Instructions
### Previous
```rust
use patina_dxe_core::*;
struct Q35;
impl ComponentInfo for Q35 {
fn configs(mut add: Add<Config>) {
add.config(patina_performance::config::PerfConfig {
enable_component: true,
enabled_measurements: patina::performance::Measurement::LoadImage | patina::performance::Measurement::StartImage
}
}
fn components(mut add: Add<Component>) {
add.component(patina_performance::component::performance_config_provider::PerformanceConfigurationProvider);
add.component(patina_performance::component::performance::Performance);
}
}
```
### After
```rust
use patina_dxe_core::*;
use patina_performance::component::*;
struct Q35;
impl ComponentInfo for Q35 {
fn components(mut add: Add<Component>) {
// Records LoadImage and StartImage measurements unless configuration is overwritten by a HOB
add.component(Performance::new().with_measurements(Measurement::LoadImage | Measurement::StartImage));
// Disabled. Enablement fully controlled by a HOB
add.component(Performance::new());
}
```
## Description Define types for SMBIOS records according the SMBIOS specs 3.0+ and update records to use types. These structs/enums/bitfields force typing when creating SMBIOS records. - [ ] Impacts functionality? - [ ] Impacts security? - [x] Breaking change? - [x] Includes tests? - [ ] Includes documentation? ## How This Was Tested `cargo make all`, plus patina-dxe-core-qemu integration: ported the Q35 and SBSA SMBIOS platform components onto the new typed records, booted Q35 in QEMU, and confirmed `q35_smbios_ffi_test` passes against the published SMBIOS table. ## Integration Instructions Downstream consumers of `patina_smbios` Type 0 / 1 / 2 / 3 / 4 / 7 / 16 / 17 / 19 records must construct fields with the new types instead of raw integers (e.g. `BiosCharacteristics::from_bits(0x08)`, `WakeUpType::PowerSwitch`, `BootUpState::Safe`). For Type 4, set `processor_family: u8 = 0xFE` and put the typed value in `processor_family2`. --------- Co-authored-by: ansley thompson <ansley.thompson@dell.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
1d3ccc2 to
4f3f455
Compare
## Description This PR consumed the major branch changes done in Patina as part of R-EFI 6.0 migration(release as version 22). - [ ] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [ ] Includes tests? - [ ] Includes documentation? ## How This Was Tested `cargo make all` ## Integration Instructions Will merge this once OpenDevicePartnership/patina#1548 is merged and patina release 22 is made. Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
…typed records (#189) ## Description Consume below major changes from Patina - patina_performance: Simplify configuration - patina_smbios: Add SMBIOS types - R-EFI 6.0 Migration --- - [ ] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [ ] Includes tests? - [ ] Includes documentation? ## How This Was Tested Booted to UEFI SHELL on q35/SBSA ## Integration Instructions Will merge this once OpenDevicePartnership/patina#1548 is merged and patina release 22 is made. Signed-off-by: Vineel Kovvuri[MSFT] <vineelko@microsoft.com>
## Description This PR consumed the major branch changes done in Patina as part of R-EFI 6.0 migration(release as version 22). - [ ] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [ ] Includes tests? - [ ] Includes documentation? ## How This Was Tested `cargo make all` ## Integration Instructions Will merge this once OpenDevicePartnership/patina#1548 is merged and patina release 22 is made.
Description
1465: patina_performance: Simplify configuration
patina_performance has a confusing configuration story. Prior to this change, configuration was done via patina
Config, A platform could optionally register a secondary component that would read a HOB (if provided) and overwrite anyConfigwith the HOB configuration.This commit works to simplify the configuration story by making a couple of changes:
Configobject. As stated in documentation throughout patina,Configis meant for configuration that is expected to be shared between multiple components. This particular configuration does not need to be shared, so it is moved to be private configuration of the component.This new story reduces patina_performance setup from three different definitions (Config, performance component, and hob component) to only one, the performance component.
How This Was Tested
Reading final configuration of the performance component on Q35 with various configuration settings both in the component and via the HOB
Previous
After
1501: patina_smbios: Add SMBIOS types
Define types for SMBIOS records according the SMBIOS specs 3.0+ and update records to use types. These structs/enums/bitfields force typing when creating SMBIOS records.
How This Was Tested
cargo make all, plus patina-dxe-core-qemu integration: ported the Q35 andSBSA SMBIOS platform components onto the new typed records, booted Q35 in
QEMU, and confirmed
q35_smbios_ffi_testpasses against the published SMBIOStable.
Integration Instructions
Downstream consumers of
patina_smbiosType 0 / 1 / 2 / 3 / 4 / 7 / 16 / 17 /19 records must construct fields with the new types instead of raw integers
(e.g.
BiosCharacteristics::from_bits(0x08),WakeUpType::PowerSwitch,BootUpState::Safe). For Type 4, setprocessor_family: u8 = 0xFEand putthe typed value in
processor_family2.Co-Authored-By: Ansley Thompson ansley.thompson@dell.com
1480: Patina: R-EFI 6.0 migration
R-EFI 6.0 Migration
Upstream r-efi 6.0 marks all
extern "efiapi"function pointers in EFI BootServices, Runtime Services, and other protocols as
unsafe, since their safetycannot be enforced by the Rust type system at compile time.
This PR audits almost all usage of UEFI service function pointers across the Patina
codebase for correctness and safety.
There are three primary areas where
unsafeusage has been audited:extern "efiapi"functionsBootServicestrait thateventually call into the FFI
Not all functions in the above categories need to be marked
unsafe. Forexample,
extern "efiapi" close_eventis not markedunsafebecause it cansafely be called with an arbitrary event parameter without causing undefined
behavior in the Patina implementation. The
eventparameter is treated as anopaque pointer and is never dereferenced.
That said, any indirect caller that dereferences this Boot Services function
pointer must still use an
unsafeblock, since the function pointer itself isdefined as
unsafein r-efi 6.0. In addition, inherently unsafe Rustfunctions (such as
core_free_pool()) are now explicitly markedunsafe.Each inspected function or call site is documented with appropriate safety
comments where necessary, and with explanations where
unsafeis notrequired.
There are no functional changes.
Clippy flags public functions that accept raw pointer parameters and pass them
across an FFI boundary without being marked
unsafe, with the followingwarning:
"this public function might dereference a raw pointer but is not marked
unsafe"In Patina, this pattern is common for functions that take
efi::Eventorefi::Handleparameters and call Boot Services function pointers. Weexplicitly suppress this lint for such functions because these types are
treated as opaque pointers and are never dereferenced:
#[allow(clippy::not_unsafe_ptr_arg_deref)]Geiger Unsafe Stats
How this was tested
Verified booting to UEFI shell from Q35, SBSA
Integration Instructions
All consumers of Patina will need to pick the newer Patina version when published and also should update their R-EFI version to 6.0.0.