Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kernel: run with interrupts enabled when possible #485

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions kernel/src/cpu/idt/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,3 +401,21 @@ global_asm!(
"#,
options(att_syntax)
);

#[repr(u32)]
#[derive(Debug, Clone, Copy)]
pub enum IdtEventType {
Unknown = 0,
External,
Software,
msft-jlange marked this conversation as resolved.
Show resolved Hide resolved
}

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),
}
}
}
1 change: 1 addition & 0 deletions kernel/src/cpu/idt/entry.S
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 15 additions & 6 deletions kernel/src/cpu/idt/svsm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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!");
msft-jlange marked this conversation as resolved.
Show resolved Hide resolved
}
msft-jlange marked this conversation as resolved.
Show resolved Hide resolved

if !user_mode(ctxt) {
panic!("Syscall handler called from kernel mode!");
}
Expand Down Expand Up @@ -275,4 +284,4 @@ global_asm!(
include_str!("entry.S"),
IF = const RFlags::IF.bits(),
options(att_syntax)
);
);
30 changes: 25 additions & 5 deletions kernel/src/cpu/irq_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
msft-jlange marked this conversation as resolved.
Show resolved Hide resolved
self.state.store(enabled, Ordering::Relaxed);
}
}

impl Drop for IrqState {
Expand Down Expand Up @@ -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());
Expand All @@ -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();
}
}
}
}
11 changes: 11 additions & 0 deletions kernel/src/cpu/percpu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 9 additions & 5 deletions kernel/src/locking/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -354,7 +354,9 @@ mod tests {

// IRQs must still be enabled
assert!(irqs_enabled());
raw_irqs_disable();
if !was_enabled {
raw_irqs_disable();
}
}
}

Expand All @@ -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);

Expand All @@ -388,7 +390,9 @@ mod tests {

// IRQs must still be enabled
assert!(irqs_enabled());
raw_irqs_disable();
if !was_enabled {
raw_irqs_disable();
}
}
}
}
23 changes: 13 additions & 10 deletions kernel/src/locking/spinlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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();
}
}
}
}
7 changes: 7 additions & 0 deletions kernel/src/platform/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
}
Expand Down
11 changes: 11 additions & 0 deletions kernel/src/platform/native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand All @@ -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!();
}
Expand Down
31 changes: 28 additions & 3 deletions kernel/src/platform/snp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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,
}
}
}

Expand All @@ -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(())
}

Expand Down Expand Up @@ -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(())
Expand All @@ -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)?;

Expand Down
Loading
Loading