From 4fa73bf38700d944d4ada86ad1df806d9a70fdbf Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Thu, 7 Sep 2023 15:01:42 +0100 Subject: [PATCH 1/6] docs: Refer to the correct kernel config to enable PVH on linux Enabling CONFIG_XEN_PVH=y is for booting on the actual Xen hypervisor. To boot using PVH on a non-Xen hypervisor, set CONFIG_PVH=y. Signed-off-by: Patrick Roy --- CHANGELOG.md | 4 ++-- docs/pvh.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee42608b050..ff93c5ebb35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -142,8 +142,8 @@ and this project adheres to 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. + Linux kernels newer than 5.0 compiled with `CONFIG_PVH=y` set this ELF Note, + as do FreeBSD kernels. ### Changed diff --git a/docs/pvh.md b/docs/pvh.md index 4a351c301ec..c65336c4de2 100644 --- a/docs/pvh.md +++ b/docs/pvh.md @@ -7,7 +7,7 @@ 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 +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 From 67ae6a9ad51ec98c581018c2f62c46044efde1fa Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Mon, 11 Nov 2024 12:20:30 +0000 Subject: [PATCH 2/6] doc: move PVH changelog entry The feature branch is so old that during rebase it got moved into the 1.8 section. Move it back to the unreleased section. Signed-off-by: Patrick Roy --- CHANGELOG.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff93c5ebb35..34ebff1a7bc 100644 --- a/CHANGELOG.md +++ b/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 newer than 5.0 compiled with `CONFIG_PVH=y` set this ELF Note, - as do FreeBSD kernels. ### Changed From 3a4057ffb06efb78f6d9638bcce2d656d0fa56d1 Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Mon, 11 Nov 2024 12:26:07 +0000 Subject: [PATCH 3/6] fix: compilation errors due to unrelated firecracker changes With PVH support, we are passing around an `EntryPoint` structure, not just an address. Extract the address from the struct, and pass it to gdb::gdb_thread instead to fix compilation. A aarch64 unittest was missing a `GuestAddress` import. Signed-off-by: Patrick Roy --- src/vmm/src/builder.rs | 2 +- src/vmm/src/vstate/vcpu/aarch64.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vmm/src/builder.rs b/src/vmm/src/builder.rs index c26b1120f69..6462304cfa6 100644 --- a/src/vmm/src/builder.rs +++ b/src/vmm/src/builder.rs @@ -349,7 +349,7 @@ 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) + 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."); diff --git a/src/vmm/src/vstate/vcpu/aarch64.rs b/src/vmm/src/vstate/vcpu/aarch64.rs index 6166f901858..259b3c39e87 100644 --- a/src/vmm/src/vstate/vcpu/aarch64.rs +++ b/src/vmm/src/vstate/vcpu/aarch64.rs @@ -301,7 +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; use crate::arch::BootProtocol; From 18a278ccea4b6620b8a7bfcf9313ed70404f8dd6 Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Mon, 11 Nov 2024 12:30:43 +0000 Subject: [PATCH 4/6] chore: fix various linter and formatter issues Mainly clippy and mdformat have changed since the feature branch was created. Signed-off-by: Patrick Roy --- docs/pvh.md | 8 ++++---- docs/rootfs-and-kernel-setup.md | 20 ++++++++++---------- src/vmm/src/arch/x86_64/gdt.rs | 1 + src/vmm/src/arch/x86_64/mod.rs | 23 ++++++++++++++++------- src/vmm/src/arch/x86_64/regs.rs | 2 +- src/vmm/src/builder.rs | 10 ++++++++-- src/vmm/src/vstate/vcpu/aarch64.rs | 20 ++++++++++---------- 7 files changed, 50 insertions(+), 34 deletions(-) diff --git a/docs/pvh.md b/docs/pvh.md index c65336c4de2..1cef74a2389 100644 --- a/docs/pvh.md +++ b/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_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). diff --git a/docs/rootfs-and-kernel-setup.md b/docs/rootfs-and-kernel-setup.md index a0ead9c36b5..73675af7963 100644 --- a/docs/rootfs-and-kernel-setup.md +++ b/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. diff --git a/src/vmm/src/arch/x86_64/gdt.rs b/src/vmm/src/arch/x86_64/gdt.rs index b34ca8c3d64..b7b99a572ae 100644 --- a/src/vmm/src/arch/x86_64/gdt.rs +++ b/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; diff --git a/src/vmm/src/arch/x86_64/mod.rs b/src/vmm/src/arch/x86_64/mod.rs index a9c2d708a92..05708af7e02 100644 --- a/src/vmm/src/arch/x86_64/mod.rs +++ b/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) diff --git a/src/vmm/src/arch/x86_64/regs.rs b/src/vmm/src/arch/x86_64/regs.rs index cfba8440150..0a4cf630bf3 100644 --- a/src/vmm/src/arch/x86_64/regs.rs +++ b/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(); diff --git a/src/vmm/src/builder.rs b/src/vmm/src/builder.rs index 6462304cfa6..8c51c7a573c 100644 --- a/src/vmm/src/builder.rs +++ b/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_point.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."); } diff --git a/src/vmm/src/vstate/vcpu/aarch64.rs b/src/vmm/src/vstate/vcpu/aarch64.rs index 259b3c39e87..eaa7865c473 100644 --- a/src/vmm/src/vstate/vcpu/aarch64.rs +++ b/src/vmm/src/vstate/vcpu/aarch64.rs @@ -302,6 +302,7 @@ mod tests { 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; use crate::arch::BootProtocol; @@ -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()) }; From c47e01176d9d83d19bdcd79c3f1d37c1266a8a4a Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Mon, 11 Nov 2024 13:04:00 +0000 Subject: [PATCH 5/6] test: verify PVH boot protocol is used Use test_api_happy_start to assert that the log message that indicates usage of PVH boot protocol is present. Only x86_64 guests support PVH boot, and on ARM we do not emit any log messages, so restrict the assertion to x86_64 platforms. Signed-off-by: Patrick Roy --- tests/framework/utils.py | 5 +++++ tests/integration_tests/functional/test_api.py | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/framework/utils.py b/tests/framework/utils.py index a8715f00e94..b9549a819ca 100644 --- a/tests/framework/utils.py +++ b/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" diff --git a/tests/integration_tests/functional/test_api.py b/tests/integration_tests/functional/test_api.py index 5aebe7b5265..424c07fd780 100644 --- a/tests/integration_tests/functional/test_api.py +++ b/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): """ From f84e3475768ea75e86b7846e615be5a1d21087f0 Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Mon, 11 Nov 2024 13:17:02 +0000 Subject: [PATCH 6/6] fix: dont hardcode "main" in gitlint test Hardcoding main means the test looks at the wrong range of commits for feature branches, which can result in problem if feature branches are long-lived. Signed-off-by: Patrick Roy --- tests/integration_tests/style/test_gitlint.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration_tests/style/test_gitlint.py b/tests/integration_tests/style/test_gitlint.py index b47286d33e9..b7ef8631327 100644 --- a/tests/integration_tests/style/test_gitlint.py +++ b/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)