build: Remove heap allocator

PR #92 introduced heap allocator to implement EFI variable feature. However, none of other components uses heap. This commit removes the heap allocator and use the `heapless` crate instead of `Vec`. This change simplifies the build arguments. Signed-off-by: Akira Moroo <[email protected]>
cloud-hypervisor · May 15, 2024 · a46a644 · a46a644
1 parent 6018ec4
commit a46a644
Show file tree

Hide file tree

Showing 12 changed files with 73 additions and 84 deletions.
diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
@@ -28,11 +28,11 @@ jobs:
       - name: Install Rust components
         run: rustup component add rust-src clippy rustfmt
       - name: Build (debug)
-        run: cargo build --target ${{ matrix.target }} -Zbuild-std=core,alloc -Zbuild-std-features=compiler-builtins-mem
+        run: cargo build --target ${{ matrix.target }} -Zbuild-std=core -Zbuild-std-features=compiler-builtins-mem
       - name: Build (release)
-        run: cargo build --release --target ${{ matrix.target }} -Zbuild-std=core,alloc -Zbuild-std-features=compiler-builtins-mem
+        run: cargo build --release --target ${{ matrix.target }} -Zbuild-std=core -Zbuild-std-features=compiler-builtins-mem
       - name: Clippy (default)
-        run: cargo clippy --target ${{ matrix.target }} -Zbuild-std=core,alloc
+        run: cargo clippy --target ${{ matrix.target }} -Zbuild-std=core
       - name: Clippy (all targets, all features)
         run: cargo clippy --all-targets --all-features
       - name: Formatting

diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml
@@ -19,9 +19,9 @@ jobs:
       - name: Install rust-src
         run: rustup component add rust-src
       - name: Build (release) for x86_64
-        run: cargo build --release --target x86_64-unknown-none.json -Zbuild-std=core,alloc -Zbuild-std-features=compiler-builtins-mem
+        run: cargo build --release --target x86_64-unknown-none.json -Zbuild-std=core -Zbuild-std-features=compiler-builtins-mem
       - name: Build (release) for aarch64
-        run: cargo build --release --target aarch64-unknown-none.json -Zbuild-std=core,alloc -Zbuild-std-features=compiler-builtins-mem
+        run: cargo build --release --target aarch64-unknown-none.json -Zbuild-std=core -Zbuild-std-features=compiler-builtins-mem
       - name: Create release
         id: create_release
         uses: actions/create-release@v1

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -29,7 +29,7 @@ efi-var = []
 bitflags = "2.5.0"
 atomic_refcell = "0.1.13"
 r-efi = { version = "4.4.0", features = ["efiapi"] }
-linked_list_allocator = "0.10.4"
+heapless = "0.8.0"
 
 [target.'cfg(target_arch = "aarch64")'.dependencies]
 tock-registers = "0.9.0"

diff --git a/README.md b/README.md
@@ -41,7 +41,7 @@ all the way into the OS.
 To compile:
 
 ```
-cargo build --release --target x86_64-unknown-none.json -Zbuild-std=core,alloc -Zbuild-std-features=compiler-builtins-mem
+cargo build --release --target x86_64-unknown-none.json -Zbuild-std=core -Zbuild-std-features=compiler-builtins-mem
 ```
 
 The result will be in:
@@ -101,7 +101,7 @@ $ qemu-system-x86_64 -machine q35,accel=kvm -cpu host,-vmx -m 1G\
 To compile:
 
 ```
-cargo build --release --target aarch64-unknown-none.json -Zbuild-std=core,alloc -Zbuild-std-features=compiler-builtins-mem
+cargo build --release --target aarch64-unknown-none.json -Zbuild-std=core -Zbuild-std-features=compiler-builtins-mem
 ```
 
 The result will be in:
@@ -121,7 +121,7 @@ will become available in the future.
 To compile:
 
 ```
-cargo build --release --target riscv64gcv-unknown-none-elf.json -Zbuild-std=core,alloc -Zbuild-std-features=compiler-builtins-mem
+cargo build --release --target riscv64gcv-unknown-none-elf.json -Zbuild-std=core -Zbuild-std-features=compiler-builtins-mem
 ```
 
 The result will be in:

diff --git a/scripts/dev_cli.sh b/scripts/dev_cli.sh
@@ -274,7 +274,7 @@ cmd_build() {
 	   --env RUSTFLAGS="$rustflags" \
 	   "$CTR_IMAGE" \
 	   cargo build --target "$target" \
-	         -Zbuild-std=core,alloc \
+	         -Zbuild-std=core \
 	         -Zbuild-std-features=compiler-builtins-mem \
 	         --target-dir "$CTR_RHF_CARGO_TARGET" \
 	         "${cargo_args[@]}" && say "Binary placed under $RHF_CARGO_TARGET/target/$build"

diff --git a/scripts/run_cargo_tests.sh b/scripts/run_cargo_tests.sh
@@ -9,7 +9,7 @@ export RUSTFLAGS="-D warnings"
 arch="$(uname -m)"
 
 do_cargo_tests() {
-    local cargo_args=("-Zbuild-std=core,alloc" "-Zbuild-std-features=compiler-builtins-mem")
+    local cargo_args=("-Zbuild-std=core" "-Zbuild-std-features=compiler-builtins-mem")
     local cmd="$1"
     local target="$2"
     local features="$3"

diff --git a/scripts/run_coreboot_integration_tests.sh b/scripts/run_coreboot_integration_tests.sh
@@ -16,7 +16,7 @@ fetch_disk_images "$WORKLOADS_DIR" "$arch"
 [ "$arch" = "x86_64" ] && target="x86_64-unknown-none"
 
 rustup component add rust-src
-cargo build --release --target "$target.json" -Zbuild-std=core,alloc -Zbuild-std-features=compiler-builtins-mem --features "coreboot"
+cargo build --release --target "$target.json" -Zbuild-std=core -Zbuild-std-features=compiler-builtins-mem --features "coreboot"
 
 RHF_BIN="$RHF_ROOT_DIR/target/$target/release/hypervisor-fw"
 COREBOOT_CONFIG_IN="$RHF_ROOT_DIR/resources/coreboot/qemu-q35-config.in"

diff --git a/scripts/run_integration_tests.sh b/scripts/run_integration_tests.sh
@@ -18,7 +18,7 @@ fetch_disk_images "$WORKLOADS_DIR" "$arch"
 [ "$arch" = "x86_64" ] && target="x86_64-unknown-none"
 
 rustup component add rust-src
-cargo build --release --target "$target.json" -Zbuild-std=core,alloc -Zbuild-std-features=compiler-builtins-mem
+cargo build --release --target "$target.json" -Zbuild-std=core -Zbuild-std-features=compiler-builtins-mem
 
 export RUST_BACKTRACE=1
 time cargo test --features "integration_tests" "integration::tests::linux::$arch" -- --test-threads=1
diff --git a/scripts/run_integration_tests_windows.sh b/scripts/run_integration_tests_windows.sh
@@ -31,7 +31,7 @@ dmsetup mknodes
 [ "$arch" = "x86_64" ] && target="x86_64-unknown-none"
 
 rustup component add rust-src
-cargo build --release --target "$target.json" -Zbuild-std=core,alloc -Zbuild-std-features=compiler-builtins-mem
+cargo build --release --target "$target.json" -Zbuild-std=core -Zbuild-std-features=compiler-builtins-mem
 
 export RUST_BACKTRACE=1
 time cargo test --features "integration_tests" "integration::tests::windows::$arch"

diff --git a/src/efi/mod.rs b/src/efi/mod.rs
@@ -1,9 +1,6 @@
 // SPDX-License-Identifier: Apache-2.0
 // Copyright © 2019 Intel Corporation
 
-#[cfg(all(not(test), not(feature = "integration_tests")))]
-use core::alloc as heap_alloc;
-
 use core::{
     ffi::c_void,
     mem::{size_of, transmute},
@@ -12,7 +9,6 @@ use core::{
 };
 
 use atomic_refcell::AtomicRefCell;
-use linked_list_allocator::LockedHeap;
 use r_efi::{
     efi::{
         self, AllocateType, Boolean, CapsuleHeader, Char16, Event, EventNotify, Guid, Handle,
@@ -64,16 +60,6 @@ struct HandleWrapper {
 
 pub static ALLOCATOR: AtomicRefCell<Allocator> = AtomicRefCell::new(Allocator::new());
 
-#[cfg(not(test))]
-#[global_allocator]
-pub static HEAP_ALLOCATOR: LockedHeap = LockedHeap::empty();
-
-#[cfg(all(not(test), not(feature = "integration_tests")))]
-#[alloc_error_handler]
-fn heap_alloc_error_handler(layout: heap_alloc::Layout) -> ! {
-    panic!("heap allocation error: {:?}", layout);
-}
-
 pub static VARIABLES: AtomicRefCell<VariableAllocator> =
     AtomicRefCell::new(VariableAllocator::new());
 
@@ -1078,7 +1064,6 @@ impl DevicePath {
 }
 
 const PAGE_SIZE: u64 = 4096;
-const HEAP_SIZE: usize = 2 << 20; /* 2MiB */
 
 // Populate allocator from E820, fixed ranges for the firmware and the loaded binary.
 fn populate_allocator(info: &dyn bootinfo::Info, image_address: u64, image_size: u64) {
@@ -1128,28 +1113,8 @@ fn populate_allocator(info: &dyn bootinfo::Info, image_address: u64, image_size:
         image_size / PAGE_SIZE,
         image_address,
     );
-
-    // Initialize heap allocator
-    init_heap_allocator(HEAP_SIZE);
 }
 
-#[cfg(not(test))]
-fn init_heap_allocator(size: usize) {
-    let (status, heap_start) = ALLOCATOR.borrow_mut().allocate_pages(
-        efi::ALLOCATE_ANY_PAGES,
-        efi::BOOT_SERVICES_CODE,
-        size as u64 / PAGE_SIZE,
-        0,
-    );
-    assert!(status == Status::SUCCESS);
-    unsafe {
-        HEAP_ALLOCATOR.lock().init(heap_start as *mut _, size);
-    }
-}
-
-#[cfg(test)]
-fn init_heap_allocator(_: usize) {}
-
 #[repr(C)]
 struct LoadedImageWrapper {
     hw: HandleWrapper,

diff --git a/src/efi/var.rs b/src/efi/var.rs
@@ -1,20 +1,19 @@
 // SPDX-License-Identifier: Apache-2.0
 // Copyright (C) 2021 Akira Moroo
 
-#[cfg(not(test))]
-extern crate alloc;
-
-#[cfg(not(test))]
-use alloc::vec::Vec;
-
+use heapless::Vec;
 use r_efi::efi;
 
+const MAX_VAR_NAME: usize = 64;
+const MAX_VAR_DATA: usize = 1024;
+const MAX_VAR_NUM: usize = 128;
+
 #[derive(Debug)]
 struct Descriptor {
-    name: Vec<u16>,
+    name: Vec<u16, MAX_VAR_NAME>,
     guid: efi::Guid,
     attr: u32,
-    data: Vec<u8>,
+    data: Vec<u8, MAX_VAR_DATA>,
 }
 
 impl Descriptor {
@@ -29,7 +28,7 @@ impl Descriptor {
 }
 
 pub struct VariableAllocator {
-    allocations: Vec<Descriptor>,
+    allocations: Vec<Descriptor, MAX_VAR_NUM>,
 }
 
 impl VariableAllocator {
@@ -49,8 +48,10 @@ impl VariableAllocator {
         }
 
         let s = unsafe { core::slice::from_raw_parts(name, len + 1) };
-        let mut name: Vec<u16> = Vec::new();
-        name.extend_from_slice(s);
+        let mut name: Vec<u16, MAX_VAR_NAME> = Vec::new();
+        if name.extend_from_slice(s).is_err() {
+            return None;
+        }
         let guid = unsafe { &*guid };
         (0..self.allocations.len())
             .find(|&i| name == self.allocations[i].name && guid == &self.allocations[i].guid)
@@ -119,13 +120,19 @@ impl VariableAllocator {
             }
             let mut a = Descriptor::new();
             let name = unsafe { core::slice::from_raw_parts(name, len + 1) };
-            a.name.extend_from_slice(name);
+            if a.name.extend_from_slice(name).is_err() {
+                return efi::Status::OUT_OF_RESOURCES;
+            }
             a.guid = unsafe { *guid };
             a.attr = attr & !efi::VARIABLE_APPEND_WRITE;
             let src = unsafe { core::slice::from_raw_parts(data as *const u8, size) };
-            a.data.extend_from_slice(src);
+            if a.data.extend_from_slice(src).is_err() {
+                return efi::Status::OUT_OF_RESOURCES;
+            }
 
-            self.allocations.push(a);
+            if self.allocations.push(a).is_err() {
+                return efi::Status::OUT_OF_RESOURCES;
+            }
 
             return efi::Status::SUCCESS;
         }
@@ -144,7 +151,9 @@ impl VariableAllocator {
                 return efi::Status::INVALID_PARAMETER;
             }
             let src = unsafe { core::slice::from_raw_parts(data as *const u8, size) };
-            a.data.extend_from_slice(src);
+            if a.data.extend_from_slice(src).is_err() {
+                return efi::Status::OUT_OF_RESOURCES;
+            }
             return efi::Status::SUCCESS;
         }
 
@@ -159,7 +168,9 @@ impl VariableAllocator {
         }
         a.data.clear();
         let src = unsafe { core::slice::from_raw_parts(data as *const u8, size) };
-        a.data.extend_from_slice(src);
+        if a.data.extend_from_slice(src).is_err() {
+            return efi::Status::OUT_OF_RESOURCES;
+        }
 
         efi::Status::SUCCESS
     }