From 643cfb0c113a7bf65abd89a22577e8e0827e1832 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Sat, 30 Jul 2022 11:16:01 -0400 Subject: [PATCH 1/3] xtask: Always allow writing to OVMF vars In the AArch64 version of OVMF currently used in CI, it crashes if the vars aren't writable. (A newer version that I tested locally doesn't have this problem.) Having vars be writable is a more normal UEFI environment anyway though, so remove the special case for AArch64 and make the vars always writable. Since the vars file might be in some read-only location, or owned by root (e.g. a system package installed to /usr), a temporary copy of the vars file is made before using it. Also add a `PflashMode` enum to make the call sites of `add_pflash_args` a little clearer. --- xtask/src/qemu.rs | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/xtask/src/qemu.rs b/xtask/src/qemu.rs index 1647ee9b5..05ac71017 100644 --- a/xtask/src/qemu.rs +++ b/xtask/src/qemu.rs @@ -17,7 +17,6 @@ use tempfile::TempDir; struct OvmfPaths { code: PathBuf, vars: PathBuf, - vars_read_only: bool, } impl OvmfPaths { @@ -26,19 +25,14 @@ impl OvmfPaths { UefiArch::AArch64 => Self { code: dir.join("QEMU_EFI-pflash.raw"), vars: dir.join("vars-template-pflash.raw"), - // The OVMF implementation for AArch64 won't boot unless - // the vars file is writeable. - vars_read_only: false, }, UefiArch::IA32 => Self { code: dir.join("OVMF32_CODE.fd"), vars: dir.join("OVMF32_VARS.fd"), - vars_read_only: true, }, UefiArch::X86_64 => Self { code: dir.join("OVMF_CODE.fd"), vars: dir.join("OVMF_VARS.fd"), - vars_read_only: true, }, } } @@ -84,10 +78,18 @@ impl OvmfPaths { } } -fn add_pflash_args(cmd: &mut Command, file: &Path, read_only: bool) { +enum PflashMode { + ReadOnly, + ReadWrite, +} + +fn add_pflash_args(cmd: &mut Command, file: &Path, mode: PflashMode) { // Build the argument as an OsString to avoid requiring a UTF-8 path. let mut arg = OsString::from("if=pflash,format=raw,readonly="); - arg.push(if read_only { "on" } else { "off" }); + arg.push(match mode { + PflashMode::ReadOnly => "on", + PflashMode::ReadWrite => "off", + }); arg.push(",file="); arg.push(file); @@ -313,10 +315,20 @@ pub fn run_qemu(arch: UefiArch, opt: &QemuOpt) -> Result<()> { } } + let tmp_dir = TempDir::new()?; + let tmp_dir = tmp_dir.path(); + // Set up OVMF. let ovmf_paths = OvmfPaths::find(opt, arch)?; - add_pflash_args(&mut cmd, &ovmf_paths.code, /*read_only=*/ true); - add_pflash_args(&mut cmd, &ovmf_paths.vars, ovmf_paths.vars_read_only); + + // Make a copy of the OVMF vars file so that it can be used + // read+write without modifying the original. Under AArch64, some + // versions of OVMF won't boot if the vars file isn't writeable. + let ovmf_vars = tmp_dir.join("ovmf_vars"); + fs_err::copy(&ovmf_paths.vars, &ovmf_vars)?; + + add_pflash_args(&mut cmd, &ovmf_paths.code, PflashMode::ReadOnly); + add_pflash_args(&mut cmd, &ovmf_vars, PflashMode::ReadWrite); // Mount a local directory as a FAT partition. cmd.arg("-drive"); @@ -331,9 +343,6 @@ pub fn run_qemu(arch: UefiArch, opt: &QemuOpt) -> Result<()> { cmd.args(&["-display", "none"]); } - let tmp_dir = TempDir::new()?; - let tmp_dir = tmp_dir.path(); - let test_disk = tmp_dir.join("test_disk.fat.img"); create_mbr_test_disk(&test_disk)?; From 6a57854f725ac84df231f09b586f479b76fac084 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Sat, 30 Jul 2022 17:35:53 -0400 Subject: [PATCH 2/3] xtask: Add default QEMU executable directory on Windows The QEMU installer for Windows does not automatically add the directory containing the QEMU executables to the PATH. Add the default directory to the PATH to make it more likely that QEMU will be found on Windows. The directory is appended, so if a different directory on the PATH already has the QEMU binary this change won't affect anything. --- xtask/src/qemu.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/xtask/src/qemu.rs b/xtask/src/qemu.rs index 05ac71017..70ac4092d 100644 --- a/xtask/src/qemu.rs +++ b/xtask/src/qemu.rs @@ -11,7 +11,7 @@ use std::ffi::OsString; use std::io::{BufRead, BufReader, Read, Write}; use std::path::{Path, PathBuf}; use std::process::{Child, Command, Stdio}; -use std::thread; +use std::{env, thread}; use tempfile::TempDir; struct OvmfPaths { @@ -265,6 +265,18 @@ pub fn run_qemu(arch: UefiArch, opt: &QemuOpt) -> Result<()> { }; let mut cmd = Command::new(qemu_exe); + if platform::is_windows() { + // The QEMU installer for Windows does not automatically add the + // directory containing the QEMU executables to the PATH. Add + // the default directory to the PATH to make it more likely that + // QEMU will be found on Windows. (The directory is appended, so + // if a different directory on the PATH already has the QEMU + // binary this change won't affect anything.) + let mut path = env::var_os("PATH").unwrap_or_default(); + path.push(r";C:\Program Files\qemu"); + cmd.env("PATH", path); + } + // Disable default devices. // QEMU by defaults enables a ton of devices which slow down boot. cmd.arg("-nodefaults"); From 81f8380ed3e2e4fb6bfe0d1f1d326fa98e089445 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Sat, 30 Jul 2022 17:04:49 -0400 Subject: [PATCH 3/3] xtask: Rework the OVMF search to make it more robust Prior to this commit we assumed that OVMF files for a given guest arch have consistent file names across host operating systems, varying only in the directory path. This turns out not to be a good assumption though; the locations and names of OVMF files installed by system packages very a lot between operating systems. To make it easier to figure out where stuff is, add a number of OS-specific functions that provide the fulls OVMF code and vars paths. For now these functions cover Arch, CentOS, Debian/Ubuntu, Fedora, and Windows. A selection of candidate paths is then collected (one set of paths for Linux, a different set for Windows) and searched. Along with the above changes, the `--ovmf-dir` option has been replaced with separate `--ovmf-code` and `--ovmf-vars` options. When set, these paths will be used instead of searching the candidate paths. The `uefi-test-runner` directory is no longer automatically searched for OVMF files since we no longer assume we know what these files would be named. Hopefully the improved candidate search makes this moot, and the `--ovmf-code`/`--ovmf-vars` options can still be used to search there if desired. CI changes: * The aarch64 job has been simplified a bit since the system location of the files is now sufficient for `cargo xtask run` to work. * The ia32 job now explicitly sets the location of the downloaded OVMF files. Fixes https://github.com/rust-osdev/uefi-rs/issues/469 --- .github/workflows/rust.yml | 9 +- README.md | 3 +- xtask/src/opt.rs | 8 +- xtask/src/qemu.rs | 207 +++++++++++++++++++++++++++++++------ 4 files changed, 184 insertions(+), 43 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 5d9ff4541..80288058f 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -23,9 +23,6 @@ jobs: sudo add-apt-repository ppa:canonical-server/server-backports sudo apt-get update sudo apt-get install qemu-system-arm qemu-efi-aarch64 -y - # Copy the files so that the vars file isn't read-only. - cp /usr/share/AAVMF/AAVMF_CODE.fd uefi-test-runner/QEMU_EFI-pflash.raw - cp /usr/share/AAVMF/AAVMF_VARS.fd uefi-test-runner/vars-template-pflash.raw - name: Install stable uses: actions-rs/toolchain@v1 @@ -102,8 +99,8 @@ jobs: # guard against external changes breaking the CI. EDK2_NIGHTLY_COMMIT: 'ebb83e5475d49418afc32857f66111949928bcdc' run: | - curl -o uefi-test-runner/OVMF32_CODE.fd https://raw.githubusercontent.com/retrage/edk2-nightly/${EDK2_NIGHTLY_COMMIT}/bin/RELEASEIa32_OVMF_CODE.fd - curl -o uefi-test-runner/OVMF32_VARS.fd https://raw.githubusercontent.com/retrage/edk2-nightly/${EDK2_NIGHTLY_COMMIT}/bin/RELEASEIa32_OVMF_VARS.fd + curl -o OVMF32_CODE.fd https://raw.githubusercontent.com/retrage/edk2-nightly/${EDK2_NIGHTLY_COMMIT}/bin/RELEASEIa32_OVMF_CODE.fd + curl -o OVMF32_VARS.fd https://raw.githubusercontent.com/retrage/edk2-nightly/${EDK2_NIGHTLY_COMMIT}/bin/RELEASEIa32_OVMF_VARS.fd - name: Install stable uses: actions-rs/toolchain@v1 @@ -121,7 +118,7 @@ jobs: run: cargo xtask build --target ia32 - name: Run VM tests - run: cargo xtask run --target ia32 --headless --ci + run: cargo xtask run --target ia32 --headless --ci --ovmf-code OVMF32_CODE.fd --ovmf-vars OVMF32_VARS.fd timeout-minutes: 2 test: diff --git a/README.md b/README.md index 4f819422b..750f8fa0c 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,8 @@ Available commands: Especially useful if you want to run the tests under [WSL](https://docs.microsoft.com/en-us/windows/wsl) on Windows. - `--headless`: run QEMU without a GUI - - `--ovmf-dir `: directory in which to look for OVMF files + - `--ovmf-code `: path of an OVMF code file + - `--ovmf-vars `: path of an OVMF vars file - `--release`: build in release mode - `--target {x86_64,ia32,aarch64}`: choose target UEFI arch - `test`: run unit tests and doctests on the host diff --git a/xtask/src/opt.rs b/xtask/src/opt.rs index d0bd0316e..1f5fcc0cf 100644 --- a/xtask/src/opt.rs +++ b/xtask/src/opt.rs @@ -140,9 +140,13 @@ pub struct QemuOpt { #[clap(long, action)] pub headless: bool, - /// Directory in which to look for OVMF files. + /// Path of an OVMF code file. #[clap(long, action)] - pub ovmf_dir: Option, + pub ovmf_code: Option, + + /// Path of an OVMF vars file. + #[clap(long, action)] + pub ovmf_vars: Option, } /// Build uefi-test-runner and run it in QEMU. diff --git a/xtask/src/qemu.rs b/xtask/src/qemu.rs index 70ac4092d..e6c1d4126 100644 --- a/xtask/src/qemu.rs +++ b/xtask/src/qemu.rs @@ -14,67 +14,206 @@ use std::process::{Child, Command, Stdio}; use std::{env, thread}; use tempfile::TempDir; +#[derive(Clone, Copy, Debug)] +enum OvmfFileType { + Code, + Vars, +} + +impl OvmfFileType { + fn as_str(&self) -> &'static str { + match self { + Self::Code => "code", + Self::Vars => "vars", + } + } +} + struct OvmfPaths { code: PathBuf, vars: PathBuf, } impl OvmfPaths { - fn from_dir(dir: &Path, arch: UefiArch) -> Self { + fn get_path(&self, file_type: OvmfFileType) -> &Path { + match file_type { + OvmfFileType::Code => &self.code, + OvmfFileType::Vars => &self.vars, + } + } + + /// Get the Arch Linux OVMF paths for the given guest arch. + fn arch_linux(arch: UefiArch) -> Self { match arch { + // Package "edk2-armvirt". UefiArch::AArch64 => Self { - code: dir.join("QEMU_EFI-pflash.raw"), - vars: dir.join("vars-template-pflash.raw"), + code: "/usr/share/edk2-armvirt/aarch64/QEMU_CODE.fd".into(), + vars: "/usr/share/edk2-armvirt/aarch64/QEMU_VARS.fd".into(), }, + // Package "edk2-ovmf". UefiArch::IA32 => Self { - code: dir.join("OVMF32_CODE.fd"), - vars: dir.join("OVMF32_VARS.fd"), + code: "/usr/share/edk2-ovmf/ia32/OVMF_CODE.fd".into(), + vars: "/usr/share/edk2-ovmf/ia32/OVMF_VARS.fd".into(), }, + // Package "edk2-ovmf". UefiArch::X86_64 => Self { - code: dir.join("OVMF_CODE.fd"), - vars: dir.join("OVMF_VARS.fd"), + code: "/usr/share/edk2-ovmf/x64/OVMF_CODE.fd".into(), + vars: "/usr/share/edk2-ovmf/x64/OVMF_VARS.fd".into(), }, } } - fn exists(&self) -> bool { - self.code.exists() && self.vars.exists() + /// Get the CentOS OVMF paths for the given guest arch. + fn centos_linux(arch: UefiArch) -> Option { + match arch { + // Package "edk2-aarch64". + UefiArch::AArch64 => Some(Self { + code: "/usr/share/edk2/aarch64/QEMU_EFI-pflash.raw".into(), + vars: "/usr/share/edk2/aarch64/vars-template-pflash.raw".into(), + }), + // There's no official ia32 package. + UefiArch::IA32 => None, + // Package "edk2-ovmf". + UefiArch::X86_64 => Some(Self { + // Use the `.secboot` variant because the CentOS package + // doesn't have a plain "OVMF_CODE.fd". + code: "/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd".into(), + vars: "/usr/share/edk2/ovmf/OVMF_VARS.fd".into(), + }), + } } - /// Find path to OVMF files. - fn find(opt: &QemuOpt, arch: UefiArch) -> Result { - // If the path is specified in the settings, use it. - if let Some(ovmf_dir) = &opt.ovmf_dir { - let ovmf_paths = Self::from_dir(ovmf_dir, arch); - if ovmf_paths.exists() { - return Ok(ovmf_paths); - } - bail!("OVMF files not found in {}", ovmf_dir.display()); + /// Get the Debian OVMF paths for the given guest arch. These paths + /// also work on Ubuntu. + fn debian_linux(arch: UefiArch) -> Self { + match arch { + // Package "qemu-efi-aarch64". + UefiArch::AArch64 => Self { + code: "/usr/share/AAVMF/AAVMF_CODE.fd".into(), + vars: "/usr/share/AAVMF/AAVMF_VARS.fd".into(), + }, + // Package "ovmf-ia32". + UefiArch::IA32 => Self { + code: "/usr/share/OVMF/OVMF32_CODE_4M.secboot.fd".into(), + vars: "/usr/share/OVMF/OVMF32_VARS_4M.fd".into(), + }, + // Package "ovmf". + UefiArch::X86_64 => Self { + code: "/usr/share/OVMF/OVMF_CODE.fd".into(), + vars: "/usr/share/OVMF/OVMF_VARS.fd".into(), + }, + } + } + + /// Get the Fedora OVMF paths for the given guest arch. + fn fedora_linux(arch: UefiArch) -> Self { + match arch { + // Package "edk2-aarch64". + UefiArch::AArch64 => Self { + code: "/usr/share/edk2/aarch64/QEMU_EFI-pflash.raw".into(), + vars: "/usr/share/edk2/aarch64/vars-template-pflash.raw".into(), + }, + // Package "edk2-ovmf-ia32". + UefiArch::IA32 => Self { + code: "/usr/share/edk2/ovmf-ia32/OVMF_CODE.fd".into(), + vars: "/usr/share/edk2/ovmf-ia32/OVMF_VARS.fd".into(), + }, + // Package "edk2-ovmf". + UefiArch::X86_64 => Self { + code: "/usr/share/edk2/ovmf/OVMF_CODE.fd".into(), + vars: "/usr/share/edk2/ovmf/OVMF_VARS.fd".into(), + }, } + } - // Check whether the test runner directory contains the files. - let ovmf_dir = Path::new("uefi-test-runner"); - let ovmf_paths = Self::from_dir(ovmf_dir, arch); - if ovmf_paths.exists() { - return Ok(ovmf_paths); + /// Get the Windows OVMF paths for the given guest arch. + fn windows(arch: UefiArch) -> Self { + match arch { + UefiArch::AArch64 => Self { + code: r"C:\Program Files\qemu\share\edk2-aarch64-code.fd".into(), + vars: r"C:\Program Files\qemu\share\edk2-arm-vars.fd".into(), + }, + UefiArch::IA32 => Self { + code: r"C:\Program Files\qemu\share\edk2-i386-code.fd".into(), + vars: r"C:\Program Files\qemu\share\edk2-i386-vars.fd".into(), + }, + UefiArch::X86_64 => Self { + code: r"C:\Program Files\qemu\share\edk2-x86_64-code.fd".into(), + // There's no x86_64 vars file, but the i386 one works. + vars: r"C:\Program Files\qemu\share\edk2-i386-vars.fd".into(), + }, } + } + /// Get candidate paths where OVMF code/vars might exist for the + /// given guest arch and host platform. + fn get_candidate_paths(arch: UefiArch) -> Vec { + let mut candidates = Vec::new(); if platform::is_linux() { - let possible_paths = [ - // Most distros, including CentOS, Fedora, Debian, and Ubuntu. - Path::new("/usr/share/OVMF"), - // Arch Linux. - Path::new("/usr/share/ovmf/x64"), - ]; - for path in possible_paths { - let ovmf_paths = Self::from_dir(path, arch); - if ovmf_paths.exists() { - return Ok(ovmf_paths); + candidates.push(Self::arch_linux(arch)); + if let Some(candidate) = Self::centos_linux(arch) { + candidates.push(candidate); + } + candidates.push(Self::debian_linux(arch)); + candidates.push(Self::fedora_linux(arch)); + } + if platform::is_windows() { + candidates.push(Self::windows(arch)); + } + candidates + } + + /// Search for an OVMF file (either code or vars). + /// + /// If `user_provided_path` is not None, it is always used. An error + /// is returned if the path does not exist. + /// + /// Otherwise, the paths in `candidates` are searched to find one + /// that exists. If none of them exist, an error is returned. + fn find_ovmf_file( + file_type: OvmfFileType, + user_provided_path: &Option, + candidates: &[Self], + ) -> Result { + if let Some(path) = user_provided_path { + // The user provided an exact path to use; verify that it + // exists. + if path.exists() { + Ok(path.to_owned()) + } else { + bail!( + "ovmf {} file does not exist: {}", + file_type.as_str(), + path.display() + ); + } + } else { + for candidate in candidates { + let path = candidate.get_path(file_type); + if path.exists() { + return Ok(path.to_owned()); } } + + bail!( + "no ovmf {} file found in candidates: {:?}", + file_type.as_str(), + candidates + .iter() + .map(|c| c.get_path(file_type)) + .collect::>(), + ); } + } + + /// Find path to OVMF files. + fn find(opt: &QemuOpt, arch: UefiArch) -> Result { + let candidates = Self::get_candidate_paths(arch); + + let code = Self::find_ovmf_file(OvmfFileType::Code, &opt.ovmf_code, &candidates)?; + let vars = Self::find_ovmf_file(OvmfFileType::Vars, &opt.ovmf_vars, &candidates)?; - bail!("OVMF files not found anywhere"); + Ok(Self { code, vars }) } }