refactoring timer controller

Signed-off-by: Wojciech Ozga <woz@zurich.ibm.com>

security-monitor/src/confidential_flow/finite_state_machine.rs

@@ -154,7 +154,7 @@ impl<'a> ConfidentialFlow<'a> {
         let declassifier =
             DeclassifyToHypervisor::EnabledInterrupts(ExposeEnabledInterrupts::from_confidential_hart(self.confidential_hart()));
 
-        TimerController::try_write(|controller| Ok(controller.store_vs_timer(&mut self))).unwrap();
+        TimerController::new(&mut self).store_vs_timer();
 
         ControlDataStorage::try_confidential_vm(self.confidential_vm_id(), |mut confidential_vm| {
             // Run heavy context switch when giving back the confidential hart to the confidential VM.
@@ -223,7 +223,8 @@ impl<'a> ConfidentialFlow<'a> {
         // We must restore the control and status registers (CSRs) that might have changed during execution of the security monitor.
         // We call it here because it is just before exiting to the assembly context switch, so we are sure that these CSRs have their
         // final values.
-        let interrupts = (self.confidential_hart().csrs().hvip.read_from_main_memory() | self.confidential_hart().csrs().vstip)
+        let interrupts = (self.confidential_hart().csrs().hvip.read_from_main_memory()
+            | self.confidential_hart().csrs().pending_interrupts)
             & self.confidential_hart().csrs().allowed_external_interrupts;
         self.confidential_hart().csrs().hvip.write(interrupts);
         let address = self.confidential_hart_mut().address();

security-monitor/src/confidential_flow/handlers/interrupts/expose_enabled_interrupts.rs

@@ -13,7 +13,8 @@ impl ExposeEnabledInterrupts {
     pub fn from_confidential_hart(confidential_hart: &ConfidentialHart) -> Self {
         Self {
             vsie: confidential_hart.csrs().vsie.read(),
-            vstimecmp: confidential_hart.csrs().vstimecmp, // vstimecmp: confidential_hart.sstc().vstimecmp.read(),
+            vstimecmp: confidential_hart.csrs().vstimecmp.unwrap_or(usize::MAX - 1), /* vstimecmp:
+                                                                                      * confidential_hart.sstc().vstimecmp.read(), */
         }
     }
 

security-monitor/src/confidential_flow/handlers/interrupts/handle_interrupt.rs

@@ -21,40 +21,26 @@ impl HandleInterrupt {
 
     pub fn handle(self, mut confidential_flow: ConfidentialFlow) -> ! {
         use crate::core::architecture::specification::*;
-        // debug!(
-        //     "Interrupt {:x} mepc={:x}",
-        //     self.pending_interrupts,
-        //     confidential_flow.confidential_hart_mut().csrs().mepc.read_from_main_memory()
-        // );
         if self.pending_interrupts & MIE_SSIP_MASK > 0 {
             // One of the reasons why the confidential hart was interrupted with SSIP is that it got an `ConfidentialHartRemoteCommand` from
-            // another confidential hart. If this is the case, we must process all queued requests before resuming confidential hart's execution.
-            // This is done as part of the procedure that resumes confidential hart execution.
+            // another confidential hart. If this is the case, we must process all queued requests before resuming confidential hart's
+            // execution. This is done as part of the procedure that resumes confidential hart execution.
             confidential_flow.confidential_hart_mut().csrs_mut().mip.read_and_clear_bits(MIE_SSIP_MASK);
             confidential_flow.resume_confidential_hart_execution();
-        } else if self.pending_interrupts & MIE_MTIP_MASK > 0 || self.pending_interrupts & MIE_VSTIP_MASK > 0 {
-            let stimecmp = confidential_flow.confidential_hart_mut().csrs_mut().stimecmp;
-            let is_vstip = TimerController::try_write(|controller| {
-                let mtime = controller.read_mtime();
-                let stip: bool = controller.s_timer_interrupted(&mut confidential_flow, mtime);
-                let vstip: bool = controller.vs_timer_interrupted(&mut confidential_flow, mtime);
-                if vstip {
-                    controller.handle_vs_interrupt(&mut confidential_flow);
-                    controller.set_s_timer(&mut confidential_flow, stimecmp);
-                }
-                let report_vs_interrupt = vstip && !stip;
-                Ok(report_vs_interrupt)
-            })
-            .unwrap_or(false);
-            if is_vstip {
-                confidential_flow.resume_confidential_hart_execution();
-            } else {
-                confidential_flow.into_non_confidential_flow().declassify_and_exit_to_hypervisor(DeclassifyToHypervisor::Interrupt(self))
-            }
         } else {
             // The only interrupts that we can see here are:
             // * M-mode timer that the security monitor set to preemt execution of a confidential VM
-            // * M-mode software or external interrupt
+            // * S-mode timer that the hypervisor set to preemt execution of a confidential VM
+            // * M-mode external interrupt
+            let mut controller = TimerController::new(&mut confidential_flow);
+            if controller.vs_timer_interrupted() {
+                controller.handle_vs_interrupt();
+                if !controller.s_timer_interrupted() {
+                    confidential_flow.resume_confidential_hart_execution();
+                } else {
+                    controller.set_s_timer();
+                }
+            }
             confidential_flow.into_non_confidential_flow().declassify_and_exit_to_hypervisor(DeclassifyToHypervisor::Interrupt(self))
         }
     }

security-monitor/src/confidential_flow/handlers/time/mod.rs

@@ -20,7 +20,7 @@ impl SetTimer {
     }
 
     pub fn handle(self, mut confidential_flow: ConfidentialFlow) -> ! {
-        TimerController::try_write(|controller| Ok(controller.set_next_event_for_vs_mode(&mut confidential_flow, self.ncycle))).unwrap();
+        TimerController::new(&mut confidential_flow).set_next_event_for_vs_mode(self.ncycle);
         confidential_flow.apply_and_exit_to_confidential_hart(ApplyToConfidentialHart::SbiResponse(SbiResponse::success()))
     }
 }

security-monitor/src/core/architecture/riscv/control_status_registers.rs

@@ -67,8 +67,8 @@ pub struct ControlStatusRegisters {
     pub vsatp: ReadWriteRiscvCsr<CSR_VSATP>,
     // timer
     pub stimecmp: usize,
-    pub vstimecmp: usize,
-    pub vstip: usize,
+    pub vstimecmp: Option<usize>,
+    pub pending_interrupts: usize,
     pub allowed_external_interrupts: usize,
 }
 
@@ -128,8 +128,8 @@ impl ControlStatusRegisters {
             vsatp: ReadWriteRiscvCsr::new(),
             // timer
             stimecmp: 0,
-            vstimecmp: 0,
-            vstip: 0,
+            vstimecmp: None,
+            pending_interrupts: 0,
             allowed_external_interrupts: 0,
         };
         csrs

security-monitor/src/core/control_data/confidential_hart.rs

@@ -97,7 +97,7 @@ impl ConfidentialHart {
         // the `vsie` register reflects `hie`, so we set up `hie` allowing only VS-level interrupts
         confidential_hart_state.csrs_mut().hie.save_value_in_main_memory(Self::INTERRUPT_DELEGATION);
         // Allow only hypervisor's timer interrupts to preemt confidential VM's execution
-        confidential_hart_state.csrs_mut().mie.save_value_in_main_memory(MIE_STIP_MASK | MIE_MTIP_MASK | MIE_SSIP_MASK);
+        confidential_hart_state.csrs_mut().mie.save_value_in_main_memory(MIE_STIP_MASK | MIE_MTIP_MASK | MIE_MEIP_MASK | MIE_SSIP_MASK);
         confidential_hart_state.csrs_mut().htimedelta.save_value_in_main_memory(htimedelta);
         // Setup the M-mode trap handler to the security monitor's entry point
         confidential_hart_state.csrs_mut().mtvec.save_value_in_main_memory(enter_from_confidential_hart_asm as usize);
@@ -114,7 +114,6 @@ impl ConfidentialHart {
         // assert!(HardwareSetup::is_extension_supported(HardwareExtension::SupervisorTimerExtension));
         // // Preempt execution as fast as possible to allow hypervisor control confidential hart execution duration
         // confidential_hart_state.sstc_mut().stimecmp.save_value_in_main_memory(0);
-        confidential_hart_state.csrs_mut().vstimecmp = usize::MAX - 1;
 
         if HardwareSetup::is_extension_supported(HardwareExtension::FloatingPointExtension) {
             confidential_hart_state.csrs_mut().mstatus.enable_bits_on_saved_value(SR_FS_INITIAL);

security-monitor/src/core/initialization/mod.rs

@@ -10,7 +10,6 @@ use crate::core::interrupt_controller::InterruptController;
 use crate::core::memory_layout::{ConfidentialMemoryAddress, MemoryLayout};
 use crate::core::memory_protector::HypervisorMemoryProtector;
 use crate::core::page_allocator::{Page, PageAllocator, UnAllocated};
-use crate::core::timer_controller::TimerController;
 use crate::error::Error;
 use alloc::vec::Vec;
 use core::mem::size_of;
@@ -187,7 +186,6 @@ fn initalize_security_monitor_state(
     unsafe { PageAllocator::initialize(page_allocator_start_address, page_allocator_end_address)? };
 
     InterruptController::initialize()?;
-    TimerController::initialize()?;
     ControlDataStorage::initialize()?;
     HardwareSetup::initialize()?;
 

security-monitor/src/core/timer_controller/mod.rs

@@ -10,130 +10,108 @@ extern "C" {
     fn sbi_timer_value() -> u64;
 }
 
-const NOT_INITIALIZED_TIMER_CONTROLLER: &str = "Bug. Could not access timer controller because it has not been initialized";
-
-pub static TIMER_CONTROLLER: Once<RwLock<TimerController>> = Once::new();
-
-pub struct TimerController {}
-
-impl<'a> TimerController {
-    pub fn initialize() -> Result<(), Error> {
-        let controller = Self::new()?;
-        ensure_not!(TIMER_CONTROLLER.is_completed(), Error::Reinitialization())?;
-        TIMER_CONTROLLER.call_once(|| RwLock::new(controller));
-        Ok(())
-    }
+pub struct TimerController<'a, 'b> {
+    current_time: usize,
+    confidential_flow: &'a mut ConfidentialFlow<'b>,
+}
 
-    fn new() -> Result<Self, Error> {
-        Ok(Self {})
+impl<'a, 'b> TimerController<'a, 'b> {
+    pub fn new(confidential_flow: &'a mut ConfidentialFlow<'b>) -> Self {
+        Self { current_time: CSR.time.read(), confidential_flow }
     }
 
-    pub fn set_next_event_for_vs_mode(&mut self, confidential_flow: &mut ConfidentialFlow, ncycle: usize) {
-        confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp = ncycle;
-        if self.is_vs_timer_running(confidential_flow) {
-            self.set_vs_timer(confidential_flow, ncycle);
+    pub fn set_next_event_for_vs_mode(&mut self, next_event: usize) {
+        if next_event >= usize::MAX - 1 {
+            self.confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp = None;
+        } else {
+            self.confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp = Some(next_event);
+            if self.should_set_vs_timer() {
+                self.set_vs_timer();
+            }
         }
     }
 
-    fn is_vs_timer_programmed(&mut self, confidential_flow: &mut ConfidentialFlow) -> bool {
-        let vstimecmp = confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp;
-        vstimecmp > 0 && vstimecmp < usize::MAX - 1
+    fn is_vs_timer_programmed(&mut self) -> bool {
+        self.confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp.is_some()
     }
 
-    fn is_vs_timer_running(&mut self, confidential_flow: &mut ConfidentialFlow) -> bool {
-        let stimecmp = confidential_flow.confidential_hart_mut().csrs_mut().stimecmp;
-        let vstimecmp = confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp;
-        vstimecmp > 0 && vstimecmp <= stimecmp
+    fn should_set_vs_timer(&mut self) -> bool {
+        let stimecmp = self.confidential_flow.confidential_hart_mut().csrs_mut().stimecmp;
+        self.confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp.and_then(|v| Some(v <= stimecmp)).unwrap_or(false)
     }
 
-    pub fn s_timer_interrupted(&mut self, confidential_flow: &mut ConfidentialFlow, mtime: usize) -> bool {
-        let stimecmp = confidential_flow.confidential_hart_mut().csrs_mut().stimecmp;
-        mtime >= stimecmp
+    pub fn s_timer_interrupted(&mut self) -> bool {
+        self.current_time >= self.confidential_flow.confidential_hart_mut().csrs_mut().stimecmp
     }
 
-    pub fn vs_timer_interrupted(&mut self, confidential_flow: &mut ConfidentialFlow, mtime: usize) -> bool {
-        let vstimecmp = confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp;
-        vstimecmp > 0 && mtime >= vstimecmp
+    pub fn vs_timer_interrupted(&mut self) -> bool {
+        self.confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp.and_then(|v| Some(self.current_time >= v)).unwrap_or(false)
     }
 
-    pub fn handle_vs_interrupt(&mut self, confidential_flow: &mut ConfidentialFlow) {
-        // debug!("Inject VSTIP");
-        confidential_flow.confidential_hart_mut().csrs_mut().vstip |= MIE_VSTIP_MASK;
-        // confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp = usize::MAX - 1;
+    pub fn handle_vs_interrupt(&mut self) {
+        self.confidential_flow.confidential_hart_mut().csrs_mut().pending_interrupts |= MIE_VSTIP_MASK;
     }
 
-    pub fn store_vs_timer(&mut self, confidential_flow: &mut ConfidentialFlow) {
-        let vstimecmp = confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp;
-        let stimecmp = confidential_flow.confidential_hart_mut().csrs_mut().stimecmp;
-        let mtime = self.read_mtime();
-
-        // debug!("store_vs_timer mtime={:x} vstimecmp={:x} stimecmp={:x}", mtime, vstimecmp, stimecmp);
-        if self.vs_timer_interrupted(confidential_flow, mtime) {
-            self.handle_vs_interrupt(confidential_flow);
-        }
-        self.set_s_timer(confidential_flow, stimecmp);
-
-        let cycles_left = if vstimecmp > mtime && vstimecmp < usize::MAX - 1 { vstimecmp - mtime } else { usize::MAX - 1 };
-        if cycles_left > 0 && cycles_left < usize::MAX - 1 {
-            // debug!("Cycles lesft {:x}", cycles_left);
+    pub fn store_vs_timer(&mut self) {
+        if self.vs_timer_interrupted() {
+            self.handle_vs_interrupt();
         }
-        confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp = cycles_left;
-    }
-
-    pub fn restore_vs_timer(&mut self, confidential_flow: &mut ConfidentialFlow) {
-        let mtime = self.read_mtime();
-        let old_timer = self.read_m_timer(confidential_flow);
-        confidential_flow.confidential_hart_mut().csrs_mut().stimecmp = old_timer + 50000;
-        let vstimecmp = confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp;
 
-        // timer not programmed then nothing to restore
-        if !self.is_vs_timer_programmed(confidential_flow) {
-            return;
+        if let Some(v) = self.confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp {
+            let cycles_left = if v > self.current_time { v - self.current_time } else { 0 };
+            self.confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp = Some(cycles_left);
         }
 
-        let new_timer = mtime + vstimecmp;
-        // debug!("restore_vs_timer mtime={:x} vstimecmp={:x} stimecmp={:x} new_timer={:x}", mtime, vstimecmp, old_timer, new_timer);
-        confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp = new_timer;
-
-        if self.vs_timer_interrupted(confidential_flow, mtime) {
-            self.handle_vs_interrupt(confidential_flow);
-        } else if self.is_vs_timer_running(confidential_flow) {
-            self.set_vs_timer(confidential_flow, new_timer);
+        self.set_s_timer();
+    }
+
+    pub fn restore_vs_timer(&mut self) {
+        // let current_time = self.current_time();
+        // let mtimer = self.read_m_timer(confidential_flow);
+        // debug!("{:x}", mtimer - current_time);
+        self.confidential_flow.confidential_hart_mut().csrs_mut().stimecmp = self.current_time + 80000;
+
+        if let Some(v) = self.confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp {
+            // debug!("left {:x}", v);
+            self.confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp = self.current_time.checked_add(v);
+
+            if self.vs_timer_interrupted() {
+                self.handle_vs_interrupt();
+                self.set_s_timer();
+            } else if self.should_set_vs_timer() {
+                self.set_vs_timer();
+            } else {
+                self.set_s_timer();
+            }
+        } else {
+            self.set_s_timer();
         }
     }
 
-    pub fn set_vs_timer(&self, confidential_flow: &mut ConfidentialFlow, ncycle: usize) {
-        // debug!("Restore VS timer {:x}", ncycle);
-        self.set_m_timer(confidential_flow, ncycle);
-        confidential_flow.confidential_hart_mut().csrs_mut().vstip &= !MIE_VSTIP_MASK;
-        confidential_flow.confidential_hart_mut().csrs_mut().mip.read_and_clear_bits(MIE_MTIP_MASK);
-    }
-
-    pub fn set_s_timer(&self, confidential_flow: &mut ConfidentialFlow, ncycle: usize) {
-        // debug!("Restore S timer {:x}", ncycle);
-        self.set_m_timer(confidential_flow, ncycle);
-        confidential_flow.confidential_hart_mut().csrs_mut().mip.read_and_clear_bits(MIE_MTIP_MASK);
-    }
-
-    pub fn read_mtime(&self) -> usize {
-        CSR.time.read()
-    }
-
-    fn read_m_timer(&self, confidential_flow: &mut ConfidentialFlow) -> usize {
-        confidential_flow.swap_mscratch();
+    fn read_m_timer(&mut self) -> usize {
+        self.confidential_flow.swap_mscratch();
         let m_timer = (unsafe { sbi_timer_value() }) as usize;
-        confidential_flow.swap_mscratch();
+        self.confidential_flow.swap_mscratch();
         m_timer
     }
 
-    fn set_m_timer(&self, confidential_flow: &mut ConfidentialFlow, ncycle: usize) {
-        confidential_flow.swap_mscratch();
-        unsafe { sbi_timer_event_start(ncycle as u64) };
-        confidential_flow.swap_mscratch();
+    fn set_m_timer(&mut self, next_event: usize) {
+        self.confidential_flow.swap_mscratch();
+        unsafe { sbi_timer_event_start(next_event as u64) };
+        self.confidential_flow.swap_mscratch();
     }
 
-    pub fn try_write<F, O>(op: O) -> Result<F, Error>
-    where O: FnOnce(&mut RwLockWriteGuard<'static, TimerController>) -> Result<F, Error> {
-        op(&mut TIMER_CONTROLLER.get().expect(NOT_INITIALIZED_TIMER_CONTROLLER).write())
+    pub fn set_s_timer(&mut self) {
+        let next_event = self.confidential_flow.confidential_hart_mut().csrs_mut().stimecmp;
+        self.set_m_timer(next_event);
+        self.confidential_flow.confidential_hart_mut().csrs_mut().mip.read_and_clear_bits(MIE_MTIP_MASK);
+    }
+
+    fn set_vs_timer(&mut self) {
+        if let Some(next_event) = self.confidential_flow.confidential_hart_mut().csrs_mut().vstimecmp {
+            self.set_m_timer(next_event);
+            self.confidential_flow.confidential_hart_mut().csrs_mut().pending_interrupts &= !MIE_VSTIP_MASK;
+            self.confidential_flow.confidential_hart_mut().csrs_mut().mip.read_and_clear_bits(MIE_MTIP_MASK);
+        }
     }
 }

security-monitor/src/non_confidential_flow/handlers/cove_host_extension/run_confidential_hart.rs

@@ -32,7 +32,7 @@ impl RunConfidentialHart {
         match non_confidential_flow.into_confidential_flow(self.confidential_vm_id, self.confidential_hart_id) {
             Ok((allowed_external_interrupts, mut confidential_flow)) => {
                 self.allowed_external_interrupts = allowed_external_interrupts;
-                TimerController::try_write(|controller| Ok(controller.restore_vs_timer(&mut confidential_flow))).unwrap();
+                TimerController::new(&mut confidential_flow).restore_vs_timer();
                 confidential_flow
                     .declassify_to_confidential_hart(DeclassifyToConfidentialVm::Resume(self))
                     .resume_confidential_hart_execution()
@@ -58,11 +58,6 @@ impl RunConfidentialHart {
 
         let new_hvip = self.hvip & self.allowed_external_interrupts;
 
-        use crate::core::architecture::specification::*;
-        // if new_hvip > 0 {
-        // debug!("hypervisor injects {:x}!", new_hvip);
-        // }
-
         confidential_hart.csrs_mut().allowed_external_interrupts = self.allowed_external_interrupts;
         confidential_hart.csrs_mut().hvip.save_value_in_main_memory(new_hvip);
     }