Skip to content

[feature/patina-boot] patina_boot: Add discover_boot_options helper#1447

Open
kat-perez wants to merge 8 commits intoOpenDevicePartnership:feature/patina-bootfrom
kat-perez:kp/boot-option-discovery
Open

[feature/patina-boot] patina_boot: Add discover_boot_options helper#1447
kat-perez wants to merge 8 commits intoOpenDevicePartnership:feature/patina-bootfrom
kat-perez:kp/boot-option-discovery

Conversation

@kat-perez
Copy link
Copy Markdown
Contributor

@kat-perez kat-perez commented Apr 1, 2026

Description

Add discover_boot_options() helper to patina_boot::helpers that reads UEFI BootOrder and Boot#### variables to build a BootConfig from standard UEFI boot options.

This enables any BootOrchestrator implementation that consumes BootConfig to use UEFI-compliant boot variables instead of requiring platforms to hardcode device paths. The function:

  • Reads BootOrder to determine boot attempt order
  • Parses each Boot#### EFI_LOAD_OPTION structure to extract device paths
  • Filters out inactive boot options (LOAD_OPTION_ACTIVE)
  • Gracefully skips unreadable or malformed entries
  • Returns a populated BootConfig with discovered devices in priority order

  • Impacts functionality?
  • Impacts security?
  • Breaking change?
  • Includes tests?
  • Includes documentation?

How This Was Tested

  • Unit tests covering: single/multiple boot options, inactive option filtering, unreadable variable handling, truncated load option data, empty BootOrder, and hex variable name generation
  • Integration tested with patina-dxe-core-qemu feature/patina-boot on QEMU Q35 — full boot to UEFI Shell 2.0

Integration Instructions

Platforms can call discover_boot_options() with runtime services to automatically populate a BootConfig from UEFI boot variables instead of constructing device paths manually. This works with any BootOrchestrator implementation that accepts a BootConfig:

let config = discover_boot_options(&runtime_services)?;
add.component(BootDispatcher::new(SimpleBootManager::new(config)));

@patina-automation
Copy link
Copy Markdown
Contributor

patina-automation bot commented Apr 1, 2026

✅ QEMU Validation Passed

All QEMU validation jobs completed successfully.

Note: Q35 is only built on Windows hosts (QEMU boot is disabled due to a QEMU vfat issue).

Workflow run: https://github.com/OpenDevicePartnership/patina/actions/runs/23876918302

Boot Time to EFI Shell

Platform Elapsed
Q35 (Linux Host) 28.1s
SBSA (Linux Host) 1m 3s

Dependencies

Repository Ref
patina 5428475
patina-dxe-core-qemu 3eb6066
patina-fw-patcher 29264b2
patina-qemu firmware v3.0.0
patina-qemu build script bfc657f

This comment was automatically generated by the Patina QEMU PR Validation Post workflow.

@github-actions github-actions bot added impact:non-functional Does not have a functional impact impact:testing Affects testing labels Apr 1, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 1, 2026

Codecov Report

❌ Patch coverage is 96.87500% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
components/patina_boot/src/helpers.rs 96.88% 6 Missing ⚠️

📢 Thoughts on this report? Let us know!

@kat-perez kat-perez force-pushed the kp/boot-option-discovery branch 2 times, most recently from 8cee855 to 1892744 Compare April 1, 2026 20:13
…DevicePartnership#1225)

Adds BootOrchestration component, simple console discovery, simple
BootOption Config

- [x] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [x] Includes tests?
- [x] Includes documentation?

QEMU Platform Integration:
- Q35
- SBSA

N/A
… boot options (OpenDevicePartnership#1272)

## Description

Add hotkey detection support to the boot orchestrator, allowing
platforms to configure
alternate boot options that are used when a hotkey (e.g., F12) is
pressed during boot.

Changes:
- Add `detect_hotkey()` helper function to check for hotkey press via
SimpleTextInput protocol
- Add `hotkey_devices` field and `with_hotkey_device()` builder to
`BootOptions`
- Update `BootOrchestrator` to use alternate boot options when hotkey is
detected

---

- [x] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [x] Includes tests?
- [ ] Includes documentation?

## How This Was Tested

- Unit tests for `detect_hotkey()` (no input handles case)
- Unit tests for `hotkey_devices` config (single device, multiple
devices, combined with hotkey)
- `cargo test -p patina_boot` passes

## Integration Instructions

Platforms can configure hotkey boot options:

```rust
BootOptions::new()
    .with_device(primary_device)
    .with_hotkey(0x16) // F12 scancode
    .with_hotkey_device(alternate_device)
```

Closes OpenDevicePartnership#1228
…e_system_table (OpenDevicePartnership#1284)

Release SYSTEM_TABLE lock (TPL_NOTIFY) before accessing
ComponentDispatcher (TPL_APPLICATION) to avoid TPL violation.

PR OpenDevicePartnership#1225 lowered ComponentDispatcher from TPL_NOTIFY to TPL_APPLICATION
to allow components to use boot services, but this created a conflict
when initialize_system_table held SYSTEM_TABLE while setting
boot/runtime services on ComponentDispatcher.

Fix: Extract boot/runtime services pointers while holding SYSTEM_TABLE
lock, then release it before accessing ComponentDispatcher.

- [x] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

- Unit tests pass
- QEMU Q35 boots without TPL violation panic

N/A
…ion (OpenDevicePartnership#1290)

Add support for expanding partial (short-form) device paths to full device paths by matching against the device topology.

- Add `is_partial_device_path()` to detect partial paths (start with
Media/Messaging nodes instead of Hardware/ACPI)
- Add `expand_device_path()` to find matching full paths by enumerating
device handles
- Wire expansion into `boot_from_device_path()` for transparent handling
- Currently supports HardDrive nodes with GPT partition signature
matching

This enables booting from Boot#### variables containing partial device paths like `HD(1,GPT,<GUID>)\EFI\BOOT\BOOTX64.EFI`.

- [x] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [x] Includes tests?
- [ ] Includes documentation?

- Unit tests for `is_partial_device_path()`, `expand_device_path()`, and
signature matching
- `cargo test -p patina_boot` passes (32 tests)
- QEMU Q35 platform test passes

N/A

Closes OpenDevicePartnership#1280
…stration design (OpenDevicePartnership#1333)

## Description

Refactors `patina_boot` from a monolithic `BootOrchestrator` component +
`Config<BootOptions>` pattern to a trait-based design:

- **`BootOrchestrator` trait** — defines the boot flow interface with
`execute() -> Result<!, EfiError>`, enforcing at the type level that
successful boot never returns
- **`BootDispatcher`** — the Patina component that installs the BDS
architectural protocol and delegates to a `BootOrchestrator`
implementation
- **`SimpleBootManager`** — a default `BootOrchestrator` implementation
for platforms with straightforward boot topologies
- **`BootConfig`** — unified boot configuration (replaces previous
`BootOptions` + `SimpleBootConfig` split), requires at least one device
at construction (compile-time enforcement)

Also updates `helpers.rs` imports from removed
`uefi_protocol::device_path` to
`device_path::paths`/`device_path::node_defs`.

`patina_dxe_core` changes (image handle plumbing) split out to OpenDevicePartnership#1374.

- [x] Impacts functionality?
- [ ] Impacts security?
- [x] Breaking change?
- [x] Includes tests?
- [ ] Includes documentation?

## How This Was Tested

1. Unit tests: 35 pass (`cargo test -p patina_boot`)
2. Integration tested on QEMU Q35 — all components dispatched, BDS phase
ran, boot options attempted, failure handler fired correctly
3. CI: fmt, clippy, all platforms pass

## Integration Instructions

Update boot orchestration usage from:
```rust
use patina_boot::{component::BootOrchestrator, config::BootOptions};
// In configs():
add.config(BootOptions::new()...);
// In components():
add.component(BootOrchestrator);
```

To:
```rust
use patina_boot::{BootDispatcher, SimpleBootManager, config::BootConfig};

add.component(BootDispatcher::new(SimpleBootManager::new(
    BootConfig::new(primary_device_path())
        .with_device(fallback_device_path())
        .with_hotkey(0x16)
        .with_hotkey_device(usb_device_path())
        .with_failure_handler(|| { /* ... */ }),
)));
```
…penDevicePartnership#1375)

## Description

Rewrite `discover_console_devices()` from a stub into a full
implementation that enumerates console protocol handles, builds
multi-instance device paths, and writes `ConIn`, `ConOut`, and `ErrOut`
UEFI global variables via `SetVariable`.

- Adds `EFI_GLOBAL_VARIABLE` GUID to `patina::guids`
- Adds `build_multi_instance_device_path()` helper for constructing
multi-instance device paths from protocol handles
- Updates `is_partial_device_path()` to recognize FV/FvFile paths as
non-partial
- Includes get_variable readback verification with device path display
logging

Depends on OpenDevicePartnership#1333. Closes OpenDevicePartnership#1230

- [x] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

## How This Was Tested

Verified on QEMU Q35 with VGA enabled. Console variables written and
read back successfully:
- ConIn: 24 bytes (SimpleTextInput)
- ConOut: 60 bytes (SimpleTextOutput + GOP)
- ErrOut: 30 bytes (SimpleTextOutput)

## Integration Instructions

N/A
…with DxeServices (OpenDevicePartnership#1422)

## Description

Interleave controller connection with DXE driver dispatch during device
enumeration. Connecting controllers can discover new firmware volumes
(e.g., PCI option ROMs) that contain drivers for devices behind that
controller. Those drivers must be dispatched before the next round of
enumeration, otherwise the devices they serve will not be found.

`SimpleBootManager` uses `interleave_connect_and_dispatch()` to
alternate
between connecting controllers and dispatching newly-discovered drivers
until both stabilize. The `DxeDispatch` service trait (from OpenDevicePartnership#1421) is
consumed via dependency injection.

Note: `interleave_connect_and_dispatch()` currently uses `connect_all()`
which connects every controller on every round. This is functional but
inefficient for platforms with large device topologies — a future
optimization could connect only newly-discovered controllers.

- [x] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [x] Includes tests?
- [ ] Includes documentation?

## How This Was Tested

- Built SBSA DXE core binary with `BootDispatcher` + `SimpleBootManager`
replacing TianoCore BdsDxe
- Booted Windows ARM64 under QEMU SBSA-ref emulation with Patina BDS
handling the full boot flow
- Verified interleaving: controller connection discovered AHCI device,
partial device path expanded to full path, Windows bootloader loaded,
ExitBootServices completed

## Integration Instructions

Depends on OpenDevicePartnership#1421 (`DxeDispatch` service trait) for platform binary
integration.

Remove TianoCore `BdsDxe.inf` from platform DSC/FDF since the
`BootDispatcher` provides the BDS architectural protocol.
@kat-perez kat-perez force-pushed the feature/patina-boot branch from f168dff to d9458cb Compare April 1, 2026 20:29
@kat-perez kat-perez force-pushed the kp/boot-option-discovery branch from 1892744 to f39c078 Compare April 1, 2026 20:38
@kat-perez kat-perez marked this pull request as ready for review April 1, 2026 21:05
@kat-perez kat-perez changed the title patina_boot: Add discover_boot_options helper [feature/patina-boot] patina_boot: Add discover_boot_options helper Apr 1, 2026
Comment thread components/patina_boot/src/helpers.rs Outdated
const LOAD_OPTION_ACTIVE: u32 = 0x00000001;

/// Null-terminated UTF-16 `BootOrder` variable name.
const BOOT_ORDER_VARIABLE_NAME: &[u16] =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed this on earlier ones, but for these various UEFI spec defined things, we should be taking them to r-efi as well. Hosting them here until they are merged there is fine.

Comment thread components/patina_boot/src/helpers.rs Outdated
name.push(c as u16);
}
for c in hex_chars {
name.push(c.to_ascii_uppercase() as u16);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't these all digits? What does to_ascii_uppercase do on a digit?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

char::from_digit(n, 16) returns lowercase hex and UEFI variable names use uppercase. The to_ascii_uppercase was there to fix the casing. I changed this to use format!("Boot{:04X}", option_number) to specify more clearly that we want uppercase hex.

Comment thread components/patina_boot/src/helpers.rs Outdated
return None;
}

let attributes = u32::from_le_bytes([data[0], data[1], data[2], data[3]]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of from_le_bytes throughout all these changes. A place to use zerocopy?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use zerocopy

@kat-perez kat-perez force-pushed the kp/boot-option-discovery branch from f39c078 to 97c8715 Compare April 1, 2026 23:44
Reads BootOrder and Boot#### UEFI variables to build a BootConfig from
UEFI-compliant boot options, enabling SimpleBootManager to use standard
boot variables instead of platform-hardcoded device paths.
@kat-perez kat-perez force-pushed the kp/boot-option-discovery branch from 97c8715 to 5428475 Compare April 1, 2026 23:51
@kat-perez kat-perez requested a review from os-d April 2, 2026 15:19
// Connect each handle recursively
for &handle in handles.iter() {
// SAFETY: Empty driver handle list and null device path are valid per UEFI spec
let _ = unsafe { boot_services.connect_controller(handle, Vec::new(), ptr::null_mut(), true) };
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this allocates a new vector for each handle which could be a lot of allocations, right? Do you know how many allocations (calls) are being made on QEMU?

Comment on lines +549 to +551
let boot_order_name: &[u16] = &[
'B' as u16, 'o' as u16, 'o' as u16, 't' as u16, 'O' as u16, 'r' as u16, 'd' as u16, 'e' as u16, 'r' as u16, 0,
];
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You used encode_utf16() elsewhere in the file. Is there a reason not to use it here? Like:

let boot_order_name: Vec<u16> = "BootOrder\0".encode_utf16().collect();


let file_path_bytes = &rest[offset..file_path_end];

// SAFETY: file_path_bytes points to a valid device path from the EFI_LOAD_OPTION.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that this depends on the boot variable (e.g. Boot####) to have correctly sized device path nodes. For example, if a boot UEFI variable were written with an very large node length (possible since these are just non-authenticated UEFI variables), this call to try_from_ptr() will call from_raw_parts() which will use that length to read past the actual device path buffer.

Let me know if this is already checked somewhere, but, if not, I suggest that each device path node length is validated before handing the pointer to try_from_ptr().

Comment on lines +132 to +134
if let Err(e) = helpers::signal_ready_to_boot(boot_services) {
log::error!("signal_ready_to_boot failed: {:?}", e);
}
Copy link
Copy Markdown
Collaborator

@makubacki makubacki Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm reading this right, it is just calling ready to boot once and then looping through boot options? If so, it is expected to be called per boot option attempt.

After all SysPrep#### variables have been launched and exited, the platform shall notify EFI_EVENT_GROUP_READY_TO_BOOT and EFI_EVENT_GROUP_AFTER_READY_TO_BOOT event groups. This should happen when the Boot Manager is about to load and execute Boot#### variables with Attributes set to LOAD_OPTION_CATEGORY_BOOT according to the order defined by BootOrder.

https://uefi.org/specs/UEFI/2.11/03_Boot_Manager.html#required-system-preparation-applications

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

impact:non-functional Does not have a functional impact impact:testing Affects testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants