Final PVH fixups

CHANGELOG.md

@@ -12,6 +12,11 @@ and this project adheres to
 
 ### Changed
 
+- Added support for PVH boot mode. This is used when an x86 kernel provides the
+  appropriate ELF Note to indicate that PVH boot mode is supported. Linux
+  kernels newer than 5.0 compiled with `CONFIG_PVH=y` set this ELF Note, as do
+  FreeBSD kernels.
+
 ### Deprecated
 
 ### Removed
@@ -140,10 +145,6 @@ and this project adheres to
   [random for clones](docs/snapshotting/random-for-clones.md) documention for
   more info on VMGenID. VMGenID state is part of the snapshot format of
   Firecracker. As a result, Firecracker snapshot version is now 2.0.0.
-- Added support for PVH boot mode.  This is used when an x86 kernel provides
-  the appropriate ELF Note to indicate that PVH boot mode is supported.
-  Linux kernels compiled with CONFIG_XEN_PVH=y set this ELF Note, as do
-  FreeBSD kernels.
 
 ### Changed
 

docs/pvh.md

@@ -3,13 +3,13 @@
 Firecracker supports booting x86 kernels in "PVH direct boot" mode
 [as specified by the Xen project](https://github.com/xen-project/xen/blob/master/docs/misc/pvh.pandoc).
 If a kernel is provided which contains the XEN_ELFNOTE_PHYS32_ENTRY ELF Note
-then this boot mode will be used.  This boot mode was designed for virtualized
+then this boot mode will be used. This boot mode was designed for virtualized
 environments which load the kernel directly, and is simpler than the "Linux
 boot" mode which is designed to be launched from a legacy boot loader.
 
-PVH boot mode can be enabled for Linux by setting CONFIG_XEN_PVH=y in the
-kernel configuration.  (This is not the default setting.)
+PVH boot mode can be enabled for Linux by setting `CONFIG_PVH=y` in the kernel
+configuration. (This is not the default setting.)
 
 PVH boot mode is enabled by default in FreeBSD, which has support for
-Firecracker starting with FreeBSD 14.0.  Instructions on building a FreeBSD
+Firecracker starting with FreeBSD 14.0. Instructions on building a FreeBSD
 kernel and root filesystem are available [here](rootfs-and-kernel-setup.md).

docs/rootfs-and-kernel-setup.md

@@ -191,16 +191,16 @@ Firecracker.
 Here's a quick step-by-step guide to building a FreeBSD rootfs and kernel that
 Firecracker can boot:
 
-1. Boot a FreeBSD system.  In EC2, the
+1. Boot a FreeBSD system. In EC2, the
    [FreeBSD 13 Marketplace image](https://aws.amazon.com/marketplace/pp/prodview-ukzmy5dzc6nbq)
    is a good option; you can also use weekly snapshot AMIs published by the
-   FreeBSD project.  (Firecracker support is in FreeBSD 14 and later, so you'll
+   FreeBSD project. (Firecracker support is in FreeBSD 14 and later, so you'll
    need FreeBSD 13 or later to build it.)
 
    The build will require about 50 GB of disk space, so size the disk
    appropriately.
 
-1. Log in to the FreeBSD system and become root.  If using EC2, you'll want to
+1. Log in to the FreeBSD system and become root. If using EC2, you'll want to
    ssh in as `ec2-user` with your chosen SSH key and then `su` to become root.
 
 1. Install git and check out the FreeBSD src tree:
@@ -220,10 +220,10 @@ Firecracker can boot:
    make -C /usr/src/release firecracker DESTDIR=`pwd`
    ```
 
-You should now have a rootfs `freebsd-rootfs.bin` and a kernel `freebsd-kern.bin`
-in the current directory (or elsewhere if you change the `DESTDIR` value) that
-you can boot with Firecracker.  Note that the FreeBSD rootfs generated in this
-manner is somewhat minimized compared to "stock" FreeBSD; it omits utilities
-which are only relevant on physical systems (e.g., utilities related to floppy
-disks, USB devices, and some network interfaces) and also debug files and the
-system compiler.
+You should now have a rootfs `freebsd-rootfs.bin` and a kernel
+`freebsd-kern.bin` in the current directory (or elsewhere if you change the
+`DESTDIR` value) that you can boot with Firecracker. Note that the FreeBSD
+rootfs generated in this manner is somewhat minimized compared to "stock"
+FreeBSD; it omits utilities which are only relevant on physical systems (e.g.,
+utilities related to floppy disks, USB devices, and some network interfaces) and
+also debug files and the system compiler.

src/vmm/src/arch/x86_64/gdt.rs

@@ -49,6 +49,7 @@ fn get_base(entry: u64) -> u64 {
 // (For more information concerning the formats of segment descriptors, VMCS fields, et cetera,
 // please consult the Intel Software Developer Manual.)
 fn get_limit(entry: u64) -> u32 {
+    #[allow(clippy::cast_possible_truncation)] // clearly, truncation is not possible
     let limit: u32 =
         ((((entry) & 0x000F_0000_0000_0000) >> 32) | ((entry) & 0x0000_0000_0000_FFFF)) as u32;
 

src/vmm/src/arch/x86_64/mod.rs

@@ -27,7 +27,6 @@ use linux_loader::configurator::linux::LinuxBootConfigurator;
 use linux_loader::configurator::pvh::PvhBootConfigurator;
 use linux_loader::configurator::{BootConfigurator, BootParams};
 use linux_loader::loader::bootparam::boot_params;
-
 use linux_loader::loader::elf::start_info::{
     hvm_memmap_table_entry, hvm_modlist_entry, hvm_start_info,
 };
@@ -136,7 +135,8 @@ pub fn configure_system(
     boot_prot: BootProtocol,
 ) -> Result<(), ConfigurationError> {
     // Note that this puts the mptable at the last 1k of Linux's 640k base RAM
-    mptable::setup_mptable(guest_mem, resource_allocator, num_cpus).map_err(ConfigurationError::MpTableSetup)?;
+    mptable::setup_mptable(guest_mem, resource_allocator, num_cpus)
+        .map_err(ConfigurationError::MpTableSetup)?;
 
     match boot_prot {
         BootProtocol::PvhBoot => {
@@ -214,6 +214,7 @@ fn configure_pvh(
     // boot_params.  This will be stored at PVH_INFO_START address, and %rbx
     // will be initialized to contain PVH_INFO_START prior to starting the
     // guest, as required by the PVH ABI.
+    #[allow(clippy::cast_possible_truncation)] // the vec lenghts are single digit integers
     let mut start_info = hvm_start_info {
         magic: XEN_HVM_START_MAGIC_VALUE,
         version: 1,
@@ -275,10 +276,11 @@ fn configure_64bit_boot(
 
     let himem_start = GuestAddress(layout::HIMEM_START);
 
-    let mut params = boot_params::default();
-
     // Set the location of RSDP in Boot Parameters to help the guest kernel find it faster.
-    params.acpi_rsdp_addr = layout::RSDP_ADDR;
+    let mut params = boot_params {
+        acpi_rsdp_addr: layout::RSDP_ADDR,
+        ..boot_params::default()
+    };
     params.hdr.type_of_loader = KERNEL_LOADER_OTHER;
     params.hdr.boot_flag = KERNEL_BOOT_FLAG_MAGIC;
     params.hdr.header = KERNEL_HDR_MAGIC;
@@ -388,8 +390,15 @@ mod tests {
         let no_vcpus = 4;
         let gm = single_region_mem(0x10000);
         let mut resource_allocator = ResourceAllocator::new().unwrap();
-        let config_err =
-            configure_system(&gm, &mut resource_allocator, GuestAddress(0), 0, &None, 1, BootProtocol::LinuxBoot);
+        let config_err = configure_system(
+            &gm,
+            &mut resource_allocator,
+            GuestAddress(0),
+            0,
+            &None,
+            1,
+            BootProtocol::LinuxBoot,
+        );
         assert_eq!(
             config_err.unwrap_err(),
             super::ConfigurationError::MpTableSetup(mptable::MptableError::NotEnoughMemory)

src/vmm/src/arch/x86_64/regs.rs

@@ -406,7 +406,7 @@ mod tests {
         [BootProtocol::LinuxBoot, BootProtocol::PvhBoot]
             .iter()
             .for_each(|boot_prot| {
-                assert!(vcpu.set_sregs(&Default::default()).is_ok());
+                vcpu.set_sregs(&Default::default()).unwrap();
                 setup_sregs(&gm, &vcpu, *boot_prot).unwrap();
 
                 let mut sregs: kvm_sregs = vcpu.get_sregs().unwrap();

src/vmm/src/builder.rs

@@ -349,8 +349,14 @@ pub fn build_microvm_for_boot(
 
     #[cfg(feature = "gdb")]
     if let Some(gdb_socket_path) = &vm_resources.vm_config.gdb_socket_path {
-        gdb::gdb_thread(vmm.clone(), vcpu_fds, gdb_rx, entry_addr, gdb_socket_path)
-            .map_err(GdbServer)?;
+        gdb::gdb_thread(
+            vmm.clone(),
+            vcpu_fds,
+            gdb_rx,
+            entry_point.entry_addr,
+            gdb_socket_path,
+        )
+        .map_err(GdbServer)?;
     } else {
         debug!("No GDB socket provided not starting gdb server.");
     }

src/vmm/src/vstate/vcpu/aarch64.rs

@@ -301,6 +301,7 @@ mod tests {
     use std::os::unix::io::AsRawFd;
 
     use kvm_bindings::{KVM_ARM_VCPU_PSCI_0_2, KVM_REG_SIZE_U64};
+    use vm_memory::GuestAddress;
 
     use super::*;
     use crate::arch::aarch64::regs::Aarch64RegisterRef;
@@ -344,16 +345,15 @@ mod tests {
             cpu_config: CpuConfiguration::default(),
         };
 
-        vcpu
-            .configure(
-                &vm_mem,
-                EntryPoint {
-                    entry_addr: GuestAddress(crate::arch::get_kernel_start()),
-                    protocol: BootProtocol::LinuxBoot,
-                },
-                &vcpu_config,
-            )
-            .unwrap();
+        vcpu.configure(
+            &vm_mem,
+            EntryPoint {
+                entry_addr: GuestAddress(crate::arch::get_kernel_start()),
+                protocol: BootProtocol::LinuxBoot,
+            },
+            &vcpu_config,
+        )
+        .unwrap();
 
         unsafe { libc::close(vcpu.fd.as_raw_fd()) };
 

tests/framework/utils.py

@@ -673,3 +673,8 @@ def __enter__(self):
 
     def __exit__(self, _type, _value, _traceback):
         signal.alarm(0)
+
+
+def pvh_supported() -> bool:
+    """Checks if PVH boot is supported"""
+    return platform.architecture() == "x86_64"

tests/integration_tests/functional/test_api.py

@@ -16,7 +16,7 @@
 
 import host_tools.drive as drive_tools
 import host_tools.network as net_tools
-from framework import utils_cpuid
+from framework import utils, utils_cpuid
 from framework.utils import get_firecracker_version_from_toml, is_io_uring_supported
 
 MEM_LIMIT = 1000000000
@@ -42,6 +42,9 @@ def test_api_happy_start(uvm_plain):
 
     test_microvm.start()
 
+    if utils.pvh_supported():
+        assert "Kernel loaded using PVH boot protocol" in test_microvm.log_data
+
 
 def test_drive_io_engine(uvm_plain):
     """

tests/integration_tests/style/test_gitlint.py

@@ -5,6 +5,7 @@
 import os
 
 from framework import utils
+from framework.ab_test import DEFAULT_A_REVISION
 
 
 def test_gitlint():
@@ -15,6 +16,6 @@ def test_gitlint():
     os.environ["LANG"] = "C.UTF-8"
 
     rc, _, stderr = utils.run_cmd(
-        "gitlint --commits origin/main..HEAD -C ../.gitlint --extra-path framework/gitlint_rules.py",
+        f"gitlint --commits origin/{DEFAULT_A_REVISION}..HEAD -C ../.gitlint --extra-path framework/gitlint_rules.py",
     )
     assert rc == 0, "Commit message violates gitlint rules: {}".format(stderr)