ohcl_boot: Always enable logging ASAP (#1876)

smalis-msft · web-flow · commit af84210fc98c · 2025-08-21T11:32:38.000-07:00
There's no reason not to have it. This also removes the configuration of
logging and the ability to disable it, since we still only have the one
output backend.
diff --git a/openhcl/openhcl_boot/src/boot_logger.rs b/openhcl/openhcl_boot/src/boot_logger.rs
@@ -19,12 +19,6 @@ use core::fmt::Write;
 use minimal_rt::arch::InstrIoAccess;
 use minimal_rt::arch::Serial;
 
-/// The logging type to use.
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-pub enum LoggerType {
-    Serial,
-}
-
 enum Logger {
     #[cfg(target_arch = "x86_64")]
     Serial(Serial<InstrIoAccess>),
@@ -55,19 +49,16 @@ pub static BOOT_LOGGER: BootLogger = BootLogger {
 };
 
 /// Initialize the boot logger. This replaces any previous init calls.
-///
-/// If a given `logger_type` is unavailable on a given isolation type, the
-/// logger will ignore it, and no logging will be initialized.
-pub fn boot_logger_init(isolation_type: IsolationType, logger_type: LoggerType) {
+pub fn boot_logger_init(isolation_type: IsolationType, com3_serial_available: bool) {
     let mut logger = BOOT_LOGGER.logger.borrow_mut();
 
-    *logger = match (isolation_type, logger_type) {
+    *logger = match (isolation_type, com3_serial_available) {
         #[cfg(target_arch = "x86_64")]
-        (IsolationType::None, LoggerType::Serial) => Logger::Serial(Serial::init(InstrIoAccess)),
+        (IsolationType::None, true) => Logger::Serial(Serial::init(InstrIoAccess)),
         #[cfg(target_arch = "aarch64")]
-        (IsolationType::None, LoggerType::Serial) => Logger::Serial(Serial::init()),
+        (IsolationType::None, true) => Logger::Serial(Serial::init()),
         #[cfg(target_arch = "x86_64")]
-        (IsolationType::Tdx, LoggerType::Serial) => Logger::TdxSerial(Serial::init(TdxIoAccess)),
+        (IsolationType::Tdx, true) => Logger::TdxSerial(Serial::init(TdxIoAccess)),
         _ => Logger::None,
     };
 }
diff --git a/openhcl/openhcl_boot/src/cmdline.rs b/openhcl/openhcl_boot/src/cmdline.rs
@@ -3,16 +3,8 @@
 
 //! Command line arguments and parsing for openhcl_boot.
 
-use crate::boot_logger::LoggerType;
 use underhill_confidentiality::OPENHCL_CONFIDENTIAL_DEBUG_ENV_VAR_NAME;
 
-/// Enable boot logging in the bootloader.
-///
-/// Format of `OPENHCL_BOOT_LOG=<logger>`, with valid loggers being:
-///     - `com3`: use the com3 serial port, available on no isolation or Tdx.
-const BOOT_LOG: &str = "OPENHCL_BOOT_LOG=";
-const SERIAL_LOGGER: &str = "com3";
-
 /// Enable the private VTL2 GPA pool for page allocations. This is only enabled
 /// via the command line, because in order to support the VTL2 GPA pool
 /// generically, the boot shim must read serialized data from the previous
@@ -36,7 +28,6 @@ const SIDECAR: &str = "OPENHCL_SIDECAR=";
 
 #[derive(Debug, PartialEq)]
 pub struct BootCommandLineOptions {
-    pub logger: Option<LoggerType>,
     pub confidential_debug: bool,
     pub enable_vtl2_gpa_pool: Option<u64>,
     pub sidecar: bool,
@@ -46,7 +37,6 @@ pub struct BootCommandLineOptions {
 impl BootCommandLineOptions {
     pub const fn new() -> Self {
         BootCommandLineOptions {
-            logger: None,
             confidential_debug: false,
             enable_vtl2_gpa_pool: None,
             sidecar: true, // sidecar is enabled by default
@@ -59,18 +49,10 @@ impl BootCommandLineOptions {
     /// Parse arguments from a command line.
     pub fn parse(&mut self, cmdline: &str) {
         for arg in cmdline.split_whitespace() {
-            if arg.starts_with(BOOT_LOG) {
-                if let Some(SERIAL_LOGGER) = arg.split_once('=').map(|(_, arg)| arg) {
-                    self.logger = Some(LoggerType::Serial)
-                }
-            } else if arg.starts_with(OPENHCL_CONFIDENTIAL_DEBUG_ENV_VAR_NAME) {
+            if arg.starts_with(OPENHCL_CONFIDENTIAL_DEBUG_ENV_VAR_NAME) {
                 let arg = arg.split_once('=').map(|(_, arg)| arg);
                 if arg.is_some_and(|a| a != "0") {
                     self.confidential_debug = true;
-                    // Explicit logger specification overrides this default.
-                    if self.logger.is_none() {
-                        self.logger = Some(LoggerType::Serial);
-                    }
                 }
             } else if arg.starts_with(ENABLE_VTL2_GPA_POOL) {
                 self.enable_vtl2_gpa_pool = arg.split_once('=').and_then(|(_, arg)| {
@@ -105,59 +87,6 @@ mod tests {
         options
     }
 
-    #[test]
-    fn test_console_parsing() {
-        assert_eq!(
-            parse_boot_command_line("OPENHCL_BOOT_LOG=com3"),
-            BootCommandLineOptions {
-                logger: Some(LoggerType::Serial),
-                ..BootCommandLineOptions::new()
-            }
-        );
-
-        assert_eq!(
-            parse_boot_command_line("OPENHCL_BOOT_LOG=1"),
-            BootCommandLineOptions {
-                logger: None,
-                ..BootCommandLineOptions::new()
-            }
-        );
-
-        assert_eq!(
-            parse_boot_command_line("OPENHCL_BOOT_LOG=random"),
-            BootCommandLineOptions {
-                logger: None,
-                ..BootCommandLineOptions::new()
-            }
-        );
-
-        assert_eq!(
-            parse_boot_command_line("OPENHCL_BOOT_LOG==com3"),
-            BootCommandLineOptions {
-                logger: None,
-                ..BootCommandLineOptions::new()
-            }
-        );
-
-        assert_eq!(
-            parse_boot_command_line("OPENHCL_BOOT_LOGserial"),
-            BootCommandLineOptions {
-                logger: None,
-                ..BootCommandLineOptions::new()
-            }
-        );
-
-        let cmdline = format!("{OPENHCL_CONFIDENTIAL_DEBUG_ENV_VAR_NAME}=1");
-        assert_eq!(
-            parse_boot_command_line(&cmdline),
-            BootCommandLineOptions {
-                logger: Some(LoggerType::Serial),
-                confidential_debug: true,
-                ..BootCommandLineOptions::new()
-            }
-        );
-    }
-
     #[test]
     fn test_vtl2_gpa_pool_parsing() {
         assert_eq!(
diff --git a/openhcl/openhcl_boot/src/main.rs b/openhcl/openhcl_boot/src/main.rs
@@ -30,7 +30,6 @@ use crate::hypercall::hvcall;
 use crate::single_threaded::off_stack;
 use arrayvec::ArrayString;
 use arrayvec::ArrayVec;
-use boot_logger::LoggerType;
 use cmdline::BootCommandLineOptions;
 use core::fmt::Write;
 use dt::BootTimes;
@@ -652,6 +651,8 @@ fn shim_main(shim_params_raw_offset: isize) -> ! {
         enable_enlightened_panic();
     }
 
+    let boot_reftime = get_ref_time(p.isolation_type);
+
     // The support code for the fast hypercalls does not set
     // the Guest ID if it is not set yet as opposed to the slow
     // hypercall code path where that is done automatically.
@@ -662,24 +663,16 @@ fn shim_main(shim_params_raw_offset: isize) -> ! {
         hvcall().initialize();
     }
 
-    // Enable early log output if requested in the static command line.
-    // Also check for confidential debug mode if we're isolated.
     let mut static_options = BootCommandLineOptions::new();
     if let Some(cmdline) = p.command_line().command_line() {
         static_options.parse(cmdline);
     }
-    if let Some(typ) = static_options.logger {
-        boot_logger_init(p.isolation_type, typ);
-        log!("openhcl_boot: early debugging enabled");
-    }
 
     let hw_debug_bit = get_hw_debug_bit(p.isolation_type);
     let can_trust_host = p.isolation_type == IsolationType::None
         || static_options.confidential_debug
         || hw_debug_bit;
 
-    let boot_reftime = get_ref_time(p.isolation_type);
-
     let mut dt_storage = off_stack!(PartitionInfo, PartitionInfo::new());
     let partition_info =
         match PartitionInfo::read_from_dt(&p, &mut dt_storage, static_options, can_trust_host) {
@@ -688,6 +681,11 @@ fn shim_main(shim_params_raw_offset: isize) -> ! {
             Err(e) => panic!("unable to read device tree params {}", e),
         };
 
+    // Enable logging ASAP. This is fine even when isolated, as we don't have
+    // any access to secrets in the boot shim.
+    boot_logger_init(p.isolation_type, partition_info.com3_serial_available);
+    log!("openhcl_boot: logging enabled");
+
     // Confidential debug will show up in boot_options only if included in the
     // static command line, or if can_trust_host is true (so the dynamic command
     // line has been parsed).
@@ -724,20 +722,6 @@ fn shim_main(shim_params_raw_offset: isize) -> ! {
         partition_info.vtl0_alias_map = None;
     }
 
-    if can_trust_host {
-        // Enable late log output if requested in the dynamic command line.
-        // Confidential debug is only allowed in the static command line.
-        if let Some(typ) = partition_info.boot_options.logger {
-            boot_logger_init(p.isolation_type, typ);
-        } else if partition_info.com3_serial_available && cfg!(target_arch = "x86_64") {
-            // If COM3 is available and we can trust the host, enable log output even
-            // if it wasn't otherwise requested.
-            boot_logger_init(p.isolation_type, LoggerType::Serial);
-        }
-    }
-
-    log!("openhcl_boot: entered shim_main");
-
     if partition_info.cpus.is_empty() {
         panic!("no cpus");
     }
