From 09d8474a2c1152c5db3f3805d2892e8deb746a94 Mon Sep 17 00:00:00 2001 From: Wojciech Ozga Date: Mon, 27 Jan 2025 05:48:26 -0600 Subject: [PATCH] refactoring timer controller Signed-off-by: Wojciech Ozga --- .../confidential_flow/finite_state_machine.rs | 5 +- .../interrupts/expose_enabled_interrupts.rs | 3 +- .../handlers/interrupts/handle_interrupt.rs | 40 ++--- .../confidential_flow/handlers/time/mod.rs | 2 +- .../riscv/control_status_registers.rs | 8 +- .../core/control_data/confidential_hart.rs | 3 +- .../src/core/initialization/mod.rs | 2 - .../src/core/timer_controller/mod.rs | 170 ++++++++---------- .../run_confidential_hart.rs | 7 +- 9 files changed, 99 insertions(+), 141 deletions(-) diff --git a/security-monitor/src/confidential_flow/finite_state_machine.rs b/security-monitor/src/confidential_flow/finite_state_machine.rs index 00d692d..d30121d 100644 --- a/security-monitor/src/confidential_flow/finite_state_machine.rs +++ b/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(); diff --git a/security-monitor/src/confidential_flow/handlers/interrupts/expose_enabled_interrupts.rs b/security-monitor/src/confidential_flow/handlers/interrupts/expose_enabled_interrupts.rs index 25f0506..fd29a6f 100644 --- a/security-monitor/src/confidential_flow/handlers/interrupts/expose_enabled_interrupts.rs +++ b/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(), */ } } diff --git a/security-monitor/src/confidential_flow/handlers/interrupts/handle_interrupt.rs b/security-monitor/src/confidential_flow/handlers/interrupts/handle_interrupt.rs index a5df16d..9dbdb72 100644 --- a/security-monitor/src/confidential_flow/handlers/interrupts/handle_interrupt.rs +++ b/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)) } } diff --git a/security-monitor/src/confidential_flow/handlers/time/mod.rs b/security-monitor/src/confidential_flow/handlers/time/mod.rs index 0679110..27495a7 100644 --- a/security-monitor/src/confidential_flow/handlers/time/mod.rs +++ b/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())) } } diff --git a/security-monitor/src/core/architecture/riscv/control_status_registers.rs b/security-monitor/src/core/architecture/riscv/control_status_registers.rs index 8b993fd..aa50722 100644 --- a/security-monitor/src/core/architecture/riscv/control_status_registers.rs +++ b/security-monitor/src/core/architecture/riscv/control_status_registers.rs @@ -67,8 +67,8 @@ pub struct ControlStatusRegisters { pub vsatp: ReadWriteRiscvCsr, // timer pub stimecmp: usize, - pub vstimecmp: usize, - pub vstip: usize, + pub vstimecmp: Option, + 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 diff --git a/security-monitor/src/core/control_data/confidential_hart.rs b/security-monitor/src/core/control_data/confidential_hart.rs index 2764fc1..25b884f 100644 --- a/security-monitor/src/core/control_data/confidential_hart.rs +++ b/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); diff --git a/security-monitor/src/core/initialization/mod.rs b/security-monitor/src/core/initialization/mod.rs index 171e9d3..6ef2b2e 100644 --- a/security-monitor/src/core/initialization/mod.rs +++ b/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()?; diff --git a/security-monitor/src/core/timer_controller/mod.rs b/security-monitor/src/core/timer_controller/mod.rs index f33aadf..34bc10f 100644 --- a/security-monitor/src/core/timer_controller/mod.rs +++ b/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> = 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 { - 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(op: O) -> Result - where O: FnOnce(&mut RwLockWriteGuard<'static, TimerController>) -> Result { - 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); + } } } diff --git a/security-monitor/src/non_confidential_flow/handlers/cove_host_extension/run_confidential_hart.rs b/security-monitor/src/non_confidential_flow/handlers/cove_host_extension/run_confidential_hart.rs index 0d5cafc..57e1ac3 100644 --- a/security-monitor/src/non_confidential_flow/handlers/cove_host_extension/run_confidential_hart.rs +++ b/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); }