Merge pull request #130 from Freax13/fix/misc-safety-2

miscellaneous fixes

Author: joergroedel

src/console.rs

@@ -10,37 +10,44 @@ use crate::utils::immut_after_init::ImmutAfterInitCell;
 use core::fmt;
 use log;
 
-#[derive(Debug)]
 pub struct Console {
-    writer: *const dyn Terminal,
+    writer: Option<&'static dyn Terminal>,
 }
 
 impl Console {
-    pub fn set(&mut self, w: *mut dyn Terminal) {
-        self.writer = w;
+    pub fn set(&mut self, w: &'static dyn Terminal) {
+        self.writer = Some(w);
     }
 }
 
 impl fmt::Write for Console {
     fn write_str(&mut self, s: &str) -> fmt::Result {
-        if self.writer.is_null() {
-            return Ok(());
-        }
-
-        for ch in s.bytes() {
-            unsafe {
-                (*self.writer).put_byte(ch);
+        if let Some(writer) = self.writer {
+            for ch in s.bytes() {
+                writer.put_byte(ch);
             }
         }
 
         Ok(())
     }
 }
 
-pub static WRITER: SpinLock<Console> = SpinLock::new(unsafe {
-    Console {
-        writer: &DEFAULT_SERIAL_PORT,
+impl fmt::Debug for Console {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("Console")
+            .field(
+                "writer",
+                &format_args!(
+                    "{:?}",
+                    self.writer.as_ref().map(|writer| writer as *const _)
+                ),
+            )
+            .finish()
     }
+}
+
+pub static WRITER: SpinLock<Console> = SpinLock::new(Console {
+    writer: Some(&DEFAULT_SERIAL_PORT),
 });
 static CONSOLE_INITIALIZED: ImmutAfterInitCell<bool> = ImmutAfterInitCell::new(false);
 

src/cpu/gdt.rs

@@ -10,7 +10,7 @@ use crate::types::{SVSM_CS, SVSM_DS, SVSM_TSS};
 use core::arch::asm;
 use core::mem;
 
-#[repr(packed)]
+#[repr(C, packed(2))]
 pub struct GdtDesc {
     size: u16,
     addr: VirtAddr,
@@ -59,11 +59,6 @@ pub fn load_tss(tss: &X86Tss) {
     }
 }
 
-static mut GDT_DESC: GdtDesc = GdtDesc {
-    size: 0,
-    addr: VirtAddr::null(),
-};
-
 pub fn gdt_base_limit() -> (u64, u32) {
     unsafe {
         let gdt_entries = GDT_SIZE as usize;
@@ -75,10 +70,10 @@ pub fn gdt_base_limit() -> (u64, u32) {
 
 pub fn load_gdt() {
     unsafe {
-        let vaddr = VirtAddr::from(GDT.as_ptr());
-
-        GDT_DESC.addr = vaddr;
-        GDT_DESC.size = (GDT_SIZE * 8) - 1;
+        let gdt_desc: GdtDesc = GdtDesc {
+            size: (GDT_SIZE * 8) - 1,
+            addr: VirtAddr::from(GDT.as_ptr()),
+        };
 
         asm!(r#" /* Load GDT */
              lgdt   (%rax)
@@ -97,7 +92,7 @@ pub fn load_gdt() {
              lretq
         1:
              "#,
-            in("rax") &GDT_DESC,
+            in("rax") &gdt_desc,
             in("rdx") SVSM_CS,
             in("rcx") SVSM_DS,
             options(att_syntax));

src/fs/api.rs

@@ -59,14 +59,14 @@ impl FsError {
     impl_fs_err!(file_not_found, FileNotFound);
 }
 
-pub trait File: Debug {
+pub trait File: Debug + Send + Sync {
     fn read(&self, buf: &mut [u8], offset: usize) -> Result<usize, SvsmError>;
     fn write(&self, buf: &[u8], offset: usize) -> Result<usize, SvsmError>;
     fn truncate(&self, size: usize) -> Result<usize, SvsmError>;
     fn size(&self) -> usize;
 }
 
-pub trait Directory: Debug {
+pub trait Directory: Debug + Send + Sync {
     fn list(&self) -> Vec<FileName>;
     fn lookup_entry(&self, name: FileName) -> Result<DirEntry, SvsmError>;
     fn create_file(&self, name: FileName) -> Result<Arc<dyn File>, SvsmError>;

src/fs/filesystem.rs

@@ -67,8 +67,6 @@ pub struct FileHandle {
     handle: SpinLock<RawFileHandle>,
 }
 
-unsafe impl Sync for FileHandle {}
-
 impl FileHandle {
     pub fn new(file: &Arc<dyn File>) -> Self {
         FileHandle {
@@ -102,8 +100,6 @@ struct SvsmFs {
     root: Option<Arc<RamDirectory>>,
 }
 
-unsafe impl Sync for SvsmFs {}
-
 impl SvsmFs {
     const fn new() -> Self {
         SvsmFs { root: None }

src/fs/ramfs.rs

@@ -171,8 +171,6 @@ pub struct RamFile {
     rawfile: RWLock<RawRamFile>,
 }
 
-unsafe impl Sync for RamFile {}
-
 impl RamFile {
     #[allow(dead_code)]
     pub fn new() -> Self {
@@ -205,9 +203,6 @@ pub struct RamDirectory {
     entries: RWLock<Vec<DirectoryEntry>>,
 }
 
-unsafe impl Sync for RamDirectory {}
-unsafe impl Send for RamDirectory {}
-
 impl RamDirectory {
     pub fn new() -> Self {
         RamDirectory {

src/io.rs

@@ -6,7 +6,7 @@
 
 use core::arch::asm;
 
-pub trait IOPort {
+pub trait IOPort: Sync {
     fn outb(&self, port: u16, value: u8) {
         unsafe { asm!("outb %al, %dx", in("al") value, in("dx") port, options(att_syntax)) }
     }

src/locking/spinlock.rs

@@ -42,7 +42,8 @@ pub struct SpinLock<T: Debug> {
     data: UnsafeCell<T>,
 }
 
-unsafe impl<T: Debug> Sync for SpinLock<T> {}
+unsafe impl<T: Debug + Send> Send for SpinLock<T> {}
+unsafe impl<T: Debug + Send> Sync for SpinLock<T> {}
 
 impl<T: Debug> SpinLock<T> {
     pub const fn new(data: T) -> Self {

src/mm/alloc.rs

@@ -1183,6 +1183,19 @@ impl SvsmAllocator {
         }
     }
 
+    /// Resets the internal state. This is equivalent to reassigning `self`
+    /// with `Self::new()`.
+    #[cfg(test)]
+    fn reset(&self) {
+        *self.slab_size_32.lock() = Slab::new(32);
+        *self.slab_size_64.lock() = Slab::new(64);
+        *self.slab_size_128.lock() = Slab::new(128);
+        *self.slab_size_256.lock() = Slab::new(256);
+        *self.slab_size_512.lock() = Slab::new(512);
+        *self.slab_size_1024.lock() = Slab::new(1024);
+        *self.slab_size_2048.lock() = Slab::new(2048);
+    }
+
     fn get_slab(&self, size: usize) -> Option<&SpinLock<Slab>> {
         if size <= 32 {
             Some(&self.slab_size_32)
@@ -1248,7 +1261,7 @@ unsafe impl GlobalAlloc for SvsmAllocator {
 
 #[cfg_attr(any(target_os = "none"), global_allocator)]
 #[cfg_attr(not(target_os = "none"), allow(dead_code))]
-static mut ALLOCATOR: SvsmAllocator = SvsmAllocator::new();
+static ALLOCATOR: SvsmAllocator = SvsmAllocator::new();
 
 pub fn root_mem_init(pstart: PhysAddr, vstart: VirtAddr, page_count: usize) {
     {
@@ -1319,7 +1332,7 @@ impl Drop for TestRootMem<'_> {
 
         // Reset the Slabs
         *SLAB_PAGE_SLAB.lock() = SlabPageSlab::new();
-        unsafe { ALLOCATOR = SvsmAllocator::new() };
+        ALLOCATOR.reset();
     }
 }
 

src/mm/pagetable.rs

@@ -736,3 +736,8 @@ impl DerefMut for PageTableRef {
         unsafe { &mut *self.pgtable_ptr }
     }
 }
+
+/// SAFETY: `PageTableRef` is more or less equivalent to a mutable reference to
+///         a PageTable and so if `&mut PageTable` implements `Send` so does
+///         `PageTableRef`.
+unsafe impl Send for PageTableRef where &'static mut PageTable: Send {}

src/mm/validate.rs

@@ -287,3 +287,5 @@ impl ValidBitmap {
         }
     }
 }
+
+unsafe impl Send for ValidBitmap {}

src/serial.rs

@@ -25,7 +25,7 @@ pub const DLH: u16 = 1; // Divisor Latch High
 pub const RCVRDY: u8 = 0x01;
 pub const XMTRDY: u8 = 0x20;
 
-pub trait Terminal {
+pub trait Terminal: Sync {
     fn put_byte(&self, _ch: u8) {}
     fn get_byte(&self) -> u8 {
         0
@@ -88,7 +88,7 @@ impl<'a> Terminal for SerialPort<'a> {
     }
 }
 
-pub static mut DEFAULT_SERIAL_PORT: SerialPort = SerialPort {
+pub static DEFAULT_SERIAL_PORT: SerialPort = SerialPort {
     driver: &DEFAULT_IO_DRIVER,
     port: SERIAL_PORT,
 };

src/sev/ghcb.rs

@@ -8,7 +8,6 @@ use crate::address::{Address, PhysAddr, VirtAddr};
 use crate::cpu::flush_tlb_global_sync;
 use crate::cpu::msr::{write_msr, SEV_GHCB};
 use crate::error::SvsmError;
-use crate::io::IOPort;
 use crate::mm::pagetable::get_init_pgtable_locked;
 use crate::mm::validate::{
     valid_bitmap_clear_valid_4k, valid_bitmap_set_valid_4k, valid_bitmap_valid_addr,
@@ -17,12 +16,9 @@ use crate::mm::virt_to_phys;
 use crate::sev::sev_snp_enabled;
 use crate::sev::utils::raw_vmgexit;
 use crate::types::{PageSize, PAGE_SIZE_2M};
-use core::cell::RefCell;
 use core::{mem, ptr};
 
-use super::msr_protocol::{
-    invalidate_page_msr, register_ghcb_gpa_msr, request_termination_msr, validate_page_msr,
-};
+use super::msr_protocol::{invalidate_page_msr, register_ghcb_gpa_msr, validate_page_msr};
 use super::{pvalidate, PvalidateOp};
 
 // TODO: Fix this when Rust gets decent compile time struct offset support
@@ -486,50 +482,3 @@ impl GHCB {
         Ok(())
     }
 }
-
-pub struct GHCBIOPort<'a> {
-    pub ghcb: RefCell<&'a mut GHCB>,
-}
-
-impl<'a> GHCBIOPort<'a> {
-    pub fn new(ghcb: RefCell<&'a mut GHCB>) -> Self {
-        GHCBIOPort { ghcb }
-    }
-}
-unsafe impl<'a> Sync for GHCBIOPort<'a> {}
-
-impl<'a> IOPort for GHCBIOPort<'a> {
-    fn outb(&self, port: u16, value: u8) {
-        let mut g = self.ghcb.borrow_mut();
-        let ret = g.ioio_out(port, GHCBIOSize::Size8, value as u64);
-        if ret.is_err() {
-            request_termination_msr();
-        }
-    }
-
-    fn inb(&self, port: u16) -> u8 {
-        let mut g = self.ghcb.borrow_mut();
-        let ret = g.ioio_in(port, GHCBIOSize::Size8);
-        match ret {
-            Ok(v) => (v & 0xff) as u8,
-            Err(_e) => request_termination_msr(),
-        }
-    }
-
-    fn outw(&self, port: u16, value: u16) {
-        let mut g = self.ghcb.borrow_mut();
-        let ret = g.ioio_out(port, GHCBIOSize::Size16, value as u64);
-        if ret.is_err() {
-            request_termination_msr();
-        }
-    }
-
-    fn inw(&self, port: u16) -> u16 {
-        let mut g = self.ghcb.borrow_mut();
-        let ret = g.ioio_in(port, GHCBIOSize::Size16);
-        match ret {
-            Ok(v) => (v & 0xffff) as u16,
-            Err(_e) => request_termination_msr(),
-        }
-    }
-}

src/stage2.rs

@@ -73,7 +73,7 @@ fn shutdown_percpu() {
 }
 
 static CONSOLE_IO: SVSMIOPort = SVSMIOPort::new();
-static mut CONSOLE_SERIAL: SerialPort = SerialPort {
+static CONSOLE_SERIAL: SerialPort = SerialPort {
     driver: &CONSOLE_IO,
     port: SERIAL_PORT,
 };
@@ -95,9 +95,7 @@ fn setup_env() {
     setup_stage2_allocator();
     init_percpu();
 
-    unsafe {
-        WRITER.lock().set(&mut CONSOLE_SERIAL);
-    }
+    WRITER.lock().set(&CONSOLE_SERIAL);
     init_console();
 
     // Console is fully working now and any unsupported configuration can be

src/svsm.rs

@@ -290,7 +290,7 @@ pub fn memory_init(launch_info: &KernelLaunchInfo) {
 }
 
 static CONSOLE_IO: SVSMIOPort = SVSMIOPort::new();
-static mut CONSOLE_SERIAL: SerialPort = SerialPort {
+static CONSOLE_SERIAL: SerialPort = SerialPort {
     driver: &CONSOLE_IO,
     port: SERIAL_PORT,
 };
@@ -381,9 +381,7 @@ pub extern "C" fn svsm_start(li: &KernelLaunchInfo, vb_addr: usize) {
     }
     idt_init();
 
-    unsafe {
-        WRITER.lock().set(&mut CONSOLE_SERIAL);
-    }
+    WRITER.lock().set(&CONSOLE_SERIAL);
     init_console();
     install_console_logger("SVSM");