Improve UEFI sys documentation

Add more comments to help anyone interested in checking out the internal
implementation.

Signed-off-by: Ayush <ayushsingh1325@gmail.com>

Author: Ayush1325

library/std/src/os/uefi/env.rs

@@ -49,6 +49,8 @@ pub fn get_runtime_services() -> Option<NonNull<RuntimeServices>> {
 }
 
 #[unstable(feature = "uefi_std", issue = "none")]
+/// Open Protocol on a handle
+/// Implemented using `EFI_BOOT_SERVICES.OpenProtocol()`
 pub fn open_protocol<T>(
     handle: NonNull<c_void>,
     mut protocol_guid: Guid,
@@ -79,6 +81,8 @@ pub fn open_protocol<T>(
 }
 
 #[unstable(feature = "uefi_std", issue = "none")]
+// Locate handles with a particula protocol Guid
+/// Implemented using `EFI_BOOT_SERVICES.LocateHandles()`
 pub fn locate_handles(mut guid: Guid) -> io::Result<Vec<NonNull<c_void>>> {
     fn inner(
         guid: &mut Guid,

library/std/src/os/uefi/ffi.rs

@@ -11,7 +11,14 @@ pub trait OsStrExt: Sealed {
     /// Note: The returned string is NULL terminated.
     /// Note: The supplied should not contain NULL.
     fn to_ffi_string(&self) -> Vec<u16> {
-        let mut v: Vec<u16> = self.to_ucs2().map(u16::from).collect();
+        let mut v: Vec<u16> = self
+            .to_ucs2()
+            .map(|x| match x {
+                Ok(c) => c,
+                Err(_) => ucs2::Ucs2Char::REPLACEMENT_CHARACTER,
+            })
+            .map(u16::from)
+            .collect();
         v.push(0);
         v.shrink_to_fit();
         v

library/std/src/sys/uefi/alloc.rs

@@ -1,7 +1,13 @@
+//! Global Allocator for UEFI.
+//! Uses `EFI_BOOT_SERVICES.AllocatePool()` and `EFI_BOOT_SERVICES.FreePool()`.
+//! Takes a lot of inspiration from Windows allocator for Alignment > 8.
+
 use crate::alloc::{GlobalAlloc, Layout, System};
 use crate::os::uefi;
 
 const POOL_ALIGNMENT: usize = 8;
+// FIXME: Maybe allow chaing the MEMORY_TYPE. However, since allocation is done even before main,
+// there will be a few allocations with the default MEMORY_TYPE.
 const MEMORY_TYPE: u32 = r_efi::efi::LOADER_DATA;
 
 #[stable(feature = "alloc_system_type", since = "1.28.0")]
@@ -80,7 +86,7 @@ unsafe fn align_ptr(ptr: *mut u8, align: usize) -> *mut u8 {
         // SAFETY: Because the size and alignment of a header is <= `MIN_ALIGN` and `aligned`
         // is aligned to at least `MIN_ALIGN` and has at least `MIN_ALIGN` bytes of padding before
         // it, it is safe to write a header directly before it.
-        unsafe { core::ptr::write((aligned as *mut Header).offset(-1), Header(ptr)) };
+        unsafe { crate::ptr::write((aligned as *mut Header).offset(-1), Header(ptr)) };
 
         aligned
     } else {
@@ -93,7 +99,7 @@ unsafe fn unalign_ptr(ptr: *mut u8, align: usize) -> *mut u8 {
     if align > POOL_ALIGNMENT {
         // SAFETY: Because of the contract of `System`, `ptr` is guaranteed to be non-null
         // and have a header readable directly before it.
-        unsafe { core::ptr::read((ptr as *mut Header).offset(-1)).0 }
+        unsafe { crate::ptr::read((ptr as *mut Header).offset(-1)).0 }
     } else {
         ptr
     }

library/std/src/sys/uefi/args.rs

@@ -1,3 +1,5 @@
+//! Args related functionality for UEFI. Takes a lot of inspiration of Windows args
+
 use super::common;
 use crate::ffi::OsString;
 use crate::fmt;
@@ -11,6 +13,8 @@ pub struct Args {
     parsed_args_list: vec::IntoIter<OsString>,
 }
 
+// Get the Supplied arguments for loaded image.
+// Uses EFI_LOADED_IMAGE_PROTOCOL
 pub fn args() -> Args {
     use r_efi::efi::protocols::loaded_image;
 

library/std/src/sys/uefi/fs.rs

@@ -1,4 +1,4 @@
-//! Implemented using File Protocol
+//! File System functionality for UEFI
 
 use crate::ffi::{OsStr, OsString};
 use crate::fmt;
@@ -9,7 +9,6 @@ use crate::sys::time::{SystemTime, UNIX_EPOCH};
 use crate::sys::unsupported;
 use r_efi::protocols::file;
 
-// FIXME: Do not store FileProtocol Instead store Handle
 pub struct File {
     ptr: uefi_fs::FileProtocol,
     path: PathBuf,
@@ -149,6 +148,7 @@ impl Iterator for ReadDir {
     fn next(&mut self) -> Option<io::Result<DirEntry>> {
         let dir_entry = self.inner.read_dir_entry(self.path.as_path());
         if let Some(Ok(ref x)) = dir_entry {
+            // Ignore `.` and `..`
             if x.file_name().as_os_str() == OsStr::new(".")
                 || x.file_name().as_os_str() == OsStr::new("..")
             {
@@ -382,6 +382,7 @@ pub fn readdir(p: &Path) -> io::Result<ReadDir> {
     Ok(ReadDir { inner, path: abs_path })
 }
 
+// Just Delete the file since symlinks are not supported
 pub fn unlink(p: &Path) -> io::Result<()> {
     let open_mode = file::MODE_READ | file::MODE_WRITE;
     let attr = 0;
@@ -431,14 +432,17 @@ pub fn try_exists(path: &Path) -> io::Result<bool> {
     }
 }
 
+// Symlink not supported
 pub fn readlink(_p: &Path) -> io::Result<PathBuf> {
     unsupported()
 }
 
+// Symlink not supported
 pub fn symlink(_original: &Path, _link: &Path) -> io::Result<()> {
     unsupported()
 }
 
+// Symlink not supported
 pub fn link(_src: &Path, _dst: &Path) -> io::Result<()> {
     unsupported()
 }
@@ -459,7 +463,7 @@ pub fn lstat(p: &Path) -> io::Result<FileAttr> {
     stat(p)
 }
 
-// Not sure how to implement. Tryied doing a round conversion from EFI_DEVICE_PATH protocol but
+// Not sure how to implement. Tried doing a round conversion from EFI_DEVICE_PATH protocol but
 // that doesn't work either.
 pub fn canonicalize(_p: &Path) -> io::Result<PathBuf> {
     unsupported()
@@ -533,6 +537,9 @@ mod uefi_fs {
             FileProtocol { inner }
         }
 
+        // Can open any file as long as it is possible to convert path to EFI_DEVICE_PATH_PROTOCOL
+        // using `EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL.ConvertTextToDevicePath()`.
+        // If relative path is provided, then opens the file in the EFI_LOADED_IMAGE_DEVICE_PATH
         pub fn from_path(path: &Path, open_mode: u64, attr: u64) -> io::Result<Self> {
             match path.prefix() {
                 None => {
@@ -559,6 +566,7 @@ mod uefi_fs {
             }
         }
 
+        // Open the volume on the device_handle the image was loaded from.
         fn get_rootfs() -> io::Result<FileProtocol> {
             use r_efi::protocols::loaded_image;
 
@@ -581,6 +589,7 @@ mod uefi_fs {
             Self::get_volume(device_handle)
         }
 
+        // Open a volume having a particular prefix.
         fn get_volume_from_prefix(prefix: &OsStr) -> io::Result<Self> {
             use r_efi::protocols::{device_path, simple_file_system};
 
@@ -608,6 +617,7 @@ mod uefi_fs {
             Err(io::error::const_io_error!(io::ErrorKind::NotFound, "Volume Not Found"))
         }
 
+        // Open volume on device_handle using SIMPLE_FILE_SYSTEM_PROTOCOL
         fn get_volume(device_handle: NonNull<crate::ffi::c_void>) -> io::Result<Self> {
             use r_efi::protocols::simple_file_system;
 
@@ -636,6 +646,7 @@ mod uefi_fs {
             }
         }
 
+        // Open a file from current EFI_FILE_PATH_PROTOCOL
         pub fn open(&self, path: &Path, open_mode: u64, attr: u64) -> io::Result<FileProtocol> {
             let mut file_opened: MaybeUninit<*mut file::Protocol> = MaybeUninit::uninit();
             unsafe {
@@ -684,6 +695,7 @@ mod uefi_fs {
             unsafe { Self::flush_raw(self.inner.as_ptr()) }
         }
 
+        // Read a Directory.
         pub fn read_dir_entry(&self, base_path: &Path) -> Option<io::Result<DirEntry>> {
             let mut buf_size = 0usize;
             if let Err(e) = unsafe {
@@ -718,6 +730,7 @@ mod uefi_fs {
             Some(Ok(DirEntry { attr, name, path }))
         }
 
+        // Get current file info
         pub fn get_file_info(&self) -> io::Result<uefi::raw::VariableSizeType<file::Info>> {
             let mut buf_size = 0usize;
             match unsafe {
@@ -749,6 +762,7 @@ mod uefi_fs {
             }
         }
 
+        // Set file size. Useful for truncation
         pub fn set_file_size(&self, file_size: u64) -> io::Result<()> {
             use r_efi::efi::Time;
 
@@ -771,6 +785,7 @@ mod uefi_fs {
             }
         }
 
+        // Set file attributes
         pub fn set_file_attr(&self, attribute: u64) -> io::Result<()> {
             use r_efi::efi::Time;
 
@@ -793,6 +808,8 @@ mod uefi_fs {
             }
         }
 
+        // Change file name. It seems possible to provide a relative path as file name. Thus it
+        // also acts as move
         pub fn set_file_name(&self, file_name: &OsStr) -> io::Result<()> {
             use r_efi::efi::Time;
 
@@ -825,7 +842,10 @@ mod uefi_fs {
             }
         }
 
+        // Delete a file.
         pub fn delete(self) -> io::Result<()> {
+            // Deleting the file makes the pointer invalid. Thus calling drop on it later will
+            // cause UB
             let file = crate::mem::ManuallyDrop::new(self);
             unsafe { Self::delete_raw(file.inner.as_ptr()) }
         }

library/std/src/sys/uefi/mod.rs

@@ -77,6 +77,7 @@ pub fn decode_error_kind(code: i32) -> crate::io::ErrorKind {
 }
 
 pub fn abort_internal() -> ! {
+    // First try to use EFI_BOOT_SERVICES.Exit()
     if let (Some(boot_services), Some(handle)) =
         (uefi::env::get_boot_services(), uefi::env::get_system_handle())
     {

library/std/src/sys/uefi/os.rs

@@ -9,6 +9,7 @@ use crate::os::uefi::ffi::OsStrExt;
 use crate::os::uefi::io::status_to_io_error;
 use crate::path::{self, PathBuf};
 
+// Return EFI_ABORTED as Status
 pub fn errno() -> i32 {
     r_efi::efi::Status::ABORTED.as_usize() as i32
 }
@@ -18,6 +19,7 @@ pub fn error_string(errno: i32) -> String {
     status_to_io_error(r).to_string()
 }
 
+// Implemented using EFI_LOADED_IMAGE_DEVICE_PATH_PROTOCOL
 pub fn getcwd() -> io::Result<PathBuf> {
     let mut p = current_exe()?;
     p.pop();
@@ -65,6 +67,7 @@ impl StdError for JoinPathsError {
     }
 }
 
+// Implemented using EFI_LOADED_IMAGE_DEVICE_PATH_PROTOCOL
 pub fn current_exe() -> io::Result<PathBuf> {
     use r_efi::efi::protocols::{device_path, loaded_image_device_path};
 
@@ -102,7 +105,7 @@ pub fn setenv(key: &OsStr, val: &OsStr) -> io::Result<()> {
         unsetenv(key)
     } else {
         unsafe {
-            uefi_vars::set_variable_raw(
+            uefi_vars::set_variable(
                 key.to_ffi_string().as_mut_ptr(),
                 val.len(),
                 val.bytes().as_ptr() as *mut crate::ffi::c_void,
@@ -113,7 +116,7 @@ pub fn setenv(key: &OsStr, val: &OsStr) -> io::Result<()> {
 
 pub fn unsetenv(key: &OsStr) -> io::Result<()> {
     match unsafe {
-        uefi_vars::set_variable_raw(key.to_ffi_string().as_mut_ptr(), 0, crate::ptr::null_mut())
+        uefi_vars::set_variable(key.to_ffi_string().as_mut_ptr(), 0, crate::ptr::null_mut())
     } {
         Ok(_) => Ok(()),
         Err(e) => match e.kind() {
@@ -138,20 +141,23 @@ pub fn exit(code: i32) -> ! {
         Err(_) => r_efi::efi::Status::ABORTED,
     };
 
+    // First try to use EFI_BOOT_SERVICES.Exit()
     if let (Some(boot_services), Some(handle)) =
         (uefi::env::get_boot_services(), uefi::env::get_system_handle())
     {
         let _ =
             unsafe { ((*boot_services.as_ptr()).exit)(handle.as_ptr(), code, 0, [0].as_mut_ptr()) };
     }
 
+    // If exit is not possible, the call abort
     crate::intrinsics::abort()
 }
 
 pub fn getpid() -> u32 {
     panic!("no pids on this platform")
 }
 
+// Implement variables using Variable Services in EFI_RUNTIME_SERVICES
 pub(crate) mod uefi_vars {
     // It is possible to directly store and use UTF-8 data. So no need to convert to and from UCS-2
     use super::super::common;
@@ -171,7 +177,7 @@ pub(crate) mod uefi_vars {
         &[0x7a, 0x31, 0x92, 0xc0, 0x79, 0xd2],
     );
 
-    pub(crate) unsafe fn set_variable_raw(
+    pub(crate) unsafe fn set_variable(
         variable_name: *mut u16,
         data_size: usize,
         data: *mut crate::ffi::c_void,
@@ -192,40 +198,42 @@ pub(crate) mod uefi_vars {
     }
 
     pub(crate) fn get_variable(key: &OsStr) -> Option<OsString> {
-        let runtime_services = uefi::env::get_runtime_services()?;
         let mut buf_size = 0;
-        let mut guid = SHELL_VARIABLE_GUID;
-        let r = unsafe {
-            ((*runtime_services.as_ptr()).get_variable)(
-                key.to_ffi_string().as_mut_ptr(),
-                &mut guid,
-                crate::ptr::null_mut(),
-                &mut buf_size,
-                crate::ptr::null_mut(),
-            )
-        };
-
-        if r.is_error() && r != r_efi::efi::Status::BUFFER_TOO_SMALL {
-            return None;
+        let mut key_buf = key.to_ffi_string();
+
+        if let Err(e) =
+            unsafe { get_vaiable_raw(key_buf.as_mut_ptr(), &mut buf_size, crate::ptr::null_mut()) }
+        {
+            if e.kind() != io::ErrorKind::FileTooLarge {
+                return None;
+            }
         }
 
         let mut buf: Vec<u8> = Vec::with_capacity(buf_size);
+        unsafe { get_vaiable_raw(key_buf.as_mut_ptr(), &mut buf_size, buf.as_mut_ptr().cast()) }
+            .ok()?;
+
+        unsafe { buf.set_len(buf_size) };
+        Some(OsString::from(String::from_utf8(buf).ok()?))
+    }
+
+    unsafe fn get_vaiable_raw(
+        key: *mut u16,
+        data_size: &mut usize,
+        data: *mut crate::ffi::c_void,
+    ) -> io::Result<()> {
+        let runtime_services =
+            uefi::env::get_runtime_services().ok_or(common::RUNTIME_SERVICES_ERROR)?;
         let mut guid = SHELL_VARIABLE_GUID;
         let r = unsafe {
             ((*runtime_services.as_ptr()).get_variable)(
-                key.to_ffi_string().as_mut_ptr(),
+                key,
                 &mut guid,
                 crate::ptr::null_mut(),
-                &mut buf_size,
-                buf.as_mut_ptr().cast(),
+                data_size,
+                data,
             )
         };
-
-        if r.is_error() {
-            None
-        } else {
-            unsafe { buf.set_len(buf_size) };
-            Some(OsString::from(String::from_utf8(buf).ok()?))
-        }
+        if r.is_error() { Err(status_to_io_error(r)) } else { Ok(()) }
     }
 }

library/std/src/sys/uefi/pipe.rs

@@ -1,3 +1,5 @@
+//! An implementation of Pipes using UEFI variables
+
 use crate::os::uefi::ffi::OsStrExt;
 use crate::{
     ffi::OsStr,