Skip to content

perf: Simplify generic type parameters in performance entry points #1426

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
makubacki merged 1 commit into from
Mar 24, 2026
Merged

perf: Simplify generic type parameters in performance entry points #1426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 36 additions & 49 deletions components/patina_performance/src/component/performance.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

use crate::{component::protocol::create_performance_measurement_efiapi, config, mm};
use alloc::{boxed::Box, string::String, vec::Vec};
use core::{clone::Clone, convert::AsRef};
use patina::{
boot_services::{BootServices, StandardBootServices, event::EventType, tpl::Tpl},
component::{
Expand Down Expand Up @@ -44,7 +43,7 @@ pub use mu_rust_helpers::function;
use patina::guids::EVENT_GROUP_END_OF_DXE;

/// Context parameter for the Ready-to-Boot event callback that fetches MM performance records.
type MmPerformanceEventContext<BB, B, F> = Box<(BB, &'static TplMutex<F, B>, Service<dyn MmCommunication>)>;
type MmPerformanceEventContext<B, F> = Box<(B, &'static TplMutex<F, B>, Service<dyn MmCommunication>)>;

/// Performance Component.
pub struct Performance;
Expand Down Expand Up @@ -85,29 +84,27 @@ impl Performance {
}

/// Entry point that have generic parameter.
fn _entry_point<BB, B, RR, R, P, F>(
fn _entry_point<B, R, P, F>(
self,
boot_services: BB,
runtime_services: RR,
boot_services: B,
runtime_services: R,
records_buffers_hobs: Option<P>,
mm_comm_service: Option<Service<dyn MmCommunication>>,
fbpt: &'static TplMutex<F, B>,
timer: Service<dyn ArchTimerFunctionality>,
) -> Result<(), EfiError>
where
BB: AsRef<B> + Clone + 'static,
B: BootServices + 'static,
RR: AsRef<R> + Clone + 'static,
R: RuntimeServices + 'static,
B: BootServices + Clone + 'static,
R: RuntimeServices + Clone + 'static,
P: HobPerformanceDataExtractor,
F: FirmwareBasicBootPerfTable,
{
// Register EndOfDxe event to allocate the boot performance table and report the table address through status code.
boot_services.as_ref().create_event_ex(
boot_services.create_event_ex(
EventType::NOTIFY_SIGNAL,
Tpl::CALLBACK,
Some(event_callback::report_fbpt_record_buffer),
Box::new((BB::clone(&boot_services), RR::clone(&runtime_services), fbpt)),
Box::new((boot_services.clone(), runtime_services.clone(), fbpt)),
&EVENT_GROUP_END_OF_DXE,
)?;

Expand All @@ -134,7 +131,7 @@ impl Performance {
}

// Install the protocol interfaces for DXE performance.
boot_services.as_ref().install_protocol_interface(
boot_services.install_protocol_interface(
None,
Box::new(EdkiiPerformanceMeasurement {
create_performance_measurement: create_performance_measurement_efiapi,
Expand All @@ -146,11 +143,11 @@ impl Performance {
if let Some(mm_comm_service) = mm_comm_service {
// TODO: Replace direct usage of the boot services event services with a Patina service
// when available.
boot_services.as_ref().create_event_ex(
boot_services.create_event_ex(
EventType::NOTIFY_SIGNAL,
Tpl::CALLBACK,
Some(fetch_and_add_mm_performance_records::<BB, B, F>),
Box::new((BB::clone(&boot_services), fbpt, mm_comm_service)),
Some(fetch_and_add_mm_performance_records::<B, F>),
Box::new((boot_services.clone(), fbpt, mm_comm_service)),
&EVENT_GROUP_READY_TO_BOOT,
)?;
} else {
Expand All @@ -164,7 +161,7 @@ impl Performance {
// Install configuration table for performance property.
// SAFETY: `install_configuration_table` requires that the data match the GUID; PERFORMANCE_PROTOCOL matches `PerformanceProperty`.
unsafe {
boot_services.as_ref().install_configuration_table(
boot_services.install_configuration_table(
&PERFORMANCE_PROTOCOL,
Box::new(PerformanceProperty::new(
timer.perf_frequency(),
Expand Down Expand Up @@ -410,16 +407,15 @@ where
}

/// Adds MM performance records to the FBPT.
pub extern "efiapi" fn fetch_and_add_mm_performance_records<BB, B, F>(
pub extern "efiapi" fn fetch_and_add_mm_performance_records<B, F>(
event: r_efi::efi::Event,
ctx: MmPerformanceEventContext<BB, B, F>,
ctx: MmPerformanceEventContext<B, F>,
) where
BB: AsRef<B> + Clone,
B: BootServices + 'static,
B: BootServices + Clone + 'static,
F: FirmwareBasicBootPerfTable,
{
let (boot_services, fbpt, comm_service) = *ctx;
let _ = boot_services.as_ref().close_event(event);
let _ = boot_services.close_event(event);

if let Err(e) = process_mm_performance_records(&comm_service, fbpt) {
log::error!("Performance: {}", e);
Expand All @@ -429,7 +425,6 @@ pub extern "efiapi" fn fetch_and_add_mm_performance_records<BB, B, F>(
#[cfg(test)]
mod tests {
use super::*;
use alloc::rc::Rc;
use core::assert_eq;
use r_efi::efi;

Expand Down Expand Up @@ -537,8 +532,8 @@ mod tests {
// Test that an event to report the fbpt at the end of dxe is created.
boot_services
.expect_create_event_ex::<Box<(
Rc<MockBootServices>,
Rc<MockRuntimeServices>,
MockBootServices,
MockRuntimeServices,
&TplMutex<MockFirmwareBasicBootPerfTable, MockBootServices>,
)>>()
.once()
Expand All @@ -547,9 +542,7 @@ mod tests {
assert_eq!(&Tpl::CALLBACK, notify_tpl);
assert_eq!(
event_callback::report_fbpt_record_buffer::<
Rc<_>,
MockBootServices,
Rc<_>,
MockRuntimeServices,
MockFirmwareBasicBootPerfTable,
> as *const () as usize,
Expand All @@ -571,17 +564,15 @@ mod tests {
let mut fbpt = MockFirmwareBasicBootPerfTable::new();
fbpt.expect_set_perf_records().once().return_const(());

let boot_services_rc = Rc::new(boot_services);

// TplMutex owns its BootServices instance
let fbpt = TplMutex::new((*boot_services_rc).clone(), Tpl::NOTIFY, fbpt);
// TplMutex owns its own BootServices instance (clone creates a new mock with default TPL expectations)
let fbpt = TplMutex::new(boot_services.clone(), Tpl::NOTIFY, fbpt);

// Leak the fbpt to create a 'static reference for testing.
let fbpt = Box::leak(Box::new(fbpt));

let _ = Performance._entry_point(
boot_services_rc,
Rc::new(runtime_services),
boot_services,
runtime_services,
Some(hob_perf_data_extractor),
None,
fbpt,
Expand Down Expand Up @@ -610,23 +601,19 @@ mod tests {
let mut entry_point_mock = MockBootServices::new();
entry_point_mock
.expect_create_event_ex::<Box<(
Rc<MockBootServices>,
Rc<MockRuntimeServices>,
MockBootServices,
MockRuntimeServices,
&TplMutex<MockFirmwareBasicBootPerfTable, MockBootServices>,
)>>()
.once()
.return_const_st(Ok(TEST_EVENT_HANDLE));
entry_point_mock
.expect_create_event_ex::<MmPerformanceEventContext<
Rc<MockBootServices>,
MockBootServices,
MockFirmwareBasicBootPerfTable,
>>()
.expect_create_event_ex::<MmPerformanceEventContext<MockBootServices, MockFirmwareBasicBootPerfTable>>()
.once()
.withf_st(|_, _, f, _, group| {
(f.unwrap() as usize)
== fetch_and_add_mm_performance_records::<Rc<_>, MockBootServices, MockFirmwareBasicBootPerfTable>
as * const () as usize
== fetch_and_add_mm_performance_records::<MockBootServices, MockFirmwareBasicBootPerfTable>
as *const () as usize
&& group == &EVENT_GROUP_READY_TO_BOOT
})
.return_const_st(Ok(TEST_EVENT_HANDLE_2));
Expand All @@ -648,8 +635,8 @@ mod tests {
let mm_service: Service<dyn MmCommunication> = Service::mock(Box::new(FakeComm));
let timer: Service<dyn ArchTimerFunctionality> = Service::mock(Box::new(MockTimer {}));
let _ = Performance._entry_point(
Rc::new(entry_point_mock),
Rc::new(runtime_services),
entry_point_mock,
runtime_services,
Option::<MockHobPerformanceDataExtractor>::None,
Some(mm_service),
fbpt_ref,
Expand Down Expand Up @@ -695,9 +682,9 @@ mod tests {
let fbpt_ref: &'static TplMutex<_, _> = Box::leak(Box::new(fbpt_mutex));

let mm_service: Service<dyn MmCommunication> = Service::mock(Box::new(ZeroSizeComm));
fetch_and_add_mm_performance_records::<Rc<MockBootServices>, MockBootServices, MockFirmwareBasicBootPerfTable>(
fetch_and_add_mm_performance_records::<MockBootServices, MockFirmwareBasicBootPerfTable>(
TEST_EVENT_HANDLE,
Box::new((Rc::new(callback_mock), fbpt_ref, mm_service)),
Box::new((callback_mock, fbpt_ref, mm_service)),
);
}

Expand Down Expand Up @@ -759,9 +746,9 @@ mod tests {
let fbpt_ref: &'static TplMutex<_, _> = Box::leak(Box::new(fbpt_mutex));

let mm_service: Service<dyn MmCommunication> = Service::mock(Box::new(OneRecordComm::new()));
fetch_and_add_mm_performance_records::<Rc<MockBootServices>, MockBootServices, MockFirmwareBasicBootPerfTable>(
fetch_and_add_mm_performance_records::<MockBootServices, MockFirmwareBasicBootPerfTable>(
TEST_EVENT_HANDLE,
Box::new((Rc::new(callback_mock), fbpt_ref, mm_service)),
Box::new((callback_mock, fbpt_ref, mm_service)),
);
}

Expand Down Expand Up @@ -842,9 +829,9 @@ mod tests {

let mm_service: Service<dyn MmCommunication> =
Service::mock(Box::new(MultiChunks { buf: all_records, fetches: Cell::new(0) }));
fetch_and_add_mm_performance_records::<Rc<MockBootServices>, MockBootServices, MockFirmwareBasicBootPerfTable>(
fetch_and_add_mm_performance_records::<MockBootServices, MockFirmwareBasicBootPerfTable>(
TEST_EVENT_HANDLE,
Box::new((Rc::new(callback_mock), fbpt_ref, mm_service)),
Box::new((callback_mock, fbpt_ref, mm_service)),
);
}

Expand Down
30 changes: 12 additions & 18 deletions sdk/patina/src/performance/measurement.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
use alloc::boxed::Box;
use core::{
clone::Clone,
convert::AsRef,
ffi::c_void,
mem,
ops::BitOr,
Expand Down Expand Up @@ -53,29 +52,25 @@ pub mod event_callback {
use super::*;

/// Reports the Firmware Basic Boot Performance Table (FBPT) record buffer.
pub extern "efiapi" fn report_fbpt_record_buffer<BB, B, RR, R, F>(
event: efi::Event,
ctx: Box<(BB, RR, &TplMutex<F, B>)>,
) where
BB: AsRef<B> + Clone,
B: BootServices + 'static,
RR: AsRef<R> + Clone + 'static,
R: RuntimeServices + 'static,
pub extern "efiapi" fn report_fbpt_record_buffer<B, R, F>(event: efi::Event, ctx: Box<(B, R, &TplMutex<F, B>)>)
where
B: BootServices + Clone + 'static,
R: RuntimeServices + Clone + 'static,
F: FirmwareBasicBootPerfTable,
{
let (boot_services, runtime_services, fbpt) = *ctx;
let _ = boot_services.as_ref().close_event(event);
let _ = boot_services.close_event(event);

let Ok(fbpt_address) = fbpt.lock().report_table(
performance::table::find_previous_table_address(runtime_services.as_ref()),
boot_services.as_ref(),
) else {
let Ok(fbpt_address) = fbpt
.lock()
.report_table(performance::table::find_previous_table_address(&runtime_services), &boot_services)
else {
log::error!("Performance: Fail to report FBPT.");
return;
};

// SAFETY: `p` is the only mutable reference to the `StatusCodeRuntimeProtocol` in this scope.
let Ok(p) = (unsafe { boot_services.as_ref().locate_protocol::<StatusCodeRuntimeProtocol>(None) }) else {
let Ok(p) = (unsafe { boot_services.locate_protocol::<StatusCodeRuntimeProtocol>(None) }) else {
log::error!("Performance: Fail to find status code protocol.");
return;
};
Expand All @@ -95,7 +90,7 @@ pub mod event_callback {
// SAFETY: This operation is valid because the expected configuration type of a entry with guid `EDKII_FPDT_EXTENDED_FIRMWARE_PERFORMANCE`
// is a usize and the memory address is a valid and point to an FBPT.
let status = unsafe {
boot_services.as_ref().install_configuration_table_unchecked(
boot_services.install_configuration_table_unchecked(
&EDKII_FPDT_EXTENDED_FIRMWARE_PERFORMANCE,
fbpt_address as *mut c_void,
)
Expand Down Expand Up @@ -499,7 +494,6 @@ mod tests {

use crate::{self as patina, device_path::fv_types::MediaFwVolDevicePath};

use alloc::rc::Rc;
use core::{
mem::MaybeUninit,
ptr,
Expand Down Expand Up @@ -585,7 +579,7 @@ mod tests {

event_callback::report_fbpt_record_buffer(
1_usize as efi::Event,
Box::new((Rc::new(boot_services), Rc::new(runtime_services), fbpt)),
Box::new((boot_services, runtime_services, fbpt)),
);

assert!(REPORT_STATUS_CODE_CALLED.load(Ordering::Relaxed));
Expand Down
8 changes: 8 additions & 0 deletions sdk/patina/src/runtime_services.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,14 @@ impl RuntimeServices for StandardRuntimeServices {
}
}

/// Clone implementation for MockRuntimeServices that creates a new mock with default expectations.
#[cfg(any(test, feature = "mockall"))]
impl Clone for MockRuntimeServices {
fn clone(&self) -> Self {
Self::new()
}
}

#[cfg(test)]
#[coverage(off)]
pub(crate) mod test {
Expand Down
Loading