Snap for 11084970 from eb5a5c6f79a1649a21b9963f954a73a46ebf464f to 24Q1-release

Change-Id: I152a08d55c296d7f60e1bcb3d86e6b6b1978acde

12 files changed, 584 insertions, 17 deletions

diff --git a/gbl/efi/BUILD b/gbl/efi/BUILD
index 7f3c858..7224db6 100644
--- a/gbl/efi/BUILD
+++ b/gbl/efi/BUILD
@@ -36,6 +36,7 @@ rust_binary(
         "panic=abort",
     ],
     deps = [
+        "@gbl//libboot",
         "@gbl//libbootimg",
         "@gbl//libefi",
         "@gbl//libfdt",
diff --git a/gbl/integration/aosp_u-boot-mainline/workspace.bzl b/gbl/integration/aosp_u-boot-mainline/workspace.bzl
index ae3ae21..8c1ffc4 100644
--- a/gbl/integration/aosp_u-boot-mainline/workspace.bzl
+++ b/gbl/integration/aosp_u-boot-mainline/workspace.bzl
@@ -54,7 +54,14 @@ def define_gbl_workspace(name = None):
     native.new_local_repository(
         name = "linux_x86_64_sysroot",
         path = "build/kernel/build-tools",
-        build_file_content = "",
+        build_file_content = """exports_files(glob(["**/*"]))
+cc_library(
+    name = "linux_x86_64_sysroot_include",
+    hdrs = glob(["sysroot/usr/include/**/*.h"]),
+    includes = [ "sysroot/usr/include" ],
+    visibility = ["//visibility:public"],
+)
+""",
     )
 
     android_rust_prebuilts(
diff --git a/gbl/libboot/BUILD b/gbl/libboot/BUILD
new file mode 100644
index 0000000..1159e50
--- /dev/null
+++ b/gbl/libboot/BUILD
@@ -0,0 +1,63 @@
+# Copyright (C) 2023 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.
+
+load("@rules_rust//bindgen:bindgen.bzl", "rust_bindgen")
+load("@rules_rust//rust:defs.bzl", "rust_library")
+
+package(
+    default_visibility = ["//visibility:public"],
+)
+
+CUSTOM_DERIVES = "AsBytes,FromBytes,FromZeroes"
+
+rust_bindgen(
+    name = "x86_bootparam_bindgen",
+    bindgen_flags = [
+        "--ctypes-prefix",
+        "core::ffi",
+        "--use-core",
+        "--allowlist-type",
+        "boot_params",
+        "--with-derive-default",
+        "--with-derive-custom-struct=.*={}".format(CUSTOM_DERIVES),
+        "--with-derive-custom-union=.*={}".format(CUSTOM_DERIVES),
+        "--raw-line",
+        """
+#![allow(non_camel_case_types)]
+#![allow(non_snake_case)]
+#![cfg_attr(not(test), no_std)]
+use zerocopy::{AsBytes, FromBytes, FromZeroes};""",
+    ],
+    cc_lib = "@linux_x86_64_sysroot//:linux_x86_64_sysroot_include",
+    header = "@linux_x86_64_sysroot//:sysroot/usr/include/x86_64-linux-gnu/asm/bootparam.h",
+)
+
+rust_library(
+    name = "x86_bootparam_defs",
+    srcs = [":x86_bootparam_bindgen"],
+    crate_root = ":x86_bootparam_bindgen",
+    data = [":x86_bootparam_bindgen"],
+    deps = ["@zerocopy"],
+)
+
+rust_library(
+    name = "libboot",
+    srcs = glob(["**/*.rs"]),
+    crate_name = "boot",
+    edition = "2021",
+    deps = [
+        ":x86_bootparam_defs",
+        "@zerocopy",
+    ],
+)
diff --git a/gbl/libboot/src/aarch64.rs b/gbl/libboot/src/aarch64.rs
new file mode 100644
index 0000000..7fa7a9b
--- /dev/null
+++ b/gbl/libboot/src/aarch64.rs
@@ -0,0 +1,69 @@
+// Copyright 2023, 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.
+
+//! Docs for booting on AArch64 is at:
+//!
+//!   https://www.kernel.org/doc/html/v5.11/arm64/booting.html
+
+use core::arch::asm;
+
+#[derive(Debug, PartialEq)]
+pub enum ExceptionLevel {
+    EL0,
+    EL1,
+    EL2,
+    EL3,
+}
+
+/// Gets the current EL;
+pub fn current_el() -> ExceptionLevel {
+    let mut el: u64;
+    // SAFETY: The assembly code only read current exception level.
+    unsafe {
+        asm!(
+            "mrs {el}, CurrentEL",
+            el = out(reg) el,
+        );
+    }
+    el = (el >> 2) & 3;
+    match el {
+        0 => ExceptionLevel::EL0,
+        1 => ExceptionLevel::EL1,
+        2 => ExceptionLevel::EL2,
+        3 => ExceptionLevel::EL3,
+        v => panic!("Unknown EL {v}"),
+    }
+}
+
+/// Boots a Linux kernel in mode EL2 or lower with the given FDT blob.
+///
+/// # Safety
+///
+/// Caller must ensure that `kernel` contains a valid Linux kernel.
+pub unsafe fn jump_linux_el2_or_lower(kernel: &[u8], fdt: &[u8]) -> ! {
+    assert_ne!(current_el(), ExceptionLevel::EL3);
+    // TODO(b/305093905): Perform other initialization including disabling MMU, flushing cache and
+    // masking interrupt.
+
+    asm!(
+        "mov x1, 0",
+        "mov x2, 0",
+        "mov x3, 0",
+        "br x4",
+        in("x4") kernel.as_ptr() as usize,
+        in("x0") fdt.as_ptr() as usize,
+    );
+
+    unreachable!();
+}
diff --git a/gbl/libboot/src/lib.rs b/gbl/libboot/src/lib.rs
new file mode 100644
index 0000000..bd878db
--- /dev/null
+++ b/gbl/libboot/src/lib.rs
@@ -0,0 +1,35 @@
+// Copyright 2023, 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.
+
+#![cfg_attr(not(test), no_std)]
+
+// Library error type.
+#[derive(Debug)]
+pub enum BootError {
+    IntegerOverflow,
+    InvalidInput,
+    InvalidZImage,
+    UnsupportedZImage,
+    E820MemoryMapCallbackError(i64),
+}
+
+/// Library result type,
+pub type Result<T> = core::result::Result<T, BootError>;
+
+#[cfg(target_arch = "aarch64")]
+pub mod aarch64;
+#[cfg(target_arch = "riscv64")]
+pub mod riscv64;
+#[cfg(any(target_arch = "x86_64", target_arch = "x86"))]
+pub mod x86;
diff --git a/gbl/libboot/src/riscv64.rs b/gbl/libboot/src/riscv64.rs
new file mode 100644
index 0000000..7af5242
--- /dev/null
+++ b/gbl/libboot/src/riscv64.rs
@@ -0,0 +1,34 @@
+// Copyright 2023, 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.
+
+use core::arch::asm;
+
+/// Boots a Linux kernel with the given boot hart ID and FDT blob.
+///
+/// # Safety
+///
+/// Caller must ensure that `kernel` contains a valid Linux kernel.
+pub unsafe fn jump_linux(kernel: &[u8], boot_hart_id: usize, fdt: &[u8]) -> ! {
+    // No official documentation exists yet. This is equivalent to a C function call taking
+    // the hart ID and FDT address as parameters.
+    asm!(
+        "csrw satp, zero",
+        "jr {ep}",
+        in("a0") boot_hart_id,
+        in("a1") fdt.as_ptr() as usize,
+        ep = in(reg) kernel.as_ptr() as usize,
+    );
+
+    unreachable!();
+}
diff --git a/gbl/libboot/src/x86.rs b/gbl/libboot/src/x86.rs
new file mode 100644
index 0000000..37213f5
--- /dev/null
+++ b/gbl/libboot/src/x86.rs
@@ -0,0 +1,252 @@
+// Copyright 2023, 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.
+
+//! Boot protocol implementation for x86 platforms.
+//!
+//! For linux, the library currently only supports bzimage and protocol version 2.06+.
+//! Specifically, modern memory layout is used, protected kernel is loaded to high address at
+//! 0x100000 and command line size can be greater than 255 characters.
+//!
+//!                     ~                        ~
+//!                     |  Protected-mode kernel |
+//!             100000  +------------------------+
+//!                     |  I/O memory hole       |
+//!             0A0000  +------------------------+
+//!                     |  Reserved for BIOS     |      Leave as much as possible unused
+//!                     ~                        ~
+//!                     |  Command line          |      (Can also be below the X+10000 mark)
+//!                     +------------------------+
+//!                     |  Stack/heap            |      For use by the kernel real-mode code.
+//!  low_mem_addr+08000 +------------------------+
+//!                     |  Kernel setup          |      The kernel real-mode code.
+//!                     |  Kernel boot sector    |      The kernel legacy boot sector.
+//!        low_mem_addr +------------------------+
+//!                     |  Boot loader           |      <- Boot sector entry point 0000:7C00
+//!             001000  +------------------------+
+//!                     |  Reserved for MBR/BIOS |
+//!             000800  +------------------------+
+//!                     |  Typically used by MBR |
+//!             000600  +------------------------+
+//!                     |  BIOS use only         |
+//!             000000  +------------------------+
+//!
+//! See https://www.kernel.org/doc/html/v5.11/x86/boot.html#the-linux-x86-boot-protocol for more
+//! detail.
+
+use crate::*;
+
+use core::arch::asm;
+use core::slice::from_raw_parts_mut;
+
+pub use x86_bootparam_defs::{boot_params, e820entry, setup_header};
+use zerocopy::{AsBytes, FromBytes, FromZeroes, Ref};
+
+// Sector size is fixed to 512
+const SECTOR_SIZE: usize = 512;
+/// Boot sector and setup code section is 32K at most.
+const BOOT_SETUP_LOAD_SIZE: usize = 0x8000;
+/// Address for loading the protected mode kernel
+const LOAD_ADDR_HIGH: usize = 0x10_0000;
+// Flag value to use high address for protected mode kernel.
+const LOAD_FLAG_LOADED_HIGH: u8 = 0x1;
+
+/// Constant for E820 address range type.
+pub const E820_ADDRESS_TYPE_RAM: u32 = 1;
+pub const E820_ADDRESS_TYPE_RESERVED: u32 = 2;
+pub const E820_ADDRESS_TYPE_ACPI: u32 = 3;
+pub const E820_ADDRESS_TYPE_NVS: u32 = 4;
+pub const E820_ADDRESS_TYPE_UNUSABLE: u32 = 5;
+pub const E820_ADDRESS_TYPE_PMEM: u32 = 7;
+
+/// Wrapper for `struct boot_params {}` C structure
+#[repr(transparent)]
+#[derive(Copy, Clone, AsBytes, FromBytes, FromZeroes)]
+pub struct BootParams(boot_params);
+
+impl BootParams {
+    /// Cast a bytes into a reference of BootParams header
+    pub fn from_bytes_ref(buffer: &[u8]) -> Result<&BootParams> {
+        Ok(Ref::<_, BootParams>::new_from_prefix(buffer)
+            .ok_or_else(|| BootError::InvalidInput)?
+            .0
+            .into_ref())
+    }
+
+    /// Cast a bytes into a mutable reference of BootParams header.
+    pub fn from_bytes_mut(buffer: &mut [u8]) -> Result<&mut BootParams> {
+        Ok(Ref::<_, BootParams>::new_from_prefix(buffer)
+            .ok_or_else(|| BootError::InvalidInput)?
+            .0
+            .into_mut())
+    }
+
+    /// Return a mutable reference of the `setup_header` struct field in `boot_params`
+    pub fn setup_header_mut(&mut self) -> &mut setup_header {
+        &mut self.0.hdr
+    }
+
+    /// Return a const reference of the `setup_header` struct field in `boot_params`
+    pub fn setup_header_ref(&self) -> &setup_header {
+        &self.0.hdr
+    }
+
+    /// Checks whether image is valid and version is supported.
+    pub fn check(&self) -> Result<()> {
+        // Check magic.
+        if !(self.setup_header_ref().boot_flag == 0xAA55
+            && self.setup_header_ref().header.to_le_bytes() == *b"HdrS")
+        {
+            return Err(BootError::InvalidZImage);
+        }
+
+        // Check if it is bzimage and version is supported.
+        if !(self.0.hdr.version >= 0x0206
+            && ((self.setup_header_ref().loadflags & LOAD_FLAG_LOADED_HIGH) != 0))
+        {
+            return Err(BootError::UnsupportedZImage);
+        }
+
+        Ok(())
+    }
+
+    /// Gets the number of sectors in the setup code section.
+    pub fn setup_sects(&self) -> usize {
+        match self.setup_header_ref().setup_sects {
+            0 => 4,
+            v => v as usize,
+        }
+    }
+
+    /// Gets the offset to the protected mode kernel in the image.
+    ///
+    /// The value is also the same as the sum of legacy boot sector plus setup code size.
+    pub fn kernel_off(&self) -> usize {
+        // one boot sector + setup sectors
+        (1 + self.setup_sects()) * SECTOR_SIZE
+    }
+
+    /// Gets e820 map entries.
+    pub fn e820_map(&mut self) -> &mut [e820entry] {
+        &mut self.0.e820_map[..]
+    }
+}
+
+/// Boots a Linux bzimage.
+///
+/// # Args
+///
+/// * `kernel`: Buffer holding the loaded bzimage.
+///
+/// * `ramdisk`: Buffer holding the loaded ramdisk.
+///
+/// * `cmdline`: Command line argument blob.
+///
+/// * `mmap_cb`: A caller provided callback for setting the e820 memory map. The callback takes in
+///     a mutable reference of e820 map entries (&mut [e820entry]). On success, it should return
+///     the number of used entries. On error, it can return a
+///     `BootError::E820MemoryMapCallbackError(<code>)` to propagate a custom error code.
+///
+/// * `low_mem_addr`: The lowest memory touched by the bootloader section. This is where boot param
+///      starts.
+///
+/// * The API is not expected to return on success.
+///
+/// # Safety
+///
+/// * Caller must ensure that `kernel` contains a valid Linux kernel and `low_mem_addr` is valid
+///
+/// * Caller must ensure that there is enough memory at address 0x10_0000 for relocating `kernel`.
+pub unsafe fn boot_linux_bzimage<F>(
+    kernel: &[u8],
+    ramdisk: &[u8],
+    cmdline: &[u8],
+    mmap_cb: F,
+    low_mem_addr: usize,
+) -> Result<()>
+where
+    F: FnOnce(&mut [e820entry]) -> Result<u8>,
+{
+    let bootparam = BootParams::from_bytes_ref(&kernel[..])?;
+    bootparam.check()?;
+
+    // low memory address greater than 0x9_0000 is bogus.
+    assert!(low_mem_addr <= 0x9_0000);
+    let boot_param_buffer = from_raw_parts_mut(low_mem_addr as *mut _, BOOT_SETUP_LOAD_SIZE);
+    // Note: We currently boot directly from protected mode kernel and bypass real-mode kernel.
+    // Thus we omit the heap section. Revisit this if we encounter platforms that have to boot from
+    // real-mode kernel.
+    let cmdline_start = low_mem_addr + BOOT_SETUP_LOAD_SIZE;
+    // Should not reach into I/O memory hole section.
+    assert!(cmdline_start + cmdline.len() <= 0x0A0000);
+    let cmdline_buffer = from_raw_parts_mut(cmdline_start as *mut _, cmdline.len());
+
+    let boot_sector_size = bootparam.kernel_off();
+    // Copy protected mode kernel to load address
+    from_raw_parts_mut(LOAD_ADDR_HIGH as *mut u8, kernel[boot_sector_size..].len())
+        .clone_from_slice(&kernel[boot_sector_size..]);
+
+    // Copy over boot params to boot sector to prepare for fix-up.
+    boot_param_buffer.fill(0);
+    boot_param_buffer[..boot_sector_size].clone_from_slice(&kernel[..boot_sector_size]);
+
+    let bootparam_fixup = BootParams::from_bytes_mut(boot_param_buffer)?;
+
+    // Sets commandline.
+    cmdline_buffer.clone_from_slice(cmdline);
+    bootparam_fixup.setup_header_mut().cmd_line_ptr = cmdline_start.try_into().unwrap();
+    bootparam_fixup.setup_header_mut().cmdline_size = cmdline.len().try_into().unwrap();
+
+    // Sets ramdisk address.
+    bootparam_fixup.setup_header_mut().ramdisk_image =
+        (ramdisk.as_ptr() as usize).try_into().map_err(|_| BootError::IntegerOverflow)?;
+    bootparam_fixup.setup_header_mut().ramdisk_size =
+        ramdisk.len().try_into().map_err(|_| BootError::IntegerOverflow)?;
+
+    // Sets to loader type "special loader". (Anything other than 0, otherwise linux kernel ignores
+    // ramdisk.)
+    bootparam_fixup.setup_header_mut().type_of_loader = 0xff;
+
+    // Fix up e820 memory map.
+    let num_entries = mmap_cb(bootparam_fixup.e820_map())?;
+    bootparam_fixup.0.e820_entries = num_entries;
+
+    // Clears stack pointers, interrupt and jumps to protected mode kernel.
+    #[cfg(target_arch = "x86_64")]
+    unsafe {
+        asm!(
+            "xor ebp, ebp",
+            "xor esp, esp",
+            "cld",
+            "cli",
+            "jmp {ep}",
+            ep = in(reg) LOAD_ADDR_HIGH,
+            in("rsi") low_mem_addr,
+        );
+    }
+    #[cfg(target_arch = "x86")]
+    unsafe {
+        asm!(
+            "xor ebp, ebp",
+            "xor esp, esp",
+            "mov esi, eax",
+            "cld",
+            "cli",
+            "jmp {ep}",
+            ep = in(reg) LOAD_ADDR_HIGH,
+            in("eax") low_mem_addr,
+        );
+    }
+
+    Ok(())
+}
diff --git a/gbl/libefi/BUILD b/gbl/libefi/BUILD
index aa134d7..7507ddc 100644
--- a/gbl/libefi/BUILD
+++ b/gbl/libefi/BUILD
@@ -28,6 +28,7 @@ cc_library(
         "defs/protocols/block_io_protocol.h",
         "defs/protocols/device_path_protocol.h",
         "defs/protocols/loaded_image_protocol.h",
+        "defs/protocols/riscv_efi_boot_protocol.h",
         "defs/protocols/simple_text_output_protocol.h",
         "defs/system_table.h",
         "defs/types.h",
diff --git a/gbl/libefi/defs/efi.h b/gbl/libefi/defs/efi.h
index 0fcaed5..2162768 100644
--- a/gbl/libefi/defs/efi.h
+++ b/gbl/libefi/defs/efi.h
@@ -29,6 +29,7 @@
 #include "protocols/block_io_protocol.h"
 #include "protocols/device_path_protocol.h"
 #include "protocols/loaded_image_protocol.h"
+#include "protocols/riscv_efi_boot_protocol.h"
 #include "protocols/simple_text_output_protocol.h"
 
 #endif  // __EFI_H__
diff --git a/gbl/libefi/defs/protocols/riscv_efi_boot_protocol.h b/gbl/libefi/defs/protocols/riscv_efi_boot_protocol.h
new file mode 100644
index 0000000..528844b
--- /dev/null
+++ b/gbl/libefi/defs/protocols/riscv_efi_boot_protocol.h
@@ -0,0 +1,32 @@
+/*
+ * Copyright (C) 2023 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.
+ *
+ */
+
+#ifndef __RISCV_EFI_BOOT_PROTOCOL_H__
+#define __RISCV_EFI_BOOT_PROTOCOL_H__
+
+#include <stddef.h>
+
+#include "types.h"
+
+// Source: https://github.com/riscv-non-isa/riscv-uefi
+struct EfiRiscvBootProtocol {
+  uint64_t revision;
+
+  EfiStatus (*get_boot_hartid)(struct EfiRiscvBootProtocol* self, size_t* boot_hartid);
+};
+
+#endif  // __RISCV_EFI_BOOT_PROTOCOL_H__
diff --git a/gbl/libefi/src/lib.rs b/gbl/libefi/src/lib.rs
index dabb1c7..c337e5d 100644
--- a/gbl/libefi/src/lib.rs
+++ b/gbl/libefi/src/lib.rs
@@ -73,6 +73,7 @@ pub use protocol::DevicePathToTextProtocol;
 pub use protocol::LoadedImageProtocol;
 pub use protocol::Protocol;
 pub use protocol::ProtocolInfo;
+pub use protocol::RiscvBootProtocol;
 pub use protocol::SimpleTextOutputProtocol;
 
 mod error {
@@ -199,11 +200,17 @@ impl<'a> SystemTable<'a> {
 
     /// Gets the `EFI_SYSTEM_TABLE.ConOut` field.
     pub fn con_out(&self) -> EfiResult<Protocol<'a, SimpleTextOutputProtocol>> {
-        Ok(Protocol::<SimpleTextOutputProtocol>::new(
-            DeviceHandle(null_mut()), // No device handle. This protocol is a permanent reference.
-            self.table.con_out,
-            self.efi_entry,
-        ))
+        // SAFETY: `EFI_SYSTEM_TABLE.ConOut` is a pointer to EfiSimpleTextOutputProtocol structure
+        // by definition. It lives until ExitBootService and thus as long as `self.efi_entry` or,
+        // 'a
+        Ok(unsafe {
+            Protocol::<SimpleTextOutputProtocol>::new(
+                // No device handle. This protocol is a permanent reference.
+                DeviceHandle(null_mut()),
+                self.table.con_out,
+                self.efi_entry,
+            )
+        })
     }
 
     /// Gets the `EFI_SYSTEM_TABLE.ConfigurationTable` array.
@@ -268,8 +275,10 @@ impl<'a> BootServices<'a> {
                 EFI_OPEN_PROTOCOL_ATTRIBUTE_BY_HANDLE_PROTOCOL
             )?;
         }
-
-        Ok(Protocol::<T>::new(handle, out_handle as *mut _, self.efi_entry))
+        // SAFETY: `EFI_SYSTEM_TABLE.OpenProtocol` returns a valid pointer to `T::InterfaceType`
+        // on success. The pointer remains valid until closed by
+        // `EFI_BOOT_SERVICES.CloseProtocol()` when Protocol goes out of scope.
+        Ok(unsafe { Protocol::<T>::new(handle, out_handle as *mut _, self.efi_entry) })
     }
 
     /// Wrapper of `EFI_BOOT_SERVICES.CloseProtocol()`.
diff --git a/gbl/libefi/src/protocol.rs b/gbl/libefi/src/protocol.rs
index 327ccf8..b24de7b 100644
--- a/gbl/libefi/src/protocol.rs
+++ b/gbl/libefi/src/protocol.rs
@@ -40,7 +40,15 @@ pub struct Protocol<'a, T: ProtocolInfo> {
 /// Protocol<T> will have additional implementation based on type `T`.
 impl<'a, T: ProtocolInfo> Protocol<'a, T> {
     /// Create a new instance with the given device handle, interface pointer and `EfiEntry` data.
-    pub(crate) fn new(
+    ///
+    /// # Safety
+    ///
+    /// Caller needs to ensure that
+    ///
+    /// * `interface` points to a valid object of type T::InterfaceType.
+    ///
+    /// * Object pointed to by `interface` must live as long as the create `Protocol` or 'a.
+    pub(crate) unsafe fn new(
         device: DeviceHandle,
         interface: *mut T::InterfaceType,
         efi_entry: &'a EfiEntry,
@@ -108,7 +116,11 @@ impl ProtocolInfo for BlockIoProtocol {
 impl Protocol<'_, BlockIoProtocol> {
     /// Wrapper of `EFI_BLOCK_IO_PROTOCOL.read_blocks()`
     pub fn read_blocks(&self, lba: u64, buffer: &mut [u8]) -> EfiResult<()> {
-        // SAFETY: EFI protocol method call.
+        // SAFETY:
+        // `self.interface()?` guarantees self.interface is non-null and points to a valid object
+        // established by `Protocol::new()`.
+        // `self.interface` is input parameter and will not be retained. It outlives the call.
+        // `buffer` remains valid during the call.
         unsafe {
             efi_call!(
                 self.interface()?.read_blocks,
@@ -123,7 +135,11 @@ impl Protocol<'_, BlockIoProtocol> {
 
     /// Wrapper of `EFI_BLOCK_IO_PROTOCOL.write_blocks()`
     pub fn write_blocks(&self, lba: u64, buffer: &[u8]) -> EfiResult<()> {
-        // SAFETY: EFI protocol method call.
+        // SAFETY:
+        // `self.interface()?` guarantees self.interface is non-null and points to a valid object
+        // established by `Protocol::new()`.
+        // `self.interface` is input parameter and will not be retained. It outlives the call.
+        // `buffer` remains valid during the call.
         unsafe {
             efi_call!(
                 self.interface()?.write_blocks,
@@ -138,13 +154,19 @@ impl Protocol<'_, BlockIoProtocol> {
 
     /// Wrapper of `EFI_BLOCK_IO_PROTOCOL.flush_blocks()`
     pub fn flush_blocks(&self) -> EfiResult<()> {
-        // SAFETY: EFI protocol method call.
+        // SAFETY:
+        // `self.interface()?` guarantees `self.interface` is non-null and points to a valid object
+        // established by `Protocol::new()`.
+        // `self.interface` is input parameter and will not be retained. It outlives the call.
         unsafe { efi_call!(self.interface()?.flush_blocks, self.interface) }
     }
 
     /// Wrapper of `EFI_BLOCK_IO_PROTOCOL.reset()`
     pub fn reset(&self, extended_verification: bool) -> EfiResult<()> {
-        // SAFETY: EFI protocol method call.
+        // SAFETY:
+        // `self.interface()?` guarantees `self.interface` is non-null and points to a valid object
+        // established by `Protocol::new()`.
+        // `self.interface` is input parameter and will not be retained. It outlives the call.
         unsafe { efi_call!(self.interface()?.reset, self.interface, extended_verification) }
     }
 
@@ -169,7 +191,10 @@ impl ProtocolInfo for SimpleTextOutputProtocol {
 impl Protocol<'_, SimpleTextOutputProtocol> {
     /// Wrapper of `EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.OutputString()`
     pub fn output_string(&self, msg: *mut char16_t) -> EfiResult<()> {
-        // SAFETY: EFI protocol method call.
+        // SAFETY:
+        // `self.interface()?` guarantees `self.interface` is non-null and points to a valid object
+        // established by `Protocol::new()`.
+        // `self.interface` is input parameter and will not be retained. It outlives the call.
         unsafe { efi_call!(self.interface()?.output_string, self.interface, msg) }
     }
 }
@@ -234,7 +259,10 @@ impl<'a> Protocol<'a, DevicePathToTextProtocol> {
             .convert_device_path_to_text
             .as_ref()
             .ok_or_else::<EfiError, _>(|| EFI_STATUS_NOT_FOUND.into())?;
-        // SAFETY: EFI protocol method call.
+        // SAFETY:
+        // `self.interface()?` guarantees `self.interface` is non-null and points to a valid object
+        // established by `Protocol::new()`.
+        // `self.interface` is input parameter and will not be retained. It outlives the call.
         let res = unsafe { f(device_path.interface_ptr(), display_only, allow_shortcuts) };
         Ok(DevicePathText::new(res, self.efi_entry))
     }
@@ -313,6 +341,35 @@ impl<'a> Protocol<'a, LoadedImageProtocol> {
     }
 }
 
+/// RISCV_EFI_BOOT_PROTOCOL
+pub struct RiscvBootProtocol;
+
+impl ProtocolInfo for RiscvBootProtocol {
+    type InterfaceType = EfiRiscvBootProtocol;
+
+    const GUID: EfiGuid =
+        EfiGuid::new(0xccd15fec, 0x6f73, 0x4eec, [0x83, 0x95, 0x3e, 0x69, 0xe4, 0xb9, 0x40, 0xbf]);
+}
+
+impl<'a> Protocol<'a, RiscvBootProtocol> {
+    pub fn get_boot_hartid(&self) -> EfiResult<usize> {
+        let mut boot_hart_id: usize = 0;
+        // SAFETY:
+        // `self.interface()?` guarantees `self.interface` is non-null and points to a valid object
+        // established by `Protocol::new()`.
+        // `self.interface` is input parameter and will not be retained. It outlives the call.
+        // `&mut boot_hart_id` is output parameter and will not be retained. It outlives the call.
+        unsafe {
+            efi_call!(self.interface()?.get_boot_hartid, self.interface, &mut boot_hart_id)?;
+        }
+        Ok(boot_hart_id)
+    }
+
+    pub fn revision(&self) -> EfiResult<u64> {
+        Ok(self.interface()?.revision)
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::*;
@@ -322,8 +379,14 @@ mod test {
     fn test_dont_close_protocol_without_device_handle() {
         run_test(|image_handle, systab_ptr| {
             let efi_entry = EfiEntry { image_handle, systab_ptr };
-            {
-                Protocol::<BlockIoProtocol>::new(DeviceHandle(null_mut()), 2 as *mut _, &efi_entry);
+            let mut block_io: EfiBlockIoProtocol = Default::default();
+            // SAFETY: `block_io` is a EfiBlockIoProtocol and out lives the created Protocol.
+            unsafe {
+                Protocol::<BlockIoProtocol>::new(
+                    DeviceHandle(null_mut()),
+                    &mut block_io as *mut _,
+                    &efi_entry,
+                );
             }
             efi_call_traces().with(|traces| {
                 assert_eq!(traces.borrow_mut().close_protocol_trace.inputs.len(), 0);