Merge pull request #595 from msft-jlange/immut

utils: overhaul `ImmutAfterInitCell`

kernel/src/console.rs

@@ -10,6 +10,7 @@ use crate::locking::SpinLock;
 use crate::serial::{SerialPort, Terminal, DEFAULT_SERIAL_PORT};
 use crate::utils::immut_after_init::{ImmutAfterInitCell, ImmutAfterInitResult};
 use core::fmt;
+use core::sync::atomic::{AtomicBool, Ordering};
 use release::COCONUT_VERSION;
 
 #[derive(Clone, Copy, Debug)]
@@ -35,12 +36,16 @@ impl Console {
 static WRITER: SpinLock<Console> = SpinLock::new(Console {
     writer: &DEFAULT_SERIAL_PORT,
 });
-static CONSOLE_INITIALIZED: ImmutAfterInitCell<bool> = ImmutAfterInitCell::new(false);
+
+// CONSOLE_INITIALIZED is only ever written during the single-proc phase of
+// boot, so it can safely be written with relaxed ordering.  FOr the same
+// reason, it can always safely be read with relaxed ordering.
+static CONSOLE_INITIALIZED: AtomicBool = AtomicBool::new(false);
 static CONSOLE_SERIAL: ImmutAfterInitCell<SerialPort<'_>> = ImmutAfterInitCell::uninit();
 
 fn init_console(writer: &'static dyn Terminal) -> ImmutAfterInitResult<()> {
     WRITER.lock().writer = writer;
-    CONSOLE_INITIALIZED.reinit(&true)?;
+    CONSOLE_INITIALIZED.store(true, Ordering::Relaxed);
     log::info!(
         "COCONUT Secure Virtual Machine Service Module Version {}",
         COCONUT_VERSION
@@ -50,7 +55,7 @@ fn init_console(writer: &'static dyn Terminal) -> ImmutAfterInitResult<()> {
 
 pub fn init_svsm_console(writer: &'static dyn IOPort, port: u16) -> Result<(), SvsmError> {
     CONSOLE_SERIAL
-        .init(&SerialPort::new(writer, port))
+        .init_from_ref(&SerialPort::new(writer, port))
         .map_err(|_| SvsmError::Console)?;
     (*CONSOLE_SERIAL).init();
     init_console(&*CONSOLE_SERIAL).map_err(|_| SvsmError::Console)
@@ -59,7 +64,7 @@ pub fn init_svsm_console(writer: &'static dyn IOPort, port: u16) -> Result<(), S
 #[doc(hidden)]
 pub fn _print(args: fmt::Arguments<'_>) {
     use core::fmt::Write;
-    if !*CONSOLE_INITIALIZED {
+    if !CONSOLE_INITIALIZED.load(Ordering::Relaxed) {
         return;
     }
     WRITER.lock().write_fmt(args).unwrap();
@@ -71,7 +76,7 @@ pub fn _print(args: fmt::Arguments<'_>) {
 ///
 /// * `buffer`: u8 slice with bytes to write.
 pub fn console_write(buffer: &[u8]) {
-    if !*CONSOLE_INITIALIZED {
+    if !CONSOLE_INITIALIZED.load(Ordering::Relaxed) {
         return;
     }
     WRITER.lock().write_bytes(buffer);
@@ -133,7 +138,7 @@ impl log::Log for ConsoleLogger {
 static CONSOLE_LOGGER: ImmutAfterInitCell<ConsoleLogger> = ImmutAfterInitCell::uninit();
 
 pub fn install_console_logger(component: &'static str) -> ImmutAfterInitResult<()> {
-    CONSOLE_LOGGER.init(&ConsoleLogger::new(component))?;
+    CONSOLE_LOGGER.init_from_ref(&ConsoleLogger::new(component))?;
 
     if let Err(e) = log::set_logger(&*CONSOLE_LOGGER) {
         // Failed to install the ConsoleLogger, presumably because something had

kernel/src/cpu/smp.rs

@@ -19,7 +19,6 @@ use crate::hyperv;
 use crate::platform::{SvsmPlatform, SVSM_PLATFORM};
 use crate::requests::{request_loop, request_processing_main};
 use crate::task::{schedule_init, start_kernel_task};
-use crate::utils::immut_after_init::immut_after_init_set_multithreaded;
 
 use alloc::string::String;
 use bootlib::kernel_launch::ApStartContext;
@@ -41,7 +40,6 @@ fn start_cpu(platform: &dyn SvsmPlatform, apic_id: u32) -> Result<(), SvsmError>
 }
 
 pub fn start_secondary_cpus(platform: &dyn SvsmPlatform, cpus: &[ACPICPUInfo]) {
-    immut_after_init_set_multithreaded();
     let mut count: usize = 0;
     for c in cpus.iter().filter(|c| c.apic_id != 0 && c.enabled) {
         log::info!("Launching AP with APIC-ID {}", c.apic_id);

kernel/src/hyperv/hv.rs

@@ -186,7 +186,7 @@ pub fn hyperv_setup_hypercalls() -> Result<(), SvsmError> {
         .map_4k(hypercall_va, virt_to_phys(page), PTEntryFlags::exec())?;
 
     HYPERV_HYPERCALL_CODE_PAGE
-        .init(&hypercall_va)
+        .init(hypercall_va)
         .expect("Hypercall code page already allocated");
 
     // Set the guest OS ID.  The value is arbitrary.
@@ -202,7 +202,7 @@ pub fn hyperv_setup_hypercalls() -> Result<(), SvsmError> {
     let vsm_status = hyperv::HvRegisterVsmVpStatus::from(vsm_status_value);
     let current_vtl = vsm_status.active_vtl();
     CURRENT_VTL
-        .init(&current_vtl)
+        .init(current_vtl)
         .expect("Current VTL already initialized");
 
     Ok(())

kernel/src/mm/address_space.rs

@@ -59,7 +59,7 @@ pub fn init_kernel_mapping_info(
         heap_mapping,
     };
     FIXED_MAPPING
-        .init(&mapping)
+        .init_from_ref(&mapping)
         .expect("Already initialized fixed mapping info");
 }
 
@@ -273,7 +273,7 @@ mod tests {
             kernel_mapping,
             heap_mapping: None,
         };
-        KERNEL_MAPPING_TEST.init(&mapping).unwrap();
+        KERNEL_MAPPING_TEST.init_from_ref(&mapping).unwrap();
         *initialized = true;
     }
 

kernel/src/mm/pagetable.rs

@@ -32,10 +32,10 @@ use alloc::boxed::Box;
 const ENTRY_COUNT: usize = 512;
 
 /// Mask for private page table entry.
-static PRIVATE_PTE_MASK: ImmutAfterInitCell<usize> = ImmutAfterInitCell::new(0);
+static PRIVATE_PTE_MASK: ImmutAfterInitCell<usize> = ImmutAfterInitCell::uninit();
 
 /// Mask for shared page table entry.
-static SHARED_PTE_MASK: ImmutAfterInitCell<usize> = ImmutAfterInitCell::new(0);
+static SHARED_PTE_MASK: ImmutAfterInitCell<usize> = ImmutAfterInitCell::uninit();
 
 /// Maximum physical address supported by the system.
 static MAX_PHYS_ADDR: ImmutAfterInitCell<u64> = ImmutAfterInitCell::uninit();
@@ -47,32 +47,25 @@ static PHYS_ADDR_SIZE: ImmutAfterInitCell<u32> = ImmutAfterInitCell::uninit();
 pub const LAUNCH_VMSA_ADDR: PhysAddr = PhysAddr::new(0xFFFFFFFFF000);
 
 /// Feature mask for page table entry flags.
-static FEATURE_MASK: ImmutAfterInitCell<PTEntryFlags> =
-    ImmutAfterInitCell::new(PTEntryFlags::empty());
-
-/// Re-initializes early paging settings.
-pub fn paging_init_early(platform: &dyn SvsmPlatform) -> ImmutAfterInitResult<()> {
-    init_encrypt_mask(platform)?;
-
-    let mut feature_mask = PTEntryFlags::all();
-    feature_mask.remove(PTEntryFlags::GLOBAL);
-    FEATURE_MASK.reinit(&feature_mask)
-}
+static FEATURE_MASK: ImmutAfterInitCell<PTEntryFlags> = ImmutAfterInitCell::uninit();
 
 /// Initializes paging settings.
-pub fn paging_init(platform: &dyn SvsmPlatform) -> ImmutAfterInitResult<()> {
+pub fn paging_init(platform: &dyn SvsmPlatform, suppress_global: bool) -> ImmutAfterInitResult<()> {
     init_encrypt_mask(platform)?;
 
-    let feature_mask = PTEntryFlags::all();
-    FEATURE_MASK.reinit(&feature_mask)
+    let mut feature_mask = PTEntryFlags::all();
+    if suppress_global {
+        feature_mask.remove(PTEntryFlags::GLOBAL);
+    }
+    FEATURE_MASK.init(feature_mask)
 }
 
 /// Initializes the encrypt mask.
 fn init_encrypt_mask(platform: &dyn SvsmPlatform) -> ImmutAfterInitResult<()> {
     let masks = platform.get_page_encryption_masks();
 
-    PRIVATE_PTE_MASK.reinit(&masks.private_pte_mask)?;
-    SHARED_PTE_MASK.reinit(&masks.shared_pte_mask)?;
+    PRIVATE_PTE_MASK.init(masks.private_pte_mask)?;
+    SHARED_PTE_MASK.init(masks.shared_pte_mask)?;
 
     let guest_phys_addr_size = (masks.phys_addr_sizes >> 16) & 0xff;
     let host_phys_addr_size = masks.phys_addr_sizes & 0xff;
@@ -85,15 +78,15 @@ fn init_encrypt_mask(platform: &dyn SvsmPlatform) -> ImmutAfterInitResult<()> {
         guest_phys_addr_size
     };
 
-    PHYS_ADDR_SIZE.reinit(&phys_addr_size)?;
+    PHYS_ADDR_SIZE.init(phys_addr_size)?;
 
     // If the C-bit is a physical address bit however, the guest physical
     // address space is effectively reduced by 1 bit.
     // - APM2, 15.34.6 Page Table Support
     let effective_phys_addr_size = cmp::min(masks.addr_mask_width, phys_addr_size);
 
     let max_addr = 1 << effective_phys_addr_size;
-    MAX_PHYS_ADDR.reinit(&max_addr)
+    MAX_PHYS_ADDR.init(max_addr)
 }
 
 /// Returns the private encrypt mask value.

kernel/src/platform/mod.rs

@@ -225,7 +225,7 @@ impl DerefMut for SvsmPlatformCell {
 }
 
 pub fn init_platform_type(platform_type: SvsmPlatformType) {
-    SVSM_PLATFORM_TYPE.init(&platform_type).unwrap();
+    SVSM_PLATFORM_TYPE.init(platform_type).unwrap();
 }
 
 pub fn halt() {

kernel/src/platform/snp.rs

@@ -99,7 +99,7 @@ impl SvsmPlatform for SnpPlatform {
 
     fn env_setup(&mut self, _debug_serial_port: u16, vtom: usize) -> Result<(), SvsmError> {
         sev_status_init();
-        VTOM.init(&vtom).map_err(|_| SvsmError::PlatformInit)?;
+        VTOM.init(vtom).map_err(|_| SvsmError::PlatformInit)?;
         Ok(())
     }
 

kernel/src/platform/tdp.rs

@@ -50,7 +50,7 @@ impl SvsmPlatform for TdpPlatform {
 
     fn env_setup(&mut self, debug_serial_port: u16, vtom: usize) -> Result<(), SvsmError> {
         assert_ne!(vtom, 0);
-        VTOM.init(&vtom).map_err(|_| SvsmError::PlatformInit)?;
+        VTOM.init(vtom).map_err(|_| SvsmError::PlatformInit)?;
         // Serial console device can be initialized immediately
         init_svsm_console(&GHCI_IO_DRIVER, debug_serial_port)
     }

kernel/src/sev/msr_protocol.rs

@@ -130,7 +130,7 @@ pub fn init_hypervisor_ghcb_features() -> Result<(), GhcbMsrError> {
         }
 
         GHCB_HV_FEATURES
-            .init(&features)
+            .init(features)
             .expect("Already initialized GHCB HV features");
         Ok(())
     } else {

kernel/src/sev/status.rs

@@ -142,7 +142,7 @@ pub fn sev_flags() -> SEVStatusFlags {
 pub fn sev_status_init() {
     let status: SEVStatusFlags = read_sev_status();
     SEV_FLAGS
-        .init(&status)
+        .init(status)
         .expect("Already initialized SEV flags");
 }
 

kernel/src/stage2.rs

@@ -28,7 +28,7 @@ use svsm::error::SvsmError;
 use svsm::fw_cfg::FwCfg;
 use svsm::igvm_params::IgvmParams;
 use svsm::mm::alloc::{memory_info, print_memory_info, root_mem_init};
-use svsm::mm::pagetable::{paging_init_early, PTEntryFlags, PageTable};
+use svsm::mm::pagetable::{paging_init, PTEntryFlags, PageTable};
 use svsm::mm::validate::{
     init_valid_bitmap_alloc, valid_bitmap_addr, valid_bitmap_set_valid_range,
 };
@@ -101,6 +101,10 @@ fn setup_env(
         PhysAddr::from(u64::from(STAGE2_START)),
     );
 
+    let cpuid_page = unsafe { &*(launch_info.cpuid_page as *const SnpCpuidTable) };
+    register_cpuid_table(cpuid_page);
+    paging_init(platform, true).expect("Failed to initialize early paging");
+
     // Use the low 640 KB of memory as the heap.
     let lowmem_region = MemoryRegion::new(VirtAddr::from(0u64), 640 * 1024);
     let heap_mapping = FixedAddressMappingRange::new(
@@ -116,11 +120,6 @@ fn setup_env(
         .validate_virtual_page_range(lowmem_region, PageValidateOp::Validate)
         .expect("failed to validate low 640 KB");
 
-    let cpuid_page = unsafe { &*(launch_info.cpuid_page as *const SnpCpuidTable) };
-
-    register_cpuid_table(cpuid_page);
-    paging_init_early(platform).expect("Failed to initialize early paging");
-
     // Configure the heap to exist from 64 KB to 640 KB.
     setup_stage2_allocator(0x10000, 0xA0000);
 

kernel/src/svsm.rs

@@ -145,7 +145,7 @@ fn init_cpuid_table(addr: VirtAddr) {
     }
 
     CPUID_PAGE
-        .init(table)
+        .init_from_ref(table)
         .expect("Already initialized CPUID page");
     register_cpuid_table(&CPUID_PAGE);
 }
@@ -171,7 +171,7 @@ extern "C" fn svsm_start(li: &KernelLaunchInfo, vb_addr: usize) -> ! {
     let debug_serial_port = li.debug_serial_port;
 
     LAUNCH_INFO
-        .init(li)
+        .init_from_ref(li)
         .expect("Already initialized launch info");
 
     let mut platform = SvsmPlatformCell::new(li.platform_type, li.suppress_svsm_interrupts);
@@ -212,7 +212,7 @@ extern "C" fn svsm_start(li: &KernelLaunchInfo, vb_addr: usize) -> ! {
         Err(e) => panic!("error reading kernel ELF: {}", e),
     };
 
-    paging_init(&*platform).expect("Failed to initialize paging");
+    paging_init(&*platform, false).expect("Failed to initialize paging");
     let init_pgtable =
         init_page_table(&launch_info, &kernel_elf).expect("Could not initialize the page table");
     // SAFETY: we are initializing the state, including stack and registers
@@ -258,7 +258,7 @@ extern "C" fn svsm_start(li: &KernelLaunchInfo, vb_addr: usize) -> ! {
         .expect("Alternate injection required but not available");
 
     SVSM_PLATFORM
-        .init(&platform)
+        .init(platform)
         .expect("Failed to initialize SVSM platform object");
 
     sse_init();