Skip to content

Commit

Permalink
utils: permit move initialization of ImmutAfterInitCell
Browse files Browse the repository at this point in the history
For maximum usefulness, it should be possible to support
`ImmutAfterInitCell` for types that do not implement `Copy`.  For
example, some structures that require global initialization may contain
atomics, which do not support `Copy`.  However, it is also important not
to require all initialization to be performed using moves, since a move
into a cell requires a temporary copy on the stack, and some global
types are especially large.  Therefore `ImmutAfterInitCell` can support
move-based initialization for all types, and by-reference initialization
for types that support `Copy`.

Signed-off-by: Jon Lange <[email protected]>
  • Loading branch information
msft-jlange committed Jan 22, 2025
1 parent bc3e99f commit 806d532
Show file tree
Hide file tree
Showing 11 changed files with 69 additions and 30 deletions.
4 changes: 2 additions & 2 deletions kernel/src/console.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,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)
Expand Down Expand Up @@ -130,7 +130,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
Expand Down
4 changes: 2 additions & 2 deletions kernel/src/hyperv/hv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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(())
Expand Down
4 changes: 2 additions & 2 deletions kernel/src/mm/address_space.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}

Expand Down Expand Up @@ -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;
}

Expand Down
10 changes: 5 additions & 5 deletions kernel/src/mm/pagetable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,15 @@ pub fn paging_init(platform: &dyn SvsmPlatform, suppress_global: bool) -> ImmutA
if suppress_global {
feature_mask.remove(PTEntryFlags::GLOBAL);
}
FEATURE_MASK.init(&feature_mask)
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.init(&masks.private_pte_mask)?;
SHARED_PTE_MASK.init(&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;
Expand All @@ -78,15 +78,15 @@ fn init_encrypt_mask(platform: &dyn SvsmPlatform) -> ImmutAfterInitResult<()> {
guest_phys_addr_size
};

PHYS_ADDR_SIZE.init(&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.init(&max_addr)
MAX_PHYS_ADDR.init(max_addr)
}

/// Returns the private encrypt mask value.
Expand Down
2 changes: 1 addition & 1 deletion kernel/src/platform/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion kernel/src/platform/snp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}

Expand Down
2 changes: 1 addition & 1 deletion kernel/src/platform/tdp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,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)
}
Expand Down
2 changes: 1 addition & 1 deletion kernel/src/sev/msr_protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion kernel/src/sev/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}

Expand Down
6 changes: 3 additions & 3 deletions kernel/src/svsm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,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);
}
Expand All @@ -169,7 +169,7 @@ pub 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);
Expand Down Expand Up @@ -259,7 +259,7 @@ pub 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();
Expand Down
61 changes: 50 additions & 11 deletions kernel/src/utils/immut_after_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@ pub enum ImmutAfterInitError {
/// # use svsm::utils::immut_after_init::ImmutAfterInitCell;
/// static X: ImmutAfterInitCell<i32> = ImmutAfterInitCell::uninit();
/// pub fn main() {
/// unsafe { X.init(&123) };
/// unsafe { X.init_from_ref(&123) };
/// assert_eq!(*X, 123);
/// }
/// ```
#[derive(Debug)]
pub struct ImmutAfterInitCell<T: Copy> {
pub struct ImmutAfterInitCell<T> {
#[doc(hidden)]
data: UnsafeCell<MaybeUninit<T>>,
#[doc(hidden)]
Expand All @@ -58,7 +58,7 @@ const IMMUT_UNINIT: u8 = 0;
const IMMUT_INIT_IN_PROGRESS: u8 = 1;
const IMMUT_INITIALIZED: u8 = 2;

impl<T: Copy> ImmutAfterInitCell<T> {
impl<T> ImmutAfterInitCell<T> {
/// Create an unitialized `ImmutAfterInitCell` instance. The value must get
/// initialized by means of [`Self::init()`] before first usage.
pub const fn uninit() -> Self {
Expand Down Expand Up @@ -107,21 +107,41 @@ impl<T: Copy> ImmutAfterInitCell<T> {
/// Will fail if called on an initialized instance.
///
/// * `v` - Initialization value.
pub fn init(&self, v: &T) -> ImmutAfterInitResult<()> {
pub fn init(&self, v: T) -> ImmutAfterInitResult<()> {
self.try_init()?;
// SAFETY: Successful completion of `try_init` conveys the exclusive
// right to populate the contents of the cell.
unsafe {
let data = &mut *self.data.get();
data.write(v);
self.complete_init();
}
Ok(())
}

/// Initialize an uninitialized `ImmutAfterInitCell` instance from a
/// reference.
/// Will fail if called on an initialized instance.
///
/// * `v` - Initialization reference.
pub fn init_from_ref(&self, r: &T) -> ImmutAfterInitResult<()>
where
T: Copy,
{
self.try_init()?;
// SAFETY: Successful completion of `try_init` conveys the exclusive
// right to populate the contents of the cell.
unsafe {
(*self.data.get())
.as_mut_ptr()
.copy_from_nonoverlapping(v, 1);
.copy_from_nonoverlapping(r, 1);
self.complete_init();
}
Ok(())
}
}

impl<T: Copy> Deref for ImmutAfterInitCell<T> {
impl<T> Deref for ImmutAfterInitCell<T> {
type Target = T;

/// Dereference the wrapped value. Will panic if called on an
Expand All @@ -131,8 +151,27 @@ impl<T: Copy> Deref for ImmutAfterInitCell<T> {
}
}

unsafe impl<T: Copy + Send> Send for ImmutAfterInitCell<T> {}
unsafe impl<T: Copy + Send + Sync> Sync for ImmutAfterInitCell<T> {}
impl<T> Drop for ImmutAfterInitCell<T> {
fn drop(&mut self) {
// Dropping is only required if the cell has been initialized.
if self.init.load(Ordering::Relaxed) == IMMUT_INITIALIZED {
// SAFETY: the initialization check ensures that the cell holds
// initialized data.
unsafe {
let cell = &mut *self.data.get();
// This drop will never occur for a cell that was initialized
// from a reference, because initialization from a reference
// requires `Copy` and types that implement `Copy` do not
// implement `Drop`, and thus this will have no effect for such
// types.
cell.assume_init_drop();
}
}
}
}

unsafe impl<T: Send> Send for ImmutAfterInitCell<T> {}
unsafe impl<T: Send + Sync> Sync for ImmutAfterInitCell<T> {}

/// A reference to a memory location which is effectively immutable after
/// initalization code has run.
Expand All @@ -149,7 +188,7 @@ unsafe impl<T: Copy + Send + Sync> Sync for ImmutAfterInitCell<T> {}
/// static X: ImmutAfterInitCell<i32> = ImmutAfterInitCell::uninit();
/// static RX: ImmutAfterInitRef<'_, i32> = ImmutAfterInitRef::uninit();
/// fn main() {
/// unsafe { X.init(&123) };
/// unsafe { X.init_from_ref(&123) };
/// unsafe { RX.init_from_cell(&X) };
/// assert_eq!(*RX, 123);
/// }
Expand Down Expand Up @@ -215,7 +254,7 @@ impl<'a, T: Copy> ImmutAfterInitRef<'a, T> {
where
'b: 'a,
{
self.ptr.init(&(r as *const T))
self.ptr.init(r as *const T)
}

/// Dereference the referenced value with lifetime propagation. Will panic
Expand All @@ -236,7 +275,7 @@ impl<'a, T: Copy> ImmutAfterInitRef<'a, T> {
where
'b: 'a,
{
self.ptr.init(&(cell.try_get_inner()? as *const T))
self.ptr.init(cell.try_get_inner()? as *const T)
}
}

Expand Down

0 comments on commit 806d532

Please sign in to comment.