Merge pull request #651 from nicholasbishop/bishop-fix-windows-tests

Various fixes for tests under Windows

Author: phip1611

.github/workflows/rust.yml

@@ -141,12 +141,19 @@ jobs:
     name: Check that the build works on a Windows target
     runs-on: windows-latest
     steps:
+      - name: Install QEMU
+        run: choco install qemu
+
       - name: Checkout sources
         uses: actions/checkout@v3
 
       - name: Build
         run: cargo xtask build
 
+      - name: Run VM tests
+        run: cargo xtask run --target x86_64 --ci
+        timeout-minutes: 2
+
   # Run the build with our current nightly MSRV (specified in
   # ./msrv_toolchain.toml). This serves to check that we don't
   # accidentally start relying on a new feature without intending

uefi-test-runner/Cargo.toml

@@ -15,7 +15,11 @@ log = { version = "0.4.17", default-features = false }
 qemu-exit = "3.0.0"
 
 [features]
-# This feature should only be enabled in our CI, it disables some tests
-# which currently fail in that environment (see #103 for discussion).
-ci = []
+# Enable the multiprocessor test. This only works if KVM is enabled.
+multi_processor = []
 
+# Enable the TPM v1 test.
+tpm_v1 = []
+
+# Enable the TPM v2 test.
+tpm_v2 = []

uefi-test-runner/src/proto/pi/mp.rs

@@ -9,8 +9,10 @@ use uefi::Status;
 const NUM_CPUS: usize = 4;
 
 pub fn test(bt: &BootServices) {
-    // These tests break CI. See #103.
-    if cfg!(feature = "ci") {
+    // Skip the test if the `multi_processing` feature is not
+    // enabled. This toggle is needed because the test only works with
+    // KVM, which is not available in CI or on Windows.
+    if cfg!(not(feature = "multi_processor")) {
         return;
     }
 

uefi-test-runner/src/proto/tcg.rs

@@ -46,16 +46,16 @@ fn tcg_v1_read_pcr(tcg: &mut v1::Tcg, pcr_index: PcrIndex) -> v1::Sha1Digest {
 }
 
 fn test_tcg_v1(bt: &BootServices) {
+    // Skip the test of the `tpm_v1` feature is not enabled.
+    if cfg!(not(feature = "tpm_v1")) {
+        return;
+    }
+
     info!("Running TCG v1 test");
 
-    let handle = if let Ok(handle) = bt.get_handle_for_protocol::<v1::Tcg>() {
-        handle
-    } else if cfg!(all(feature = "ci", target_arch = "x86_64")) {
-        panic!("TPM v1 is required on x86_64 CI");
-    } else {
-        info!("No TCG handle found");
-        return;
-    };
+    let handle = bt
+        .get_handle_for_protocol::<v1::Tcg>()
+        .expect("no TCG handle found");
 
     let mut tcg = bt
         .open_protocol_exclusive::<v1::Tcg>(handle)
@@ -215,16 +215,16 @@ fn tcg_v2_read_pcr_8(tcg: &mut v2::Tcg) -> v1::Sha1Digest {
 }
 
 pub fn test_tcg_v2(bt: &BootServices) {
+    // Skip the test of the `tpm_v2` feature is not enabled.
+    if cfg!(not(feature = "tpm_v2")) {
+        return;
+    }
+
     info!("Running TCG v2 test");
 
-    let handle = if let Ok(handle) = bt.get_handle_for_protocol::<v2::Tcg>() {
-        handle
-    } else if cfg!(all(feature = "ci", target_arch = "x86")) {
-        panic!("TPM v2 is required on x86 (32-bit) CI");
-    } else {
-        info!("No TCG handle found");
-        return;
-    };
+    let handle = bt
+        .get_handle_for_protocol::<v2::Tcg>()
+        .expect("no TCG handle found");
 
     let mut tcg = bt
         .open_protocol_exclusive::<v2::Tcg>(handle)

xtask/src/cargo.rs

@@ -58,7 +58,9 @@ pub enum Feature {
     ServicesLogger,
 
     // `uefi-test-runner` features.
-    Ci,
+    MultiProcessor,
+    TpmV1,
+    TpmV2,
 }
 
 impl Feature {
@@ -74,7 +76,9 @@ impl Feature {
             Self::Qemu => "uefi-services/qemu",
             Self::ServicesLogger => "uefi-services/logger",
 
-            Self::Ci => "uefi-test-runner/ci",
+            Self::MultiProcessor => "uefi-test-runner/multi_processor",
+            Self::TpmV1 => "uefi-test-runner/tpm_v1",
+            Self::TpmV2 => "uefi-test-runner/tpm_v2",
         }
     }
 
@@ -89,7 +93,7 @@ impl Feature {
                 Self::Unstable,
             ],
             Package::UefiServices => vec![Self::PanicHandler, Self::Qemu, Self::ServicesLogger],
-            Package::UefiTestRunner => vec![Self::Ci],
+            Package::UefiTestRunner => vec![Self::MultiProcessor, Self::TpmV1, Self::TpmV2],
             _ => vec![],
         }
     }

xtask/src/main.rs

@@ -15,7 +15,7 @@ use anyhow::Result;
 use cargo::{fix_nested_cargo_env, Cargo, CargoAction, Feature, Package, TargetTypes};
 use clap::Parser;
 use itertools::Itertools;
-use opt::{Action, BuildOpt, ClippyOpt, DocOpt, Opt, QemuOpt};
+use opt::{Action, BuildOpt, ClippyOpt, DocOpt, Opt, QemuOpt, TpmVersion};
 use std::process::Command;
 use tempfile::TempDir;
 use util::{command_to_string, run_cmd};
@@ -122,11 +122,17 @@ fn run_miri() -> Result<()> {
 fn run_vm_tests(opt: &QemuOpt) -> Result<()> {
     let mut features = vec![];
 
-    // Always enable the ci feature when not building on Linux so that
-    // the MP test is skipped. That test doesn't work with kvm disabled
-    // (see https://github.com/rust-osdev/uefi-rs/issues/103).
-    if opt.ci || !platform::is_linux() {
-        features.push(Feature::Ci);
+    // Enable TPM tests if a TPM device is present.
+    match opt.tpm {
+        Some(TpmVersion::V1) => features.push(Feature::TpmV1),
+        Some(TpmVersion::V2) => features.push(Feature::TpmV2),
+        None => {}
+    }
+
+    // Enable the multi-processor test if KVM is available. KVM is
+    // available on Linux generally, but not in our CI.
+    if platform::is_linux() && !opt.ci {
+        features.push(Feature::MultiProcessor);
     }
 
     // Build uefi-test-runner.

xtask/src/qemu.rs

@@ -520,10 +520,11 @@ pub fn run_qemu(arch: UefiArch, opt: &QemuOpt) -> Result<()> {
     cmd.arg(drive_arg);
 
     let qemu_monitor_pipe = Pipe::new(tmp_dir, "qemu-monitor")?;
+    let serial_pipe = Pipe::new(tmp_dir, "serial")?;
 
     // Open a serial device connected to stdio. This is used for
     // printing logs and to receive and reply to commands.
-    cmd.args(["-serial", "stdio"]);
+    cmd.args(["-serial", serial_pipe.qemu_arg()]);
 
     // Map the QEMU monitor to a pair of named pipes
     cmd.args(["-qmp", qemu_monitor_pipe.qemu_arg()]);
@@ -556,14 +557,11 @@ pub fn run_qemu(arch: UefiArch, opt: &QemuOpt) -> Result<()> {
     let mut child = ChildWrapper(cmd.spawn().context("failed to launch qemu")?);
 
     let monitor_io = qemu_monitor_pipe.open_io()?;
-    let child_io = Io::new(
-        child.0.stdout.take().unwrap(),
-        child.0.stdin.take().unwrap(),
-    );
+    let serial_io = serial_pipe.open_io()?;
 
     // Capture the result to check it, but first wait for the child to
     // exit.
-    let res = process_qemu_io(monitor_io, child_io, tmp_dir);
+    let res = process_qemu_io(monitor_io, serial_io, tmp_dir);
     let status = child.0.wait()?;
 
     if let Some(echo_service) = echo_service {