libefi: migrate to u-boot dt_fixup_protocol to update device tree Use dt_fixup_protocol instead of gbl_os_configuration_protocol to apply dt runtime fixups. The main reason is that it's already used across the ecosystem (Kernel EFI stub) and has better chances for further adoption (EDK2, etc). Bug: 373400571 Bug: 366400200 Test: implemented it on pineapple side and it boots Change-Id: Ia65614185f526ed1a2f5bc32e9cd8b68e37aa41f Signed-off-by: Dmitrii Merkurev <dimorinny@google.com>
diff --git a/gbl/docs/efi_protocols.md b/gbl/docs/efi_protocols.md index e73b648..bc3d4e0 100644 --- a/gbl/docs/efi_protocols.md +++ b/gbl/docs/efi_protocols.md
@@ -121,6 +121,28 @@ Used for logging and debugging. Implementations must provide this protocol, but the functions may be no-ops. +## Community Protocols + +Protocols defined by a community and used across the ecosystem, but not officially +part of the UEFI specification. None of these protocols are required. + +### DtFixupProtocol + +* original [proposal](https://github.com/U-Boot-EFI/EFI_DT_FIXUP_PROTOCOL) +* [upstream](https://github.com/u-boot/u-boot/blob/master/include/efi_dt_fixup.h) +* optional: allows FW to modify the final device tree + +This protocol allows the firmware (FW) to inspect the final device tree and apply +necessary fixups. + +GBL will validate the applied changes and prevent booting if any of the security +limitations (listed below) are violated. Any errors will be reported through the +UEFI log. + +TODO (b/353272981): Add limitations + +This protocol was proposed by U-Boot and is currently used by the Kernel UEFI stub. + ## GBL Custom Protocols These protocols are defined by GBL to provide specific functionality that is
diff --git a/gbl/docs/gbl_os_configuration_protocol.md b/gbl/docs/gbl_os_configuration_protocol.md index fb34d29..2cd4935 100644 --- a/gbl/docs/gbl_os_configuration_protocol.md +++ b/gbl/docs/gbl_os_configuration_protocol.md
@@ -7,16 +7,16 @@ ### Summary -This protocol provides a mechanism for the EFI firmware to update OS +This protocol provides a mechanism for the EFI firmware to build and update OS configuration data: -* kernel commandline -* bootconfig -* devicetree +* device tree (select components to build the final one) +* kernel commandline (append fixups) +* bootconfig (append fixups) GBL will load and verify the base data from disk, and then call these protocol functions to give the firmware a chance to construct and adjust the data as needed -for the particular device. +for the particular device. Device tree fixup is handled by `EFI_DT_FIXUP` protocol. If no runtime modifications are necessary, this protocol may be left unimplemented. @@ -50,7 +50,6 @@ GBL_EFI_FIXUP_KERNEL_COMMAND_LINE FixupKernelCommandline; GBL_EFI_FIXUP_BOOTCONFIG FixupBootConfig; GBL_EFI_SELECT_DEVICE_TREES SelectDeviceTrees; - GBL_EFI_FIXUP_DEVICE_TREE FixupDeviceTree; GBL_EFI_FIXUP_ZBI FixupZbi; } GBL_EFI_OS_CONFIGURATION_PROTOCOL; ``` @@ -73,9 +72,6 @@ Select components such as base device tree, overlays to build the final device tree. See [`SelectDeviceTrees()`](#SelectDeviceTrees). -#### FixupDeviceTree -Applies device tree fixups. See [`FixupDeviceTree()`](#FixupDeviceTree). - #### FixupZbi Applies ZBI fixups (Fuchsia kernels only). See [`FixupZbi()`](#FixupZbi). @@ -321,73 +317,6 @@ | `EFI_SUCCESS` | Base device tree, overlays has been selected. | | `EFI_INVALID_PARAMETER` | A parameter is invalid. For example, incorrect device trees, alignment. | -## GBL_EFI_OS_CONFIGURATION_PROTOCOL.FixupDeviceTree() {#FixupDeviceTree} - -### Summary - -Inspect the final device tree and apply required fixups. - -### Prototype - -```c -typedef EFI_STATUS (EFIAPI *GBL_EFI_FIXUP_DEVICE_TREE)( - IN GBL_EFI_OS_CONFIGURATION_PROTOCOL *This, - IN OUT VOID *DeviceTree, - IN OUT UINTN *DeviceTreeBufferSize - ); -``` - -### Parameters - -Ownership of all the parameters is loaned only for the duration of the function call, and -must not be retained by the protocol after returning. - -#### This -A pointer to the `GBL_EFI_OS_CONFIGURATION_PROTOCOL` instance. - -#### DeviceTree [in, out] - -Pointer to the buffer with the device tree built by GBL. Firmware is allowed to modify -it by applying fixups with the following restrictions: - -* on return, the data must be a valid device tree. Including the FDT header with the updated - `totalsize` field to let GBL identify a new device tree size. -* result data must never exceed the provided `DeviceTreeBufferSize` -* security restrictions mentioned below - -#### DeviceTreeBufferSize [in, out] - -On function call, this points to the buffer size provided by `DeviceTree`. An implementation -is allowed to update the provided device tree up to this size. - -If the buffer is not large enough to fit the fixups, the function should update -`DeviceTreeBufferSize` with the required size (including both device tree and fixups) and -return `EFI_BUFFER_TOO_SMALL`; GBL will then allocate a larger buffer, discard all -modifications and repeat the `FixupDeviceTree` call. - -### Description - -When the device tree is built from the artifacts (`boot`, `vendor_boot`, `dtb`, `dtbo`), some -fixups (which can only be determined at run-time) may need to be applied by the firmware -implementation. To allow this, GBL provides a read/write pointer to the device tree it built -throught `FixupDeviceTree`. GBL will verify the applied changes and fail to boot if any of the -security limitations (noted below) are violated. The error message will be communicated through -UEFI log. - - -#### Security - -TODO(b/353272981): add device tree fixup limitations - -#### Status Codes Returned - -| | | -| ----------------------- | ---------------------------------------------------------------------------------------- | -| `EFI_SUCCESS` | Device tree fixup has been applied. | -| `EFI_INVALID_PARAMETER` | A parameter is invalid. For example, incorrect device trees, alignment. | -| `EFI_BUFFER_TOO_SMALL` | The buffer is too small; `DeviceTreeBufferSize` has been updated with the required size. | -| `EFI_DEVICE_ERROR` | Internal error while applying device tree fixup. | - ## GBL_EFI_OS_CONFIGURATION_PROTOCOL.FixupZbi() {#FixupZbi} TODO(b/353272981)
diff --git a/gbl/efi/src/ops.rs b/gbl/efi/src/ops.rs index 8519e88..7ffbe41 100644 --- a/gbl/efi/src/ops.rs +++ b/gbl/efi/src/ops.rs
@@ -22,7 +22,8 @@ use alloc::alloc::{alloc, handle_alloc_error, Layout}; use core::{ffi::CStr, fmt::Write, mem::MaybeUninit, num::NonZeroUsize, slice::from_raw_parts_mut}; use efi::{ - efi_print, efi_println, protocol::gbl_efi_image_loading::GblImageLoadingProtocol, + efi_print, efi_println, protocol::dt_fixup::DtFixupProtocol, + protocol::gbl_efi_image_loading::GblImageLoadingProtocol, protocol::gbl_efi_os_configuration::GblOsConfigurationProtocol, EfiEntry, }; use efi_types::{GblEfiImageInfo, PARTITION_NAME_LEN_U16}; @@ -331,13 +332,10 @@ } fn fixup_device_tree(&mut self, device_tree: &mut [u8]) -> Result<()> { - if let Ok(protocol) = self - .efi_entry - .system_table() - .boot_services() - .find_first_and_open::<GblOsConfigurationProtocol>() + if let Ok(protocol) = + self.efi_entry.system_table().boot_services().find_first_and_open::<DtFixupProtocol>() { - protocol.fixup_device_tree(device_tree)?; + protocol.fixup(device_tree)?; } Ok(())
diff --git a/gbl/libefi/mocks/protocol.rs b/gbl/libefi/mocks/protocol.rs index f7d85e1..2950c44 100644 --- a/gbl/libefi/mocks/protocol.rs +++ b/gbl/libefi/mocks/protocol.rs
@@ -179,3 +179,19 @@ /// Map to the libefi name so code under test can just use one name. pub type GblOsConfigurationProtocol = MockGblOsConfigurationProtocol; } + +/// Mock dt_fixup protocol. +pub mod dt_fixup { + use super::*; + + mock! { + /// Mock [efi::DtFixupProtocol]. + pub DtFixupProtocol { + /// Wraps `EFI_DT_FIXUP_PROTOCOL.fixup()` + pub fn fixup(&self, device_tree: &mut [u8]) -> Result<()>; + } + } + + /// Map to the libefi name so code under test can just use one name. + pub type DtFixupProtocol = MockDtFixupProtocol; +}
diff --git a/gbl/libefi/src/protocol.rs b/gbl/libefi/src/protocol.rs index 0422846..0dd7373 100644 --- a/gbl/libefi/src/protocol.rs +++ b/gbl/libefi/src/protocol.rs
@@ -22,6 +22,7 @@ pub mod block_io; pub mod block_io2; pub mod device_path; +pub mod dt_fixup; pub mod gbl_efi_ab_slot; pub mod gbl_efi_fastboot; pub mod gbl_efi_fastboot_usb;
diff --git a/gbl/libefi/src/protocol/dt_fixup.rs b/gbl/libefi/src/protocol/dt_fixup.rs new file mode 100644 index 0000000..f205643 --- /dev/null +++ b/gbl/libefi/src/protocol/dt_fixup.rs
@@ -0,0 +1,133 @@ +// Copyright 2024, The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Rust wrapper for `EFI_DT_FIXUP_PROTOCOL`. + +use crate::efi_call; +use crate::protocol::{Protocol, ProtocolInfo}; +use efi_types::{EfiDtFixupProtocol, EfiGuid, EFI_DT_APPLY_FIXUPS}; +use liberror::Result; + +/// `EFI_DT_FIXUP_PROTOCOL` implementation. +pub struct DtFixupProtocol; + +impl ProtocolInfo for DtFixupProtocol { + type InterfaceType = EfiDtFixupProtocol; + + const GUID: EfiGuid = + EfiGuid::new(0xe617d64c, 0xfe08, 0x46da, [0xf4, 0xdc, 0xbb, 0xd5, 0x87, 0x0c, 0x73, 0x00]); +} + +// Protocol interface wrappers. +impl Protocol<'_, DtFixupProtocol> { + /// Wraps `EFI_DT_FIXUP_PROTOCOL.fixup()`. + pub fn fixup(&self, device_tree: &mut [u8]) -> Result<()> { + let mut buffer_size = device_tree.len(); + + // SAFETY: + // * `self.interface()?` guarantees self.interface is non-null and points to a valid object + // established by `Protocol::new()`. + // * `device_tree` is non-null buffer available for write, used only within the call. + // * `buffer_size` is non-null usize buffer available for write, used only within the call. + unsafe { + efi_call!( + @bufsize buffer_size, + self.interface()?.fixup, + self.interface, + device_tree.as_mut_ptr() as _, + &mut buffer_size, + EFI_DT_APPLY_FIXUPS + )?; + } + + Ok(()) + } +} + +#[cfg(test)] +mod test { + use super::*; + + use crate::test::run_test_with_mock_protocol; + use efi_types::{EfiStatus, EFI_STATUS_BUFFER_TOO_SMALL, EFI_STATUS_SUCCESS}; + use liberror::Error; + use std::{ffi::c_void, slice}; + + #[test] + fn fixup_device_tree_updated() { + // Don't check actual FDT content for simplicity. + const DEVICE_TREE_BUFFER: &[u8] = b"this_is_device_tree"; + const UPDATED_DEVICE_TREE_BUFFER: &[u8] = b"this_is_device_trie"; + + // C callback implementation to modify provided FDT to UPDATED_DEVICE_TREE_BUFFER. + unsafe extern "C" fn c_modify( + _: *mut EfiDtFixupProtocol, + device_tree: *mut c_void, + buffer_size: *mut usize, + flags: u32, + ) -> EfiStatus { + assert_eq!(flags, EFI_DT_APPLY_FIXUPS); + // SAFETY: + // * `device_tree` is a valid pointer to the writtable buffer at least `buffer_size` + // size. + // * `buffer_size` is a valid pointer to usize. + let fdt_buffer = + unsafe { slice::from_raw_parts_mut(device_tree as *mut u8, *buffer_size) }; + assert_eq!(fdt_buffer, DEVICE_TREE_BUFFER); + + fdt_buffer.copy_from_slice(UPDATED_DEVICE_TREE_BUFFER); + + EFI_STATUS_SUCCESS + } + + let c_interface = EfiDtFixupProtocol { fixup: Some(c_modify), ..Default::default() }; + + run_test_with_mock_protocol(c_interface, |dt_fixup_protocol| { + let mut fdt_buffer: Vec<u8> = DEVICE_TREE_BUFFER.to_vec(); + + assert!(dt_fixup_protocol.fixup(&mut fdt_buffer[..]).is_ok()); + assert_eq!(&fdt_buffer[..], UPDATED_DEVICE_TREE_BUFFER); + }); + } + + #[test] + fn fixup_device_tree_fixup_buffer_too_small() { + const EXPECTED_REQUESTED_FIXUP_SIZE: usize = 256; + // C callback implementation to return an error. + unsafe extern "C" fn c_error( + _: *mut EfiDtFixupProtocol, + _: *mut c_void, + buffer_size: *mut usize, + _: u32, + ) -> EfiStatus { + // SAFETY: + // * `buffer_size` is a valid pointer to writtable usize buffer. + unsafe { + *buffer_size = EXPECTED_REQUESTED_FIXUP_SIZE; + } + EFI_STATUS_BUFFER_TOO_SMALL + } + + let c_interface = EfiDtFixupProtocol { fixup: Some(c_error), ..Default::default() }; + + run_test_with_mock_protocol(c_interface, |dt_fixup_protocol| { + let mut fdt_buffer = [0u8; 128]; + + assert_eq!( + dt_fixup_protocol.fixup(&mut fdt_buffer[..]), + Err(Error::BufferTooSmall(Some(EXPECTED_REQUESTED_FIXUP_SIZE))), + ); + }); + } +}
diff --git a/gbl/libefi/src/protocol/gbl_efi_os_configuration.rs b/gbl/libefi/src/protocol/gbl_efi_os_configuration.rs index 4d1a175..5bde867 100644 --- a/gbl/libefi/src/protocol/gbl_efi_os_configuration.rs +++ b/gbl/libefi/src/protocol/gbl_efi_os_configuration.rs
@@ -87,28 +87,6 @@ Ok(fixup_size) } - - /// Wraps `GBL_EFI_OS_CONFIGURATION_PROTOCOL.fixup_device_tree()`. - pub fn fixup_device_tree(&self, device_tree: &mut [u8]) -> Result<()> { - let mut buffer_size = device_tree.len(); - - // SAFETY: - // * `self.interface()?` guarantees self.interface is non-null and points to a valid object - // established by `Protocol::new()`. - // * `device_tree` is non-null buffer available for write, used only within the call. - // * `buffer_size` is non-null usize buffer available for write, used only within the call. - unsafe { - efi_call!( - @bufsize buffer_size, - self.interface()?.fixup_device_tree, - self.interface, - device_tree.as_mut_ptr() as _, - &mut buffer_size - )?; - } - - Ok(()) - } } #[cfg(test)] @@ -119,7 +97,7 @@ use efi_types::{ EfiStatus, EFI_STATUS_BUFFER_TOO_SMALL, EFI_STATUS_INVALID_PARAMETER, EFI_STATUS_SUCCESS, }; - use std::{ffi::c_void, ffi::CStr, slice}; + use std::{ffi::CStr, slice}; #[test] fn fixup_kernel_commandline_no_op() { @@ -402,74 +380,4 @@ ); }); } - - #[test] - fn fixup_device_tree_updated() { - // Don't check actual FDT content for simplicity. - const DEVICE_TREE_BUFFER: &[u8] = b"this_is_device_tree"; - const UPDATED_DEVICE_TREE_BUFFER: &[u8] = b"this_is_device_trie"; - - // C callback implementation to modify provided FDT to UPDATED_DEVICE_TREE_BUFFER. - unsafe extern "C" fn c_modify( - _: *mut GblEfiOsConfigurationProtocol, - device_tree: *mut c_void, - buffer_size: *mut usize, - ) -> EfiStatus { - // SAFETY: - // * `device_tree` is a valid pointer to the writtable buffer at least `buffer_size` - // size. - // * `buffer_size` is a valid pointer to usize. - let fdt_buffer = - unsafe { slice::from_raw_parts_mut(device_tree as *mut u8, *buffer_size) }; - assert_eq!(fdt_buffer, DEVICE_TREE_BUFFER); - - fdt_buffer.copy_from_slice(UPDATED_DEVICE_TREE_BUFFER); - - EFI_STATUS_SUCCESS - } - - let c_interface = GblEfiOsConfigurationProtocol { - fixup_device_tree: Some(c_modify), - ..Default::default() - }; - - run_test_with_mock_protocol(c_interface, |os_config_protocol| { - let mut fdt_buffer: Vec<u8> = DEVICE_TREE_BUFFER.to_vec(); - - assert!(os_config_protocol.fixup_device_tree(&mut fdt_buffer[..]).is_ok()); - assert_eq!(&fdt_buffer[..], UPDATED_DEVICE_TREE_BUFFER); - }); - } - - #[test] - fn fixup_device_tree_fixup_buffer_too_small() { - const EXPECTED_REQUESTED_FIXUP_SIZE: usize = 256; - // C callback implementation to return an error. - unsafe extern "C" fn c_error( - _: *mut GblEfiOsConfigurationProtocol, - _: *mut c_void, - buffer_size: *mut usize, - ) -> EfiStatus { - // SAFETY: - // * `buffer_size` is a valid pointer to writtable usize buffer. - unsafe { - *buffer_size = EXPECTED_REQUESTED_FIXUP_SIZE; - } - EFI_STATUS_BUFFER_TOO_SMALL - } - - let c_interface = GblEfiOsConfigurationProtocol { - fixup_device_tree: Some(c_error), - ..Default::default() - }; - - run_test_with_mock_protocol(c_interface, |os_config_protocol| { - let mut fdt_buffer = [0u8; 128]; - - assert_eq!( - os_config_protocol.fixup_device_tree(&mut fdt_buffer[..]), - Err(Error::BufferTooSmall(Some(EXPECTED_REQUESTED_FIXUP_SIZE))), - ); - }); - } }
diff --git a/gbl/libefi_types/defs/efi.h b/gbl/libefi_types/defs/efi.h index a58b9b6..90fb56c 100644 --- a/gbl/libefi_types/defs/efi.h +++ b/gbl/libefi_types/defs/efi.h
@@ -27,6 +27,7 @@ #include "protocols/block_io2_protocol.h" #include "protocols/block_io_protocol.h" #include "protocols/device_path_protocol.h" +#include "protocols/dt_fixup_protocol.h" #include "protocols/gbl_efi_ab_slot_protocol.h" #include "protocols/gbl_efi_fastboot_protocol.h" #include "protocols/gbl_efi_fastboot_usb.h"
diff --git a/gbl/libefi_types/defs/protocols/dt_fixup_protocol.h b/gbl/libefi_types/defs/protocols/dt_fixup_protocol.h new file mode 100644 index 0000000..5d220b2 --- /dev/null +++ b/gbl/libefi_types/defs/protocols/dt_fixup_protocol.h
@@ -0,0 +1,45 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +// This is a protocol proposed by U-boot and being used by Kernel UEFI stub. +// https://github.com/U-Boot-EFI/EFI_DT_FIXUP_PROTOCOL +// https://github.com/u-boot/u-boot/blob/master/include/efi_dt_fixup.h + +#ifndef __EFI_DT_FIXUP_PROTOCOL_H__ +#define __EFI_DT_FIXUP_PROTOCOL_H__ + +#include "types.h" + +const uint64_t EFI_DT_FIXUP_PROTOCOL_REVISION = 0x00010000; + +// Add nodes and update properties +const uint32_t EFI_DT_APPLY_FIXUPS = 0x00000001; +// Reserve memory according to the /reserved-memory node and the memory +// reservation block +const uint32_t EFI_DT_RESERVE_MEMORY = 0x00000002; +// Install the device-tree as configuration table +const uint32_t EFI_DT_INSTALL_TABLE = 0x00000004; +const uint32_t EFI_DT_ALL = + EFI_DT_APPLY_FIXUPS | EFI_DT_RESERVE_MEMORY | EFI_DT_INSTALL_TABLE; + +typedef struct EfiDtFixupProtocol { + uint64_t revision; + EfiStatus (*fixup)(struct EfiDtFixupProtocol* self, void* fdt, + size_t* buffer_size, uint32_t flags); +} EfiDtFixupProtocol; + +#endif // __EFI_DT_FIXUP_PROTOCOL_H__ \ No newline at end of file
diff --git a/gbl/libgbl/src/ops.rs b/gbl/libgbl/src/ops.rs index be742c8..5b1c8f9 100644 --- a/gbl/libgbl/src/ops.rs +++ b/gbl/libgbl/src/ops.rs
@@ -266,7 +266,8 @@ /// /// Modified device tree will be verified and used to boot a device. Refer to the behavior /// specified for the corresponding UEFI interface: - /// https://cs.android.com/android/platform/superproject/main/+/main:bootable/libbootloader/gbl/docs/gbl_os_configuration_protocol.md + /// https://cs.android.com/android/platform/superproject/main/+/main:bootable/libbootloader/gbl/docs/efi_protocols.md + /// https://github.com/U-Boot-EFI/EFI_DT_FIXUP_PROTOCOL fn fixup_device_tree(&mut self, device_tree: &mut [u8]) -> Result<(), Error>; }