Make interrupt retry delay configurable

Signed-off-by: Ludvig Liljenberg <[email protected]>

Author: ludfjig

src/hyperlight_host/src/hypervisor/hyperv_linux.rs

@@ -65,6 +65,7 @@ use super::{
 use crate::hypervisor::HyperlightExit;
 use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags};
 use crate::mem::ptr::{GuestPtr, RawPtr};
+use crate::sandbox::SandboxConfiguration;
 #[cfg(gdb)]
 use crate::HyperlightError;
 use crate::{log_then_return, new_error, Result};
@@ -318,6 +319,7 @@ impl HypervLinuxDriver {
         entrypoint_ptr: GuestPtr,
         rsp_ptr: GuestPtr,
         pml4_ptr: GuestPtr,
+        config: &SandboxConfiguration,
         #[cfg(gdb)] gdb_conn: Option<DebugCommChannel<DebugResponse, DebugMsg>>,
     ) -> Result<Self> {
         let mshv = Mshv::new()?;
@@ -397,6 +399,7 @@ impl HypervLinuxDriver {
                 running: AtomicBool::new(false),
                 cancel_requested: AtomicBool::new(false),
                 tid: AtomicU64::new(unsafe { libc::pthread_self() }),
+                retry_delay: config.get_interrupt_retry_delay(),
                 dropped: AtomicBool::new(false),
             }),
 
@@ -853,11 +856,14 @@ mod tests {
             MemoryRegionFlags::READ | MemoryRegionFlags::WRITE | MemoryRegionFlags::EXECUTE,
             crate::mem::memory_region::MemoryRegionType::Code,
         );
+        let config: SandboxConfiguration = Default::default();
+
         super::HypervLinuxDriver::new(
             regions.build(),
             entrypoint_ptr,
             rsp_ptr,
             pml4_ptr,
+            &config,
             #[cfg(gdb)]
             None,
         )

src/hyperlight_host/src/hypervisor/kvm.rs

@@ -40,6 +40,7 @@ use super::{
 };
 use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags};
 use crate::mem::ptr::{GuestPtr, RawPtr};
+use crate::sandbox::SandboxConfiguration;
 #[cfg(gdb)]
 use crate::HyperlightError;
 use crate::{log_then_return, new_error, Result};
@@ -301,6 +302,7 @@ impl KVMDriver {
         pml4_addr: u64,
         entrypoint: u64,
         rsp: u64,
+        config: &SandboxConfiguration,
         #[cfg(gdb)] gdb_conn: Option<DebugCommChannel<DebugResponse, DebugMsg>>,
     ) -> Result<Self> {
         let kvm = Kvm::new()?;
@@ -352,6 +354,7 @@ impl KVMDriver {
                 running: AtomicBool::new(false),
                 cancel_requested: AtomicBool::new(false),
                 tid: AtomicU64::new(unsafe { libc::pthread_self() }),
+                retry_delay: config.get_interrupt_retry_delay(),
                 dropped: AtomicBool::new(false),
             }),
 

src/hyperlight_host/src/hypervisor/mod.rs

@@ -64,6 +64,8 @@ use std::str::FromStr;
 #[cfg(any(kvm, mshv))]
 use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
 use std::sync::{Arc, Mutex};
+#[cfg(any(kvm, mshv))]
+use std::time::Duration;
 
 #[cfg(gdb)]
 use gdb::VcpuStopReason;
@@ -353,6 +355,8 @@ pub(super) struct LinuxInterruptHandle {
     cancel_requested: AtomicBool,
     /// Whether the corresponding vm is dropped
     dropped: AtomicBool,
+    /// Retry delay between signals sent to the vcpu thread
+    retry_delay: Duration,
 }
 
 #[cfg(any(kvm, mshv))]
@@ -369,7 +373,7 @@ impl InterruptHandle for LinuxInterruptHandle {
             unsafe {
                 libc::pthread_kill(self.tid.load(Ordering::Relaxed) as _, signal_number);
             }
-            std::thread::sleep(std::time::Duration::from_micros(50));
+            std::thread::sleep(self.retry_delay);
         }
 
         sent_signal
@@ -391,7 +395,7 @@ pub(crate) mod tests {
     use crate::mem::ptr::RawPtr;
     use crate::sandbox::uninitialized::GuestBinary;
     use crate::sandbox::uninitialized_evolve::set_up_hypervisor_partition;
-    use crate::sandbox::UninitializedSandbox;
+    use crate::sandbox::{SandboxConfiguration, UninitializedSandbox};
     use crate::{is_hypervisor_present, new_error, Result};
 
     #[cfg(gdb)]
@@ -432,13 +436,11 @@ pub(crate) mod tests {
 
         let filename = dummy_guest_as_string().map_err(|e| new_error!("{}", e))?;
 
-        let sandbox = UninitializedSandbox::new(GuestBinary::FilePath(filename.clone()), None)?;
+        let config: SandboxConfiguration = Default::default();
+        let sandbox =
+            UninitializedSandbox::new(GuestBinary::FilePath(filename.clone()), Some(config))?;
         let (_hshm, mut gshm) = sandbox.mgr.build();
-        let mut vm = set_up_hypervisor_partition(
-            &mut gshm,
-            #[cfg(gdb)]
-            &sandbox.debug_info,
-        )?;
+        let mut vm = set_up_hypervisor_partition(&mut gshm, &config)?;
         vm.initialise(
             RawPtr::from(0x230000),
             1234567890,

src/hyperlight_host/src/sandbox/config.rs

@@ -15,6 +15,7 @@ limitations under the License.
 */
 
 use std::cmp::max;
+use std::time::Duration;
 
 use tracing::{instrument, Span};
 
@@ -55,6 +56,13 @@ pub struct SandboxConfiguration {
     /// field should be represented as an `Option`, that type is not
     /// FFI-safe, so it cannot be.
     heap_size_override: u64,
+    /// Delay between interrupt retries. This duration specifies how long to wait
+    /// between attempts to send signals to the thread running the sandbox's VCPU.
+    /// Multiple retries may be necessary because signals only interrupt the VCPU
+    /// thread when the vcpu thread is in kernel space. There's a narrow window during which a
+    /// signal can be delivered to the thread, but the thread may not yet
+    /// have entered kernel space.
+    interrupt_retry_delay: Duration,
 }
 
 impl SandboxConfiguration {
@@ -66,6 +74,8 @@ impl SandboxConfiguration {
     pub const DEFAULT_OUTPUT_SIZE: usize = 0x4000;
     /// The minimum size of output data
     pub const MIN_OUTPUT_SIZE: usize = 0x2000;
+    /// The default interrupt retry delay
+    pub const DEFAULT_INTERRUPT_RETRY_DELAY: Duration = Duration::from_micros(500);
 
     #[allow(clippy::too_many_arguments)]
     /// Create a new configuration for a sandbox with the given sizes.
@@ -75,13 +85,15 @@ impl SandboxConfiguration {
         output_data_size: usize,
         stack_size_override: Option<u64>,
         heap_size_override: Option<u64>,
+        interrupt_retry_delay: Duration,
         #[cfg(gdb)] guest_debug_info: Option<DebugInfo>,
     ) -> Self {
         Self {
             input_data_size: max(input_data_size, Self::MIN_INPUT_SIZE),
             output_data_size: max(output_data_size, Self::MIN_OUTPUT_SIZE),
             stack_size_override: stack_size_override.unwrap_or(0),
             heap_size_override: heap_size_override.unwrap_or(0),
+            interrupt_retry_delay,
 
             #[cfg(gdb)]
             guest_debug_info,
@@ -114,6 +126,16 @@ impl SandboxConfiguration {
         self.heap_size_override = heap_size;
     }
 
+    /// Sets the interrupt retry delay
+    pub fn set_interrupt_retry_delay(&mut self, delay: Duration) {
+        self.interrupt_retry_delay = delay;
+    }
+
+    /// Get the delay between retries for interrupts
+    pub fn get_interrupt_retry_delay(&self) -> Duration {
+        self.interrupt_retry_delay
+    }
+
     /// Sets the configuration for the guest debug
     #[cfg(gdb)]
     #[instrument(skip_all, parent = Span::current(), level= "Trace")]
@@ -172,6 +194,7 @@ impl Default for SandboxConfiguration {
             Self::DEFAULT_OUTPUT_SIZE,
             None,
             None,
+            Self::DEFAULT_INTERRUPT_RETRY_DELAY,
             #[cfg(gdb)]
             None,
         )
@@ -194,6 +217,7 @@ mod tests {
             OUTPUT_DATA_SIZE_OVERRIDE,
             Some(STACK_SIZE_OVERRIDE),
             Some(HEAP_SIZE_OVERRIDE),
+            SandboxConfiguration::DEFAULT_INTERRUPT_RETRY_DELAY,
             #[cfg(gdb)]
             None,
         );
@@ -219,6 +243,7 @@ mod tests {
             SandboxConfiguration::MIN_OUTPUT_SIZE - 1,
             None,
             None,
+            SandboxConfiguration::DEFAULT_INTERRUPT_RETRY_DELAY,
             #[cfg(gdb)]
             None,
         );

src/hyperlight_host/src/sandbox/uninitialized.rs

@@ -22,8 +22,6 @@ use std::sync::{Arc, Mutex};
 use log::LevelFilter;
 use tracing::{instrument, Span};
 
-#[cfg(gdb)]
-use super::config::DebugInfo;
 use super::host_funcs::{default_writer_func, FunctionRegistry};
 use super::mem_mgr::MemMgrWrapper;
 use super::uninitialized_evolve::evolve_impl_multi_use;
@@ -67,8 +65,7 @@ pub struct UninitializedSandbox {
     /// The memory manager for the sandbox.
     pub(crate) mgr: MemMgrWrapper<ExclusiveSharedMemory>,
     pub(crate) max_guest_log_level: Option<LevelFilter>,
-    #[cfg(gdb)]
-    pub(crate) debug_info: Option<DebugInfo>,
+    pub(crate) config: SandboxConfiguration,
 }
 
 impl crate::sandbox_state::sandbox::UninitializedSandbox for UninitializedSandbox {
@@ -160,8 +157,6 @@ impl UninitializedSandbox {
 
         let sandbox_cfg = cfg.unwrap_or_default();
 
-        #[cfg(gdb)]
-        let debug_info = sandbox_cfg.get_guest_debug_info();
         let mut mem_mgr_wrapper = {
             let mut mgr = UninitializedSandbox::load_guest_binary(sandbox_cfg, &guest_binary)?;
             let stack_guard = Self::create_stack_guard();
@@ -177,8 +172,7 @@ impl UninitializedSandbox {
             host_funcs,
             mgr: mem_mgr_wrapper,
             max_guest_log_level: None,
-            #[cfg(gdb)]
-            debug_info,
+            config: sandbox_cfg,
         };
 
         // If we were passed a writer for host print register it otherwise use the default.

src/hyperlight_host/src/sandbox/uninitialized_evolve.rs

@@ -22,6 +22,7 @@ use tracing::{instrument, Span};
 use super::hypervisor::{get_available_hypervisor, HypervisorType};
 #[cfg(gdb)]
 use super::mem_access::dbg_mem_access_handler_wrapper;
+use super::SandboxConfiguration;
 use crate::hypervisor::handlers::{MemAccessHandlerCaller, OutBHandlerCaller};
 use crate::hypervisor::Hypervisor;
 use crate::mem::layout::SandboxMemoryLayout;
@@ -68,11 +69,7 @@ where
     ) -> Result<ResSandbox>,
 {
     let (hshm, mut gshm) = u_sbox.mgr.build();
-    let mut vm = set_up_hypervisor_partition(
-        &mut gshm,
-        #[cfg(gdb)]
-        &u_sbox.debug_info,
-    )?;
+    let mut vm = set_up_hypervisor_partition(&mut gshm, &u_sbox.config)?;
     let outb_hdl = outb_handler_wrapper(hshm.clone(), u_sbox.host_funcs.clone());
 
     let seed = {
@@ -143,7 +140,7 @@ pub(super) fn evolve_impl_multi_use(u_sbox: UninitializedSandbox) -> Result<Mult
 
 pub(crate) fn set_up_hypervisor_partition(
     mgr: &mut SandboxMemoryManager<GuestSharedMemory>,
-    #[cfg(gdb)] debug_info: &Option<DebugInfo>,
+    #[cfg_attr(target_os = "windows", allow(unused_variables))] config: &SandboxConfiguration,
 ) -> Result<Box<dyn Hypervisor>> {
     let mem_size = u64::try_from(mgr.shared_mem.mem_size())?;
     let mut regions = mgr.layout.get_memory_regions(&mgr.shared_mem)?;
@@ -179,10 +176,10 @@ pub(crate) fn set_up_hypervisor_partition(
 
     // Create gdb thread if gdb is enabled and the configuration is provided
     #[cfg(gdb)]
-    let gdb_conn = if let Some(DebugInfo { port }) = debug_info {
+    let gdb_conn = if let Some(DebugInfo { port }) = config.get_guest_debug_info() {
         use crate::hypervisor::gdb::create_gdb_thread;
 
-        let gdb_conn = create_gdb_thread(*port, unsafe { libc::pthread_self() });
+        let gdb_conn = create_gdb_thread(port, unsafe { libc::pthread_self() });
 
         // in case the gdb thread creation fails, we still want to continue
         // without gdb
@@ -206,6 +203,7 @@ pub(crate) fn set_up_hypervisor_partition(
                 entrypoint_ptr,
                 rsp_ptr,
                 pml4_ptr,
+                config,
                 #[cfg(gdb)]
                 gdb_conn,
             )?;
@@ -219,6 +217,7 @@ pub(crate) fn set_up_hypervisor_partition(
                 pml4_ptr.absolute()?,
                 entrypoint_ptr.absolute()?,
                 rsp_ptr.absolute()?,
+                config,
                 #[cfg(gdb)]
                 gdb_conn,
             )?;