Merge pull request #1031 from nicholasbishop/bishop-fix-output-again

Fix logger connection after opening serial protocol

Author: phip1611

uefi-test-runner/src/main.rs

@@ -7,8 +7,11 @@ extern crate log;
 extern crate alloc;
 
 use alloc::string::ToString;
+use alloc::vec::Vec;
 use uefi::prelude::*;
 use uefi::proto::console::serial::Serial;
+use uefi::proto::device_path::build::{self, DevicePathBuilder};
+use uefi::proto::device_path::messaging::Vendor;
 use uefi::table::boot::MemoryType;
 use uefi::Result;
 use uefi_services::{print, println};
@@ -112,6 +115,33 @@ fn send_request_helper(serial: &mut Serial, request: HostRequest) -> Result {
     }
 }
 
+/// Reconnect the serial device to the output console.
+///
+/// This must be called after opening the serial protocol in exclusive mode, as
+/// that breaks the connection to the console, which in turn prevents logs from
+/// getting to the host.
+fn reconnect_serial_to_console(boot_services: &BootServices, serial_handle: Handle) {
+    let mut storage = Vec::new();
+    // Create a device path that specifies the terminal type.
+    let terminal_guid = if cfg!(target_arch = "aarch64") {
+        Vendor::VT_100
+    } else {
+        Vendor::VT_UTF8
+    };
+    let terminal_device_path = DevicePathBuilder::with_vec(&mut storage)
+        .push(&build::messaging::Vendor {
+            vendor_guid: terminal_guid,
+            vendor_defined_data: &[],
+        })
+        .unwrap()
+        .finalize()
+        .unwrap();
+
+    boot_services
+        .connect_controller(serial_handle, None, Some(terminal_device_path), true)
+        .expect("failed to reconnect serial to console");
+}
+
 /// Send the `request` string to the host via the `serial` device, then
 /// wait up to 10 seconds to receive a reply. Returns an error if the
 /// reply is not `"OK\n"`.
@@ -145,7 +175,7 @@ fn send_request_to_host(bt: &BootServices, request: HostRequest) {
     // device, which was broken when we opened the protocol in exclusive
     // mode above.
     drop(serial);
-    let _ = bt.connect_controller(serial_handle, None, None, true);
+    reconnect_serial_to_console(bt, serial_handle);
 
     if let Err(err) = res {
         panic!("request failed: \"{request:?}\": {:?}", err.status());
@@ -156,13 +186,18 @@ fn shutdown(mut st: SystemTable<Boot>) -> ! {
     // Get our text output back.
     st.stdout().reset(false).unwrap();
 
-    info!("Testing complete, shutting down...");
-
     // Tell the host that tests are done. We are about to exit boot
     // services, so we can't easily communicate with the host any later
     // than this.
     send_request_to_host(st.boot_services(), HostRequest::TestsComplete);
 
+    // Send a special log to the host so that we can verify that logging works
+    // up until exiting boot services. See `reconnect_serial_to_console` for the
+    // type of regression this prevents.
+    info!("LOGGING_STILL_WORKING_RIGHT_BEFORE_EBS");
+
+    info!("Testing complete, shutting down...");
+
     // Exit boot services as a proof that it works :)
     let (st, _iter) = st.exit_boot_services(MemoryType::LOADER_DATA);
 

uefi-test-runner/src/proto/console/serial.rs

@@ -1,3 +1,4 @@
+use crate::reconnect_serial_to_console;
 use uefi::proto::console::serial::{ControlBits, Serial};
 use uefi::table::boot::BootServices;
 use uefi::{Result, ResultExt, Status};
@@ -66,7 +67,7 @@ pub unsafe fn test(bt: &BootServices) {
     // device, which was broken when we opened the protocol in exclusive
     // mode above.
     drop(serial);
-    let _ = bt.connect_controller(handle, None, None, true);
+    reconnect_serial_to_console(bt, handle);
 
     if let Err(err) = res {
         panic!("serial test failed: {:?}", err.status());

uefi/CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Added
 - Implemented `PartialEq<char>` for `Char8` and `Char16`.
 - Added `CStr16::from_char16_with_nul` and `Char16::from_char16_with_nul_unchecked`.
+- Added terminal GUID constants to `device_path::messaging::Vendor`.
 
 ## Changed
 - `DevicePath::to_string` and `DevicePathNode::to_string` now return

uefi/src/proto/device_path/device_path_gen.rs

@@ -2413,6 +2413,17 @@ pub mod messaging {
 
     }
 
+    impl Vendor {
+        /// PC-ANSI terminal GUID.
+        pub const PC_ANSI: Guid = guid!("e0c14753-f9be-11d2-9a0c-0090273fc14d");
+        /// VT-100 terminal GUID.
+        pub const VT_100: Guid = guid!("dfa66065-b419-11d3-9a2d-0090273fc14d");
+        /// VT-100+ terminal GUID.
+        pub const VT_100_PLUS: Guid = guid!("7baec70b-57e0-4c76-8e87-2f9e28088343");
+        /// VT-UTF8 terminal GUID.
+        pub const VT_UTF8: Guid = guid!("ad15a0d6-8bec-4acf-a073-d01de77e2d88");
+    }
+
     newtype_enum! { # [doc = " iSCSI network protocol."] pub enum IscsiProtocol : u16 => { # [doc = " TCP."] TCP = 0x0000 , }
 
     }

xtask/src/device_path/spec.rs

@@ -686,6 +686,20 @@ mod messaging {
         vendor_defined_data: [u8],
     }
 
+    impl Vendor {
+        /// PC-ANSI terminal GUID.
+        pub const PC_ANSI: Guid = guid!("e0c14753-f9be-11d2-9a0c-0090273fc14d");
+
+        /// VT-100 terminal GUID.
+        pub const VT_100: Guid = guid!("dfa66065-b419-11d3-9a2d-0090273fc14d");
+
+        /// VT-100+ terminal GUID.
+        pub const VT_100_PLUS: Guid = guid!("7baec70b-57e0-4c76-8e87-2f9e28088343");
+
+        /// VT-UTF8 terminal GUID.
+        pub const VT_UTF8: Guid = guid!("ad15a0d6-8bec-4acf-a073-d01de77e2d88");
+    }
+
     // The spec defines a couple specific messaging-vendor types here,
     // one for UART and one for SAS. These are sort of subclasses of
     // `messaging::Vendor` so they don't quite fit the usual pattern of

xtask/src/qemu.rs

@@ -311,6 +311,7 @@ impl Io {
 
 fn process_qemu_io(mut monitor_io: Io, mut serial_io: Io, tmp_dir: &Path) -> Result<()> {
     let mut tests_complete = false;
+    let mut logging_still_working_right_before_ebs = false;
 
     // This regex is used to detect and strip ANSI escape codes. These
     // escapes are added by the console output protocol when writing to
@@ -371,6 +372,11 @@ fn process_qemu_io(mut monitor_io: Io, mut serial_io: Io, tmp_dir: &Path) -> Res
             tests_complete = true;
 
             reply_ok()?;
+        } else if line.ends_with("LOGGING_STILL_WORKING_RIGHT_BEFORE_EBS") {
+            // The app sends this right before calling
+            // `exit_boot_services`. This serves as a test that we didn't break
+            // logging by opening the serial device in exclusive mode.
+            logging_still_working_right_before_ebs = true;
         } else {
             println!("{line}");
         }
@@ -380,6 +386,10 @@ fn process_qemu_io(mut monitor_io: Io, mut serial_io: Io, tmp_dir: &Path) -> Res
         bail!("tests did not complete successfully");
     }
 
+    if !logging_still_working_right_before_ebs {
+        bail!("logging stopped working sometime before exiting boot services");
+    }
+
     Ok(())
 }