svsm: permit early initialization of console logging

On platforms that do not require extensive platform setup (e.g. GHCB)
for console output services, initialize the console logger as early as
possible to facilitate earlier logging of initialization activity.

Signed-off-by: Jon Lange <[email protected]>

kernel/src/console.rs

@@ -30,7 +30,9 @@ static CONSOLE_INITIALIZED: ImmutAfterInitCell<bool> = ImmutAfterInitCell::new(f
 
 pub fn init_console(writer: &'static dyn Terminal) -> ImmutAfterInitResult<()> {
     WRITER.lock().writer = writer;
-    CONSOLE_INITIALIZED.reinit(&true)
+    CONSOLE_INITIALIZED.reinit(&true)?;
+    log::info!("COCONUT Secure Virtual Machine Service Module");
+    Ok(())
 }
 
 #[doc(hidden)]

kernel/src/error.rs

@@ -70,6 +70,8 @@ pub enum SvsmError {
     InvalidBytes,
     /// Errors related to firmware parsing
     Firmware,
+    /// Errors related to console operation
+    Console,
     /// Errors related to firmware configuration contents
     FwCfg(FwCfgError),
     /// Errors related to ACPI parsing.

kernel/src/platform/mod.rs

@@ -44,11 +44,11 @@ pub enum PageStateChangeOp {
 /// underlying architectures.
 pub trait SvsmPlatform {
     /// Performs basic early initialization of the runtime environment.
-    fn env_setup(&mut self);
+    fn env_setup(&mut self, debug_serial_port: u16) -> Result<(), SvsmError>;
 
     /// Performs initialization of the platform runtime environment after
-    /// console logging has been initialized.
-    fn env_setup_late(&mut self);
+    /// the core system environment has been initialized.
+    fn env_setup_late(&mut self, debug_serial_port: u16) -> Result<(), SvsmError>;
 
     /// Completes initialization of a per-CPU object during construction.
     fn setup_percpu(&self, cpu: &PerCpu) -> Result<(), SvsmError>;
@@ -62,8 +62,9 @@ pub trait SvsmPlatform {
     /// Establishes state required for guest/host communication.
     fn setup_guest_host_comm(&mut self, cpu: &PerCpu, is_bsp: bool);
 
-    /// Obtains a console I/O port reference.
-    fn get_console_io_port(&self) -> &'static dyn IOPort;
+    /// Obtains a reference to an I/O port implemetation appropriate to the
+    /// platform.
+    fn get_io_port(&self) -> &'static dyn IOPort;
 
     /// Performs a page state change between private and shared states.
     fn page_state_change(

kernel/src/platform/native.rs

@@ -5,16 +5,21 @@
 // Author: Jon Lange <[email protected]>
 
 use crate::address::{PhysAddr, VirtAddr};
+use crate::console::init_console;
 use crate::cpu::cpuid::CpuidResult;
 use crate::cpu::msr::write_msr;
 use crate::cpu::percpu::PerCpu;
 use crate::error::SvsmError;
-use crate::platform::{IOPort, PageEncryptionMasks, PageStateChangeOp, SvsmPlatform};
+use crate::io::IOPort;
+use crate::platform::{PageEncryptionMasks, PageStateChangeOp, SvsmPlatform};
+use crate::serial::SerialPort;
 use crate::svsm_console::NativeIOPort;
 use crate::types::PageSize;
+use crate::utils::immut_after_init::ImmutAfterInitCell;
 use crate::utils::MemoryRegion;
 
 static CONSOLE_IO: NativeIOPort = NativeIOPort::new();
+static CONSOLE_SERIAL: ImmutAfterInitCell<SerialPort<'_>> = ImmutAfterInitCell::uninit();
 
 const APIC_MSR_ICR: u32 = 0x830;
 
@@ -34,8 +39,19 @@ impl Default for NativePlatform {
 }
 
 impl SvsmPlatform for NativePlatform {
-    fn env_setup(&mut self) {}
-    fn env_setup_late(&mut self) {}
+    fn env_setup(&mut self, debug_serial_port: u16) -> Result<(), SvsmError> {
+        // In the native platform, console output does not require the use of
+        // any platform services, so it can be initialized immediately.
+        CONSOLE_SERIAL
+            .init(&SerialPort::new(&CONSOLE_IO, debug_serial_port))
+            .map_err(|_| SvsmError::Console)?;
+        (*CONSOLE_SERIAL).init();
+        init_console(&*CONSOLE_SERIAL).map_err(|_| SvsmError::Console)
+    }
+
+    fn env_setup_late(&mut self, _debug_serial_port: u16) -> Result<(), SvsmError> {
+        Ok(())
+    }
 
     fn setup_percpu(&self, _cpu: &PerCpu) -> Result<(), SvsmError> {
         Ok(())
@@ -58,7 +74,7 @@ impl SvsmPlatform for NativePlatform {
 
     fn setup_guest_host_comm(&mut self, _cpu: &PerCpu, _is_bsp: bool) {}
 
-    fn get_console_io_port(&self) -> &'static dyn IOPort {
+    fn get_io_port(&self) -> &'static dyn IOPort {
         &CONSOLE_IO
     }
 

kernel/src/platform/snp.rs

@@ -5,12 +5,14 @@
 // Author: Jon Lange <[email protected]>
 
 use crate::address::{PhysAddr, VirtAddr};
+use crate::console::init_console;
 use crate::cpu::cpuid::cpuid_table;
 use crate::cpu::percpu::{current_ghcb, PerCpu};
 use crate::error::ApicError::Registration;
 use crate::error::SvsmError;
 use crate::io::IOPort;
 use crate::platform::{PageEncryptionMasks, PageStateChangeOp, SvsmPlatform};
+use crate::serial::SerialPort;
 use crate::sev::hv_doorbell::current_hv_doorbell;
 use crate::sev::msr_protocol::{hypervisor_ghcb_features, verify_ghcb_version, GHCBHvFeatures};
 use crate::sev::status::vtom_enabled;
@@ -19,11 +21,13 @@ use crate::sev::{
 };
 use crate::svsm_console::SVSMIOPort;
 use crate::types::PageSize;
+use crate::utils::immut_after_init::ImmutAfterInitCell;
 use crate::utils::MemoryRegion;
 
 use core::sync::atomic::{AtomicU32, Ordering};
 
 static CONSOLE_IO: SVSMIOPort = SVSMIOPort::new();
+static CONSOLE_SERIAL: ImmutAfterInitCell<SerialPort<'_>> = ImmutAfterInitCell::uninit();
 
 static APIC_EMULATION_REG_COUNT: AtomicU32 = AtomicU32::new(0);
 
@@ -43,13 +47,20 @@ impl Default for SnpPlatform {
 }
 
 impl SvsmPlatform for SnpPlatform {
-    fn env_setup(&mut self) {
+    fn env_setup(&mut self, _debug_serial_port: u16) -> Result<(), SvsmError> {
         sev_status_init();
+        Ok(())
     }
 
-    fn env_setup_late(&mut self) {
+    fn env_setup_late(&mut self, debug_serial_port: u16) -> Result<(), SvsmError> {
+        CONSOLE_SERIAL
+            .init(&SerialPort::new(&CONSOLE_IO, debug_serial_port))
+            .map_err(|_| SvsmError::Console)?;
+        (*CONSOLE_SERIAL).init();
+        init_console(&*CONSOLE_SERIAL).map_err(|_| SvsmError::Console)?;
         sev_status_verify();
-        init_hypervisor_ghcb_features().expect("Failed to obtain hypervisor GHCB features");
+        init_hypervisor_ghcb_features()?;
+        Ok(())
     }
 
     fn setup_percpu(&self, cpu: &PerCpu) -> Result<(), SvsmError> {
@@ -102,7 +113,7 @@ impl SvsmPlatform for SnpPlatform {
         cpu.register_ghcb().expect("Failed to register GHCB");
     }
 
-    fn get_console_io_port(&self) -> &'static dyn IOPort {
+    fn get_io_port(&self) -> &'static dyn IOPort {
         &CONSOLE_IO
     }
 

kernel/src/platform/tdp.rs

@@ -5,16 +5,22 @@
 // Author: Peter Fang <[email protected]>
 
 use crate::address::{PhysAddr, VirtAddr};
+use crate::console::init_console;
 use crate::cpu::cpuid::CpuidResult;
 use crate::cpu::percpu::PerCpu;
 use crate::error::SvsmError;
 use crate::io::IOPort;
 use crate::platform::{PageEncryptionMasks, PageStateChangeOp, SvsmPlatform};
+use crate::serial::SerialPort;
 use crate::svsm_console::SVSMIOPort;
 use crate::types::PageSize;
+use crate::utils::immut_after_init::ImmutAfterInitCell;
 use crate::utils::MemoryRegion;
 
+// FIXME - SVSMIOPort doesn't work on TDP, but the platform does not yet have
+// an alternative available.
 static CONSOLE_IO: SVSMIOPort = SVSMIOPort::new();
+static CONSOLE_SERIAL: ImmutAfterInitCell<SerialPort<'_>> = ImmutAfterInitCell::uninit();
 
 #[derive(Clone, Copy, Debug)]
 pub struct TdpPlatform {}
@@ -32,9 +38,17 @@ impl Default for TdpPlatform {
 }
 
 impl SvsmPlatform for TdpPlatform {
-    fn env_setup(&mut self) {}
+    fn env_setup(&mut self, _debug_serial_port: u16) -> Result<(), SvsmError> {
+        Ok(())
+    }
 
-    fn env_setup_late(&mut self) {}
+    fn env_setup_late(&mut self, debug_serial_port: u16) -> Result<(), SvsmError> {
+        CONSOLE_SERIAL
+            .init(&SerialPort::new(&CONSOLE_IO, debug_serial_port))
+            .map_err(|_| SvsmError::Console)?;
+        (*CONSOLE_SERIAL).init();
+        init_console(&*CONSOLE_SERIAL).map_err(|_| SvsmError::Console)
+    }
 
     fn setup_percpu(&self, _cpu: &PerCpu) -> Result<(), SvsmError> {
         Err(SvsmError::Tdx)
@@ -57,7 +71,7 @@ impl SvsmPlatform for TdpPlatform {
 
     fn setup_guest_host_comm(&mut self, _cpu: &PerCpu, _is_bsp: bool) {}
 
-    fn get_console_io_port(&self) -> &'static dyn IOPort {
+    fn get_io_port(&self) -> &'static dyn IOPort {
         &CONSOLE_IO
     }
 

kernel/src/stage2.rs

@@ -19,7 +19,7 @@ use cpuarch::snp_cpuid::SnpCpuidTable;
 use elf::ElfError;
 use svsm::address::{Address, PhysAddr, VirtAddr};
 use svsm::config::SvsmConfig;
-use svsm::console::{init_console, install_console_logger};
+use svsm::console::install_console_logger;
 use svsm::cpu::cpuid::{dump_cpuid_table, register_cpuid_table};
 use svsm::cpu::gdt;
 use svsm::cpu::idt::stage2::{early_idt_init, early_idt_init_no_ghcb};
@@ -37,9 +37,7 @@ use svsm::mm::validate::{
     init_valid_bitmap_alloc, valid_bitmap_addr, valid_bitmap_set_valid_range,
 };
 use svsm::platform::{PageStateChangeOp, SvsmPlatform, SvsmPlatformCell};
-use svsm::serial::SerialPort;
 use svsm::types::{PageSize, PAGE_SIZE, PAGE_SIZE_2M};
-use svsm::utils::immut_after_init::ImmutAfterInitCell;
 use svsm::utils::{halt, is_aligned, MemoryRegion};
 
 extern "C" {
@@ -74,18 +72,20 @@ fn shutdown_percpu() {
         .expect("Failed to shut down percpu data (including GHCB)");
 }
 
-static CONSOLE_SERIAL: ImmutAfterInitCell<SerialPort<'_>> = ImmutAfterInitCell::uninit();
-
 fn setup_env(
     config: &SvsmConfig<'_>,
     platform: &mut dyn SvsmPlatform,
     launch_info: &Stage2LaunchInfo,
 ) {
     gdt().load();
     early_idt_init_no_ghcb();
-    platform.env_setup();
 
+    let debug_serial_port = config.debug_serial_port();
     install_console_logger("Stage2").expect("Console logger already initialized");
+    platform
+        .env_setup(debug_serial_port)
+        .expect("Early environment setup failed");
+
     init_kernel_mapping_info(
         VirtAddr::null(),
         VirtAddr::from(640 * 1024usize),
@@ -101,19 +101,14 @@ fn setup_env(
     // Init IDT again with handlers requiring GHCB (eg. #VC handler)
     early_idt_init();
 
-    CONSOLE_SERIAL
-        .init(&SerialPort::new(
-            platform.get_console_io_port(),
-            config.debug_serial_port(),
-        ))
-        .expect("console serial output already configured");
-    (*CONSOLE_SERIAL).init();
-    init_console(&*CONSOLE_SERIAL).expect("Console writer already initialized");
-
-    // Console is fully working now and any unsupported configuration can be
-    // properly reported.
+    // Complete initializtion of the platform.  After that point, the console
+    // will be fully working and any unsupported configuration can be properly
+    // reported.
+    platform
+        .env_setup_late(debug_serial_port)
+        .expect("Late environment setup failed");
+
     dump_cpuid_table();
-    platform.env_setup_late();
 }
 
 /// Map and validate the specified virtual memory region at the given physical
@@ -158,7 +153,7 @@ fn get_svsm_config(
 ) -> Result<SvsmConfig<'static>, SvsmError> {
     if launch_info.igvm_params == 0 {
         return Ok(SvsmConfig::FirmwareConfig(FwCfg::new(
-            platform.get_console_io_port(),
+            platform.get_io_port(),
         )));
     }
 
@@ -335,8 +330,6 @@ pub extern "C" fn stage2_main(launch_info: &Stage2LaunchInfo) {
     let config = get_svsm_config(launch_info, platform).expect("Failed to get SVSM configuration");
     setup_env(&config, platform, launch_info);
 
-    log::info!("COCONUT Secure Virtual Machine Service Module (SVSM) Stage 2 Loader");
-
     // Get the available physical memory region for the kernel
     let kernel_region = config
         .find_kernel_region()

kernel/src/svsm.rs

@@ -18,7 +18,7 @@ use core::slice;
 use cpuarch::snp_cpuid::SnpCpuidTable;
 use svsm::address::{PhysAddr, VirtAddr};
 use svsm::config::SvsmConfig;
-use svsm::console::{init_console, install_console_logger};
+use svsm::console::install_console_logger;
 use svsm::cpu::control_regs::{cr0_init, cr4_init};
 use svsm::cpu::cpuid::{dump_cpuid_table, register_cpuid_table};
 use svsm::cpu::efer::efer_init;
@@ -43,10 +43,8 @@ use svsm::mm::virtualrange::virt_log_usage;
 use svsm::mm::{init_kernel_mapping_info, PerCPUPageMappingGuard};
 use svsm::platform::{SvsmPlatformCell, SVSM_PLATFORM};
 use svsm::requests::{request_loop, request_processing_main, update_mappings};
-use svsm::serial::SerialPort;
 use svsm::sev::utils::{rmp_adjust, RMPFlags};
 use svsm::sev::{secrets_page, secrets_page_mut};
-use svsm::svsm_console::SVSMIOPort;
 use svsm::svsm_paging::{init_page_table, invalidate_early_boot_memory};
 use svsm::task::exec_user;
 use svsm::task::{create_kernel_task, schedule_init};
@@ -236,9 +234,6 @@ pub fn memory_init(launch_info: &KernelLaunchInfo) {
     );
 }
 
-static CONSOLE_IO: SVSMIOPort = SVSMIOPort::new();
-static CONSOLE_SERIAL: ImmutAfterInitCell<SerialPort<'_>> = ImmutAfterInitCell::uninit();
-
 pub fn boot_stack_info() {
     // SAFETY: this is only unsafe because `bsp_stack_end` is an extern
     // static, but we're simply printing its address. We are not creating a
@@ -317,7 +312,10 @@ pub extern "C" fn svsm_start(li: &KernelLaunchInfo, vb_addr: usize) {
     cr0_init();
     cr4_init();
     efer_init();
-    platform.env_setup();
+    install_console_logger("SVSM").expect("Console logger already initialized");
+    platform
+        .env_setup(debug_serial_port)
+        .expect("Early environment setup failed");
 
     memory_init(&launch_info);
     migrate_valid_bitmap().expect("Failed to migrate valid-bitmap");
@@ -358,19 +356,11 @@ pub extern "C" fn svsm_start(li: &KernelLaunchInfo, vb_addr: usize) {
         .expect("Failed to allocate idle task for BSP");
 
     idt_init();
-
-    CONSOLE_SERIAL
-        .init(&SerialPort::new(&CONSOLE_IO, debug_serial_port))
-        .expect("console serial output already configured");
-    (*CONSOLE_SERIAL).init();
-
-    init_console(&*CONSOLE_SERIAL).expect("Console writer already initialized");
-    install_console_logger("SVSM").expect("Console logger already initialized");
-
-    log::info!("COCONUT Secure Virtual Machine Service Module (SVSM)");
+    platform
+        .env_setup_late(debug_serial_port)
+        .expect("Late environment setup failed");
 
     dump_cpuid_table();
-    platform.env_setup_late();
 
     let mem_info = memory_info();
     print_memory_info(&mem_info);
@@ -417,7 +407,7 @@ pub extern "C" fn svsm_main() {
         }
         SvsmConfig::IgvmConfig(igvm_params)
     } else {
-        SvsmConfig::FirmwareConfig(FwCfg::new(&CONSOLE_IO))
+        SvsmConfig::FirmwareConfig(FwCfg::new(SVSM_PLATFORM.as_dyn_ref().get_io_port()))
     };
 
     init_memory_map(&config, &LAUNCH_INFO).expect("Failed to init guest memory map");