aports/testing/firecracker/20-vm-fdt-Replace-libfdt-with-vm-fdt.patch

From 61f4979803c7957d78a91f327bf2cfa03ba8cd13 Mon Sep 17 00:00:00 2001
From: AlexandruCihodaru <cihodar@amazon.com>
Date: Thu, 2 Sep 2021 23:46:45 +0300
Subject: [PATCH] [vm-fdt]: Replace libfdt with vm-fdt

Deleted helper functions that have equivalent in vm-fdt and used
vm-fdt to create the FDT.

Signed-off-by: AlexandruCihodaru <cihodar@amazon.com>
Thanks: Andreea Florescu <fandree@amazon.com>
---
 src/arch/src/aarch64/fdt.rs | 530 +++++++++++-------------------------
 src/arch/src/aarch64/mod.rs |   3 +-
 src/vmm/src/builder.rs      |   2 +-
 3 files changed, 166 insertions(+), 369 deletions(-)

diff --git a/src/arch/src/aarch64/fdt.rs b/src/arch/src/aarch64/fdt.rs
index b1db1873..f06c2dca 100644
--- a/src/arch/src/aarch64/fdt.rs
+++ b/src/arch/src/aarch64/fdt.rs
@@ -5,24 +5,19 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the THIRD-PARTY file.
 
-use libc::{c_int, c_void};
 use std::collections::HashMap;
-use std::ffi::{CStr, CString, NulError};
 use std::fmt::Debug;
-use std::ptr::null;
-use std::{io, result};
-
-use libfdt_bindings::*;
+use std::result;
 
 use super::super::DeviceType;
 use super::super::InitrdConfig;
 use super::cache_info::{read_cache_config, CacheEntry};
 use super::get_fdt_addr;
 use super::gic::GICDevice;
-use super::layout::FDT_MAX_SIZE;
-use crate::aarch64::fdt::Error::CstringFDTTransform;
 use vm_memory::{Address, Bytes, GuestAddress, GuestMemory, GuestMemoryError, GuestMemoryMmap};
 
+use vm_fdt::{Error as VmFdtError, FdtWriter, FdtWriterNode};
+
 // This is a value for uniquely identifying the FDT node declaring the interrupt controller.
 const GIC_PHANDLE: u32 = 1;
 // This is a value for uniquely identifying the FDT node containing the clock definition.
@@ -60,251 +55,74 @@ pub trait DeviceInfoForFDT {
 /// Errors thrown while configuring the Flattened Device Tree for aarch64.
 #[derive(Debug)]
 pub enum Error {
-    /// Failed to append node to the FDT.
-    AppendFDTNode(io::Error),
-    /// Failed to append a property to the FDT.
-    AppendFDTProperty(io::Error),
-    /// Syscall for creating FDT failed.
-    CreateFDT(io::Error),
-    /// Failed to obtain a C style string.
-    CstringFDTTransform(NulError),
-    /// Failure in calling syscall for terminating this FDT.
-    FinishFDTReserveMap(io::Error),
-    /// Failure in populating the cache information for the vcpus.
+    CreateFdt(VmFdtError),
     ReadCacheInfo(String),
     /// Failure in writing FDT in memory.
-    WriteFDTToMemory(GuestMemoryError),
+    WriteFdtToMemory(GuestMemoryError),
+}
+
+impl From<VmFdtError> for Error {
+    fn from(e: VmFdtError) -> Self {
+        Error::CreateFdt(e)
+    }
 }
+
 type Result<T> = result::Result<T, Error>;
 
 /// Creates the flattened device tree for this aarch64 microVM.
 pub fn create_fdt<T: DeviceInfoForFDT + Clone + Debug, S: std::hash::BuildHasher>(
     guest_mem: &GuestMemoryMmap,
     vcpu_mpidr: Vec<u64>,
-    cmdline: &CStr,
+    cmdline: &str,
     device_info: &HashMap<(DeviceType, String), T, S>,
     gic_device: &dyn GICDevice,
     initrd: &Option<InitrdConfig>,
 ) -> Result<Vec<u8>> {
     // Allocate stuff necessary for storing the blob.
-    let mut fdt = vec![0; FDT_MAX_SIZE];
-
-    allocate_fdt(&mut fdt)?;
+    let mut fdt_writer = FdtWriter::new()?;
 
     // For an explanation why these nodes were introduced in the blob take a look at
     // https://github.com/torvalds/linux/blob/master/Documentation/devicetree/booting-without-of.txt#L845
     // Look for "Required nodes and properties".
 
     // Header or the root node as per above mentioned documentation.
-    append_begin_node(&mut fdt, "")?;
-    append_property_string(&mut fdt, "compatible", "linux,dummy-virt")?;
+    let root = fdt_writer.begin_node("")?;
+    fdt_writer.property_string("compatible", "linux,dummy-virt")?;
     // For info on #address-cells and size-cells read "Note about cells and address representation"
     // from the above mentioned txt file.
-    append_property_u32(&mut fdt, "#address-cells", ADDRESS_CELLS)?;
-    append_property_u32(&mut fdt, "#size-cells", SIZE_CELLS)?;
+    fdt_writer.property_u32("#address-cells", ADDRESS_CELLS)?;
+    fdt_writer.property_u32("#size-cells", SIZE_CELLS)?;
     // This is not mandatory but we use it to point the root node to the node
     // containing description of the interrupt controller for this VM.
-    append_property_u32(&mut fdt, "interrupt-parent", GIC_PHANDLE)?;
-    create_cpu_nodes(&mut fdt, &vcpu_mpidr)?;
-    create_memory_node(&mut fdt, guest_mem)?;
-    create_chosen_node(&mut fdt, cmdline, initrd)?;
-    create_gic_node(&mut fdt, gic_device)?;
-    create_timer_node(&mut fdt)?;
-    create_clock_node(&mut fdt)?;
-    create_psci_node(&mut fdt)?;
-    create_devices_node(&mut fdt, &device_info)?;
+    fdt_writer.property_u32("interrupt-parent", GIC_PHANDLE)?;
+    create_cpu_nodes(&mut fdt_writer, &vcpu_mpidr)?;
+    create_memory_node(&mut fdt_writer, guest_mem)?;
+    create_chosen_node(&mut fdt_writer, cmdline, initrd)?;
+    create_gic_node(&mut fdt_writer, gic_device)?;
+    create_timer_node(&mut fdt_writer)?;
+    create_clock_node(&mut fdt_writer)?;
+    create_psci_node(&mut fdt_writer)?;
+    create_devices_node(&mut fdt_writer, &device_info)?;
 
     // End Header node.
-    append_end_node(&mut fdt)?;
+    fdt_writer.end_node(root)?;
 
     // Allocate another buffer so we can format and then write fdt to guest.
-    let mut fdt_final = vec![0; FDT_MAX_SIZE];
-    finish_fdt(&mut fdt, &mut fdt_final)?;
+    let fdt_final = fdt_writer.finish()?;
 
     // Write FDT to memory.
     let fdt_address = GuestAddress(get_fdt_addr(&guest_mem));
     guest_mem
         .write_slice(fdt_final.as_slice(), fdt_address)
-        .map_err(Error::WriteFDTToMemory)?;
+        .map_err(Error::WriteFdtToMemory)?;
     Ok(fdt_final)
 }
 
-// Following are auxiliary functions for allocating and finishing the FDT.
-fn allocate_fdt(fdt: &mut Vec<u8>) -> Result<()> {
-    // Safe since we allocated this array with FDT_MAX_SIZE.
-    let mut fdt_ret = unsafe { fdt_create(fdt.as_mut_ptr() as *mut c_void, FDT_MAX_SIZE as c_int) };
-
-    if fdt_ret != 0 {
-        return Err(Error::CreateFDT(io::Error::last_os_error()));
-    }
-
-    // The flattened device trees created with fdt_create() contains a list of
-    // reserved memory areas. We need to call `fdt_finish_reservemap` so as to make sure that there is a
-    // terminator in the reservemap list and whatever happened to be at the
-    // start of the FDT data section would end up being interpreted as
-    // reservemap entries.
-    // Safe since we previously allocated this array.
-    fdt_ret = unsafe { fdt_finish_reservemap(fdt.as_mut_ptr() as *mut c_void) };
-    if fdt_ret != 0 {
-        return Err(Error::FinishFDTReserveMap(io::Error::last_os_error()));
-    }
-    Ok(())
-}
-
-fn finish_fdt(from_fdt: &mut Vec<u8>, to_fdt: &mut Vec<u8>) -> Result<()> {
-    // Safe since we allocated `fdt_final` and previously passed in its size.
-    let mut fdt_ret = unsafe { fdt_finish(from_fdt.as_mut_ptr() as *mut c_void) };
-    if fdt_ret != 0 {
-        return Err(Error::FinishFDTReserveMap(io::Error::last_os_error()));
-    }
-
-    // Safe because we allocated both arrays with the correct size.
-    fdt_ret = unsafe {
-        fdt_open_into(
-            from_fdt.as_mut_ptr() as *mut c_void,
-            to_fdt.as_mut_ptr() as *mut c_void,
-            FDT_MAX_SIZE as i32,
-        )
-    };
-    if fdt_ret != 0 {
-        return Err(Error::FinishFDTReserveMap(io::Error::last_os_error()));
-    }
-
-    // Safe since we allocated `to_fdt`.
-    fdt_ret = unsafe { fdt_pack(to_fdt.as_mut_ptr() as *mut c_void) };
-    if fdt_ret != 0 {
-        return Err(Error::FinishFDTReserveMap(io::Error::last_os_error()));
-    }
-    Ok(())
-}
-
-// Following are auxiliary functions for appending nodes to FDT.
-fn append_begin_node(fdt: &mut Vec<u8>, name: &str) -> Result<()> {
-    let cstr_name = CString::new(name).map_err(CstringFDTTransform)?;
-
-    // Safe because we allocated fdt and converted name to a CString
-    let fdt_ret = unsafe { fdt_begin_node(fdt.as_mut_ptr() as *mut c_void, cstr_name.as_ptr()) };
-    if fdt_ret != 0 {
-        return Err(Error::AppendFDTNode(io::Error::last_os_error()));
-    }
-    Ok(())
-}
-
-fn append_end_node(fdt: &mut Vec<u8>) -> Result<()> {
-    // Safe because we allocated fdt.
-    let fdt_ret = unsafe { fdt_end_node(fdt.as_mut_ptr() as *mut c_void) };
-    if fdt_ret != 0 {
-        return Err(Error::AppendFDTNode(io::Error::last_os_error()));
-    }
-    Ok(())
-}
-
-// Following are auxiliary functions for appending property nodes to the nodes of the FDT.
-fn append_property_u32(fdt: &mut Vec<u8>, name: &str, val: u32) -> Result<()> {
-    append_property(fdt, name, &to_be32(val))
-}
-
-fn append_property_u64(fdt: &mut Vec<u8>, name: &str, val: u64) -> Result<()> {
-    append_property(fdt, name, &to_be64(val))
-}
-
-fn append_property_string(fdt: &mut Vec<u8>, name: &str, value: &str) -> Result<()> {
-    let cstr_value = CString::new(value).map_err(CstringFDTTransform)?;
-    append_property_cstring(fdt, name, &cstr_value)
-}
-
-fn append_property_cstring(fdt: &mut Vec<u8>, name: &str, cstr_value: &CStr) -> Result<()> {
-    let value_bytes = cstr_value.to_bytes_with_nul();
-    let cstr_name = CString::new(name).map_err(CstringFDTTransform)?;
-    // Safe because we allocated fdt, converted name and value to CStrings
-    let fdt_ret = unsafe {
-        fdt_property(
-            fdt.as_mut_ptr() as *mut c_void,
-            cstr_name.as_ptr(),
-            value_bytes.as_ptr() as *mut c_void,
-            value_bytes.len() as i32,
-        )
-    };
-    if fdt_ret != 0 {
-        return Err(Error::AppendFDTProperty(io::Error::last_os_error()));
-    }
-    Ok(())
-}
-
-fn append_property_null(fdt: &mut Vec<u8>, name: &str) -> Result<()> {
-    let cstr_name = CString::new(name).map_err(CstringFDTTransform)?;
-
-    // Safe because we allocated fdt, converted name to a CString
-    let fdt_ret = unsafe {
-        fdt_property(
-            fdt.as_mut_ptr() as *mut c_void,
-            cstr_name.as_ptr(),
-            null(),
-            0,
-        )
-    };
-    if fdt_ret != 0 {
-        return Err(Error::AppendFDTProperty(io::Error::last_os_error()));
-    }
-    Ok(())
-}
-
-fn append_property(fdt: &mut Vec<u8>, name: &str, val: &[u8]) -> Result<()> {
-    let cstr_name = CString::new(name).map_err(CstringFDTTransform)?;
-    let val_ptr = val.as_ptr() as *const c_void;
-
-    // Safe because we allocated fdt and converted name to a CString
-    let fdt_ret = unsafe {
-        fdt_property(
-            fdt.as_mut_ptr() as *mut c_void,
-            cstr_name.as_ptr(),
-            val_ptr,
-            val.len() as i32,
-        )
-    };
-    if fdt_ret != 0 {
-        return Err(Error::AppendFDTProperty(io::Error::last_os_error()));
-    }
-    Ok(())
-}
-
-fn append_cache_property_u32(fdt: &mut Vec<u8>, name: &str, val: Option<u32>) -> Result<()> {
-    if let Some(cache_attr) = val {
-        append_property_u32(fdt, name, cache_attr)?;
-    }
-    Ok(())
-}
-
-// Auxiliary functions for writing u32/u64 numbers in big endian order.
-fn to_be32(input: u32) -> [u8; 4] {
-    u32::to_be_bytes(input)
-}
-
-fn to_be64(input: u64) -> [u8; 8] {
-    u64::to_be_bytes(input)
-}
-
-// Helper functions for generating a properly formatted byte vector using 32-bit/64-bit cells.
-fn generate_prop32(cells: &[u32]) -> Vec<u8> {
-    let mut ret: Vec<u8> = Vec::new();
-    for &e in cells {
-        ret.extend(to_be32(e).iter());
-    }
-    ret
-}
-
-fn generate_prop64(cells: &[u64]) -> Vec<u8> {
-    let mut ret: Vec<u8> = Vec::new();
-    for &e in cells {
-        ret.extend(to_be64(e).iter());
-    }
-    ret
-}
-
 // Following are the auxiliary function for creating the different nodes that we append to our FDT.
-fn create_cpu_nodes(fdt: &mut Vec<u8>, vcpu_mpidr: &[u64]) -> Result<()> {
+fn create_cpu_nodes(fdt: &mut FdtWriter, vcpu_mpidr: &[u64]) -> Result<()> {
     // Since the L1 caches are not shareable among CPUs and they are direct attributes of the
     // cpu in the device tree, we process the L1 and non-L1 caches separately.
+    // We use sysfs for extracting the cache information.
     let mut l1_caches: Vec<CacheEntry> = Vec::new();
     let mut non_l1_caches: Vec<CacheEntry> = Vec::new();
     // We use sysfs for extracting the cache information.
@@ -312,49 +130,42 @@ fn create_cpu_nodes(fdt: &mut Vec<u8>, vcpu_mpidr: &[u64]) -> Result<()> {
         .map_err(|e| Error::ReadCacheInfo(e.to_string()))?;
 
     // See https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/arm/cpus.yaml.
-    append_begin_node(fdt, "cpus")?;
+    let cpus = fdt.begin_node("cpus")?;
     // As per documentation, on ARM v8 64-bit systems value should be set to 2.
-    append_property_u32(fdt, "#address-cells", 0x02)?;
-    append_property_u32(fdt, "#size-cells", 0x0)?;
+    fdt.property_u32("#address-cells", 0x02)?;
+    fdt.property_u32("#size-cells", 0x0)?;
     let num_cpus = vcpu_mpidr.len();
     for (cpu_index, mpidr) in vcpu_mpidr.iter().enumerate() {
-        let cpu_name = format!("cpu@{:x}", cpu_index);
-        append_begin_node(fdt, &cpu_name)?;
-        append_property_string(fdt, "device_type", "cpu")?;
-        append_property_string(fdt, "compatible", "arm,arm-v8")?;
+        let cpu = fdt.begin_node(&format!("cpu@{:x}", cpu_index))?;
+        fdt.property_string("device_type", "cpu")?;
+        fdt.property_string("compatible", "arm,arm-v8")?;
         // The power state coordination interface (PSCI) needs to be enabled for
         // all vcpus.
-        append_property_string(fdt, "enable-method", "psci")?;
+        fdt.property_string("enable-method", "psci")?;
         // Set the field to first 24 bits of the MPIDR - Multiprocessor Affinity Register.
         // See http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0488c/BABHBJCI.html.
-        append_property_u64(fdt, "reg", mpidr & 0x7F_FFFF)?;
+        fdt.property_u64("reg", mpidr & 0x7FFFFF)?;
 
         for cache in l1_caches.iter() {
             // Please check out
             // https://github.com/devicetree-org/devicetree-specification/releases/download/v0.3/devicetree-specification-v0.3.pdf,
             // section 3.8.
-            append_cache_property_u32(
-                fdt,
-                cache.type_.of_cache_size(),
-                cache.size_.map(|s| s as u32),
-            )?;
-            append_cache_property_u32(
-                fdt,
-                cache.type_.of_cache_line_size(),
-                cache.line_size.map(|l| l as u32),
-            )?;
-            append_cache_property_u32(
-                fdt,
-                cache.type_.of_cache_sets(),
-                cache.number_of_sets.map(|s| s as u32),
-            )?;
+            if let Some(size) = cache.size_ {
+                fdt.property_u32(cache.type_.of_cache_size(), size as u32)?;
+            }
+            if let Some(line_size) = cache.line_size {
+                fdt.property_u32(cache.type_.of_cache_line_size(), line_size as u32)?;
+            }
+            if let Some(number_of_sets) = cache.number_of_sets {
+                fdt.property_u32(cache.type_.of_cache_sets(), number_of_sets as u32)?;
+            }
         }
 
         // Some of the non-l1 caches can be shared amongst CPUs. You can see an example of a shared scenario
         // in https://github.com/devicetree-org/devicetree-specification/releases/download/v0.3/devicetree-specification-v0.3.pdf,
         // 3.8.1 Example.
         let mut prev_level = 1;
-        let mut node = false;
+        let mut cache_node: Option<FdtWriterNode> = None;
         for cache in non_l1_caches.iter() {
             // We append the next-level-cache node (the node that specifies the cache hierarchy)
             // in the next iteration. For example,
@@ -371,141 +182,127 @@ fn create_cpu_nodes(fdt: &mut Vec<u8>, vcpu_mpidr: &[u64]) -> Result<()> {
                     as u32;
 
             if prev_level != cache.level {
-                append_property_u32(fdt, "next-level-cache", cache_phandle)?;
-                if prev_level > 1 {
-                    append_end_node(fdt)?;
-                    node = false;
+                fdt.property_u32("next-level-cache", cache_phandle)?;
+                if prev_level > 1 && cache_node.is_some() {
+                    fdt.end_node(cache_node.take().unwrap())?;
                 }
             }
 
             if cpu_index % cache.cpus_per_unit as usize == 0 {
-                node = true;
-                append_begin_node(
-                    fdt,
-                    &format!(
-                        "l{}-{}-cache",
-                        cache.level,
-                        cpu_index / cache.cpus_per_unit as usize
-                    ),
-                )?;
-                append_property_u32(fdt, "phandle", cache_phandle)?;
-                append_property_string(fdt, "compatible", "cache")?;
-                append_property_u32(fdt, "cache-level", cache.level as u32)?;
-                append_cache_property_u32(
-                    fdt,
-                    cache.type_.of_cache_size(),
-                    cache.size_.map(|s| s as u32),
-                )?;
-
-                append_cache_property_u32(
-                    fdt,
-                    cache.type_.of_cache_line_size(),
-                    cache.line_size.map(|l| l as u32),
-                )?;
-                append_cache_property_u32(
-                    fdt,
-                    cache.type_.of_cache_sets(),
-                    cache.number_of_sets.map(|s| s as u32),
-                )?;
+                cache_node = Some(fdt.begin_node(&format!(
+                    "l{}-{}-cache",
+                    cache.level,
+                    cpu_index / cache.cpus_per_unit as usize
+                ))?);
+                fdt.property_u32("phandle", cache_phandle)?;
+                fdt.property_string("compatible", "cache")?;
+                fdt.property_u32("cache-level", cache.level as u32)?;
+                if let Some(size) = cache.size_ {
+                    fdt.property_u32(cache.type_.of_cache_size(), size as u32)?;
+                }
+                if let Some(line_size) = cache.line_size {
+                    fdt.property_u32(cache.type_.of_cache_line_size(), line_size as u32)?;
+                }
+                if let Some(number_of_sets) = cache.number_of_sets {
+                    fdt.property_u32(cache.type_.of_cache_sets(), number_of_sets as u32)?;
+                }
                 if let Some(cache_type) = cache.type_.of_cache_type() {
-                    append_property_null(fdt, cache_type)?;
+                    fdt.property_null(cache_type)?;
                 }
                 prev_level = cache.level;
             }
         }
-        if node {
-            append_end_node(fdt)?;
+        if let Some(node) = cache_node {
+            fdt.end_node(node)?;
         }
-        append_end_node(fdt)?;
+
+        fdt.end_node(cpu)?;
     }
-    append_end_node(fdt)?;
+    fdt.end_node(cpus)?;
+
     Ok(())
 }
 
-fn create_memory_node(fdt: &mut Vec<u8>, guest_mem: &GuestMemoryMmap) -> Result<()> {
+fn create_memory_node(fdt: &mut FdtWriter, guest_mem: &GuestMemoryMmap) -> Result<()> {
     let mem_size = guest_mem.last_addr().raw_value() - super::layout::DRAM_MEM_START + 1;
     // See https://github.com/torvalds/linux/blob/master/Documentation/devicetree/booting-without-of.txt#L960
     // for an explanation of this.
-    let mem_reg_prop = generate_prop64(&[super::layout::DRAM_MEM_START as u64, mem_size as u64]);
+    let mem_reg_prop = &[super::layout::DRAM_MEM_START as u64, mem_size as u64];
+
+    let mem = fdt.begin_node("memory")?;
+    fdt.property_string("device_type", "memory")?;
+    fdt.property_array_u64("reg", mem_reg_prop)?;
+    fdt.end_node(mem)?;
 
-    append_begin_node(fdt, "memory")?;
-    append_property_string(fdt, "device_type", "memory")?;
-    append_property(fdt, "reg", &mem_reg_prop)?;
-    append_end_node(fdt)?;
     Ok(())
 }
 
 fn create_chosen_node(
-    fdt: &mut Vec<u8>,
-    cmdline: &CStr,
+    fdt: &mut FdtWriter,
+    cmdline: &str,
     initrd: &Option<InitrdConfig>,
 ) -> Result<()> {
-    append_begin_node(fdt, "chosen")?;
-    append_property_cstring(fdt, "bootargs", cmdline)?;
+    let chosen = fdt.begin_node("chosen")?;
+    fdt.property_string("bootargs", cmdline)?;
 
     if let Some(initrd_config) = initrd {
-        append_property_u64(
-            fdt,
+        fdt.property_u64(
             "linux,initrd-start",
             initrd_config.address.raw_value() as u64,
         )?;
-        append_property_u64(
-            fdt,
+        fdt.property_u64(
             "linux,initrd-end",
             initrd_config.address.raw_value() + initrd_config.size as u64,
         )?;
     }
 
-    append_end_node(fdt)?;
+    fdt.end_node(chosen)?;
 
     Ok(())
 }
 
-fn create_gic_node(fdt: &mut Vec<u8>, gic_device: &dyn GICDevice) -> Result<()> {
-    let gic_reg_prop = generate_prop64(gic_device.device_properties());
-
-    append_begin_node(fdt, "intc")?;
-    append_property_string(fdt, "compatible", gic_device.fdt_compatibility())?;
-    append_property_null(fdt, "interrupt-controller")?;
+fn create_gic_node(fdt: &mut FdtWriter, gic_device: &dyn GICDevice) -> Result<()> {
+    let interrupt = fdt.begin_node("intc")?;
+    fdt.property_string("compatible", gic_device.fdt_compatibility())?;
+    fdt.property_null("interrupt-controller")?;
     // "interrupt-cells" field specifies the number of cells needed to encode an
     // interrupt source. The type shall be a <u32> and the value shall be 3 if no PPI affinity description
     // is required.
-    append_property_u32(fdt, "#interrupt-cells", 3)?;
-    append_property(fdt, "reg", &gic_reg_prop)?;
-    append_property_u32(fdt, "phandle", GIC_PHANDLE)?;
-    append_property_u32(fdt, "#address-cells", 2)?;
-    append_property_u32(fdt, "#size-cells", 2)?;
-    append_property_null(fdt, "ranges")?;
+    fdt.property_u32("#interrupt-cells", 3)?;
+    fdt.property_array_u64("reg", &gic_device.device_properties())?;
+    fdt.property_u32("phandle", GIC_PHANDLE)?;
+    fdt.property_u32("#address-cells", 2)?;
+    fdt.property_u32("#size-cells", 2)?;
+    fdt.property_null("ranges")?;
+
     let gic_intr = [
         GIC_FDT_IRQ_TYPE_PPI,
         gic_device.fdt_maint_irq(),
         IRQ_TYPE_LEVEL_HI,
     ];
-    let gic_intr_prop = generate_prop32(&gic_intr);
 
-    append_property(fdt, "interrupts", &gic_intr_prop)?;
-    append_end_node(fdt)?;
+    fdt.property_array_u32("interrupts", &gic_intr)?;
+    fdt.end_node(interrupt)?;
 
     Ok(())
 }
 
-fn create_clock_node(fdt: &mut Vec<u8>) -> Result<()> {
+fn create_clock_node(fdt: &mut FdtWriter) -> Result<()> {
     // The Advanced Peripheral Bus (APB) is part of the Advanced Microcontroller Bus Architecture
     // (AMBA) protocol family. It defines a low-cost interface that is optimized for minimal power
     // consumption and reduced interface complexity.
     // PCLK is the clock source and this node defines exactly the clock for the APB.
-    append_begin_node(fdt, "apb-pclk")?;
-    append_property_string(fdt, "compatible", "fixed-clock")?;
-    append_property_u32(fdt, "#clock-cells", 0x0)?;
-    append_property_u32(fdt, "clock-frequency", 24_000_000)?;
-    append_property_string(fdt, "clock-output-names", "clk24mhz")?;
-    append_property_u32(fdt, "phandle", CLOCK_PHANDLE)?;
-    append_end_node(fdt)?;
-
+    let clock = fdt.begin_node("apb-pclk")?;
+    fdt.property_string("compatible", "fixed-clock")?;
+    fdt.property_u32("#clock-cells", 0x0)?;
+    fdt.property_u32("clock-frequency", 24_000_000)?;
+    fdt.property_string("clock-output-names", "clk24mhz")?;
+    fdt.property_u32("phandle", CLOCK_PHANDLE)?;
+    fdt.end_node(clock)?;
     Ok(())
 }
 
-fn create_timer_node(fdt: &mut Vec<u8>) -> Result<()> {
+fn create_timer_node(fdt: &mut FdtWriter) -> Result<()> {
     // See
     // https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/interrupt-controller/arch_timer.txt
     // These are fixed interrupt numbers for the timer device.
@@ -518,85 +315,88 @@ fn create_timer_node(fdt: &mut Vec<u8>) -> Result<()> {
         timer_reg_cells.push(irq);
         timer_reg_cells.push(IRQ_TYPE_LEVEL_HI);
     }
-    let timer_reg_prop = generate_prop32(timer_reg_cells.as_slice());
-
-    append_begin_node(fdt, "timer")?;
-    append_property_string(fdt, "compatible", compatible)?;
-    append_property_null(fdt, "always-on")?;
-    append_property(fdt, "interrupts", &timer_reg_prop)?;
-    append_end_node(fdt)?;
 
+    let timer = fdt.begin_node("timer")?;
+    fdt.property_string("compatible", compatible)?;
+    fdt.property_null("always-on")?;
+    fdt.property_array_u32("interrupts", &timer_reg_cells)?;
+    fdt.end_node(timer)?;
     Ok(())
 }
 
-fn create_psci_node(fdt: &mut Vec<u8>) -> Result<()> {
+fn create_psci_node(fdt: &mut FdtWriter) -> Result<()> {
     let compatible = "arm,psci-0.2";
-    append_begin_node(fdt, "psci")?;
-    append_property_string(fdt, "compatible", compatible)?;
+
+    let psci = fdt.begin_node("psci")?;
+    fdt.property_string("compatible", compatible)?;
     // Two methods available: hvc and smc.
     // As per documentation, PSCI calls between a guest and hypervisor may use the HVC conduit instead of SMC.
     // So, since we are using kvm, we need to use hvc.
-    append_property_string(fdt, "method", "hvc")?;
-    append_end_node(fdt)?;
+    fdt.property_string("method", "hvc")?;
+    fdt.end_node(psci)?;
 
     Ok(())
 }
 
 fn create_virtio_node<T: DeviceInfoForFDT + Clone + Debug>(
-    fdt: &mut Vec<u8>,
+    fdt: &mut FdtWriter,
     dev_info: &T,
 ) -> Result<()> {
-    let device_reg_prop = generate_prop64(&[dev_info.addr(), dev_info.length()]);
-    let irq = generate_prop32(&[GIC_FDT_IRQ_TYPE_SPI, dev_info.irq(), IRQ_TYPE_EDGE_RISING]);
+    let virtio_mmio = fdt.begin_node(&format!("virtio_mmio@{:x}", dev_info.addr()))?;
 
-    append_begin_node(fdt, &format!("virtio_mmio@{:x}", dev_info.addr()))?;
-    append_property_string(fdt, "compatible", "virtio,mmio")?;
-    append_property(fdt, "reg", &device_reg_prop)?;
-    append_property(fdt, "interrupts", &irq)?;
-    append_property_u32(fdt, "interrupt-parent", GIC_PHANDLE)?;
-    append_end_node(fdt)?;
+    fdt.property_string("compatible", "virtio,mmio")?;
+    fdt.property_array_u64("reg", &[dev_info.addr(), dev_info.length()])?;
+    fdt.property_array_u32(
+        "interrupts",
+        &[GIC_FDT_IRQ_TYPE_SPI, dev_info.irq(), IRQ_TYPE_EDGE_RISING],
+    )?;
+    fdt.property_u32("interrupt-parent", GIC_PHANDLE)?;
+    fdt.end_node(virtio_mmio)?;
 
     Ok(())
 }
 
 fn create_serial_node<T: DeviceInfoForFDT + Clone + Debug>(
-    fdt: &mut Vec<u8>,
+    fdt: &mut FdtWriter,
     dev_info: &T,
 ) -> Result<()> {
-    let serial_reg_prop = generate_prop64(&[dev_info.addr(), dev_info.length()]);
-    let irq = generate_prop32(&[GIC_FDT_IRQ_TYPE_SPI, dev_info.irq(), IRQ_TYPE_EDGE_RISING]);
-
-    append_begin_node(fdt, &format!("uart@{:x}", dev_info.addr()))?;
-    append_property_string(fdt, "compatible", "ns16550a")?;
-    append_property(fdt, "reg", &serial_reg_prop)?;
-    append_property_u32(fdt, "clocks", CLOCK_PHANDLE)?;
-    append_property_string(fdt, "clock-names", "apb_pclk")?;
-    append_property(fdt, "interrupts", &irq)?;
-    append_end_node(fdt)?;
+    let serial = fdt.begin_node(&format!("uart@{:x}", dev_info.addr()))?;
+
+    fdt.property_string("compatible", "ns16550a")?;
+    fdt.property_array_u64("reg", &[dev_info.addr(), dev_info.length()])?;
+    fdt.property_u32("clocks", CLOCK_PHANDLE)?;
+    fdt.property_string("clock-names", "apb_pclk")?;
+    fdt.property_array_u32(
+        "interrupts",
+        &[GIC_FDT_IRQ_TYPE_SPI, dev_info.irq(), IRQ_TYPE_EDGE_RISING],
+    )?;
+    fdt.end_node(serial)?;
 
     Ok(())
 }
 
 fn create_rtc_node<T: DeviceInfoForFDT + Clone + Debug>(
-    fdt: &mut Vec<u8>,
+    fdt: &mut FdtWriter,
     dev_info: &T,
 ) -> Result<()> {
     let compatible = b"arm,pl031\0arm,primecell\0";
-    let rtc_reg_prop = generate_prop64(&[dev_info.addr(), dev_info.length()]);
-    let irq = generate_prop32(&[GIC_FDT_IRQ_TYPE_SPI, dev_info.irq(), IRQ_TYPE_LEVEL_HI]);
-    append_begin_node(fdt, &format!("rtc@{:x}", dev_info.addr()))?;
-    append_property(fdt, "compatible", compatible)?;
-    append_property(fdt, "reg", &rtc_reg_prop)?;
-    append_property(fdt, "interrupts", &irq)?;
-    append_property_u32(fdt, "clocks", CLOCK_PHANDLE)?;
-    append_property_string(fdt, "clock-names", "apb_pclk")?;
-    append_end_node(fdt)?;
+
+    let rtc = fdt.begin_node(&format!("rtc@{:x}", dev_info.addr()))?;
+    fdt.property("compatible", compatible)?;
+    fdt.property_array_u64("reg", &[dev_info.addr(), dev_info.length()])?;
+    fdt.property_array_u32(
+        "interrupts",
+        &[GIC_FDT_IRQ_TYPE_SPI, dev_info.irq(), IRQ_TYPE_LEVEL_HI],
+    )?;
+    fdt.property_u32("clocks", CLOCK_PHANDLE)?;
+    fdt.property_string("clock-names", "apb_pclk")?;
+    fdt.end_node(rtc)?;
 
     Ok(())
 }
 
 fn create_devices_node<T: DeviceInfoForFDT + Clone + Debug, S: std::hash::BuildHasher>(
-    fdt: &mut Vec<u8>,
+    fdt: &mut FdtWriter,
     dev_info: &HashMap<(DeviceType, String), T, S>,
 ) -> Result<()> {
     // Create one temp Vec to store all virtio devices
@@ -688,7 +488,7 @@ mod tests {
         assert!(create_fdt(
             &mem,
             vec![0],
-            &CString::new("console=tty0").unwrap(),
+            "console=tty0",
             &dev_info,
             gic.as_ref(),
             &None,
@@ -710,10 +510,10 @@ mod tests {
             _ => panic!("Unexpected gic version!"),
         };
 
-        let mut current_dtb_bytes = create_fdt(
+        let current_dtb_bytes = create_fdt(
             &mem,
             vec![0],
-            &CString::new("console=tty0").unwrap(),
+            "console=tty0",
             &HashMap::<(DeviceType, std::string::String), MMIODeviceInfo>::new(),
             gic.as_ref(),
             &None,
@@ -746,7 +546,6 @@ mod tests {
         buf.extend_from_slice(saved_dtb_bytes);
 
         set_size(&mut buf, pos, val);
-        set_size(&mut current_dtb_bytes, pos, val);
         let original_fdt = device_tree::DeviceTree::load(&buf).unwrap();
         let generated_fdt = device_tree::DeviceTree::load(&current_dtb_bytes).unwrap();
         assert_eq!(
@@ -774,10 +573,10 @@ mod tests {
             size: 0x1000,
         };
 
-        let mut current_dtb_bytes = create_fdt(
+        let current_dtb_bytes = create_fdt(
             &mem,
             vec![0],
-            &CString::new("console=tty0").unwrap(),
+            "console=tty0",
             &HashMap::<(DeviceType, std::string::String), MMIODeviceInfo>::new(),
             gic.as_ref(),
             &Some(initrd),
@@ -810,7 +609,6 @@ mod tests {
         buf.extend_from_slice(saved_dtb_bytes);
 
         set_size(&mut buf, pos, val);
-        set_size(&mut current_dtb_bytes, pos, val);
         let original_fdt = device_tree::DeviceTree::load(&buf).unwrap();
         let generated_fdt = device_tree::DeviceTree::load(&current_dtb_bytes).unwrap();
         assert_eq!(
diff --git a/src/arch/src/aarch64/mod.rs b/src/arch/src/aarch64/mod.rs
index 0ddbf776..fb6baf54 100644
--- a/src/arch/src/aarch64/mod.rs
+++ b/src/arch/src/aarch64/mod.rs
@@ -12,7 +12,6 @@ pub mod regs;
 
 use std::cmp::min;
 use std::collections::HashMap;
-use std::ffi::CStr;
 use std::fmt::Debug;
 
 pub use self::fdt::DeviceInfoForFDT;
@@ -52,7 +51,7 @@ pub fn arch_memory_regions(size: usize) -> Vec<(GuestAddress, usize)> {
 /// * `initrd` - Information about an optional initrd.
 pub fn configure_system<T: DeviceInfoForFDT + Clone + Debug, S: std::hash::BuildHasher>(
     guest_mem: &GuestMemoryMmap,
-    cmdline_cstring: &CStr,
+    cmdline_cstring: &str,
     vcpu_mpidr: Vec<u64>,
     device_info: &HashMap<(DeviceType, String), T, S>,
     gic_device: &dyn GICDevice,
diff --git a/src/vmm/src/builder.rs b/src/vmm/src/builder.rs
index 5f5b7db1..6f38d9c3 100644
--- a/src/vmm/src/builder.rs
+++ b/src/vmm/src/builder.rs
@@ -783,7 +783,7 @@ pub fn configure_system_for_boot(
             .collect();
         arch::aarch64::configure_system(
             &vmm.guest_memory,
-            &boot_cmdline.as_cstring().map_err(LoadCommandline)?,
+            &boot_cmdline.as_str(),
             vcpu_mpidr,
             vmm.mmio_device_manager.get_device_info(),
             vmm.vm.get_irqchip(),
-- 
2.31.1