Merge pull request #485 from msft-jlange/use_irq

kernel: run with interrupts enabled when possible

kernel/src/cpu/idt/common.rs

@@ -401,3 +401,21 @@ global_asm!(
         "#,
     options(att_syntax)
 );
+
+#[repr(u32)]
+#[derive(Debug, Clone, Copy)]
+pub enum IdtEventType {
+    Unknown = 0,
+    External,
+    Software,
+}
+
+impl IdtEventType {
+    pub fn is_external_interrupt(&self, vector: usize) -> bool {
+        match self {
+            Self::External => true,
+            Self::Software => false,
+            Self::Unknown => SVSM_PLATFORM.as_dyn_ref().is_external_interrupt(vector),
+        }
+    }
+}

kernel/src/cpu/idt/entry.S

@@ -42,6 +42,7 @@ asm_entry_\name:
 	push_regs
 	movl	$\vector, %esi
 	movq	%rsp, %rdi
+	xorl	%edx, %edx
 	call	ex_handler_\handler
 	jmp	default_return
 .endm

kernel/src/cpu/idt/svsm.rs

@@ -10,10 +10,10 @@ use super::super::percpu::{current_task, this_cpu};
 use super::super::tss::IST_DF;
 use super::super::vc::handle_vc_exception;
 use super::common::{
-    idt_mut, user_mode, IdtEntry, PageFaultError, AC_VECTOR, BP_VECTOR, BR_VECTOR, CP_VECTOR,
-    DB_VECTOR, DE_VECTOR, DF_VECTOR, GP_VECTOR, HV_VECTOR, INT_INJ_VECTOR, MCE_VECTOR, MF_VECTOR,
-    NMI_VECTOR, NM_VECTOR, NP_VECTOR, OF_VECTOR, PF_VECTOR, SS_VECTOR, SX_VECTOR, TS_VECTOR,
-    UD_VECTOR, VC_VECTOR, XF_VECTOR,
+    idt_mut, user_mode, IdtEntry, IdtEventType, PageFaultError, AC_VECTOR, BP_VECTOR, BR_VECTOR,
+    CP_VECTOR, DB_VECTOR, DE_VECTOR, DF_VECTOR, GP_VECTOR, HV_VECTOR, INT_INJ_VECTOR, MCE_VECTOR,
+    MF_VECTOR, NMI_VECTOR, NM_VECTOR, NP_VECTOR, OF_VECTOR, PF_VECTOR, SS_VECTOR, SX_VECTOR,
+    TS_VECTOR, UD_VECTOR, VC_VECTOR, XF_VECTOR,
 };
 use crate::address::VirtAddr;
 use crate::cpu::registers::RFlags;
@@ -227,7 +227,16 @@ extern "C" fn ex_handler_vmm_communication(ctxt: &mut X86ExceptionContext, vecto
 
 // System Call SoftIRQ handler
 #[no_mangle]
-extern "C" fn ex_handler_system_call(ctxt: &mut X86ExceptionContext) {
+extern "C" fn ex_handler_system_call(
+    ctxt: &mut X86ExceptionContext,
+    vector: usize,
+    event_type: IdtEventType,
+) {
+    // Ensure that this vector was not invoked as a hardware interrupt vector.
+    if event_type.is_external_interrupt(vector) {
+        panic!("Syscall handler invoked as external interrupt!");
+    }
+
     if !user_mode(ctxt) {
         panic!("Syscall handler called from kernel mode!");
     }

kernel/src/cpu/irq_state.rs

@@ -152,6 +152,19 @@ impl IrqState {
     pub fn count(&self) -> isize {
         self.count.load(Ordering::Relaxed)
     }
+
+    /// Changes whether interrupts will be enabled when the nesting count
+    /// drops to zero.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that the current nesting count is non-zero,
+    /// and must ensure that the specified value is appropriate for the
+    /// current environment.
+    pub unsafe fn set_restore_state(&self, enabled: bool) {
+        assert!(self.count.load(Ordering::Relaxed) != 0);
+        self.state.store(enabled, Ordering::Relaxed);
+    }
 }
 
 impl Drop for IrqState {
@@ -212,21 +225,24 @@ mod tests {
     #[test]
     #[cfg_attr(not(test_in_svsm), ignore = "Can only be run inside guest")]
     fn irq_enable_disable() {
-        assert!(irqs_disabled());
         unsafe {
+            let was_enabled = irqs_enabled();
             raw_irqs_enable();
             assert!(irqs_enabled());
             raw_irqs_disable();
             assert!(irqs_disabled());
+            if was_enabled {
+                raw_irqs_enable();
+            }
         }
     }
 
     #[test]
     #[cfg_attr(not(test_in_svsm), ignore = "Can only be run inside guest")]
     fn irq_state() {
-        assert!(irqs_disabled());
         unsafe {
             let state = IrqState::new();
+            let was_enabled = irqs_enabled();
             raw_irqs_enable();
             state.disable();
             assert!(irqs_disabled());
@@ -235,22 +251,26 @@ mod tests {
             assert!(irqs_disabled());
             state.enable();
             assert!(irqs_enabled());
-            raw_irqs_disable();
+            if !was_enabled {
+                raw_irqs_disable();
+            }
         }
     }
 
     #[test]
     #[cfg_attr(not(test_in_svsm), ignore = "Can only be run inside guest")]
     fn irq_guard_test() {
-        assert!(irqs_disabled());
         unsafe {
+            let was_enabled = irqs_enabled();
             raw_irqs_enable();
             assert!(irqs_enabled());
             let g1 = IrqGuard::new();
             assert!(irqs_disabled());
             drop(g1);
             assert!(irqs_enabled());
-            raw_irqs_disable();
+            if !was_enabled {
+                raw_irqs_disable();
+            }
         }
     }
 }

kernel/src/cpu/percpu.rs

@@ -825,6 +825,17 @@ impl PerCpu {
     }
 
     pub fn schedule_init(&self) -> TaskPointer {
+        // If the platform permits the use of interrupts, then ensure that
+        // interrupts will be enabled on the current CPU when leaving the
+        // scheduler environment.  This is done after disabling interrupts
+        // for scheduler initialization so that the first interrupt that can
+        // be received will always observe that there is a current task and
+        // not the boot thread.
+        if SVSM_PLATFORM.as_dyn_ref().use_interrupts() {
+            unsafe {
+                self.irq_state.set_restore_state(true);
+            }
+        }
         let task = self.runqueue.lock_write().schedule_init();
         self.current_stack.set(task.stack_bounds());
         task

kernel/src/locking/rwlock.rs

@@ -330,11 +330,11 @@ mod tests {
     #[cfg_attr(not(test_in_svsm), ignore = "Can only be run inside guest")]
     fn rw_lock_irq_unsafe() {
         use crate::cpu::irq_state::{raw_irqs_disable, raw_irqs_enable};
-        use crate::cpu::{irqs_disabled, irqs_enabled};
+        use crate::cpu::irqs_enabled;
         use crate::locking::*;
 
-        assert!(irqs_disabled());
         unsafe {
+            let was_enabled = irqs_enabled();
             raw_irqs_enable();
             let lock = RWLock::new(0);
 
@@ -354,7 +354,9 @@ mod tests {
 
             // IRQs must still be enabled
             assert!(irqs_enabled());
-            raw_irqs_disable();
+            if !was_enabled {
+                raw_irqs_disable();
+            }
         }
     }
 
@@ -365,8 +367,8 @@ mod tests {
         use crate::cpu::{irqs_disabled, irqs_enabled};
         use crate::locking::*;
 
-        assert!(irqs_disabled());
         unsafe {
+            let was_enabled = irqs_enabled();
             raw_irqs_enable();
             let lock = RWLockIrqSafe::new(0);
 
@@ -388,7 +390,9 @@ mod tests {
 
             // IRQs must still be enabled
             assert!(irqs_enabled());
-            raw_irqs_disable();
+            if !was_enabled {
+                raw_irqs_disable();
+            }
         }
     }
 }

kernel/src/locking/spinlock.rs

@@ -224,9 +224,8 @@ mod tests {
     #[test]
     #[cfg_attr(not(test_in_svsm), ignore = "Can only be run inside guest")]
     fn spin_lock_irq_unsafe() {
-        assert!(irqs_disabled());
-
         unsafe {
+            let was_enabled = irqs_enabled();
             raw_irqs_enable();
 
             let spin_lock = SpinLock::new(0);
@@ -235,16 +234,17 @@ mod tests {
             drop(guard);
             assert!(irqs_enabled());
 
-            raw_irqs_disable();
+            if !was_enabled {
+                raw_irqs_disable();
+            }
         }
     }
 
     #[test]
     #[cfg_attr(not(test_in_svsm), ignore = "Can only be run inside guest")]
     fn spin_lock_irq_safe() {
-        assert!(irqs_disabled());
-
         unsafe {
+            let was_enabled = irqs_enabled();
             raw_irqs_enable();
 
             let spin_lock = SpinLockIrqSafe::new(0);
@@ -253,16 +253,17 @@ mod tests {
             drop(guard);
             assert!(irqs_enabled());
 
-            raw_irqs_disable();
+            if !was_enabled {
+                raw_irqs_disable();
+            }
         }
     }
 
     #[test]
     #[cfg_attr(not(test_in_svsm), ignore = "Can only be run inside guest")]
     fn spin_trylock_irq_safe() {
-        assert!(irqs_disabled());
-
         unsafe {
+            let was_enabled = irqs_enabled();
             raw_irqs_enable();
 
             let spin_lock = SpinLockIrqSafe::new(0);
@@ -276,8 +277,10 @@ mod tests {
             drop(g1);
             assert!(irqs_enabled());
 
-            // Leave with IRQs disabled, as test was entered.
-            raw_irqs_disable();
+            // Leave with IRQs configured as test was entered.
+            if !was_enabled {
+                raw_irqs_disable();
+            }
         }
     }
 }

kernel/src/platform/mod.rs

@@ -115,12 +115,19 @@ pub trait SvsmPlatform {
     /// Queries the state of APIC registration on this system.
     fn query_apic_registration_state(&self) -> bool;
 
+    /// Determines whether the platform supports interrupts to the SVSM.
+    fn use_interrupts(&self) -> bool;
+
     /// Signal an IRQ on one or more CPUs.
     fn post_irq(&self, icr: u64) -> Result<(), SvsmError>;
 
     /// Perform an EOI of the current interrupt.
     fn eoi(&self);
 
+    /// Determines whether a given interrupt vector was invoked as an external
+    /// interrupt.
+    fn is_external_interrupt(&self, vector: usize) -> bool;
+
     /// Start an additional processor.
     fn start_cpu(&self, cpu: &PerCpu, start_rip: u64) -> Result<(), SvsmError>;
 }

kernel/src/platform/native.rs

@@ -128,6 +128,10 @@ impl SvsmPlatform for NativePlatform {
         false
     }
 
+    fn use_interrupts(&self) -> bool {
+        true
+    }
+
     fn post_irq(&self, icr: u64) -> Result<(), SvsmError> {
         write_msr(APIC_MSR_ICR, icr);
         Ok(())
@@ -137,6 +141,13 @@ impl SvsmPlatform for NativePlatform {
         todo!();
     }
 
+    fn is_external_interrupt(&self, _vector: usize) -> bool {
+        // For a native platform, the hypervisor is fully trusted with all
+        // event delivery, so all events are assumed not to be external
+        // interrupts.
+        false
+    }
+
     fn start_cpu(&self, _cpu: &PerCpu, _start_rip: u64) -> Result<(), SvsmError> {
         todo!();
     }

kernel/src/platform/snp.rs

@@ -19,7 +19,7 @@ use crate::sev::hv_doorbell::current_hv_doorbell;
 use crate::sev::msr_protocol::{
     hypervisor_ghcb_features, request_termination_msr, verify_ghcb_version, GHCBHvFeatures,
 };
-use crate::sev::status::vtom_enabled;
+use crate::sev::status::{sev_restricted_injection, vtom_enabled};
 use crate::sev::{
     init_hypervisor_ghcb_features, pvalidate_range, sev_status_init, sev_status_verify, PvalidateOp,
 };
@@ -69,11 +69,15 @@ impl From<PageValidateOp> for PvalidateOp {
 }
 
 #[derive(Clone, Copy, Debug)]
-pub struct SnpPlatform {}
+pub struct SnpPlatform {
+    can_use_interrupts: bool,
+}
 
 impl SnpPlatform {
     pub fn new() -> Self {
-        Self {}
+        Self {
+            can_use_interrupts: false,
+        }
     }
 }
 
@@ -87,6 +91,14 @@ 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)?;
+
+        // Now that SEV status is initialized, determine whether this platform
+        // supports the use of SVSM interrupts.  SVSM interrupts are supported
+        // if this system uses restricted injection.
+        if sev_restricted_injection() {
+            self.can_use_interrupts = true;
+        }
+
         Ok(())
     }
 
@@ -255,6 +267,10 @@ impl SvsmPlatform for SnpPlatform {
         APIC_EMULATION_REG_COUNT.load(Ordering::Relaxed) > 0
     }
 
+    fn use_interrupts(&self) -> bool {
+        self.can_use_interrupts
+    }
+
     fn post_irq(&self, icr: u64) -> Result<(), SvsmError> {
         current_ghcb().hv_ipi(icr)?;
         Ok(())
@@ -270,6 +286,15 @@ impl SvsmPlatform for SnpPlatform {
         }
     }
 
+    fn is_external_interrupt(&self, _vector: usize) -> bool {
+        // When restricted injection is active, the event disposition is
+        // already known to the caller and thus need not be examined.  When
+        // restricted injection is not active, the hypervisor must be trusted
+        // with all event delivery, so all events are assumed not to be
+        // external interrupts.
+        false
+    }
+
     fn start_cpu(&self, cpu: &PerCpu, start_rip: u64) -> Result<(), SvsmError> {
         let (vmsa_pa, sev_features) = cpu.alloc_svsm_vmsa(*VTOM as u64, start_rip)?;