Skip to content

Commit f318bfb

Browse files
author
Sherry Fan
committed
minor improvements
1 parent 24d701b commit f318bfb

File tree

15 files changed

+161
-28
lines changed

15 files changed

+161
-28
lines changed

‎Cargo.toml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,6 @@ opt-level = "s"
3535
codegen-units = 1
3636
lto = true
3737
incremental = true
38+
39+
[workspace.lints.clippy]
40+
undocumented_unsafe_blocks = "warn"

‎Makefile.toml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,10 +147,10 @@ args = ["llvm-cov", "--no-report"]
147147

148148
# Clippy linting
149149
[tasks.clippy]
150-
description = "Run clippy for the package"
150+
description = "Run cargo clippy."
151+
clear = true
151152
command = "cargo"
152-
args = ["clippy", "-p", "${PACKAGE_NAME}", "--target", "${UEFI_TARGET}", "--", "-D", "warnings"]
153-
dependencies = ["setup"]
153+
args = ["clippy", "--all-targets", "--all-features", "--", "-D", "warnings"]
154154

155155
# Format code
156156
[tasks.fmt]
@@ -197,4 +197,4 @@ script = "cspell --quiet --no-progress --no-summary --dot --gitignore -e \"{.g
197197

198198
[tasks.all]
199199
description = "Run all tasks for PR readiness."
200-
dependencies = ["deny", "clippy", "cspell", "build", "test", "coverage", "fmt", "doc"]
200+
dependencies = ["deny", "clippy", "cspell", "build", "test", "coverage", "fmt", "doc"]

‎services_benchmark_test/Cargo.toml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ repository.workspace = true
1010
license.workspace = true
1111
description = "Core service benchmarks"
1212

13+
[lints]
14+
workspace = true
15+
1316
[lib]
1417
name = "services_benchmark_test"
1518
path = "src/lib.rs"

‎services_benchmark_test/src/bench.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
//! Defines constants and benchmarks used to evaluate core performance.
2+
//!
3+
//! Copyright (c) Microsoft Corporation.
4+
//!
5+
//! SPDX-License-Identifier: Apache-2.0
6+
//!
7+
18
use patina::uefi_protocol::ProtocolInterface;
29
use r_efi::efi;
310

@@ -17,12 +24,14 @@ const TEST_GUID2: efi::Guid =
1724

1825
pub struct TestProtocol1 {}
1926

27+
// SAFETY: This is a test protocol with no layout requirements.
2028
unsafe impl ProtocolInterface for TestProtocol1 {
2129
const PROTOCOL_GUID: efi::Guid = TEST_GUID1;
2230
}
2331

2432
pub struct TestProtocol2 {}
2533

34+
// SAFETY: This is a test protocol with no layout requirements.
2635
unsafe impl ProtocolInterface for TestProtocol2 {
2736
const PROTOCOL_GUID: efi::Guid = TEST_GUID2;
2837
}

‎services_benchmark_test/src/bench/controller.rs‎

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
1+
//! Benchmarks for UEFI controller connection mechanism.
2+
//!
3+
//! Copyright (c) Microsoft Corporation.
4+
//!
5+
//! SPDX-License-Identifier: Apache-2.0
6+
//!
7+
18
#[cfg(target_os = "uefi")]
29
use alloc::{boxed::Box, vec};
10+
use uefi::proto::driver;
311

412
#[cfg(not(target_os = "uefi"))]
513
use std::{boxed::Box, vec};
@@ -44,57 +52,62 @@ pub(crate) fn bench_connect_controller(_handle: efi::Handle, num_calls: usize) -
4452
}
4553

4654
// Setup controller, driver, and image handles with test protocols.
47-
let controller_handle = BOOT_SERVICES
55+
let controller_install = BOOT_SERVICES
4856
.install_protocol_interface(None, Box::new(TestProtocol1 {}))
49-
.map_err(|e| BenchError::BenchSetup("Failed to install protocol interface for controller", e))?
50-
.0;
57+
.map_err(|e| BenchError::BenchSetup("Failed to install protocol interface for controller", e))?;
5158

52-
let driver_handle = BOOT_SERVICES
59+
let driver_install = BOOT_SERVICES
5360
.install_protocol_interface(
5461
None,
5562
Box::new(efi::protocols::device_path::Protocol { r#type: 4, sub_type: 5, length: [0, 0] }),
5663
)
57-
.map_err(|e| BenchError::BenchSetup("Failed to install protocol interface for driver", e))?
58-
.0;
64+
.map_err(|e| BenchError::BenchSetup("Failed to install protocol interface for driver", e))?;
5965

60-
let image_handle = BOOT_SERVICES
66+
let image_install = BOOT_SERVICES
6167
.install_protocol_interface(None, Box::new(TestProtocol2 {}))
62-
.map_err(|e| BenchError::BenchSetup("Failed to install protocol interface for image", e))?
63-
.0;
68+
.map_err(|e| BenchError::BenchSetup("Failed to install protocol interface for image", e))?;
6469

6570
let binding = Box::new(efi::protocols::driver_binding::Protocol {
6671
version: 10,
6772
supported: mock_supported,
6873
start: mock_start,
6974
stop: mock_stop,
70-
driver_binding_handle: driver_handle,
71-
image_handle,
75+
driver_binding_handle: driver_install.0,
76+
image_handle: image_install.0,
7277
});
7378

74-
let driver_binding_key = BOOT_SERVICES
75-
.install_protocol_interface(Some(driver_handle), binding)
76-
.map_err(|e| BenchError::BenchSetup("Failed to install protocol interface for driver binding", e))?
77-
.1;
79+
let driver_binding = BOOT_SERVICES
80+
.install_protocol_interface(Some(driver_install.0), binding)
81+
.map_err(|e| BenchError::BenchSetup("Failed to install protocol interface for driver binding", e))?;
7882

7983
let mut stats: Stats<f64> = Stats::new();
8084
for _ in 0..num_calls {
8185
let start = Arch::cpu_count();
8286
// SAFETY: All handles and pointers are valid (constructed by benchmark).
8387
unsafe {
8488
BOOT_SERVICES
85-
.connect_controller(controller_handle, vec![driver_handle], core::ptr::null_mut(), false)
89+
.connect_controller(controller_install.0, vec![driver_install.0], core::ptr::null_mut(), false)
8690
.map_err(|e| BenchError::BenchTest("Failed to connect controller", e))?;
8791
}
8892
let end = Arch::cpu_count();
8993
stats.update((end - start) as f64);
9094
BOOT_SERVICES
91-
.disconnect_controller(controller_handle, None, None)
95+
.disconnect_controller(controller_install.0, None, None)
9296
.map_err(|e| BenchError::BenchCleanup("Failed to disconnect controller", e))?;
9397
}
9498

9599
// Uninstall protocols to prevent side effects.
96100
BOOT_SERVICES
97-
.uninstall_protocol_interface(driver_handle, driver_binding_key)
101+
.uninstall_protocol_interface(driver_binding.0, driver_binding.1)
102+
.map_err(|e| BenchError::BenchCleanup("Failed to uninstall protocol interface", e))?;
103+
BOOT_SERVICES
104+
.uninstall_protocol_interface(driver_install.0, driver_install.1)
105+
.map_err(|e| BenchError::BenchCleanup("Failed to uninstall protocol interface", e))?;
106+
BOOT_SERVICES
107+
.uninstall_protocol_interface(image_install.0, image_install.1)
108+
.map_err(|e| BenchError::BenchCleanup("Failed to uninstall protocol interface", e))?;
109+
BOOT_SERVICES
110+
.uninstall_protocol_interface(controller_install.0, controller_install.1)
98111
.map_err(|e| BenchError::BenchCleanup("Failed to uninstall protocol interface", e))?;
99112

100113
Ok(stats)

‎services_benchmark_test/src/bench/event.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
//! Benchmarks for event handling.
2+
//!
3+
//! Copyright (c) Microsoft Corporation.
4+
//!
5+
//! SPDX-License-Identifier: Apache-2.0
6+
//!
7+
18
#[cfg(target_os = "uefi")]
29
use alloc::vec::Vec;
310

‎services_benchmark_test/src/bench/image.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
//! Benchmarks for image loading and execution.
2+
//!
3+
//! Copyright (c) Microsoft Corporation.
4+
//!
5+
//! SPDX-License-Identifier: Apache-2.0
6+
//!
7+
18
use mu_rust_helpers::perf_timer::{Arch, ArchFunctionality as _};
29
use patina::boot_services::BootServices;
310
use r_efi::efi;

‎services_benchmark_test/src/bench/memory.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
//! Benchmarks for memory operations.
2+
//!
3+
//! Copyright (c) Microsoft Corporation.
4+
//!
5+
//! SPDX-License-Identifier: Apache-2.0
6+
//!
7+
18
use mu_rust_helpers::perf_timer::{Arch, ArchFunctionality as _};
29
use patina::{
310
base::UEFI_PAGE_SIZE,

‎services_benchmark_test/src/bench/misc.rs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
//! Benchmarks for general UEFI services.
2+
//!
3+
//! Copyright (c) Microsoft Corporation.
4+
//!
5+
//! SPDX-License-Identifier: Apache-2.0
6+
//!
7+
18
use core::ffi::c_void;
29

310
use mu_rust_helpers::perf_timer::{Arch, ArchFunctionality as _};
@@ -31,6 +38,7 @@ pub(crate) fn bench_install_configuration_table(
3138
let mut stats: Stats<f64> = Stats::new();
3239
for _ in 0..num_calls {
3340
let start = Arch::cpu_count();
41+
// SAFETY: The test configuration table has no specific layout requirements.
3442
unsafe {
3543
// We do not need to clean up the installed table on each iteration as
3644
// installing a table with a duplicate GUID simply overwrites the previous entry.
@@ -41,5 +49,12 @@ pub(crate) fn bench_install_configuration_table(
4149
let end = Arch::cpu_count();
4250
stats.update((end - start) as f64);
4351
}
52+
// Remove the table by passing a NULL pointer.
53+
// SAFETY: The test configuration table has no specific layout requirements.
54+
(unsafe {
55+
BOOT_SERVICES
56+
.install_configuration_table(&TEST_GUID1, core::ptr::null_mut() as *const u64 as *mut c_void)
57+
.map_err(|e| BenchError::BenchCleanup("Failed to remove configuration table", e))
58+
})?;
4459
Ok(stats)
4560
}

‎services_benchmark_test/src/bench/protocol.rs‎

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
//! Benchmarks for protocol handling.
2+
//!
3+
//! Copyright (c) Microsoft Corporation.
4+
//!
5+
//! SPDX-License-Identifier: Apache-2.0
6+
//!
7+
18
use core::ffi::c_void;
29

310
use mu_rust_helpers::perf_timer::{Arch, ArchFunctionality as _};
@@ -53,6 +60,7 @@ pub(crate) fn bench_open_protocol(_handle: efi::Handle, num_calls: usize) -> Res
5360
let mut stats: Stats<f64> = Stats::new();
5461
for _ in 0..num_calls {
5562
let start = Arch::cpu_count();
63+
// SAFETY: The resulting interface reference is not used at all during the test.
5664
(unsafe {
5765
BOOT_SERVICES
5866
.open_protocol::<TestProtocol1>(
@@ -99,6 +107,7 @@ pub(crate) fn bench_close_protocol(_handle: efi::Handle, num_calls: usize) -> Re
99107
.map_err(|e| BenchError::BenchSetup("Failed to install protocol handle", e))?;
100108
let mut stats: Stats<f64> = Stats::new();
101109
for _ in 0..num_calls {
110+
// SAFETY: The resulting interface reference is not used at all during the test.
102111
unsafe {
103112
BOOT_SERVICES
104113
.open_protocol::<TestProtocol1>(
@@ -117,40 +126,58 @@ pub(crate) fn bench_close_protocol(_handle: efi::Handle, num_calls: usize) -> Re
117126
let end = Arch::cpu_count();
118127
stats.update((end - start) as f64);
119128
}
129+
130+
// Uninstall mock protocols after benchmarking.
131+
BOOT_SERVICES
132+
.uninstall_protocol_interface(protocol_install.0, protocol_install.1)
133+
.map_err(|e| BenchError::BenchCleanup("Failed to uninstall protocol", e))?;
134+
BOOT_SERVICES
135+
.uninstall_protocol_interface(agent_install.0, agent_install.1)
136+
.map_err(|e| BenchError::BenchCleanup("Failed to uninstall agent protocol", e))?;
137+
BOOT_SERVICES
138+
.uninstall_protocol_interface(controller_install.0, controller_install.1)
139+
.map_err(|e| BenchError::BenchCleanup("Failed to uninstall controller protocol", e))?;
140+
120141
Ok(stats)
121142
}
122143

123144
/// Benchmarks protocol handling performance.
124145
/// This is a legacy method but is still included due to needing to support legacy UEFI (1.0).
125146
pub(crate) fn bench_handle_protocol(_handle: efi::Handle, num_calls: usize) -> Result<Stats<f64>, BenchError> {
126147
// Set up and install the protocol to be accessed.
127-
let protocol_handle = BOOT_SERVICES
148+
let protocol_install = BOOT_SERVICES
128149
.install_protocol_interface(None, Box::new(TestProtocol1 {}))
129-
.map_err(|e| BenchError::BenchSetup("Failed to install protocol", e))?
130-
.0;
150+
.map_err(|e| BenchError::BenchSetup("Failed to install protocol", e))?;
131151
let mut stats: Stats<f64> = Stats::new();
132152
for _ in 0..num_calls {
133153
let start = Arch::cpu_count();
154+
// SAFETY: The resulting interface reference is not used at all during the test.
134155
(unsafe {
135156
BOOT_SERVICES
136-
.handle_protocol::<TestProtocol1>(protocol_handle)
157+
.handle_protocol::<TestProtocol1>(protocol_install.0)
137158
.map_err(|e| BenchError::BenchTest("Failed to handle protocol", e))
138159
})?;
139160

140161
let end = Arch::cpu_count();
141162
stats.update((end - start) as f64);
142163
}
164+
// Uninstall mock protocol after benchmarking.
165+
BOOT_SERVICES
166+
.uninstall_protocol_interface(protocol_install.0, protocol_install.1)
167+
.map_err(|e| BenchError::BenchCleanup("Failed to uninstall protocol", e))?;
143168
Ok(stats)
144169
}
145170

146171
/// Benchmarks device path resolution.
147172
pub(crate) fn bench_locate_device_path(handle: efi::Handle, num_calls: usize) -> Result<Stats<f64>, BenchError> {
148-
// Install a protocol on the current image to get a valid device handle.
173+
// Find existing protocol handles to locate device path.
174+
// SAFETY: There is only one reference to the `loaded_image_protocol` interface.
149175
let loaded_image_protocol = unsafe {
150176
BOOT_SERVICES
151177
.handle_protocol::<efi::protocols::loaded_image::Protocol>(handle)
152178
.map_err(|e| BenchError::BenchSetup("Failed to get loaded image protocol.", e))?
153179
};
180+
// SAFETY: There is only one reference to the `device_path_protocol` interface.
154181
let device_path_protocol = unsafe {
155182
BOOT_SERVICES
156183
.handle_protocol::<efi::protocols::device_path::Protocol>(loaded_image_protocol.device_handle)
@@ -161,6 +188,7 @@ pub(crate) fn bench_locate_device_path(handle: efi::Handle, num_calls: usize) ->
161188
for _ in 0..num_calls {
162189
let mut device_path_ptr = device_path_protocol as *mut efi::protocols::device_path::Protocol;
163190
let start = Arch::cpu_count();
191+
// SAFETY: The device path has been constructed above as a valid pointer.
164192
unsafe {
165193
BOOT_SERVICES
166194
.locate_device_path(&efi::protocols::device_path::PROTOCOL_GUID, &mut device_path_ptr as *mut _)
@@ -240,13 +268,19 @@ pub(crate) fn bench_reinstall_protocol_interface(
240268
.map_err(|e| BenchError::BenchSetup("Failed to install dummy protocol", e))?;
241269

242270
let start = Arch::cpu_count();
243-
BOOT_SERVICES
271+
let reinstall = BOOT_SERVICES
244272
.reinstall_protocol_interface(protocol_install.0, protocol_install.1, new_interface)
245273
.map_err(|e| BenchError::BenchTest("Failed to reinstall protocol interface", e))?;
246274

247275
let end = Arch::cpu_count();
248276
stats.update((end - start) as f64);
277+
278+
// Cleanup: Uninstall the protocol after benchmarking. (It will be installed and reinstalled in the next iteration.)
279+
BOOT_SERVICES
280+
.uninstall_protocol_interface(protocol_install.0, reinstall.0)
281+
.map_err(|e| BenchError::BenchCleanup("Failed to uninstall protocol interface", e))?;
249282
}
283+
250284
Ok(stats)
251285
}
252286

@@ -272,5 +306,11 @@ pub(crate) fn bench_uninstall_protocol_interface(
272306
.install_protocol_interface(None, Box::new(TestProtocol1 {}))
273307
.map_err(|e| BenchError::BenchCleanup("Failed to install a new dummy protocol", e))?;
274308
}
309+
310+
// Installation from last iteration cleanup.
311+
BOOT_SERVICES
312+
.uninstall_protocol_interface(protocol_install.0, protocol_install.1)
313+
.map_err(|e| BenchError::BenchCleanup("Failed to uninstall protocol interface", e))?;
314+
275315
Ok(stats)
276316
}

0 commit comments

Comments
 (0)