[feature/patina-boot] patina_boot: Add ConnectController service trait

Add a pluggable ConnectController trait for controller connection
strategies during device enumeration, replacing the hardcoded
connect_all() call in SimpleBootManager.

- ConnectController trait with single connect() method, generic over
  B: BootServices for mockable unit testing
- ConnectAllStrategy default implementation preserves existing behavior
- SimpleBootManager::with_connect_strategy() constructor for custom
  strategies; SimpleBootManager::new() is unchanged
- interleave_connect_and_dispatch() made generic over the connection
  function

Enables platforms to implement selective device enumeration (e.g.,
PCI-only, skip USB for headless) without writing a full custom
BootOrchestrator.

Ref: OpenDevicePartnership/modern-payload#205

Author: kat-perez

components/patina_boot/src/connect_controller.rs

@@ -0,0 +1,44 @@
+//! Connect Controller service interface.
+//!
+//! Defines the [`ConnectController`] trait for pluggable controller connection
+//! strategies during device enumeration.
+//!
+//! ## License
+//!
+//! Copyright (c) Microsoft Corporation.
+//!
+//! SPDX-License-Identifier: Apache-2.0
+//!
+
+use patina::{
+    boot_services::{BootServices, StandardBootServices},
+    error::Result,
+};
+
+/// Pluggable controller connection strategy for device enumeration.
+///
+/// The default [`ConnectAllStrategy`](crate::ConnectAllStrategy) connects all
+/// controllers recursively. Platforms can implement this trait on a struct or
+/// pass a closure directly (blanket impl for `Fn(&B) -> Result<()> + Send + Sync + 'static`):
+///
+/// ```rust,ignore
+/// SimpleBootManager::with_connect_strategy(config, |bs: &StandardBootServices| {
+///     connect_pci(bs)?;
+///     connect_usb(bs)
+/// });
+/// ```
+pub trait ConnectController<B: BootServices = StandardBootServices>: Send + Sync + 'static {
+    /// Perform one connection pass. The caller handles looping and
+    /// interleaving with DXE dispatch.
+    fn connect(&self, boot_services: &B) -> Result<()>;
+}
+
+impl<B, F> ConnectController<B> for F
+where
+    B: BootServices,
+    F: Fn(&B) -> Result<()> + Send + Sync + 'static,
+{
+    fn connect(&self, boot_services: &B) -> Result<()> {
+        self(boot_services)
+    }
+}

components/patina_boot/src/lib.rs

@@ -33,9 +33,13 @@
 pub mod boot_dispatcher;
 pub mod boot_orchestrator;
 pub mod config;
+pub mod connect_controller;
 pub mod helpers;
 pub mod orchestrators;
+pub mod strategies;
 
 pub use boot_dispatcher::BootDispatcher;
 pub use boot_orchestrator::BootOrchestrator;
+pub use connect_controller::ConnectController;
 pub use orchestrators::SimpleBootManager;
+pub use strategies::{ConnectAllStrategy, ConnectByProtocolStrategy};

components/patina_boot/src/orchestrators/simple_boot_manager.rs

@@ -15,46 +15,50 @@
 //!
 extern crate alloc;
 
-use alloc::vec::Vec;
+use alloc::{boxed::Box, vec::Vec};
 
 use patina::{
-    boot_services::StandardBootServices, device_path::paths::DevicePathBuf, error::EfiError,
+    boot_services::{BootServices, StandardBootServices},
+    device_path::paths::DevicePathBuf,
+    error::EfiError,
     runtime_services::StandardRuntimeServices,
 };
 use r_efi::efi;
 
 use patina::component::service::dxe_dispatch::DxeDispatch;
 
-use patina::boot_services::BootServices;
-
-use crate::{boot_orchestrator::BootOrchestrator, config::BootConfig, helpers};
+use crate::{
+    boot_orchestrator::BootOrchestrator, config::BootConfig, connect_controller::ConnectController, helpers,
+    strategies::ConnectAllStrategy,
+};
 
 /// Interleave controller connection with DXE driver dispatch.
 ///
-/// Alternates between connecting all controllers and dispatching newly loaded
-/// drivers (e.g., PCI option ROMs) until both the device topology stabilizes
-/// and no new drivers are dispatched.
+/// Alternates between the given connect function and dispatching newly loaded
+/// drivers (e.g., PCI option ROM drivers) until both the device topology
+/// stabilizes and no new drivers are dispatched.
 ///
 /// This ensures that drivers loaded from firmware volumes during device
 /// enumeration (such as PCI option ROM drivers) are dispatched before
 /// continuing enumeration, allowing those drivers to bind to newly
 /// discovered controllers.
 fn interleave_connect_and_dispatch<B: BootServices, D: DxeDispatch + ?Sized>(
+    connect_fn: impl Fn(&B) -> patina::error::Result<()>,
     boot_services: &B,
     dxe_services: &D,
 ) -> patina::error::Result<()> {
     const MAX_ROUNDS: usize = 10;
 
     for _round in 0..MAX_ROUNDS {
-        helpers::connect_all(boot_services)?;
+        connect_fn(boot_services)?;
         if !dxe_services.dispatch()? {
             return Ok(());
         }
     }
 
     debug_assert!(false, "connect-dispatch interleaving did not converge after {MAX_ROUNDS} rounds");
 
-    Ok(())
+    Err(EfiError::Timeout)
 }
 
 /// Simple boot manager implementing [`BootOrchestrator`].
@@ -73,11 +77,16 @@ fn interleave_connect_and_dispatch<B: BootServices, D: DxeDispatch + ?Sized>(
 /// 7. Call failure handler if all options exhausted
 pub struct SimpleBootManager {
     config: BootConfig,
+    connect_strategy: Box<dyn ConnectController>,
 }
 
 impl SimpleBootManager {
     /// Create a `SimpleBootManager` from a boot configuration.
     ///
+    /// Uses [`ConnectAllStrategy`] by default. To customize which controllers
+    /// are connected during device enumeration, use
+    /// [`with_connect_strategy()`](Self::with_connect_strategy).
+    ///
     /// ## Example
     ///
     /// ```rust,ignore
@@ -92,7 +101,23 @@ impl SimpleBootManager {
     /// );
     /// ```
     pub fn new(config: BootConfig) -> Self {
-        Self { config }
+        Self { config, connect_strategy: Box::new(ConnectAllStrategy) }
+    }
+
+    /// Create a `SimpleBootManager` with a custom connection strategy.
+    ///
+    /// ## Example
+    ///
+    /// ```rust,ignore
+    /// use patina_boot::{SimpleBootManager, ConnectByProtocolStrategy, config::BootConfig};
+    ///
+    /// let manager = SimpleBootManager::with_connect_strategy(
+    ///     BootConfig::new(nvme_esp_path()),
+    ///     ConnectByProtocolStrategy::new(&[&PCI_ROOT_BRIDGE_IO_PROTOCOL_GUID]),
+    /// );
+    /// ```
+    pub fn with_connect_strategy(config: BootConfig, strategy: impl ConnectController) -> Self {
+        Self { config, connect_strategy: Box::new(strategy) }
     }
 }
 
@@ -113,7 +138,9 @@ impl BootOrchestrator for SimpleBootManager {
         dxe_dispatch: &dyn DxeDispatch,
         image_handle: efi::Handle,
     ) -> Result<!, EfiError> {
-        if let Err(e) = interleave_connect_and_dispatch(boot_services, dxe_dispatch) {
+        if let Err(e) =
+            interleave_connect_and_dispatch(|bs| self.connect_strategy.connect(bs), boot_services, dxe_dispatch)
+        {
             log::error!("interleave_connect_and_dispatch failed: {:?}", e);
         }
 
@@ -213,7 +240,7 @@ mod tests {
     }
 
     #[test]
-    fn test_interleave_single_round_no_drivers_dispatched() {
+    fn test_simple_boot_manager_interleave_single_round_no_drivers_dispatched() {
         let box_mock = leaked_boot_services_for_box();
         let mut boot_mock = MockBootServices::new();
 
@@ -222,12 +249,12 @@ mod tests {
 
         let dxe_mock = MockDxeDispatcher::new(&[Ok(false)]);
 
-        let result = interleave_connect_and_dispatch(&boot_mock, &dxe_mock);
+        let result = interleave_connect_and_dispatch(helpers::connect_all, &boot_mock, &dxe_mock);
         assert!(result.is_ok());
     }
 
     #[test]
-    fn test_interleave_multiple_rounds() {
+    fn test_simple_boot_manager_interleave_multiple_rounds() {
         let box_mock = leaked_boot_services_for_box();
         let mut boot_mock = MockBootServices::new();
 
@@ -236,24 +263,24 @@ mod tests {
 
         let dxe_mock = MockDxeDispatcher::new(&[Ok(true), Ok(false)]);
 
-        let result = interleave_connect_and_dispatch(&boot_mock, &dxe_mock);
+        let result = interleave_connect_and_dispatch(helpers::connect_all, &boot_mock, &dxe_mock);
         assert!(result.is_ok());
     }
 
     #[test]
-    fn test_interleave_connect_failure_propagates() {
+    fn test_simple_boot_manager_interleave_connect_failure_propagates() {
         let mut boot_mock = MockBootServices::new();
 
         boot_mock.expect_locate_handle_buffer().returning(|_| Err(efi::Status::NOT_FOUND));
 
         let dxe_mock = MockDxeDispatcher::new(&[]);
 
-        let result = interleave_connect_and_dispatch(&boot_mock, &dxe_mock);
+        let result = interleave_connect_and_dispatch(helpers::connect_all, &boot_mock, &dxe_mock);
         assert!(result.is_err());
     }
 
     #[test]
-    fn test_interleave_dispatch_failure_propagates() {
+    fn test_simple_boot_manager_interleave_dispatch_failure_propagates() {
         let box_mock = leaked_boot_services_for_box();
         let mut boot_mock = MockBootServices::new();
 
@@ -262,12 +289,12 @@ mod tests {
 
         let dxe_mock = MockDxeDispatcher::new(&[Err(EfiError::DeviceError)]);
 
-        let result = interleave_connect_and_dispatch(&boot_mock, &dxe_mock);
+        let result = interleave_connect_and_dispatch(helpers::connect_all, &boot_mock, &dxe_mock);
         assert!(result.is_err());
     }
 
     #[test]
-    fn test_interleave_stops_at_max_rounds() {
+    fn test_simple_boot_manager_interleave_stops_at_max_rounds() {
         let box_mock = leaked_boot_services_for_box();
         let mut boot_mock = MockBootServices::new();
 
@@ -276,12 +303,48 @@ mod tests {
 
         let dxe_mock = MockDxeDispatcher::new(&[Ok(true); 10]);
 
-        let result = interleave_connect_and_dispatch(&boot_mock, &dxe_mock);
+        let result = interleave_connect_and_dispatch(helpers::connect_all, &boot_mock, &dxe_mock);
+        assert!(result.is_err(), "non-converging interleave must return an error, not silently succeed");
+        assert_eq!(result.unwrap_err(), EfiError::Timeout);
+    }
+
+    #[test]
+    fn test_simple_boot_manager_interleave_custom_connect_fn_failure() {
+        let boot_mock = MockBootServices::new();
+        let dxe_mock = MockDxeDispatcher::new(&[]);
+
+        let result =
+            interleave_connect_and_dispatch(|_bs: &MockBootServices| Err(EfiError::DeviceError), &boot_mock, &dxe_mock);
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn test_simple_boot_manager_interleave_custom_connect_fn_success() {
+        let boot_mock = MockBootServices::new();
+        let dxe_mock = MockDxeDispatcher::new(&[Ok(false)]);
+
+        let result = interleave_connect_and_dispatch(|_bs: &MockBootServices| Ok(()), &boot_mock, &dxe_mock);
         assert!(result.is_ok());
     }
 
+    // Tests for ConnectController and with_connect_strategy
+
+    #[test]
+    fn test_simple_boot_manager_with_connect_strategy() {
+        let config = BootConfig::new(test_device_path());
+        let manager = SimpleBootManager::with_connect_strategy(config, ConnectAllStrategy);
+        assert_eq!(manager.config().devices().count(), 1);
+    }
+
+    #[test]
+    fn test_simple_boot_manager_with_closure_connect_strategy() {
+        let config = BootConfig::new(test_device_path()).with_hotkey(0x16);
+        let manager = SimpleBootManager::with_connect_strategy(config, |_bs: &StandardBootServices| Ok(()));
+        assert_eq!(manager.config().hotkey(), Some(0x16));
+    }
+
     #[test]
-    fn test_new() {
+    fn test_simple_boot_manager_new() {
         let config = BootConfig::new(test_device_path()).with_hotkey(0x16).with_hotkey_device(test_device_path());
         let manager = SimpleBootManager::new(config);
         assert_eq!(manager.config().hotkey(), Some(0x16));
@@ -290,7 +353,7 @@ mod tests {
     }
 
     #[test]
-    fn test_with_hotkey() {
+    fn test_simple_boot_manager_with_hotkey() {
         let config = BootConfig::new(test_device_path())
             .with_device(test_device_path())
             .with_hotkey(0x16)
@@ -302,7 +365,7 @@ mod tests {
     }
 
     #[test]
-    fn test_without_hotkey() {
+    fn test_simple_boot_manager_without_hotkey() {
         let config = BootConfig::new(test_device_path()).with_device(test_device_path());
         let manager = SimpleBootManager::new(config);
         assert!(manager.config().hotkey().is_none());
@@ -311,7 +374,7 @@ mod tests {
     }
 
     #[test]
-    fn test_with_failure_handler() {
+    fn test_simple_boot_manager_with_failure_handler() {
         let called = Arc::new(AtomicBool::new(false));
         let called_clone = called.clone();
 

components/patina_boot/src/strategies.rs

@@ -0,0 +1,14 @@
+//! Built-in [`ConnectController`](crate::connect_controller::ConnectController) strategies.
+//!
+//! ## License
+//!
+//! Copyright (c) Microsoft Corporation.
+//!
+//! SPDX-License-Identifier: Apache-2.0
+//!
+
+mod connect_all;
+mod connect_by_protocol;
+
+pub use connect_all::ConnectAllStrategy;
+pub use connect_by_protocol::ConnectByProtocolStrategy;

components/patina_boot/src/strategies/connect_all.rs

@@ -0,0 +1,23 @@
+//! Connect-all strategy: connect all controllers.
+//!
+//! ## License
+//!
+//! Copyright (c) Microsoft Corporation.
+//!
+//! SPDX-License-Identifier: Apache-2.0
+//!
+
+use patina::{boot_services::BootServices, error::Result};
+
+use crate::{connect_controller::ConnectController, helpers};
+
+/// Connects all controllers recursively via
+/// [`helpers::connect_all()`](crate::helpers::connect_all).
+/// Used as the default strategy by [`SimpleBootManager::new()`](crate::SimpleBootManager::new).
+pub struct ConnectAllStrategy;
+
+impl<B: BootServices> ConnectController<B> for ConnectAllStrategy {
+    fn connect(&self, boot_services: &B) -> Result<()> {
+        helpers::connect_all(boot_services)
+    }
+}